remove controller config (#1483)

canonical · Dec 6, 2024 · 1f700c2 · 1f700c2
1 parent 11b9e23
commit 1f700c2
Show file tree

Hide file tree

Showing 11 changed files with 18 additions and 449 deletions.
diff --git a/internal/db/controller.go b/internal/db/controller.go
@@ -179,51 +179,3 @@ func (d *Database) ForEachControllerModel(ctx context.Context, ctl *dbmodel.Cont
 	}
 	return nil
 }
-
-// UpsertControllerConfig upserts the controller config.
-func (d *Database) UpsertControllerConfig(ctx context.Context, cfg *dbmodel.ControllerConfig) (err error) {
-	const op = errors.Op("db.UpsertControllerConfig")
-
-	if err := d.ready(); err != nil {
-		return errors.E(op, err)
-	}
-
-	durationObserver := servermon.DurationObserver(servermon.DBQueryDurationHistogram, string(op))
-	defer durationObserver()
-	defer servermon.ErrorCounter(servermon.DBQueryErrorCount, &err, string(op))
-
-	if cfg.Name == "" {
-		return errors.E(op, errors.CodeBadRequest, `invalid config name ""`)
-	}
-
-	db := d.DB.WithContext(ctx)
-	if err := db.Save(cfg).Error; err != nil {
-		return errors.E(op)
-	}
-	return nil
-}
-
-func (d *Database) GetControllerConfig(ctx context.Context, cfg *dbmodel.ControllerConfig) (err error) {
-	const op = errors.Op("db.GetControllerConfig")
-	if err := d.ready(); err != nil {
-		return errors.E(op, err)
-	}
-
-	durationObserver := servermon.DurationObserver(servermon.DBQueryDurationHistogram, string(op))
-	defer durationObserver()
-	defer servermon.ErrorCounter(servermon.DBQueryErrorCount, &err, string(op))
-
-	if cfg.Name == "" {
-		return errors.E(op, errors.CodeNotFound, `invalid config name ""`)
-	}
-
-	db := d.DB.WithContext(ctx)
-	if err := db.Where("name = ?", cfg.Name).First(&cfg).Error; err != nil {
-		err := dbError(err)
-		if errors.ErrorCode(err) == errors.CodeNotFound {
-			return errors.E(op, err, "controller config not found")
-		}
-		return errors.E(op, err)
-	}
-	return nil
-}
diff --git a/internal/db/controller_test.go b/internal/db/controller_test.go
@@ -342,63 +342,3 @@ func (s *dbSuite) TestForEachControllerModel(c *qt.C) {
 		"00000002-0000-0000-0000-000000000004",
 	})
 }
-
-func TestUpsertControllerConfigUnconfiguredDatabase(t *testing.T) {
-	c := qt.New(t)
-
-	var d db.Database
-	err := d.UpsertControllerConfig(context.Background(), &dbmodel.ControllerConfig{})
-	c.Check(err, qt.ErrorMatches, `database not configured`)
-	c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeServerConfiguration)
-}
-
-func (s *dbSuite) TestControllerConfig(c *qt.C) {
-	ctx := context.Background()
-	err := s.Database.Migrate(context.Background(), true)
-	c.Assert(err, qt.Equals, nil)
-
-	config := dbmodel.ControllerConfig{
-		Name: "jimm",
-		Config: map[string]interface{}{
-			"key1": "value1",
-			"key2": "value2",
-		},
-	}
-	err = s.Database.UpsertControllerConfig(ctx, &config)
-	c.Assert(err, qt.IsNil)
-
-	config1 := dbmodel.ControllerConfig{
-		Name: "jimm",
-	}
-	err = s.Database.GetControllerConfig(ctx, &config1)
-	c.Assert(err, qt.IsNil)
-	c.Assert(config1, qt.DeepEquals, config)
-
-	config2 := config1
-	config2.Config = map[string]interface{}{
-		"key1": "value1.1",
-		"key2": "value2.1",
-		"key3": "value3",
-	}
-	err = s.Database.UpsertControllerConfig(ctx, &config2)
-	c.Assert(err, qt.IsNil)
-
-	err = s.Database.GetControllerConfig(ctx, &config1)
-	c.Assert(err, qt.IsNil)
-	c.Assert(config1, qt.DeepEquals, config2)
-
-	config3 := dbmodel.ControllerConfig{
-		Name: "unknown",
-	}
-	err = s.Database.GetControllerConfig(ctx, &config3)
-	c.Assert(err, qt.ErrorMatches, "controller config not found")
-}
-
-func TestGetControllerConfigUnconfiguredDatabase(t *testing.T) {
-	c := qt.New(t)
-
-	var d db.Database
-	err := d.GetControllerConfig(context.Background(), &dbmodel.ControllerConfig{})
-	c.Check(err, qt.ErrorMatches, `database not configured`)
-	c.Check(errors.ErrorCode(err), qt.Equals, errors.CodeServerConfiguration)
-}
diff --git a/internal/dbmodel/controller.go b/internal/dbmodel/controller.go
@@ -197,13 +197,3 @@ type CloudRegionControllerPriority struct {
 	// chosen when deploying to a cloud-region.
 	Priority uint
 }
