diff --git a/apiserver/facades/agent/secretsmanager/secrets.go b/apiserver/facades/agent/secretsmanager/secrets.go index d3cfec9829ed..a2a9fbe6a3cb 100644 --- a/apiserver/facades/agent/secretsmanager/secrets.go +++ b/apiserver/facades/agent/secretsmanager/secrets.go @@ -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 @@ -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 diff --git a/apiserver/facades/agent/secretsmanager/secrets_test.go b/apiserver/facades/agent/secretsmanager/secrets_test.go index 81585de1d14d..b16d8b999db1 100644 --- a/apiserver/facades/agent/secretsmanager/secrets_test.go +++ b/apiserver/facades/agent/secretsmanager/secrets_test.go @@ -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: ¶ms.Error{Message: `secret with label "foobar" already exists`, Code: params.CodeAlreadyExists}, + }}, + }) +} + func (s *SecretsManagerSuite) TestUpdateSecrets(c *gc.C) { defer s.setup(c).Finish() @@ -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: ¶ms.Error{Message: `secret with label "foobar" already exists`, Code: params.CodeAlreadyExists}, + }}, + }) +} + func (s *SecretsManagerSuite) TestRemoveSecrets(c *gc.C) { defer s.setup(c).Finish() diff --git a/state/allcollections.go b/state/allcollections.go index 264a0db90c4d..696bc4ba0193 100644 --- a/state/allcollections.go +++ b/state/allcollections.go @@ -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"}, }}, }, diff --git a/state/application.go b/state/application.go index 0ac67720dc00..3203fd123d8a 100644 --- a/state/application.go +++ b/state/application.go @@ -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 @@ -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}, @@ -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 { diff --git a/state/application_test.go b/state/application_test.go index bb493c3e9fad..b6960dfd4c1a 100644 --- a/state/application_test.go +++ b/state/application_test.go @@ -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"}, }, } @@ -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) { diff --git a/state/secrets.go b/state/secrets.go index 2a561f5dd396..cf33e8c0bc7b 100644 --- a/state/secrets.go +++ b/state/secrets.go @@ -23,6 +23,9 @@ import ( "github.com/juju/juju/state/watcher" ) +// LabelExists is returned when a duplicate label is used. +const LabelExists = errors.ConstError("label exists") + // CreateSecretParams are used to create a secret. type CreateSecretParams struct { UpdateSecretParams @@ -84,7 +87,7 @@ type secretMetadataDoc struct { ProviderID string `bson:"provider-id"` Description string `bson:"description"` - Label string `bson:"tags"` + Label string `bson:"label"` // LatestRevision is denormalised here - it is the // revision of the latest revision doc, @@ -226,12 +229,20 @@ func (s *secretsStore) CreateSecret(uri *secrets.URI, p CreateSecretParams) (*se } buildTxn := func(attempt int) ([]txn.Op, error) { + var ops []txn.Op + if p.Label != nil { + uniqueLabelOps, err := s.st.uniqueSecretLabelOps(metadataDoc.OwnerTag, *p.Label) + if err != nil { + return nil, errors.Trace(err) + } + ops = append(ops, uniqueLabelOps...) + } if attempt > 0 { if _, err := s.getSecretValue(uri, revision, false); err == nil { return nil, errors.AlreadyExistsf("secret value for %q", uri.String()) } } - ops := []txn.Op{ + ops = append(ops, []txn.Op{ { C: secretMetadataC, Id: metadataDoc.DocID, @@ -243,7 +254,7 @@ func (s *secretsStore) CreateSecret(uri *secrets.URI, p CreateSecretParams) (*se Assert: txn.DocMissing, Insert: *valueDoc, }, isOwnerAliveOp, - } + }...) if p.NextRotateTime != nil { rotateOps, err := s.secretRotationOps(uri, metadataDoc.OwnerTag, p.RotatePolicy, p.NextRotateTime) if err != nil { @@ -290,25 +301,33 @@ func (s *secretsStore) UpdateSecret(uri *secrets.URI, p UpdateSecretParams) (*se var metadataDoc secretMetadataDoc buildTxn := func(attempt int) ([]txn.Op, error) { err := secretMetadataCollection.FindId(uri.ID).One(&metadataDoc) - if errors.Cause(err) == mgo.ErrNotFound { + if err == mgo.ErrNotFound { return nil, errors.NotFoundf("secret %q", uri.String()) } if err != nil { return nil, errors.Trace(err) } + var ops []txn.Op + if p.Label != nil && *p.Label != metadataDoc.Label { + uniqueLabelOps, err := s.st.uniqueSecretLabelOps(metadataDoc.OwnerTag, *p.Label) + if err != nil { + return nil, errors.Trace(err) + } + ops = append(ops, uniqueLabelOps...) + } if err := s.updateSecretMetadataDoc(&metadataDoc, &p); err != nil { if err != nil { return nil, errors.Trace(err) } } - ops := []txn.Op{ + ops = append(ops, []txn.Op{ { C: secretMetadataC, Id: metadataDoc.DocID, Assert: txn.DocExists, Update: bson.M{"$set": metadataDoc}, }, - } + }...) _, err = s.getSecretValue(uri, metadataDoc.LatestRevision, false) revisionExists := err == nil if !revisionExists && !errors.IsNotFound(err) { @@ -316,7 +335,7 @@ func (s *secretsStore) UpdateSecret(uri *secrets.URI, p UpdateSecretParams) (*se } if len(p.Data) > 0 { if revisionExists { - return nil, errors.AlreadyExistsf("secret value for %q", uri.String()) + return nil, errors.AlreadyExistsf("secret value with revision %d for %q", metadataDoc.LatestRevision, uri.String()) } revisionDoc := s.secretRevisionDoc(uri, metadataDoc.LatestRevision, p.ExpireTime, p.Data) ops = append(ops, txn.Op{ @@ -380,7 +399,7 @@ func (st *State) nextRotateTime(docID string) (*time.Time, error) { } func (s *secretsStore) toSecretMetadata(doc *secretMetadataDoc, nextRotateTime *time.Time) (*secrets.SecretMetadata, error) { - uri, err := secrets.ParseURI(doc.DocID) + uri, err := secrets.ParseURI(s.st.localID(doc.DocID)) if err != nil { return nil, errors.Trace(err) } @@ -422,7 +441,7 @@ func (s *secretsStore) DeleteSecret(uri *secrets.URI) (err error) { logger.Warningf("aborting failed delete select transaction: %v", err2) } }() - secretMetadataCollection, closer := s.st.db().GetRawCollection(secretMetadataC) + secretMetadataCollection, closer := s.st.db().GetCollection(secretMetadataC) defer closer() // We need to read something for the txn to work with RemoveAll. var v interface{} @@ -433,52 +452,52 @@ func (s *secretsStore) DeleteSecret(uri *secrets.URI) (err error) { if err != nil { return errors.Trace(err) } - _, err = secretMetadataCollection.RemoveAll(bson.D{{ + _, err = secretMetadataCollection.Writeable().RemoveAll(bson.D{{ "_id", uri.ID, }}) if err != nil { return errors.Annotatef(err, "deleting revisions for %s", uri.ShortString()) } - secretRotateCollection, closer := s.st.db().GetRawCollection(secretRotateC) + secretRotateCollection, closer := s.st.db().GetCollection(secretRotateC) defer closer() - _, err = secretRotateCollection.RemoveAll(bson.D{{ + _, err = secretRotateCollection.Writeable().RemoveAll(bson.D{{ "_id", uri.ID, }}) if err != nil { return errors.Annotatef(err, "deleting revisions for %s", uri.ShortString()) } - secretRevisionsCollection, closer := s.st.db().GetRawCollection(secretRevisionsC) + secretRevisionsCollection, closer := s.st.db().GetCollection(secretRevisionsC) defer closer() - _, err = secretRevisionsCollection.RemoveAll(bson.D{{ - "_id", bson.D{{"$regex", fmt.Sprintf("^%s/.*", uri.ID)}}, + _, err = secretRevisionsCollection.Writeable().RemoveAll(bson.D{{ + "_id", bson.D{{"$regex", fmt.Sprintf("%s/.*", uri.ID)}}, }}) if err != nil { return errors.Annotatef(err, "deleting revisions for %s", uri.ShortString()) } - secretPermissionsCollection, closer := s.st.db().GetRawCollection(secretPermissionsC) + secretPermissionsCollection, closer := s.st.db().GetCollection(secretPermissionsC) defer closer() - _, err = secretPermissionsCollection.RemoveAll(bson.D{{ - "_id", bson.D{{"$regex", fmt.Sprintf("^%s#.*", s.st.docID(uri.ID))}}, + _, err = secretPermissionsCollection.Writeable().RemoveAll(bson.D{{ + "_id", bson.D{{"$regex", fmt.Sprintf("%s#.*", uri.ID)}}, }}) if err != nil { return errors.Annotatef(err, "deleting permissions for %s", uri.ShortString()) } - secretConsumersCollection, closer := s.st.db().GetRawCollection(secretConsumersC) + secretConsumersCollection, closer := s.st.db().GetCollection(secretConsumersC) defer closer() - _, err = secretConsumersCollection.RemoveAll(bson.D{{ - "_id", bson.D{{"$regex", fmt.Sprintf("^%s#.*", s.st.docID(uri.ID))}}, + _, err = secretConsumersCollection.Writeable().RemoveAll(bson.D{{ + "_id", bson.D{{"$regex", fmt.Sprintf("%s#.*", uri.ID)}}, }}) if err != nil { return errors.Annotatef(err, "deleting consumer info for %s", uri.ShortString()) } - refCountsCollection, closer := s.st.db().GetRawCollection(refcountsC) + refCountsCollection, closer := s.st.db().GetCollection(refcountsC) defer closer() - _, err = refCountsCollection.RemoveAll(bson.D{{ - "_id", fmt.Sprintf("%s#%s", s.st.docID(uri.ID), "consumer"), + _, err = refCountsCollection.Writeable().RemoveAll(bson.D{{ + "_id", fmt.Sprintf("%s#%s", uri.ID, "consumer"), }}) if err != nil { return errors.Annotatef(err, "deleting consumer info for %s", uri.ShortString()) @@ -594,8 +613,7 @@ func (s *secretsStore) listSecretRevisions(uri *secrets.URI, revision *int) ([]* ) if revision == nil { - id := "^" + uri.ID + "/.*" - q = bson.D{{"_id", bson.D{{"$regex", id}}}} + q = bson.D{{"_id", bson.D{{"$regex", uri.ID + "/.*"}}}} } else { q = bson.D{{"_id", secretRevisionKey(uri, *revision)}} } @@ -648,10 +666,57 @@ func (st *State) checkConsumerCountOps(uri *secrets.URI, inc int) ([]txn.Op, err if err != nil { return nil, errors.Trace(err) } - return []txn.Op{countOp, incOp}, err + return []txn.Op{countOp, incOp}, nil } incOp := nsRefcounts.JustIncRefOp(refcountsC, key, inc) - return []txn.Op{countOp, incOp}, err + return []txn.Op{countOp, incOp}, nil +} + +func secretOwnerLabelKey(ownerTag string, label string) string { + return fmt.Sprintf("secretlabel#%s#%s", ownerTag, label) +} + +func (st *State) uniqueSecretLabelOps(ownerTag string, label string) ([]txn.Op, error) { + refCountCollection, ccloser := st.db().GetCollection(refcountsC) + defer ccloser() + + key := secretOwnerLabelKey(ownerTag, label) + countOp, count, err := nsRefcounts.CurrentOp(refCountCollection, key) + if err != nil { + return nil, errors.Trace(err) + } + if count > 0 { + return nil, errors.WithType( + errors.Errorf("secret label %q owned by %s already exists", label, ownerTag), LabelExists) + } + incOp, err := nsRefcounts.CreateOrIncRefOp(refCountCollection, key, 1) + if err != nil { + return nil, errors.Trace(err) + } + return []txn.Op{countOp, incOp}, nil +} + +func (st *State) removeOwnerSecretLabelOps(ownerTag names.Tag) ([]txn.Op, error) { + refCountsCollection, closer := st.db().GetCollection(refcountsC) + defer closer() + + var ( + doc bson.M + ops []txn.Op + ) + id := secretOwnerLabelKey(ownerTag.String(), ".*") + q := bson.D{{"_id", bson.D{{"$regex", id}}}} + iter := refCountsCollection.Find(q).Iter() + for iter.Next(&doc) { + id, ok := doc["_id"].(string) + if !ok { + continue + } + count, _ := doc["refcount"].(int) + op := nsRefcounts.JustRemoveOp(refcountsC, id, count) + ops = append(ops, op) + } + return ops, iter.Close() } // GetSecretConsumer gets secret consumer metadata. @@ -664,9 +729,8 @@ func (st *State) GetSecretConsumer(uri *secrets.URI, consumerTag string) (*secre defer closer() key := secretConsumerKey(uri.ID, consumerTag) - docID := st.docID(key) var doc secretConsumerDoc - err := secretConsumersCollection.FindId(docID).One(&doc) + err := secretConsumersCollection.FindId(key).One(&doc) if errors.Cause(err) == mgo.ErrNotFound { return nil, errors.NotFoundf("consumer %q metadata for secret %q", consumerTag, uri.String()) } @@ -683,7 +747,6 @@ func (st *State) GetSecretConsumer(uri *secrets.URI, consumerTag string) (*secre // SaveSecretConsumer saves or updates secret consumer metadata. func (st *State) SaveSecretConsumer(uri *secrets.URI, consumerTag string, metadata *secrets.SecretConsumerMetadata) error { key := secretConsumerKey(uri.ID, consumerTag) - docID := st.docID(key) secretConsumersCollection, closer := st.db().GetCollection(secretConsumersC) defer closer() @@ -692,11 +755,11 @@ func (st *State) SaveSecretConsumer(uri *secrets.URI, consumerTag string, metada if err := st.checkExists(uri); err != nil { return nil, errors.Trace(err) } - err := secretConsumersCollection.FindId(docID).One(&doc) + err := secretConsumersCollection.FindId(key).One(&doc) if err == nil { return []txn.Op{{ C: secretConsumersC, - Id: docID, + Id: key, Assert: txn.DocExists, Update: bson.M{"$set": bson.M{ "label": metadata.Label, @@ -708,10 +771,10 @@ func (st *State) SaveSecretConsumer(uri *secrets.URI, consumerTag string, metada } ops := []txn.Op{{ C: secretConsumersC, - Id: docID, + Id: key, Assert: txn.DocMissing, Insert: secretConsumerDoc{ - DocID: docID, + DocID: key, ConsumerTag: consumerTag, Label: metadata.Label, CurrentRevision: metadata.CurrentRevision, @@ -741,7 +804,7 @@ func (st *State) secretUpdateConsumersOps(uri *secrets.URI, newRevision int) ([] doc secretConsumerDoc ops []txn.Op ) - id := st.docID(secretConsumerKey(uri.ID, ".*")) + id := secretConsumerKey(uri.ID, ".*") q := bson.D{{"_id", bson.D{{"$regex", id}}}} iter := secretConsumersCollection.Find(q).Iter() for iter.Next(&doc) { @@ -826,7 +889,7 @@ func (st *State) referencedSecrets(ref names.Tag, attr string) ([]*secrets.URI, ) iter := secretMetadataCollection.Find(bson.D{{attr, ref.String()}}).Select(bson.D{{"_id", 1}}).Iter() for iter.Next(&doc) { - uri, err := secrets.ParseURI(doc.DocID) + uri, err := secrets.ParseURI(st.localID(doc.DocID)) if err != nil { _ = iter.Close() return nil, errors.Trace(err) @@ -865,7 +928,6 @@ func (st *State) GrantSecretAccess(uri *secrets.URI, p SecretAccessParams) (err } key := secretConsumerKey(uri.ID, p.Subject.String()) - docID := st.docID(key) secretPermissionsCollection, closer := st.db().GetCollection(secretPermissionsC) defer closer() @@ -875,14 +937,14 @@ func (st *State) GrantSecretAccess(uri *secrets.URI, p SecretAccessParams) (err if err := st.checkExists(uri); err != nil { return nil, errors.Trace(err) } - err = secretPermissionsCollection.FindId(docID).One(&doc) + err = secretPermissionsCollection.FindId(key).One(&doc) if err == mgo.ErrNotFound { return []txn.Op{{ C: secretPermissionsC, - Id: docID, + Id: key, Assert: txn.DocMissing, Insert: secretPermissionDoc{ - DocID: docID, + DocID: key, Subject: p.Subject.String(), Scope: p.Scope.String(), Role: string(p.Role), @@ -899,7 +961,7 @@ func (st *State) GrantSecretAccess(uri *secrets.URI, p SecretAccessParams) (err } return []txn.Op{{ C: secretPermissionsC, - Id: docID, + Id: key, Assert: txn.DocExists, Update: bson.M{"$set": bson.M{ "role": p.Role, @@ -912,7 +974,6 @@ func (st *State) GrantSecretAccess(uri *secrets.URI, p SecretAccessParams) (err // RevokeSecretAccess removes any secret access role for the subject. func (st *State) RevokeSecretAccess(uri *secrets.URI, p SecretAccessParams) error { key := secretConsumerKey(uri.ID, p.Subject.String()) - docID := st.docID(key) secretPermissionsCollection, closer := st.db().GetCollection(secretPermissionsC) defer closer() @@ -925,7 +986,7 @@ func (st *State) RevokeSecretAccess(uri *secrets.URI, p SecretAccessParams) erro } return nil, errors.Trace(err) } - err := secretPermissionsCollection.FindId(docID).One(&doc) + err := secretPermissionsCollection.FindId(key).One(&doc) if err == mgo.ErrNotFound { return nil, jujutxn.ErrNoOperations } else if err != nil { @@ -934,7 +995,7 @@ func (st *State) RevokeSecretAccess(uri *secrets.URI, p SecretAccessParams) erro ops := []txn.Op{{ C: secretPermissionsC, - Id: docID, + Id: key, Assert: txn.DocExists, Remove: true, }} @@ -946,7 +1007,6 @@ func (st *State) RevokeSecretAccess(uri *secrets.URI, p SecretAccessParams) erro // SecretAccess returns the secret access role for the subject. func (st *State) SecretAccess(uri *secrets.URI, subject names.Tag) (secrets.SecretRole, error) { key := secretConsumerKey(uri.ID, subject.String()) - docID := st.docID(key) secretPermissionsCollection, closer := st.db().GetCollection(secretPermissionsC) defer closer() @@ -956,7 +1016,7 @@ func (st *State) SecretAccess(uri *secrets.URI, subject names.Tag) (secrets.Secr } var doc secretPermissionDoc - err := secretPermissionsCollection.FindId(docID).One(&doc) + err := secretPermissionsCollection.FindId(key).One(&doc) if err == mgo.ErrNotFound { return secrets.RoleNone, nil } @@ -1124,7 +1184,7 @@ func (w *secretsRotationWatcher) initial() ([]corewatcher.SecretTriggerChange, e iter := secretRotateCollection.Find(bson.D{{"owner-tag", w.owner}}).Iter() for iter.Next(&doc) { - uriStr := doc.DocID + uriStr := w.backend.localID(doc.DocID) uri, err := secrets.ParseURI(uriStr) if err != nil { _ = iter.Close() @@ -1175,7 +1235,7 @@ func (w *secretsRotationWatcher) merge(details []corewatcher.SecretTriggerChange return details, nil } if doc.TxnRevno > knownDetails.txnRevNo { - uriStr := doc.DocID + uriStr := w.backend.localID(doc.DocID) uri, err := secrets.ParseURI(uriStr) if err != nil { return nil, errors.Annotatef(err, "invalid secret URI %q", uriStr) diff --git a/state/secrets_test.go b/state/secrets_test.go index 778c4f9fd3d0..d2b6bc8d3186 100644 --- a/state/secrets_test.go +++ b/state/secrets_test.go @@ -87,10 +87,37 @@ func (s *SecretsSuite) TestCreate(c *gc.C) { UpdateTime: now, }) + p.Label = nil _, err = s.store.CreateSecret(uri, p) c.Assert(err, jc.Satisfies, errors.IsAlreadyExists) } +func (s *SecretsSuite) TestCreateDuplicateLabel(c *gc.C) { + uri := secrets.NewURI() + uri.ControllerUUID = s.State.ControllerUUID() + now := s.Clock.Now().Round(time.Second).UTC() + p := state.CreateSecretParams{ + Version: 1, + Owner: s.owner.Tag().String(), + UpdateSecretParams: state.UpdateSecretParams{ + LeaderToken: &fakeToken{}, + RotatePolicy: ptr(secrets.RotateDaily), + NextRotateTime: ptr(now.Add(time.Minute)), + Description: ptr("my secret"), + Label: ptr("foobar"), + ExpireTime: ptr(now.Add(time.Hour)), + Params: nil, + Data: map[string]string{"foo": "bar"}, + }, + } + _, err := s.store.CreateSecret(uri, p) + c.Assert(err, jc.ErrorIsNil) + uri2 := secrets.NewURI() + uri2.ControllerUUID = s.State.ControllerUUID() + _, err = s.store.CreateSecret(uri2, p) + c.Assert(errors.Is(err, state.LabelExists), jc.IsTrue) +} + func (s *SecretsSuite) TestCreateDyingOwner(c *gc.C) { err := s.owner.Destroy() c.Assert(err, jc.ErrorIsNil) @@ -313,6 +340,39 @@ func (s *SecretsSuite) TestUpdateExpiry(c *gc.C) { }) } +func (s *SecretsSuite) TestUpdateDuplicateLabel(c *gc.C) { + uri := secrets.NewURI() + uri.ControllerUUID = s.State.ControllerUUID() + cp := state.CreateSecretParams{ + Version: 1, + Owner: s.owner.Tag().String(), + UpdateSecretParams: state.UpdateSecretParams{ + LeaderToken: &fakeToken{}, + Label: ptr("label"), + Description: ptr("description"), + Data: map[string]string{"foo": "bar"}, + }, + } + _, err := s.store.CreateSecret(uri, cp) + c.Assert(err, jc.ErrorIsNil) + uri2 := secrets.NewURI() + uri2.ControllerUUID = s.State.ControllerUUID() + cp.Label = ptr("label2") + _, err = s.store.CreateSecret(uri2, cp) + c.Assert(err, jc.ErrorIsNil) + _, err = s.store.UpdateSecret(uri, state.UpdateSecretParams{ + LeaderToken: &fakeToken{}, + Label: ptr("label2"), + }) + c.Assert(errors.Is(err, state.LabelExists), jc.IsTrue) + + _, err = s.store.UpdateSecret(uri, state.UpdateSecretParams{ + LeaderToken: &fakeToken{}, + Label: ptr("label"), + }) + c.Assert(err, jc.ErrorIsNil) +} + func (s *SecretsSuite) TestUpdateData(c *gc.C) { uri := secrets.NewURI() uri.ControllerUUID = s.State.ControllerUUID() @@ -858,7 +918,7 @@ func (s *SecretsSuite) TestDelete(c *gc.C) { c.Assert(err, jc.ErrorIsNil) // Check that other secret info remains intact. - secretRevisionsCollection, closer := state.GetRawCollection(s.State, "secretRevisions") + secretRevisionsCollection, closer := state.GetCollection(s.State, "secretRevisions") defer closer() n, err := secretRevisionsCollection.FindId(uri2.ID + "/1").Count() c.Assert(err, jc.ErrorIsNil) @@ -867,7 +927,7 @@ func (s *SecretsSuite) TestDelete(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 1) - secretRotateCollection, closer := state.GetRawCollection(s.State, "secretRotate") + secretRotateCollection, closer := state.GetCollection(s.State, "secretRotate") defer closer() n, err = secretRotateCollection.FindId(uri2.ID).Count() c.Assert(err, jc.ErrorIsNil) @@ -876,30 +936,30 @@ func (s *SecretsSuite) TestDelete(c *gc.C) { c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 1) - secretConsumersCollection, closer := state.GetRawCollection(s.State, "secretConsumers") + secretConsumersCollection, closer := state.GetCollection(s.State, "secretConsumers") defer closer() - n, err = secretConsumersCollection.FindId(state.DocID(s.State, uri2.ID) + "#unit-mariadb-0").Count() + n, err = secretConsumersCollection.FindId(uri2.ID + "#unit-mariadb-0").Count() c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 1) n, err = secretConsumersCollection.Find(nil).Count() c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 1) - secretPermissionsCollection, closer := state.GetRawCollection(s.State, "secretPermissions") + secretPermissionsCollection, closer := state.GetCollection(s.State, "secretPermissions") defer closer() - n, err = secretPermissionsCollection.FindId(state.DocID(s.State, uri2.ID) + "#application-wordpress").Count() + n, err = secretPermissionsCollection.FindId(uri2.ID + "#application-wordpress").Count() c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 1) n, err = secretPermissionsCollection.Find(nil).Count() c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 1) - refCountsCollection, closer := state.GetRawCollection(s.State, "refcounts") + refCountsCollection, closer := state.GetCollection(s.State, "refcounts") defer closer() - n, err = refCountsCollection.FindId(state.DocID(s.State, uri2.ID) + "#consumer").Count() + n, err = refCountsCollection.FindId(uri2.ID + "#consumer").Count() c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 1) - n, err = refCountsCollection.FindId(state.DocID(s.State, uri1.ID) + "#consumer").Count() + n, err = refCountsCollection.FindId(uri1.ID + "#consumer").Count() c.Assert(err, jc.ErrorIsNil) c.Assert(n, gc.Equals, 0) } diff --git a/worker/uniter/runner/context/context.go b/worker/uniter/runner/context/context.go index 533e3a9943a3..4ea2a461ddc3 100644 --- a/worker/uniter/runner/context/context.go +++ b/worker/uniter/runner/context/context.go @@ -1308,8 +1308,7 @@ func (ctx *HookContext) doFlush(process string) error { commitReq, numChanges := b.Build() if numChanges > 0 { if err := ctx.unit.CommitHookChanges(commitReq); err != nil { - err = errors.Annotatef(err, "cannot apply changes") - ctx.logger.Errorf("%v", err) + ctx.logger.Errorf("cannot apply changes: %v", err) return errors.Trace(err) } } diff --git a/worker/uniter/runner/jujuc/secret-get.go b/worker/uniter/runner/jujuc/secret-get.go index 61dbb65a966e..14b2fe2e3a55 100644 --- a/worker/uniter/runner/jujuc/secret-get.go +++ b/worker/uniter/runner/jujuc/secret-get.go @@ -4,6 +4,8 @@ package jujuc import ( + "time" + "github.com/juju/cmd/v3" "github.com/juju/errors" "github.com/juju/gnuflag" @@ -22,6 +24,8 @@ type secretGetCommand struct { key string peek bool update bool + + metadata bool } // NewSecretGetCommand returns a command to get a secret value. @@ -40,6 +44,8 @@ Using --peek will fetch the latest revision just this time. Using --update will fetch the latest revision and continue to return the same revision next time unless --peek or --update is used. +Secret owners can also fetch the metadata for the secret using --metadata. +Either the ID or label can be used to identify the secret. Examples secret-get secret:9m4e2mr0ui3e8a215n4g @@ -49,6 +55,9 @@ Examples secret-get secret:9m4e2mr0ui3e8a215n4g --peek secret-get secret:9m4e2mr0ui3e8a215n4g --update secret-get secret:9m4e2mr0ui3e8a215n4g --label db-password + + secret-get secret:9m4e2mr0ui3e8a215n4g --metadata label db-password + secret-get --metadata --label db-password ` return jujucmd.Info(&cmd.Info{ Name: "secret-get [key[#base64]]", @@ -69,29 +78,92 @@ func (c *secretGetCommand) SetFlags(f *gnuflag.FlagSet) { `get the latest revision just this time`) f.BoolVar(&c.update, "update", false, `get the latest revision and also get this same revision for subsequent calls`) + f.BoolVar(&c.metadata, "metadata", false, + `get just the secret metadata`) } // Init implements cmd.Command. func (c *secretGetCommand) Init(args []string) (err error) { - if len(args) < 1 { - return errors.New("missing secret URI") + if len(args) > 0 { + c.secretUri, err = secrets.ParseURI(args[0]) + if err != nil { + return errors.NotValidf("secret URI %q", args[0]) + } + args = args[1:] } - c.secretUri, err = secrets.ParseURI(args[0]) - if err != nil { - return errors.NotValidf("secret URI %q", args[0]) + if c.metadata { + if c.secretUri == nil && c.label == "" { + return errors.New("require either a secret URI or label to fetch metadata") + } + if c.secretUri != nil && c.label != "" { + return errors.New("specify either a secret URI or label but not both to fetch metadata") + } + if c.peek || c.update { + return errors.New("--peek and --update are not valid when fetching metadata") + } + return cmd.CheckEmpty(args) + } + if c.secretUri == nil { + return errors.New("missing secret URI") } if c.peek && c.update { return errors.New("specify one of --peek or --update but not both") } - if len(args) > 1 { - c.key = args[1] - return cmd.CheckEmpty(args[2:]) + if len(args) > 0 { + c.key = args[0] + return cmd.CheckEmpty(args[1:]) } - return cmd.CheckEmpty(args[1:]) + return cmd.CheckEmpty(args) +} + +type metadataDisplay struct { + LatestRevision int `yaml:"revision" json:"revision"` + Label string `yaml:"label" json:"label"` + Description string `yaml:"description,omitempty" json:"description,omitempty"` + RotatePolicy secrets.RotatePolicy `yaml:"rotation,omitempty" json:"rotation,omitempty"` + LatestExpireTime *time.Time `yaml:"expiry,omitempty" json:"expiry,omitempty"` + NextRotateTime *time.Time `yaml:"rotates,omitempty" json:"rotates,omitempty"` } // Run implements cmd.Command. func (c *secretGetCommand) Run(ctx *cmd.Context) error { + if c.metadata { + all, err := c.ctx.SecretMetadata() + if err != nil { + return err + } + var ( + md SecretMetadata + found bool + want, got string + ) + if c.secretUri != nil { + want = c.secretUri.ID + got = c.secretUri.ID + md, found = all[c.secretUri.ID] + } else { + want = c.label + for id, m := range all { + if m.Label == c.label { + found = true + md = m + got = id + } + } + } + if !found { + return errors.NotFoundf("secret %q", want) + } + return c.out.Write(ctx, map[string]metadataDisplay{ + got: { + LatestRevision: md.LatestRevision, + Label: md.Label, + Description: md.Description, + RotatePolicy: md.RotatePolicy, + LatestExpireTime: md.LatestExpireTime, + NextRotateTime: md.NextRotateTime, + }}) + } value, err := c.ctx.GetSecret(c.secretUri, c.label, c.update, c.peek) if err != nil { return err diff --git a/worker/uniter/runner/jujuc/secret-get_test.go b/worker/uniter/runner/jujuc/secret-get_test.go index a30eaea9176c..0fec281c272b 100644 --- a/worker/uniter/runner/jujuc/secret-get_test.go +++ b/worker/uniter/runner/jujuc/secret-get_test.go @@ -23,14 +23,34 @@ type SecretGetSuite struct { var _ = gc.Suite(&SecretGetSuite{}) func (s *SecretGetSuite) TestSecretGetInit(c *gc.C) { - hctx, _ := s.ContextSuite.NewHookContext() - com, err := jujuc.NewCommand(hctx, "secret-get") - c.Assert(err, jc.ErrorIsNil) - ctx := cmdtesting.Context(c) - code := cmd.Main(jujuc.NewJujucCommandWrappedForTest(com), ctx, []string{"secret:9m4e2mr0ui3e8a215n4g", "--peek", "--update"}) - c.Assert(code, gc.Equals, 2) - c.Assert(bufferString(ctx.Stderr), gc.Equals, "ERROR specify one of --peek or --update but not both\n") + for _, t := range []struct { + args []string + err string + }{{ + args: []string{"secret:9m4e2mr0ui3e8a215n4g", "--peek", "--update"}, + err: "ERROR specify one of --peek or --update but not both", + }, { + args: []string{"--metadata"}, + err: "ERROR require either a secret URI or label to fetch metadata", + }, { + args: []string{"secret:9m4e2mr0ui3e8a215n4g", "--label", "foo", "--metadata"}, + err: "ERROR specify either a secret URI or label but not both to fetch metadata", + }, { + args: []string{"secret:9m4e2mr0ui3e8a215n4g", "--metadata", "--update"}, + err: "ERROR --peek and --update are not valid when fetching metadata", + }, { + args: []string{"secret:9m4e2mr0ui3e8a215n4g", "--metadata", "--peek"}, + err: "ERROR --peek and --update are not valid when fetching metadata", + }} { + hctx, _ := s.ContextSuite.NewHookContext() + com, err := jujuc.NewCommand(hctx, "secret-get") + c.Assert(err, jc.ErrorIsNil) + ctx := cmdtesting.Context(c) + code := cmd.Main(jujuc.NewJujucCommandWrappedForTest(com), ctx, t.args) + c.Check(code, gc.Equals, 2) + c.Check(bufferString(ctx.Stderr), gc.Equals, t.err+"\n") + } } func (s *SecretGetSuite) TestSecretGetJson(c *gc.C) { @@ -151,3 +171,41 @@ func (s *SecretGetSuite) TestSecretGetKeyBase64(c *gc.C) { c.Assert(bufferString(ctx.Stderr), gc.Equals, "") c.Assert(bufferString(ctx.Stdout), gc.Equals, "Y2VydA==\n") } + +func (s *SecretGetSuite) TestSecretGetMetadata(c *gc.C) { + hctx, _ := s.ContextSuite.NewHookContext() + + com, err := jujuc.NewCommand(hctx, "secret-get") + c.Assert(err, jc.ErrorIsNil) + ctx := cmdtesting.Context(c) + code := cmd.Main(jujuc.NewJujucCommandWrappedForTest(com), ctx, []string{"secret:9m4e2mr0ui3e8a215n4g", "--metadata"}) + c.Assert(code, gc.Equals, 0) + + c.Assert(bufferString(ctx.Stderr), gc.Equals, "") + c.Assert(bufferString(ctx.Stdout), gc.Equals, ` +9m4e2mr0ui3e8a215n4g: + revision: 666 + label: label + description: description + rotation: hourly +`[1:]) +} + +func (s *SecretGetSuite) TestSecretGetMetadataByLabel(c *gc.C) { + hctx, _ := s.ContextSuite.NewHookContext() + + com, err := jujuc.NewCommand(hctx, "secret-get") + c.Assert(err, jc.ErrorIsNil) + ctx := cmdtesting.Context(c) + code := cmd.Main(jujuc.NewJujucCommandWrappedForTest(com), ctx, []string{"--metadata", "--label", "label"}) + c.Assert(code, gc.Equals, 0) + + c.Assert(bufferString(ctx.Stderr), gc.Equals, "") + c.Assert(bufferString(ctx.Stdout), gc.Equals, ` +9m4e2mr0ui3e8a215n4g: + revision: 666 + label: label + description: description + rotation: hourly +`[1:]) +}