Merge pull request GoogleCloudPlatform#4699 from gemmahou/kms-refs

chore: refactor KMSCyptoKey and KMSKeyRing refs

Author: google-oss-prow[bot]

apis/kms/v1beta1/cryptokey_identity.go

@@ -0,0 +1,46 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1beta1
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/GoogleCloudPlatform/k8s-config-connector/apis/common/parent"
+)
+
+type KMSCryptoKeyIdentity struct {
+	parent *KMSKeyRingIdentity
+	id     string
+}
+
+func (i *KMSCryptoKeyIdentity) String() string {
+	return i.parent.String() + "/cryptoKeys/" + i.id
+}
+
+func ParseKMSCryptoKeyExternal(external string) (*KMSCryptoKeyIdentity, error) {
+	external = strings.TrimPrefix(external, "/")
+	tokens := strings.Split(external, "/")
+	// projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}}/cryptoKeys/{{key}}
+	if len(tokens) == 8 && tokens[0] == "projects" && tokens[2] == "locations" && tokens[4] == "keyRings" && tokens[6] == "cryptoKeys" {
+		return &KMSCryptoKeyIdentity{parent: &KMSKeyRingIdentity{
+			Parent: &parent.ProjectAndLocationParent{
+				ProjectID: tokens[1], Location: tokens[3],
+			}, ID: tokens[5],
+		}, id: tokens[7]}, nil
+	}
+	return nil, fmt.Errorf("format of KMSCryptoKey external=%q was not known (use projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}}/cryptoKeys/{{keyId}})", external)
+
+}

apis/kms/v1beta1/cryptokey_reference.go

@@ -55,6 +55,25 @@ func (r *kmsCryptoKeyRef) normalizedExternal(ctx context.Context, reader client.
 		}
 		return "", fmt.Errorf("reading referenced %s %s: %w", KMSCryptoKeyGVK, key, err)
 	}
-	// todo: resolve KMSCryptoKey from its name
-	return "", nil
+
+	// Get external from status.externalRef. This is the most trustworthy place.
+	actualExternalRef, _, err := unstructured.NestedString(u.Object, "status", "externalRef")
+	if err != nil {
+		return "", fmt.Errorf("reading status.externalRef: %w", err)
+	}
+	if actualExternalRef != "" {
+		return actualExternalRef, nil
+	}
+
+	// Backward compatible for resources still managed by the legacy controller and without the status.externalRef
+	// Use status.selfLink as the external value of cryptokey
+	// status.selfLink: projects/${projectId}/locations/us-central1/keyRings/kmscryptokey-${uniqueId}/cryptoKeys/kmscryptokey-${uniqueId}
+	selfLink, _, err := unstructured.NestedString(u.Object, "status", "selfLink")
+	if err != nil {
+		return "", fmt.Errorf("reading status.selfLink: %w", err)
+	}
+	if selfLink == "" {
+		return "", k8s.NewReferenceNotReadyError(u.GroupVersionKind(), key)
+	}
+	return selfLink, nil
 }

apis/kms/v1beta1/importjob_identity.go

@@ -19,15 +19,16 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/GoogleCloudPlatform/k8s-config-connector/apis/common/parent"
+
 	"github.com/GoogleCloudPlatform/k8s-config-connector/apis/common"
-	refsv1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // ImportJobIdentity defines the resource reference to KMSImportJob, which "External" field
 // holds the GCP identifier for the KRM object.
 type ImportJobIdentity struct {
-	parent *ImportJobParent
+	parent *KMSKeyRingIdentity
 	id     string
 }
 
@@ -39,38 +40,21 @@ func (i *ImportJobIdentity) ID() string {
 	return i.id
 }
 
