diff --git a/domain/secret/state/state.go b/domain/secret/state/state.go index 12c09ad7e6c..88d5428336b 100644 --- a/domain/secret/state/state.go +++ b/domain/secret/state/state.go @@ -20,7 +20,6 @@ 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" @@ -426,17 +425,23 @@ VALUES ($secretID.id)` return errors.Annotatef(err, "creating user secret %q", uri) } - dbRevision := secretRevision{ + dbRevision := &secretRevision{ ID: revisionUUID.String(), SecretID: uri.ID, Revision: 1, CreateTime: now, } - if err := st.upsertSecretRevision(ctx, tx, dbRevision, secret.ExpireTime); err != nil { + if err := st.upsertSecretRevision(ctx, tx, dbRevision); err != nil { return errors.Annotatef(err, "inserting revision for secret %q", uri) } + if secret.ExpireTime != nil { + if err := st.upsertSecretRevisionExpiry(ctx, tx, dbRevision.ID, secret.ExpireTime); err != nil { + return errors.Annotatef(err, "inserting revision expiry for secret %q", uri) + } + } + if secret.NextRotateTime != nil { if err := st.upsertSecretNextRotateTime(ctx, tx, uri, *secret.NextRotateTime); err != nil { return errors.Annotatef(err, "inserting next rotate time for secret %q", uri) @@ -468,7 +473,7 @@ func (st State) updateSecret( // the update statement needed. existingSecretQuery := ` WITH rev AS ( - SELECT MAX(revision) AS latest_revision + SELECT MAX(revision) AS latest_revision, uuid AS latest_revision_uuid FROM secret_revision WHERE secret_id = $secretID.id ) @@ -477,7 +482,8 @@ SELECT (sm.secret_id, description, auto_prune, rp.policy, - rev.latest_revision) AS (&secretInfo.*), + rev.latest_revision, + rev.latest_revision_uuid) AS (&secretInfo.*), (so.owner_kind, so.owner_id, so.label) AS (&secretOwner.*) @@ -522,6 +528,7 @@ WHERE sm.secret_id = $secretID.id` return errors.Trace(err) } existing := existingResult[0] + latestRevisionUUID := dbSecrets[0].LatestRevisionUUID // Check to be sure a duplicate label won't be used. var checkExists checkExistsFunc @@ -590,25 +597,32 @@ WHERE sm.secret_id = $secretID.id` } } - if len(secret.Data) == 0 && secret.ValueRef == nil { - return nil - } + var dbRevision *secretRevision + if len(secret.Data) != 0 || secret.ValueRef != nil { + revisionUUID, err := uuid.NewUUID() + if err != nil { + return errors.Trace(err) + } + latestRevisionUUID = revisionUUID.String() - revisionUUID, err := uuid.NewUUID() - if err != nil { - return errors.Trace(err) + nextRevision := existing.LatestRevision + 1 + dbRevision = &secretRevision{ + ID: revisionUUID.String(), + SecretID: uri.ID, + Revision: nextRevision, + CreateTime: now, + } } - - nextRevision := existing.LatestRevision + 1 - dbRevision := secretRevision{ - ID: revisionUUID.String(), - SecretID: uri.ID, - Revision: nextRevision, - CreateTime: now, + if dbRevision != nil { + if err := st.upsertSecretRevision(ctx, tx, dbRevision); err != nil { + return errors.Annotatef(err, "inserting revision for secret %q", uri) + } } + if secret.ExpireTime != nil { + if err := st.upsertSecretRevisionExpiry(ctx, tx, latestRevisionUUID, secret.ExpireTime); err != nil { + return errors.Annotatef(err, "inserting revision expiry for secret %q", uri) + } - if err := st.upsertSecretRevision(ctx, tx, dbRevision, secret.ExpireTime); err != nil { - return errors.Annotatef(err, "inserting revision for secret %q", uri) } if len(secret.Data) > 0 { @@ -899,7 +913,7 @@ ON CONFLICT(secret_id) DO UPDATE SET } func (st State) upsertSecretRevision( - ctx context.Context, tx *sqlair.TX, dbRevision secretRevision, expireTime *time.Time, + ctx context.Context, tx *sqlair.TX, dbRevision *secretRevision, ) error { insertQuery := ` INSERT INTO secret_revision (*) @@ -911,17 +925,27 @@ VALUES ($secretRevision.*)` } err = tx.Query(ctx, insertStmt, dbRevision).Run() - if err != nil || expireTime == nil { + if err != nil { return errors.Trace(err) } + return nil +} + +func (st State) upsertSecretRevisionExpiry( + ctx context.Context, tx *sqlair.TX, revisionUUID string, expireTime *time.Time, +) error { + if expireTime == nil { + return nil + } + insertExpireTimeQuery := ` INSERT INTO secret_revision_expire (*) VALUES ($secretRevisionExpire.*) ON CONFLICT(revision_uuid) DO UPDATE SET expire_time=excluded.expire_time` - expire := secretRevisionExpire{RevisionUUID: dbRevision.ID, ExpireTime: expireTime.UTC()} + expire := secretRevisionExpire{RevisionUUID: revisionUUID, ExpireTime: expireTime.UTC()} insertExpireTimeStmt, err := st.Prepare(insertExpireTimeQuery, expire) if err != nil { return errors.Trace(err) @@ -1086,7 +1110,7 @@ func (st State) GetSecret(ctx context.Context, uri *coresecrets.URI) (*coresecre } // GetRotationExpiryInfo returns the rotation expiry information for the specified secret. -func (st State) GetRotationExpiryInfo(ctx context.Context, uri *coresecrets.URI) (*secret.RotationExpiryInfo, error) { +func (st State) GetRotationExpiryInfo(ctx context.Context, uri *coresecrets.URI) (*domainsecret.RotationExpiryInfo, error) { db, err := st.DB() if err != nil { return nil, errors.Trace(err) @@ -1119,7 +1143,7 @@ GROUP BY sr.secret_id`, input, result) }); err != nil { return nil, errors.Trace(domain.CoerceError(err)) } - info := &secret.RotationExpiryInfo{ + info := &domainsecret.RotationExpiryInfo{ RotatePolicy: coresecrets.RotatePolicy(result.RotatePolicy), LatestRevision: result.LatestRevision, } @@ -3324,7 +3348,7 @@ func (st State) getSecretsRotationChanges( ctx context.Context, runner coredatabase.TxnRunner, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, secretIDs ...string, -) ([]secret.RotationInfo, error) { +) ([]domainsecret.RotationInfo, error) { if len(secretIDs) == 0 && len(appOwners) == 0 && len(unitOwners) == 0 { return nil, nil } @@ -3401,9 +3425,9 @@ GROUP BY sro.secret_id` if err != nil { return nil, errors.Trace(domain.CoerceError(err)) } - result := make([]secret.RotationInfo, len(data)) + result := make([]domainsecret.RotationInfo, len(data)) for i, d := range data { - result[i] = secret.RotationInfo{ + result[i] = domainsecret.RotationInfo{ Revision: d.Revision, NextTriggerTime: d.NextRotateTime, } @@ -3438,7 +3462,7 @@ func (st State) InitialWatchStatementForSecretsRotationChanges( // GetSecretsRotationChanges returns the rotation changes for the owners' secrets. func (st State) GetSecretsRotationChanges( ctx context.Context, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, secretIDs ...string, -) ([]secret.RotationInfo, error) { +) ([]domainsecret.RotationInfo, error) { db, err := st.DB() if err != nil { return nil, errors.Trace(err) @@ -3450,7 +3474,7 @@ func (st State) getSecretsRevisionExpiryChanges( ctx context.Context, runner coredatabase.TxnRunner, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, revisionIDs ...string, -) ([]secret.ExpiryInfo, error) { +) ([]domainsecret.ExpiryInfo, error) { if len(revisionIDs) == 0 && len(appOwners) == 0 && len(unitOwners) == 0 { return nil, nil } @@ -3528,9 +3552,9 @@ GROUP BY sr.secret_id` if err != nil { return nil, errors.Trace(domain.CoerceError(err)) } - result := make([]secret.ExpiryInfo, len(data)) + result := make([]domainsecret.ExpiryInfo, len(data)) for i, d := range data { - result[i] = secret.ExpiryInfo{ + result[i] = domainsecret.ExpiryInfo{ RevisionID: d.RevisionUUID, Revision: d.Revision, NextTriggerTime: d.ExpireTime, @@ -3566,7 +3590,7 @@ func (st State) InitialWatchStatementForSecretsRevisionExpiryChanges( // GetSecretsRevisionExpiryChanges returns the expiry changes for the owners' secret revisions. func (st State) GetSecretsRevisionExpiryChanges( ctx context.Context, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, revisionUUIDs ...string, -) ([]secret.ExpiryInfo, error) { +) ([]domainsecret.ExpiryInfo, error) { db, err := st.DB() if err != nil { return nil, errors.Trace(err) diff --git a/domain/secret/state/types.go b/domain/secret/state/types.go index f2d67ff2f09..8c1597b00f4 100644 --- a/domain/secret/state/types.go +++ b/domain/secret/state/types.go @@ -63,9 +63,10 @@ type secretInfo struct { CreateTime time.Time `db:"create_time"` UpdateTime time.Time `db:"update_time"` - NextRotateTime time.Time `db:"next_rotation_time"` - LatestExpireTime time.Time `db:"latest_expire_time"` - LatestRevision int `db:"latest_revision"` + NextRotateTime time.Time `db:"next_rotation_time"` + LatestExpireTime time.Time `db:"latest_expire_time"` + LatestRevision int `db:"latest_revision"` + LatestRevisionUUID string `db:"latest_revision_uuid"` } type secretModelOwner struct { diff --git a/domain/secret/watcher_test.go b/domain/secret/watcher_test.go index 0f851f71e05..6bfb54b84f1 100644 --- a/domain/secret/watcher_test.go +++ b/domain/secret/watcher_test.go @@ -315,38 +315,36 @@ func (s *watcherSuite) TestWatchConsumedSecretsChanges(c *gc.C) { c.Assert(watcher, gc.NotNil) defer workertest.CleanKill(c, watcher) - wC := watchertest.NewStringsWatcherC(c, watcher) + wc := watchertest.NewStringsWatcherC(c, watcher) // Wait for the initial changes. - wC.AssertChange([]string(nil)...) - s.AssertChangeStreamIdle(c) - wC.AssertNoChange() + wc.AssertOneChange() // create revision 2. createNewRevision(c, st, uri1) s.AssertChangeStreamIdle(c) - wC.AssertChange( + wc.AssertChange( uri1.String(), ) - wC.AssertNoChange() + wc.AssertNoChange() // Pretend that the agent restarted and the watcher is re-created. watcher1, err := svc.WatchConsumedSecretsChanges(ctx, "mediawiki/0") c.Assert(err, gc.IsNil) c.Assert(watcher1, gc.NotNil) defer workertest.CleanKill(c, watcher1) - wC1 := watchertest.NewStringsWatcherC(c, watcher1) - wC1.AssertChange([]string(nil)...) + wc1 := watchertest.NewStringsWatcherC(c, watcher1) + wc1.AssertChange([]string(nil)...) s.AssertChangeStreamIdle(c) - wC1.AssertChange( + wc1.AssertChange( uri1.String(), ) // The consumed revision 2 is the updated current_revision. saveConsumer(uri1, 2, "mediawiki/0") - wC.AssertNoChange() - wC1.AssertNoChange() + wc.AssertNoChange() + wc1.AssertNoChange() // Pretend that the agent restarted and the watcher is re-created again. // Since we comsume the latest revision already, so there should be no change. @@ -390,31 +388,31 @@ func (s *watcherSuite) TestWatchConsumedRemoteSecretsChanges(c *gc.C) { c.Assert(err, jc.ErrorIsNil) defer workertest.CleanKill(c, watcher) - wC := watchertest.NewStringsWatcherC(c, watcher) + wc := watchertest.NewStringsWatcherC(c, watcher) // Wait for the initial changes. - wC.AssertOneChange() + wc.AssertOneChange() err = st.UpdateRemoteSecretRevision(ctx, uri1, 2) c.Assert(err, jc.ErrorIsNil) s.AssertChangeStreamIdle(c) - wC.AssertChange(uri1.String()) + wc.AssertChange(uri1.String()) // Pretend that the agent restarted and the watcher is re-created. watcher1, err := svc.WatchConsumedSecretsChanges(ctx, "mediawiki/0") c.Assert(err, jc.ErrorIsNil) defer workertest.CleanKill(c, watcher1) - wC1 := watchertest.NewStringsWatcherC(c, watcher1) - wC1.AssertChange([]string(nil)...) - wC1.AssertChange(uri1.String()) + wc1 := watchertest.NewStringsWatcherC(c, watcher1) + wc1.AssertChange([]string(nil)...) + wc1.AssertChange(uri1.String()) // The consumed revision 2 is the updated current_revision. saveConsumer(uri1, 2, "mediawiki/0") s.AssertChangeStreamIdle(c) - wC.AssertNoChange() - wC1.AssertNoChange() + wc.AssertNoChange() + wc1.AssertNoChange() // Pretend that the agent restarted and the watcher is re-created again. // Since we consume the latest revision already, so there should be no @@ -464,40 +462,37 @@ func (s *watcherSuite) TestWatchRemoteConsumedSecretsChanges(c *gc.C) { c.Assert(watcher, gc.NotNil) defer workertest.CleanKill(c, watcher) - wC := watchertest.NewStringsWatcherC(c, watcher) + wc := watchertest.NewStringsWatcherC(c, watcher) // Wait for the initial changes. - wC.AssertChange([]string(nil)...) - s.AssertChangeStreamIdle(c) - wC.AssertNoChange() + wc.AssertOneChange() // create revision 2. createNewRevision(c, st, uri1) err = st.UpdateRemoteSecretRevision(ctx, uri1, 2) c.Assert(err, jc.ErrorIsNil) - wC.AssertChange( + wc.AssertChange( uri1.String(), ) s.AssertChangeStreamIdle(c) - wC.AssertNoChange() + wc.AssertNoChange() // Pretend that the agent restarted and the watcher is re-created. watcher1, err := svc.WatchRemoteConsumedSecretsChanges(ctx, "mediawiki") c.Assert(err, gc.IsNil) c.Assert(watcher1, gc.NotNil) defer workertest.CleanKill(c, watcher1) - wC1 := watchertest.NewStringsWatcherC(c, watcher1) - wC1.AssertChange([]string(nil)...) - wC1.AssertChange( + wc1 := watchertest.NewStringsWatcherC(c, watcher1) + wc1.AssertChange([]string(nil)...) + wc1.AssertChange( uri1.String(), ) // The consumed revision 2 is the updated current_revision. saveRemoteConsumer(uri1, 2, "mediawiki/0") - s.AssertChangeStreamIdle(c) - wC.AssertNoChange() - wC1.AssertNoChange() + wc.AssertNoChange() + wc1.AssertNoChange() // Pretend that the agent restarted and the watcher is re-created again. // Since we comsume the latest revision already, so there should be no change. @@ -507,7 +502,6 @@ func (s *watcherSuite) TestWatchRemoteConsumedSecretsChanges(c *gc.C) { defer workertest.CleanKill(c, watcher1) wC2 := watchertest.NewStringsWatcherC(c, watcher2) wC2.AssertChange([]string(nil)...) - s.AssertChangeStreamIdle(c) wC2.AssertNoChange() } @@ -544,12 +538,11 @@ func (s *watcherSuite) TestWatchSecretsRotationChanges(c *gc.C) { c.Assert(watcher, gc.NotNil) defer workertest.CleanKill(c, watcher) - wC := watchertest.NewSecretsTriggerWatcherC(c, watcher) + wc := watchertest.NewSecretsTriggerWatcherC(c, watcher) // Wait for the initial changes. - wC.AssertChange([]corewatcher.SecretTriggerChange(nil)...) - s.AssertChangeStreamIdle(c) - wC.AssertNoChange() + wc.AssertChange([]corewatcher.SecretTriggerChange(nil)...) + wc.AssertNoChange() now := time.Now() err = st.SecretRotated(ctx, uri1, now.Add(1*time.Hour)) @@ -557,7 +550,7 @@ func (s *watcherSuite) TestWatchSecretsRotationChanges(c *gc.C) { err = st.SecretRotated(ctx, uri2, now.Add(2*time.Hour)) c.Assert(err, jc.ErrorIsNil) - wC.AssertChange( + wc.AssertChange( corewatcher.SecretTriggerChange{ URI: uri1, Revision: 1, @@ -570,8 +563,7 @@ func (s *watcherSuite) TestWatchSecretsRotationChanges(c *gc.C) { }, ) - s.AssertChangeStreamIdle(c) - wC.AssertNoChange() + wc.AssertNoChange() // Pretend that the agent restarted and the watcher is re-created. watcher1, err := svc.WatchSecretsRotationChanges(context.Background(), @@ -587,9 +579,9 @@ func (s *watcherSuite) TestWatchSecretsRotationChanges(c *gc.C) { c.Assert(err, gc.IsNil) c.Assert(watcher1, gc.NotNil) defer workertest.CleanKill(c, watcher1) - wC1 := watchertest.NewSecretsTriggerWatcherC(c, watcher1) - wC1.AssertChange([]corewatcher.SecretTriggerChange(nil)...) - wC1.AssertChange( + wc1 := watchertest.NewSecretsTriggerWatcherC(c, watcher1) + wc1.AssertChange([]corewatcher.SecretTriggerChange(nil)...) + wc1.AssertChange( corewatcher.SecretTriggerChange{ URI: uri1, Revision: 1, @@ -601,8 +593,7 @@ func (s *watcherSuite) TestWatchSecretsRotationChanges(c *gc.C) { NextTriggerTime: now.Add(2 * time.Hour), }, ) - s.AssertChangeStreamIdle(c) - wC1.AssertNoChange() + wc1.AssertNoChange() } func ptr[T any](v T) *T { @@ -639,12 +630,11 @@ func (s *watcherSuite) TestWatchSecretsRevisionExpiryChanges(c *gc.C) { c.Assert(watcher, gc.NotNil) defer workertest.CleanKill(c, watcher) - wC := watchertest.NewSecretsTriggerWatcherC(c, watcher) + wc := watchertest.NewSecretsTriggerWatcherC(c, watcher) // Wait for the initial changes. - wC.AssertChange([]corewatcher.SecretTriggerChange(nil)...) - s.AssertChangeStreamIdle(c) - wC.AssertNoChange() + wc.AssertChange([]corewatcher.SecretTriggerChange(nil)...) + wc.AssertNoChange() now := time.Now() err = st.CreateCharmApplicationSecret(ctx, 1, uri1, "mysql", secret.UpsertSecretParams{ @@ -660,7 +650,7 @@ func (s *watcherSuite) TestWatchSecretsRevisionExpiryChanges(c *gc.C) { c.Assert(err, jc.ErrorIsNil) s.AssertChangeStreamIdle(c) - wC.AssertChange( + wc.AssertChange( corewatcher.SecretTriggerChange{ URI: uri1, Revision: 1, @@ -673,8 +663,7 @@ func (s *watcherSuite) TestWatchSecretsRevisionExpiryChanges(c *gc.C) { }, ) - s.AssertChangeStreamIdle(c) - wC.AssertNoChange() + wc.AssertNoChange() // Pretend that the agent restarted and the watcher is re-created. watcher1, err := svc.WatchSecretRevisionsExpiryChanges(context.Background(), @@ -690,10 +679,10 @@ func (s *watcherSuite) TestWatchSecretsRevisionExpiryChanges(c *gc.C) { c.Assert(err, gc.IsNil) c.Assert(watcher1, gc.NotNil) defer workertest.CleanKill(c, watcher1) - wC1 := watchertest.NewSecretsTriggerWatcherC(c, watcher1) - wC1.AssertChange([]corewatcher.SecretTriggerChange(nil)...) + wc1 := watchertest.NewSecretsTriggerWatcherC(c, watcher1) + wc1.AssertChange([]corewatcher.SecretTriggerChange(nil)...) s.AssertChangeStreamIdle(c) - wC1.AssertChange( + wc1.AssertChange( corewatcher.SecretTriggerChange{ URI: uri1, Revision: 1, @@ -705,6 +694,6 @@ func (s *watcherSuite) TestWatchSecretsRevisionExpiryChanges(c *gc.C) { NextTriggerTime: now.Add(2 * time.Hour).UTC(), }, ) - s.AssertChangeStreamIdle(c) - wC1.AssertNoChange() + + wc1.AssertNoChange() }