Skip to content

Commit

Permalink
Add secret-get --metadata and enforce label uniqueness
Browse files Browse the repository at this point in the history
  • Loading branch information
wallyworld committed Aug 26, 2022
1 parent d137d8c commit 47bc306
Show file tree
Hide file tree
Showing 10 changed files with 424 additions and 85 deletions.
6 changes: 6 additions & 0 deletions apiserver/facades/agent/secretsmanager/secrets.go
Expand Up @@ -47,6 +47,9 @@ func (s *SecretsManagerAPI) CreateSecrets(args params.CreateSecretArgs) (params.
for i, arg := range args.Args {
ID, err := s.createSecret(ctx, arg)
result.Results[i].Result = ID
if errors.Is(err, state.LabelExists) {
err = errors.AlreadyExistsf("secret with label %q", *arg.Label)
}
result.Results[i].Error = apiservererrors.ServerError(err)
}
return result, nil
Expand Down Expand Up @@ -118,6 +121,9 @@ func (s *SecretsManagerAPI) UpdateSecrets(args params.UpdateSecretArgs) (params.
ctx := context.Background()
for i, arg := range args.Args {
err := s.updateSecret(ctx, arg)
if errors.Is(err, state.LabelExists) {
err = errors.AlreadyExistsf("secret with label %q", *arg.Label)
}
result.Results[i].Error = apiservererrors.ServerError(err)
}
return result, nil
Expand Down
71 changes: 71 additions & 0 deletions apiserver/facades/agent/secretsmanager/secrets_test.go
Expand Up @@ -176,6 +176,41 @@ func (s *SecretsManagerSuite) TestCreateSecrets(c *gc.C) {
})
}

func (s *SecretsManagerSuite) TestCreateSecretDuplicateLabel(c *gc.C) {
defer s.setup(c).Finish()

p := secrets.CreateParams{
Version: secrets.Version,
Owner: "application-mariadb",
UpsertParams: secrets.UpsertParams{
LeaderToken: s.token,
Label: ptr("foobar"),
Data: map[string]string{"foo": "bar"},
},
}
s.leadership.EXPECT().LeadershipCheck("mariadb", "mariadb/0").Return(s.token)
s.token.EXPECT().Check(0, nil).Return(nil)
s.secretsService.EXPECT().CreateSecret(gomock.Any(), gomock.Any(), p).Return(
nil, fmt.Errorf("dup label %w", state.LabelExists),
)

results, err := s.facade.CreateSecrets(params.CreateSecretArgs{
Args: []params.CreateSecretArg{{
OwnerTag: "application-mariadb",
UpsertSecretArg: params.UpsertSecretArg{
Label: ptr("foobar"),
Data: map[string]string{"foo": "bar"},
},
}},
})
c.Assert(err, jc.ErrorIsNil)
c.Assert(results, jc.DeepEquals, params.StringResults{
Results: []params.StringResult{{
Error: &params.Error{Message: `secret with label "foobar" already exists`, Code: params.CodeAlreadyExists},
}},
})
}

func (s *SecretsManagerSuite) TestUpdateSecrets(c *gc.C) {
defer s.setup(c).Finish()

Expand Down Expand Up @@ -235,6 +270,42 @@ func (s *SecretsManagerSuite) TestUpdateSecrets(c *gc.C) {
})
}

func (s *SecretsManagerSuite) TestUpdateSecretDuplicateLabel(c *gc.C) {
defer s.setup(c).Finish()

p := secrets.UpsertParams{
LeaderToken: s.token,
Label: ptr("foobar"),
}
uri := coresecrets.NewURI()
expectURI := *uri
expectURI.ControllerUUID = coretesting.ControllerTag.Id()
s.secretsService.EXPECT().GetSecret(gomock.Any(), &expectURI).Return(&coresecrets.SecretMetadata{}, nil)
s.secretsService.EXPECT().UpdateSecret(gomock.Any(), &expectURI, p).Return(
nil, fmt.Errorf("dup label %w", state.LabelExists),
)
s.leadership.EXPECT().LeadershipCheck("mariadb", "mariadb/0").Return(s.token)
s.token.EXPECT().Check(0, nil).Return(nil)
s.expectSecretAccessQuery(1)
uri1 := *uri
uri1.ControllerUUID = "deadbeef-1bad-500d-9000-4b1d0d061111"

results, err := s.facade.UpdateSecrets(params.UpdateSecretArgs{
Args: []params.UpdateSecretArg{{
URI: uri.ShortString(),
UpsertSecretArg: params.UpsertSecretArg{
Label: ptr("foobar"),
},
}},
})
c.Assert(err, jc.ErrorIsNil)
c.Assert(results, jc.DeepEquals, params.ErrorResults{
Results: []params.ErrorResult{{
Error: &params.Error{Message: `secret with label "foobar" already exists`, Code: params.CodeAlreadyExists},
}},
})
}

