Skip to content

Commit

Permalink
Merge pull request juju#17354 from ycliuhw/implement-secret-rotated
Browse files Browse the repository at this point in the history
juju#17354

This PR implements a secret rotated service and state methods to ensure the next rotation time for the provided secret stored in the Dqlite database.
Driveby: 
1. fixed the missing next rotation time update in SecretUpdate call.
2. add UNIQUE INDEX for secret_backend_type.type.
3. align fields for secrets DDL.

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc
- [x] Comments saying why design decisions were made
- [x] Go unit tests, with comments saying what you're testing
- [ ] ~[Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing~
- [ ] ~[doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~

## QA steps

Because the WatchSecretsRotationChanges watcher has not been implemented yet, we can only test the rotation policy changes.

```
juju exec --unit dummy-source/0 "secret-add data=foo"
secret://2bfad23f-a54d-48ed-87ec-e2776e2ea0b4/cotiv4uc5e02l6npsdm0

juju exec --unit dummy-source/0 "secret-set cotiv4uc5e02l6npsdm0 --rotate hourly"
```

```
dqlite> select prdesc from secret_rotation
cotiv4uc5e02l6npsdm0|2024-05-08 07:57:02.922271014 +0000 UTC
dqlite> select srp.policy from secret_metadata sm inner join secret_rotate_policy srp on sm.rotate_policy_id = srp.id
hourly
```

## Documentation changes

No

## Links

**Jira card:** JUJU-5897
  • Loading branch information
jujubot committed May 15, 2024
2 parents ace8dbe + d6cf27d commit 06fd6aa
Show file tree
Hide file tree
Showing 9 changed files with 667 additions and 190 deletions.
264 changes: 132 additions & 132 deletions domain/schema/secret.go

Large diffs are not rendered by default.

49 changes: 49 additions & 0 deletions domain/secret/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package service
import (
"context"
"fmt"
"time"

"github.com/juju/clock"
"github.com/juju/errors"
Expand All @@ -17,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"
Expand Down Expand Up @@ -67,6 +69,9 @@ 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
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)
Expand Down Expand Up @@ -359,6 +364,15 @@ func (s *SecretService) UpdateCharmSecret(ctx context.Context, uri *secrets.URI,
}
rotatePolicy := domainsecret.MarshallRotatePolicy(params.RotatePolicy)
p.RotatePolicy = &rotatePolicy
if params.RotatePolicy.WillRotate() {
policy, err := s.st.GetRotatePolicy(ctx, uri)
if err != nil {
return errors.Trace(err)
}
if !policy.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 {
Expand Down Expand Up @@ -638,3 +652,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)
}

info, err := s.st.GetRotationExpiryInfo(ctx, uri)
if err != nil {
return errors.Trace(err)
}
if !info.RotatePolicy.WillRotate() {
s.logger.Debugf("secret %q was rotated but now is set to not rotate")
return nil
}
lastRotateTime := info.NextRotateTime
if lastRotateTime == nil {
now := s.clock.Now()
lastRotateTime = &now
}
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 := 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, info.LatestRevision)
}
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))
return s.st.SecretRotated(ctx, uri, nextRotateTime)
}
137 changes: 132 additions & 5 deletions domain/secret/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,18 +395,30 @@ 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)
s.state.EXPECT().GetSecretAccess(gomock.Any(), uri, domainsecret.AccessParams{
SubjectTypeID: domainsecret.SubjectUnit,
SubjectID: "mariadb/0",
}).Return("manage", nil)
s.state.EXPECT().UpdateSecret(gomock.Any(), uri, p).Return(nil)
s.state.EXPECT().GetRotatePolicy(gomock.Any(), uri).Return(
coresecrets.RotateNever, // 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{},
Expand Down Expand Up @@ -1072,6 +1084,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().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{
RotatePolicy: coresecrets.RotateHourly,
LatestRevision: 667,
}, nil)

err := s.service(c).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().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{
RotatePolicy: coresecrets.RotateHourly,
LatestRevision: 666,
}, nil)

err := s.service(c).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().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{
RotatePolicy: coresecrets.RotateHourly,
LatestExpireTime: ptr(time.Now().Add(50 * time.Minute)),
LatestRevision: 667,
}, nil)

err := s.service(c).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().GetRotationExpiryInfo(ctx, uri).Return(&domainsecret.RotationExpiryInfo{
RotatePolicy: coresecrets.RotateNever,
LatestRevision: 667,
}, nil)

err := s.service(c).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) {
Expand Down
117 changes: 117 additions & 0 deletions domain/secret/service/state_mock_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 06fd6aa

Please sign in to comment.