Merge pull request #3371 from yuwenma/secret-manager-wip

feat: SecretManagerSecret full test coverage & manual replication field
GoogleCloudPlatform · Dec 16, 2024 · ff59285 · ff59285
2 parents 2b39da1 + 792894d
commit ff59285
Show file tree

Hide file tree

Showing 41 changed files with 5,082 additions and 753 deletions.
diff --git a/apis/secretmanager/v1beta1/secret_types.go b/apis/secretmanager/v1beta1/secret_types.go
@@ -48,8 +48,7 @@ type SecretManagerSecretSpec struct {
 	//  This is always provided on output, regardless of what was sent on input.
 	ExpireTime *string `json:"expireTime,omitempty"`
 
-	// Input only. The TTL for the
-	//  [Secret][google.cloud.secretmanager.v1.Secret].
+	// Input only. A duration in seconds with up to nine fractional digits, ending with 's'. Example: "3.5s".
 	TTL *string `json:"ttl,omitempty"`
 
 	// Optional. Rotation policy attached to the

diff --git a/...v1_customresourcedefinition_secretmanagersecrets.secretmanager.cnrm.cloud.google.com.yaml b/...v1_customresourcedefinition_secretmanagersecrets.secretmanager.cnrm.cloud.google.com.yaml
@@ -315,7 +315,8 @@ spec:
                   type: object
                 type: array
               ttl:
-                description: Input only. The TTL for the [Secret][google.cloud.secretmanager.v1.Secret].
+                description: 'Input only. A duration in seconds with up to nine fractional
+                  digits, ending with ''s''. Example: "3.5s".'
                 type: string
               versionAliases:
                 additionalProperties:

diff --git a/dev/tasks/run-e2e b/dev/tasks/run-e2e
@@ -26,7 +26,7 @@ if [[ -z "${KUBEBUILDER_ASSETS:-}" ]]; then
 fi
 
 if [[ -z "${KCC_USE_DIRECT_RECONCILERS:-}" ]]; then
-  KCC_USE_DIRECT_RECONCILERS=ComputeForwardingRule,GKEHubFeatureMembership,SecretManagerSecret,SecretManagerSecretVersion
+  KCC_USE_DIRECT_RECONCILERS=ComputeForwardingRule,GKEHubFeatureMembership
 fi
 echo "Using direct controllers: $KCC_USE_DIRECT_RECONCILERS"
 export KCC_USE_DIRECT_RECONCILERS

diff --git a/mockgcp/mocksecretmanager/secrets.go b/mockgcp/mocksecretmanager/secrets.go
@@ -84,6 +84,13 @@ func (s *SecretsV1) populateDefaultsForSecret(ctx context.Context, obj *pb.Secre
 			return fmt.Errorf("Aliases cannot be assigned to versions that don't exist")
 		}
 	}
+	// TTL and ExpireTime are OneOf, but the GCP service always converts TTL to expireTime before storing the object.
+	if obj.GetTtl() != nil {
+		expirateTime := timestamppb.Now().AsTime().Add(obj.GetTtl().AsDuration())
+		obj.Expiration = &pb.Secret_ExpireTime{
+			ExpireTime: timestamppb.New(expirateTime),
+		}
+	}
 	return nil
 }
 
@@ -154,6 +161,10 @@ func (s *SecretsV1) UpdateSecret(ctx context.Context, req *pb.UpdateSecretReques
 			updated.Expiration = &pb.Secret_ExpireTime{
 				ExpireTime: req.Secret.GetExpireTime(),
 			}
+		case "ttl":
+			updated.Expiration = &pb.Secret_Ttl{
+				Ttl: req.Secret.GetTtl(),
+			}
 		case "expiration":
 			updated.Expiration = req.Secret.GetExpiration()
 		case "rotation.nextRotationTime":

diff --git a/pkg/controller/direct/secretmanager/secret_controller.go b/pkg/controller/direct/secretmanager/secret_controller.go
@@ -122,6 +122,18 @@ func normalizeExternal(ctx context.Context, reader client.Reader, src client.Obj
 				secret.Spec.Replication.LegacyAutomatic.CustomerManagedEncryption.KmsKeyRef = kmsKeyRef
 			}
 		}
+		if secret.Spec.Replication.UserManaged != nil {
+			for _, r := range secret.Spec.Replication.UserManaged.Replicas {
+				if r.CustomerManagedEncryption != nil {
+					kmsKeyRef := r.CustomerManagedEncryption.KmsKeyRef
+					kmsKeyRef, err := refs.ResolveKMSCryptoKeyRef(ctx, reader, src, kmsKeyRef)
+					if err != nil {
+						return err
+					}
+					r.CustomerManagedEncryption.KmsKeyRef = kmsKeyRef
+				}
+			}
+		}
 	}
 	if len(secret.Spec.TopicRefs) != 0 {
 		for _, topicRef := range secret.Spec.TopicRefs {
@@ -246,7 +258,10 @@ func (a *Adapter) Update(ctx context.Context, op *directbase.UpdateOperation) er
 	if err != nil {
 		return err
 	}
-
+	if paths.Has("ttl") {
+		paths = paths.Delete("ttl")
+		resource.Expiration = a.actual.Expiration
+	}
 	if len(paths) == 0 {
 		log.V(2).Info("no field needs update", "name", a.id)
 		return nil

diff --git a/pkg/controller/direct/secretmanager/secret_mapping.go b/pkg/controller/direct/secretmanager/secret_mapping.go
@@ -101,6 +101,9 @@ func SecretManagerSecretSpec_ToProto(mapCtx *direct.MapContext, in *krm.SecretMa
 	if oneof := direct.StringTimestamp_ToProto(mapCtx, in.ExpireTime); oneof != nil {
 		out.Expiration = &pb.Secret_ExpireTime{ExpireTime: oneof}
 	}
+	if oneof := direct.Duration_ToProto(mapCtx, in.TTL); oneof != nil {
+		out.Expiration = &pb.Secret_Ttl{Ttl: oneof}
+	}
 	// MISSING: Etag
 	out.Rotation = Rotation_ToProto(mapCtx, in.Rotation)
 	out.VersionAliases = MapStringString_ToMapStringInt64(mapCtx, in.VersionAliases)

diff --git a/...r/v1beta1/basicsecretmanagersecret/_generated_object_basicsecretmanagersecret.golden.yaml b/...r/v1beta1/basicsecretmanagersecret/_generated_object_basicsecretmanagersecret.golden.yaml
@@ -2,6 +2,7 @@ apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
 kind: SecretManagerSecret
 metadata:
   annotations:
+    alpha.cnrm.cloud.google.com/reconciler: direct
     cnrm.cloud.google.com/management-conflict-prevention-policy: none
     cnrm.cloud.google.com/project-id: ${projectId}
   finalizers:

diff --git a/...resourcefixture/testdata/basic/secretmanager/v1beta1/basicsecretmanagersecret/create.yaml b/...resourcefixture/testdata/basic/secretmanager/v1beta1/basicsecretmanagersecret/create.yaml
@@ -17,6 +17,7 @@ kind: SecretManagerSecret
 metadata:
   annotations:
     cnrm.cloud.google.com/project-id: ${projectId}
+    alpha.cnrm.cloud.google.com/reconciler: "direct"
   labels:
     label-one: value-one
   name: secretmanagersecret-${uniqueId}

diff --git a/...ted_export_fullsecretmanagersecret.golden → ...ullsecretmanagersecret-auto-direct.golden b/...ted_export_fullsecretmanagersecret.golden → ...ullsecretmanagersecret-auto-direct.golden
diff --git a/...bject_fullsecretmanagersecret.golden.yaml → ...cretmanagersecret-auto-direct.golden.yaml b/...bject_fullsecretmanagersecret.golden.yaml → ...cretmanagersecret-auto-direct.golden.yaml
@@ -2,6 +2,7 @@ apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
 kind: SecretManagerSecret
 metadata:
   annotations:
+    alpha.cnrm.cloud.google.com/reconciler: direct
     cnrm.cloud.google.com/management-conflict-prevention-policy: none
     cnrm.cloud.google.com/project-id: ${projectId}
   finalizers:

diff --git a/...v1beta1/fullsecretmanagersecret/_http.log → ...secretmanagersecret-auto-direct/_http.log b/...v1beta1/fullsecretmanagersecret/_http.log → ...secretmanagersecret-auto-direct/_http.log
@@ -692,7 +692,7 @@ x-goog-request-params: parent=projects%2F${projectId}
   "annotations": {
     "foo": "secretmanagersecret"
   },
-  "expireTime": "2025-10-02T15:01:23Z",
+  "expireTime": "2024-04-01T12:34:56.123456Z",
   "labels": {
     "cnrm-test": "true",
     "label-one": "value-one",
@@ -733,7 +733,7 @@ X-Xss-Protection: 0
   },
   "createTime": "2024-04-01T12:34:56.123456Z",
   "etag": "abcdef0123A=",
-  "expireTime": "2025-10-02T15:01:23Z",
+  "expireTime": "2024-04-01T12:34:56.123456Z",
   "labels": {
     "cnrm-test": "true",
     "label-one": "value-one",
@@ -782,7 +782,7 @@ X-Xss-Protection: 0
   },
   "createTime": "2024-04-01T12:34:56.123456Z",
   "etag": "abcdef0123A=",
-  "expireTime": "2025-10-02T15:01:23Z",
+  "expireTime": "2024-04-01T12:34:56.123456Z",
   "labels": {
     "cnrm-test": "true",
     "label-one": "value-one",
@@ -820,7 +820,7 @@ x-goog-request-params: secret.name=projects%2F${projectId}%2Fsecrets%2Fsecretman
     "foo": "secretmanagersecret"
   },
   "etag": "abcdef0123A=",
-  "expireTime": "2025-10-03T15:01:23Z",
+  "expireTime": "2024-04-01T12:34:56.123456Z",
   "labels": {
     "cnrm-test": "true",
     "label-one": "value-one",
@@ -864,7 +864,7 @@ X-Xss-Protection: 0
   },
   "createTime": "2024-04-01T12:34:56.123456Z",
   "etag": "abcdef0123A=",
-  "expireTime": "2025-10-03T15:01:23Z",
+  "expireTime": "2024-04-01T12:34:56.123456Z",
   "labels": {
     "cnrm-test": "true",
     "label-one": "value-one",
@@ -914,7 +914,7 @@ X-Xss-Protection: 0
   },
   "createTime": "2024-04-01T12:34:56.123456Z",
   "etag": "abcdef0123A=",
-  "expireTime": "2025-10-03T15:01:23Z",
+  "expireTime": "2024-04-01T12:34:56.123456Z",
   "labels": {
     "cnrm-test": "true",
     "label-one": "value-one",
@@ -965,7 +965,7 @@ X-Xss-Protection: 0
   },
   "createTime": "2024-04-01T12:34:56.123456Z",
   "etag": "abcdef0123A=",
-  "expireTime": "2025-10-03T15:01:23Z",
+  "expireTime": "2024-04-01T12:34:56.123456Z",
   "labels": {
     "cnrm-test": "true",
     "label-one": "value-one",

diff --git a/...beta1/fullsecretmanagersecret/create.yaml → ...cretmanagersecret-auto-direct/create.yaml b/...beta1/fullsecretmanagersecret/create.yaml → ...cretmanagersecret-auto-direct/create.yaml
@@ -17,6 +17,7 @@ kind: SecretManagerSecret
 metadata:
   annotations:
     cnrm.cloud.google.com/project-id: ${projectId}
+    alpha.cnrm.cloud.google.com/reconciler: "direct"
   labels:
     label-one: value-one
   name: secretmanagersecret-${uniqueId}

diff --git a/...fullsecretmanagersecret/dependencies.yaml → ...nagersecret-auto-direct/dependencies.yaml b/...fullsecretmanagersecret/dependencies.yaml → ...nagersecret-auto-direct/dependencies.yaml
diff --git a/...beta1/fullsecretmanagersecret/update.yaml → ...cretmanagersecret-auto-direct/update.yaml b/...beta1/fullsecretmanagersecret/update.yaml → ...cretmanagersecret-auto-direct/update.yaml
@@ -17,6 +17,7 @@ kind: SecretManagerSecret
 metadata:
   annotations:
     cnrm.cloud.google.com/project-id: ${projectId}
+    alpha.cnrm.cloud.google.com/reconciler: "direct"
   labels:
     label-one: value-one
     label-two: value-two

diff --git a/...retmanagersecret-auto-legacy/_generated_export_fullsecretmanagersecret-auto-legacy.golden b/...retmanagersecret-auto-legacy/_generated_export_fullsecretmanagersecret-auto-legacy.golden
@@ -0,0 +1,23 @@
+apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
+kind: SecretManagerSecret
+metadata:
+  annotations:
+    cnrm.cloud.google.com/project-id: ${projectId}
+  labels:
+    cnrm-test: "true"
+    label-one: value-one
+    label-two: value-two
+    managed-by-cnrm: "true"
+  name: secretmanagersecret-${uniqueId}
+spec:
+  annotations:
+    bar: secretmanagersecret-bar
+    foo: secretmanagersecret
+  expireTime: "2025-10-03T15:01:23Z"
+  resourceID: secretmanagersecret-${uniqueId}
+  rotation:
+    nextRotationTime: "2025-10-03T15:01:23Z"
+    rotationPeriod: 3600s
+  topics:
+  - topicRef:
+      external: projects/${projectId}/topics/topic-2-${uniqueId}
diff --git a/...nagersecret-auto-legacy/_generated_object_fullsecretmanagersecret-auto-legacy.golden.yaml b/...nagersecret-auto-legacy/_generated_object_fullsecretmanagersecret-auto-legacy.golden.yaml
@@ -0,0 +1,40 @@
+apiVersion: secretmanager.cnrm.cloud.google.com/v1beta1
+kind: SecretManagerSecret
+metadata:
+  annotations:
+    cnrm.cloud.google.com/management-conflict-prevention-policy: none
+    cnrm.cloud.google.com/project-id: ${projectId}
+    cnrm.cloud.google.com/state-into-spec: absent
+  finalizers:
+  - cnrm.cloud.google.com/finalizer
+  - cnrm.cloud.google.com/deletion-defender
+  generation: 3
+  labels:
+    cnrm-test: "true"
+    label-one: value-one
+    label-two: value-two
+  name: secretmanagersecret-${uniqueId}
+  namespace: ${uniqueId}
+spec:
+  annotations:
+    bar: secretmanagersecret-bar
+    foo: secretmanagersecret
+  expireTime: "2025-10-03T15:01:23Z"
+  replication:
+    automatic: true
+  resourceID: secretmanagersecret-${uniqueId}
+  rotation:
+    nextRotationTime: "2025-10-03T15:01:23Z"
+    rotationPeriod: 3600s
+  topics:
+  - topicRef:
+      name: topic-2-${uniqueId}
+status:
+  conditions:
+  - lastTransitionTime: "1970-01-01T00:00:00Z"
+    message: The resource is up to date
+    reason: UpToDate
+    status: "True"
+    type: Ready
+  name: projects/${projectNumber}/secrets/secretmanagersecret-${uniqueId}
+  observedGeneration: 3