[JUJU-3738] - Fix/backend ref count

apiserver/common/secrets/secrets.go

@@ -10,7 +10,6 @@ import (
 	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	"github.com/juju/names/v4"
-	"github.com/juju/utils/v3"
 
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/cloud"
@@ -355,8 +354,7 @@ func getSecretBackendInfo(statePool StatePool, backendState SecretsBackendState,
 		b   *coresecrets.SecretBackend
 		err error
 	)
-	// Check for external backends where the id is not a UUID.
-	if !utils.IsValidUUIDString(id) {
+	if !coresecrets.IsInternalSecretBackendID(id) {
 		b, err = backendState.GetSecretBackendByID(id)
 		if err != nil {
 			return nil, errors.Trace(err)

cmd/juju/secretbackends/list.go

@@ -12,12 +12,12 @@ import (
 	"github.com/juju/cmd/v3"
 	"github.com/juju/errors"
 	"github.com/juju/gnuflag"
-	"github.com/juju/utils/v3"
 
 	"github.com/juju/juju/api/client/secretbackends"
 	jujucmd "github.com/juju/juju/cmd"
 	"github.com/juju/juju/cmd/modelcmd"
 	"github.com/juju/juju/cmd/output"
+	"github.com/juju/juju/core/secrets"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/secrets/provider"
 )
@@ -146,7 +146,7 @@ func gatherSecretBackendInfo(backends []secretbackends.SecretBackend) map[string
 			info.Status = status.Error
 			info.Error = b.Error.Error()
 		}
-		if !utils.IsValidUUIDString(b.ID) {
+		if !secrets.IsInternalSecretBackendID(b.ID) {
 			info.ID = b.ID
 		}
 		if len(b.Config) > 0 {

core/secrets/secretbackend.go

@@ -8,8 +8,14 @@ import (
 	"time"
 
 	"github.com/juju/errors"
+	"github.com/juju/utils/v3"
 )
 
+// IsInternalSecretBackendID returns true if the supplied backend ID is the internal backend ID.
+func IsInternalSecretBackendID(backendID string) bool {
+	return utils.IsValidUUIDString(backendID)
+}
+
 // SecretBackend defines a secrets backend.
 type SecretBackend struct {
 	ID                  string

go.mod

@@ -53,7 +53,7 @@ require (
 	github.com/juju/clock v1.0.3
 	github.com/juju/cmd/v3 v3.0.4
 	github.com/juju/collections v1.0.2
-	github.com/juju/description/v4 v4.0.9
+	github.com/juju/description/v4 v4.0.10
 	github.com/juju/errors v1.0.0
 	github.com/juju/featureflag v1.0.0
 	github.com/juju/gnuflag v1.0.0

go.sum

@@ -768,8 +768,8 @@ github.com/juju/collections v0.0.0-20220203020748-febd7cad8a7a/go.mod h1:JWeZdyt
 github.com/juju/collections v1.0.0/go.mod h1:JWeZdyttIEbkR51z2S13+J+aCuHVe0F6meRy+P0YGDo=
 github.com/juju/collections v1.0.2 h1:y9t99Nq/uUZksJgWehiWxIr2vB1UG3hUT7LBNy1xiH8=
 github.com/juju/collections v1.0.2/go.mod h1:kYJowQZYtHDvYDfZOvgf3Mt7mjKYwm/k1nqnJoMYOUc=
-github.com/juju/description/v4 v4.0.9 h1:927hh4BtBJvYDj7y/+f7/rf315kQAkgV4QM4Fxv3KZ8=
-github.com/juju/description/v4 v4.0.9/go.mod h1:LRv+oC6zWwK+MpIEC3TCzRXjw5d75WK1HjcvNTWP+e8=
+github.com/juju/description/v4 v4.0.10 h1:7OOR9NJu0Q7fN6Yw0r+6cF9u4xTcCR1e695RJVfTuG4=
+github.com/juju/description/v4 v4.0.10/go.mod h1:LRv+oC6zWwK+MpIEC3TCzRXjw5d75WK1HjcvNTWP+e8=
 github.com/juju/errors v0.0.0-20150916125642-1b5e39b83d18/go.mod h1:W54LbzXuIE0boCoNJfwqpmkKJ1O4TCTZMetAt6jGk7Q=
 github.com/juju/errors v0.0.0-20200330140219-3fe23663418f/go.mod h1:W54LbzXuIE0boCoNJfwqpmkKJ1O4TCTZMetAt6jGk7Q=
 github.com/juju/errors v0.0.0-20210818161939-5560c4c073ff/go.mod h1:i1eL7XREII6aHpQ2gApI/v6FkVUDEBremNkcBCKYAcY=

state/migration_export.go

@@ -26,6 +26,7 @@ import (
 	"github.com/juju/juju/core/resources"
 	"github.com/juju/juju/core/secrets"
 	"github.com/juju/juju/feature"
+	secretsprovider "github.com/juju/juju/secrets/provider"
 	"github.com/juju/juju/state/migrations"
 	"github.com/juju/juju/storage/poolmanager"
 )
@@ -141,6 +142,9 @@ func (st *State) exportImpl(cfg ExportConfig, leaders map[string]string) (descri
 		EnvironVersion:     dbModel.EnvironVersion(),
 		Blocks:             blocks,
 	}
+	if args.SecretBackendID, err = export.secretBackendID(); err != nil {
+		return nil, errors.Trace(err)
+	}
 	export.model = description.NewModel(args)
 	if credsTag, credsSet := dbModel.CloudCredentialTag(); credsSet && !cfg.SkipCredentials {
 		creds, err := st.CloudCredential(credsTag)
@@ -1822,6 +1826,23 @@ func (e *exporter) operations() error {
 	return nil
 }
 
+func (e *exporter) secretBackendID() (string, error) {
+	mCfg, err := e.dbModel.Config()
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+	backendName := mCfg.SecretBackend()
+	if backendName == "" || backendName == secretsprovider.Auto || backendName == secretsprovider.Internal {
+		return "", nil
+	}
+	store := NewSecretBackends(e.st)
+	backend, err := store.GetSecretBackend(backendName)
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+	return backend.ID, nil
+}
+
 func (e *exporter) secrets() error {
 	if e.cfg.SkipSecrets {
 		return nil

state/migration_export_test.go

@@ -36,6 +36,7 @@ import (
 	"github.com/juju/juju/core/secrets"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/environs"
+	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/feature"
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/state/cloudimagemetadata"
@@ -2678,11 +2679,21 @@ func (s *MigrationExportSuite) TestRemoteRelationSettingsForUnitsInCMR(c *gc.C)
 
 func (s *MigrationExportSuite) TestSecrets(c *gc.C) {
 	store := state.NewSecrets(s.State)
+	backendStore := state.NewSecretBackends(s.State)
 	owner := s.Factory.MakeApplication(c, nil)
 	uri := secrets.NewURI()
 	createTime := time.Now().UTC().Round(time.Second)
 	next := createTime.Add(time.Minute).Round(time.Second).UTC()
 	expire := createTime.Add(2 * time.Hour).Round(time.Second).UTC()
+
+	backendID, err := backendStore.CreateSecretBackend(state.CreateSecretBackendParams{
+		Name:                "myvault",
+		BackendType:         "vault",
+		TokenRotateInterval: ptr(666 * time.Second),
+		NextRotateTime:      ptr(next),
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
 	p := state.CreateSecretParams{
 		Version: 1,
 		Owner:   owner.Tag(),
@@ -2699,14 +2710,20 @@ func (s *MigrationExportSuite) TestSecrets(c *gc.C) {
 	}
 	md, err := store.CreateSecret(uri, p)
 	c.Assert(err, jc.ErrorIsNil)
-	md, err = store.UpdateSecret(md.URI, state.UpdateSecretParams{
+
+	_, err = store.UpdateSecret(md.URI, state.UpdateSecretParams{
 		LeaderToken: &fakeToken{},
 		ValueRef: &secrets.ValueRef{
-			BackendID:  "backend-id",
+			BackendID:  backendID,
 			RevisionID: "rev-id",
 		},
 	})
 	c.Assert(err, jc.ErrorIsNil)
+
+	backendRefCount, err := s.State.ReadBackendRefCount(backendID)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(backendRefCount, gc.Equals, 1)
+
 	err = s.State.GrantSecretAccess(uri, state.SecretAccessParams{
 		LeaderToken: &fakeToken{},
 		Scope:       owner.Tag(),
@@ -2735,9 +2752,17 @@ func (s *MigrationExportSuite) TestSecrets(c *gc.C) {
 	})
 	c.Assert(err, jc.ErrorIsNil)
 
+	err = s.Model.UpdateModelConfig(map[string]interface{}{config.SecretBackendKey: "myvault"}, nil)
+	c.Assert(err, jc.ErrorIsNil)
+	mCfg, err := s.Model.ModelConfig()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(mCfg.SecretBackend(), jc.DeepEquals, "myvault")
+
 	model, err := s.State.Export(map[string]string{})
 	c.Assert(err, jc.ErrorIsNil)
 
+	c.Assert(model.SecretBackendID(), gc.Equals, backendID)
+
 	allSecrets := model.Secrets()
 	c.Assert(allSecrets, gc.HasLen, 1)
 	secret := allSecrets[0]
@@ -2755,7 +2780,7 @@ func (s *MigrationExportSuite) TestSecrets(c *gc.C) {
 	c.Assert(revisions[0].Content(), jc.DeepEquals, map[string]string{"foo": "bar"})
 	c.Assert(revisions[0].ExpireTime(), jc.DeepEquals, ptr(expire))
 	c.Assert(revisions[1].ValueRef(), gc.NotNil)
-	c.Assert(revisions[1].ValueRef().BackendID(), jc.DeepEquals, "backend-id")
+	c.Assert(revisions[1].ValueRef().BackendID(), jc.DeepEquals, backendID)
 	c.Assert(revisions[1].ValueRef().RevisionID(), jc.DeepEquals, "rev-id")
 	consumers := secret.Consumers()
 	c.Assert(consumers, gc.HasLen, 1)

state/migration_import.go

@@ -33,6 +33,7 @@ import (
 	"github.com/juju/juju/core/series"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/environs/config"
+	secretsprovider "github.com/juju/juju/secrets/provider"
 	"github.com/juju/juju/state/cloudimagemetadata"
 	"github.com/juju/juju/storage"
 	"github.com/juju/juju/storage/poolmanager"
@@ -217,6 +218,9 @@ func (ctrl *Controller) Import(model description.Model) (_ *Model, _ *State, err
 	if err := restore.storage(); err != nil {
 		return nil, nil, errors.Annotate(err, "storage")
 	}
+	if err := restore.secretBackend(); err != nil {
+		return nil, nil, errors.Annotate(err, "secret backend")
+	}
 	if err := restore.secrets(); err != nil {
 		return nil, nil, errors.Annotate(err, "secrets")
 	}
@@ -2484,7 +2488,7 @@ func (i *importer) addVolume(volume description.Volume, sb *storageBackend) erro
 		ops = append(ops, i.addVolumeAttachmentOp(tag.Id(), attachment, attachment.VolumePlanInfo()))
 	}
 
-	if attachmentPlans != nil && len(attachmentPlans) > 0 {
+	if len(attachmentPlans) > 0 {
 		for _, val := range attachmentPlans {
 			ops = append(ops, i.addVolumeAttachmentPlanOp(tag.Id(), val))
 		}
@@ -2695,13 +2699,40 @@ func (i *importer) storagePools() error {
 	return nil
 }
 
+func (i *importer) secretBackend() error {
+	mCfg, err := i.dbModel.ModelConfig()
+	if err != nil {
+		return errors.Trace(err)
+	}
+	mSecretBackendName := mCfg.SecretBackend()
+	if mSecretBackendName == "" || mSecretBackendName == secretsprovider.Auto || mSecretBackendName == secretsprovider.Internal {
+		return nil
+	}
+
+	backendID := i.model.SecretBackendID()
+	if backendID == "" {
+		// We reject if no backend ID is set, because we don't want to accidentally drain secrets to the wrong backend.
+		// So we suggest to upgrade the source controller if no backend ID in the exported data(because the source model is too old).
+		return errors.NotFoundf("secret backend config %q in model export", mSecretBackendName)
+	}
+	i.logger.Debugf("importing secret backend")
+	backends := NewSecretBackends(i.st)
+	mBackend, err := backends.GetSecretBackendByID(backendID)
+	if err != nil {
+		return errors.Annotatef(err, "cannot load secret backend %q", backendID)
+	}
+	err = i.dbModel.UpdateModelConfig(map[string]interface{}{config.SecretBackendKey: mBackend.Name}, nil)
+	return errors.Trace(err)
+}
+
 func (i *importer) secrets() error {
 	i.logger.Debugf("importing secrets")
 	backends := NewSecretBackends(i.st)
 	allBackends, err := backends.ListSecretBackends()
 	if err != nil {
 		return errors.Annotate(err, "loading secret backends")
 	}
+
 	knownBackends := set.NewStrings()
 	for _, b := range allBackends {
 		knownBackends.Add(b.ID)
@@ -2714,7 +2745,7 @@ func (i *importer) secrets() error {
 	}
 	migration.Add(func() error {
 		m := ImportSecrets{}
-		return m.Execute(&secretConsumersStateShim{
+		return m.Execute(&secretStateShim{
 			stateModelNamspaceShim: stateModelNamspaceShim{
 				Model: migration.src,
 				st:    i.st,
@@ -2736,7 +2767,7 @@ func (i *importer) remoteSecrets() error {
 	}
 	migration.Add(func() error {
 		m := ImportRemoteSecrets{}
-		return m.Execute(&secretConsumersStateShim{
+		return m.Execute(&secretStateShim{
 			stateModelNamspaceShim: stateModelNamspaceShim{
 				Model: migration.src,
 				st:    i.st,

state/migration_import_tasks.go

@@ -5,12 +5,10 @@ package state
 
 import (
 	"github.com/juju/collections/set"
+	"github.com/juju/description/v4"
 	"github.com/juju/errors"
 	"github.com/juju/mgo/v3/txn"
 	"github.com/juju/names/v4"
-	"github.com/juju/utils/v3"
-
-	"github.com/juju/description/v4"
 
 	"github.com/juju/juju/core/crossmodel"
 	"github.com/juju/juju/core/permission"
@@ -529,21 +527,31 @@ type SecretConsumersState interface {
 	SecretConsumerKey(uri *secrets.URI, subject string) string
 }
 
+// BackendRevisionCountProcesser is used to create a backend revision reference count.
+type BackendRevisionCountProcesser interface {
+	IncBackendRevisionCountOps(backendID string) ([]txn.Op, error)
+}
+
 // SecretsInput describes the input used for migrating secrets.
 type SecretsInput interface {
 	DocModelNamespace
 	SecretConsumersState
+	BackendRevisionCountProcesser
 	SecretsDescription
 }
 
-type secretConsumersStateShim struct {
+type secretStateShim struct {
 	stateModelNamspaceShim
 }
 
-func (s *secretConsumersStateShim) SecretConsumerKey(uri *secrets.URI, subject string) string {
+func (s *secretStateShim) SecretConsumerKey(uri *secrets.URI, subject string) string {
 	return s.st.secretConsumerKey(uri, subject)
 }
 
+func (s *secretStateShim) IncBackendRevisionCountOps(backendID string) ([]txn.Op, error) {
+	return s.st.incBackendRevisionCountOps(backendID, 1)
+}
+
 // ImportSecrets describes a way to import secrets from a
 // description.
 type ImportSecrets struct{}
@@ -606,7 +614,7 @@ func (ImportSecrets) Execute(src SecretsInput, runner TransactionRunner, knownSe
 					BackendID:  rev.ValueRef().BackendID(),
 					RevisionID: rev.ValueRef().RevisionID(),
 				}
-				if !utils.IsValidUUIDString(valueRef.BackendID) && !seenBackendIds.Contains(valueRef.BackendID) {
+				if !secrets.IsInternalSecretBackendID(valueRef.BackendID) && !seenBackendIds.Contains(valueRef.BackendID) {
 					if !knownSecretBackends.Contains(valueRef.BackendID) {
 						return errors.New("target controller does not have all required secret backends set up")
 					}
@@ -635,6 +643,13 @@ func (ImportSecrets) Execute(src SecretsInput, runner TransactionRunner, knownSe
 					OwnerTag:      owner.String(),
 				},
 			})
+			if valueRef != nil {
+				refOps, err := src.IncBackendRevisionCountOps(valueRef.BackendID)
+				if err != nil {
+					return errors.Trace(err)
+				}
+				ops = append(ops, refOps...)
+			}
 		}
 		for subject, access := range secret.ACL() {
 			key := src.SecretConsumerKey(uri, subject)