add firewall policy reference

apis/compute/v1beta1/computefirewallpolicy_reference.go

@@ -0,0 +1,105 @@
+// Copyright 2024 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"
+	"strings"
+
+	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 = &ComputeFirewallPolicyRef{}
+var ComputeFirewallPolicyGVK = GroupVersion.WithKind("ComputeFirewallPolicy")
+
+// ComputeFirewallPolicyRef defines the resource reference to ComputeFirewallPolicy, which "External" field
+// holds the GCP identifier for the KRM object.
+type ComputeFirewallPolicyRef struct {
+	// A reference to an externally managed ComputeFirewallPolicy resource.
+	// Should be in the format "locations/global/firewallPolicies/{{firewallPolicy}}".
+	External string `json:"external,omitempty"`
+
+	// The name of a ComputeFirewallPolicy resource.
+	Name string `json:"name,omitempty"`
+
+	// The namespace of a ComputeFirewallPolicy resource.
+	Namespace string `json:"namespace,omitempty"`
+}
+
+// NormalizedExternal provision the "External" value for other resource that depends on ComputeFirewallPolicy.
+// If the "External" is given in the other resource's spec.ComputeFirewallPolicyRef, the given value will be used.
+// Otherwise, the "Name" and "Namespace" will be used to query the actual ComputeFirewallPolicy object from the cluster.
+func (r *ComputeFirewallPolicyRef) 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", ComputeFirewallPolicyGVK.Kind)
+	}
+	// From given External
+	if r.External != "" {
+		if _, err := parseComputeFirewallPolicyExternal(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(ComputeFirewallPolicyGVK)
+	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", ComputeFirewallPolicyGVK, key, err)
+	}
+
+	// Get external from status.externalRef. This is the most trustworthy place.
+	actualExternalRef, found, err := unstructured.NestedString(u.Object, "status", "externalRef")
+	if err != nil {
+		return "", fmt.Errorf("error getting status.externalRef for %s %s/%s: %w", u.GetKind(), u.GetNamespace(), u.GetName(), err)
+	}
+
+	// If status.externalRef does not exist, it's created by legacy controller. Get values from target field.
+	if !found {
+		resourceID, _, err := unstructured.NestedString(u.Object, "spec", "resourceID")
+		if err != nil {
+			return "", fmt.Errorf("reading spec.resourceID from %v %v/%v: %w", u.GroupVersionKind().Kind, u.GetNamespace(), u.GetName(), err)
+		}
+		if resourceID == "" {
+			resourceID = u.GetName()
+		}
+		r.External = resourceID
+	} else {
+		r.External = actualExternalRef
+	}
+
+	return r.External, nil
+}
+
+func parseComputeFirewallPolicyExternal(external string) (firewallPolicy string, err error) {
+	tokens := strings.Split(external, "/")
+	if len(tokens) == 4 && tokens[0] == "locations" && tokens[1] == "global" && tokens[2] == "firewallPolicies" {
+		return tokens[3], nil
+	}
+	return "", fmt.Errorf("format of ComputeFirewallPolicy external=%q was not known (use locations/global/firewallPolicies/{{firewallPolicy}})", external)
+}

apis/compute/v1beta1/computefirewallpolicyrule_reference.go

@@ -34,16 +34,14 @@ var _ refsv1beta1.ExternalNormalizer = &ComputeFirewallPolicyRuleRef{}
 // holds the GCP identifier for the KRM object.
 type ComputeFirewallPolicyRuleRef struct {
 	// A reference to an externally managed ComputeFirewallPolicyRule resource.
-	// Should be in the format "locations/global/firewallPolicies/<firewallPolicy>/rules/<priority>".
+	// Should be in the format "locations/global/firewallPolicies/{{firewallPolicy}}/rules/{{priority}}".
 	External string `json:"external,omitempty"`
 
 	// The name of a ComputeFirewallPolicyRule resource.
 	Name string `json:"name,omitempty"`
 
 	// The namespace of a ComputeFirewallPolicyRule resource.
 	Namespace string `json:"namespace,omitempty"`
-
-	parent *ComputeFirewallPolicyRuleParent
 }
 
 // NormalizedExternal provision the "External" value for other resource that depends on ComputeFirewallPolicyRule.
