Fixes and Improvements with Vault KV Client

apis/common/v1/connection_details.go

@@ -18,7 +18,6 @@ package v1
 
 import (
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 const (
@@ -68,11 +67,11 @@ type ConnectionSecretMetadata struct {
 }
 
 // SetOwnerUID sets owner object uid label.
-func (in *ConnectionSecretMetadata) SetOwnerUID(so metav1.Object) {
+func (in *ConnectionSecretMetadata) SetOwnerUID(uid string) {
 	if in.Labels == nil {
 		in.Labels = map[string]string{}
 	}
-	in.Labels[LabelKeyOwnerUID] = string(so.GetUID())
+	in.Labels[LabelKeyOwnerUID] = uid
 }
 
 // GetOwnerUID gets owner object uid.

pkg/connection/manager.go

@@ -103,20 +103,7 @@ func (m *DetailsManager) PublishConnection(ctx context.Context, so resource.Conn
 		return false, errors.Wrap(err, errConnectStore)
 	}
 
-	if p.Metadata == nil {
-		p.Metadata = &v1.ConnectionSecretMetadata{}
-	}
-	p.Metadata.SetOwnerUID(so)
-
-	changed, err := ss.WriteKeyValues(ctx, &store.Secret{
-		ScopedName: store.ScopedName{
-			Name:  p.Name,
-			Scope: so.GetNamespace(),
-		},
-		Metadata: p.Metadata,
-		Data:     store.KeyValues(conn),
-	}, SecretToWriteMustBeOwnedBy(so))
-
+	changed, err := ss.WriteKeyValues(ctx, store.NewSecret(so, store.KeyValues(conn)), SecretToWriteMustBeOwnedBy(so))
 	return changed, errors.Wrap(err, errWriteStore)
 }
 
@@ -134,16 +121,7 @@ func (m *DetailsManager) UnpublishConnection(ctx context.Context, so resource.Co
 		return errors.Wrap(err, errConnectStore)
 	}
 
-	err = ss.DeleteKeyValues(ctx, &store.Secret{
-		ScopedName: store.ScopedName{
-			Name:  p.Name,
-			Scope: so.GetNamespace(),
-		},
-		Metadata: p.Metadata,
-		Data:     store.KeyValues(conn),
-	}, SecretToDeleteMustBeOwnedBy(so))
-
-	return errors.Wrap(err, errDeleteFromStore)
+	return errors.Wrap(ss.DeleteKeyValues(ctx, store.NewSecret(so, store.KeyValues(conn)), SecretToDeleteMustBeOwnedBy(so)), errDeleteFromStore)
 }
 
 // FetchConnection fetches connection details of a given ConnectionSecretOwner.
@@ -164,7 +142,12 @@ func (m *DetailsManager) FetchConnection(ctx context.Context, so resource.Connec
 }
 
 // PropagateConnection propagate connection details from one resource to another.
-func (m *DetailsManager) PropagateConnection(ctx context.Context, to resource.LocalConnectionSecretOwner, from resource.ConnectionSecretOwner) (propagated bool, err error) {
+func (m *DetailsManager) PropagateConnection(ctx context.Context, to resource.LocalConnectionSecretOwner, from resource.ConnectionSecretOwner) (propagated bool, err error) { // nolint:interfacer
+	// NOTE(turkenh): Had to add linter exception for "interfacer" suggestion
+	// to use "store.SecretOwner" as the type of "to" parameter. We want to
+	// keep it as "resource.LocalConnectionSecretOwner" to satisfy the
+	// ConnectionPropagater interface for XR Claims.
+
 	// Either from does not expose a connection secret, or to does not want one.
 	if from.GetPublishConnectionDetailsTo() == nil || to.GetPublishConnectionDetailsTo() == nil {
 		return false, nil
@@ -186,7 +169,7 @@ func (m *DetailsManager) PropagateConnection(ctx context.Context, to resource.Lo
 	// Make sure 'from' is the controller of the connection secret it references
 	// before we propagate it. This ensures a resource cannot use Crossplane to
 	// circumvent RBAC by propagating a secret it does not own.
-	if m := sFrom.Metadata; m == nil || m.GetOwnerUID() != string(from.GetUID()) {
+	if sFrom.GetOwner() != string(from.GetUID()) {
 		return false, errors.New(errSecretConflict)
 	}
 
@@ -195,20 +178,7 @@ func (m *DetailsManager) PropagateConnection(ctx context.Context, to resource.Lo
 		return false, errors.Wrap(err, errConnectStore)
 	}
 
-	toMeta := to.GetPublishConnectionDetailsTo().Metadata
-	if toMeta == nil {
-		toMeta = &v1.ConnectionSecretMetadata{}
-	}
-	toMeta.SetOwnerUID(to)
-	changed, err := ssTo.WriteKeyValues(ctx, &store.Secret{
-		ScopedName: store.ScopedName{
-			Name:  to.GetPublishConnectionDetailsTo().Name,
-			Scope: to.GetNamespace(),
-		},
-		Metadata: toMeta,
-		Data:     sFrom.Data,
-	}, SecretToWriteMustBeOwnedBy(to))
-
+	changed, err := ssTo.WriteKeyValues(ctx, store.NewSecret(to, sFrom.Data), SecretToWriteMustBeOwnedBy(to))
 	return changed, errors.Wrap(err, errWriteStore)
 }
 

pkg/connection/store/store.go

@@ -20,8 +20,16 @@ import (
 	"context"
 
 	v1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
+	"github.com/crossplane/crossplane-runtime/pkg/resource"
 )
 
+// SecretOwner owns a Secret.
+type SecretOwner interface {
+	resource.Object
+
+	resource.ConnectionDetailsPublisherTo
+}
+
 // KeyValues is a map with sensitive values.
 type KeyValues map[string][]byte
 
@@ -38,6 +46,43 @@ type Secret struct {
 	Data     KeyValues
 }
 
+// NewSecret returns a new Secret owned by supplied SecretOwner and with
+// supplied data.
+func NewSecret(so SecretOwner, data KeyValues) *Secret {
+	if so.GetPublishConnectionDetailsTo() == nil {
+		return nil
+	}
+	p := so.GetPublishConnectionDetailsTo()
+	if p.Metadata == nil {
+		p.Metadata = &v1.ConnectionSecretMetadata{}
+	}
+	p.Metadata.SetOwnerUID(string(so.GetUID()))
+	return &Secret{
+		ScopedName: ScopedName{
+			Name:  p.Name,
+			Scope: so.GetNamespace(),
+		},
+		Metadata: p.Metadata,
+		Data:     data,
+	}
+}
+
+// GetOwner returns the UID of the owner of secret.
+func (s *Secret) GetOwner() string {
+	if s.Metadata == nil {
+		return ""
+	}
+	return s.Metadata.GetOwnerUID()
+}
+
+// GetLabels returns the labels of the secret.
+func (s *Secret) GetLabels() map[string]string {
+	if s.Metadata == nil {
+		return nil
+	}
+	return s.Metadata.Labels
+}
+
 // A WriteOption is called before writing the desired secret over the
 // current object.
 type WriteOption func(ctx context.Context, current, desired *Secret) error