diff --git a/apis/compute/v1beta1/computefirewallpolicy_reference.go b/apis/compute/v1beta1/computefirewallpolicy_reference.go new file mode 100644 index 0000000000..ab8e519cd1 --- /dev/null +++ b/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) +} diff --git a/apis/compute/v1beta1/computefirewallpolicyrule_reference.go b/apis/compute/v1beta1/computefirewallpolicyrule_reference.go index e8e22de144..70dc4437c7 100644 --- a/apis/compute/v1beta1/computefirewallpolicyrule_reference.go +++ b/apis/compute/v1beta1/computefirewallpolicyrule_reference.go @@ -34,7 +34,7 @@ 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//rules/". + // Should be in the format "locations/global/firewallPolicies/{{firewallPolicy}}/rules/{{priority}}". External string `json:"external,omitempty"` // The name of a ComputeFirewallPolicyRule resource. @@ -42,8 +42,6 @@ type ComputeFirewallPolicyRuleRef struct { // 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//rules/)", 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 } diff --git a/apis/compute/v1beta1/firewallpolicyrule_types.go b/apis/compute/v1beta1/firewallpolicyrule_types.go index f546f0f18f..c033583621 100644 --- a/apis/compute/v1beta1/firewallpolicyrule_types.go +++ b/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"` diff --git a/apis/compute/v1beta1/zz_generated.deepcopy.go b/apis/compute/v1beta1/zz_generated.deepcopy.go index ca24003cfc..14714f34a5 100644 --- a/apis/compute/v1beta1/zz_generated.deepcopy.go +++ b/apis/compute/v1beta1/zz_generated.deepcopy.go @@ -24,6 +24,21 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ComputeFirewallPolicyRef) DeepCopyInto(out *ComputeFirewallPolicyRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComputeFirewallPolicyRef. +func (in *ComputeFirewallPolicyRef) DeepCopy() *ComputeFirewallPolicyRef { + if in == nil { + return nil + } + out := new(ComputeFirewallPolicyRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ComputeFirewallPolicyRule) DeepCopyInto(out *ComputeFirewallPolicyRule) { *out = *in @@ -83,29 +98,9 @@ func (in *ComputeFirewallPolicyRuleList) DeepCopyObject() runtime.Object { return nil } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ComputeFirewallPolicyRuleParent) DeepCopyInto(out *ComputeFirewallPolicyRuleParent) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComputeFirewallPolicyRuleParent. -func (in *ComputeFirewallPolicyRuleParent) DeepCopy() *ComputeFirewallPolicyRuleParent { - if in == nil { - return nil - } - out := new(ComputeFirewallPolicyRuleParent) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ComputeFirewallPolicyRuleRef) DeepCopyInto(out *ComputeFirewallPolicyRuleRef) { *out = *in - if in.parent != nil { - in, out := &in.parent, &out.parent - *out = new(ComputeFirewallPolicyRuleParent) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComputeFirewallPolicyRuleRef. @@ -138,7 +133,7 @@ func (in *ComputeFirewallPolicyRuleSpec) DeepCopyInto(out *ComputeFirewallPolicy } if in.FirewallPolicyRef != nil { in, out := &in.FirewallPolicyRef, &out.FirewallPolicyRef - *out = new(refsv1beta1.ComputeFirewallPolicyRef) + *out = new(ComputeFirewallPolicyRef) **out = **in } if in.Match != nil { diff --git a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computefirewallpolicyrules.compute.cnrm.cloud.google.com.yaml b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computefirewallpolicyrules.compute.cnrm.cloud.google.com.yaml index 9a47b7d509..01d367cb46 100644 --- a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computefirewallpolicyrules.compute.cnrm.cloud.google.com.yaml +++ b/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: diff --git a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go index 1cfea7f362..74c7b98315 100644 --- a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go +++ b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go @@ -53,7 +53,7 @@ type firewallPolicyRuleModel struct { var _ directbase.Model = &firewallPolicyRuleModel{} type firewallPolicyRuleAdapter struct { - id *krm.ComputeFirewallPolicyRuleRef + ref *krm.ComputeFirewallPolicyRuleRef firewallPoliciesClient *gcp.FirewallPoliciesClient desired *krm.ComputeFirewallPolicyRule actual *computepb.FirewallPolicyRule @@ -87,7 +87,7 @@ func (m *firewallPolicyRuleModel) AdapterForObject(ctx context.Context, reader c } firewallPolicyRuleAdapter := &firewallPolicyRuleAdapter{ - id: firewallPolicyRuleRef, + ref: firewallPolicyRuleRef, desired: obj, reader: reader, } @@ -109,7 +109,7 @@ func (m *firewallPolicyRuleModel) AdapterForURL(ctx context.Context, url string) func (a *firewallPolicyRuleAdapter) Find(ctx context.Context) (bool, error) { log := klog.FromContext(ctx) - log.V(2).Info("getting ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("getting ComputeFirewallPolicyRule", "name", a.ref.External) firewallPolicyRule, err := a.get(ctx) if err != nil { @@ -119,7 +119,7 @@ func (a *firewallPolicyRuleAdapter) Find(ctx context.Context) (bool, error) { if direct.IsBadRequest(err) { return false, nil } - return false, fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.id.External, err) + return false, fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.ref.External, err) } a.actual = firewallPolicyRule return true, nil @@ -132,7 +132,7 @@ func (a *firewallPolicyRuleAdapter) Create(ctx context.Context, createOp *direct } log := klog.FromContext(ctx) - log.V(2).Info("creating ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("creating ComputeFirewallPolicyRule", "name", a.ref.External) mapCtx := &direct.MapContext{} @@ -143,44 +143,36 @@ func (a *firewallPolicyRuleAdapter) Create(ctx context.Context, createOp *direct return mapCtx.Err() } - parent, err := a.id.Parent() - if err != nil { - return fmt.Errorf("get ComputeFirewallPolicyRule parent %s: %w", a.id.External, err) - } + tokens := strings.Split(a.ref.External, "/") + firewallPolicy := tokens[3] req := &computepb.AddRuleFirewallPolicyRequest{ FirewallPolicyRuleResource: firewallPolicyRule, - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: firewallPolicy, } op, err := a.firewallPoliciesClient.AddRule(ctx, req) if err != nil { - return fmt.Errorf("creating ComputeFirewallPolicyRule %s: %w", a.id.External, err) + return fmt.Errorf("creating ComputeFirewallPolicyRule %s: %w", a.ref.External, err) } if !op.Done() { err = op.Wait(ctx) if err != nil { - return fmt.Errorf("waiting ComputeFirewallPolicyRule %s create failed: %w", a.id.External, err) + return fmt.Errorf("waiting ComputeFirewallPolicyRule %s create failed: %w", a.ref.External, err) } } - log.V(2).Info("successfully created ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("successfully created ComputeFirewallPolicyRule", "name", a.ref.External) // Get the created resource created, err := a.get(ctx) if err != nil { - return fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.id.External, err) + return fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.ref.External, err) } status := &krm.ComputeFirewallPolicyRuleStatus{} status = ComputeFirewallPolicyRuleStatus_FromProto(mapCtx, created) - parent, err = a.id.Parent() - if err != nil { - return err - } - - priority := strconv.Itoa(int(*created.Priority)) - externalRef := parent.String() + "/rules/" + priority + externalRef := krm.AsComputeFirewallPolicyRuleExternal(firewallPolicy, int64(*created.Priority)) status.ExternalRef = &externalRef return createOp.UpdateStatus(ctx, status, nil) } @@ -194,7 +186,7 @@ func (a *firewallPolicyRuleAdapter) Update(ctx context.Context, updateOp *direct } log := klog.FromContext(ctx) - log.V(2).Info("updating ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("updating ComputeFirewallPolicyRule", "name", a.ref.External) mapCtx := &direct.MapContext{} desired := a.desired.DeepCopy() @@ -208,39 +200,35 @@ func (a *firewallPolicyRuleAdapter) Update(ctx context.Context, updateOp *direct updated := &computepb.FirewallPolicyRule{} - parent, err := a.id.Parent() - if err != nil { - return fmt.Errorf("get ComputeFirewallPolicyRule parent %s: %w", a.id.External, err) - } - - tokens := strings.Split(a.id.External, "/") + tokens := strings.Split(a.ref.External, "/") + firewallPolicy := tokens[3] priority, err := strconv.ParseInt(tokens[5], 10, 32) // Should not hit this error because we have verified priority in parseComputeFirewallPolicyRuleExternal` if err != nil { - return fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule %s to an integer: %w", tokens[5], a.id.External, err) + return fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule %s to an integer: %w", tokens[5], a.ref.External, err) } updateReq := &computepb.PatchRuleFirewallPolicyRequest{ FirewallPolicyRuleResource: firewallPolicyRule, - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: firewallPolicy, Priority: direct.PtrTo(int32(priority)), } op, err := a.firewallPoliciesClient.PatchRule(ctx, updateReq) if err != nil { - return fmt.Errorf("updating ComputeFirewallPolicyRule %s: %w", a.id.External, err) + return fmt.Errorf("updating ComputeFirewallPolicyRule %s: %w", a.ref.External, err) } if !op.Done() { err = op.Wait(ctx) if err != nil { - return fmt.Errorf("waiting ComputeFirewallPolicyRule %s update failed: %w", a.id.External, err) + return fmt.Errorf("waiting ComputeFirewallPolicyRule %s update failed: %w", a.ref.External, err) } } - log.V(2).Info("successfully updated ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("successfully updated ComputeFirewallPolicyRule", "name", a.ref.External) // Get the updated resource updated, err = a.get(ctx) if err != nil { - return fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.id.External, err) + return fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.ref.External, err) } status := &krm.ComputeFirewallPolicyRuleStatus{} @@ -250,7 +238,7 @@ func (a *firewallPolicyRuleAdapter) Update(ctx context.Context, updateOp *direct func (a *firewallPolicyRuleAdapter) Export(ctx context.Context) (*unstructured.Unstructured, error) { if a.actual == nil { - return nil, fmt.Errorf("firewallPolicyRule %s not found", a.id.External) + return nil, fmt.Errorf("firewallPolicyRule %s not found", a.ref.External) } mc := &direct.MapContext{} @@ -273,34 +261,30 @@ func (a *firewallPolicyRuleAdapter) Export(ctx context.Context) (*unstructured.U // Delete implements the Adapter interface. func (a *firewallPolicyRuleAdapter) Delete(ctx context.Context, deleteOp *directbase.DeleteOperation) (bool, error) { log := klog.FromContext(ctx) - log.V(2).Info("deleting ComputeFirewallPolicyRule", "name", a.id.External) - - parent, err := a.id.Parent() - if err != nil { - return false, fmt.Errorf("get ComputeFirewallPolicyRule parent %s: %w", a.id.External, err) - } + log.V(2).Info("deleting ComputeFirewallPolicyRule", "name", a.ref.External) - tokens := strings.Split(a.id.External, "/") + tokens := strings.Split(a.ref.External, "/") + firewallPolicy := tokens[3] priority, err := strconv.ParseInt(tokens[5], 10, 32) // Should not hit this error because we have verified priority in parseComputeFirewallPolicyRuleExternal` if err != nil { - return false, fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule %s to an integer: %w", tokens[5], a.id.External, err) + return false, fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule %s to an integer: %w", tokens[5], a.ref.External, err) } delReq := &computepb.RemoveRuleFirewallPolicyRequest{ - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: firewallPolicy, Priority: direct.PtrTo(int32(priority)), } op, err := a.firewallPoliciesClient.RemoveRule(ctx, delReq) if err != nil { - return false, fmt.Errorf("deleting ComputeFirewallPolicyRule %s: %w", a.id.External, err) + return false, fmt.Errorf("deleting ComputeFirewallPolicyRule %s: %w", a.ref.External, err) } if !op.Done() { err = op.Wait(ctx) if err != nil { - return false, fmt.Errorf("waiting ComputeFirewallPolicyRule %s delete failed: %w", a.id.External, err) + return false, fmt.Errorf("waiting ComputeFirewallPolicyRule %s delete failed: %w", a.ref.External, err) } } - log.V(2).Info("successfully deleted ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("successfully deleted ComputeFirewallPolicyRule", "name", a.ref.External) // Get the deleted rules _, err = a.get(ctx) @@ -311,20 +295,16 @@ func (a *firewallPolicyRuleAdapter) Delete(ctx context.Context, deleteOp *direct } func (a *firewallPolicyRuleAdapter) get(ctx context.Context) (*computepb.FirewallPolicyRule, error) { - parent, err := a.id.Parent() - if err != nil { - return nil, fmt.Errorf("get ComputeFirewallPolicyRule parent %s: %w", a.id.External, err) - } - - tokens := strings.Split(a.id.External, "/") + tokens := strings.Split(a.ref.External, "/") + firewallPolicy := tokens[3] priority, err := strconv.ParseInt(tokens[5], 10, 32) // Should not hit this error because we have verified priority in parseComputeFirewallPolicyRuleExternal` if err != nil { - return nil, fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule %s to an integer: %w", tokens[5], a.id.External, err) + return nil, fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule %s to an integer: %w", tokens[5], a.ref.External, err) } getReq := &computepb.GetRuleFirewallPolicyRequest{ - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: firewallPolicy, Priority: direct.PtrTo(int32(priority)), } return a.firewallPoliciesClient.GetRule(ctx, getReq) diff --git a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computefirewallpolicyrule.md b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computefirewallpolicyrule.md index f97f41fd68..f300769a56 100644 --- a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computefirewallpolicyrule.md +++ b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computefirewallpolicyrule.md @@ -181,7 +181,7 @@ targetServiceAccounts:

string

-

{% verbatim %}A reference to an externally managed ComputeFirewallPolicy resource. Should be in the format `locations/global/firewallPolicies/{{firewallPolicyID}}`.{% endverbatim %}

+

{% verbatim %}A reference to an externally managed ComputeFirewallPolicy resource. Should be in the format "locations/global/firewallPolicies/{{firewallPolicy}}".{% endverbatim %}

@@ -191,7 +191,7 @@ targetServiceAccounts:

string

-

{% verbatim %}The `name` field of a `ComputeFirewallPolicy` resource.{% endverbatim %}

+

{% verbatim %}The name of a ComputeFirewallPolicy resource.{% endverbatim %}

@@ -201,7 +201,7 @@ targetServiceAccounts:

string

-

{% verbatim %}The `namespace` field of a `ComputeFirewallPolicy` resource.{% endverbatim %}

+

{% verbatim %}The namespace of a ComputeFirewallPolicy resource.{% endverbatim %}