diff --git a/apis/compute/v1beta1/computefirewallpolicyrule_reference.go b/apis/compute/v1beta1/computefirewallpolicyrule_reference.go new file mode 100644 index 0000000000..42a96b36ab --- /dev/null +++ b/apis/compute/v1beta1/computefirewallpolicyrule_reference.go @@ -0,0 +1,174 @@ +// 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 = &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//rules/". + 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 := fmt.Sprintf("%v", 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 %s, since it has already assigned to %s", + 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 string) (external string) { + return parent.String() + "/rules/" + priority +} + +func parseComputeFirewallPolicyRuleExternal(external string) (parent *ComputeFirewallPolicyRuleParent, priority string, err 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//rules/)", external) + } + parent = &ComputeFirewallPolicyRuleParent{ + FirewallPolicy: tokens[3], + } + priority = tokens[5] + return parent, priority, nil +} + +func valueOf[T any](t *T) T { + var zeroVal T + if t == nil { + return zeroVal + } + return *t +} diff --git a/apis/compute/v1beta1/zz_generated.deepcopy.go b/apis/compute/v1beta1/zz_generated.deepcopy.go index 4e08b179a3..ca24003cfc 100644 --- a/apis/compute/v1beta1/zz_generated.deepcopy.go +++ b/apis/compute/v1beta1/zz_generated.deepcopy.go @@ -83,6 +83,41 @@ 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 diff --git a/apis/refs/v1beta1/computerefs.go b/apis/refs/v1beta1/computerefs.go index 9297361124..a946e2284e 100644 --- a/apis/refs/v1beta1/computerefs.go +++ b/apis/refs/v1beta1/computerefs.go @@ -281,3 +281,49 @@ type ComputeFirewallPolicyRef struct { /* The `namespace` field of a `ComputeFirewallPolicy ` resource. */ Namespace string `json:"namespace,omitempty"` } + +func ResolveComputeFirewallPolicy(ctx context.Context, reader client.Reader, src client.Object, ref *ComputeFirewallPolicyRef) (*ComputeFirewallPolicyRef, error) { + if ref == nil { + return nil, nil + } + + if ref.External != "" { + if ref.Name != "" { + return nil, fmt.Errorf("cannot specify both name and external on reference") + } + return ref, nil + } + + if ref.Name == "" { + return nil, fmt.Errorf("must specify either name or external on reference") + } + + key := types.NamespacedName{ + Namespace: ref.Namespace, + Name: ref.Name, + } + if key.Namespace == "" { + key.Namespace = src.GetNamespace() + } + + computeFirewallPolicy := &unstructured.Unstructured{} + computeFirewallPolicy.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "compute.cnrm.cloud.google.com", + Version: "v1beta1", + Kind: "ComputeFirewallPolicy", + }) + if err := reader.Get(ctx, key, computeFirewallPolicy); err != nil { + if apierrors.IsNotFound(err) { + return nil, k8s.NewReferenceNotFoundError(computeFirewallPolicy.GroupVersionKind(), key) + } + return nil, fmt.Errorf("error reading referenced ComputeFirewallPolicy %v: %w", key, err) + } + + resourceID, err := GetResourceID(computeFirewallPolicy) + if err != nil { + return nil, err + } + + return &ComputeFirewallPolicyRef{ + External: fmt.Sprintf("%s", resourceID)}, nil +} diff --git a/pkg/controller/direct/compute/firewallpolicyrule/client.go b/pkg/controller/direct/compute/firewallpolicyrule/client.go deleted file mode 100644 index cbcc0207ae..0000000000 --- a/pkg/controller/direct/compute/firewallpolicyrule/client.go +++ /dev/null @@ -1,88 +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 firewallpolicyrule - -import ( - "context" - "fmt" - "net/http" - - api "cloud.google.com/go/compute/apiv1" - "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/config" - "google.golang.org/api/option" -) - -type gcpClient struct { - config config.ControllerConfig -} - -func newGCPClient(ctx context.Context, config *config.ControllerConfig) (*gcpClient, error) { - gcpClient := &gcpClient{ - config: *config, - } - return gcpClient, nil -} - -func (m *gcpClient) options() ([]option.ClientOption, error) { - var opts []option.ClientOption - if m.config.UserAgent != "" { - opts = append(opts, option.WithUserAgent(m.config.UserAgent)) - } - if m.config.HTTPClient != nil { - // TODO: Set UserAgent in this scenario (error is: WithHTTPClient is incompatible with gRPC dial options) - - httpClient := &http.Client{} - *httpClient = *m.config.HTTPClient - httpClient.Transport = &optionsRoundTripper{ - config: m.config, - inner: m.config.HTTPClient.Transport, - } - opts = append(opts, option.WithHTTPClient(httpClient)) - } - if m.config.UserProjectOverride && m.config.BillingProject != "" { - opts = append(opts, option.WithQuotaProject(m.config.BillingProject)) - } - - // TODO: support endpoints? - // if m.config.Endpoint != "" { - // opts = append(opts, option.WithEndpoint(m.config.Endpoint)) - // } - - return opts, nil -} - -type optionsRoundTripper struct { - config config.ControllerConfig - inner http.RoundTripper -} - -func (m *optionsRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { - if m.config.UserAgent != "" { - req.Header.Set("User-Agent", m.config.UserAgent) - } - return m.inner.RoundTrip(req) -} - -func (m *gcpClient) firewallPoliciesClient(ctx context.Context) (*api.FirewallPoliciesClient, error) { - opts, err := m.options() - if err != nil { - return nil, err - } - client, err := api.NewFirewallPoliciesRESTClient(ctx, opts...) - if err != nil { - return nil, fmt.Errorf("building FirewallPolicy client: %w", err) - } - return client, err -} diff --git a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go index d8efc515e9..a11c36c1de 100644 --- a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go +++ b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go @@ -17,6 +17,8 @@ package firewallpolicyrule import ( "context" "fmt" + "strconv" + "strings" "google.golang.org/api/option" @@ -51,8 +53,7 @@ type firewallPolicyRuleModel struct { var _ directbase.Model = &firewallPolicyRuleModel{} type firewallPolicyRuleAdapter struct { - firewallPolicy string - priority int64 + id *krm.ComputeFirewallPolicyRuleRef firewallPoliciesClient *gcp.FirewallPoliciesClient desired *krm.ComputeFirewallPolicyRule actual *computepb.FirewallPolicyRule @@ -83,23 +84,15 @@ func (m *firewallPolicyRuleModel) AdapterForObject(ctx context.Context, reader c // Set label managed-by-cnrm: true obj.ObjectMeta.Labels["managed-by-cnrm"] = "true" - // Get firewall policy - firewallPolicyRef, err := ResolveComputeFirewallPolicy(ctx, reader, obj, obj.Spec.FirewallPolicyRef) + firewallPolicyRuleRef, err := krm.NewComputeFirewallPolicyRuleRef(ctx, reader, obj) if err != nil { return nil, err - } - obj.Spec.FirewallPolicyRef.External = firewallPolicyRef.External - firewallPolicy := obj.Spec.FirewallPolicyRef.External - - // Get priority - priority := obj.Spec.Priority firewallPolicyRuleAdapter := &firewallPolicyRuleAdapter{ - firewallPolicy: firewallPolicy, - priority: priority, - desired: obj, - reader: reader, + id: firewallPolicyRuleRef, + desired: obj, + reader: reader, } // Get GCP client @@ -119,7 +112,7 @@ func (m *firewallPolicyRuleModel) AdapterForURL(ctx context.Context, url string) func (a *firewallPolicyRuleAdapter) Find(ctx context.Context) (bool, error) { log := klog.FromContext(ctx).WithName(ctrlName) - log.V(2).Info("getting ComputeFirewallPolicyRule", "priority", a.priority) + log.V(2).Info("getting ComputeFirewallPolicyRule", "name", a.id.External) firewallPolicyRule, err := a.get(ctx) if err != nil { @@ -129,7 +122,7 @@ func (a *firewallPolicyRuleAdapter) Find(ctx context.Context) (bool, error) { if direct.IsBadRequest(err) { return false, nil } - return false, fmt.Errorf("getting ComputeFirewallPolicyRule %d: %w", a.priority, err) + return false, fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.id.External, err) } a.actual = firewallPolicyRule return true, nil @@ -144,7 +137,7 @@ func (a *firewallPolicyRuleAdapter) Create(ctx context.Context, createOp *direct } log := klog.FromContext(ctx).WithName(ctrlName) - log.V(2).Info("creating ComputeFirewallPolicyRule", "priority", a.priority) + log.V(2).Info("creating ComputeFirewallPolicyRule", "name", a.id.External) mapCtx := &direct.MapContext{} desired := a.desired.DeepCopy() @@ -154,32 +147,46 @@ 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: a.firewallPolicy, + FirewallPolicy: parent.FirewallPolicy, } op, err := a.firewallPoliciesClient.AddRule(ctx, req) if err != nil { - return fmt.Errorf("creating ComputeFirewallPolicyRule %d: %w", a.priority, err) + return fmt.Errorf("creating ComputeFirewallPolicyRule %s: %w", a.id.External, err) } if !op.Done() { err = op.Wait(ctx) if err != nil { - return fmt.Errorf("waiting ComputeFirewallPolicyRule %d create failed: %w", a.priority, err) + return fmt.Errorf("waiting ComputeFirewallPolicyRule %s create failed: %w", a.id.External, err) } } - log.V(2).Info("successfully created ComputeFirewallPolicyRule", "priority", a.priority) + log.V(2).Info("successfully created ComputeFirewallPolicyRule", "name", a.id.External) // Get the created resource created := &computepb.FirewallPolicyRule{} created, err = a.get(ctx) if err != nil { - return fmt.Errorf("getting ComputeFirewallPolicyRule %d: %w", a.priority, err) + return fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.id.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 + status.ExternalRef = &externalRef return createOp.UpdateStatus(ctx, status, nil) } @@ -192,7 +199,7 @@ func (a *firewallPolicyRuleAdapter) Update(ctx context.Context, updateOp *direct } log := klog.FromContext(ctx).WithName(ctrlName) - log.V(2).Info("updating ComputeFirewallPolicyRule", "priority", a.priority) + log.V(2).Info("updating ComputeFirewallPolicyRule", "name", a.id.External) mapCtx := &direct.MapContext{} desired := a.desired.DeepCopy() @@ -206,27 +213,38 @@ 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, "/") + priority, err := strconv.ParseInt(tokens[5], 10, 32) + //if err != nil { + // return fmt.Errorf("get ComputeFirewallPolicyRule parent %s: %w", a.id.External, err) + //} + updateReq := &computepb.PatchRuleFirewallPolicyRequest{ FirewallPolicyRuleResource: firewallPolicyRule, - FirewallPolicy: a.firewallPolicy, - Priority: direct.PtrTo(int32(a.priority)), + FirewallPolicy: parent.FirewallPolicy, + Priority: direct.PtrTo(int32(priority)), } op, err := a.firewallPoliciesClient.PatchRule(ctx, updateReq) if err != nil { - return fmt.Errorf("updating ComputeFirewallPolicyRule %d: %w", a.priority, err) + return fmt.Errorf("updating ComputeFirewallPolicyRule %s: %w", a.id.External, err) } if !op.Done() { err = op.Wait(ctx) if err != nil { - return fmt.Errorf("waiting ComputeFirewallPolicyRule %d update failed: %w", a.priority, err) + return fmt.Errorf("waiting ComputeFirewallPolicyRule %s update failed: %w", a.id.External, err) } } - log.V(2).Info("successfully updated ComputeFirewallPolicyRule", "priority", a.priority) + log.V(2).Info("successfully updated ComputeFirewallPolicyRule", "name", a.id.External) // Get the updated resource updated, err = a.get(ctx) if err != nil { - return fmt.Errorf("getting ComputeFirewallPolicyRule %d: %w", a.priority, err) + return fmt.Errorf("getting ComputeFirewallPolicyRule %s: %w", a.id.External, err) } status := &krm.ComputeFirewallPolicyRuleStatus{} @@ -236,7 +254,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 %d not found", a.priority) + return nil, fmt.Errorf("firewallPolicyRule %s not found", a.id.External) } mc := &direct.MapContext{} @@ -261,24 +279,35 @@ 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).WithName(ctrlName) - log.V(2).Info("deleting ComputeFirewallPolicyRule", "priority", a.priority) + 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) + } + + tokens := strings.Split(a.id.External, "/") + priority, err := strconv.ParseInt(tokens[5], 10, 32) + //if err != nil { + // return fmt.Errorf("get ComputeFirewallPolicyRule parent %s: %w", a.id.External, err) + //} delReq := &computepb.RemoveRuleFirewallPolicyRequest{ - FirewallPolicy: a.firewallPolicy, - Priority: direct.PtrTo(int32(a.priority)), + FirewallPolicy: parent.FirewallPolicy, + Priority: direct.PtrTo(int32(priority)), } op, err := a.firewallPoliciesClient.RemoveRule(ctx, delReq) if err != nil { - return false, fmt.Errorf("deleting ComputeFirewallPolicyRule %d: %w", a.priority, err) + return false, fmt.Errorf("deleting ComputeFirewallPolicyRule %s: %w", a.id.External, err) } if !op.Done() { err = op.Wait(ctx) if err != nil { - return false, fmt.Errorf("waiting ComputeFirewallPolicyRule %d delete failed: %w", a.priority, err) + return false, fmt.Errorf("waiting ComputeFirewallPolicyRule %s delete failed: %w", a.id.External, err) } } - log.V(2).Info("successfully deleted ComputeFirewallPolicyRule", "priority", a.priority) + log.V(2).Info("successfully deleted ComputeFirewallPolicyRule", "name", a.id.External) // Get the deleted rules _, err = a.get(ctx) @@ -289,9 +318,20 @@ 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, "/") + priority, err := strconv.ParseInt(tokens[5], 10, 32) + //if err != nil { + // return fmt.Errorf("get ComputeFirewallPolicyRule parent %s: %w", a.id.External, err) + //} + getReq := &computepb.GetRuleFirewallPolicyRequest{ - FirewallPolicy: a.firewallPolicy, - Priority: direct.PtrTo(int32(a.priority)), + FirewallPolicy: parent.FirewallPolicy, + Priority: direct.PtrTo(int32(priority)), } return a.firewallPoliciesClient.GetRule(ctx, getReq) } diff --git a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_externalresource.go b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_externalresource.go deleted file mode 100644 index fc8edc4366..0000000000 --- a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_externalresource.go +++ /dev/null @@ -1,26 +0,0 @@ -/* -Copyright 2024. - -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 firewallpolicyrule - -const ( - serviceDomain = "//compute.googleapis.com" -) - -type FirewallPolicyRuleIdentity struct { - firewallPolicy string - priority int64 -} diff --git a/pkg/controller/direct/compute/firewallpolicyrule/refs.go b/pkg/controller/direct/compute/firewallpolicyrule/refs.go index 6b19ad6a0c..60807e6186 100644 --- a/pkg/controller/direct/compute/firewallpolicyrule/refs.go +++ b/pkg/controller/direct/compute/firewallpolicyrule/refs.go @@ -29,49 +29,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func ResolveComputeFirewallPolicy(ctx context.Context, reader client.Reader, src client.Object, ref *refs.ComputeFirewallPolicyRef) (*refs.ComputeFirewallPolicyRef, error) { - if ref == nil { - return nil, nil - } - - if ref.External != "" { - if ref.Name != "" { - return nil, fmt.Errorf("cannot specify both name and external on reference") - } - return ref, nil - } - - if ref.Name == "" { - return nil, fmt.Errorf("must specify either name or external on reference") - } - - key := types.NamespacedName{ - Namespace: ref.Namespace, - Name: ref.Name, - } - if key.Namespace == "" { - key.Namespace = src.GetNamespace() - } - - computeFirwallPolicy, err := resolveResourceName(ctx, reader, key, schema.GroupVersionKind{ - Group: "compute.cnrm.cloud.google.com", - Version: "v1beta1", - Kind: "ComputeFirewallPolicy", - }) - - if err != nil { - return nil, err - } - - resourceID, err := refs.GetResourceID(computeFirwallPolicy) - if err != nil { - return nil, err - } - - return &refs.ComputeFirewallPolicyRef{ - External: fmt.Sprintf("%s", resourceID)}, nil -} - func ResolveComputeNetwork(ctx context.Context, reader client.Reader, src client.Object, ref *refs.ComputeNetworkRef) (*refs.ComputeNetworkRef, error) { if ref == nil { return nil, nil diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml index 95335ff087..9f17ff5aaf 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml @@ -46,6 +46,7 @@ status: reason: UpToDate status: "True" type: Ready + externalRef: locations/global/firewallPolicies/${firewallPolicyID}/rules/9000 kind: compute#firewallPolicyRule observedGeneration: 2 ruleTupleCount: 110 diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml index 65e5585f7f..7f932e3163 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml @@ -46,6 +46,7 @@ status: reason: UpToDate status: "True" type: Ready + externalRef: locations/global/firewallPolicies/${firewallPolicyID}/rules/9000 kind: compute#firewallPolicyRule observedGeneration: 2 ruleTupleCount: 110 diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_generated_object_computefirewallpolicyrule-minimal.golden.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_generated_object_computefirewallpolicyrule-minimal.golden.yaml index 77bb2c7fb2..3d27711eca 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_generated_object_computefirewallpolicyrule-minimal.golden.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_generated_object_computefirewallpolicyrule-minimal.golden.yaml @@ -29,6 +29,7 @@ status: reason: UpToDate status: "True" type: Ready + externalRef: locations/global/firewallPolicies/${firewallPolicyID}/rules/9000 kind: compute#firewallPolicyRule observedGeneration: 2 ruleTupleCount: 2 diff --git a/tests/e2e/normalize.go b/tests/e2e/normalize.go index 1c6d8f4ce1..d98dfc0999 100644 --- a/tests/e2e/normalize.go +++ b/tests/e2e/normalize.go @@ -288,6 +288,22 @@ func normalizeKRMObject(t *testing.T, u *unstructured.Unstructured, project test } } + // Get firewall policy id from firewall policy rule's externalRef and replace it + externalRef, _, _ := unstructured.NestedString(u.Object, "status", "externalRef") + if externalRef != "" { + tokens := strings.Split(externalRef, "/") + n := len(tokens) + if n >= 2 { + typeName := tokens[len(tokens)-2] + firewallPolicyId := tokens[len(tokens)-3] + if typeName == "rules" { + visitor.stringTransforms = append(visitor.stringTransforms, func(path string, s string) string { + return strings.ReplaceAll(s, firewallPolicyId, "${firewallPolicyID}") + }) + } + } + } + resourceID, _, _ := unstructured.NestedString(u.Object, "spec", "resourceID") if resourceID != "" { switch u.GroupVersionKind() { @@ -303,6 +319,7 @@ func normalizeKRMObject(t *testing.T, u *unstructured.Unstructured, project test case schema.GroupVersionKind{Group: "compute.cnrm.cloud.google.com", Version: "v1beta1", Kind: "ComputeFirewallPolicy"}: visitor.stringTransforms = append(visitor.stringTransforms, func(path string, s string) string { return strings.ReplaceAll(s, resourceID, "${firewallPolicyID}") + }) } }