-func (i *ImportJobIdentity) Parent() *ImportJobParent {
+func (i *ImportJobIdentity) Parent() *KMSKeyRingIdentity {
 	return i.parent
 }
 
-type ImportJobParent struct {
-	ProjectID string
-	Location  string
-	KeyRingID string
-}
-
-func (p *ImportJobParent) String() string {
-	return "projects/" + p.ProjectID + "/locations/" + p.Location + "/keyRings/" + p.KeyRingID
-}
-
 // New builds a ImportJobIdentity from the Config Connector ImportJob object.
 func NewImportJobIdentity(ctx context.Context, reader client.Reader, obj *KMSImportJob) (*ImportJobIdentity, error) {
 
 	// Get Parent
-	kmsKeyRing, err := refsv1beta1.ResolveKMSKeyRingRef(ctx, reader, obj, obj.Spec.KMSKeyRingRef)
+	kmsKeyRingExternal, err := obj.Spec.KMSKeyRingRef.NormalizedExternal(ctx, reader, obj.Namespace)
 	if err != nil {
 		return nil, err
 	}
-	var parent *ImportJobParent
-	tokens := strings.Split(kmsKeyRing.Ref.External, "/")
-	if len(tokens) == 6 && tokens[0] == "projects" && tokens[2] == "locations" && tokens[4] == "keyRings" {
-		parent = &ImportJobParent{
-			ProjectID: tokens[1],
-			Location:  tokens[3],
-			KeyRingID: tokens[5],
-		}
-	} else {
-		return nil, fmt.Errorf("format of KMSKeyRingRef external=%q was not known (use projects/[kms_project_id]/locations/[region]/keyRings/[key_ring_id])", kmsKeyRing.Ref.External)
+	kmsKeyRing, err := ParseKMSKeyRingExternal(kmsKeyRingExternal)
+	if err != nil {
+		return nil, err
 	}
 
 	// Get desired ID
@@ -84,38 +68,36 @@ func NewImportJobIdentity(ctx context.Context, reader client.Reader, obj *KMSImp
 
 	// Use approved External
 	externalRef := common.ValueOf(obj.Status.ExternalRef)
-	var actualParent *ImportJobParent
-	var actualResourceID string
 	if externalRef != "" {
 		// Validate desired with actual
-		actualParent, actualResourceID, err = ParseImportJobExternal(externalRef)
+		actualParent, actualResourceID, err := ParseImportJobExternal(externalRef)
 		if err != nil {
 			return nil, err
 		}
-		if actualParent.String() != parent.String() {
-			return nil, fmt.Errorf("spec.kmsKeyRingRef changed, expect %s, got %s", actualParent.String(), kmsKeyRing.Ref.External)
+		if actualParent.String() != kmsKeyRing.String() {
+			return nil, fmt.Errorf("spec.kmsKeyRingRef changed, expect %s, got %s", actualParent.String(), kmsKeyRing.String())
 		}
 		if actualResourceID != resourceID {
 			return nil, fmt.Errorf("cannot reset `metadata.name` or `spec.resourceID` to %s, since it has already assigned to %s",
 				resourceID, actualResourceID)
 		}
 	}
 	return &ImportJobIdentity{
-		parent: parent,
+		parent: kmsKeyRing,
 		id:     resourceID,
 	}, nil
 }
 
-func ParseImportJobExternal(external string) (parent *ImportJobParent, resourceID string, err error) {
+func ParseImportJobExternal(external string) (*KMSKeyRingIdentity, string, error) {
 	tokens := strings.Split(external, "/")
 	if len(tokens) != 8 || tokens[0] != "projects" || tokens[2] != "locations" || tokens[4] != "keyRings" || tokens[6] != "importJobs" {
 		return nil, "", fmt.Errorf("format of KMSImportJob external=%q was not known (use projects/{{projectID}}/locations/{{location}}/keyRings/{{keyRingID}}/importJobs/{{importJobID}})", external)
 	}
-	parent = &ImportJobParent{
-		ProjectID: tokens[1],
-		Location:  tokens[3],
-		KeyRingID: tokens[5],
+	p := &KMSKeyRingIdentity{
+		Parent: &parent.ProjectAndLocationParent{
+			ProjectID: tokens[1], Location: tokens[3],
+		}, ID: tokens[5],
 	}
-	resourceID = tokens[7]
-	return parent, resourceID, nil
+	resourceID := tokens[7]
+	return p, resourceID, nil
 }

apis/kms/v1beta1/importjob_types.go

@@ -15,7 +15,6 @@
 package v1beta1
 
 import (
-	refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
 	"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/apis/k8s/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -25,7 +24,7 @@ var KMSImportJobGVK = GroupVersion.WithKind("KMSImportJob")
 // KMSImportJobSpec defines the desired state of KMSImportJob
 // +kcc:spec:proto=google.cloud.kms.v1.ImportJob
 type KMSImportJobSpec struct {
-	KMSKeyRingRef *refs.KMSKeyRingRef `json:"kmsKeyRingRef"`
+	KMSKeyRingRef *KMSKeyRingRef `json:"kmsKeyRingRef"`
 
 	// The KMSImportJob name. If not given, the metadata.name will be used.
 	ResourceID *string `json:"resourceID,omitempty"`

apis/kms/v1beta1/keyhandle_identity.go

@@ -49,10 +49,6 @@ func (p *KMSKeyHandleParent) String() string {
 	return "projects/" + p.ProjectID + "/locations/" + p.Location
 }
 
-func asKMSKeyHandleExternal(parent *KMSKeyHandleParent, resourceID string) (external string) {
-	return parent.String() + "/keyHandles/" + resourceID
-}
-
 func NewKMSKeyHandleIdentity(ctx context.Context, reader client.Reader, obj *KMSKeyHandle) (*KMSKeyHandleIdentity, error) {
 	id := &KMSKeyHandleIdentity{}
 

apis/kms/v1beta1/keyhandle_reference.go

@@ -54,7 +54,7 @@ func (r *kmsKeyHandleRef) normalizedExternal(ctx context.Context, reader client.
 		}
 		return "", fmt.Errorf("reading referenced %s %s: %w", KMSKeyHandleGVK, key, err)
 	}
-	// Use status.observedState.kmsKey instead of status.externalRef as the external resourceID of autoKey
+	// Use status.observedState.kmsKey instead of status.externalRef as the external value of autoKey
 	// status.externalRef: projects/${projectId}/locations/us-central1/keyHandles/1a1a1a-222b-3cc3-d444-e555ee555555
 	// status.observedState.kmsKey: projects/${key_project}/locations/us-central1/keyRings/autokey/cryptoKeys/${projectNumber}-compute-disk-${generated-id}
 	kmsKey, _, err := unstructured.NestedString(u.Object, "status", "observedState", "kmsKey")

apis/kms/v1beta1/keyring_identity.go

@@ -0,0 +1,43 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1beta1
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/GoogleCloudPlatform/k8s-config-connector/apis/common/parent"
+)
+
+type KMSKeyRingIdentity struct {
+	Parent *parent.ProjectAndLocationParent
+	ID     string
+}
+
+func (i *KMSKeyRingIdentity) String() string {
+	return i.Parent.String() + "/keyRings/" + i.ID
+}
+
+func ParseKMSKeyRingExternal(external string) (*KMSKeyRingIdentity, error) {
+	external = strings.TrimPrefix(external, "/")
+	tokens := strings.Split(external, "/")
+	// projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}}
+	if len(tokens) == 6 && tokens[0] == "projects" && tokens[2] == "locations" && tokens[4] == "keyRings" {
+		return &KMSKeyRingIdentity{Parent: &parent.ProjectAndLocationParent{
+			ProjectID: tokens[1], Location: tokens[3],
+		}, ID: tokens[5]}, nil
+	}
+	return nil, fmt.Errorf("format of KMSKeyRing external=%q was not known (use projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}})", external)
+}

apis/kms/v1beta1/keyring_reference.go

@@ -0,0 +1,102 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package v1beta1
+
+import (
+	"context"
+	"fmt"
+
+	refsv1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
+	"github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+var _ refsv1beta1.ExternalNormalizer = &KMSKeyRingRef{}
+
+// KMSKeyRingRef defines the resource reference to KMSKeyRing, which "External" field
+// holds the GCP identifier for the KRM object.
+type KMSKeyRingRef struct {
+	// A reference to an externally managed KMSKeyRing.
+	// Should be in the format `projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}}`.
+	External string `json:"external,omitempty"`
+
+	// The `name` of a `KMSKeyRing` resource.
+	Name string `json:"name,omitempty"`
+	// The `namespace` of a `KMSKeyRing` resource.
+	Namespace string `json:"namespace,omitempty"`
+}
+
+// NormalizedExternal provision the "External" value for other resource that depends on KMSKeyRingRef.
+// If the "External" is given in the other resource's spec.KMSKeyRingRef, the given value will be used.
+// Otherwise, the "Name" and "Namespace" will be used to query the actual KMSKeyRingRef object from the cluster.
+func (r *KMSKeyRingRef) NormalizedExternal(ctx context.Context, reader client.Reader, otherNamespace string) (string, error) {
+	if r.External != "" && r.Name != "" {
+		return "", fmt.Errorf("cannot specify both name and external on %s reference", KMSKeyRingGVK.Kind)
+	}
+	// From given External
+	// External should be in the `projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}}` format
+	if r.External != "" {
+		if _, err := ParseKMSKeyRingExternal(r.External); err != nil {
+			return "", err
+		}
+		return r.External, nil
+	}
+
+	// From the Config Connector object
+	if r.Namespace == "" {
+		r.Namespace = otherNamespace
+	}
+	key := types.NamespacedName{Name: r.Name, Namespace: r.Namespace}
+	u := &unstructured.Unstructured{}
+	u.SetGroupVersionKind(KMSKeyRingGVK)
+	if err := reader.Get(ctx, key, u); err != nil {
+		if apierrors.IsNotFound(err) {
+			return "", k8s.NewReferenceNotFoundError(u.GroupVersionKind(), key)
+		}
+		return "", fmt.Errorf("reading referenced %s %s: %w", KMSKeyRingGVK, key, err)
+	}
+
+	// Get external from status.externalRef. This is the most trustworthy place.
+	actualExternalRef, _, err := unstructured.NestedString(u.Object, "status", "externalRef")
+	if err != nil {
+		return "", fmt.Errorf("reading status.externalRef: %w", err)
+	}
+	if actualExternalRef != "" {
+		r.External = actualExternalRef
+		return r.External, nil
+	}
+
+	// Backward compatible for resources still managed by the legacy controller and without the status.externalRef
+	resourceID, err := refsv1beta1.GetResourceID(u)
+	if err != nil {
+		return "", err
+	}
+
+	projectID, err := refsv1beta1.ResolveProjectID(ctx, reader, u)
+	if err != nil {
+		return "", err
+	}
+
+	location, err := refsv1beta1.GetLocation(u)
+	if err != nil {
+		return "", err
+	}
+
+	r.External = fmt.Sprintf("projects/%s/locations/%s/keyRings/%s", projectID, location, resourceID)
+	return r.External, nil
+}

apis/kms/v1beta1/keyring.go apis/kms/v1beta1/keyring_types.goapis/kms/v1beta1/keyring.go renamed to apis/kms/v1beta1/keyring_types.go

@@ -34,8 +34,8 @@ type KMSKeyRef_OneOf struct {
 	KMSKeyHandleRef *kmsKeyHandleRef `json:"keyHandleRef,omitempty"`
 
 	// A reference to an externally managed KMSCryptoKey or KMSKeyHandle(AutoKey).
-	// Should be in the format `projects/{{kms_project_id}}/locations/{{region}}/keyRings/{{key_ring_id}}/cryptoKeys/{{key}}`.
-	// For AutoKey, replace {{key_ring_id}} to `autokey`, i.e. `projects/{{kms_project_id}}/locations/{{region}}/keyRings/autokey/cryptoKeys/{{key}}`.
+	// Should be in the format `projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}}/cryptoKeys/{{keyId}}`.
+	// For AutoKey, replace {{keyRingId}} to `autokey`, i.e. `projects/{{projectId}}/locations/{{location}}/keyRings/autokey/cryptoKeys/{{keyId}}`.
 	External string `json:"external,omitempty"`
 }
 
@@ -50,7 +50,7 @@ func (r *KMSKeyRef_OneOf) NormalizedExternal(ctx context.Context, reader client.
 		if len(tokens) == 8 && tokens[0] == "projects" && tokens[2] == "locations" && tokens[4] == "keyRings" && tokens[6] == "cryptoKeys" {
 			return r.External, nil
 		}
-		return "", fmt.Errorf("format of KMSKeyRef external=%q was not known (use projects/{{kms_project_id}}/locations/{{region}}/keyRings/{{key_ring_id}}/cryptoKeys/{{key}})", r.External)
+		return "", fmt.Errorf("format of KMSKeyRef external=%q was not known (use projects/{{projectId}}/locations/{{location}}/keyRings/{{keyRingId}}/cryptoKeys/{{keyId}})", r.External)
 	}
 
 	// Resolve the KCC managed reference resource by its name