diff --git a/go.mod b/go.mod index 83d82e3af5d..fa38e9ec99d 100644 --- a/go.mod +++ b/go.mod @@ -154,6 +154,7 @@ require ( github.com/openzipkin/zipkin-go v0.4.3 // @grafana/oss-big-tent github.com/patrickmn/go-cache v2.1.0+incompatible // @grafana/alerting-backend github.com/phpdave11/gofpdi v1.0.14 // @grafana/sharing-squad + github.com/pressly/goose/v3 v3.26.0 // @grafana/identity-access-team github.com/prometheus/alertmanager v0.28.2 // @grafana/alerting-backend github.com/prometheus/client_golang v1.23.2 // @grafana/alerting-backend github.com/prometheus/client_model v0.6.2 // @grafana/grafana-backend-group @@ -557,7 +558,6 @@ require ( github.com/pkg/errors v0.9.1 // indirect github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect - github.com/pressly/goose/v3 v3.26.0 // indirect github.com/prometheus/common/sigv4 v0.1.0 // indirect github.com/prometheus/exporter-toolkit v0.14.0 // indirect github.com/prometheus/procfs v0.19.2 // indirect @@ -681,6 +681,7 @@ require ( github.com/go-openapi/swag/stringutils v0.25.4 // indirect github.com/go-openapi/swag/typeutils v0.25.4 // indirect github.com/go-openapi/swag/yamlutils v0.25.4 // indirect + github.com/gophercloud/gophercloud/v2 v2.9.0 // indirect github.com/lufia/plan9stats v0.0.0-20240909124753-873cd0166683 // indirect github.com/magiconair/properties v1.8.10 // indirect github.com/moby/go-archive v0.1.0 // indirect diff --git a/go.sum b/go.sum index 2b3b2cb4e3f..7d2582cf711 100644 --- a/go.sum +++ b/go.sum @@ -1607,9 +1607,8 @@ github.com/googleapis/gnostic v0.3.0/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTV github.com/googleapis/go-type-adapters v1.0.0/go.mod h1:zHW75FOG2aur7gAO2B+MLby+cLsWGBF62rFAi7WjWO4= github.com/googleapis/google-cloud-go-testing v0.0.0-20200911160855-bcd43fbb19e8/go.mod h1:dvDLG8qkwmyD9a/MJJN3XJcT3xFxOKAvTZGvuZmac9g= github.com/gophercloud/gophercloud v0.3.0/go.mod h1:vxM41WHh5uqHVBMZHzuwNOHh8XEoIEcSTewFxm1c5g8= -github.com/gophercloud/gophercloud v1.13.0 h1:8iY9d1DAbzMW6Vok1AxbbK5ZaUjzMp0tdyt4fX9IeJ0= -github.com/gophercloud/gophercloud/v2 v2.6.0 h1:XJKQ0in3iHOZHVAFMXq/OhjCuvvG+BKR0unOqRfG1EI= -github.com/gophercloud/gophercloud/v2 v2.6.0/go.mod h1:Ki/ILhYZr/5EPebrPL9Ej+tUg4lqx71/YH2JWVeU+Qk= +github.com/gophercloud/gophercloud/v2 v2.9.0 h1:Y9OMrwKF9EDERcHFSOTpf/6XGoAI0yOxmsLmQki4LPM= +github.com/gophercloud/gophercloud/v2 v2.9.0/go.mod h1:Ki/ILhYZr/5EPebrPL9Ej+tUg4lqx71/YH2JWVeU+Qk= github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= github.com/gopherjs/gopherjs v1.17.2 h1:fQnZVsXk8uxXIStYb0N4bGk7jeyTalG/wsZjQ25dO0g= github.com/gopherjs/gopherjs v1.17.2/go.mod h1:pRRIvn/QzFLrKfvEz3qUuEhtE/zLCWfreZ6J5gM2i+k= diff --git a/go.work.sum b/go.work.sum index ca22b546c86..f676971746a 100644 --- a/go.work.sum +++ b/go.work.sum @@ -533,12 +533,12 @@ github.com/campoy/embedmd v1.0.0 h1:V4kI2qTJJLf4J29RzI/MAt2c3Bl4dQSYPuflzwFH2hY= github.com/campoy/embedmd v1.0.0/go.mod h1:oxyr9RCiSXg0M3VJ3ks0UGfp98BpSSGr0kpiX3MzVl8= github.com/cenkalti/backoff/v5 v5.0.2/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/census-instrumentation/opencensus-proto v0.4.1 h1:iKLQ0xPNFxR/2hzXZMrBo8f1j86j5WHzznCCQxV/b8g= +github.com/centrifugal/centrifuge v0.37.2/go.mod h1:aj4iRJGhzi3SlL8iUtVezxway1Xf8g+hmNQkLLO7sS8= +github.com/centrifugal/protocol v0.16.2/go.mod h1:Q7OpS/8HMXDnL7f9DpNx24IhG96MP88WPpVTTCdrokI= github.com/charmbracelet/harmonica v0.2.0 h1:8NxJWRWg/bzKqqEaaeFNipOu77YR5t8aSwG4pgaUBiQ= github.com/charmbracelet/harmonica v0.2.0/go.mod h1:KSri/1RMQOZLbw7AHqgcBycp8pgJnQMYYT8QZRqZ1Ao= github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91 h1:payRxjMjKgx2PaCWLZ4p3ro9y97+TVLZNaRZgJwSVDQ= github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U= -github.com/centrifugal/centrifuge v0.37.2/go.mod h1:aj4iRJGhzi3SlL8iUtVezxway1Xf8g+hmNQkLLO7sS8= -github.com/centrifugal/protocol v0.16.2/go.mod h1:Q7OpS/8HMXDnL7f9DpNx24IhG96MP88WPpVTTCdrokI= github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d h1:77cEq6EriyTZ0g/qfRdp61a3Uu/AWrgIq2s0ClJV1g0= github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d/go.mod h1:8EPpVsBuRksnlj1mLy4AWzRNQYxauNi62uWcE3to6eA= github.com/chenzhuoyu/iasm v0.9.0 h1:9fhXjVzq5hUy2gkhhgHl95zG2cEAhw9OSGs8toWWAwo= @@ -875,6 +875,7 @@ github.com/gookit/color v1.4.2/go.mod h1:fqRyamkC1W8uxl+lxCQxOT09l/vYfZ+QeiX3rKQ github.com/gookit/color v1.5.0/go.mod h1:43aQb+Zerm/BWh2GnrgOQm7ffz7tvQXEKV6BFMl7wAo= github.com/gookit/color v1.5.4 h1:FZmqs7XOyGgCAxmWyPslpiok1k05wmY3SJTytgvYFs0= github.com/gookit/color v1.5.4/go.mod h1:pZJOeOS8DM43rXbp4AZo1n9zCU2qjpcRko0b6/QJi9w= +github.com/gophercloud/gophercloud v1.13.0 h1:8iY9d1DAbzMW6Vok1AxbbK5ZaUjzMp0tdyt4fX9IeJ0= github.com/gophercloud/gophercloud v1.13.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM= github.com/gorilla/context v1.1.1 h1:AWwleXJkX/nhcU9bZSnZoi3h/qGYqQAGhq6zZe/aQW8= github.com/gorilla/css v1.0.0 h1:BQqNyPTi50JCFMTw/b67hByjMVXZRwGha6wxVGkeihY= diff --git a/pkg/services/authz/zanzana/store/migration/migrator.go b/pkg/services/authz/zanzana/store/migration/migrator.go index b3e1f9a4d89..0bd475475f0 100644 --- a/pkg/services/authz/zanzana/store/migration/migrator.go +++ b/pkg/services/authz/zanzana/store/migration/migrator.go @@ -1,6 +1,9 @@ package migration import ( + "context" + "database/sql" + "errors" "fmt" "strings" @@ -11,6 +14,11 @@ import ( "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util/xorm" "github.com/openfga/openfga/pkg/storage/migrate" + "github.com/pressly/goose/v3" +) + +var ( + openFGATables = []string{"tuple", "authorization_model", "store", "assertion", "changelog", "goose_db_version"} ) func Run(cfg *setting.Cfg, dbType string, grafanaDBConfig *sqlstore.DatabaseConfig, logger log.Logger) error { @@ -43,7 +51,7 @@ func Run(cfg *setting.Cfg, dbType string, grafanaDBConfig *sqlstore.DatabaseConf Engine: dbType, } - if err := migrate.RunMigrations(migrationConfig); err != nil { + if err := runOpenFGAMigrations(migrationConfig, logger); err != nil { return fmt.Errorf("failed to run openfga migrations: %w", err) } @@ -54,9 +62,53 @@ func Run(cfg *setting.Cfg, dbType string, grafanaDBConfig *sqlstore.DatabaseConf return nil } +func runOpenFGAMigrations(migrationConfig migrate.MigrationConfig, logger log.Logger) error { + err := migrate.RunMigrations(migrationConfig) + if err == nil { + return nil + } + + // if an error occurs during migrations, it means that the goose schema is inconsistent with the openfga schema. + // since zanzana is a derived state, we can reset the schema state and retry. + logger.Warn("openfga migrations failed due to inconsistent goose schema/version state; resetting and retrying migrations", "error", err) + + if resetErr := resetOpenFGASchema(migrationConfig.Engine, migrationConfig.URI); resetErr != nil { + return fmt.Errorf("schema reset failed: %w", errors.Join(err, resetErr)) + } + + if retryErr := migrate.RunMigrations(migrationConfig); retryErr != nil { + return retryErr + } + + return nil +} + +// resetOpenFGASchema drops the openfga tables to ensure migrations will run from a clean state. +// openfga tables are derived state and state will be rebuilt from reconciliation. +func resetOpenFGASchema(engine, uri string) (retErr error) { + db, err := openDB(engine, uri) + if err != nil { + return fmt.Errorf("failed to open db for openfga schema reset: %w", err) + } + defer func() { + if err := db.Close(); err != nil && retErr == nil { + retErr = fmt.Errorf("failed to close db: %w", err) + } + }() + + for _, table := range openFGATables { + // strings are hard-coded, so this is safe. + // #nosec G201 nosemgrep: gosec.G201 + if _, err := db.ExecContext(context.Background(), fmt.Sprintf("DROP TABLE IF EXISTS %s", table)); err != nil { + return fmt.Errorf("failed to drop openfga table %s: %w", table, err) + } + } + + return nil +} + func RunWithMigrator(m *migrator.Migrator, cfg *setting.Cfg) error { - openfgaTables := []string{"tuple", "authorization_model", "store", "assertion", "changelog"} - for _, table := range openfgaTables { + for _, table := range openFGATables { m.AddMigration(fmt.Sprintf("Drop existing openfga table %s", table), migrator.NewDropTableMigration(table)) } @@ -68,6 +120,17 @@ func RunWithMigrator(m *migrator.Migrator, cfg *setting.Cfg) error { ) } +func openDB(engine, uri string) (*sql.DB, error) { + db, err := goose.OpenDBWithDriver(engine, uri) + if err == nil { + return db, nil + } + if engine == "sqlite" { + return goose.OpenDBWithDriver("sqlite3", uri) + } + return nil, err +} + // constructPostgresConnStrForOpenFGA parses a PostgreSQL connection string into a map of key-value pairs // parses into a format like // postgresql://grafana:password@127.0.0.1:5432/grafana?sslmode=disable&lock_timeout=2s&statement_timeout=10s diff --git a/pkg/services/authz/zanzana/store/migration/migrator_test.go b/pkg/services/authz/zanzana/store/migration/migrator_test.go new file mode 100644 index 00000000000..ff41a1456f6 --- /dev/null +++ b/pkg/services/authz/zanzana/store/migration/migrator_test.go @@ -0,0 +1,74 @@ +package migration + +import ( + "testing" + + "github.com/grafana/grafana/pkg/infra/log" + "github.com/openfga/openfga/pkg/storage/migrate" + "github.com/pressly/goose/v3" + "github.com/stretchr/testify/require" +) + +func TestRunOpenFGAMigrations_ResetsGooseVersionTableOnErrNoNextVersion(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + dbPath := tmpDir + "/openfga-test.db" + + // intentionally corrupt the goose version table + db, err := goose.OpenDBWithDriver("sqlite3", dbPath) + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + _, err = goose.EnsureDBVersion(db) + require.NoError(t, err) + + _, err = db.Exec("UPDATE goose_db_version SET is_applied = 0") + require.NoError(t, err) + _, err = goose.GetDBVersion(db) + require.ErrorIs(t, err, goose.ErrNoNextVersion) + + cfg := migrate.MigrationConfig{ + Engine: "sqlite", + URI: dbPath, + } + require.NoError(t, runOpenFGAMigrations(cfg, log.NewNopLogger())) + + // openFGA migrations should have established a valid current version. + db2, err := goose.OpenDBWithDriver("sqlite3", dbPath) + require.NoError(t, err) + t.Cleanup(func() { _ = db2.Close() }) + v, err := goose.GetDBVersion(db2) + require.NoError(t, err) + require.GreaterOrEqual(t, v, int64(0)) +} + +func TestRunOpenFGAMigrations_ResetsSchemaWhenGooseVersionInconsistentButSchemaExists(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + dbPath := tmpDir + "/openfga-test.db" + + cfg := migrate.MigrationConfig{ + Engine: "sqlite", + URI: dbPath, + } + require.NoError(t, runOpenFGAMigrations(cfg, log.NewNopLogger())) + + db, err := goose.OpenDBWithDriver("sqlite3", dbPath) + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + _, err = db.Exec("UPDATE goose_db_version SET is_applied = 0") + require.NoError(t, err) + _, err = goose.GetDBVersion(db) + require.ErrorIs(t, err, goose.ErrNoNextVersion) + + require.NoError(t, runOpenFGAMigrations(cfg, log.NewNopLogger())) + + db2, err := goose.OpenDBWithDriver("sqlite3", dbPath) + require.NoError(t, err) + t.Cleanup(func() { _ = db2.Close() }) + _, err = goose.GetDBVersion(db2) + require.NoError(t, err) +}