func (s *SecretsManagerSuite) TestRemoveSecrets(c *gc.C) {
defer s.setup(c).Finish()

Expand Down
14 changes: 7 additions & 7 deletions state/allcollections.go
Expand Up @@ -541,32 +541,32 @@ func allCollections() CollectionSchema {
cloudServicesC: {},

secretMetadataC: {
global: true,
indexes: []mgo.Index{{
Key: []string{"controller-uuid", "model-uuid", "_id"},
Key: []string{"model-uuid", "_id"},
}},
},

secretRevisionsC: {
global: true,
indexes: []mgo.Index{{
Key: []string{"revision", "_id"},
}},
},

secretConsumersC: {
indexes: []mgo.Index{{
Key: []string{"consumer-tag"},
Key: []string{"consumer-tag", "model-uuid"},
}},
},

secretPermissionsC: {
indexes: []mgo.Index{{
Key: []string{"subject-tag", "scope-tag"},
Key: []string{"subject-tag", "scope-tag", "model-uuid"},
}},
},

secretRotateC: {
global: true,
indexes: []mgo.Index{{
Key: []string{"owner-tag"},
Key: []string{"owner-tag", "model-uuid"},
}},
},

Expand Down
10 changes: 10 additions & 0 deletions state/application.go
Expand Up @@ -691,6 +691,11 @@ func (a *Application) removeOps(asserts bson.D, op *ForcedOperation) ([]txn.Op,
return nil, errors.Trace(err)
}
ops = append(ops, secretPermissionsOps...)
secretLabelOps, err := a.st.removeOwnerSecretLabelOps(a.ApplicationTag())
if err != nil {
return nil, errors.Trace(err)
}
ops = append(ops, secretLabelOps...)

// Note that appCharmDecRefOps might not catch the final decref
// when run in a transaction that decrefs more than once. So we
Expand Down Expand Up @@ -2717,6 +2722,10 @@ func (a *Application) removeUnitOps(u *Unit, asserts bson.D, op *ForcedOperation
if op.FatalError(err) {
return nil, errors.Trace(err)
}
secretLabelOps, err := a.st.removeOwnerSecretLabelOps(u.Tag())
if op.FatalError(err) {
return nil, errors.Trace(err)
}

observedFieldsMatch := bson.D{
{"charmurl", u.doc.CharmURL},
Expand All @@ -2743,6 +2752,7 @@ func (a *Application) removeUnitOps(u *Unit, asserts bson.D, op *ForcedOperation
ops = append(ops, resOps...)
ops = append(ops, hostOps...)
ops = append(ops, secretPermissionsOps...)
ops = append(ops, secretLabelOps...)

m, err := a.st.Model()
if err != nil {
Expand Down
7 changes: 5 additions & 2 deletions state/application_test.go
Expand Up @@ -3245,6 +3245,7 @@ func (s *ApplicationSuite) TestDestroyAlsoDeletesOwnedSecrets(c *gc.C) {
Owner: s.mysql.Tag().String(),
UpdateSecretParams: state.UpdateSecretParams{
LeaderToken: &fakeToken{},
Label: ptr("label"),
Data: map[string]string{"foo": "bar"},
},
}
Expand All @@ -3253,8 +3254,10 @@ func (s *ApplicationSuite) TestDestroyAlsoDeletesOwnedSecrets(c *gc.C) {

err = s.mysql.Destroy()
c.Assert(err, jc.ErrorIsNil)
_, err = store.GetSecret(uri)
c.Assert(err, jc.Satisfies, errors.IsNotFound)
// Create again, no label clash.
s.AddTestingApplication(c, "mysql", s.charm)
_, err = store.CreateSecret(uri, cp)
c.Assert(err, jc.ErrorIsNil)
}

func (s *ApplicationSuite) TestApplicationCleanupRemovesStorageConstraints(c *gc.C) {
Expand Down

0 comments on commit 47bc306

Please sign in to comment.