From 819f608bf5468ec8da00d9753adb236b3914ced3 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 9 Jan 2025 10:14:12 +0200 Subject: [PATCH 1/3] chore: don't allow nil credential store Currently we allow a nil credential store for legacy functionality to store secrets in Postgres or Vault. Nowadays secrets go to the CredentialStore interface which can be Vault/Postgres/Other. Secrets/sensitive data are now no longer in dbmodel types. --- internal/db/cloudcredential.go | 4 +- internal/db/cloudcredential_test.go | 12 +-- internal/dbmodel/cloudcredential.go | 7 -- internal/dbmodel/cloudcredential_test.go | 4 - internal/dbmodel/controller.go | 10 -- internal/dbmodel/controller_test.go | 13 +-- .../postgres/019_remove_secrets_in_db.up.sql | 6 ++ internal/jimm/cloudcredential.go | 49 ++-------- internal/jimm/cloudcredential_test.go | 92 +++++++++---------- internal/jimm/controller.go | 17 +--- internal/jimm/controller_test.go | 46 +++++----- internal/jimm/jimm.go | 16 +--- internal/jimm/model.go | 9 +- internal/jimm/types.go | 7 ++ internal/jujuapi/controller.go | 3 +- internal/jujuapi/jimm.go | 17 ++-- internal/jujuclient/client_test.go | 10 +- internal/jujuclient/dial_test.go | 10 +- internal/jujuclient/ping_test.go | 10 +- internal/jujuclient/storage_test.go | 30 +++--- internal/testutils/cmdtest/jimmsuite.go | 12 ++- internal/testutils/jimmtest/env.go | 39 +++----- internal/testutils/jimmtest/gorm.go | 6 +- internal/testutils/jimmtest/jimm.go | 5 +- .../jimmtest/mocks/jimm_controller_mock.go | 9 +- internal/testutils/jimmtest/suite.go | 12 ++- 26 files changed, 176 insertions(+), 279 deletions(-) create mode 100644 internal/dbmodel/sql/postgres/019_remove_secrets_in_db.up.sql create mode 100644 internal/jimm/types.go diff --git a/internal/db/cloudcredential.go b/internal/db/cloudcredential.go index b72c0766f..14b83822e 100644 --- a/internal/db/cloudcredential.go +++ b/internal/db/cloudcredential.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package db @@ -35,7 +35,7 @@ func (d *Database) SetCloudCredential(ctx context.Context, cred *dbmodel.CloudCr {Name: "owner_identity_name"}, {Name: "name"}, }, - DoUpdates: clause.AssignmentColumns([]string{"auth_type", "label", "attributes_in_vault", "attributes", "valid"}), + DoUpdates: clause.AssignmentColumns([]string{"auth_type", "label", "valid"}), }).Create(&cred).Error; err != nil { return errors.E(op, dbError(err)) } diff --git a/internal/db/cloudcredential_test.go b/internal/db/cloudcredential_test.go index f91d2bdae..774fae361 100644 --- a/internal/db/cloudcredential_test.go +++ b/internal/db/cloudcredential_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package db_test @@ -119,11 +119,6 @@ func (s *dbSuite) TestSetCloudCredentialUpdate(c *qt.C) { cred.Cloud.Regions = nil cred.Label = "test label" - cred.Attributes = dbmodel.StringMap{ - "key1": "value1", - "key2": "value2", - } - cred.AttributesInVault = true cred.Valid = sql.NullBool{ Bool: true, Valid: true, @@ -139,12 +134,7 @@ func (s *dbSuite) TestSetCloudCredentialUpdate(c *qt.C) { err = s.Database.GetCloudCredential(context.Background(), &dbCred) c.Assert(err, qt.Equals, nil) c.Assert(dbCred, jimmtest.DBObjectEquals, cred) - c.Assert(dbCred.Attributes, qt.DeepEquals, dbmodel.StringMap{ - "key1": "value1", - "key2": "value2", - }) c.Assert(dbCred.Label, qt.Equals, "test label") - c.Assert(dbCred.AttributesInVault, qt.IsTrue) c.Assert(dbCred.Valid.Valid, qt.IsTrue) c.Assert(dbCred.Valid.Bool, qt.IsTrue) } diff --git a/internal/dbmodel/cloudcredential.go b/internal/dbmodel/cloudcredential.go index 5c70e7347..44d1fdc8f 100644 --- a/internal/dbmodel/cloudcredential.go +++ b/internal/dbmodel/cloudcredential.go @@ -31,13 +31,6 @@ type CloudCredential struct { // Label is an optional label for the credential. Label string - // AttributesInVault indicates whether the attributes are stored in - // the configured vault key-value store, rather than this database. - AttributesInVault bool - - // Attributes contains the attributes of the credential. - Attributes StringMap - // Valid stores whether the cloud-credential is known to be valid. Valid sql.NullBool diff --git a/internal/dbmodel/cloudcredential_test.go b/internal/dbmodel/cloudcredential_test.go index 560c7884a..1f5d3ec8a 100644 --- a/internal/dbmodel/cloudcredential_test.go +++ b/internal/dbmodel/cloudcredential_test.go @@ -40,10 +40,6 @@ func TestCloudCredential(t *testing.T) { Owner: *i, AuthType: "empty", Label: "test label", - Attributes: dbmodel.StringMap{ - "a": "b", - "c": "d", - }, } result := db.Create(&cred) c.Assert(result.Error, qt.IsNil) diff --git a/internal/dbmodel/controller.go b/internal/dbmodel/controller.go index 4eab14dba..ea9d77451 100644 --- a/internal/dbmodel/controller.go +++ b/internal/dbmodel/controller.go @@ -32,14 +32,6 @@ type Controller struct { // purposes. UUID string `gorm:"not null"` - // AdminIdentityName is the identity name that JIMM uses to connect to the - // controller. - AdminIdentityName string - - // AdminPassword is the password that JIMM uses to connect to the - // controller. - AdminPassword string - // CACertificate is the CA certificate required to access this // controller. This is only set if the controller endpoint's // certificate is not signed by a public CA. @@ -112,7 +104,6 @@ func (c Controller) ToAPIControllerInfo() apiparams.ControllerInfo { var ci apiparams.ControllerInfo ci.Name = c.Name ci.UUID = c.UUID - ci.Username = c.AdminIdentityName ci.PublicAddress = c.PublicAddress for _, hps := range c.Addresses { for _, hp := range hps { @@ -122,7 +113,6 @@ func (c Controller) ToAPIControllerInfo() apiparams.ControllerInfo { ci.CACertificate = c.CACertificate ci.CloudTag = names.NewCloudTag(c.CloudName).String() ci.CloudRegion = c.CloudRegion - ci.Username = c.AdminIdentityName ci.AgentVersion = c.AgentVersion switch { case c.UnavailableSince.Valid: diff --git a/internal/dbmodel/controller_test.go b/internal/dbmodel/controller_test.go index 5274e2090..1faa5f245 100644 --- a/internal/dbmodel/controller_test.go +++ b/internal/dbmodel/controller_test.go @@ -40,13 +40,11 @@ func TestController(t *testing.T) { c.Assert(result.Error, qt.IsNil) ctl := dbmodel.Controller{ - Name: "test-controller", - UUID: "00000000-0000-0000-0000-000000000001", - AdminIdentityName: "admin", - AdminPassword: "pw", - CACertificate: "ca-cert", - PublicAddress: "controller.example.com:443", - CloudName: "test-cloud", + Name: "test-controller", + UUID: "00000000-0000-0000-0000-000000000001", + CACertificate: "ca-cert", + PublicAddress: "controller.example.com:443", + CloudName: "test-cloud", Addresses: dbmodel.HostPorts([][]jujuparams.HostPort{{{ Address: jujuparams.Address{ Value: "1.1.1.1", @@ -166,7 +164,6 @@ func TestToAPIControllerInfo(t *testing.T) { CloudRegion: cl.Regions[0], Priority: dbmodel.CloudRegionControllerPriorityDeployed, }} - ctl.AdminIdentityName = "admin" ctl.AgentVersion = "1.2.3" ci := ctl.ToAPIControllerInfo() diff --git a/internal/dbmodel/sql/postgres/019_remove_secrets_in_db.up.sql b/internal/dbmodel/sql/postgres/019_remove_secrets_in_db.up.sql new file mode 100644 index 000000000..d82d456c9 --- /dev/null +++ b/internal/dbmodel/sql/postgres/019_remove_secrets_in_db.up.sql @@ -0,0 +1,6 @@ +-- deletes secrets that were directly stored in the database. + +ALTER TABLE cloud_credentials DROP COLUMN IF EXISTS attributes_in_vault; +ALTER TABLE cloud_credentials DROP COLUMN IF EXISTS attributes; +ALTER TABLE controllers DROP COLUMN IF EXISTS admin_identity_name; +ALTER TABLE controllers DROP COLUMN IF EXISTS admin_password; diff --git a/internal/jimm/cloudcredential.go b/internal/jimm/cloudcredential.go index 81b6590ce..2fc773090 100644 --- a/internal/jimm/cloudcredential.go +++ b/internal/jimm/cloudcredential.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm @@ -41,7 +41,6 @@ func (j *JIMM) GetCloudCredential(ctx context.Context, user *openfga.User, tag n if err != nil { return nil, errors.E(op, err) } - credential.Attributes = nil return &credential, nil } @@ -177,7 +176,6 @@ func (j *JIMM) UpdateCloudCredential(ctx context.Context, user *openfga.User, ar } credential.AuthType = args.Credential.AuthType - credential.Attributes = args.Credential.Attributes if !args.SkipCheck { err := j.forEachController(ctx, controllers, func(ctl *dbmodel.Controller, api API) error { @@ -204,7 +202,7 @@ func (j *JIMM) UpdateCloudCredential(ctx context.Context, user *openfga.User, ar return result, nil } - if err := j.updateCredential(ctx, &credential); err != nil { + if err := j.updateCredential(ctx, &credential, args.Credential.Attributes); err != nil { return result, errors.E(op, err) } @@ -227,32 +225,18 @@ func (j *JIMM) UpdateCloudCredential(ctx context.Context, user *openfga.User, ar } // updateCredential updates the credential stored in JIMM's database. -func (j *JIMM) updateCredential(ctx context.Context, credential *dbmodel.CloudCredential) error { +func (j *JIMM) updateCredential(ctx context.Context, credential *dbmodel.CloudCredential, attr map[string]string) error { const op = errors.Op("jimm.updateCredential") - if j.CredentialStore == nil { - zapctx.Debug(ctx, "credential store is nil!") - credential.AttributesInVault = false - if err := j.Database.SetCloudCredential(ctx, credential); err != nil { - return errors.E(op, err) - } - return nil - } - - credential1 := *credential - credential1.Attributes = nil - credential1.AttributesInVault = true - if err := j.Database.SetCloudCredential(ctx, &credential1); err != nil { + if err := j.Database.SetCloudCredential(ctx, credential); err != nil { zapctx.Error(ctx, "failed to store credential id", zap.Error(err)) return errors.E(op, err) } - if err := j.CredentialStore.Put(ctx, credential.ResourceTag(), credential.Attributes); err != nil { + if err := j.CredentialStore.Put(ctx, credential.ResourceTag(), attr); err != nil { zapctx.Error(ctx, "failed to store credentials", zap.Error(err)) return errors.E(op, err) } - zapctx.Info(ctx, "credential store location", zap.Bool("vault", credential.AttributesInVault)) - return nil } @@ -263,13 +247,10 @@ func (j *JIMM) updateControllerCloudCredential( ) ([]jujuparams.UpdateCredentialModelResult, error) { const op = errors.Op("jimm.updateControllerCloudCredential") - attr := cred.Attributes - if attr == nil { - var err error - attr, err = j.getCloudCredentialAttributes(ctx, cred) - if err != nil { - return nil, errors.E(op, err) - } + var err error + attr, err := j.getCloudCredentialAttributes(ctx, cred) + if err != nil { + return nil, errors.E(op, err) } models, err := f(ctx, jujuparams.TaggedCredential{ @@ -302,7 +283,6 @@ func (j *JIMM) ForEachUserCloudCredential(ctx context.Context, u *dbmodel.Identi errStop := errors.E("stop") var iterErr error err := j.Database.ForEachCloudCredential(ctx, u.Name, cloud, func(cred *dbmodel.CloudCredential) error { - cred.Attributes = nil iterErr = f(cred) if iterErr != nil { return errStop @@ -365,17 +345,6 @@ func (j *JIMM) GetCloudCredentialAttributes(ctx context.Context, user *openfga.U func (j *JIMM) getCloudCredentialAttributes(ctx context.Context, cred *dbmodel.CloudCredential) (map[string]string, error) { const op = errors.Op("jimm.getCloudCredentialAttributes") - if !cred.AttributesInVault { - if err := j.Database.GetCloudCredential(ctx, cred); err != nil { - return nil, errors.E(op, err) - } - return map[string]string(cred.Attributes), nil - } - - // Attributes have to be loaded from vault. - if j.CredentialStore == nil { - return nil, errors.E(op, errors.CodeServerConfiguration, "vault not configured") - } attr, err := j.CredentialStore.Get(ctx, cred.ResourceTag()) if err != nil { return nil, errors.E(op, err) diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index 48380b8e4..a0530785b 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test @@ -31,6 +31,10 @@ func TestUpdateCloudCredential(t *testing.T) { arch := "amd64" mem := uint64(8096) cores := uint64(8) + testAttributes := map[string]string{ + "key1": "value1", + "key2": "value2", + } tests := []struct { about string @@ -99,6 +103,9 @@ func TestUpdateCloudCredential(t *testing.T) { err = j.Database.SetCloudCredential(context.Background(), &cred) c.Assert(err, qt.Equals, nil) + err = j.CredentialStore.Put(context.Background(), cred.ResourceTag(), testAttributes) + c.Assert(err, qt.Equals, nil) + cred.Cloud = dbmodel.Cloud{ Name: "test-cloud", Type: "test-provider", @@ -116,20 +123,13 @@ func TestUpdateCloudCredential(t *testing.T) { arg := jimm.UpdateCloudCredentialArgs{ CredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), Credential: jujuparams.CloudCredential{ - Attributes: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - AuthType: "test-auth-type", + Attributes: testAttributes, + AuthType: "test-auth-type", }, } expectedCredential := cred expectedCredential.AuthType = "test-auth-type" - expectedCredential.Attributes = map[string]string{ - "key1": "value1", - "key2": "value2", - } m := dbmodel.Model{ UUID: sql.NullString{ @@ -214,6 +214,9 @@ func TestUpdateCloudCredential(t *testing.T) { err = j.Database.SetCloudCredential(context.Background(), &cred) c.Assert(err, qt.Equals, nil) + err = j.CredentialStore.Put(context.Background(), cred.ResourceTag(), testAttributes) + c.Assert(err, qt.Equals, nil) + cred.Cloud = dbmodel.Cloud{ Name: "test-cloud", Type: "test-provider", @@ -231,11 +234,8 @@ func TestUpdateCloudCredential(t *testing.T) { arg := jimm.UpdateCloudCredentialArgs{ CredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), Credential: jujuparams.CloudCredential{ - Attributes: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - AuthType: "test-auth-type", + Attributes: testAttributes, + AuthType: "test-auth-type", }, } return u, arg, dbmodel.CloudCredential{}, "test error" @@ -305,6 +305,9 @@ func TestUpdateCloudCredential(t *testing.T) { err = j.Database.SetCloudCredential(context.Background(), &cred) c.Assert(err, qt.Equals, nil) + err = j.CredentialStore.Put(context.Background(), cred.ResourceTag(), testAttributes) + c.Assert(err, qt.Equals, nil) + _, err = j.AddModel(context.Background(), user, &jimm.ModelCreateArgs{ Name: "test-model", Owner: names.NewUserTag(u.Name), @@ -317,11 +320,8 @@ func TestUpdateCloudCredential(t *testing.T) { arg := jimm.UpdateCloudCredentialArgs{ CredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), Credential: jujuparams.CloudCredential{ - Attributes: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - AuthType: "test-auth-type", + Attributes: testAttributes, + AuthType: "test-auth-type", }, } return u, arg, dbmodel.CloudCredential{}, "test error" @@ -399,6 +399,9 @@ func TestUpdateCloudCredential(t *testing.T) { err = j.Database.SetCloudCredential(context.Background(), &cred) c.Assert(err, qt.Equals, nil) + err = j.CredentialStore.Put(context.Background(), cred.ResourceTag(), testAttributes) + c.Assert(err, qt.Equals, nil) + mi, err := j.AddModel(context.Background(), alice, &jimm.ModelCreateArgs{ Name: "test-model", Owner: names.NewUserTag("eve@canonical.com"), @@ -411,11 +414,8 @@ func TestUpdateCloudCredential(t *testing.T) { arg := jimm.UpdateCloudCredentialArgs{ CredentialTag: names.NewCloudCredentialTag("test-cloud/eve@canonical.com/test-credential-1"), Credential: jujuparams.CloudCredential{ - Attributes: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - AuthType: "test-auth-type", + Attributes: testAttributes, + AuthType: "test-auth-type", }, } m := dbmodel.Model{ @@ -441,12 +441,8 @@ func TestUpdateCloudCredential(t *testing.T) { Type: cloud.Type, }, OwnerIdentityName: eve.Name, - Attributes: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - AuthType: "test-auth-type", - Models: []dbmodel.Model{m}, + AuthType: "test-auth-type", + Models: []dbmodel.Model{m}, }, "" }, }, { @@ -513,6 +509,9 @@ func TestUpdateCloudCredential(t *testing.T) { err = j.Database.SetCloudCredential(context.Background(), &cred) c.Assert(err, qt.Equals, nil) + err = j.CredentialStore.Put(context.Background(), cred.ResourceTag(), testAttributes) + c.Assert(err, qt.Equals, nil) + cred.Cloud = dbmodel.Cloud{ Name: "test-cloud", Type: "test-provider", @@ -530,21 +529,14 @@ func TestUpdateCloudCredential(t *testing.T) { arg := jimm.UpdateCloudCredentialArgs{ CredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), Credential: jujuparams.CloudCredential{ - Attributes: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - AuthType: "test-auth-type", + Attributes: testAttributes, + AuthType: "test-auth-type", }, SkipCheck: true, } expectedCredential := cred expectedCredential.AuthType = "test-auth-type" - expectedCredential.Attributes = map[string]string{ - "key1": "value1", - "key2": "value2", - } m := dbmodel.Model{ UUID: sql.NullString{ String: mi.UUID, @@ -626,6 +618,9 @@ func TestUpdateCloudCredential(t *testing.T) { err = j.Database.SetCloudCredential(context.Background(), &cred) c.Assert(err, qt.Equals, nil) + err = j.CredentialStore.Put(context.Background(), cred.ResourceTag(), testAttributes) + c.Assert(err, qt.Equals, nil) + cred.Cloud = dbmodel.Cloud{ Name: "test-cloud", Type: "test-provider", @@ -642,11 +637,8 @@ func TestUpdateCloudCredential(t *testing.T) { arg := jimm.UpdateCloudCredentialArgs{ CredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), Credential: jujuparams.CloudCredential{ - Attributes: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - AuthType: "test-auth-type", + Attributes: testAttributes, + AuthType: "test-auth-type", }, SkipUpdate: true, } @@ -779,7 +771,7 @@ func TestUpdateCloudCredential(t *testing.T) { API: api, }, }, - jimmtest.UnsetCredentialStore, // this test relies on credential attributes being stored in postgres + jimmtest.UsePostgresAsCredentialStore, // this test relies on credential attributes being stored in postgres ) u, arg, expectedCredential, expectedError := test.createEnv(c, j) @@ -804,6 +796,10 @@ func TestUpdateCloudCredential(t *testing.T) { c.Assert(err, qt.Equals, nil) c.Assert(credential, jimmtest.DBObjectEquals, expectedCredential) + + gotAttributes, err := j.CredentialStore.Get(ctx, credential.ResourceTag()) + c.Assert(err, qt.Equals, nil) + c.Assert(gotAttributes, qt.DeepEquals, testAttributes) } else { c.Assert(err, qt.ErrorMatches, expectedError) } @@ -1450,9 +1446,6 @@ func TestForEachUserCloudCredential(t *testing.T) { if test.f == nil { test.f = func(cred *dbmodel.CloudCredential) error { credentials = append(credentials, cred.Tag().String()) - if cred.Attributes != nil { - return errors.E("credential contains attributes") - } return nil } } @@ -1651,7 +1644,6 @@ func TestCloudCredentialAttributeStore(t *testing.T) { Name: "test", Type: "test-provider", }, - AttributesInVault: true, }) attr, _, err := j.GetCloudCredentialAttributes(ctx, user, &cred, true) c.Assert(err, qt.IsNil) diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go index 0db50d1bf..41b2fe182 100644 --- a/internal/jimm/controller.go +++ b/internal/jimm/controller.go @@ -218,7 +218,7 @@ func addControllerTx(ctx context.Context, j *JIMM, jujuClouds []dbmodel.Cloud, c // code of CodeAlreadyExists will be returned. If the controller cannot be // contacted then an error with a code of CodeConnectionFailed will be // returned. -func (j *JIMM) AddController(ctx context.Context, user *openfga.User, ctl *dbmodel.Controller) error { +func (j *JIMM) AddController(ctx context.Context, user *openfga.User, ctl *dbmodel.Controller, creds ControllerCreds) error { const op = errors.Op("jimm.AddController") if err := j.checkJimmAdmin(user); err != nil { @@ -268,20 +268,11 @@ func (j *JIMM) AddController(ctx context.Context, user *openfga.User, ctl *dbmod } } - // TODO(ale8k): This shouldn't be necessary to check, but tests need updating - // to set insecure credential store explicitly. - if j.CredentialStore != nil { - err := j.CredentialStore.PutControllerCredentials(ctx, ctl.Name, ctl.AdminIdentityName, ctl.AdminPassword) - if err != nil { - return errors.E(op, err, "failed to store controller credentials") - } + err = j.CredentialStore.PutControllerCredentials(ctx, ctl.Name, creds.AdminIdentityName, creds.AdminPassword) + if err != nil { + return errors.E(op, err, "failed to store controller credentials") } - // Credential store will always be set either to vault or explicitly insecure, - // no need to be persist in db. - ctl.AdminIdentityName = "" - ctl.AdminPassword = "" - if err := addControllerTx(ctx, j, dbClouds, ctl); err != nil { zapctx.Error(ctx, "failed to add controller", zaputil.Error(err)) if errors.ErrorCode(err) == errors.CodeAlreadyExists { diff --git a/internal/jimm/controller_test.go b/internal/jimm/controller_test.go index 26886a2dc..f11ae42e4 100644 --- a/internal/jimm/controller_test.go +++ b/internal/jimm/controller_test.go @@ -144,12 +144,14 @@ func TestAddController(t *testing.T) { c.Assert(err, qt.IsNil) ctl1 := dbmodel.Controller{ - Name: "test-controller", - AdminIdentityName: "admin", - AdminPassword: "5ecret", - PublicAddress: "example.com:443", + Name: "test-controller", + PublicAddress: "example.com:443", } - err = j.AddController(context.Background(), alice, &ctl1) + ctlCreds := jimm.ControllerCreds{ + AdminIdentityName: "user", + AdminPassword: "secret", + } + err = j.AddController(context.Background(), alice, &ctl1, ctlCreds) c.Assert(err, qt.IsNil) ctl2 := dbmodel.Controller{ @@ -160,12 +162,10 @@ func TestAddController(t *testing.T) { c.Check(ctl2, qt.CmpEquals(cmpopts.EquateEmpty(), cmpopts.IgnoreTypes(dbmodel.CloudRegion{})), ctl1) ctl3 := dbmodel.Controller{ - Name: "test-controller-2", - AdminIdentityName: "admin", - AdminPassword: "5ecret", - PublicAddress: "example.com:443", + Name: "test-controller-2", + PublicAddress: "example.com:443", } - err = j.AddController(context.Background(), alice, &ctl3) + err = j.AddController(context.Background(), alice, &ctl3, ctlCreds) c.Assert(err, qt.IsNil) ctl4 := dbmodel.Controller{ @@ -389,15 +389,15 @@ func TestAddControllerWithVault(t *testing.T) { c.Assert(err, qt.IsNil) ctl1 := dbmodel.Controller{ - Name: "test-controller", + Name: "test-controller", + PublicAddress: "example.com:443", + } + ctlCreds := jimm.ControllerCreds{ AdminIdentityName: "admin", - AdminPassword: "5ecret", - PublicAddress: "example.com:443", + AdminPassword: "secret", } - err = j.AddController(context.Background(), alice, &ctl1) + err = j.AddController(context.Background(), alice, &ctl1, ctlCreds) c.Assert(err, qt.IsNil) - c.Assert(ctl1.AdminIdentityName, qt.Equals, "") - c.Assert(ctl1.AdminPassword, qt.Equals, "") ctl2 := dbmodel.Controller{ Name: "test-controller", @@ -409,18 +409,14 @@ func TestAddControllerWithVault(t *testing.T) { username, password, err := store.GetControllerCredentials(ctx, ctl1.Name) c.Assert(err, qt.IsNil) c.Assert(username, qt.Equals, "admin") - c.Assert(password, qt.Equals, "5ecret") + c.Assert(password, qt.Equals, "secret") ctl3 := dbmodel.Controller{ - Name: "test-controller-2", - AdminIdentityName: "admin", - AdminPassword: "5ecretToo", - PublicAddress: "example.com:443", + Name: "test-controller-2", + PublicAddress: "example.com:443", } - err = j.AddController(context.Background(), alice, &ctl3) + err = j.AddController(context.Background(), alice, &ctl3, ctlCreds) c.Assert(err, qt.IsNil) - c.Assert(ctl3.AdminIdentityName, qt.Equals, "") - c.Assert(ctl3.AdminPassword, qt.Equals, "") ctl4 := dbmodel.Controller{ Name: "test-controller-2", @@ -432,7 +428,7 @@ func TestAddControllerWithVault(t *testing.T) { username, password, err = store.GetControllerCredentials(ctx, ctl4.Name) c.Assert(err, qt.IsNil) c.Assert(username, qt.Equals, "admin") - c.Assert(password, qt.Equals, "5ecretToo") + c.Assert(password, qt.Equals, "secret") } const testEarliestControllerVersionEnv = `clouds: diff --git a/internal/jimm/jimm.go b/internal/jimm/jimm.go index 0b743c9e4..3cd13ebe1 100644 --- a/internal/jimm/jimm.go +++ b/internal/jimm/jimm.go @@ -234,9 +234,7 @@ type Parameters struct { Dialer Dialer // CredentialStore is a store for the attributes of a - // cloud credential and controller credentials. If this is - // not configured then the attributes - // are stored in the standard database. + // cloud credential and controller credentials. CredentialStore credentials.CredentialStore // Pubsub is a pub-sub hub used for buffering model summaries. @@ -836,15 +834,9 @@ func fillMigrationTarget(db *db.Database, credStore credentials.CredentialStore, if err != nil { return jujuparams.MigrationTargetInfo{}, 0, err } - adminUser := dbController.AdminIdentityName - adminPass := dbController.AdminPassword - if adminPass == "" { - u, p, err := credStore.GetControllerCredentials(ctx, controllerName) - if err != nil { - return jujuparams.MigrationTargetInfo{}, 0, err - } - adminUser = u - adminPass = p + adminUser, adminPass, err := credStore.GetControllerCredentials(ctx, controllerName) + if err != nil { + return jujuparams.MigrationTargetInfo{}, 0, err } if adminUser == "" || adminPass == "" { return jujuparams.MigrationTargetInfo{}, 0, errors.E("missing target controller credentials") diff --git a/internal/jimm/model.go b/internal/jimm/model.go index 76f98f1ee..553d72fbe 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm @@ -539,13 +539,8 @@ func (b *modelBuilder) CreateControllerModel() *modelBuilder { func (b *modelBuilder) updateCredential(ctx context.Context, api API, cred *dbmodel.CloudCredential) error { var err error - cred1 := *cred - cred1.Attributes, err = b.jimm.getCloudCredentialAttributes(ctx, cred) - if err != nil { - return err - } - _, err = b.jimm.updateControllerCloudCredential(ctx, &cred1, api.UpdateCredential) + _, err = b.jimm.updateControllerCloudCredential(ctx, cred, api.UpdateCredential) return err } diff --git a/internal/jimm/types.go b/internal/jimm/types.go new file mode 100644 index 000000000..c11868864 --- /dev/null +++ b/internal/jimm/types.go @@ -0,0 +1,7 @@ +// Copyright 2025 Canonical. +package jimm + +type ControllerCreds struct { + AdminIdentityName string + AdminPassword string +} diff --git a/internal/jujuapi/controller.go b/internal/jujuapi/controller.go index f424b0e4d..e24230887 100644 --- a/internal/jujuapi/controller.go +++ b/internal/jujuapi/controller.go @@ -12,6 +12,7 @@ import ( "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/errors" + "github.com/canonical/jimm/v3/internal/jimm" "github.com/canonical/jimm/v3/internal/jujuapi/rpc" "github.com/canonical/jimm/v3/internal/openfga" jimmversion "github.com/canonical/jimm/v3/version" @@ -51,7 +52,7 @@ func init() { // ControllerService defines the methods used to manage controllers. type ControllerService interface { - AddController(ctx context.Context, user *openfga.User, ctl *dbmodel.Controller) error + AddController(ctx context.Context, user *openfga.User, ctl *dbmodel.Controller, creds jimm.ControllerCreds) error ControllerInfo(ctx context.Context, name string) (*dbmodel.Controller, error) EarliestControllerVersion(ctx context.Context) (version.Number, error) ListControllers(ctx context.Context, user *openfga.User) ([]dbmodel.Controller, error) diff --git a/internal/jujuapi/jimm.go b/internal/jujuapi/jimm.go index 5c0aa6829..5bd868660 100644 --- a/internal/jujuapi/jimm.go +++ b/internal/jujuapi/jimm.go @@ -18,6 +18,7 @@ import ( "github.com/canonical/jimm/v3/internal/db" "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/errors" + "github.com/canonical/jimm/v3/internal/jimm" "github.com/canonical/jimm/v3/internal/jujuapi/rpc" ofganames "github.com/canonical/jimm/v3/internal/openfga/names" "github.com/canonical/jimm/v3/pkg/api/params" @@ -204,16 +205,18 @@ func (r *controllerRoot) AddController(ctx context.Context, req apiparams.AddCon // TODO(ale8k): Don't build dbmodel here, do it as params to AddController. ctl := dbmodel.Controller{ - UUID: req.UUID, - Name: req.Name, - PublicAddress: req.PublicAddress, - CACertificate: req.CACertificate, + UUID: req.UUID, + Name: req.Name, + PublicAddress: req.PublicAddress, + CACertificate: req.CACertificate, + TLSHostname: req.TLSHostname, + Addresses: dbmodel.HostPorts{jujuparams.FromProviderHostPorts(nphps)}, + } + ctlCreds := jimm.ControllerCreds{ AdminIdentityName: req.Username, AdminPassword: req.Password, - TLSHostname: req.TLSHostname, - Addresses: dbmodel.HostPorts{jujuparams.FromProviderHostPorts(nphps)}, } - if err := r.jimm.AddController(ctx, r.user, &ctl); err != nil { + if err := r.jimm.AddController(ctx, r.user, &ctl, ctlCreds); err != nil { zapctx.Error(ctx, "failed to add controller", zaputil.Error(err)) return apiparams.ControllerInfo{}, errors.E(op, err) } diff --git a/internal/jujuclient/client_test.go b/internal/jujuclient/client_test.go index b3e370349..d123bd0de 100644 --- a/internal/jujuclient/client_test.go +++ b/internal/jujuclient/client_test.go @@ -38,12 +38,10 @@ func (s *clientSuite) TestStatus(c *gc.C) { info := s.APIInfo(c) ctl := dbmodel.Controller{ - UUID: info.ControllerUUID, - Name: s.ControllerConfig.ControllerName(), - CACertificate: info.CACert, - AdminIdentityName: info.Tag.Id(), - AdminPassword: info.Password, - PublicAddress: info.Addrs[0], + UUID: info.ControllerUUID, + Name: s.ControllerConfig.ControllerName(), + CACertificate: info.CACert, + PublicAddress: info.Addrs[0], } models, err := s.API.UpdateCredential(ctx, cred) diff --git a/internal/jujuclient/dial_test.go b/internal/jujuclient/dial_test.go index 0837f1cca..35bf7991e 100644 --- a/internal/jujuclient/dial_test.go +++ b/internal/jujuclient/dial_test.go @@ -71,12 +71,10 @@ var _ = gc.Suite(&dialSuite{}) func (s *dialSuite) TestDial(c *gc.C) { info := s.APIInfo(c) ctl := dbmodel.Controller{ - UUID: s.ControllerConfig.ControllerUUID(), - Name: s.ControllerConfig.ControllerName(), - CACertificate: info.CACert, - AdminIdentityName: info.Tag.Id(), - AdminPassword: info.Password, - PublicAddress: info.Addrs[0], + UUID: s.ControllerConfig.ControllerUUID(), + Name: s.ControllerConfig.ControllerName(), + CACertificate: info.CACert, + PublicAddress: info.Addrs[0], } api, err := s.Dialer.Dial(context.Background(), &ctl, names.ModelTag{}, nil) c.Assert(err, gc.Equals, nil) diff --git a/internal/jujuclient/ping_test.go b/internal/jujuclient/ping_test.go index a87257498..7c2497b37 100644 --- a/internal/jujuclient/ping_test.go +++ b/internal/jujuclient/ping_test.go @@ -22,12 +22,10 @@ func (s *pingSuite) TestPing(c *gc.C) { info := s.APIInfo(c) ctl := dbmodel.Controller{ - UUID: s.ControllerConfig.ControllerUUID(), - Name: s.ControllerConfig.ControllerName(), - CACertificate: info.CACert, - AdminIdentityName: info.Tag.Id(), - AdminPassword: info.Password, - PublicAddress: info.Addrs[0], + UUID: s.ControllerConfig.ControllerUUID(), + Name: s.ControllerConfig.ControllerName(), + CACertificate: info.CACert, + PublicAddress: info.Addrs[0], } api, err := s.Dialer.Dial(ctx, &ctl, names.ModelTag{}, nil) c.Assert(err, gc.Equals, nil) diff --git a/internal/jujuclient/storage_test.go b/internal/jujuclient/storage_test.go index 6aae59632..04032f0b3 100644 --- a/internal/jujuclient/storage_test.go +++ b/internal/jujuclient/storage_test.go @@ -35,12 +35,10 @@ func (s *storageSuite) TestListFilesystems(c *gc.C) { info := s.APIInfo(c) ctl := dbmodel.Controller{ - UUID: s.ControllerConfig.ControllerUUID(), - Name: s.ControllerConfig.ControllerName(), - CACertificate: info.CACert, - AdminIdentityName: info.Tag.Id(), - AdminPassword: info.Password, - PublicAddress: info.Addrs[0], + UUID: s.ControllerConfig.ControllerUUID(), + Name: s.ControllerConfig.ControllerName(), + CACertificate: info.CACert, + PublicAddress: info.Addrs[0], } models, err := s.API.UpdateCredential(ctx, cred) @@ -81,12 +79,10 @@ func (s *storageSuite) TestListVolumes(c *gc.C) { info := s.APIInfo(c) ctl := dbmodel.Controller{ - UUID: s.ControllerConfig.ControllerUUID(), - Name: s.ControllerConfig.ControllerName(), - CACertificate: info.CACert, - AdminIdentityName: info.Tag.Id(), - AdminPassword: info.Password, - PublicAddress: info.Addrs[0], + UUID: s.ControllerConfig.ControllerUUID(), + Name: s.ControllerConfig.ControllerName(), + CACertificate: info.CACert, + PublicAddress: info.Addrs[0], } models, err := s.API.UpdateCredential(ctx, cred) @@ -127,12 +123,10 @@ func (s *storageSuite) TestListStorageDetails(c *gc.C) { info := s.APIInfo(c) ctl := dbmodel.Controller{ - UUID: s.ControllerConfig.ControllerUUID(), - Name: s.ControllerConfig.ControllerName(), - CACertificate: info.CACert, - AdminIdentityName: info.Tag.Id(), - AdminPassword: info.Password, - PublicAddress: info.Addrs[0], + UUID: s.ControllerConfig.ControllerUUID(), + Name: s.ControllerConfig.ControllerName(), + CACertificate: info.CACert, + PublicAddress: info.Addrs[0], } models, err := s.API.UpdateCredential(ctx, cred) diff --git a/internal/testutils/cmdtest/jimmsuite.go b/internal/testutils/cmdtest/jimmsuite.go index 349b043e4..ebd62deeb 100644 --- a/internal/testutils/cmdtest/jimmsuite.go +++ b/internal/testutils/cmdtest/jimmsuite.go @@ -196,12 +196,14 @@ func (s *JimmCmdSuite) RefreshControllerAddress(c *gc.C) { func (s *JimmCmdSuite) AddController(c *gc.C, name string, info *api.Info) { ctl := &dbmodel.Controller{ - UUID: info.ControllerUUID, - Name: name, + UUID: info.ControllerUUID, + Name: name, + CACertificate: info.CACert, + Addresses: nil, + } + ctlCreds := jimm.ControllerCreds{ AdminIdentityName: info.Tag.Id(), AdminPassword: info.Password, - CACertificate: info.CACert, - Addresses: nil, } ctl.Addresses = make(dbmodel.HostPorts, 0, len(info.Addrs)) for _, addr := range info.Addrs { @@ -214,7 +216,7 @@ func (s *JimmCmdSuite) AddController(c *gc.C, name string, info *api.Info) { } adminUser := openfga.NewUser(s.AdminUser, s.OFGAClient) adminUser.JimmAdmin = true - err := s.JIMM.AddController(context.Background(), adminUser, ctl) + err := s.JIMM.AddController(context.Background(), adminUser, ctl, ctlCreds) c.Assert(err, gc.Equals, nil) } diff --git a/internal/testutils/jimmtest/env.go b/internal/testutils/jimmtest/env.go index f68f0f97c..7e9fc0514 100644 --- a/internal/testutils/jimmtest/env.go +++ b/internal/testutils/jimmtest/env.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimmtest @@ -182,17 +182,6 @@ func (m Model) addModelRelations(c *qt.C, db *db.Database, client *openfga.OFGAC // addControllerRelations adds permissions the model should have and adds permissions for users to the controller. func (ctl Controller) addControllerRelations(c *qt.C, client *openfga.OFGAClient) { - if ctl.dbo.AdminIdentityName != "" { - userIdentity, err := dbmodel.NewIdentity(ctl.dbo.AdminIdentityName) - c.Assert(err, qt.IsNil) - - user := openfga.NewUser( - userIdentity, - client, - ) - err = user.SetControllerAccess(context.Background(), ctl.dbo.ResourceTag(), ofganames.AdministratorRelation) - c.Assert(err, qt.IsNil) - } err := client.AddCloudController(context.Background(), names.NewCloudTag(ctl.Cloud), ctl.dbo.ResourceTag()) c.Assert(err, qt.IsNil) } @@ -385,11 +374,10 @@ func (cl *Cloud) DBObject(c Tester, db *db.Database) dbmodel.Cloud { // A CloudCredential represents the definition of a cloud credential in a // test environment. type CloudCredential struct { - Owner string `json:"owner"` - Cloud string `json:"cloud"` - Name string `json:"name"` - AuthType string `json:"auth-type"` - Attributes map[string]string `json:"attributes"` + Owner string `json:"owner"` + Cloud string `json:"cloud"` + Name string `json:"name"` + AuthType string `json:"auth-type"` env *Environment dbo dbmodel.CloudCredential @@ -405,7 +393,6 @@ func (cc *CloudCredential) DBObject(c Tester, db *db.Database) dbmodel.CloudCred cc.dbo.Owner = cc.env.User(cc.Owner).DBObject(c, db) cc.dbo.OwnerIdentityName = cc.dbo.Owner.Name cc.dbo.AuthType = cc.AuthType - cc.dbo.Attributes = cc.Attributes err := db.SetCloudCredential(context.Background(), &cc.dbo) if err != nil { @@ -418,14 +405,12 @@ func (cc *CloudCredential) DBObject(c Tester, db *db.Database) dbmodel.CloudCred // A Controller represents the definition of a controller in a test // environment. type Controller struct { - Name string `json:"name"` - UUID string `json:"uuid"` - Cloud string `json:"cloud"` - CloudRegion string `json:"region"` - CloudRegions []CloudRegionControllerPriority `json:"cloud-regions"` - AgentVersion string `json:"agent-version"` - AdminUser string `json:"admin-user"` - AdminPassword string `json:"admin-password"` + Name string `json:"name"` + UUID string `json:"uuid"` + Cloud string `json:"cloud"` + CloudRegion string `json:"region"` + CloudRegions []CloudRegionControllerPriority `json:"cloud-regions"` + AgentVersion string `json:"agent-version"` env *Environment dbo dbmodel.Controller @@ -438,8 +423,6 @@ func (ctl *Controller) DBObject(c Tester, db *db.Database) dbmodel.Controller { ctl.dbo.Name = ctl.Name ctl.dbo.UUID = ctl.UUID ctl.dbo.AgentVersion = ctl.AgentVersion - ctl.dbo.AdminIdentityName = ctl.AdminUser - ctl.dbo.AdminPassword = ctl.AdminPassword ctl.dbo.CloudName = ctl.Cloud ctl.dbo.CloudRegion = ctl.CloudRegion ctl.dbo.CloudRegions = make([]dbmodel.CloudRegionControllerPriority, len(ctl.CloudRegions)) diff --git a/internal/testutils/jimmtest/gorm.go b/internal/testutils/jimmtest/gorm.go index f8a24cfd6..7da8d4e6e 100644 --- a/internal/testutils/jimmtest/gorm.go +++ b/internal/testutils/jimmtest/gorm.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. // Package jimmtest contains useful helpers for testing JIMM. package jimmtest @@ -65,7 +65,7 @@ func PostgresDBWithDbName(t Tester, nowFunc func() time.Time) (*gorm.DB, string) templateDatabaseName, _, err := getOrCreateTemplateDatabase() if err != nil { - t.Fatalf("template database does not exist") + t.Fatalf("template database does not exist: %s", err) } suggestedName := "jimm_test_" + t.Name() @@ -322,7 +322,7 @@ func getOrCreateTemplateDatabase() (string, string, error) { templateName, templateDSN, err := createTemplateDatabase() if err != nil { - return "", "", errors.E(err, "error creating template database") + return "", "", fmt.Errorf("error creating template database: %w", err) } templateDatabaseDSN = templateDSN diff --git a/internal/testutils/jimmtest/jimm.go b/internal/testutils/jimmtest/jimm.go index ca1fd8496..027e93173 100644 --- a/internal/testutils/jimmtest/jimm.go +++ b/internal/testutils/jimmtest/jimm.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimmtest @@ -21,6 +21,9 @@ var ( UnsetCredentialStore Option = func(j *jimm.JIMM) { j.CredentialStore = nil } + UsePostgresAsCredentialStore Option = func(j *jimm.JIMM) { + j.CredentialStore = j.Database + } ) func NewJIMM(t Tester, additionalParameters *jimm.Parameters, options ...Option) *jimm.JIMM { diff --git a/internal/testutils/jimmtest/mocks/jimm_controller_mock.go b/internal/testutils/jimmtest/mocks/jimm_controller_mock.go index 57bf530ac..9f54810b1 100644 --- a/internal/testutils/jimmtest/mocks/jimm_controller_mock.go +++ b/internal/testutils/jimmtest/mocks/jimm_controller_mock.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package mocks @@ -9,12 +9,13 @@ import ( "github.com/canonical/jimm/v3/internal/dbmodel" "github.com/canonical/jimm/v3/internal/errors" + "github.com/canonical/jimm/v3/internal/jimm" "github.com/canonical/jimm/v3/internal/openfga" ) // ControllerService is an implementation of the jujuapi.ControllerService interface. type ControllerService struct { - AddController_ func(ctx context.Context, u *openfga.User, ctl *dbmodel.Controller) error + AddController_ func(ctx context.Context, u *openfga.User, ctl *dbmodel.Controller, creds jimm.ControllerCreds) error ControllerInfo_ func(ctx context.Context, name string) (*dbmodel.Controller, error) EarliestControllerVersion_ func(ctx context.Context) (version.Number, error) ListControllers_ func(ctx context.Context, user *openfga.User) ([]dbmodel.Controller, error) @@ -22,11 +23,11 @@ type ControllerService struct { SetControllerDeprecated_ func(ctx context.Context, user *openfga.User, controllerName string, deprecated bool) error } -func (j *ControllerService) AddController(ctx context.Context, u *openfga.User, ctl *dbmodel.Controller) error { +func (j *ControllerService) AddController(ctx context.Context, u *openfga.User, ctl *dbmodel.Controller, creds jimm.ControllerCreds) error { if j.AddController_ == nil { return errors.E(errors.CodeNotImplemented) } - return j.AddController_(ctx, u, ctl) + return j.AddController_(ctx, u, ctl, creds) } func (j *ControllerService) ControllerInfo(ctx context.Context, name string) (*dbmodel.Controller, error) { diff --git a/internal/testutils/jimmtest/suite.go b/internal/testutils/jimmtest/suite.go index 9f3691b8d..19ad641c1 100644 --- a/internal/testutils/jimmtest/suite.go +++ b/internal/testutils/jimmtest/suite.go @@ -278,12 +278,14 @@ func (s *JIMMSuite) NewUser(u *dbmodel.Identity) *openfga.User { func (s *JIMMSuite) AddController(c *gc.C, name string, info *api.Info) { ctl := &dbmodel.Controller{ - UUID: info.ControllerUUID, - Name: name, + UUID: info.ControllerUUID, + Name: name, + CACertificate: info.CACert, + Addresses: nil, + } + ctlCreds := jimm.ControllerCreds{ AdminIdentityName: info.Tag.Id(), AdminPassword: info.Password, - CACertificate: info.CACert, - Addresses: nil, } ctl.Addresses = make(dbmodel.HostPorts, 0, len(info.Addrs)) for _, addr := range info.Addrs { @@ -294,7 +296,7 @@ func (s *JIMMSuite) AddController(c *gc.C, name string, info *api.Info) { Port: hp.Port(), }}) } - err := s.JIMM.AddController(context.Background(), s.AdminUser, ctl) + err := s.JIMM.AddController(context.Background(), s.AdminUser, ctl, ctlCreds) c.Assert(err, gc.Equals, nil) } From de3895e591160b854ea47779333915349eaa9359 Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Thu, 9 Jan 2025 12:36:12 +0200 Subject: [PATCH 2/3] chore: fix tests that require credential attributes --- cmd/jimmctl/cmd/listcontrollers_test.go | 5 +- internal/db/cloudcredential_test.go | 6 - internal/db/secrets.go | 5 +- internal/dbmodel/controller_test.go | 3 +- internal/jimm/cloudcredential_test.go | 96 ++++++++------ internal/jimm/controller_test.go | 12 +- internal/jimm/model_test.go | 165 +++++++++++++----------- internal/jimm/service_account_test.go | 6 +- internal/testutils/jimmtest/jimm.go | 9 -- pkg/api/params/params.go | 6 +- 10 files changed, 167 insertions(+), 146 deletions(-) diff --git a/cmd/jimmctl/cmd/listcontrollers_test.go b/cmd/jimmctl/cmd/listcontrollers_test.go index 3d519e54e..c704efae5 100644 --- a/cmd/jimmctl/cmd/listcontrollers_test.go +++ b/cmd/jimmctl/cmd/listcontrollers_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package cmd_test @@ -23,7 +23,6 @@ var ( -----END CERTIFICATE----- cloudtag: cloud-` + jimmtest.TestCloudName + ` cloudregion: ` + jimmtest.TestCloudRegionName + ` - username: admin agentversion: .* status: status: available @@ -41,7 +40,6 @@ var ( -----END CERTIFICATE----- cloudtag: cloud-` + jimmtest.TestCloudName + ` cloudregion: ` + jimmtest.TestCloudRegionName + ` - username: admin agentversion: .* status: status: available @@ -57,7 +55,6 @@ var ( cacertificate: "" cloudtag: "" cloudregion: "" - username: "" agentversion: .* status: status: available diff --git a/internal/db/cloudcredential_test.go b/internal/db/cloudcredential_test.go index 774fae361..c534a1806 100644 --- a/internal/db/cloudcredential_test.go +++ b/internal/db/cloudcredential_test.go @@ -209,15 +209,9 @@ cloud-credentials: - name: cred-1 cloud: cloud-1 owner: alice@canonical.com - attributes: - k1: v1 - k2: v2 - name: cred-2 cloud: cloud-1 owner: bob@canonical.com - attributes: - k1: v1 - k2: v2 - name: cred-3 cloud: cloud-2 owner: alice@canonical.com diff --git a/internal/db/secrets.go b/internal/db/secrets.go index 7d73b1ccf..903546297 100644 --- a/internal/db/secrets.go +++ b/internal/db/secrets.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package db @@ -126,6 +126,9 @@ func (d *Database) Get(ctx context.Context, tag names.CloudCredentialTag) (_ map secret := dbmodel.NewSecret(tag.Kind(), tag.String(), nil) err = d.GetSecret(ctx, &secret) if err != nil { + if errors.ErrorCode(err) == errors.CodeNotFound { + return nil, nil + } zapctx.Error(ctx, "failed to get secret data", zap.Error(err)) return nil, errors.E(op, err) } diff --git a/internal/dbmodel/controller_test.go b/internal/dbmodel/controller_test.go index 1faa5f245..4e620e571 100644 --- a/internal/dbmodel/controller_test.go +++ b/internal/dbmodel/controller_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package dbmodel_test @@ -179,7 +179,6 @@ func TestToAPIControllerInfo(t *testing.T) { CACertificate: "ca-cert", CloudTag: names.NewCloudTag("test-cloud").String(), CloudRegion: "test-region", - Username: "admin", AgentVersion: "1.2.3", Status: jujuparams.EntityStatus{ Status: "available", diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index a0530785b..5615aefab 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -23,6 +23,7 @@ import ( "github.com/canonical/jimm/v3/internal/openfga" ofganames "github.com/canonical/jimm/v3/internal/openfga/names" "github.com/canonical/jimm/v3/internal/testutils/jimmtest" + "github.com/canonical/jimm/v3/internal/vault" ) func TestUpdateCloudCredential(t *testing.T) { @@ -771,7 +772,6 @@ func TestUpdateCloudCredential(t *testing.T) { API: api, }, }, - jimmtest.UsePostgresAsCredentialStore, // this test relies on credential attributes being stored in postgres ) u, arg, expectedCredential, expectedError := test.createEnv(c, j) @@ -1365,15 +1365,9 @@ cloud-credentials: - name: cred-1 cloud: cloud-1 owner: alice@canonical.com - attributes: - k1: v1 - k2: v2 - name: cred-2 cloud: cloud-1 owner: bob@canonical.com - attributes: - k1: v1 - k2: v2 - name: cred-3 cloud: cloud-2 owner: alice@canonical.com @@ -1474,11 +1468,6 @@ cloud-credentials: cloud: test-cloud owner: bob@canonical.com auth-type: oauth2 - attributes: - client-email: bob@example.com - client-id: 1234 - private-key: super-secret - project-id: 5678 - name: cred-2 cloud: test-cloud owner: bob@canonical.com @@ -1495,6 +1484,7 @@ var getCloudCredentialAttributesTests = []struct { hidden bool jimmAdmin bool cred string + skipAttributes bool expectAttributes map[string]string expectRedacted []string expectError string @@ -1515,6 +1505,7 @@ var getCloudCredentialAttributesTests = []struct { username: "bob@canonical.com", jimmAdmin: true, cred: "cred-2", + skipAttributes: true, expectAttributes: map[string]string{}, expectRedacted: nil, }, { @@ -1556,37 +1547,66 @@ var getCloudCredentialAttributesTests = []struct { }} func TestGetCloudCredentialAttributes(t *testing.T) { - c := qt.New(t) + attributes := map[string]string{ + "client-email": "bob@example.com", + "client-id": "1234", + "private-key": "super-secret", + "project-id": "5678", + } for _, test := range getCloudCredentialAttributesTests { - c.Run(test.name, func(c *qt.C) { - ctx := context.Background() - - j := jimmtest.NewJIMM(c, nil) - - env := jimmtest.ParseEnvironment(c, getCloudCredentialAttributesEnv) - env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, j.OpenFGAClient) - u := env.User("bob@canonical.com").DBObject(c, j.Database) - userBob := openfga.NewUser(&u, j.OpenFGAClient) - credTag := fmt.Sprintf("test-cloud/bob@canonical.com/%s", test.cred) - cred, err := j.GetCloudCredential(ctx, userBob, names.NewCloudCredentialTag(credTag)) - c.Assert(err, qt.IsNil) + c := qt.New(t) + // Run each test twice, once with Vault as a credential store + // and again with Postgres as a credential store. + client, path, roleID, roleSecretID, ok := jimmtest.VaultClient(c) + c.Assert(ok, qt.IsTrue) + vaultStore := &vault.VaultStore{ + Client: client, + RoleID: roleID, + RoleSecretID: roleSecretID, + KVPath: path, + } + jimmWithVault := jimm.Parameters{CredentialStore: vaultStore} + + testF := func(jp *jimm.Parameters) func(c *qt.C) { + return func(c *qt.C) { + ctx := context.Background() + + j := jimmtest.NewJIMM(c, jp) + + env := jimmtest.ParseEnvironment(c, getCloudCredentialAttributesEnv) + env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, j.OpenFGAClient) + + u := env.User("bob@canonical.com").DBObject(c, j.Database) + userBob := openfga.NewUser(&u, j.OpenFGAClient) + + credTag := names.NewCloudCredentialTag(fmt.Sprintf("test-cloud/bob@canonical.com/%s", test.cred)) + cred, err := j.GetCloudCredential(ctx, userBob, credTag) + c.Assert(err, qt.IsNil) + + if !test.skipAttributes { + err = j.CredentialStore.Put(ctx, credTag, attributes) + c.Assert(err, qt.IsNil) + } - u = env.User(test.username).DBObject(c, j.Database) - userTest := openfga.NewUser(&u, j.OpenFGAClient) - userTest.JimmAdmin = test.jimmAdmin - attr, redacted, err := j.GetCloudCredentialAttributes(ctx, userTest, cred, test.hidden) - if test.expectError != "" { - c.Check(err, qt.ErrorMatches, test.expectError) - if test.expectErrorCode != "" { - c.Check(errors.ErrorCode(err), qt.Equals, test.expectErrorCode) + u = env.User(test.username).DBObject(c, j.Database) + userTest := openfga.NewUser(&u, j.OpenFGAClient) + userTest.JimmAdmin = test.jimmAdmin + attr, redacted, err := j.GetCloudCredentialAttributes(ctx, userTest, cred, test.hidden) + if test.expectError != "" { + c.Check(err, qt.ErrorMatches, test.expectError) + if test.expectErrorCode != "" { + c.Check(errors.ErrorCode(err), qt.Equals, test.expectErrorCode) + } + return } - return + c.Assert(err, qt.IsNil) + c.Check(attr, qt.DeepEquals, test.expectAttributes) + c.Check(redacted, qt.DeepEquals, test.expectRedacted) } - c.Assert(err, qt.IsNil) - c.Check(attr, qt.DeepEquals, test.expectAttributes) - c.Check(redacted, qt.DeepEquals, test.expectRedacted) - }) + } + c.Run(test.name+"-postgres", testF(nil)) + c.Run(test.name+"-vault", testF(&jimmWithVault)) } } diff --git a/internal/jimm/controller_test.go b/internal/jimm/controller_test.go index f11ae42e4..abe6af367 100644 --- a/internal/jimm/controller_test.go +++ b/internal/jimm/controller_test.go @@ -237,12 +237,14 @@ func TestAddControllerWithCloudWithoutRegions(t *testing.T) { c.Assert(err, qt.IsNil) ctl1 := dbmodel.Controller{ - Name: "test-controller", - AdminIdentityName: "admin", - AdminPassword: "5ecret", - PublicAddress: "example.com:443", + Name: "test-controller", + PublicAddress: "example.com:443", + } + ctlCreds := jimm.ControllerCreds{ + AdminIdentityName: "user", + AdminPassword: "secret", } - err = j.AddController(context.Background(), alice, &ctl1) + err = j.AddController(context.Background(), alice, &ctl1, ctlCreds) c.Assert(err, qt.IsNil) ctl2 := dbmodel.Controller{ diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go index fdaa23d99..c3945ab4e 100644 --- a/internal/jimm/model_test.go +++ b/internal/jimm/model_test.go @@ -138,9 +138,12 @@ var addModelTests = []struct { createModel func(ctx context.Context, args *jujuparams.ModelCreateArgs, mi *jujuparams.ModelInfo) error username string jimmAdmin bool - args jujuparams.ModelCreateArgs - expectModel dbmodel.Model - expectError string + // This cloudCredTag is used to manually populate a dummy cloud credential + // into JIMM's credential store and then applied onto args before adding a model. + cloudCredTag names.CloudCredentialTag + args jujuparams.ModelCreateArgs + expectModel dbmodel.Model + expectError string }{{ name: "CreateModelWithCloudRegion", env: ` @@ -208,14 +211,14 @@ users: - user: bob access: read `[1:])), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectModel: dbmodel.Model{ Name: "test-model", @@ -312,15 +315,15 @@ users: - user: bob access: read `[1:])), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ Name: "test-model", OwnerTag: names.NewUserTag("alice@canonical.com").String(), CloudTag: names.NewCloudTag("test-cloud").String(), // Creating a model without specifying the cloud region - CloudRegion: "", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + CloudRegion: "", }, expectModel: dbmodel.Model{ Name: "test-model", @@ -417,14 +420,14 @@ users: - user: bob access: read `[1:])), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectModel: dbmodel.Model{ Name: "test-model", @@ -513,14 +516,14 @@ users: - user: bob access: read `[1:]), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("bob@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("bob@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectModel: dbmodel.Model{ Name: "test-model", @@ -607,13 +610,13 @@ users: - user: bob access: read `[1:]), - username: "alice@canonical.com", + username: "alice@canonical.com", + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("bob@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("bob@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectError: "unauthorized", }, { @@ -659,14 +662,14 @@ controllers: createModel: func(ctx context.Context, args *jujuparams.ModelCreateArgs, mi *jujuparams.ModelInfo) error { return errors.E("a test error") }, - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectError: "a test error", }, { @@ -737,14 +740,14 @@ users: - user: bob access: read `[1:]), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectError: "model alice@canonical.com/test-model already exists", }, { @@ -799,14 +802,14 @@ users: - user: bob access: read `[1:]), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectError: "failed to update cloud credential: a silly error", }, { @@ -856,14 +859,14 @@ users: - user: alice@canonical.com access: admin `[1:]), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), + CloudTag: names.NewCloudTag("test-cloud").String(), + CloudRegion: "test-region-1", }, expectError: "unauthorized", }, { @@ -936,12 +939,12 @@ users: - user: bob access: read `[1:])), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), }, expectModel: dbmodel.Model{ Name: "test-model", @@ -1043,12 +1046,12 @@ users: - user: bob access: read `[1:])), - username: "alice@canonical.com", - jimmAdmin: true, + username: "alice@canonical.com", + jimmAdmin: true, + cloudCredTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1"), args: jujuparams.ModelCreateArgs{ - Name: "test-model", - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + Name: "test-model", + OwnerTag: names.NewUserTag("alice@canonical.com").String(), }, expectError: "no cloud specified for model; please specify one", }} @@ -1073,12 +1076,16 @@ func TestAddModel(t *testing.T) { env := jimmtest.ParseEnvironment(c, test.env) env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, j.OpenFGAClient) + err := j.CredentialStore.Put(ctx, test.cloudCredTag, map[string]string{"key": "value"}) + c.Assert(err, qt.IsNil) + dbUser := env.User(test.username).DBObject(c, j.Database) user := openfga.NewUser(&dbUser, j.OpenFGAClient) user.JimmAdmin = test.jimmAdmin + test.args.CloudCredentialTag = test.cloudCredTag.String() args := jimm.ModelCreateArgs{} - err := args.FromJujuModelCreateArgs(&test.args) + err = args.FromJujuModelCreateArgs(&test.args) c.Assert(err, qt.IsNil) _, err = j.AddModel(context.Background(), user, &args) @@ -2683,7 +2690,11 @@ func TestUpdateModelCredential(t *testing.T) { dbUser := env.User(test.username).DBObject(c, j.Database) user := openfga.NewUser(&dbUser, j.OpenFGAClient) - err := j.ChangeModelCredential( + testAttributes := map[string]string{"key": "value"} + err := j.CredentialStore.Put(ctx, names.NewCloudCredentialTag(test.credential), testAttributes) + c.Assert(err, qt.IsNil) + + err = j.ChangeModelCredential( ctx, user, names.NewModelTag(test.uuid), @@ -2799,13 +2810,17 @@ controllers: err = j.Database.DeleteController(ctx, &controller) c.Assert(err, qt.IsNil) + cloudCredTag := names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1") + err = j.CredentialStore.Put(ctx, cloudCredTag, map[string]string{"key": "value"}) + c.Assert(err, qt.IsNil) + args := jimm.ModelCreateArgs{} err = args.FromJujuModelCreateArgs(&jujuparams.ModelCreateArgs{ Name: "test-model", OwnerTag: names.NewUserTag("alice@canonical.com").String(), CloudTag: names.NewCloudTag("test-cloud").String(), CloudRegion: "test-region-1", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/test-credential-1").String(), + CloudCredentialTag: cloudCredTag.String(), }) c.Assert(err, qt.IsNil) diff --git a/internal/jimm/service_account_test.go b/internal/jimm/service_account_test.go index c2b9b1495..bd5d875dd 100644 --- a/internal/jimm/service_account_test.go +++ b/internal/jimm/service_account_test.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package jimm_test @@ -114,6 +114,10 @@ func TestCopyServiceAccountCredential(t *testing.T) { err = j.Database.SetCloudCredential(context.Background(), &cred) c.Assert(err, qt.Equals, nil) + credAttr := map[string]string{"key": "value"} + err = j.CredentialStore.Put(ctx, cred.ResourceTag(), credAttr) + c.Assert(err, qt.Equals, nil) + _, res, err := j.CopyServiceAccountCredential(ctx, user, svcAcc, cred.ResourceTag()) c.Assert(err, qt.Equals, nil) newCred := dbmodel.CloudCredential{ diff --git a/internal/testutils/jimmtest/jimm.go b/internal/testutils/jimmtest/jimm.go index 027e93173..b58573828 100644 --- a/internal/testutils/jimmtest/jimm.go +++ b/internal/testutils/jimmtest/jimm.go @@ -17,15 +17,6 @@ var now = (time.Time{}).UTC().Round(time.Millisecond) type Option func(j *jimm.JIMM) -var ( - UnsetCredentialStore Option = func(j *jimm.JIMM) { - j.CredentialStore = nil - } - UsePostgresAsCredentialStore Option = func(j *jimm.JIMM) { - j.CredentialStore = j.Database - } -) - func NewJIMM(t Tester, additionalParameters *jimm.Parameters, options ...Option) *jimm.JIMM { auth := NewMockOAuthAuthenticator(t, nil) diff --git a/pkg/api/params/params.go b/pkg/api/params/params.go index b3a2ecf83..64d899df1 100644 --- a/pkg/api/params/params.go +++ b/pkg/api/params/params.go @@ -1,4 +1,4 @@ -// Copyright 2024 Canonical. +// Copyright 2025 Canonical. package params @@ -168,10 +168,6 @@ type ControllerInfo struct { // CloudRegion is the region that this controller is running in. CloudRegion string `json:"cloud-region,omitempty"` - // Username contains the username that JIMM uses to connect to the - // controller. - Username string `json:"username"` - // The version of the juju agent running on the controller. AgentVersion string `json:"agent-version"` From 93a83746162148e5e61e54a83e9aaaf9a91bc99f Mon Sep 17 00:00:00 2001 From: Kian Parvin Date: Fri, 10 Jan 2025 09:32:16 +0200 Subject: [PATCH 3/3] chore: missing godoc --- internal/jimm/cloudcredential.go | 1 - internal/jimm/types.go | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/jimm/cloudcredential.go b/internal/jimm/cloudcredential.go index 2fc773090..49368bfcd 100644 --- a/internal/jimm/cloudcredential.go +++ b/internal/jimm/cloudcredential.go @@ -247,7 +247,6 @@ func (j *JIMM) updateControllerCloudCredential( ) ([]jujuparams.UpdateCredentialModelResult, error) { const op = errors.Op("jimm.updateControllerCloudCredential") - var err error attr, err := j.getCloudCredentialAttributes(ctx, cred) if err != nil { return nil, errors.E(op, err) diff --git a/internal/jimm/types.go b/internal/jimm/types.go index c11868864..7ac4d860c 100644 --- a/internal/jimm/types.go +++ b/internal/jimm/types.go @@ -1,6 +1,9 @@ // Copyright 2025 Canonical. + package jimm +// ControllerCreds represent the admin username and password +// used to authenticate with a Juju controller via basic auth. type ControllerCreds struct { AdminIdentityName string AdminPassword string