@@ -75,98 +73,82 @@ func (r *ComputeFirewallPolicyRuleRef) NormalizedExternal(ctx context.Context, r
 		return "", fmt.Errorf("reading referenced %s %s: %w", ComputeFirewallPolicyRuleGVK, key, err)
 	}
 	// Get external from status.externalRef. This is the most trustworthy place.
-	actualExternalRef, _, err := unstructured.NestedString(u.Object, "status", "externalRef")
+	actualExternalRef, found, err := unstructured.NestedString(u.Object, "status", "externalRef")
 	if err != nil {
-		return "", fmt.Errorf("reading status.externalRef: %w", err)
+		return "", fmt.Errorf("error getting status.externalRef for %s %s/%s: %w", u.GetKind(), u.GetNamespace(), u.GetName(), err)
 	}
-	if actualExternalRef == "" {
-		return "", fmt.Errorf("ComputeFirewallPolicyRule is not ready yet")
+
+	// If status.externalRef does not exist, it's created by legacy controller. Get values from target field.
+	if !found {
+		resourceID, _, err := unstructured.NestedString(u.Object, "spec", "resourceID")
+		if err != nil {
+			return "", fmt.Errorf("reading spec.resourceID from %v %v/%v: %w", u.GroupVersionKind().Kind, u.GetNamespace(), u.GetName(), err)
+		}
+		if resourceID == "" {
+			resourceID = u.GetName()
+		}
+		r.External = resourceID
+	} else {
+		r.External = actualExternalRef
 	}
-	r.External = actualExternalRef
 	return r.External, nil
 }
 
 // New builds a NewComputeFirewallPolicyRuleRef from the Config Connector ComputeFirewallPolicyRule object.
 func NewComputeFirewallPolicyRuleRef(ctx context.Context, reader client.Reader, obj *ComputeFirewallPolicyRule) (*ComputeFirewallPolicyRuleRef, error) {
-	id := &ComputeFirewallPolicyRuleRef{}
+	ref := &ComputeFirewallPolicyRuleRef{}
 
-	firewallPolicyRef, err := refsv1beta1.ResolveComputeFirewallPolicy(ctx, reader, obj, obj.Spec.FirewallPolicyRef)
+	firewallPolicyRef := obj.Spec.FirewallPolicyRef
+	normalizedRef, err := firewallPolicyRef.NormalizedExternal(ctx, reader, obj.Namespace)
 	if err != nil {
 		return nil, err
 	}
-	firewallPolicy := firewallPolicyRef.External
+	firewallPolicy := normalizedRef
 	if firewallPolicy == "" {
 		return nil, fmt.Errorf("cannot resolve firewallPolicy")
 	}
 
-	id.parent = &ComputeFirewallPolicyRuleParent{FirewallPolicy: firewallPolicy}
-
 	// Get priority. Priority is a required field
 	priority := obj.Spec.Priority
 
 	// Use approved External
 	externalRef := valueOf(obj.Status.ExternalRef)
 	if externalRef == "" {
-		id.External = asComputeFirewallPolicyRuleExternal(id.parent, priority)
-		return id, nil
+		ref.External = AsComputeFirewallPolicyRuleExternal(firewallPolicy, priority)
+		return ref, nil
 	}
 
 	// Validate desired with actual
-	actualParent, actualPriority, err := parseComputeFirewallPolicyRuleExternal(externalRef)
+	actualFirewallPolicy, actualPriority, err := parseComputeFirewallPolicyRuleExternal(externalRef)
 	if err != nil {
 		return nil, err
 	}
-	if actualParent.FirewallPolicy != firewallPolicy {
-		return nil, fmt.Errorf("spec.firewallPolicyRef changed, expect %s, got %s", actualParent.FirewallPolicy, firewallPolicy)
+	if actualFirewallPolicy != firewallPolicy {
+		return nil, fmt.Errorf("spec.firewallPolicyRef changed, expect %s, got %s", actualFirewallPolicy, firewallPolicy)
 	}
 	if actualPriority != priority {
 		return nil, fmt.Errorf("cannot reset `spec.priority` to %d, since it has already assigned to %d",
 			priority, actualPriority)
 	}
-	id.External = externalRef
-	id.parent = &ComputeFirewallPolicyRuleParent{FirewallPolicy: firewallPolicy}
-	return id, nil
-}
-
-func (r *ComputeFirewallPolicyRuleRef) Parent() (*ComputeFirewallPolicyRuleParent, error) {
-	if r.parent != nil {
-		return r.parent, nil
-	}
-	if r.External != "" {
-		parent, _, err := parseComputeFirewallPolicyRuleExternal(r.External)
-		if err != nil {
-			return nil, err
-		}
-		return parent, nil
-	}
-	return nil, fmt.Errorf("ComputeFirewallPolicyRule not initialized from `NewComputeFirewallPolicyRuleRef` or `NormalizedExternal`")
+	ref.External = externalRef
+	return ref, nil
 }
 
-type ComputeFirewallPolicyRuleParent struct {
-	FirewallPolicy string
-}
-
-func (p *ComputeFirewallPolicyRuleParent) String() string {
-	return "locations/global/firewallPolicies/" + p.FirewallPolicy
-}
-
-func asComputeFirewallPolicyRuleExternal(parent *ComputeFirewallPolicyRuleParent, priority int64) (external string) {
+func AsComputeFirewallPolicyRuleExternal(firewallPolicy string, priority int64) (external string) {
 	p := strconv.Itoa(int(priority))
-	return parent.String() + "/rules/" + p
+	return "locations/global/firewallPolicies/" + firewallPolicy + "/rules/" + p
 }
 
-func parseComputeFirewallPolicyRuleExternal(external string) (parent *ComputeFirewallPolicyRuleParent, priority int64, err error) {
+func parseComputeFirewallPolicyRuleExternal(external string) (firewallPolicy string, priority int64, err error) {
 	tokens := strings.Split(external, "/")
-	if len(tokens) != 6 || tokens[0] != "locations" || tokens[2] != "firewallPolicies" || tokens[4] != "rules" {
-		return nil, -1, fmt.Errorf("format of ComputeFirewallPolicyRule external=%q was not known (use firewallPolicies/<firewallPolicy>/rules/<priority>)", external)
-	}
-	parent = &ComputeFirewallPolicyRuleParent{
-		FirewallPolicy: tokens[3],
+	if len(tokens) != 6 || tokens[0] != "locations" || tokens[1] != "global" || tokens[2] != "firewallPolicies" || tokens[4] != "rules" {
+		return "", -1, fmt.Errorf("format of ComputeFirewallPolicyRule external=%q was not known (use location/global/firewallPolicies/{{firewallPolicy}}/rules/{{priority}})", external)
 	}
+	firewallPolicy = tokens[3]
 	p, err := strconv.ParseInt(tokens[5], 10, 32)
 	if err != nil {
-		return nil, -1, fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule external=%q to an integer: %w", tokens[5], external, err)
+		return "", -1, fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule external=%q to an integer: %w", tokens[5], external, err)
 	}
 	priority = p
-	return parent, priority, nil
+	return firewallPolicy, priority, nil
 }

apis/compute/v1beta1/firewallpolicyrule_types.go

@@ -103,7 +103,7 @@ type ComputeFirewallPolicyRuleSpec struct {
 	EnableLogging *bool `json:"enableLogging,omitempty"`
 
 	/* Immutable. */
-	FirewallPolicyRef *refs.ComputeFirewallPolicyRef `json:"firewallPolicyRef"`
+	FirewallPolicyRef *ComputeFirewallPolicyRef `json:"firewallPolicyRef"`
 
 	/* A match condition that incoming traffic is evaluated against. If it evaluates to true, the corresponding 'action' is enforced. */
 	Match *FirewallPolicyRuleMatch `json:"match"`

config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computefirewallpolicyrules.compute.cnrm.cloud.google.com.yaml

@@ -102,14 +102,13 @@ spec:
                 properties:
                   external:
                     description: A reference to an externally managed ComputeFirewallPolicy
-                      resource. Should be in the format `locations/global/firewallPolicies/{{firewallPolicyID}}`.
+                      resource. Should be in the format "locations/global/firewallPolicies/{{firewallPolicy}}".
                     type: string
                   name:
-                    description: The `name` field of a `ComputeFirewallPolicy` resource.
+                    description: The name of a ComputeFirewallPolicy resource.
                     type: string
                   namespace:
-                    description: The `namespace` field of a `ComputeFirewallPolicy`
-                      resource.
+                    description: The namespace of a ComputeFirewallPolicy resource.
                     type: string
                 type: object
               match: