Merge pull request GoogleCloudPlatform#2797 from yuwenma/secret-manag…

…er-3-controller-2

feat: direct controller for SecretManagerSecret (part 1)

apis/secretmanager/v1beta1/secretmanagersecret_types.go

@@ -39,24 +39,9 @@ type SecretManagerSecretSpec struct {
 	//  The replication policy cannot be changed after the Secret has been created.
 	Replication *Replication `json:"replication,omitempty"`
 
-	/*NOTYET
-	// The labels assigned to this Secret.
-	//
-	//  Label keys must be between 1 and 63 characters long, have a UTF-8 encoding
-	//  of maximum 128 bytes, and must conform to the following PCRE regular
-	//  expression: `[\p{Ll}\p{Lo}][\p{Ll}\p{Lo}\p{N}_-]{0,62}`
-	//
-	//  Label values must be between 0 and 63 characters long, have a UTF-8
-	//  encoding of maximum 128 bytes, and must conform to the following PCRE
-	//  regular expression: `[\p{Ll}\p{Lo}\p{N}_-]{0,63}`
-	//
-	//  No more than 64 labels can be assigned to a given resource.
-	Labels map[string]string `json:"labels,omitempty"`
-	*/
-
 	// Optional. A list of up to 10 Pub/Sub topics to which messages are published
 	//  when control plane operations are called on the secret or its versions.
-	TopicRefs []TopicRef `json:"topics,omitempty"`
+	TopicRefs []*TopicRef `json:"topics,omitempty"`
 
 	// Optional. Timestamp in UTC when the
 	//  [Secret][google.cloud.secretmanager.v1.Secret] is scheduled to expire.
@@ -125,7 +110,7 @@ type SecretManagerSecretSpec struct {
 
 type TopicRef struct {
 	// +required
-	PubSubTopicRef refv1beta1.PubSubTopicRef `json:"topicRef,omitempty"`
+	PubSubTopicRef *refv1beta1.PubSubTopicRef `json:"topicRef,omitempty"`
 }
 
 // +kcc:proto=google.cloud.secretmanager.v1.CustomerManagedEncryption
@@ -144,7 +129,7 @@ type CustomerManagedEncryption struct {
 	//  replication policy type, Cloud KMS CryptoKeys must reside in `global`.
 	//
 	//  The expected format is `projects/*/locations/*/keyRings/*/cryptoKeys/*`.
-	KmsKeyRef refv1beta1.KMSCryptoKeyRef `json:"kmsKeyRef,omitempty"`
+	KmsKeyRef *refv1beta1.KMSCryptoKeyRef `json:"kmsKeyRef,omitempty"`
 }
 
 // SecretManagerSecretStatus defines the config connector machine state of SecretManagerSecret

dev/tasks/run-e2e

@@ -24,7 +24,7 @@ echo "Downloading envtest assets..."
 export KUBEBUILDER_ASSETS=$(go run sigs.k8s.io/controller-runtime/tools/setup-envtest@latest use -p path)
 
 if [[ -z "${KCC_USE_DIRECT_RECONCILERS:-}" ]]; then
-  KCC_USE_DIRECT_RECONCILERS=ComputeForwardingRule,GKEHubFeatureMembership
+  KCC_USE_DIRECT_RECONCILERS=ComputeForwardingRule,GKEHubFeatureMembership,SecretManagerSecret
 fi
 echo "Using direct controllers: $KCC_USE_DIRECT_RECONCILERS"
 export KCC_USE_DIRECT_RECONCILERS

dev/tools/controllerbuilder/generate.sh

@@ -33,14 +33,6 @@ go run . generate-types \
     --kind DataflowFlexTemplateJob \
     --proto-resource FlexTemplateRuntimeEnvironment
 
-go run main.go generate-types \
-     --service google.cloud.secretmanager.v1  \
-     --proto-source-path ../proto-to-mapper/build/googleapis.pb \
-     --output-api ${APIS_DIR} \
-     --kind SecretManagerSecret  \ 
-     --proto-resource Secret \
-     --api-version "secretmanager.cnrm.cloud.google.com/v1beta1"
-
 go run . generate-mapper \
     --proto-source-path ../proto-to-mapper/build/googleapis.pb \
     --service google.dataflow.v1beta3 \
@@ -203,5 +195,22 @@ go run . generate-mapper \
     --output-dir ${OUTPUT_MAPPER} \
     --api-dir ${APIS_DIR}
 
+# SecretManager
+go run main.go generate-types \
+     --service google.cloud.secretmanager.v1 \
+     --proto-source-path ../proto-to-mapper/build/googleapis.pb \
+     --output-api ${APIS_DIR} \
+     --kind SecretManagerSecret \
+     --proto-resource Secret \
+     --api-version "secretmanager.cnrm.cloud.google.com/v1beta1"
+
+go run . generate-mapper \
+   --proto-source-path ../proto-to-mapper/build/googleapis.pb \
+   --service google.cloud.secretmanager.v1 \
+   --api-version "secretmanager.cnrm.cloud.google.com/v1beta1" \
+   --api-go-package-path  $REPO_ROOT/apis/ \
+   --output-dir $REPO_ROOT/pkg/controller/direct/ \
+   --api-dir $REPO_ROOT/apis/
+
 # Fix up formatting
 ${REPO_ROOT}/dev/tasks/fix-gofmt

dev/tools/controllerbuilder/template/apis/refs.go

@@ -80,7 +80,7 @@ func (r *{{.Kind}}Ref) NormalizedExternal(ctx context.Context, reader client.Rea
 	if err != nil {
 		return "", fmt.Errorf("reading status.externalRef: %w", err)
 	}
-	if actualExternalRef == ""
+	if actualExternalRef == "" {
 		return "", k8s.NewReferenceNotReadyError(u.GroupVersionKind(), key)
 	}
 	r.External = actualExternalRef

go.mod

@@ -21,6 +21,7 @@ require (
 	cloud.google.com/go/profiler v0.4.1
 	cloud.google.com/go/redis v1.17.0
 	cloud.google.com/go/resourcemanager v1.10.0
+	cloud.google.com/go/secretmanager v1.14.0
 	cloud.google.com/go/securesourcemanager v1.1.1
 	cloud.google.com/go/security v1.18.0
 	cloud.google.com/go/workstations v1.1.1

mockgcp/mocksecretmanager/secrets.go

@@ -16,6 +16,7 @@ package mocksecretmanager
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -62,22 +63,30 @@ func (s *SecretsV1) CreateSecret(ctx context.Context, req *pb.CreateSecretReques
 	obj.Name = fqn
 	obj.CreateTime = timestamppb.Now()
 	if obj.Replication == nil {
-		obj.Replication = &pb.Replication{}
-	}
-	if obj.Replication.Replication == nil {
-		obj.Replication.Replication = &pb.Replication_Automatic_{
-			Automatic: &pb.Replication_Automatic{},
-		}
+		return nil, fmt.Errorf("Secret.replication must be specified.")
 	}
 	obj.Etag = computeEtag(obj)
-
+	if err := s.populateDefaultsForSecret(ctx, obj); err != nil {
+		return nil, err
+	}
 	if err := s.storage.Create(ctx, fqn, obj); err != nil {
 		return nil, err
 	}
 
 	return obj, nil
 }
 
+func (s *SecretsV1) populateDefaultsForSecret(ctx context.Context, obj *pb.Secret) error {
+	for _, version := range obj.VersionAliases {
+		versionName := obj.Name + "/versions/" + strconv.FormatInt(version, 64)
+		_, err := s.GetSecretVersion(ctx, &pb.GetSecretVersionRequest{Name: versionName})
+		if err != nil {
+			return fmt.Errorf("Aliases cannot be assigned to versions that don't exist")
+		}
+	}
+	return nil
+}
+
 // Gets metadata for a given [Secret][google.cloud.secretmanager.v1.Secret].
 func (s *SecretsV1) GetSecret(ctx context.Context, req *pb.GetSecretRequest) (*pb.Secret, error) {
 	name, err := s.parseSecretName(req.Name)
@@ -122,28 +131,41 @@ func (s *SecretsV1) UpdateSecret(ctx context.Context, req *pb.UpdateSecretReques
 	if len(paths) == 0 {
 		return nil, status.Errorf(codes.InvalidArgument, "update_mask is required")
 	}
-	// TODO: Some sort of helper for fieldmask?
 	for _, path := range paths {
 		switch path {
 		case "topics":
 			updated.Topics = req.Secret.GetTopics()
-		case "customer_managed_encryption":
+		case "customerManagedEncryption":
 			updated.CustomerManagedEncryption = req.Secret.GetCustomerManagedEncryption()
 		case "rotation":
 			updated.Rotation = req.Secret.GetRotation()
+			if len(req.Secret.GetTopics()) == 0 {
+				return nil, fmt.Errorf("There must be at least one topic configured when a Rotation policy is set.")
+			}
+		case "rotation.rotationPeriod":
+			updated.Rotation.RotationPeriod = req.Secret.GetRotation().RotationPeriod
 		case "annotations":
 			updated.Annotations = req.Secret.GetAnnotations()
 		case "labels":
 			updated.Labels = req.Secret.GetLabels()
-		case "version_aliases":
+		case "versionAliases":
 			updated.VersionAliases = req.Secret.GetVersionAliases()
+		case "expireTime":
+			updated.Expiration = &pb.Secret_ExpireTime{
+				ExpireTime: req.Secret.GetExpireTime(),
+			}
 		case "expiration":
 			updated.Expiration = req.Secret.GetExpiration()
+		case "rotation.nextRotationTime":
+			updated.Rotation.NextRotationTime = req.Secret.Rotation.NextRotationTime
 		default:
 			return nil, status.Errorf(codes.InvalidArgument, "update_mask path %q not valid", path)
 		}
 	}
 
+	if err := s.populateDefaultsForSecret(ctx, updated); err != nil {
+		return nil, err
+	}
 	if err := s.storage.Update(ctx, fqn, updated); err != nil {
 		return nil, err
 	}