refactor: move IAMServiceAccount reference resolution into API package

This makes it easier to consume.

apis/gkehub/v1beta1/gkehubfeaturemembership_types.go

@@ -85,7 +85,7 @@ type FeaturemembershipConfigmanagement struct {
 
 type FeaturemembershipGit struct {
 	// +optional
-	GcpServiceAccountRef *refs.GcpServiceAccountRef `json:"gcpServiceAccountRef,omitempty"`
+	GcpServiceAccountRef *refs.IAMServiceAccountRef `json:"gcpServiceAccountRef,omitempty"`
 
 	/* URL for the HTTPS proxy to be used when communicating with the Git repo. */
 	// +optional
@@ -148,7 +148,7 @@ type FeaturemembershipMonitoring struct {
 
 type FeaturemembershipOci struct {
 	// +optional
-	GcpServiceAccountRef *refs.GcpServiceAccountRef `json:"gcpServiceAccountRef,omitempty"`
+	GcpServiceAccountRef *refs.IAMServiceAccountRef `json:"gcpServiceAccountRef,omitempty"`
 
 	/* The absolute path of the directory that contains the local resources. Default: the root directory of the image. */
 	// +optional

apis/refs/v1beta1/gcpserviceaccountref.go

@@ -14,20 +14,111 @@
 
 package v1beta1
 
-type MetricsGcpServiceAccountRef struct {
-	/* The Email of the Google Cloud Service Account (GSA) used for exporting Config Sync metrics to Cloud Monitoring. The GSA should have the Monitoring Metric Writer(roles/monitoring.metricWriter) IAM role. The Kubernetes ServiceAccount `default` in the namespace `config-management-monitoring` should be bound to the GSA. Allowed value: The `email` field of an `IAMServiceAccount` resource. */
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+type IAMServiceAccountRef struct {
+	/* The GCP Service Account Email used for auth when secretType is gcpServiceAccount. Allowed value: The `email` field of an `IAMServiceAccount` resource. */
 	External string `json:"external,omitempty"`
 	/* Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names */
 	Name string `json:"name,omitempty"`
 	/* Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ */
 	Namespace string `json:"namespace,omitempty"`
 }
 
-type GcpServiceAccountRef struct {
-	/* The GCP Service Account Email used for auth when secretType is gcpServiceAccount. Allowed value: The `email` field of an `IAMServiceAccount` resource. */
+type MetricsGcpServiceAccountRef struct {
+	/* The Email of the Google Cloud Service Account (GSA) used for exporting Config Sync metrics to Cloud Monitoring. The GSA should have the Monitoring Metric Writer(roles/monitoring.metricWriter) IAM role. The Kubernetes ServiceAccount `default` in the namespace `config-management-monitoring` should be bound to the GSA. Allowed value: The `email` field of an `IAMServiceAccount` resource. */
 	External string `json:"external,omitempty"`
 	/* Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names */
 	Name string `json:"name,omitempty"`
 	/* Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/ */
 	Namespace string `json:"namespace,omitempty"`
 }
+
+func (r *MetricsGcpServiceAccountRef) Resolve(ctx context.Context, reader client.Reader, src client.Object) error {
+	if r == nil {
+		return nil
+	}
+
+	serviceAccountInfo, err := resolveServiceAccount(ctx, reader, src, r.Name, r.Namespace, r.External)
+	if err != nil {
+		return err
+	}
+	*r = MetricsGcpServiceAccountRef{External: serviceAccountInfo.External}
+	return nil
+}
+
+func (r *IAMServiceAccountRef) Resolve(ctx context.Context, reader client.Reader, src client.Object) error {
+	if r == nil {
+		return nil
+	}
+
+	serviceAccountInfo, err := resolveServiceAccount(ctx, reader, src, r.Name, r.Namespace, r.External)
+	if err != nil {
+		return err
+	}
+	*r = IAMServiceAccountRef{External: serviceAccountInfo.External}
+	return nil
+}
+
+type serviceAccountInfo struct {
+	External string
+}
+
+func resolveServiceAccount(ctx context.Context, reader client.Reader, src client.Object, name, namespace, external string) (*serviceAccountInfo, error) {
+	if external != "" {
+		if name != "" {
+			return nil, fmt.Errorf("cannot specify both name and external on an IAMServiceAccount reference")
+		}
+
+		if strings.Contains(external, "@") {
+			return &serviceAccountInfo{External: external}, nil
+		}
+		return nil, fmt.Errorf("format of IAMServiceAccount reference external=%q was not known (use email address)", external)
+	}
+
+	if name == "" {
+		return nil, fmt.Errorf("must specify either name or external on an IAMServiceAccount reference")
+	}
+
+	key := types.NamespacedName{
+		Namespace: namespace,
+		Name:      name,
+	}
+	if key.Namespace == "" {
+		key.Namespace = src.GetNamespace()
+	}
+
+	computenetwork := &unstructured.Unstructured{}
+	computenetwork.SetGroupVersionKind(schema.GroupVersionKind{
+		Group:   "iam.cnrm.cloud.google.com",
+		Version: "v1beta1",
+		Kind:    "IAMServiceAccount",
+	})
+	if err := reader.Get(ctx, key, computenetwork); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil, fmt.Errorf("referenced IAMServiceAccount %v not found", key)
+		}
+		return nil, fmt.Errorf("error reading referenced IAMServiceAccount %v: %w", key, err)
+	}
+
+	email, _, err := unstructured.NestedString(computenetwork.Object, "status", "email")
+	if err != nil {
+		return nil, fmt.Errorf("reading status.email from IAMServiceAccount %v: %w", key, err)
+	}
+	// if the status.email not populated, should we construct the email from spec.resourceID or metadata.name.
+	if email == "" {
+		return nil, fmt.Errorf("status.email is empty from IAMServiceAccount %v, expected not-empty", key)
+	}
+
+	return &serviceAccountInfo{External: email}, nil
+}

pkg/controller/direct/gkehub/featuremembership_controller.go

@@ -100,11 +100,10 @@ func (m *gkeHubModel) AdapterForObject(ctx context.Context, reader client.Reader
 	if err != nil {
 		return nil, err
 	}
-	apiObj, err := featureMembershipSpecKRMtoMembershipFeatureSpecAPI(&obj.Spec)
-	if err != nil {
+	if err := resolveIAMReferences(ctx, reader, obj); err != nil {
 		return nil, err
 	}
-	err = setIAMReferences(ctx, reader, obj, apiObj)
+	apiObj, err := featureMembershipSpecKRMtoMembershipFeatureSpecAPI(&obj.Spec)
 	if err != nil {
 		return nil, err
 	}
@@ -118,45 +117,20 @@ func (m *gkeHubModel) AdapterForObject(ctx context.Context, reader client.Reader
 	}, nil
 }
 
-func setIAMReferences(ctx context.Context, reader client.Reader, obj *krm.GKEHubFeatureMembership, apiObj *featureapi.MembershipFeatureSpec) error {
+func resolveIAMReferences(ctx context.Context, reader client.Reader, obj *krm.GKEHubFeatureMembership) error {
 	spec := obj.Spec
 	if spec.Configmanagement != nil && spec.Configmanagement.ConfigSync != nil {
-		if spec.Configmanagement.ConfigSync.MetricsGcpServiceAccountRef != nil {
-			val, err := resolveMetricsGcpServiceAccountRef(ctx, reader, spec.Configmanagement.ConfigSync.MetricsGcpServiceAccountRef, obj.GetNamespace())
-			if err != nil {
-				return err
-			}
-			// play it safe here to check apiObj ref path exists. The path should be initialized in featureMembershipSpecKRMtoMembershipFeatureSpecAPI if the KRM fields not empty.
-			if apiObj.Configmanagement != nil && apiObj.Configmanagement.ConfigSync != nil {
-				apiObj.Configmanagement.ConfigSync.MetricsGcpServiceAccountEmail = val
-			} else {
-				return fmt.Errorf("apiObj is not initialized properly, expected to see apiObj.Configmanagement.ConfigSync not nil")
-			}
+		if err := spec.Configmanagement.ConfigSync.MetricsGcpServiceAccountRef.Resolve(ctx, reader, obj); err != nil {
+			return err
 		}
 		if spec.Configmanagement.ConfigSync.Git != nil {
-			if spec.Configmanagement.ConfigSync.Git.GcpServiceAccountRef != nil {
-				val, err := resolveGcpServiceAccountRef(ctx, reader, spec.Configmanagement.ConfigSync.Git.GcpServiceAccountRef, obj.GetNamespace())
-				if err != nil {
-					return err
-				}
-				if apiObj.Configmanagement != nil && apiObj.Configmanagement.ConfigSync != nil && apiObj.Configmanagement.ConfigSync.Git != nil {
-					apiObj.Configmanagement.ConfigSync.Git.GcpServiceAccountEmail = val
-				} else {
-					return fmt.Errorf("apiObj is not initialized properly, expected to see apiObj.Configmanagement.ConfigSync.Git not nil")
-				}
+			if err := spec.Configmanagement.ConfigSync.Git.GcpServiceAccountRef.Resolve(ctx, reader, obj); err != nil {
+				return err
 			}
 		}
 		if spec.Configmanagement.ConfigSync.Oci != nil {
-			if spec.Configmanagement.ConfigSync.Oci.GcpServiceAccountRef != nil {
-				val, err := resolveGcpServiceAccountRef(ctx, reader, spec.Configmanagement.ConfigSync.Oci.GcpServiceAccountRef, obj.GetNamespace())
-				if err != nil {
-					return err
-				}
-				if apiObj.Configmanagement != nil && apiObj.Configmanagement.ConfigSync != nil && apiObj.Configmanagement.ConfigSync.Oci != nil {
-					apiObj.Configmanagement.ConfigSync.Oci.GcpServiceAccountEmail = val
-				} else {
-					return fmt.Errorf("apiObj is not initialized properly, expected to see apiObj.Configmanagement.ConfigSync.Oci not nil")
-				}
+			if err := spec.Configmanagement.ConfigSync.Oci.GcpServiceAccountRef.Resolve(ctx, reader, obj); err != nil {
+				return err
 			}
 		}
 	}

pkg/controller/direct/gkehub/references.go

@@ -20,7 +20,6 @@ import (
 	"strings"
 
 	krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/gkehub/v1beta1"
-	"github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -154,59 +153,3 @@ func resolveFeatureRef(ctx context.Context, reader client.Reader, obj *krm.GKEHu
 		id: fmt.Sprintf("projects/%s/locations/%s/features/%s", projectID, featureLocation, featureName),
 	}, nil
 }
-
-func resolveMetricsGcpServiceAccountRef(ctx context.Context, reader client.Reader, r *v1beta1.MetricsGcpServiceAccountRef, resourceNamespace string) (string, error) {
-	name := r.Name
-	namespace := r.Namespace
-	external := r.External
-	return getSAEmailWith(ctx, reader, name, namespace, external, resourceNamespace)
-}
-
-func resolveGcpServiceAccountRef(ctx context.Context, reader client.Reader, r *v1beta1.GcpServiceAccountRef, resourceNamespace string) (string, error) {
-	name := r.Name
-	namespace := r.Namespace
-	external := r.External
-	return getSAEmailWith(ctx, reader, name, namespace, external, resourceNamespace)
-
-}
-
-func getSAEmailWith(ctx context.Context, reader client.Reader, name, namespace, external, resourceNamespace string) (string, error) {
-	if external != "" {
-		if name != "" {
-			return "", fmt.Errorf("cannot specify both name and external on IAMServiceAccount reference")
-		}
-		return external, nil
-	}
-	if name == "" {
-		return "", fmt.Errorf("must specify either name or external on IAMServiceAccount reference")
-	}
-	if namespace == "" {
-		namespace = resourceNamespace
-	}
-
-	key := types.NamespacedName{
-		Namespace: namespace,
-		Name:      name,
-	}
-	sa := &unstructured.Unstructured{}
-	sa.SetGroupVersionKind(schema.GroupVersionKind{
-		Group:   "iam.cnrm.cloud.google.com",
-		Version: "v1beta1",
-		Kind:    "IAMServiceAccount",
-	})
-	if err := reader.Get(ctx, key, sa); err != nil {
-		if apierrors.IsNotFound(err) {
-			return "", fmt.Errorf("referenced IAMServiceAccount %v not found", key)
-		}
-		return "", fmt.Errorf("error reading referenced IAMServiceAccount %v: %w", key, err)
-	}
-	email, _, err := unstructured.NestedString(sa.Object, "status", "email")
-	if err != nil {
-		return "", fmt.Errorf("reading status.email from IAMServiceAccount %v: %w", key, err)
-	}
-	// if the status.email not populated, should we construct the email from spec.resourceID or metadata.name.
-	if email == "" {
-		return "", fmt.Errorf("status.email is empty from IAMServiceAccount %v, expected not-empty", key)
-	}
-	return email, nil
-}