From 3d604bcf54e42dab61cd90c02f19103b388abea9 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Wed, 29 May 2024 21:14:59 +1000 Subject: [PATCH 1/3] Add state methods for secret expiry watcher; --- domain/secret/state/state.go | 166 +++++++++++++++++++++++++---- domain/secret/state/state_test.go | 169 ++++++++++++++++++++++++++++++ domain/secret/state/types.go | 7 ++ 3 files changed, 323 insertions(+), 19 deletions(-) diff --git a/domain/secret/state/state.go b/domain/secret/state/state.go index 9492b71a079..12c09ad7e6c 100644 --- a/domain/secret/state/state.go +++ b/domain/secret/state/state.go @@ -3087,11 +3087,11 @@ FROM secret_revision_obsolete sro } if len(appOwners) > 0 && len(unitOwners) > 0 { queryParams = append(queryParams, appOwners, unitOwners) - joins = append(joins, - `LEFT JOIN secret_application_owner sao ON sr.secret_id = sao.secret_id`, - `LEFT JOIN application ON application.uuid = sao.application_uuid`, - `LEFT JOIN secret_unit_owner suo ON sr.secret_id = suo.secret_id`, - `LEFT JOIN unit ON unit.uuid = suo.unit_uuid`, + joins = append(joins, ` + LEFT JOIN secret_application_owner sao ON sr.secret_id = sao.secret_id + LEFT JOIN application ON application.uuid = sao.application_uuid + LEFT JOIN secret_unit_owner suo ON sr.secret_id = suo.secret_id + LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], ) conditions = append(conditions, `AND ( sao.application_uuid IS NOT NULL AND application.name IN ($ApplicationOwners[:]) @@ -3099,16 +3099,16 @@ FROM secret_revision_obsolete sro )`) } else if len(appOwners) > 0 { queryParams = append(queryParams, appOwners) - joins = append(joins, - `LEFT JOIN secret_application_owner sao ON sr.secret_id = sao.secret_id`, - `LEFT JOIN application ON application.uuid = sao.application_uuid`, + joins = append(joins, ` + LEFT JOIN secret_application_owner sao ON sr.secret_id = sao.secret_id + LEFT JOIN application ON application.uuid = sao.application_uuid`[1:], ) conditions = append(conditions, "AND sao.application_uuid IS NOT NULL AND application.name IN ($ApplicationOwners[:])") } else if len(unitOwners) > 0 { queryParams = append(queryParams, unitOwners) - joins = append(joins, - `LEFT JOIN secret_unit_owner suo ON sr.secret_id = suo.secret_id`, - `LEFT JOIN unit ON unit.uuid = suo.unit_uuid`, + joins = append(joins, ` + LEFT JOIN secret_unit_owner suo ON sr.secret_id = suo.secret_id + LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], ) conditions = append(conditions, "AND suo.unit_uuid IS NOT NULL AND unit.unit_id IN ($UnitOwners[:])") } @@ -3347,10 +3347,10 @@ FROM secret_rotation sro if len(appOwners) > 0 && len(unitOwners) > 0 { queryParams = append(queryParams, appOwners, unitOwners) joins = append(joins, ` -LEFT JOIN secret_application_owner sao ON sro.secret_id = sao.secret_id -LEFT JOIN application ON application.uuid = sao.application_uuid -LEFT JOIN secret_unit_owner suo ON sro.secret_id = suo.secret_id -LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], + LEFT JOIN secret_application_owner sao ON sro.secret_id = sao.secret_id + LEFT JOIN application ON application.uuid = sao.application_uuid + LEFT JOIN secret_unit_owner suo ON sro.secret_id = suo.secret_id + LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], ) conditions = append(conditions, `( sao.application_uuid IS NOT NULL AND application.name IN ($ApplicationOwners[:]) @@ -3359,15 +3359,15 @@ LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], } else if len(appOwners) > 0 { queryParams = append(queryParams, appOwners) joins = append(joins, ` -LEFT JOIN secret_application_owner sao ON sro.secret_id = sao.secret_id -LEFT JOIN application ON application.uuid = sao.application_uuid`[1:], + LEFT JOIN secret_application_owner sao ON sro.secret_id = sao.secret_id + LEFT JOIN application ON application.uuid = sao.application_uuid`[1:], ) conditions = append(conditions, "sao.application_uuid IS NOT NULL AND application.name IN ($ApplicationOwners[:])") } else if len(unitOwners) > 0 { queryParams = append(queryParams, unitOwners) joins = append(joins, ` -LEFT JOIN secret_unit_owner suo ON sro.secret_id = suo.secret_id -LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], + LEFT JOIN secret_unit_owner suo ON sro.secret_id = suo.secret_id + LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], ) conditions = append(conditions, "suo.unit_uuid IS NOT NULL AND unit.unit_id IN ($UnitOwners[:])") } @@ -3445,3 +3445,131 @@ func (st State) GetSecretsRotationChanges( } return st.getSecretsRotationChanges(ctx, db, appOwners, unitOwners, secretIDs...) } + +func (st State) getSecretsRevisionExpiryChanges( + ctx context.Context, runner coredatabase.TxnRunner, + appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, + revisionIDs ...string, +) ([]secret.ExpiryInfo, error) { + if len(revisionIDs) == 0 && len(appOwners) == 0 && len(unitOwners) == 0 { + return nil, nil + } + + q := ` +SELECT + sr.secret_id AS &secretRevisionExpireChange.secret_id, + sre.revision_uuid AS &secretRevisionExpireChange.revision_uuid, + sre.expire_time AS &secretRevisionExpireChange.expire_time, + MAX(sr.revision) AS &secretRevisionExpireChange.revision +FROM secret_revision_expire sre + JOIN secret_revision sr ON sr.uuid = sre.revision_uuid` + + var queryParams []any + var joins []string + conditions := []string{} + if len(revisionIDs) > 0 { + queryParams = append(queryParams, revisionUUIDs(revisionIDs)) + conditions = append(conditions, "sre.revision_uuid IN ($revisionUUIDs[:])") + } + if len(appOwners) > 0 && len(unitOwners) > 0 { + queryParams = append(queryParams, appOwners, unitOwners) + joins = append(joins, ` + LEFT JOIN secret_application_owner sao ON sr.secret_id = sao.secret_id + LEFT JOIN application ON application.uuid = sao.application_uuid + LEFT JOIN secret_unit_owner suo ON sr.secret_id = suo.secret_id + LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], + ) + conditions = append(conditions, `( + sao.application_uuid IS NOT NULL AND application.name IN ($ApplicationOwners[:]) + OR suo.unit_uuid IS NOT NULL AND unit.unit_id IN ($UnitOwners[:]) +)`) + } else if len(appOwners) > 0 { + queryParams = append(queryParams, appOwners) + joins = append(joins, ` + LEFT JOIN secret_application_owner sao ON sr.secret_id = sao.secret_id + LEFT JOIN application ON application.uuid = sao.application_uuid`[1:], + ) + conditions = append(conditions, "sao.application_uuid IS NOT NULL AND application.name IN ($ApplicationOwners[:])") + } else if len(unitOwners) > 0 { + queryParams = append(queryParams, unitOwners) + joins = append(joins, ` + LEFT JOIN secret_unit_owner suo ON sr.secret_id = suo.secret_id + LEFT JOIN unit ON unit.uuid = suo.unit_uuid`[1:], + ) + conditions = append(conditions, "suo.unit_uuid IS NOT NULL AND unit.unit_id IN ($UnitOwners[:])") + } + if len(joins) > 0 { + q += fmt.Sprintf("\n%s", strings.Join(joins, "\n")) + } + if len(conditions) > 0 { + q += fmt.Sprintf("\nWHERE %s", strings.Join(conditions, "\nAND ")) + } + q += ` +GROUP BY sr.secret_id` + st.logger.Tracef( + "revisionIDs %+v, appOwners: %+v, unitOwners: %+v, query: \n%s", + revisionIDs, appOwners, unitOwners, q, + ) + + stmt, err := st.Prepare(q, append(queryParams, secretRevisionExpireChange{})...) + if err != nil { + return nil, errors.Trace(err) + } + var data []secretRevisionExpireChange + err = runner.Txn(ctx, func(ctx context.Context, tx *sqlair.TX) error { + err := tx.Query(ctx, stmt, queryParams...).GetAll(&data) + if errors.Is(err, sqlair.ErrNoRows) { + // It's ok because the secret or the expiry was just deleted. + return nil + } + return errors.Trace(err) + }) + + if err != nil { + return nil, errors.Trace(domain.CoerceError(err)) + } + result := make([]secret.ExpiryInfo, len(data)) + for i, d := range data { + result[i] = secret.ExpiryInfo{ + RevisionID: d.RevisionUUID, + Revision: d.Revision, + NextTriggerTime: d.ExpireTime, + } + uri, err := coresecrets.ParseURI(d.SecretID) + if err != nil { + return nil, errors.Trace(err) + } + result[i].URI = uri + } + return result, nil +} + +// InitialWatchStatementForSecretsRevisionExpiryChanges returns the initial watch statement +// and the table name for watching secret revision expiry changes. +func (st State) InitialWatchStatementForSecretsRevisionExpiryChanges( + appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, +) (string, eventsource.NamespaceQuery) { + queryFunc := func(ctx context.Context, runner coredatabase.TxnRunner) ([]string, error) { + result, err := st.getSecretsRevisionExpiryChanges(ctx, runner, appOwners, unitOwners) + if err != nil { + return nil, errors.Trace(err) + } + revisionUUIDs := make([]string, len(result)) + for i, d := range result { + revisionUUIDs[i] = d.RevisionID + } + return revisionUUIDs, nil + } + return "secret_revision_expire", queryFunc +} + +// 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) { + db, err := st.DB() + if err != nil { + return nil, errors.Trace(err) + } + return st.getSecretsRevisionExpiryChanges(ctx, db, appOwners, unitOwners, revisionUUIDs...) +} diff --git a/domain/secret/state/state_test.go b/domain/secret/state/state_test.go index 62afff2c37d..bb325de609d 100644 --- a/domain/secret/state/state_test.go +++ b/domain/secret/state/state_test.go @@ -3345,6 +3345,175 @@ func (s *stateSuite) TestGetSecretsRotationChanges(c *gc.C) { c.Check(result, gc.HasLen, 0) } +func (s *stateSuite) prepareWatchForWatchStatementForSecretsRevisionExpiryChanges(c *gc.C, ctx context.Context, st *State) (time.Time, *coresecrets.URI, *coresecrets.URI) { + s.setupUnits(c, "mysql") + s.setupUnits(c, "mediawiki") + + now := time.Now() + uri1 := coresecrets.NewURI() + err := st.CreateCharmApplicationSecret(ctx, 1, uri1, "mysql", domainsecret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo": "bar", "hello": "world"}, + ExpireTime: ptr(now.Add(1 * time.Hour)), + }) + c.Assert(err, jc.ErrorIsNil) + + uri2 := coresecrets.NewURI() + err = st.CreateCharmUnitSecret(ctx, 1, uri2, "mediawiki/0", domainsecret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo": "bar", "hello": "world"}, + }) + c.Assert(err, jc.ErrorIsNil) + err = st.UpdateSecret(context.Background(), uri2, domainsecret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo-new": "bar-new"}, + ExpireTime: ptr(now.Add(2 * time.Hour)), + }) + c.Assert(err, jc.ErrorIsNil) + return now, uri1, uri2 +} + +func (s *stateSuite) TestInitialWatchStatementForSecretsRevisionExpiryChanges(c *gc.C) { + st := newSecretState(c, s.TxnRunnerFactory()) + ctx := context.Background() + _, uri1, uri2 := s.prepareWatchForWatchStatementForSecretsRevisionExpiryChanges(c, ctx, st) + + tableName, f := st.InitialWatchStatementForSecretsRevisionExpiryChanges(domainsecret.ApplicationOwners{"mysql"}, domainsecret.UnitOwners{"mediawiki/0"}) + c.Check(tableName, gc.Equals, "secret_revision_expire") + result, err := f(ctx, s.TxnRunner()) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []string{ + getRevUUID(c, s.DB(), uri1, 1), + getRevUUID(c, s.DB(), uri2, 2), + }) + + tableName, f = st.InitialWatchStatementForSecretsRevisionExpiryChanges(domainsecret.ApplicationOwners{"mysql"}, nil) + c.Check(tableName, gc.Equals, "secret_revision_expire") + result, err = f(ctx, s.TxnRunner()) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []string{ + getRevUUID(c, s.DB(), uri1, 1), + }) + + tableName, f = st.InitialWatchStatementForSecretsRevisionExpiryChanges(nil, domainsecret.UnitOwners{"mediawiki/0"}) + c.Check(tableName, gc.Equals, "secret_revision_expire") + result, err = f(ctx, s.TxnRunner()) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []string{ + getRevUUID(c, s.DB(), uri2, 2), + }) + + tableName, f = st.InitialWatchStatementForSecretsRevisionExpiryChanges(nil, nil) + c.Check(tableName, gc.Equals, "secret_revision_expire") + result, err = f(ctx, s.TxnRunner()) + c.Check(err, jc.ErrorIsNil) + c.Check(result, gc.HasLen, 0) +} + +func (s *stateSuite) TestGetSecretsRevisionExpiryChanges(c *gc.C) { + st := newSecretState(c, s.TxnRunnerFactory()) + ctx := context.Background() + now, uri1, uri2 := s.prepareWatchForWatchStatementForSecretsRevisionExpiryChanges(c, ctx, st) + + result, err := st.GetSecretsRevisionExpiryChanges(ctx, domainsecret.ApplicationOwners{"mysql"}, domainsecret.UnitOwners{"mediawiki/0"}) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []domainsecret.ExpiryInfo{ + { + URI: uri1, + Revision: 1, + RevisionID: getRevUUID(c, s.DB(), uri1, 1), + NextTriggerTime: now.Add(1 * time.Hour).UTC(), + }, + { + URI: uri2, + Revision: 2, + RevisionID: getRevUUID(c, s.DB(), uri2, 2), + NextTriggerTime: now.Add(2 * time.Hour).UTC(), + }, + }) + + result, err = st.GetSecretsRevisionExpiryChanges(ctx, + domainsecret.ApplicationOwners{"mysql", "mediawiki"}, domainsecret.UnitOwners{"mysql/0", "mediawiki/0"}, + getRevUUID(c, s.DB(), uri1, 1), + ) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []domainsecret.ExpiryInfo{ + { + URI: uri1, + Revision: 1, + RevisionID: getRevUUID(c, s.DB(), uri1, 1), + NextTriggerTime: now.Add(1 * time.Hour).UTC(), + }, + }) + + result, err = st.GetSecretsRevisionExpiryChanges(ctx, + domainsecret.ApplicationOwners{"mysql", "mediawiki"}, domainsecret.UnitOwners{"mysql/0", "mediawiki/0"}, + getRevUUID(c, s.DB(), uri2, 2), + ) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []domainsecret.ExpiryInfo{ + { + URI: uri2, + Revision: 2, + RevisionID: getRevUUID(c, s.DB(), uri2, 2), + NextTriggerTime: now.Add(2 * time.Hour).UTC(), + }, + }) + + result, err = st.GetSecretsRevisionExpiryChanges(ctx, domainsecret.ApplicationOwners{"mysql"}, nil) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []domainsecret.ExpiryInfo{ + { + URI: uri1, + Revision: 1, + RevisionID: getRevUUID(c, s.DB(), uri1, 1), + NextTriggerTime: now.Add(1 * time.Hour).UTC(), + }, + }) + + // The uri2 is not owned by mysql, so it should not be returned. + result, err = st.GetSecretsRevisionExpiryChanges(ctx, domainsecret.ApplicationOwners{"mysql"}, nil, + getRevUUID(c, s.DB(), uri1, 1), + getRevUUID(c, s.DB(), uri2, 2), + ) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []domainsecret.ExpiryInfo{ + { + URI: uri1, + Revision: 1, + RevisionID: getRevUUID(c, s.DB(), uri1, 1), + NextTriggerTime: now.Add(1 * time.Hour).UTC(), + }, + }) + + result, err = st.GetSecretsRevisionExpiryChanges(ctx, nil, domainsecret.UnitOwners{"mediawiki/0"}) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []domainsecret.ExpiryInfo{ + { + URI: uri2, + Revision: 2, + RevisionID: getRevUUID(c, s.DB(), uri2, 2), + NextTriggerTime: now.Add(2 * time.Hour).UTC(), + }, + }) + + // The uri1 is not owned by mediawiki/0, so it should not be returned. + result, err = st.GetSecretsRevisionExpiryChanges(ctx, nil, domainsecret.UnitOwners{"mediawiki/0"}, + getRevUUID(c, s.DB(), uri1, 1), + getRevUUID(c, s.DB(), uri2, 2), + ) + c.Check(err, jc.ErrorIsNil) + c.Check(result, jc.SameContents, []domainsecret.ExpiryInfo{ + { + URI: uri2, + Revision: 2, + RevisionID: getRevUUID(c, s.DB(), uri2, 2), + NextTriggerTime: now.Add(2 * time.Hour).UTC(), + }, + }) + + result, err = st.GetSecretsRevisionExpiryChanges(ctx, nil, nil) + c.Check(err, jc.ErrorIsNil) + c.Check(result, gc.HasLen, 0) +} + func (s *stateSuite) TestSecretRotated(c *gc.C) { st := newSecretState(c, s.TxnRunnerFactory()) ctx := context.Background() diff --git a/domain/secret/state/types.go b/domain/secret/state/types.go index 8e4e4548c7f..f2d67ff2f09 100644 --- a/domain/secret/state/types.go +++ b/domain/secret/state/types.go @@ -121,6 +121,13 @@ type secretRevisionExpire struct { ExpireTime time.Time `db:"expire_time"` } +type secretRevisionExpireChange struct { + SecretID string `db:"secret_id"` + RevisionUUID string `db:"revision_uuid"` + Revision int `db:"revision"` + ExpireTime time.Time `db:"expire_time"` +} + type secretContent struct { RevisionUUID string `db:"revision_uuid"` Name string `db:"name"` From 3313fbca9b10d36ec782c535a0e41773e4868325 Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Wed, 29 May 2024 21:15:41 +1000 Subject: [PATCH 2/3] Implement WatchSecretRevisionsExpiryChanges watcher; --- domain/secret/service/service.go | 8 ++ domain/secret/service/service_test.go | 83 ++++++++++++++++++ domain/secret/service/state_mock_test.go | 83 ++++++++++++++++++ domain/secret/service/watcher.go | 68 ++++++--------- domain/secret/types.go | 8 ++ domain/secret/watcher_test.go | 105 ++++++++++++++++++++++- 6 files changed, 314 insertions(+), 41 deletions(-) diff --git a/domain/secret/service/service.go b/domain/secret/service/service.go index 3dc1f1f59c9..3bea765cd75 100644 --- a/domain/secret/service/service.go +++ b/domain/secret/service/service.go @@ -93,6 +93,14 @@ type State interface { GetSecretsRotationChanges( ctx context.Context, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, secretIDs ...string, ) ([]domainsecret.RotationInfo, error) + + // For watching secret revision expiry changes. + InitialWatchStatementForSecretsRevisionExpiryChanges( + appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, + ) (string, eventsource.NamespaceQuery) + GetSecretsRevisionExpiryChanges( + ctx context.Context, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, revisionUUIDs ...string, + ) ([]domainsecret.ExpiryInfo, error) } // WatcherFactory describes methods for creating watchers. diff --git a/domain/secret/service/service_test.go b/domain/secret/service/service_test.go index 9e071add80b..05d29c2f06a 100644 --- a/domain/secret/service/service_test.go +++ b/domain/secret/service/service_test.go @@ -1692,3 +1692,86 @@ func (s *serviceSuite) TestWatchSecretsRotationChanges(c *gc.C) { ) wC.AssertNoChange() } + +func (s *serviceSuite) TestWatchSecretRevisionsExpiryChanges(c *gc.C) { + ctrl := gomock.NewController(c) + defer ctrl.Finish() + + s.state = NewMockState(ctrl) + mockWatcherFactory := NewMockWatcherFactory(ctrl) + + uri1 := coresecrets.NewURI() + uri2 := coresecrets.NewURI() + + ch := make(chan []string) + mockStringWatcher := NewMockStringsWatcher(ctrl) + mockStringWatcher.EXPECT().Changes().Return(ch).AnyTimes() + mockStringWatcher.EXPECT().Wait().Return(nil).AnyTimes() + mockStringWatcher.EXPECT().Kill().AnyTimes() + + var namespaceQuery eventsource.NamespaceQuery = func(context.Context, database.TxnRunner) ([]string, error) { + return nil, nil + } + s.state.EXPECT().InitialWatchStatementForSecretsRevisionExpiryChanges( + domainsecret.ApplicationOwners{"mediawiki"}, domainsecret.UnitOwners{"mysql/0", "mysql/1"}, + ).Return("secret_revision_expire", namespaceQuery) + mockWatcherFactory.EXPECT().NewNamespaceWatcher("secret_revision_expire", changestream.All, gomock.Any()).Return(mockStringWatcher, nil) + + now := time.Now() + s.state.EXPECT().GetSecretsRevisionExpiryChanges(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, appOwners domainsecret.ApplicationOwners, unitOwners domainsecret.UnitOwners, revisionUUIDs ...string) ([]domainsecret.ExpiryInfo, error) { + c.Assert(appOwners, jc.SameContents, domainsecret.ApplicationOwners{"mediawiki"}) + c.Assert(unitOwners, jc.SameContents, domainsecret.UnitOwners{"mysql/0", "mysql/1"}) + c.Assert(revisionUUIDs, jc.SameContents, []string{"revision-uuid-1", "revision-uuid-2"}) + return []domainsecret.ExpiryInfo{ + { + URI: uri1, + Revision: 1, + NextTriggerTime: now, + }, + { + URI: uri2, + Revision: 2, + NextTriggerTime: now.Add(2 * time.Hour), + }, + }, nil + }) + svc := NewWatchableService(s.state, loggertesting.WrapCheckLog(c), mockWatcherFactory, nil) + w, err := svc.WatchSecretRevisionsExpiryChanges(context.Background(), + CharmSecretOwner{ + Kind: ApplicationOwner, + ID: "mediawiki", + }, + CharmSecretOwner{ + Kind: UnitOwner, + ID: "mysql/0", + }, + CharmSecretOwner{ + Kind: UnitOwner, + ID: "mysql/1", + }, + ) + c.Assert(err, jc.ErrorIsNil) + c.Assert(w, gc.NotNil) + defer workertest.CleanKill(c, w) + wC := watchertest.NewSecretsTriggerWatcherC(c, w) + + select { + case ch <- []string{"revision-uuid-1", "revision-uuid-2"}: + case <-time.After(coretesting.ShortWait): + c.Fatalf("timed out waiting for the initial changes") + } + + wC.AssertChange( + watcher.SecretTriggerChange{ + URI: uri1, + Revision: 1, + NextTriggerTime: now, + }, + watcher.SecretTriggerChange{ + URI: uri2, + Revision: 2, + NextTriggerTime: now.Add(2 * time.Hour), + }, + ) + wC.AssertNoChange() +} diff --git a/domain/secret/service/state_mock_test.go b/domain/secret/service/state_mock_test.go index 87973db5dbc..e2ab3d738a2 100644 --- a/domain/secret/service/state_mock_test.go +++ b/domain/secret/service/state_mock_test.go @@ -764,6 +764,50 @@ func (c *MockStateGetSecretValueCall) DoAndReturn(f func(context.Context, *secre return c } +// GetSecretsRevisionExpiryChanges mocks base method. +func (m *MockState) GetSecretsRevisionExpiryChanges(arg0 context.Context, arg1 secret.ApplicationOwners, arg2 secret.UnitOwners, arg3 ...string) ([]secret.ExpiryInfo, error) { + m.ctrl.T.Helper() + varargs := []any{arg0, arg1, arg2} + for _, a := range arg3 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetSecretsRevisionExpiryChanges", varargs...) + ret0, _ := ret[0].([]secret.ExpiryInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSecretsRevisionExpiryChanges indicates an expected call of GetSecretsRevisionExpiryChanges. +func (mr *MockStateMockRecorder) GetSecretsRevisionExpiryChanges(arg0, arg1, arg2 any, arg3 ...any) *MockStateGetSecretsRevisionExpiryChangesCall { + mr.mock.ctrl.T.Helper() + varargs := append([]any{arg0, arg1, arg2}, arg3...) + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSecretsRevisionExpiryChanges", reflect.TypeOf((*MockState)(nil).GetSecretsRevisionExpiryChanges), varargs...) + return &MockStateGetSecretsRevisionExpiryChangesCall{Call: call} +} + +// MockStateGetSecretsRevisionExpiryChangesCall wrap *gomock.Call +type MockStateGetSecretsRevisionExpiryChangesCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateGetSecretsRevisionExpiryChangesCall) Return(arg0 []secret.ExpiryInfo, arg1 error) *MockStateGetSecretsRevisionExpiryChangesCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateGetSecretsRevisionExpiryChangesCall) Do(f func(context.Context, secret.ApplicationOwners, secret.UnitOwners, ...string) ([]secret.ExpiryInfo, error)) *MockStateGetSecretsRevisionExpiryChangesCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateGetSecretsRevisionExpiryChangesCall) DoAndReturn(f func(context.Context, secret.ApplicationOwners, secret.UnitOwners, ...string) ([]secret.ExpiryInfo, error)) *MockStateGetSecretsRevisionExpiryChangesCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // GetSecretsRotationChanges mocks base method. func (m *MockState) GetSecretsRotationChanges(arg0 context.Context, arg1 secret.ApplicationOwners, arg2 secret.UnitOwners, arg3 ...string) ([]secret.RotationInfo, error) { m.ctrl.T.Helper() @@ -1080,6 +1124,45 @@ func (c *MockStateInitialWatchStatementForRemoteConsumedSecretsChangesFromOfferi return c } +// InitialWatchStatementForSecretsRevisionExpiryChanges mocks base method. +func (m *MockState) InitialWatchStatementForSecretsRevisionExpiryChanges(arg0 secret.ApplicationOwners, arg1 secret.UnitOwners) (string, eventsource.NamespaceQuery) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InitialWatchStatementForSecretsRevisionExpiryChanges", arg0, arg1) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(eventsource.NamespaceQuery) + return ret0, ret1 +} + +// InitialWatchStatementForSecretsRevisionExpiryChanges indicates an expected call of InitialWatchStatementForSecretsRevisionExpiryChanges. +func (mr *MockStateMockRecorder) InitialWatchStatementForSecretsRevisionExpiryChanges(arg0, arg1 any) *MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InitialWatchStatementForSecretsRevisionExpiryChanges", reflect.TypeOf((*MockState)(nil).InitialWatchStatementForSecretsRevisionExpiryChanges), arg0, arg1) + return &MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall{Call: call} +} + +// MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall wrap *gomock.Call +type MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall) Return(arg0 string, arg1 eventsource.NamespaceQuery) *MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall) Do(f func(secret.ApplicationOwners, secret.UnitOwners) (string, eventsource.NamespaceQuery)) *MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall) DoAndReturn(f func(secret.ApplicationOwners, secret.UnitOwners) (string, eventsource.NamespaceQuery)) *MockStateInitialWatchStatementForSecretsRevisionExpiryChangesCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // InitialWatchStatementForSecretsRotationChanges mocks base method. func (m *MockState) InitialWatchStatementForSecretsRotationChanges(arg0 secret.ApplicationOwners, arg1 secret.UnitOwners) (string, eventsource.NamespaceQuery) { m.ctrl.T.Helper() diff --git a/domain/secret/service/watcher.go b/domain/secret/service/watcher.go index c8ade2d9cba..22fab270b97 100644 --- a/domain/secret/service/watcher.go +++ b/domain/secret/service/watcher.go @@ -12,7 +12,6 @@ import ( "github.com/juju/errors" "github.com/juju/worker/v4" "github.com/juju/worker/v4/catacomb" - "gopkg.in/tomb.v2" "github.com/juju/juju/core/changestream" "github.com/juju/juju/core/logger" @@ -125,47 +124,36 @@ func (s *WatchableService) WatchObsolete(ctx context.Context, owners ...CharmSec return newSecretWatcher(w, s.logger, processChanges) } -// TODO(secrets) - replace with real watcher -func newMockTriggerWatcher(ch watcher.SecretTriggerChannel) *mockSecretTriggerWatcher { - w := &mockSecretTriggerWatcher{ch: ch} - w.tomb.Go(func() error { - <-w.tomb.Dying() - return tomb.ErrDying - }) - return w -} - -type mockSecretTriggerWatcher struct { - tomb tomb.Tomb - ch watcher.SecretTriggerChannel -} - -func (w *mockSecretTriggerWatcher) Changes() watcher.SecretTriggerChannel { - return w.ch -} - -func (w *mockSecretTriggerWatcher) Stop() error { - w.Kill() - return w.Wait() -} - -func (w *mockSecretTriggerWatcher) Kill() { - w.tomb.Kill(nil) -} - -func (w *mockSecretTriggerWatcher) Err() error { - return w.tomb.Err() -} - -func (w *mockSecretTriggerWatcher) Wait() error { - return w.tomb.Wait() -} - // WatchSecretRevisionsExpiryChanges returns a watcher that notifies when the expiry time of a secret revision changes. func (s *WatchableService) WatchSecretRevisionsExpiryChanges(ctx context.Context, owners ...CharmSecretOwner) (watcher.SecretTriggerWatcher, error) { - ch := make(chan []watcher.SecretTriggerChange, 1) - ch <- []watcher.SecretTriggerChange{} - return newMockTriggerWatcher(ch), nil + if len(owners) == 0 { + return nil, errors.New("at least one owner must be provided") + } + + appOwners, unitOwners := splitCharmSecretOwners(owners...) + table, query := s.st.InitialWatchStatementForSecretsRevisionExpiryChanges(appOwners, unitOwners) + w, err := s.watcherFactory.NewNamespaceWatcher( + table, changestream.All, query, + ) + if err != nil { + return nil, errors.Trace(err) + } + processChanges := func(ctx context.Context, revisionUUIDs ...string) ([]watcher.SecretTriggerChange, error) { + result, err := s.st.GetSecretsRevisionExpiryChanges(ctx, appOwners, unitOwners, revisionUUIDs...) + if err != nil { + return nil, errors.Trace(err) + } + changes := make([]watcher.SecretTriggerChange, len(result)) + for i, r := range result { + changes[i] = watcher.SecretTriggerChange{ + URI: r.URI, + Revision: r.Revision, + NextTriggerTime: r.NextTriggerTime, + } + } + return changes, nil + } + return newSecretWatcher(w, s.logger, processChanges) } // WatchSecretsRotationChanges returns a watcher that notifies when the rotation time of a secret changes. diff --git a/domain/secret/types.go b/domain/secret/types.go index 1e90724f8ba..a134bb7e897 100644 --- a/domain/secret/types.go +++ b/domain/secret/types.go @@ -92,3 +92,11 @@ type RotationInfo struct { Revision int NextTriggerTime time.Time } + +// ExpiryInfo holds information about the expiry of a secret revision. +type ExpiryInfo struct { + URI *secrets.URI + Revision int + RevisionID string + NextTriggerTime time.Time +} diff --git a/domain/secret/watcher_test.go b/domain/secret/watcher_test.go index cc189b173c5..0f851f71e05 100644 --- a/domain/secret/watcher_test.go +++ b/domain/secret/watcher_test.go @@ -556,7 +556,6 @@ func (s *watcherSuite) TestWatchSecretsRotationChanges(c *gc.C) { c.Assert(err, jc.ErrorIsNil) err = st.SecretRotated(ctx, uri2, now.Add(2*time.Hour)) c.Assert(err, jc.ErrorIsNil) - c.Logf("uri1: %v, uri2: %v", uri1, uri2) wC.AssertChange( corewatcher.SecretTriggerChange{ @@ -605,3 +604,107 @@ func (s *watcherSuite) TestWatchSecretsRotationChanges(c *gc.C) { s.AssertChangeStreamIdle(c) wC1.AssertNoChange() } + +func ptr[T any](v T) *T { + return &v +} + +func (s *watcherSuite) TestWatchSecretsRevisionExpiryChanges(c *gc.C) { + s.setupUnits(c, "mysql") + s.setupUnits(c, "mediawiki") + + ctx := context.Background() + svc, st := s.setupServiceAndState(c) + + uri1 := coresecrets.NewURI() + uri2 := coresecrets.NewURI() + c.Logf("uri1: %v, uri2: %v", uri1, uri2) + + err := st.CreateCharmUnitSecret(ctx, 1, uri2, "mediawiki/0", secret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo": "bar", "hello": "world"}, + }) + c.Assert(err, jc.ErrorIsNil) + + watcher, err := svc.WatchSecretRevisionsExpiryChanges(context.Background(), + service.CharmSecretOwner{ + Kind: service.ApplicationOwner, + ID: "mysql", + }, + service.CharmSecretOwner{ + Kind: service.UnitOwner, + ID: "mediawiki/0", + }, + ) + c.Assert(err, gc.IsNil) + c.Assert(watcher, gc.NotNil) + defer workertest.CleanKill(c, watcher) + + wC := watchertest.NewSecretsTriggerWatcherC(c, watcher) + + // Wait for the initial changes. + wC.AssertChange([]corewatcher.SecretTriggerChange(nil)...) + s.AssertChangeStreamIdle(c) + wC.AssertNoChange() + + now := time.Now() + err = st.CreateCharmApplicationSecret(ctx, 1, uri1, "mysql", secret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo": "bar", "hello": "world"}, + ExpireTime: ptr(now.Add(1 * time.Hour)), + }) + c.Assert(err, jc.ErrorIsNil) + + err = st.UpdateSecret(context.Background(), uri2, secret.UpsertSecretParams{ + Data: coresecrets.SecretData{"foo-new": "bar-new"}, + ExpireTime: ptr(now.Add(2 * time.Hour)), + }) + c.Assert(err, jc.ErrorIsNil) + + s.AssertChangeStreamIdle(c) + wC.AssertChange( + corewatcher.SecretTriggerChange{ + URI: uri1, + Revision: 1, + NextTriggerTime: now.Add(1 * time.Hour).UTC(), + }, + corewatcher.SecretTriggerChange{ + URI: uri2, + Revision: 2, + NextTriggerTime: now.Add(2 * time.Hour).UTC(), + }, + ) + + s.AssertChangeStreamIdle(c) + wC.AssertNoChange() + + // Pretend that the agent restarted and the watcher is re-created. + watcher1, err := svc.WatchSecretRevisionsExpiryChanges(context.Background(), + service.CharmSecretOwner{ + Kind: service.ApplicationOwner, + ID: "mysql", + }, + service.CharmSecretOwner{ + Kind: service.UnitOwner, + ID: "mediawiki/0", + }, + ) + 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)...) + s.AssertChangeStreamIdle(c) + wC1.AssertChange( + corewatcher.SecretTriggerChange{ + URI: uri1, + Revision: 1, + NextTriggerTime: now.Add(1 * time.Hour).UTC(), + }, + corewatcher.SecretTriggerChange{ + URI: uri2, + Revision: 2, + NextTriggerTime: now.Add(2 * time.Hour).UTC(), + }, + ) + s.AssertChangeStreamIdle(c) + wC1.AssertNoChange() +} From 23410b725d8dd92a31bd80b465726c6e98fd666a Mon Sep 17 00:00:00 2001 From: Kelvin Liu Date: Thu, 30 May 2024 19:27:21 +1000 Subject: [PATCH 3/3] Fix a bug(no expire set) in UpdateCharmSecret method; --- domain/secret/state/state.go | 90 +++++++++++++++++++----------- domain/secret/state/types.go | 7 ++- domain/secret/watcher_test.go | 101 +++++++++++++++------------------- 3 files changed, 106 insertions(+), 92 deletions(-) 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() }