From 8eeafecff40588467a9f03dee2433f49f607b13e Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Wed, 8 May 2024 15:49:28 +1000 Subject: [PATCH 1/8] Implement SecretRotated service call; --- domain/secret/service/service.go | 37 ++++++++ domain/secret/service/service_test.go | 115 +++++++++++++++++++++++ domain/secret/service/state_mock_test.go | 15 +++ domain/secret/service/watcher.go | 38 -------- domain/secret/state/state.go | 24 ++++- domain/secret/state/state_test.go | 25 +++++ domain/secret/state/types.go | 6 +- 7 files changed, 214 insertions(+), 46 deletions(-) diff --git a/domain/secret/service/service.go b/domain/secret/service/service.go index c4a1285e84b..6fa509adf85 100644 --- a/domain/secret/service/service.go +++ b/domain/secret/service/service.go @@ -6,6 +6,7 @@ package service import ( "context" "fmt" + "time" "github.com/juju/clock" "github.com/juju/errors" @@ -67,6 +68,7 @@ type State interface { GetRevisionIDsForObsolete( ctx context.Context, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, revisionUUID ...string, ) ([]string, error) + SecretRotated(ctx context.Context, uri *secrets.URI, next time.Time) error // For watching consumed local secret changes. InitialWatchStatementForConsumedSecretsChange(unitName string) (string, eventsource.NamespaceQuery) @@ -638,3 +640,38 @@ func (s *SecretService) ChangeSecretBackend(ctx context.Context, uri *secrets.UR // TODO(secrets) return nil } + +// SecretRotated rotates the secret with the specified URI. +func (s *SecretService) SecretRotated(ctx context.Context, uri *secrets.URI, params SecretRotatedParams) error { + if err := s.canManage(ctx, uri, params.Accessor, params.LeaderToken); err != nil { + return errors.Trace(err) + } + + md, err := s.GetSecret(ctx, uri) + if err != nil { + return errors.Trace(err) + } + if !md.RotatePolicy.WillRotate() { + s.logger.Debugf("secret %q was rotated but now is set to not rotate") + return nil + } + lastRotateTime := md.NextRotateTime + if lastRotateTime == nil { + now := s.clock.Now() + lastRotateTime = &now + } + nextRotateTime := *md.RotatePolicy.NextRotateTime(*lastRotateTime) + s.logger.Debugf("secret %q was rotated: rev was %d, now %d", uri.ID, params.OriginalRevision, md.LatestRevision) + // If the secret will expire before it is due to be next rotated, rotate sooner to allow + // the charm a chance to update it before it expires. + willExpire := md.LatestExpireTime != nil && md.LatestExpireTime.Before(nextRotateTime) + forcedRotateTime := lastRotateTime.Add(secrets.RotateRetryDelay) + if willExpire { + s.logger.Warningf("secret %q rev %d will expire before next scheduled rotation", uri.ID, md.LatestRevision) + } + if willExpire && forcedRotateTime.Before(*md.LatestExpireTime) || !params.Skip && md.LatestRevision == params.OriginalRevision { + nextRotateTime = forcedRotateTime + } + s.logger.Debugf("secret %q next rotate time is now: %s", uri.ID, nextRotateTime.UTC().Format(time.RFC3339)) + return s.st.SecretRotated(ctx, uri, nextRotateTime) +} diff --git a/domain/secret/service/service_test.go b/domain/secret/service/service_test.go index 4fc8c1ffdf9..6cdffeebfce 100644 --- a/domain/secret/service/service_test.go +++ b/domain/secret/service/service_test.go @@ -1072,6 +1072,121 @@ func (s *serviceSuite) TestGetSecretGrants(c *gc.C) { }}) } +func (s *serviceSuite) TestSecretsRotated(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + uri := coresecrets.NewURI() + ctx := context.Background() + nextRotateTime := time.Now().Add(time.Hour) + + s.state.EXPECT().GetSecretAccess(gomock.Any(), uri, domainsecret.AccessParams{ + SubjectTypeID: domainsecret.SubjectUnit, + SubjectID: "mariadb/0", + }).Return("manage", nil) + s.state.EXPECT().SecretRotated(ctx, uri, nextRotateTime).Return(errors.New("boom")) + s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + RotatePolicy: coresecrets.RotateHourly, + LatestRevision: 667, + }, nil) + + err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + LeaderToken: successfulToken{}, + Accessor: SecretAccessor{ + Kind: UnitAccessor, + ID: "mariadb/0", + }, + OriginalRevision: 666, + }) + c.Assert(err, gc.ErrorMatches, `boom`) +} + +func (s *serviceSuite) TestSecretsRotatedRetry(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + uri := coresecrets.NewURI() + ctx := context.Background() + nextRotateTime := time.Now().Add(coresecrets.RotateRetryDelay) + + s.state.EXPECT().GetSecretAccess(gomock.Any(), uri, domainsecret.AccessParams{ + SubjectTypeID: domainsecret.SubjectUnit, + SubjectID: "mariadb/0", + }).Return("manage", nil) + s.state.EXPECT().SecretRotated(ctx, uri, nextRotateTime).Return(errors.New("boom")) + s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + RotatePolicy: coresecrets.RotateHourly, + LatestRevision: 666, + }, nil) + + err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + LeaderToken: successfulToken{}, + Accessor: SecretAccessor{ + Kind: UnitAccessor, + ID: "mariadb/0", + }, + OriginalRevision: 666, + }) + c.Assert(err, gc.ErrorMatches, `boom`) +} + +func (s *serviceSuite) TestSecretsRotatedForce(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + uri := coresecrets.NewURI() + ctx := context.Background() + nextRotateTime := time.Now().Add(coresecrets.RotateRetryDelay) + + s.state.EXPECT().GetSecretAccess(gomock.Any(), uri, domainsecret.AccessParams{ + SubjectTypeID: domainsecret.SubjectUnit, + SubjectID: "mariadb/0", + }).Return("manage", nil) + s.state.EXPECT().SecretRotated(ctx, uri, nextRotateTime).Return(errors.New("boom")) + s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + RotatePolicy: coresecrets.RotateHourly, + LatestExpireTime: ptr(time.Now().Add(50 * time.Minute)), + LatestRevision: 667, + }, nil) + + err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + LeaderToken: successfulToken{}, + Accessor: SecretAccessor{ + Kind: UnitAccessor, + ID: "mariadb/0", + }, + OriginalRevision: 666, + }) + c.Assert(err, gc.ErrorMatches, `boom`) +} + +func (s *serviceSuite) TestSecretsRotatedThenNever(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + uri := coresecrets.NewURI() + ctx := context.Background() + + s.state.EXPECT().GetSecretAccess(gomock.Any(), uri, domainsecret.AccessParams{ + SubjectTypeID: domainsecret.SubjectUnit, + SubjectID: "mariadb/0", + }).Return("manage", nil) + s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + RotatePolicy: coresecrets.RotateNever, + LatestRevision: 667, + }, nil) + + err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + LeaderToken: successfulToken{}, + Accessor: SecretAccessor{ + Kind: UnitAccessor, + ID: "mariadb/0", + }, + OriginalRevision: 666, + }) + c.Assert(err, jc.ErrorIsNil) +} + /* // TODO(secrets) - tests copied from facade which need to be re-implemented here func (s *serviceSuite) TestGetSecretContentConsumerFirstTime(c *gc.C) { diff --git a/domain/secret/service/state_mock_test.go b/domain/secret/service/state_mock_test.go index b635f1f6e02..e27b5544047 100644 --- a/domain/secret/service/state_mock_test.go +++ b/domain/secret/service/state_mock_test.go @@ -12,6 +12,7 @@ package service import ( context "context" reflect "reflect" + time "time" secrets "github.com/juju/juju/core/secrets" eventsource "github.com/juju/juju/core/watcher/eventsource" @@ -1312,6 +1313,20 @@ func (c *MockStateSaveSecretRemoteConsumerCall) DoAndReturn(f func(context.Conte return c } +// SecretRotated mocks base method. +func (m *MockState) SecretRotated(arg0 context.Context, arg1 *secrets.URI, arg2 time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SecretRotated", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// SecretRotated indicates an expected call of SecretRotated. +func (mr *MockStateMockRecorder) SecretRotated(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SecretRotated", reflect.TypeOf((*MockState)(nil).SecretRotated), arg0, arg1, arg2) +} + // UpdateRemoteSecretRevision mocks base method. func (m *MockState) UpdateRemoteSecretRevision(arg0 context.Context, arg1 *secrets.URI, arg2 int) error { m.ctrl.T.Helper() diff --git a/domain/secret/service/watcher.go b/domain/secret/service/watcher.go index e8fc7e0725f..6927ec6675b 100644 --- a/domain/secret/service/watcher.go +++ b/domain/secret/service/watcher.go @@ -5,7 +5,6 @@ package service import ( "context" - "time" "github.com/juju/clock" "github.com/juju/collections/set" @@ -16,7 +15,6 @@ import ( "github.com/juju/juju/core/changestream" "github.com/juju/juju/core/logger" - "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/eventsource" "github.com/juju/juju/core/watcher/watchertest" @@ -280,42 +278,6 @@ func (s *WatchableService) WatchObsoleteUserSecrets(ctx context.Context) (watche return watchertest.NewMockNotifyWatcher(ch), nil } -func (s *WatchableService) SecretRotated(ctx context.Context, uri *secrets.URI, params SecretRotatedParams) error { - if err := s.canManage(ctx, uri, params.Accessor, params.LeaderToken); err != nil { - return errors.Trace(err) - } - - md, err := s.GetSecret(ctx, uri) - if err != nil { - return errors.Trace(err) - } - if !md.RotatePolicy.WillRotate() { - s.logger.Debugf("secret %q was rotated but now is set to not rotate") - return nil - } - lastRotateTime := md.NextRotateTime - if lastRotateTime == nil { - now := s.clock.Now() - lastRotateTime = &now - } - nextRotateTime := *md.RotatePolicy.NextRotateTime(*lastRotateTime) - s.logger.Debugf("secret %q was rotated: rev was %d, now %d", uri.ID, params.OriginalRevision, md.LatestRevision) - // If the secret will expire before it is due to be next rotated, rotate sooner to allow - // the charm a chance to update it before it expires. - willExpire := md.LatestExpireTime != nil && md.LatestExpireTime.Before(nextRotateTime) - forcedRotateTime := lastRotateTime.Add(secrets.RotateRetryDelay) - if willExpire { - s.logger.Warningf("secret %q rev %d will expire before next scheduled rotation", uri.ID, md.LatestRevision) - } - if willExpire && forcedRotateTime.Before(*md.LatestExpireTime) || !params.Skip && md.LatestRevision == params.OriginalRevision { - nextRotateTime = forcedRotateTime - } - s.logger.Debugf("secret %q next rotate time is now: %s", uri.ID, nextRotateTime.UTC().Format(time.RFC3339)) - - // TODO(secrets) - return nil -} - // WatchSecretBackendChanged notifies when the model secret backend has changed. func (s *WatchableService) WatchSecretBackendChanged(ctx context.Context) (watcher.NotifyWatcher, error) { ch := make(chan struct{}, 1) diff --git a/domain/secret/state/state.go b/domain/secret/state/state.go index 3221f097ab1..01ea71163b9 100644 --- a/domain/secret/state/state.go +++ b/domain/secret/state/state.go @@ -484,7 +484,7 @@ WHERE sm.secret_id = $secretID.id } var ( - dbSecrets secrets + dbSecrets secretInfos dbsecretOwners []secretOwner ) secretIDParam := secretID{ID: uri.ID} @@ -1112,7 +1112,7 @@ FROM secret_metadata sm } var ( - dbSecrets secrets + dbSecrets secretInfos dbsecretOwners []secretOwner ) err = tx.Query(ctx, queryStmt, queryParams...).GetAll(&dbSecrets, &dbsecretOwners) @@ -1255,7 +1255,7 @@ FROM secret_metadata sm } var ( - dbSecrets secrets + dbSecrets secretInfos dbsecretOwners []secretOwner ) err = tx.Query(ctx, queryStmt, queryParams...).GetAll(&dbSecrets, &dbsecretOwners) @@ -1423,7 +1423,7 @@ WHERE mso.label = $M.label return nil, errors.Trace(err) } - var dbSecrets secrets + var dbSecrets secretInfos if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { err := tx.Query(ctx, queryStmt, sqlair.M{"label": label}).GetAll(&dbSecrets) if err != nil && !errors.Is(err, sqlair.ErrNoRows) { @@ -2506,7 +2506,7 @@ AND (subject_type_id = $secretAccessorType.unit_type_id AND subject_id IN ($u if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { var err error var ( - dbSecrets secrets + dbSecrets secretInfos dbValueRefs secretValueRefs ) err = tx.Query(ctx, queryStmt, queryParams...).GetAll(&dbSecrets, &dbValueRefs) @@ -3131,3 +3131,17 @@ DELETE FROM secret WHERE id = $secretID.id` } return nil } + +// SecretRotated updates the next rotation time for the specified secret. +func (st State) SecretRotated(ctx context.Context, uri *coresecrets.URI, next time.Time) error { + db, err := st.DB() + if err != nil { + return errors.Trace(err) + } + + err = db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + err := st.upsertSecretNextRotateTime(ctx, tx, uri, next) + return errors.Trace(err) + }) + return errors.Trace(domain.CoerceError(err)) +} diff --git a/domain/secret/state/state_test.go b/domain/secret/state/state_test.go index 757ee09f57e..96f99cf574a 100644 --- a/domain/secret/state/state_test.go +++ b/domain/secret/state/state_test.go @@ -3105,3 +3105,28 @@ func (s *stateSuite) TestGetRemoteConsumedSecretURIsWithChangesFromOfferingSide( uri1.String(), }) } + +func (s *stateSuite) TestSecretRotated(c *gc.C) { + st := newSecretState(c, s.TxnRunnerFactory()) + ctx := context.Background() + + s.setupUnits(c, "mysql") + uri := coresecrets.NewURI() + err := st.CreateCharmApplicationSecret(ctx, 1, uri, "mysql", domainsecret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo": "bar", "hello": "world"}, + }) + c.Assert(err, jc.ErrorIsNil) + + next := time.Now().Add(1 * time.Hour) + err = st.SecretRotated(ctx, uri, next) + c.Assert(err, jc.ErrorIsNil) + + row := s.DB().QueryRowContext(context.Background(), ` +SELECT next_rotation_time +FROM secret_rotation +WHERE secret_id = ?`, uri.ID) + var nextRotationTime time.Time + err = row.Scan(&nextRotationTime) + c.Assert(err, jc.ErrorIsNil) + c.Assert(nextRotationTime.Equal(next), jc.IsTrue) +} diff --git a/domain/secret/state/types.go b/domain/secret/state/types.go index 2346133d506..e5fdad3d871 100644 --- a/domain/secret/state/types.go +++ b/domain/secret/state/types.go @@ -197,9 +197,9 @@ var ownerKindParam = ownerKind{ Application: string(coresecrets.ApplicationOwner), } -type secrets []secretInfo +type secretInfos []secretInfo -func (rows secrets) toSecretMetadata(secretOwners []secretOwner) ([]*coresecrets.SecretMetadata, error) { +func (rows secretInfos) toSecretMetadata(secretOwners []secretOwner) ([]*coresecrets.SecretMetadata, error) { if len(rows) != len(secretOwners) { // Should never happen. return nil, errors.New("row length mismatch composing secret results") @@ -236,7 +236,7 @@ func (rows secrets) toSecretMetadata(secretOwners []secretOwner) ([]*coresecrets return result, nil } -func (rows secrets) toSecretRevisionRef(refs secretValueRefs) ([]*coresecrets.SecretRevisionRef, error) { +func (rows secretInfos) toSecretRevisionRef(refs secretValueRefs) ([]*coresecrets.SecretRevisionRef, error) { if len(rows) != len(refs) { // Should never happen. return nil, errors.New("row length mismatch composing secret results") From 5bb4f0eef4b1b8a5ce170d6f6bed5691f9001974 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Wed, 8 May 2024 17:36:38 +1000 Subject: [PATCH 2/8] Fix the missing next rotation time during dating secret process; --- domain/secret/service/service.go | 9 +++++++++ domain/secret/service/service_test.go | 22 +++++++++++++++++----- domain/secret/state/state.go | 20 +++++++++++++------- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/domain/secret/service/service.go b/domain/secret/service/service.go index 6fa509adf85..75acabb976d 100644 --- a/domain/secret/service/service.go +++ b/domain/secret/service/service.go @@ -361,6 +361,15 @@ func (s *SecretService) UpdateCharmSecret(ctx context.Context, uri *secrets.URI, } rotatePolicy := domainsecret.MarshallRotatePolicy(params.RotatePolicy) p.RotatePolicy = &rotatePolicy + if params.RotatePolicy.WillRotate() { + md, err := s.GetSecret(ctx, uri) + if err != nil { + return errors.Trace(err) + } + if !md.RotatePolicy.WillRotate() { + p.NextRotateTime = params.RotatePolicy.NextRotateTime(s.clock.Now()) + } + } if len(params.Data) > 0 { p.Data = make(map[string]string) for k, v := range params.Data { diff --git a/domain/secret/service/service_test.go b/domain/secret/service/service_test.go index 6cdffeebfce..fcee2bc12d7 100644 --- a/domain/secret/service/service_test.go +++ b/domain/secret/service/service_test.go @@ -395,10 +395,11 @@ func (s *serviceSuite) TestUpdateCharmSecret(c *gc.C) { uri := coresecrets.NewURI() p := domainsecret.UpsertSecretParams{ - RotatePolicy: ptr(domainsecret.RotateDaily), - Description: ptr("a secret"), - Label: ptr("my secret"), - Data: coresecrets.SecretData{"foo": "bar"}, + RotatePolicy: ptr(domainsecret.RotateDaily), + Description: ptr("a secret"), + Label: ptr("my secret"), + Data: coresecrets.SecretData{"foo": "bar"}, + NextRotateTime: ptr(time.Now().AddDate(0, 0, 1)), } s.state = NewMockState(ctrl) @@ -406,7 +407,18 @@ func (s *serviceSuite) TestUpdateCharmSecret(c *gc.C) { SubjectTypeID: domainsecret.SubjectUnit, SubjectID: "mariadb/0", }).Return("manage", nil) - s.state.EXPECT().UpdateSecret(gomock.Any(), uri, p).Return(nil) + s.state.EXPECT().GetSecret(gomock.Any(), uri).Return(&coresecrets.SecretMetadata{ + // No rotate policy. + }, nil) + s.state.EXPECT().UpdateSecret(gomock.Any(), uri, gomock.Any()).DoAndReturn(func(_ context.Context, _ *coresecrets.URI, got domainsecret.UpsertSecretParams) error { + c.Assert(got.NextRotateTime, gc.NotNil) + c.Assert(*got.NextRotateTime, jc.Almost, *p.NextRotateTime) + got.NextRotateTime = nil + want := p + want.NextRotateTime = nil + c.Assert(got, jc.DeepEquals, want) + return nil + }) err := s.service(c).UpdateCharmSecret(context.Background(), uri, UpdateCharmSecretParams{ LeaderToken: successfulToken{}, diff --git a/domain/secret/state/state.go b/domain/secret/state/state.go index 01ea71163b9..2f0fb7d8299 100644 --- a/domain/secret/state/state.go +++ b/domain/secret/state/state.go @@ -496,14 +496,15 @@ WHERE sm.secret_id = $secretID.id return errors.Trace(err) } - existing, err := dbSecrets.toSecretMetadata(dbsecretOwners) + existingResult, err := dbSecrets.toSecretMetadata(dbsecretOwners) if err != nil { return errors.Trace(err) } + existing := existingResult[0] // Check to be sure a duplicate label won't be used. var checkExists checkExistsFunc - switch kind := existing[0].Owner.Kind; kind { + switch kind := existing.Owner.Kind; kind { case coresecrets.ModelOwner: checkExists = st.checkUserSecretLabelExists case coresecrets.ApplicationOwner: @@ -511,13 +512,13 @@ WHERE sm.secret_id = $secretID.id return secreterrors.AutoPruneNotSupported } // Query selects the app uuid as owner id. - checkExists = st.checkApplicationSecretLabelExists(existing[0].Owner.ID) + checkExists = st.checkApplicationSecretLabelExists(existing.Owner.ID) case coresecrets.UnitOwner: if secret.AutoPrune != nil && *secret.AutoPrune { return secreterrors.AutoPruneNotSupported } // Query selects the unit uuid as owner id. - checkExists = st.checkUnitSecretLabelExists(existing[0].Owner.ID) + checkExists = st.checkUnitSecretLabelExists(existing.Owner.ID) default: // Should never happen. return errors.Errorf("unexpected secret owner kind %q", kind) @@ -535,7 +536,7 @@ WHERE sm.secret_id = $secretID.id Version: dbSecrets[0].Version, Description: dbSecrets[0].Description, AutoPrune: dbSecrets[0].AutoPrune, - RotatePolicyID: int(domainsecret.MarshallRotatePolicy(&existing[0].RotatePolicy)), + RotatePolicyID: int(domainsecret.MarshallRotatePolicy(&existing.RotatePolicy)), UpdateTime: now, } dbSecret.UpdateTime = now @@ -545,7 +546,7 @@ WHERE sm.secret_id = $secretID.id } if secret.Label != nil { - if err := st.upsertSecretLabel(ctx, tx, existing[0].URI, *secret.Label, existing[0].Owner); err != nil { + if err := st.upsertSecretLabel(ctx, tx, existing.URI, *secret.Label, existing.Owner); err != nil { return errors.Annotatef(err, "updating label for secret %q", uri) } } @@ -562,6 +563,11 @@ WHERE sm.secret_id = $secretID.id return errors.Annotatef(err, "deleting next rotate record for secret %q", uri) } } + if secret.NextRotateTime != nil { + if err := st.upsertSecretNextRotateTime(ctx, tx, uri, *secret.NextRotateTime); err != nil { + return errors.Annotatef(err, "updating next rotate time for secret %q", uri) + } + } if len(secret.Data) == 0 && secret.ValueRef == nil { return nil @@ -572,7 +578,7 @@ WHERE sm.secret_id = $secretID.id return errors.Trace(err) } - nextRevision := existing[0].LatestRevision + 1 + nextRevision := existing.LatestRevision + 1 dbRevision := secretRevision{ ID: revisionUUID.String(), SecretID: uri.ID, From 992a3fe6b8c08c6fe3b1bf2db70ae8a5074ad897 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Mon, 13 May 2024 12:54:38 +1000 Subject: [PATCH 3/8] Add GetRotatePolicy for getting retation dolicy; --- domain/secret/service/service.go | 5 +++-- domain/secret/state/state.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/domain/secret/service/service.go b/domain/secret/service/service.go index 75acabb976d..169ed9aa1de 100644 --- a/domain/secret/service/service.go +++ b/domain/secret/service/service.go @@ -69,6 +69,7 @@ type State interface { ctx context.Context, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, revisionUUID ...string, ) ([]string, error) SecretRotated(ctx context.Context, uri *secrets.URI, next time.Time) error + GetRotatePolicy(ctx context.Context, uri *secrets.URI) (secrets.RotatePolicy, error) // For watching consumed local secret changes. InitialWatchStatementForConsumedSecretsChange(unitName string) (string, eventsource.NamespaceQuery) @@ -362,11 +363,11 @@ func (s *SecretService) UpdateCharmSecret(ctx context.Context, uri *secrets.URI, rotatePolicy := domainsecret.MarshallRotatePolicy(params.RotatePolicy) p.RotatePolicy = &rotatePolicy if params.RotatePolicy.WillRotate() { - md, err := s.GetSecret(ctx, uri) + policy, err := s.st.GetRotatePolicy(ctx, uri) if err != nil { return errors.Trace(err) } - if !md.RotatePolicy.WillRotate() { + if !policy.WillRotate() { p.NextRotateTime = params.RotatePolicy.NextRotateTime(s.clock.Now()) } } diff --git a/domain/secret/state/state.go b/domain/secret/state/state.go index 2f0fb7d8299..617b943d3da 100644 --- a/domain/secret/state/state.go +++ b/domain/secret/state/state.go @@ -1051,6 +1051,34 @@ func (st State) GetSecret(ctx context.Context, uri *coresecrets.URI) (*coresecre return secrets[0], nil } +// GetRotatePolicy returns the rotate policy for the specified secret. +func (st State) GetRotatePolicy(ctx context.Context, uri *coresecrets.URI) (coresecrets.RotatePolicy, error) { + db, err := st.DB() + if err != nil { + return "", errors.Trace(err) + } + stmt, err := st.Prepare(` +SELECT srp.policy +FROM secret_metadata sm +INNER JOIN secret_rotate_policy srp ON srp.id = sm.rotate_policy_id +WHERE sm.secret_id = $secretID.id`, secretID{}) + if err != nil { + return "", errors.Trace(err) + } + + var rotatePolicy string + if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + err := tx.Query(ctx, stmt, secretID{ID: uri.ID}).Get(&rotatePolicy) + if errors.Is(err, sqlair.ErrNoRows) { + return fmt.Errorf("rotate policy for %q not found%w", uri, errors.Hide(secreterrors.SecretNotFound)) + } + return errors.Trace(err) + }); err != nil { + return "", errors.Trace(domain.CoerceError(err)) + } + return coresecrets.RotatePolicy(rotatePolicy), nil +} + func (st State) listSecretsAnyOwner( ctx context.Context, tx *sqlair.TX, uri *coresecrets.URI, ) ([]*coresecrets.SecretMetadata, error) { From 7b45fbbb844a9adfa72d03b90f96be753ec92e3e Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Tue, 14 May 2024 18:01:59 +1000 Subject: [PATCH 4/8] Add GetRotationExpiryInfo for getting rotation and expiry data; --- domain/secret/state/state.go | 66 +++++++++++++++++++++--- domain/secret/state/state_test.go | 85 +++++++++++++++++++++++++++++++ domain/secret/types.go | 12 +++++ 3 files changed, 155 insertions(+), 8 deletions(-) diff --git a/domain/secret/state/state.go b/domain/secret/state/state.go index 617b943d3da..8dc0f1b45e9 100644 --- a/domain/secret/state/state.go +++ b/domain/secret/state/state.go @@ -20,6 +20,7 @@ import ( "github.com/juju/juju/domain" applicationerrors "github.com/juju/juju/domain/application/errors" modelerrors "github.com/juju/juju/domain/model/errors" + "github.com/juju/juju/domain/secret" domainsecret "github.com/juju/juju/domain/secret" secreterrors "github.com/juju/juju/domain/secret/errors" uniterrors "github.com/juju/juju/domain/unit/errors" @@ -1051,32 +1052,81 @@ func (st State) GetSecret(ctx context.Context, uri *coresecrets.URI) (*coresecre return secrets[0], nil } +// GetRotationExpiryInfo returns the rotation expiry information for the specified secret. +func (st State) GetRotationExpiryInfo(ctx context.Context, uri *coresecrets.URI) (*secret.RotationExpiryInfo, error) { + db, err := st.DB() + if err != nil { + return nil, errors.Trace(err) + } + + stmt, err := st.Prepare(` +WITH rev AS + (SELECT uuid,MAX(revision) AS latest_revision + FROM secret_revision + WHERE secret_id = $secretID.id) +SELECT sp.policy AS &secretInfo.policy, + sro.next_rotation_time AS &secretInfo.next_rotation_time, + sre.expire_time AS &secretInfo.latest_expire_time, + rev.latest_revision AS &secretInfo.latest_revision +FROM secret_metadata sm, rev +INNER JOIN secret_rotate_policy sp ON sp.id = sm.rotate_policy_id +LEFT JOIN secret_rotation sro ON sro.secret_id = sm.secret_id +LEFT JOIN secret_revision_expire sre ON sre.revision_uuid = rev.uuid +WHERE sm.secret_id = $secretID.id`, secretID{}, secretInfo{}) + + if err != nil { + return nil, errors.Trace(err) + } + + var result secretInfo + if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + err := tx.Query(ctx, stmt, secretID{ID: uri.ID}).Get(&result) + if errors.Is(err, sqlair.ErrNoRows) { + return fmt.Errorf("secret %q not found%w", uri, errors.Hide(secreterrors.SecretNotFound)) + } + return errors.Trace(err) + }); err != nil { + return nil, errors.Trace(domain.CoerceError(err)) + } + info := &secret.RotationExpiryInfo{ + RotatePolicy: coresecrets.RotatePolicy(result.RotatePolicy), + LatestRevision: result.LatestRevision, + } + if !result.NextRotateTime.IsZero() { + info.NextRotateTime = &result.NextRotateTime + } + if !result.LatestExpireTime.IsZero() { + info.LatestExpireTime = &result.LatestExpireTime + } + return info, nil +} + // GetRotatePolicy returns the rotate policy for the specified secret. func (st State) GetRotatePolicy(ctx context.Context, uri *coresecrets.URI) (coresecrets.RotatePolicy, error) { db, err := st.DB() if err != nil { - return "", errors.Trace(err) + return coresecrets.RotateNever, errors.Trace(err) } stmt, err := st.Prepare(` -SELECT srp.policy +SELECT srp.policy AS &secretInfo.policy FROM secret_metadata sm INNER JOIN secret_rotate_policy srp ON srp.id = sm.rotate_policy_id -WHERE sm.secret_id = $secretID.id`, secretID{}) +WHERE sm.secret_id = $secretID.id`, secretID{}, secretInfo{}) if err != nil { - return "", errors.Trace(err) + return coresecrets.RotateNever, errors.Trace(err) } - var rotatePolicy string + var info secretInfo if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { - err := tx.Query(ctx, stmt, secretID{ID: uri.ID}).Get(&rotatePolicy) + err := tx.Query(ctx, stmt, secretID{ID: uri.ID}).Get(&info) if errors.Is(err, sqlair.ErrNoRows) { return fmt.Errorf("rotate policy for %q not found%w", uri, errors.Hide(secreterrors.SecretNotFound)) } return errors.Trace(err) }); err != nil { - return "", errors.Trace(domain.CoerceError(err)) + return coresecrets.RotateNever, errors.Trace(domain.CoerceError(err)) } - return coresecrets.RotatePolicy(rotatePolicy), nil + return coresecrets.RotatePolicy(info.RotatePolicy), nil } func (st State) listSecretsAnyOwner( diff --git a/domain/secret/state/state_test.go b/domain/secret/state/state_test.go index 96f99cf574a..59f5388a494 100644 --- a/domain/secret/state/state_test.go +++ b/domain/secret/state/state_test.go @@ -76,6 +76,91 @@ func (s *stateSuite) TestGetSecretNotFound(c *gc.C) { c.Assert(err, jc.ErrorIs, secreterrors.SecretNotFound) } +func (s *stateSuite) TestGetRotatePolicy(c *gc.C) { + s.setupUnits(c, "mysql") + + st := newSecretState(c, s.TxnRunnerFactory()) + + expireTime := time.Now().Add(2 * time.Hour) + rotateTime := time.Now().Add(time.Hour) + sp := domainsecret.UpsertSecretParams{ + Description: ptr("my secretMetadata"), + Label: ptr("my label"), + Data: coresecrets.SecretData{"foo": "bar"}, + RotatePolicy: ptr(domainsecret.RotateYearly), + ExpireTime: ptr(expireTime), + NextRotateTime: ptr(rotateTime), + } + uri := coresecrets.NewURI() + ctx := context.Background() + err := st.CreateCharmApplicationSecret(ctx, 1, uri, "mysql", sp) + c.Assert(err, jc.ErrorIsNil) + + result, err := st.GetRotatePolicy(context.Background(), uri) + c.Assert(err, jc.ErrorIsNil) + c.Assert(result, gc.Equals, coresecrets.RotateYearly) +} + +func (s *stateSuite) TestGetRotatePolicyNotFound(c *gc.C) { + st := newSecretState(c, s.TxnRunnerFactory()) + + _, err := st.GetRotatePolicy(context.Background(), coresecrets.NewURI()) + c.Assert(err, jc.ErrorIs, secreterrors.SecretNotFound) +} + +func (s *stateSuite) TestGetRotationExpiryInfo(c *gc.C) { + s.setupUnits(c, "mysql") + + st := newSecretState(c, s.TxnRunnerFactory()) + + expireTime := time.Now().Add(2 * time.Hour) + rotateTime := time.Now().Add(time.Hour) + sp := domainsecret.UpsertSecretParams{ + Description: ptr("my secretMetadata"), + Label: ptr("my label"), + Data: coresecrets.SecretData{"foo": "bar"}, + RotatePolicy: ptr(domainsecret.RotateYearly), + ExpireTime: ptr(expireTime), + NextRotateTime: ptr(rotateTime), + } + uri := coresecrets.NewURI() + ctx := context.Background() + err := st.CreateCharmApplicationSecret(ctx, 1, uri, "mysql", sp) + c.Assert(err, jc.ErrorIsNil) + + result, err := st.GetRotationExpiryInfo(context.Background(), uri) + c.Assert(err, jc.ErrorIsNil) + c.Assert(result, jc.DeepEquals, &domainsecret.RotationExpiryInfo{ + RotatePolicy: coresecrets.RotateYearly, + LatestExpireTime: ptr(expireTime.UTC()), + NextRotateTime: ptr(rotateTime.UTC()), + LatestRevision: 1, + }) + + newExpireTime := expireTime.Add(2 * time.Hour) + err = st.UpdateSecret(ctx, uri, domainsecret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo": "bar1"}, + ExpireTime: ptr(newExpireTime), + }) + c.Assert(err, jc.ErrorIsNil) + + result, err = st.GetRotationExpiryInfo(context.Background(), uri) + c.Assert(err, jc.ErrorIsNil) + c.Assert(result, jc.DeepEquals, &domainsecret.RotationExpiryInfo{ + RotatePolicy: coresecrets.RotateYearly, + LatestExpireTime: ptr(newExpireTime.UTC()), + NextRotateTime: ptr(rotateTime.UTC()), + LatestRevision: 2, + }) +} + +func (s *stateSuite) TestGetRotationExpiryInfoNotFound(c *gc.C) { + st := newSecretState(c, s.TxnRunnerFactory()) + + _, err := st.GetRotationExpiryInfo(context.Background(), coresecrets.NewURI()) + c.Assert(err, jc.ErrorIs, secreterrors.SecretNotFound) +} + func (s *stateSuite) TestGetSecretRevisionNotFound(c *gc.C) { st := newSecretState(c, s.TxnRunnerFactory()) diff --git a/domain/secret/types.go b/domain/secret/types.go index c94aeeec8cf..9516ebcc382 100644 --- a/domain/secret/types.go +++ b/domain/secret/types.go @@ -73,3 +73,15 @@ type AccessScope struct { ScopeTypeID GrantScopeType ScopeID string } + +// RotationExpiryInfo holds information about the rotation and expiry of a secret. +type RotationExpiryInfo struct { + // RotatePolicy is the rotation policy of the secret. + RotatePolicy secrets.RotatePolicy + // NextRotateTime is when the secret should be rotated. + NextRotateTime *time.Time + // LatestExpireTime is the expire time of the most recent revision. + LatestExpireTime *time.Time + // LatestRevision is the most recent secret revision. + LatestRevision int +} From 0ad40e17fb364d407dfa42ce1fd7fd6e748d199e Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Tue, 14 May 2024 18:02:51 +1000 Subject: [PATCH 5/8] Refactor UpdateCharmSecret and SecretRotated to use GetRotationExpiryInfo; --- domain/secret/service/service.go | 22 +++++++++-------- domain/secret/service/service_test.go | 14 +++++------ domain/secret/service/state_mock_test.go | 30 ++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/domain/secret/service/service.go b/domain/secret/service/service.go index 169ed9aa1de..2d4b2ac2142 100644 --- a/domain/secret/service/service.go +++ b/domain/secret/service/service.go @@ -18,6 +18,7 @@ import ( "github.com/juju/juju/core/secrets" "github.com/juju/juju/core/watcher" "github.com/juju/juju/core/watcher/eventsource" + "github.com/juju/juju/domain/secret" domainsecret "github.com/juju/juju/domain/secret" secreterrors "github.com/juju/juju/domain/secret/errors" backenderrors "github.com/juju/juju/domain/secretbackend/errors" @@ -70,6 +71,7 @@ type State interface { ) ([]string, error) SecretRotated(ctx context.Context, uri *secrets.URI, next time.Time) error GetRotatePolicy(ctx context.Context, uri *secrets.URI) (secrets.RotatePolicy, error) + GetRotationExpiryInfo(ctx context.Context, uri *secrets.URI) (*secret.RotationExpiryInfo, error) // For watching consumed local secret changes. InitialWatchStatementForConsumedSecretsChange(unitName string) (string, eventsource.NamespaceQuery) @@ -364,10 +366,10 @@ func (s *SecretService) UpdateCharmSecret(ctx context.Context, uri *secrets.URI, p.RotatePolicy = &rotatePolicy if params.RotatePolicy.WillRotate() { policy, err := s.st.GetRotatePolicy(ctx, uri) - if err != nil { + if err != nil && !errors.Is(err, secreterrors.SecretNotFound) { return errors.Trace(err) } - if !policy.WillRotate() { + if errors.Is(err, secreterrors.SecretNotFound) || !policy.WillRotate() { p.NextRotateTime = params.RotatePolicy.NextRotateTime(s.clock.Now()) } } @@ -657,29 +659,29 @@ func (s *SecretService) SecretRotated(ctx context.Context, uri *secrets.URI, par return errors.Trace(err) } - md, err := s.GetSecret(ctx, uri) + info, err := s.st.GetRotationExpiryInfo(ctx, uri) if err != nil { return errors.Trace(err) } - if !md.RotatePolicy.WillRotate() { + if !info.RotatePolicy.WillRotate() { s.logger.Debugf("secret %q was rotated but now is set to not rotate") return nil } - lastRotateTime := md.NextRotateTime + lastRotateTime := info.NextRotateTime if lastRotateTime == nil { now := s.clock.Now() lastRotateTime = &now } - nextRotateTime := *md.RotatePolicy.NextRotateTime(*lastRotateTime) - s.logger.Debugf("secret %q was rotated: rev was %d, now %d", uri.ID, params.OriginalRevision, md.LatestRevision) + nextRotateTime := *info.RotatePolicy.NextRotateTime(*lastRotateTime) + s.logger.Debugf("secret %q was rotated: rev was %d, now %d", uri.ID, params.OriginalRevision, info.LatestRevision) // If the secret will expire before it is due to be next rotated, rotate sooner to allow // the charm a chance to update it before it expires. - willExpire := md.LatestExpireTime != nil && md.LatestExpireTime.Before(nextRotateTime) + willExpire := info.LatestExpireTime != nil && info.LatestExpireTime.Before(nextRotateTime) forcedRotateTime := lastRotateTime.Add(secrets.RotateRetryDelay) if willExpire { - s.logger.Warningf("secret %q rev %d will expire before next scheduled rotation", uri.ID, md.LatestRevision) + s.logger.Warningf("secret %q rev %d will expire before next scheduled rotation", uri.ID, info.LatestRevision) } - if willExpire && forcedRotateTime.Before(*md.LatestExpireTime) || !params.Skip && md.LatestRevision == params.OriginalRevision { + if willExpire && forcedRotateTime.Before(*info.LatestExpireTime) || !params.Skip && info.LatestRevision == params.OriginalRevision { nextRotateTime = forcedRotateTime } s.logger.Debugf("secret %q next rotate time is now: %s", uri.ID, nextRotateTime.UTC().Format(time.RFC3339)) diff --git a/domain/secret/service/service_test.go b/domain/secret/service/service_test.go index fcee2bc12d7..f8ee4c6e3ef 100644 --- a/domain/secret/service/service_test.go +++ b/domain/secret/service/service_test.go @@ -407,9 +407,9 @@ func (s *serviceSuite) TestUpdateCharmSecret(c *gc.C) { SubjectTypeID: domainsecret.SubjectUnit, SubjectID: "mariadb/0", }).Return("manage", nil) - s.state.EXPECT().GetSecret(gomock.Any(), uri).Return(&coresecrets.SecretMetadata{ - // No rotate policy. - }, nil) + s.state.EXPECT().GetRotatePolicy(gomock.Any(), uri).Return( + coresecrets.RotateNever, // No rotate policy. + secreterrors.SecretNotFound) s.state.EXPECT().UpdateSecret(gomock.Any(), uri, gomock.Any()).DoAndReturn(func(_ context.Context, _ *coresecrets.URI, got domainsecret.UpsertSecretParams) error { c.Assert(got.NextRotateTime, gc.NotNil) c.Assert(*got.NextRotateTime, jc.Almost, *p.NextRotateTime) @@ -1097,7 +1097,7 @@ func (s *serviceSuite) TestSecretsRotated(c *gc.C) { SubjectID: "mariadb/0", }).Return("manage", nil) s.state.EXPECT().SecretRotated(ctx, uri, nextRotateTime).Return(errors.New("boom")) - s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + s.state.EXPECT().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{ RotatePolicy: coresecrets.RotateHourly, LatestRevision: 667, }, nil) @@ -1126,7 +1126,7 @@ func (s *serviceSuite) TestSecretsRotatedRetry(c *gc.C) { SubjectID: "mariadb/0", }).Return("manage", nil) s.state.EXPECT().SecretRotated(ctx, uri, nextRotateTime).Return(errors.New("boom")) - s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + s.state.EXPECT().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{ RotatePolicy: coresecrets.RotateHourly, LatestRevision: 666, }, nil) @@ -1155,7 +1155,7 @@ func (s *serviceSuite) TestSecretsRotatedForce(c *gc.C) { SubjectID: "mariadb/0", }).Return("manage", nil) s.state.EXPECT().SecretRotated(ctx, uri, nextRotateTime).Return(errors.New("boom")) - s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + s.state.EXPECT().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{ RotatePolicy: coresecrets.RotateHourly, LatestExpireTime: ptr(time.Now().Add(50 * time.Minute)), LatestRevision: 667, @@ -1183,7 +1183,7 @@ func (s *serviceSuite) TestSecretsRotatedThenNever(c *gc.C) { SubjectTypeID: domainsecret.SubjectUnit, SubjectID: "mariadb/0", }).Return("manage", nil) - s.state.EXPECT().GetSecret(ctx, uri).Return(&coresecrets.SecretMetadata{ + s.state.EXPECT().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{ RotatePolicy: coresecrets.RotateNever, LatestRevision: 667, }, nil) diff --git a/domain/secret/service/state_mock_test.go b/domain/secret/service/state_mock_test.go index e27b5544047..5e549557c66 100644 --- a/domain/secret/service/state_mock_test.go +++ b/domain/secret/service/state_mock_test.go @@ -410,6 +410,36 @@ func (c *MockStateGetRevisionIDsForObsoleteCall) DoAndReturn(f func(context.Cont return c } +// GetRotatePolicy mocks base method. +func (m *MockState) GetRotatePolicy(arg0 context.Context, arg1 *secrets.URI) (secrets.RotatePolicy, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRotatePolicy", arg0, arg1) + ret0, _ := ret[0].(secrets.RotatePolicy) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRotatePolicy indicates an expected call of GetRotatePolicy. +func (mr *MockStateMockRecorder) GetRotatePolicy(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRotatePolicy", reflect.TypeOf((*MockState)(nil).GetRotatePolicy), arg0, arg1) +} + +// GetRotationExpiryInfo mocks base method. +func (m *MockState) GetRotationExpiryInfo(arg0 context.Context, arg1 *secrets.URI) (*secret.RotationExpiryInfo, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRotationExpiryInfo", arg0, arg1) + ret0, _ := ret[0].(*secret.RotationExpiryInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRotationExpiryInfo indicates an expected call of GetRotationExpiryInfo. +func (mr *MockStateMockRecorder) GetRotationExpiryInfo(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRotationExpiryInfo", reflect.TypeOf((*MockState)(nil).GetRotationExpiryInfo), arg0, arg1) +} + // GetSecret mocks base method. func (m *MockState) GetSecret(arg0 context.Context, arg1 *secrets.URI) (*secrets.SecretMetadata, error) { m.ctrl.T.Helper() From 4cb6c928067f03e2a8dfe9eaac88e83c39aa3b2b Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Tue, 14 May 2024 18:21:53 +1000 Subject: [PATCH 6/8] Add UNIQUE INDEX for secret_backend_type.type and align fields for secrets DDL; --- domain/schema/secret.go | 264 +++++++++++++------------- domain/secret/service/service_test.go | 8 +- 2 files changed, 136 insertions(+), 136 deletions(-) diff --git a/domain/schema/secret.go b/domain/schema/secret.go index bed5158a158..ca6f15c43d2 100644 --- a/domain/schema/secret.go +++ b/domain/schema/secret.go @@ -13,66 +13,66 @@ func secretBackendSchema() schema.Patch { -- Controller database tables for secret backends. CREATE TABLE secret_backend_type ( - id INT PRIMARY KEY, - type TEXT NOT NULL, + id INT PRIMARY KEY, + type TEXT NOT NULL, description TEXT, - CONSTRAINT chk_empty_type - CHECK(type != ''), - CONSTRAINT uniq_secret_backend_type_type - UNIQUE(type) + CONSTRAINT chk_empty_type + CHECK(type != '') ); +CREATE UNIQUE INDEX idx_secret_backend_type_type ON secret_backend_type (type); + INSERT INTO secret_backend_type VALUES (0, 'controller', 'the juju controller secret backend'), (1, 'kubernetes', 'the kubernetes secret backend'), (2, 'vault', 'the vault secret backend'); CREATE TABLE secret_backend ( - uuid TEXT PRIMARY KEY, - name TEXT NOT NULL, - backend_type_id INT NOT NULL, + uuid TEXT PRIMARY KEY, + name TEXT NOT NULL, + backend_type_id INT NOT NULL, token_rotate_interval INT, - CONSTRAINT chk_empty_name + CONSTRAINT chk_empty_name CHECK(name != ''), - CONSTRAINT fk_secret_backend_type_id - FOREIGN KEY (backend_type_id) - REFERENCES secret_backend_type (id) + CONSTRAINT fk_secret_backend_type_id + FOREIGN KEY (backend_type_id) + REFERENCES secret_backend_type (id) ); CREATE UNIQUE INDEX idx_secret_backend_name ON secret_backend (name); CREATE TABLE secret_backend_config ( backend_uuid TEXT NOT NULL, - name TEXT NOT NULL, - content TEXT NOT NULL, - CONSTRAINT chk_empty_name + name TEXT NOT NULL, + content TEXT NOT NULL, + CONSTRAINT chk_empty_name CHECK(name != ''), - CONSTRAINT chk_empty_content + CONSTRAINT chk_empty_content CHECK(content != ''), - CONSTRAINT pk_secret_backend_config + CONSTRAINT pk_secret_backend_config PRIMARY KEY (backend_uuid, name), - CONSTRAINT fk_secret_backend_config_backend_uuid + CONSTRAINT fk_secret_backend_config_backend_uuid FOREIGN KEY (backend_uuid) - REFERENCES secret_backend (uuid) + REFERENCES secret_backend (uuid) ); CREATE TABLE secret_backend_rotation ( - backend_uuid TEXT PRIMARY KEY, + backend_uuid TEXT PRIMARY KEY, next_rotation_time DATETIME NOT NULL, - CONSTRAINT fk_secret_backend_rotation_secret_backend_uuid - FOREIGN KEY (backend_uuid) - REFERENCES secret_backend (uuid) + CONSTRAINT fk_secret_backend_rotation_secret_backend_uuid + FOREIGN KEY (backend_uuid) + REFERENCES secret_backend (uuid) ); CREATE TABLE model_secret_backend ( - model_uuid TEXT PRIMARY KEY, + model_uuid TEXT PRIMARY KEY, secret_backend_uuid TEXT NOT NULL, - CONSTRAINT fk_model_secret_backend_model_uuid - FOREIGN KEY (model_uuid) - REFERENCES model (uuid), - CONSTRAINT fk_model_secret_backend_secret_backend_uuid - FOREIGN KEY (secret_backend_uuid) - REFERENCES secret_backend (uuid) + CONSTRAINT fk_model_secret_backend_model_uuid + FOREIGN KEY (model_uuid) + REFERENCES model (uuid), + CONSTRAINT fk_model_secret_backend_secret_backend_uuid + FOREIGN KEY (secret_backend_uuid) + REFERENCES secret_backend (uuid) ); CREATE VIEW v_model_secret_backend AS @@ -94,8 +94,8 @@ func secretSchema() schema.Patch { -- Model database tables for secrets. CREATE TABLE secret_rotate_policy ( - id INT PRIMARY KEY, - policy TEXT NOT NULL, + id INT PRIMARY KEY, + policy TEXT NOT NULL, CONSTRAINT chk_empty_policy CHECK(policy != '') ); @@ -120,108 +120,108 @@ CREATE TABLE secret ( -- is used on the consumer side of cross -- model secrets. CREATE TABLE secret_reference ( - secret_id TEXT PRIMARY KEY, + secret_id TEXT PRIMARY KEY, latest_revision INT NOT NULL, - CONSTRAINT fk_secret_id + CONSTRAINT fk_secret_id FOREIGN KEY (secret_id) - REFERENCES secret (id) + REFERENCES secret (id) ); CREATE TABLE secret_metadata ( - secret_id TEXT PRIMARY KEY, - version INT NOT NULL, - description TEXT, + secret_id TEXT PRIMARY KEY, + version INT NOT NULL, + description TEXT, rotate_policy_id INT NOT NULL, - auto_prune BOOLEAN NOT NULL DEFAULT (FALSE), - create_time DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')), - update_time DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')), - CONSTRAINT fk_secret_id - FOREIGN KEY (secret_id) - REFERENCES secret (id), - CONSTRAINT fk_secret_rotate_policy - FOREIGN KEY (rotate_policy_id) - REFERENCES secret_rotate_policy (id) + auto_prune BOOLEAN NOT NULL DEFAULT (FALSE), + create_time DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')), + update_time DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')), + CONSTRAINT fk_secret_id + FOREIGN KEY (secret_id) + REFERENCES secret (id), + CONSTRAINT fk_secret_rotate_policy + FOREIGN KEY (rotate_policy_id) + REFERENCES secret_rotate_policy (id) ); CREATE TABLE secret_rotation ( - secret_id TEXT PRIMARY KEY, + secret_id TEXT PRIMARY KEY, next_rotation_time DATETIME NOT NULL, - CONSTRAINT fk_secret_rotation_secret_metadata_id - FOREIGN KEY (secret_id) - REFERENCES secret_metadata (secret_id) + CONSTRAINT fk_secret_rotation_secret_metadata_id + FOREIGN KEY (secret_id) + REFERENCES secret_metadata (secret_id) ); -- 1:1 CREATE TABLE secret_value_ref ( - revision_uuid TEXT PRIMARY KEY, + revision_uuid TEXT PRIMARY KEY, -- backend_uuid is the UUID of the backend in the controller database. - backend_uuid TEXT NOT NULL, - revision_id TEXT NOT NULL, - CONSTRAINT fk_secret_value_ref_secret_revision_uuid + backend_uuid TEXT NOT NULL, + revision_id TEXT NOT NULL, + CONSTRAINT fk_secret_value_ref_secret_revision_uuid FOREIGN KEY (revision_uuid) - REFERENCES secret_revision (uuid) + REFERENCES secret_revision (uuid) ); -- 1:many CREATE TABLE secret_content ( - revision_uuid TEXT NOT NULL, - name TEXT NOT NULL, - content TEXT NOT NULL, - CONSTRAINT chk_empty_name + revision_uuid TEXT NOT NULL, + name TEXT NOT NULL, + content TEXT NOT NULL, + CONSTRAINT chk_empty_name CHECK(name != ''), - CONSTRAINT chk_empty_content + CONSTRAINT chk_empty_content CHECK(content != ''), - CONSTRAINT pk_secret_content_revision_uuid_name + CONSTRAINT pk_secret_content_revision_uuid_name PRIMARY KEY (revision_uuid,name), - CONSTRAINT fk_secret_content_secret_revision_uuid + CONSTRAINT fk_secret_content_secret_revision_uuid FOREIGN KEY (revision_uuid) - REFERENCES secret_revision (uuid) + REFERENCES secret_revision (uuid) ); CREATE INDEX idx_secret_content_revision_uuid ON secret_content (revision_uuid); CREATE TABLE secret_revision ( - uuid TEXT PRIMARY KEY, - secret_id TEXT NOT NULL, - revision INT NOT NULL, - create_time DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')), - CONSTRAINT fk_secret_revision_secret_metadata_id + uuid TEXT PRIMARY KEY, + secret_id TEXT NOT NULL, + revision INT NOT NULL, + create_time DATETIME NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW', 'utc')), + CONSTRAINT fk_secret_revision_secret_metadata_id FOREIGN KEY (secret_id) - REFERENCES secret_metadata (secret_id) + REFERENCES secret_metadata (secret_id) ); CREATE UNIQUE INDEX idx_secret_revision_secret_id_revision ON secret_revision (secret_id,revision); CREATE TABLE secret_revision_obsolete ( - revision_uuid TEXT PRIMARY KEY, - obsolete BOOLEAN NOT NULL DEFAULT (FALSE), + revision_uuid TEXT PRIMARY KEY, + obsolete BOOLEAN NOT NULL DEFAULT (FALSE), -- pending_delete is true if the revision is to be deleted. -- It will not be drained to a new active backend. pending_delete BOOLEAN NOT NULL DEFAULT (FALSE), - CONSTRAINT fk_secret_revision_obsolete_revision_uuid + CONSTRAINT fk_secret_revision_obsolete_revision_uuid FOREIGN KEY (revision_uuid) - REFERENCES secret_revision (uuid) + REFERENCES secret_revision (uuid) ); CREATE TABLE secret_revision_expire ( - revision_uuid TEXT PRIMARY KEY, - expire_time DATETIME NOT NULL, - CONSTRAINT fk_secret_revision_expire_revision_uuid + revision_uuid TEXT PRIMARY KEY, + expire_time DATETIME NOT NULL, + CONSTRAINT fk_secret_revision_expire_revision_uuid FOREIGN KEY (revision_uuid) - REFERENCES secret_revision (uuid) + REFERENCES secret_revision (uuid) ); CREATE TABLE secret_application_owner ( - secret_id TEXT NOT NULL, + secret_id TEXT NOT NULL, application_uuid TEXT NOT NULL, - label TEXT, - CONSTRAINT fk_secret_application_owner_secret_metadata_id - FOREIGN KEY (secret_id) - REFERENCES secret_metadata (secret_id), - CONSTRAINT fk_secret_application_owner_application_uuid - FOREIGN KEY (application_uuid) - REFERENCES application (uuid) - PRIMARY KEY (secret_id, application_uuid) + label TEXT, + CONSTRAINT fk_secret_application_owner_secret_metadata_id + FOREIGN KEY (secret_id) + REFERENCES secret_metadata (secret_id), + CONSTRAINT fk_secret_application_owner_application_uuid + FOREIGN KEY (application_uuid) + REFERENCES application (uuid) + PRIMARY KEY (secret_id, application_uuid) ); CREATE INDEX idx_secret_application_owner_secret_id ON secret_application_owner (secret_id); @@ -229,16 +229,16 @@ CREATE INDEX idx_secret_application_owner_secret_id ON secret_application_owner CREATE UNIQUE INDEX idx_secret_application_owner_label ON secret_application_owner (label,application_uuid) WHERE label != ''; CREATE TABLE secret_unit_owner ( - secret_id TEXT NOT NULL, - unit_uuid TEXT NOT NULL, - label TEXT, - CONSTRAINT fk_secret_unit_owner_secret_metadata_id + secret_id TEXT NOT NULL, + unit_uuid TEXT NOT NULL, + label TEXT, + CONSTRAINT fk_secret_unit_owner_secret_metadata_id FOREIGN KEY (secret_id) - REFERENCES secret_metadata (secret_id), - CONSTRAINT fk_secret_unit_owner_unit_uuid + REFERENCES secret_metadata (secret_id), + CONSTRAINT fk_secret_unit_owner_unit_uuid FOREIGN KEY (unit_uuid) - REFERENCES unit (uuid) - PRIMARY KEY (secret_id, unit_uuid) + REFERENCES unit (uuid) + PRIMARY KEY (secret_id, unit_uuid) ); CREATE INDEX idx_secret_unit_owner_secret_id ON secret_unit_owner (secret_id); @@ -246,29 +246,29 @@ CREATE INDEX idx_secret_unit_owner_secret_id ON secret_unit_owner (secret_id); CREATE UNIQUE INDEX idx_secret_unit_owner_label ON secret_unit_owner (label,unit_uuid) WHERE label != ''; CREATE TABLE secret_model_owner ( - secret_id TEXT PRIMARY KEY, - label TEXT, - CONSTRAINT fk_secret_model_owner_secret_metadata_id + secret_id TEXT PRIMARY KEY, + label TEXT, + CONSTRAINT fk_secret_model_owner_secret_metadata_id FOREIGN KEY (secret_id) - REFERENCES secret_metadata (secret_id) + REFERENCES secret_metadata (secret_id) ); CREATE UNIQUE INDEX idx_secret_model_owner_label ON secret_model_owner (label) WHERE label != ''; CREATE TABLE secret_unit_consumer ( - secret_id TEXT NOT NULL, + secret_id TEXT NOT NULL, -- source model uuid may be this model or a different model -- possibly on another controller source_model_uuid TEXT NOT NULL, - unit_uuid TEXT NOT NULL, - label TEXT, - current_revision INT NOT NULL, - CONSTRAINT fk_secret_unit_consumer_unit_uuid - FOREIGN KEY (unit_uuid) - REFERENCES unit (uuid), - CONSTRAINT fk_secret_unit_consumer_secret_id - FOREIGN KEY (secret_id) - REFERENCES secret (id) + unit_uuid TEXT NOT NULL, + label TEXT, + current_revision INT NOT NULL, + CONSTRAINT fk_secret_unit_consumer_unit_uuid + FOREIGN KEY (unit_uuid) + REFERENCES unit (uuid), + CONSTRAINT fk_secret_unit_consumer_secret_id + FOREIGN KEY (secret_id) + REFERENCES secret (id) ); CREATE UNIQUE INDEX idx_secret_unit_consumer_secret_id_unit_uuid ON secret_unit_consumer (secret_id,unit_uuid); @@ -277,20 +277,20 @@ CREATE UNIQUE INDEX idx_secret_unit_consumer_label ON secret_unit_consumer (labe -- This table records the tracked revisions from -- units in the consuming model for cross model secrets. CREATE TABLE secret_remote_unit_consumer ( - secret_id TEXT NOT NULL, + secret_id TEXT NOT NULL, -- unit_id is the anonymised name of the unit -- from the consuming model. - unit_id TEXT NOT NULL, + unit_id TEXT NOT NULL, current_revision INT NOT NULL, - CONSTRAINT fk_secret_remote_unit_consumer_secret_metadata_id - FOREIGN KEY (secret_id) - REFERENCES secret_metadata (secret_id) + CONSTRAINT fk_secret_remote_unit_consumer_secret_metadata_id + FOREIGN KEY (secret_id) + REFERENCES secret_metadata (secret_id) ); CREATE UNIQUE INDEX idx_secret_remote_unit_consumer_secret_id_unit_id ON secret_remote_unit_consumer (secret_id,unit_id); CREATE TABLE secret_role ( - id INT PRIMARY KEY, + id INT PRIMARY KEY, role TEXT ); @@ -324,36 +324,36 @@ INSERT INTO secret_grant_scope_type VALUES (3, 'relation'); CREATE TABLE secret_permission ( - secret_id TEXT NOT NULL, - role_id INT NOT NULL, + secret_id TEXT NOT NULL, + role_id INT NOT NULL, -- subject_uuid is the entity which -- has been granted access to a secret. -- It will be an application, unit, or model uuid. - subject_uuid TEXT NOT NULL, + subject_uuid TEXT NOT NULL, subject_type_id INT NOT NULL, -- scope_uuid is the entity which -- defines the scope of the grant. -- It will be an application, unit, relation, or model uuid. - scope_uuid TEXT NOT NULL, - scope_type_id TEXT NOT NULL, - CONSTRAINT pk_secret_permission_secret_id_subject_uuid + scope_uuid TEXT NOT NULL, + scope_type_id TEXT NOT NULL, + CONSTRAINT pk_secret_permission_secret_id_subject_uuid PRIMARY KEY (secret_id,subject_uuid), - CONSTRAINT chk_empty_scope_uuid + CONSTRAINT chk_empty_scope_uuid CHECK(scope_uuid != ''), - CONSTRAINT chk_empty_subject_uuid + CONSTRAINT chk_empty_subject_uuid CHECK(subject_uuid != ''), - CONSTRAINT fk_secret_permission_secret_id + CONSTRAINT fk_secret_permission_secret_id FOREIGN KEY (secret_id) - REFERENCES secret_metadata (secret_id), - CONSTRAINT fk_secret_permission_secret_role_id + REFERENCES secret_metadata (secret_id), + CONSTRAINT fk_secret_permission_secret_role_id FOREIGN KEY (role_id) - REFERENCES secret_role (id), - CONSTRAINT fk_secret_permission_secret_grant_subject_type_id + REFERENCES secret_role (id), + CONSTRAINT fk_secret_permission_secret_grant_subject_type_id FOREIGN KEY (subject_type_id) - REFERENCES secret_grant_subject_type (id), - CONSTRAINT fk_secret_permission_secret_grant_scope_type_id + REFERENCES secret_grant_subject_type (id), + CONSTRAINT fk_secret_permission_secret_grant_scope_type_id FOREIGN KEY (scope_type_id) - REFERENCES secret_grant_scope_type (id) + REFERENCES secret_grant_scope_type (id) ); CREATE INDEX idx_secret_permission_secret_id ON secret_permission (secret_id); diff --git a/domain/secret/service/service_test.go b/domain/secret/service/service_test.go index f8ee4c6e3ef..000088ff474 100644 --- a/domain/secret/service/service_test.go +++ b/domain/secret/service/service_test.go @@ -1102,7 +1102,7 @@ func (s *serviceSuite) TestSecretsRotated(c *gc.C) { LatestRevision: 667, }, nil) - err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + err := s.service(c).SecretRotated(ctx, uri, SecretRotatedParams{ LeaderToken: successfulToken{}, Accessor: SecretAccessor{ Kind: UnitAccessor, @@ -1131,7 +1131,7 @@ func (s *serviceSuite) TestSecretsRotatedRetry(c *gc.C) { LatestRevision: 666, }, nil) - err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + err := s.service(c).SecretRotated(ctx, uri, SecretRotatedParams{ LeaderToken: successfulToken{}, Accessor: SecretAccessor{ Kind: UnitAccessor, @@ -1161,7 +1161,7 @@ func (s *serviceSuite) TestSecretsRotatedForce(c *gc.C) { LatestRevision: 667, }, nil) - err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + err := s.service(c).SecretRotated(ctx, uri, SecretRotatedParams{ LeaderToken: successfulToken{}, Accessor: SecretAccessor{ Kind: UnitAccessor, @@ -1188,7 +1188,7 @@ func (s *serviceSuite) TestSecretsRotatedThenNever(c *gc.C) { LatestRevision: 667, }, nil) - err := s.service().SecretRotated(ctx, uri, SecretRotatedParams{ + err := s.service(c).SecretRotated(ctx, uri, SecretRotatedParams{ LeaderToken: successfulToken{}, Accessor: SecretAccessor{ Kind: UnitAccessor, From a5a28ea1b5e00df335a35a7af42d98bc00f446f5 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Tue, 14 May 2024 18:44:09 +1000 Subject: [PATCH 7/8] Re-generate mocks; --- domain/secret/service/state_mock_test.go | 84 ++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 6 deletions(-) diff --git a/domain/secret/service/state_mock_test.go b/domain/secret/service/state_mock_test.go index 5e549557c66..943d1593ae9 100644 --- a/domain/secret/service/state_mock_test.go +++ b/domain/secret/service/state_mock_test.go @@ -420,9 +420,33 @@ func (m *MockState) GetRotatePolicy(arg0 context.Context, arg1 *secrets.URI) (se } // GetRotatePolicy indicates an expected call of GetRotatePolicy. -func (mr *MockStateMockRecorder) GetRotatePolicy(arg0, arg1 any) *gomock.Call { +func (mr *MockStateMockRecorder) GetRotatePolicy(arg0, arg1 any) *MockStateGetRotatePolicyCall { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRotatePolicy", reflect.TypeOf((*MockState)(nil).GetRotatePolicy), arg0, arg1) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRotatePolicy", reflect.TypeOf((*MockState)(nil).GetRotatePolicy), arg0, arg1) + return &MockStateGetRotatePolicyCall{Call: call} +} + +// MockStateGetRotatePolicyCall wrap *gomock.Call +type MockStateGetRotatePolicyCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateGetRotatePolicyCall) Return(arg0 secrets.RotatePolicy, arg1 error) *MockStateGetRotatePolicyCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateGetRotatePolicyCall) Do(f func(context.Context, *secrets.URI) (secrets.RotatePolicy, error)) *MockStateGetRotatePolicyCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateGetRotatePolicyCall) DoAndReturn(f func(context.Context, *secrets.URI) (secrets.RotatePolicy, error)) *MockStateGetRotatePolicyCall { + c.Call = c.Call.DoAndReturn(f) + return c } // GetRotationExpiryInfo mocks base method. @@ -435,9 +459,33 @@ func (m *MockState) GetRotationExpiryInfo(arg0 context.Context, arg1 *secrets.UR } // GetRotationExpiryInfo indicates an expected call of GetRotationExpiryInfo. -func (mr *MockStateMockRecorder) GetRotationExpiryInfo(arg0, arg1 any) *gomock.Call { +func (mr *MockStateMockRecorder) GetRotationExpiryInfo(arg0, arg1 any) *MockStateGetRotationExpiryInfoCall { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRotationExpiryInfo", reflect.TypeOf((*MockState)(nil).GetRotationExpiryInfo), arg0, arg1) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRotationExpiryInfo", reflect.TypeOf((*MockState)(nil).GetRotationExpiryInfo), arg0, arg1) + return &MockStateGetRotationExpiryInfoCall{Call: call} +} + +// MockStateGetRotationExpiryInfoCall wrap *gomock.Call +type MockStateGetRotationExpiryInfoCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateGetRotationExpiryInfoCall) Return(arg0 *secret.RotationExpiryInfo, arg1 error) *MockStateGetRotationExpiryInfoCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateGetRotationExpiryInfoCall) Do(f func(context.Context, *secrets.URI) (*secret.RotationExpiryInfo, error)) *MockStateGetRotationExpiryInfoCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateGetRotationExpiryInfoCall) DoAndReturn(f func(context.Context, *secrets.URI) (*secret.RotationExpiryInfo, error)) *MockStateGetRotationExpiryInfoCall { + c.Call = c.Call.DoAndReturn(f) + return c } // GetSecret mocks base method. @@ -1352,9 +1400,33 @@ func (m *MockState) SecretRotated(arg0 context.Context, arg1 *secrets.URI, arg2 } // SecretRotated indicates an expected call of SecretRotated. -func (mr *MockStateMockRecorder) SecretRotated(arg0, arg1, arg2 any) *gomock.Call { +func (mr *MockStateMockRecorder) SecretRotated(arg0, arg1, arg2 any) *MockStateSecretRotatedCall { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SecretRotated", reflect.TypeOf((*MockState)(nil).SecretRotated), arg0, arg1, arg2) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SecretRotated", reflect.TypeOf((*MockState)(nil).SecretRotated), arg0, arg1, arg2) + return &MockStateSecretRotatedCall{Call: call} +} + +// MockStateSecretRotatedCall wrap *gomock.Call +type MockStateSecretRotatedCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateSecretRotatedCall) Return(arg0 error) *MockStateSecretRotatedCall { + c.Call = c.Call.Return(arg0) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateSecretRotatedCall) Do(f func(context.Context, *secrets.URI, time.Time) error) *MockStateSecretRotatedCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateSecretRotatedCall) DoAndReturn(f func(context.Context, *secrets.URI, time.Time) error) *MockStateSecretRotatedCall { + c.Call = c.Call.DoAndReturn(f) + return c } // UpdateRemoteSecretRevision mocks base method. From d6cf27d84595fcc769e0060e2eec6dd3d69ee632 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Wed, 15 May 2024 18:11:04 +1000 Subject: [PATCH 8/8] Format SQL; --- domain/secret/service/service.go | 4 ++-- domain/secret/service/service_test.go | 2 +- domain/secret/state/state.go | 32 ++++++++++++++------------- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/domain/secret/service/service.go b/domain/secret/service/service.go index 2d4b2ac2142..4cd0c2d5b89 100644 --- a/domain/secret/service/service.go +++ b/domain/secret/service/service.go @@ -366,10 +366,10 @@ func (s *SecretService) UpdateCharmSecret(ctx context.Context, uri *secrets.URI, p.RotatePolicy = &rotatePolicy if params.RotatePolicy.WillRotate() { policy, err := s.st.GetRotatePolicy(ctx, uri) - if err != nil && !errors.Is(err, secreterrors.SecretNotFound) { + if err != nil { return errors.Trace(err) } - if errors.Is(err, secreterrors.SecretNotFound) || !policy.WillRotate() { + if !policy.WillRotate() { p.NextRotateTime = params.RotatePolicy.NextRotateTime(s.clock.Now()) } } diff --git a/domain/secret/service/service_test.go b/domain/secret/service/service_test.go index 000088ff474..464ff32f342 100644 --- a/domain/secret/service/service_test.go +++ b/domain/secret/service/service_test.go @@ -409,7 +409,7 @@ func (s *serviceSuite) TestUpdateCharmSecret(c *gc.C) { }).Return("manage", nil) s.state.EXPECT().GetRotatePolicy(gomock.Any(), uri).Return( coresecrets.RotateNever, // No rotate policy. - secreterrors.SecretNotFound) + nil) s.state.EXPECT().UpdateSecret(gomock.Any(), uri, gomock.Any()).DoAndReturn(func(_ context.Context, _ *coresecrets.URI, got domainsecret.UpsertSecretParams) error { c.Assert(got.NextRotateTime, gc.NotNil) c.Assert(*got.NextRotateTime, jc.Almost, *p.NextRotateTime) diff --git a/domain/secret/state/state.go b/domain/secret/state/state.go index 8dc0f1b45e9..3a3c845a52b 100644 --- a/domain/secret/state/state.go +++ b/domain/secret/state/state.go @@ -1058,29 +1058,31 @@ func (st State) GetRotationExpiryInfo(ctx context.Context, uri *coresecrets.URI) if err != nil { return nil, errors.Trace(err) } - + input := secretID{ID: uri.ID} + result := secretInfo{} stmt, err := st.Prepare(` -WITH rev AS - (SELECT uuid,MAX(revision) AS latest_revision +WITH rev AS ( + SELECT uuid, MAX(revision) AS latest_revision FROM secret_revision - WHERE secret_id = $secretID.id) -SELECT sp.policy AS &secretInfo.policy, - sro.next_rotation_time AS &secretInfo.next_rotation_time, - sre.expire_time AS &secretInfo.latest_expire_time, - rev.latest_revision AS &secretInfo.latest_revision + WHERE secret_id = $secretID.id +) +SELECT + sp.policy AS &secretInfo.policy, + sro.next_rotation_time AS &secretInfo.next_rotation_time, + sre.expire_time AS &secretInfo.latest_expire_time, + rev.latest_revision AS &secretInfo.latest_revision FROM secret_metadata sm, rev -INNER JOIN secret_rotate_policy sp ON sp.id = sm.rotate_policy_id -LEFT JOIN secret_rotation sro ON sro.secret_id = sm.secret_id -LEFT JOIN secret_revision_expire sre ON sre.revision_uuid = rev.uuid -WHERE sm.secret_id = $secretID.id`, secretID{}, secretInfo{}) + JOIN secret_rotate_policy sp ON sp.id = sm.rotate_policy_id + LEFT JOIN secret_rotation sro ON sro.secret_id = sm.secret_id + LEFT JOIN secret_revision_expire sre ON sre.revision_uuid = rev.uuid +WHERE sm.secret_id = $secretID.id`, input, result) if err != nil { return nil, errors.Trace(err) } - var result secretInfo if err := db.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { - err := tx.Query(ctx, stmt, secretID{ID: uri.ID}).Get(&result) + err := tx.Query(ctx, stmt, input).Get(&result) if errors.Is(err, sqlair.ErrNoRows) { return fmt.Errorf("secret %q not found%w", uri, errors.Hide(secreterrors.SecretNotFound)) } @@ -1110,7 +1112,7 @@ func (st State) GetRotatePolicy(ctx context.Context, uri *coresecrets.URI) (core stmt, err := st.Prepare(` SELECT srp.policy AS &secretInfo.policy FROM secret_metadata sm -INNER JOIN secret_rotate_policy srp ON srp.id = sm.rotate_policy_id + JOIN secret_rotate_policy srp ON srp.id = sm.rotate_policy_id WHERE sm.secret_id = $secretID.id`, secretID{}, secretInfo{}) if err != nil { return coresecrets.RotateNever, errors.Trace(err)