-
-// ControllerConfig stores controller configuration.
-type ControllerConfig struct {
-	gorm.Model
-
-	// Name is the name given to this configuration.
-	Name string `gorm:"not null;uniqueIndex"`
-	// Config stores the controller configuration
-	Config Map
-}
diff --git a/internal/dbmodel/controller_test.go b/internal/dbmodel/controller_test.go
@@ -228,25 +228,3 @@ func TestToJujuRedirectInfoResult(t *testing.T) {
 		CACert: "ca-cert",
 	})
 }
-
-func TestControllerConfig(t *testing.T) {
-	c := qt.New(t)
-	db := gormDB(c)
-
-	cfg := dbmodel.ControllerConfig{
-		Name: "test-config",
-		Config: map[string]interface{}{
-			"key1": float64(1),
-			"key2": "test-value-2",
-			"key3": 42.0,
-		},
-	}
-	result := db.Create(&cfg)
-	c.Assert(result.Error, qt.IsNil)
-
-	var cfg2 dbmodel.ControllerConfig
-	result = db.Where("name = ?", "test-config").First(&cfg2)
-	c.Assert(result.Error, qt.IsNil)
-
-	c.Check(cfg2, qt.DeepEquals, cfg)
-}
diff --git a/internal/dbmodel/sql/postgres/1_15.sql b/internal/dbmodel/sql/postgres/1_15.sql
@@ -0,0 +1,5 @@
+-- 1_15.sql is a migration to delete controller configs
+DROP TABLE controller_configs;
+
+UPDATE versions SET major=1, minor=15 WHERE component='jimmdb';
+
diff --git a/internal/dbmodel/version.go b/internal/dbmodel/version.go
@@ -20,7 +20,7 @@ const (
 	// Minor is the minor version of the model described in the dbmodel
 	// package. It should be incremented for any change made to the
 	// database model from database model in a released JIMM.
-	Minor = 14
+	Minor = 15
 )
 
 type Version struct {

diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go
@@ -678,54 +678,6 @@ func (m *modelImporter) handleModelDeltas(ctx context.Context) error {
 	return nil
 }
 
-// SetControllerConfig changes the value of specified controller configuration
-// settings.
-func (j *JIMM) SetControllerConfig(ctx context.Context, user *openfga.User, args jujuparams.ControllerConfigSet) error {
-	const op = errors.Op("jimm.SetControllerConfig")
-
-	if !user.JimmAdmin {
-		return errors.E(op, errors.CodeUnauthorized, "unauthorized")
-	}
-
-	err := j.Database.Transaction(func(tx *db.Database) error {
-		config := dbmodel.ControllerConfig{
-			Name: "jimm",
-		}
-		err := tx.GetControllerConfig(ctx, &config)
-		if err != nil && errors.ErrorCode(err) != errors.CodeNotFound {
-			return err
-		}
-		if config.Config == nil {
-			config.Config = make(map[string]interface{})
-		}
-		for key, value := range args.Config {
-			config.Config[key] = value
-		}
-		return tx.UpsertControllerConfig(ctx, &config)
-	})
-	if err != nil {
-		return errors.E(op, err)
-	}
-	return nil
-}
-
-// GetControllerConfig returns jimm's controller config.
-func (j *JIMM) GetControllerConfig(ctx context.Context, u *dbmodel.Identity) (*dbmodel.ControllerConfig, error) {
-	const op = errors.Op("jimm.GetControllerConfig")
-	config := dbmodel.ControllerConfig{
-		Name:   "jimm",
-		Config: make(map[string]interface{}),
-	}
-	err := j.Database.GetControllerConfig(ctx, &config)
-	if err != nil {
-		if errors.ErrorCode(err) == errors.CodeNotFound {
-			return &config, nil
-		}
-		return nil, errors.E(op, err)
-	}
-	return &config, nil
-}
-
 // UpdateMigratedModel asserts that the model has been migrated to the
 // specified controller and updates the internal model representation.
 func (j *JIMM) UpdateMigratedModel(ctx context.Context, user *openfga.User, modelTag names.ModelTag, targetControllerName string) error {

diff --git a/internal/jimm/controller_test.go b/internal/jimm/controller_test.go
@@ -1029,172 +1029,6 @@ func TestImportModel(t *testing.T) {
 	}
 }
 
-const testControllerConfigEnv = `
-users:
-- username: alice@canonical.com
-- username: eve@canonical.com
-- username: fred@canonical.com
-`
-
-func TestSetControllerConfig(t *testing.T) {
-	c := qt.New(t)
-
-	tests := []struct {
-		about          string
-		user           string
-		args           jujuparams.ControllerConfigSet
-		jimmAdmin      bool
-		expectedError  string
-		expectedConfig dbmodel.ControllerConfig
-	}{{
-		about: "admin allowed to set config",
-		user:  "alice@canonical.com",
-		args: jujuparams.ControllerConfigSet{
-			Config: map[string]interface{}{
-				"key1": "value1",
-				"key2": "value2",
-				"key3": "value3",
-			},
-		},
-		jimmAdmin: true,
-		expectedConfig: dbmodel.ControllerConfig{
-			Name: "jimm",
-			Config: map[string]interface{}{
-				"key1": "value1",
-				"key2": "value2",
-				"key3": "value3",
-			},
-		},
-	}, {
-		about: "add-model user - unauthorized",
-		user:  "eve@canonical.com",
-		args: jujuparams.ControllerConfigSet{
-			Config: map[string]interface{}{
-				"key1": "value1",
-				"key2": "value2",
-				"key3": "value3",
-			},
-		},
-		expectedError: "unauthorized",
-	}, {
-		about: "login user - unauthorized",
-		user:  "fred@canonical.com",
-		args: jujuparams.ControllerConfigSet{
-			Config: map[string]interface{}{
-				"key1": "value1",
-				"key2": "value2",
-				"key3": "value3",
-			},
-		},
-		expectedError: "unauthorized",
-	}}
-
-	for _, test := range tests {
-		c.Run(test.about, func(c *qt.C) {
-			j := jimmtest.NewJIMM(c, nil)
-
-			env := jimmtest.ParseEnvironment(c, testControllerConfigEnv)
-			env.PopulateDB(c, j.Database)
-
-			dbUser := env.User(test.user).DBObject(c, j.Database)
-			user := openfga.NewUser(&dbUser, nil)
-			user.JimmAdmin = test.jimmAdmin
-
-			ctx := context.Background()
-
-			err := j.SetControllerConfig(ctx, user, test.args)
-			if test.expectedError == "" {
-				c.Assert(err, qt.IsNil)
-
-				cfg := dbmodel.ControllerConfig{
-					Name: "jimm",
-				}
-				err = j.Database.GetControllerConfig(ctx, &cfg)
-				c.Assert(err, qt.IsNil)
-				c.Assert(cfg, jimmtest.DBObjectEquals, test.expectedConfig)
-			} else {
-				c.Assert(err, qt.ErrorMatches, test.expectedError)
-			}
-		})
-	}
-}
-
-func TestGetControllerConfig(t *testing.T) {
-	c := qt.New(t)
-
-	tests := []struct {
-		about          string
-		user           string
-		jimmAdmin      bool
-		expectedError  string
-		expectedConfig dbmodel.ControllerConfig
-	}{{
-		about:     "admin allowed to set config",
-		user:      "alice@canonical.com",
-		jimmAdmin: true,
-		expectedConfig: dbmodel.ControllerConfig{
-			Name: "jimm",
-			Config: map[string]interface{}{
-				"key1": "value1",
-			},
-		},
-	}, {
-		about:     "add-model user - unauthorized",
-		user:      "eve@canonical.com",
-		jimmAdmin: false,
-		expectedConfig: dbmodel.ControllerConfig{
-			Name: "jimm",
-			Config: map[string]interface{}{
-				"key1": "value1",
-			},
-		},
-	}, {
-		about:     "login user - unauthorized",
-		user:      "fred@canonical.com",
-		jimmAdmin: false,
-		expectedConfig: dbmodel.ControllerConfig{
-			Name: "jimm",
-			Config: map[string]interface{}{
-				"key1": "value1",
-			},
-		},
-	}}
-
-	for _, test := range tests {
-		c.Run(test.about, func(c *qt.C) {
-			j := jimmtest.NewJIMM(c, nil)
-
-			env := jimmtest.ParseEnvironment(c, testImportModelEnv)
-			env.PopulateDB(c, j.Database)
-
-			dbSuperuser := env.User("alice@canonical.com").DBObject(c, j.Database)
-			superuser := openfga.NewUser(&dbSuperuser, nil)
-			superuser.JimmAdmin = true
-
-			dbUser := env.User(test.user).DBObject(c, j.Database)
-			user := openfga.NewUser(&dbUser, nil)
-			user.JimmAdmin = test.jimmAdmin
-
-			ctx := context.Background()
-
-			err := j.SetControllerConfig(ctx, superuser, jujuparams.ControllerConfigSet{
-				Config: map[string]interface{}{
-					"key1": "value1",
-				},
-			})
-			c.Assert(err, qt.Equals, nil)
-
-			cfg, err := j.GetControllerConfig(ctx, user.Identity)
-			if test.expectedError == "" {
-				c.Assert(err, qt.IsNil)
-				c.Assert(cfg, jimmtest.DBObjectEquals, &test.expectedConfig)
-			} else {
-				c.Assert(err, qt.ErrorMatches, test.expectedError)
-			}
-		})
-	}
-}
-
 const testUpdateMigratedModelEnv = `
 users:
 - username: alice@canonical.com