diff --git a/apis/compute/v1beta1/computefirewallpolicyrule_reference.go b/apis/compute/v1beta1/computefirewallpolicyrule_reference.go deleted file mode 100644 index 014a9abb43..0000000000 --- a/apis/compute/v1beta1/computefirewallpolicyrule_reference.go +++ /dev/null @@ -1,172 +0,0 @@ -// 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" - "strconv" - "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 = &ComputeFirewallPolicyRuleRef{} - -// ComputeFirewallPolicyRuleRef defines the resource reference to ComputeFirewallPolicyRule, which "External" field -// 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}}". - 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. -// If the "External" is given in the other resource's spec.ComputeFirewallPolicyRuleRef, the given value will be used. -// Otherwise, the "Name" and "Namespace" will be used to query the actual ComputeFirewallPolicyRule object from the cluster. -func (r *ComputeFirewallPolicyRuleRef) 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", ComputeFirewallPolicyRuleGVK.Kind) - } - // From given External - if r.External != "" { - if _, _, err := parseComputeFirewallPolicyRuleExternal(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(ComputeFirewallPolicyRuleGVK) - 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", ComputeFirewallPolicyRuleGVK, 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 == "" { - return "", fmt.Errorf("ComputeFirewallPolicyRule is not ready yet") - } - 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{} - - firewallPolicyRef, err := refsv1beta1.ResolveComputeFirewallPolicy(ctx, reader, obj, obj.Spec.FirewallPolicyRef) - if err != nil { - return nil, err - } - firewallPolicy := firewallPolicyRef.External - 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 - } - - // Validate desired with actual - actualParent, 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 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`") -} - -type ComputeFirewallPolicyRuleParent struct { - FirewallPolicy string -} - -func (p *ComputeFirewallPolicyRuleParent) String() string { - return "locations/global/firewallPolicies/" + p.FirewallPolicy -} - -func asComputeFirewallPolicyRuleExternal(parent *ComputeFirewallPolicyRuleParent, priority int64) (external string) { - p := strconv.Itoa(int(priority)) - return parent.String() + "/rules/" + p -} - -func parseComputeFirewallPolicyRuleExternal(external string) (parent *ComputeFirewallPolicyRuleParent, 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], - } - 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) - } - priority = p - return parent, priority, nil -} diff --git a/apis/compute/v1beta1/firewallpolicyrule_identity.go b/apis/compute/v1beta1/firewallpolicyrule_identity.go new file mode 100644 index 0000000000..04b73a17ba --- /dev/null +++ b/apis/compute/v1beta1/firewallpolicyrule_identity.go @@ -0,0 +1,87 @@ +// 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" + "strconv" + + "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" +) + +type FirewallPolicyRuleIdentity struct { + id int64 + parent *FirewallPolicyRuleParent +} + +func (i *FirewallPolicyRuleIdentity) String() string { + p := strconv.Itoa(int(i.id)) + return i.parent.String() + "/rules/" + p +} + +func (i *FirewallPolicyRuleIdentity) Parent() *FirewallPolicyRuleParent { + return i.parent +} + +func (i *FirewallPolicyRuleIdentity) ID() int64 { + return i.id +} + +type FirewallPolicyRuleParent struct { + FirewallPolicy string +} + +func (p *FirewallPolicyRuleParent) String() string { + return "locations/global/firewallPolicies/" + p.FirewallPolicy +} + +func NewSecretIdentity(ctx context.Context, reader client.Reader, obj *ComputeFirewallPolicyRule) (*FirewallPolicyRuleIdentity, error) { + //Get parent + firewallPolicyRef, err := refsv1beta1.ResolveComputeFirewallPolicy(ctx, reader, obj, obj.Spec.FirewallPolicyRef) + if err != nil { + return nil, err + } + firewallPolicy := firewallPolicyRef.External + if firewallPolicy == "" { + return nil, fmt.Errorf("cannot resolve firewallPolicy") + } + + // Get priority. Priority is a required field + priority := obj.Spec.Priority + + // Use approved External + externalRef := common.ValueOf(obj.Status.ExternalRef) + if externalRef != "" { + actualIdentity, err := parseFirewallPolicyRuleExternal(externalRef) + if err != nil { + return nil, err + } + if actualIdentity.parent.FirewallPolicy != firewallPolicy { + return nil, fmt.Errorf("spec.FirewallPolicyRef changed, expect %s, got %s", actualIdentity.parent.FirewallPolicy, firewallPolicy) + } + if actualIdentity.id != priority { + return nil, fmt.Errorf("cannot reset `spec.priority` to %d, since it has already assigned to %d", + priority, actualIdentity.id) + } + } + + return &FirewallPolicyRuleIdentity{ + parent: &FirewallPolicyRuleParent{FirewallPolicy: firewallPolicy}, + id: priority, + }, nil +} diff --git a/apis/compute/v1beta1/firewallpolicyrule_reference.go b/apis/compute/v1beta1/firewallpolicyrule_reference.go new file mode 100644 index 0000000000..8fad7a909e --- /dev/null +++ b/apis/compute/v1beta1/firewallpolicyrule_reference.go @@ -0,0 +1,98 @@ +// 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" + "strconv" + "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 = &FirewallPolicyRuleRef{} + +// FirewallPolicyRuleRef defines the resource reference to ComputeFirewallPolicyRule, which "External" field +// holds the GCP identifier for the KRM object. +type FirewallPolicyRuleRef struct { + // A reference to an externally managed ComputeFirewallPolicyRule resource. + // 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"` +} + +// NormalizedExternal provision the "External" value for other resource that depends on ComputeFirewallPolicyRule. +// If the "External" is given in the other resource's spec.ComputeFirewallPolicyRuleRef, the given value will be used. +// Otherwise, the "Name" and "Namespace" will be used to query the actual ComputeFirewallPolicyRule object from the cluster. +func (r *FirewallPolicyRuleRef) 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", ComputeFirewallPolicyRuleGVK.Kind) + } + // From given External + if r.External != "" { + if _, err := parseFirewallPolicyRuleExternal(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(ComputeFirewallPolicyRuleGVK) + 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", ComputeFirewallPolicyRuleGVK, 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 == "" { + return "", fmt.Errorf("ComputeFirewallPolicyRule is not ready yet") + } + r.External = actualExternalRef + return r.External, nil +} + +func parseFirewallPolicyRuleExternal(external string) (*FirewallPolicyRuleIdentity, error) { + tokens := strings.Split(external, "/") + if len(tokens) != 6 || tokens[0] != "locations" || tokens[2] != "firewallPolicies" || tokens[4] != "rules" { + return nil, fmt.Errorf("format of ComputeFirewallPolicyRule external=%q was not known (use firewallPolicies/{{firewallPolicy}}/rules/{{priority}})", external) + } + id := &FirewallPolicyRuleIdentity{} + _, err := strconv.ParseInt(tokens[5], 10, 32) + if err != nil { + return nil, fmt.Errorf("error convert priority %s of ComputeFirewallPolicyRule external=%q to an integer: %w", tokens[5], external, err) + } + return id, nil +} diff --git a/apis/compute/v1beta1/zz_generated.deepcopy.go b/apis/compute/v1beta1/zz_generated.deepcopy.go index ca24003cfc..4a610a2381 100644 --- a/apis/compute/v1beta1/zz_generated.deepcopy.go +++ b/apis/compute/v1beta1/zz_generated.deepcopy.go @@ -83,41 +83,6 @@ 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. -func (in *ComputeFirewallPolicyRuleRef) DeepCopy() *ComputeFirewallPolicyRuleRef { - if in == nil { - return nil - } - out := new(ComputeFirewallPolicyRuleRef) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ComputeFirewallPolicyRuleSpec) DeepCopyInto(out *ComputeFirewallPolicyRuleSpec) { *out = *in @@ -644,6 +609,26 @@ func (in *ComputeTargetTCPProxyStatus) DeepCopy() *ComputeTargetTCPProxyStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallPolicyRuleIdentity) DeepCopyInto(out *FirewallPolicyRuleIdentity) { + *out = *in + if in.parent != nil { + in, out := &in.parent, &out.parent + *out = new(FirewallPolicyRuleParent) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallPolicyRuleIdentity. +func (in *FirewallPolicyRuleIdentity) DeepCopy() *FirewallPolicyRuleIdentity { + if in == nil { + return nil + } + out := new(FirewallPolicyRuleIdentity) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FirewallPolicyRuleLayer4Configs) DeepCopyInto(out *FirewallPolicyRuleLayer4Configs) { *out = *in @@ -736,6 +721,36 @@ func (in *FirewallPolicyRuleMatch) DeepCopy() *FirewallPolicyRuleMatch { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallPolicyRuleParent) DeepCopyInto(out *FirewallPolicyRuleParent) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallPolicyRuleParent. +func (in *FirewallPolicyRuleParent) DeepCopy() *FirewallPolicyRuleParent { + if in == nil { + return nil + } + out := new(FirewallPolicyRuleParent) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FirewallPolicyRuleRef) DeepCopyInto(out *FirewallPolicyRuleRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FirewallPolicyRuleRef. +func (in *FirewallPolicyRuleRef) DeepCopy() *FirewallPolicyRuleRef { + if in == nil { + return nil + } + out := new(FirewallPolicyRuleRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ForwardingruleFilterLabels) DeepCopyInto(out *ForwardingruleFilterLabels) { *out = *in diff --git a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go index 1cfea7f362..e02e42c98d 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 + id *krm.FirewallPolicyRuleIdentity firewallPoliciesClient *gcp.FirewallPoliciesClient desired *krm.ComputeFirewallPolicyRule actual *computepb.FirewallPolicyRule @@ -81,13 +81,13 @@ func (m *firewallPolicyRuleModel) AdapterForObject(ctx context.Context, reader c return nil, fmt.Errorf("error converting to %T: %w", obj, err) } - firewallPolicyRuleRef, err := krm.NewComputeFirewallPolicyRuleRef(ctx, reader, obj) + id, err := krm.NewSecretIdentity(ctx, reader, obj) if err != nil { return nil, err } firewallPolicyRuleAdapter := &firewallPolicyRuleAdapter{ - id: firewallPolicyRuleRef, + id: id, 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.id) 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.id, 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.id) mapCtx := &direct.MapContext{} @@ -143,44 +143,34 @@ 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) - } - req := &computepb.AddRuleFirewallPolicyRequest{ FirewallPolicyRuleResource: firewallPolicyRule, - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: a.id.Parent().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.id, 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.id, err) } } - log.V(2).Info("successfully created ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("successfully created ComputeFirewallPolicyRule", "name", a.id) // 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.id, 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 := a.id.Parent().String() + "/rules/" + priority status.ExternalRef = &externalRef return createOp.UpdateStatus(ctx, status, nil) } @@ -194,7 +184,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.id) mapCtx := &direct.MapContext{} desired := a.desired.DeepCopy() @@ -208,39 +198,34 @@ 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.id.String(), "/") 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.id, err) } updateReq := &computepb.PatchRuleFirewallPolicyRequest{ FirewallPolicyRuleResource: firewallPolicyRule, - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: a.id.Parent().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.id, 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.id, err) } } - log.V(2).Info("successfully updated ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("successfully updated ComputeFirewallPolicyRule", "name", a.id) // 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.id, err) } status := &krm.ComputeFirewallPolicyRuleStatus{} @@ -250,7 +235,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.id) } mc := &direct.MapContext{} @@ -273,34 +258,29 @@ 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.id) - tokens := strings.Split(a.id.External, "/") + tokens := strings.Split(a.id.String(), "/") 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.id, err) } delReq := &computepb.RemoveRuleFirewallPolicyRequest{ - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: a.id.Parent().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.id, 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.id, err) } } - log.V(2).Info("successfully deleted ComputeFirewallPolicyRule", "name", a.id.External) + log.V(2).Info("successfully deleted ComputeFirewallPolicyRule", "name", a.id) // Get the deleted rules _, err = a.get(ctx) @@ -311,20 +291,15 @@ 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.id.String(), "/") 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.id, err) } getReq := &computepb.GetRuleFirewallPolicyRequest{ - FirewallPolicy: parent.FirewallPolicy, + FirewallPolicy: a.id.Parent().FirewallPolicy, Priority: direct.PtrTo(int32(priority)), } return a.firewallPoliciesClient.GetRule(ctx, getReq)