diff --git a/apis/compute/v1beta1/computebackendservice_reference.go b/apis/compute/v1beta1/computebackendservice_reference.go new file mode 100644 index 0000000000..4f65407c21 --- /dev/null +++ b/apis/compute/v1beta1/computebackendservice_reference.go @@ -0,0 +1,163 @@ +// 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" + + "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s" + + refsv1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ refsv1beta1.ExternalNormalizer = &ComputeBackendServiceRef{} +var ComputeBackendServiceGVK = GroupVersion.WithKind("ComputeBackendService") + +// ComputeBackendServiceRef defines the resource reference to ComputeBackendService, which "External" field +// holds the GCP identifier for the KRM object. +type ComputeBackendServiceRef struct { + // The value of an externally managed ComputeBackendService resource. + // Should be in the format "projects/{{project}}/global/backendServices/{{backendService}}" + // or "projects/{{project}}/regions/{{region}}/backendServices/{{backendService}}". + External string `json:"external,omitempty"` + + // The name of a ComputeBackendService resource. + Name string `json:"name,omitempty"` + + // The namespace of a ComputeBackendService resource. + Namespace string `json:"namespace,omitempty"` + + //parent *ComputeBackendServiceParent +} + +// NormalizedExternal provision the "External" value for other resource that depends on ComputeBackendService. +// If the "External" is given in the other resource's spec.ComputeBackendServiceRef, the given value will be used. +// Otherwise, the "Name" and "Namespace" will be used to query the actual ComputeBackendService object from the cluster. +func (r *ComputeBackendServiceRef) NormalizedExternal(ctx context.Context, reader client.Reader, otherNamespace string) (string, error) { + referenceContext := refsv1beta1.ReferenceContext{IsDirectOnly: false, TargetField: "status.selfLink"} + // Get value from spec.ComputeBackendServiceRef.external + if r.External != "" { + if r.Name != "" { + return "", fmt.Errorf("cannot specify both name and external on reference") + } + if referenceContext.IsDirectOnly { + if _, _, err := parseComputeTargetTCPProxyExternal(r.External); err != nil { + return "", err + } + } + // To ensure backward compatibility for existing users, we do not enforce external format + // for non-direct resources + return r.External, nil + } + + if r.Name == "" { + return "", fmt.Errorf("must specify either name or external on reference") + } + + // Get value from the Config Connector object + if r.Namespace == "" { + r.Namespace = otherNamespace + } + key := types.NamespacedName{Name: r.Name, Namespace: r.Namespace} + + u, err := refsv1beta1.ResolveResourceName(ctx, reader, key, ComputeBackendServiceGVK) + if err != nil { + return "", err + } + + 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 object is DirectOnly, it is created by direct controller. + // Get value from status.externalRef, which is the most trustworthy source. + if referenceContext.IsDirectOnly { + if !found || actualExternalRef == "" { + return "", fmt.Errorf("status.externalRef is required for DirectOnly resources, but is missing or empty for %s %s/%s", u.GetKind(), u.GetNamespace(), u.GetName()) + } + r.External = actualExternalRef + } + + // If object not DirectOnly, it can be created by either direct controller or legacy controller, depends on user's settings. + // If status.externalRef does not exist, it's created by legacy controller. Get values from target field. + if !found { + targetField := referenceContext.TargetField + tokens := strings.Split(targetField, ".") + targetField, found, err := unstructured.NestedString(u.Object, tokens...) + if err != nil { + return "", fmt.Errorf("error getting target field %s for %s %s/%s: %w", targetField, u.GetKind(), u.GetNamespace(), u.GetName(), err) + } + if !found || targetField == "" { + return "", fmt.Errorf("target field %s is required but is missing or empty for %s %s/%s", targetField, u.GetKind(), u.GetNamespace(), u.GetName()) + } + r.External = targetField + } else { + // If status.externalRef exists, it's created by direct controller. Get value from status.externalRef. + r.External = actualExternalRef + } + return r.External, nil +} + +func (r *ComputeBackendServiceRef) Parent() (*ComputeBackendServiceParent, error) { + if r.External != "" { + parent, _, err := ParseComputeBackendServiceExternal(r.External) + if err != nil { + return nil, err + } + return parent, nil + } + return nil, fmt.Errorf("ComputeBackendServiceRef not initialized by `NewComputeBackendServiceRef` or resolved by `ResolveExternal`") +} + +type ComputeBackendServiceParent struct { + ProjectID string + Region string +} + +func (p *ComputeBackendServiceParent) String() string { + if p.Region == "global" { + return "projects/" + p.ProjectID + "/global" + } else { + return "projects/" + p.ProjectID + "/regions/" + p.Region + } +} + +func ParseComputeBackendServiceExternal(external string) (parent *ComputeBackendServiceParent, resourceID string, err error) { + external = strings.TrimPrefix(external, "/") + tokens := strings.Split(external, "/") + if len(tokens) == 5 && tokens[0] == "projects" && tokens[3] == "backendServices" { + parent = &ComputeBackendServiceParent{ + ProjectID: tokens[1], + Region: tokens[2], + } + resourceID = tokens[4] + return parent, resourceID, nil + } else if len(tokens) == 6 && tokens[0] == "projects" && tokens[2] == "regions" && tokens[4] == "backendServices" { + parent = &ComputeBackendServiceParent{ + ProjectID: tokens[1], + Region: tokens[3], + } + resourceID = tokens[5] + return parent, resourceID, nil + } + acceptedFormat := "projects/{{project}}/global/backendServices/{{backendService}} or projects/{{project}}/regions/{{region}}/backendServices/{{backendService}}" + return nil, "", k8s.NewInvalidFormatError(external, acceptedFormat) +} diff --git a/apis/compute/v1beta1/forwardingrule_types.go b/apis/compute/v1beta1/forwardingrule_types.go index 7933ba9b7d..6847ba1256 100644 --- a/apis/compute/v1beta1/forwardingrule_types.go +++ b/apis/compute/v1beta1/forwardingrule_types.go @@ -144,7 +144,7 @@ type ComputeForwardingRuleSpec struct { /* A ComputeBackendService to receive the matched traffic. This is used only for internal load balancing. */ // +optional - BackendServiceRef *refs.ComputeBackendServiceRef `json:"backendServiceRef,omitempty"` + BackendServiceRef *ComputeBackendServiceRef `json:"backendServiceRef,omitempty"` /* Immutable. An optional description of this resource. Provide this property when you create the resource. */ diff --git a/apis/compute/v1beta1/targettcpproxy_types.go b/apis/compute/v1beta1/targettcpproxy_types.go index fb17ec5ece..79237ae1c0 100644 --- a/apis/compute/v1beta1/targettcpproxy_types.go +++ b/apis/compute/v1beta1/targettcpproxy_types.go @@ -15,7 +15,6 @@ package v1beta1 import ( - refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/apis/k8s/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -34,7 +33,7 @@ var ( type ComputeTargetTCPProxySpec struct { // A reference to the ComputeBackendService resource. // +required - BackendServiceRef *refs.ComputeBackendServiceRef `json:"backendServiceRef"` + BackendServiceRef *ComputeBackendServiceRef `json:"backendServiceRef"` // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Description is immutable" // Immutable. An optional description of this resource. diff --git a/apis/compute/v1beta1/zz_generated.deepcopy.go b/apis/compute/v1beta1/zz_generated.deepcopy.go index ca24003cfc..43ccf5a04b 100644 --- a/apis/compute/v1beta1/zz_generated.deepcopy.go +++ b/apis/compute/v1beta1/zz_generated.deepcopy.go @@ -24,6 +24,36 @@ 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 *ComputeBackendServiceParent) DeepCopyInto(out *ComputeBackendServiceParent) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComputeBackendServiceParent. +func (in *ComputeBackendServiceParent) DeepCopy() *ComputeBackendServiceParent { + if in == nil { + return nil + } + out := new(ComputeBackendServiceParent) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ComputeBackendServiceRef) DeepCopyInto(out *ComputeBackendServiceRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ComputeBackendServiceRef. +func (in *ComputeBackendServiceRef) DeepCopy() *ComputeBackendServiceRef { + if in == nil { + return nil + } + out := new(ComputeBackendServiceRef) + 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 @@ -285,7 +315,7 @@ func (in *ComputeForwardingRuleSpec) DeepCopyInto(out *ComputeForwardingRuleSpec } if in.BackendServiceRef != nil { in, out := &in.BackendServiceRef, &out.BackendServiceRef - *out = new(refsv1beta1.ComputeBackendServiceRef) + *out = new(ComputeBackendServiceRef) **out = **in } if in.Description != nil { @@ -559,7 +589,7 @@ func (in *ComputeTargetTCPProxySpec) DeepCopyInto(out *ComputeTargetTCPProxySpec *out = *in if in.BackendServiceRef != nil { in, out := &in.BackendServiceRef, &out.BackendServiceRef - *out = new(refsv1beta1.ComputeBackendServiceRef) + *out = new(ComputeBackendServiceRef) **out = **in } if in.Description != nil { diff --git a/apis/refs/v1beta1/computerefs.go b/apis/refs/v1beta1/computerefs.go index e56d511619..da99a2377a 100644 --- a/apis/refs/v1beta1/computerefs.go +++ b/apis/refs/v1beta1/computerefs.go @@ -256,15 +256,6 @@ type ComputeAddressRef struct { Namespace string `json:"namespace,omitempty"` } -type ComputeBackendServiceRef struct { - /* The ComputeBackendService selflink in the form "projects/{{project}}/global/backendServices/{{name}}" or "projects/{{project}}/regions/{{region}}/backendServices/{{name}}" when not managed by Config Connector. */ - External string `json:"external,omitempty"` - /* The `name` field of a `ComputeBackendService` resource. */ - Name string `json:"name,omitempty"` - /* The `namespace` field of a `ComputeBackendService` resource. */ - Namespace string `json:"namespace,omitempty"` -} - type ComputeServiceAttachmentRef struct { /* The ComputeServiceAttachment selflink in the form "projects/{{project}}/regions/{{region}}/serviceAttachments/{{name}}" when not managed by Config Connector. */ External string `json:"external,omitempty"` diff --git a/apis/refs/v1beta1/helper.go b/apis/refs/v1beta1/helper.go index df6733442b..625b5334ad 100644 --- a/apis/refs/v1beta1/helper.go +++ b/apis/refs/v1beta1/helper.go @@ -15,8 +15,14 @@ package v1beta1 import ( + "context" "fmt" + "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -41,3 +47,16 @@ func GetLocation(u *unstructured.Unstructured) (string, error) { } return location, nil } + +func ResolveResourceName(ctx context.Context, reader client.Reader, key client.ObjectKey, gvk schema.GroupVersionKind) (*unstructured.Unstructured, error) { + resource := &unstructured.Unstructured{} + resource.SetGroupVersionKind(gvk) + if err := reader.Get(ctx, key, resource); err != nil { + if apierrors.IsNotFound(err) { + return nil, k8s.NewReferenceNotFoundError(resource.GroupVersionKind(), key) + } + return nil, fmt.Errorf("error reading referenced %v %v: %w", gvk.Kind, key, err) + } + + return resource, nil +} diff --git a/apis/refs/v1beta1/interface.go b/apis/refs/v1beta1/interface.go index 561a1bf0a7..578e1d92d3 100644 --- a/apis/refs/v1beta1/interface.go +++ b/apis/refs/v1beta1/interface.go @@ -20,6 +20,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) +type ReferenceContext struct { + // Whether a referenced resource is only managed by direct controller + IsDirectOnly bool + + // the referenced resource's field that will be extracted and set as the value of referenced field + // If IsDirectOnly is false, TargetField is required + TargetField string +} + type ExternalNormalizer interface { // NormalizedExternal expects the implemented struct has a "External" field, and this function // assigns a value to the "External" field if it is empty. diff --git a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computeforwardingrules.compute.cnrm.cloud.google.com.yaml b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computeforwardingrules.compute.cnrm.cloud.google.com.yaml index 7b8ae5b62d..c472f3e139 100644 --- a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computeforwardingrules.compute.cnrm.cloud.google.com.yaml +++ b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computeforwardingrules.compute.cnrm.cloud.google.com.yaml @@ -113,16 +113,15 @@ spec: - external properties: external: - description: The ComputeBackendService selflink in the form "projects/{{project}}/global/backendServices/{{name}}" - or "projects/{{project}}/regions/{{region}}/backendServices/{{name}}" - when not managed by Config Connector. + description: The value of an externally managed ComputeBackendService + resource. Should be in the format "projects/{{project}}/global/backendServices/{{backendService}}" + or "projects/{{project}}/regions/{{region}}/backendServices/{{backendService}}". type: string name: - description: The `name` field of a `ComputeBackendService` resource. + description: The name of a ComputeBackendService resource. type: string namespace: - description: The `namespace` field of a `ComputeBackendService` - resource. + description: The namespace of a ComputeBackendService resource. type: string type: object description: diff --git a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computetargettcpproxies.compute.cnrm.cloud.google.com.yaml b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computetargettcpproxies.compute.cnrm.cloud.google.com.yaml index 185478604c..dd7a95f70f 100644 --- a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computetargettcpproxies.compute.cnrm.cloud.google.com.yaml +++ b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_computetargettcpproxies.compute.cnrm.cloud.google.com.yaml @@ -80,16 +80,15 @@ spec: - external properties: external: - description: The ComputeBackendService selflink in the form "projects/{{project}}/global/backendServices/{{name}}" - or "projects/{{project}}/regions/{{region}}/backendServices/{{name}}" - when not managed by Config Connector. + description: The value of an externally managed ComputeBackendService + resource. Should be in the format "projects/{{project}}/global/backendServices/{{backendService}}" + or "projects/{{project}}/regions/{{region}}/backendServices/{{backendService}}". type: string name: - description: The `name` field of a `ComputeBackendService` resource. + description: The name of a ComputeBackendService resource. type: string namespace: - description: The `namespace` field of a `ComputeBackendService` - resource. + description: The namespace of a ComputeBackendService resource. type: string type: object description: diff --git a/pkg/controller/direct/compute/forwardingrule/forwardingrule_controller.go b/pkg/controller/direct/compute/forwardingrule/forwardingrule_controller.go index b2d8e0a683..98915f1c8e 100644 --- a/pkg/controller/direct/compute/forwardingrule/forwardingrule_controller.go +++ b/pkg/controller/direct/compute/forwardingrule/forwardingrule_controller.go @@ -27,6 +27,7 @@ import ( gcp "cloud.google.com/go/compute/apiv1" computepb "cloud.google.com/go/compute/apiv1/computepb" krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/compute/v1beta1" + refsv1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/config" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct/directbase" @@ -539,3 +540,39 @@ func setStatus(u *unstructured.Unstructured, typedStatus any) error { return nil } + +// This function get the normalized external values and convert it to the API required format +func resolveBackendService(ctx context.Context, reader client.Reader, obj *krm.ComputeForwardingRule) error { + // API required format: selfLink + referenceContext := refsv1beta1.ReferenceContext{IsDirectOnly: false, TargetField: "status.selfLink"} + computeBasePath := "https://www.googleapis.com/compute/v1/" + ref := obj.Spec.BackendServiceRef + if ref != nil { + // Get normalized external + _, err := ref.NormalizedExternal(ctx, reader, obj.GetNamespace()) + if err != nil { + return fmt.Errorf("failed to get BackendServiceRef: %w", err) + } + // Convert normalized external to API required format + v := ref.External + // If object is DirectOnly, it is created by direct controller. + if referenceContext.IsDirectOnly { + // add the compute prefix in front + obj.Spec.BackendServiceRef.External = computeBasePath + v + return nil + } + // If object not DirectOnly, it can be created by either direct controller or legacy controller, depends on user's settings. + _, _, err = krm.ParseComputeBackendServiceExternal(v) + // Value follows KCC external format, likely it's created by direct controller + if err == nil { + // add the compute prefix in front + obj.Spec.BackendServiceRef.External = computeBasePath + v + return nil + } + // For backward compatibility, we also accept values that does not match the KCC external format.(likely it's created by legacy controller) + // Return the value as is and let the API handle it + obj.Spec.BackendServiceRef.External = v + return nil + } + return nil +} diff --git a/pkg/controller/direct/compute/forwardingrule/mapper.go b/pkg/controller/direct/compute/forwardingrule/mapper.go index 2c346d8202..e90c7934de 100644 --- a/pkg/controller/direct/compute/forwardingrule/mapper.go +++ b/pkg/controller/direct/compute/forwardingrule/mapper.go @@ -52,16 +52,16 @@ func ComputeForwardingRuleSpec_IpAddress_FromProto(mapCtx *direct.MapContext, in return out } -func ComputeForwardingRuleSpec_BackendSeriviceRef_FromProto(mapCtx *direct.MapContext, in string) *refs.ComputeBackendServiceRef { +func ComputeForwardingRuleSpec_BackendSeriviceRef_FromProto(mapCtx *direct.MapContext, in string) *krm.ComputeBackendServiceRef { if in == "" { return nil } - return &refs.ComputeBackendServiceRef{ + return &krm.ComputeBackendServiceRef{ External: in, } } -func ComputeForwardingRuleSpec_BackendSeriviceRef_ToProto(mapCtx *direct.MapContext, in *refs.ComputeBackendServiceRef) *string { +func ComputeForwardingRuleSpec_BackendSeriviceRef_ToProto(mapCtx *direct.MapContext, in *krm.ComputeBackendServiceRef) *string { if in == nil { return nil } diff --git a/pkg/controller/direct/compute/forwardingrule/refs.go b/pkg/controller/direct/compute/forwardingrule/refs.go index 4be0547f64..730fa5ee76 100644 --- a/pkg/controller/direct/compute/forwardingrule/refs.go +++ b/pkg/controller/direct/compute/forwardingrule/refs.go @@ -178,49 +178,6 @@ func ResolveComputeAddress(ctx context.Context, reader client.Reader, src client External: address}, nil } -func ResolveComputeBackendService(ctx context.Context, reader client.Reader, src client.Object, ref *refs.ComputeBackendServiceRef) (*refs.ComputeBackendServiceRef, 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() - } - - computeBackendService, err := resolveResourceName(ctx, reader, key, schema.GroupVersionKind{ - Group: "compute.cnrm.cloud.google.com", - Version: "v1beta1", - Kind: "ComputeBackendService", - }) - if err != nil { - return nil, err - } - - // targetField: self_link - // See compute servicemappings for details - selfLink, _, err := unstructured.NestedString(computeBackendService.Object, "status", "selfLink") - if err != nil || selfLink == "" { - return nil, fmt.Errorf("cannot get selfLink for referenced %s %v (status.selfLink is empty)", computeBackendService.GetKind(), computeBackendService.GetNamespace()) - } - return &refs.ComputeBackendServiceRef{ - External: selfLink}, nil -} - func ResolveComputeServiceAttachment(ctx context.Context, reader client.Reader, src client.Object, ref *refs.ComputeServiceAttachmentRef) (*refs.ComputeServiceAttachmentRef, error) { if ref == nil { return nil, nil @@ -557,13 +514,8 @@ func resolveDependencies(ctx context.Context, reader client.Reader, obj *krm.Com } // Get backend service - if obj.Spec.BackendServiceRef != nil { - backendServiceRef, err := ResolveComputeBackendService(ctx, reader, obj, obj.Spec.BackendServiceRef) - if err != nil { - return err - - } - obj.Spec.BackendServiceRef.External = backendServiceRef.External + if err := resolveBackendService(ctx, reader, obj); err != nil { + return err } // Get ip address, ip address is optional diff --git a/pkg/controller/direct/compute/targettcpproxy/mapper.go b/pkg/controller/direct/compute/targettcpproxy/mapper.go index e0aea08ec6..881a30645d 100644 --- a/pkg/controller/direct/compute/targettcpproxy/mapper.go +++ b/pkg/controller/direct/compute/targettcpproxy/mapper.go @@ -15,21 +15,20 @@ package targettcpproxy import ( - refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1" - + krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/compute/v1beta1" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct" ) -func ComputeTargetTCPProxySpec_BackendServiceRef_FromProto(mapCtx *direct.MapContext, in string) *refs.ComputeBackendServiceRef { +func ComputeTargetTCPProxySpec_BackendServiceRef_FromProto(mapCtx *direct.MapContext, in string) *krm.ComputeBackendServiceRef { if in == "" { return nil } - return &refs.ComputeBackendServiceRef{ + return &krm.ComputeBackendServiceRef{ External: in, } } -func ComputeTargetTCPProxySpec_BackendServiceRef_ToProto(mapCtx *direct.MapContext, in *refs.ComputeBackendServiceRef) *string { +func ComputeTargetTCPProxySpec_BackendServiceRef_ToProto(mapCtx *direct.MapContext, in *krm.ComputeBackendServiceRef) *string { if in == nil { return nil } diff --git a/pkg/controller/direct/compute/targettcpproxy/refs.go b/pkg/controller/direct/compute/targettcpproxy/refs.go deleted file mode 100644 index 3c54111662..0000000000 --- a/pkg/controller/direct/compute/targettcpproxy/refs.go +++ /dev/null @@ -1,100 +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 targettcpproxy - -import ( - "context" - "fmt" - - "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/k8s" - apierrors "k8s.io/apimachinery/pkg/api/errors" - - krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/compute/v1beta1" - - refs "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func ResolveComputeBackendService(ctx context.Context, reader client.Reader, src client.Object, ref *refs.ComputeBackendServiceRef) (*refs.ComputeBackendServiceRef, 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() - } - - computeBackendService, err := resolveResourceName(ctx, reader, key, schema.GroupVersionKind{ - Group: "compute.cnrm.cloud.google.com", - Version: "v1beta1", - Kind: "ComputeBackendService", - }) - if err != nil { - return nil, err - } - - // targetField: self_link - // See compute servicemappings for details - selfLink, _, err := unstructured.NestedString(computeBackendService.Object, "status", "selfLink") - if err != nil || selfLink == "" { - return nil, fmt.Errorf("cannot get selfLink for referenced %s %v (status.selfLink is empty)", computeBackendService.GetKind(), computeBackendService.GetNamespace()) - } - return &refs.ComputeBackendServiceRef{ - External: selfLink}, nil -} - -func resolveResourceName(ctx context.Context, reader client.Reader, key client.ObjectKey, gvk schema.GroupVersionKind) (*unstructured.Unstructured, error) { - resource := &unstructured.Unstructured{} - resource.SetGroupVersionKind(gvk) - if err := reader.Get(ctx, key, resource); err != nil { - if apierrors.IsNotFound(err) { - return nil, k8s.NewReferenceNotFoundError(resource.GroupVersionKind(), key) - } - return nil, fmt.Errorf("error reading referenced %v %v: %w", gvk.Kind, key, err) - } - - return resource, nil -} - -func resolveDependencies(ctx context.Context, reader client.Reader, obj *krm.ComputeTargetTCPProxy) error { - // Get backend service - if obj.Spec.BackendServiceRef != nil { - backendServiceRef, err := ResolveComputeBackendService(ctx, reader, obj, obj.Spec.BackendServiceRef) - if err != nil { - return err - - } - obj.Spec.BackendServiceRef.External = backendServiceRef.External - } - return nil -} diff --git a/pkg/controller/direct/compute/targettcpproxy/targettcpproxy_controller.go b/pkg/controller/direct/compute/targettcpproxy/targettcpproxy_controller.go index 0969c46ac8..bb9b01f700 100644 --- a/pkg/controller/direct/compute/targettcpproxy/targettcpproxy_controller.go +++ b/pkg/controller/direct/compute/targettcpproxy/targettcpproxy_controller.go @@ -25,6 +25,7 @@ import ( gcp "cloud.google.com/go/compute/apiv1" computepb "cloud.google.com/go/compute/apiv1/computepb" krm "github.com/GoogleCloudPlatform/k8s-config-connector/apis/compute/v1beta1" + refsv1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/config" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct" "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct/directbase" @@ -159,9 +160,8 @@ func (a *targetTCPProxyAdapter) Find(ctx context.Context) (bool, error) { } func (a *targetTCPProxyAdapter) Create(ctx context.Context, createOp *directbase.CreateOperation) error { - var err error - - err = resolveDependencies(ctx, a.reader, a.desired) + // Resolve dependencies + err := resolveBackendService(ctx, a.reader, a.desired) if err != nil { return err } @@ -171,7 +171,6 @@ func (a *targetTCPProxyAdapter) Create(ctx context.Context, createOp *directbase mapCtx := &direct.MapContext{} desired := a.desired.DeepCopy() - targetTCPProxy := ComputeTargetTCPProxySpec_ToProto(mapCtx, &desired.Spec) if mapCtx.Err() != nil { return mapCtx.Err() @@ -234,9 +233,8 @@ func (a *targetTCPProxyAdapter) Create(ctx context.Context, createOp *directbase } func (a *targetTCPProxyAdapter) Update(ctx context.Context, updateOp *directbase.UpdateOperation) error { - var err error - - err = resolveDependencies(ctx, a.reader, a.desired) + // Resolve dependencies + err := resolveBackendService(ctx, a.reader, a.desired) if err != nil { return err } @@ -402,3 +400,39 @@ func (a *targetTCPProxyAdapter) get(ctx context.Context) (*computepb.TargetTcpPr return a.regionalTargetTcpProxiesClient.Get(ctx, getReq) } } + +// This function get the normalized external values and convert it to the API required format +func resolveBackendService(ctx context.Context, reader client.Reader, obj *krm.ComputeTargetTCPProxy) error { + // API required format: selfLink + referenceContext := refsv1beta1.ReferenceContext{IsDirectOnly: false, TargetField: "status.selfLink"} + computeBasePath := "https://www.googleapis.com/compute/v1/" + ref := obj.Spec.BackendServiceRef + if ref != nil { + // Get normalized external + _, err := ref.NormalizedExternal(ctx, reader, obj.GetNamespace()) + if err != nil { + return fmt.Errorf("failed to get BackendServiceRef: %w", err) + } + // Convert normalized external to API required format + v := ref.External + // If object is DirectOnly, it is created by direct controller. + if referenceContext.IsDirectOnly { + // add the compute prefix in front + obj.Spec.BackendServiceRef.External = computeBasePath + v + return nil + } + // If object not DirectOnly, it can be created by either direct controller or legacy controller, depends on user's settings. + _, _, err = krm.ParseComputeBackendServiceExternal(v) + // Value follows KCC external format, likely it's created by direct controller + if err == nil { + // add the compute prefix in front + obj.Spec.BackendServiceRef.External = computeBasePath + v + return nil + } + // For backward compatibility, we also accept values that does not match the KCC external format.(likely it's created by legacy controller) + // Return the value as is and let the API handle it + obj.Spec.BackendServiceRef.External = v + return nil + } + return nil +} diff --git a/pkg/k8s/errors.go b/pkg/k8s/errors.go index 21bb05c215..829491388a 100644 --- a/pkg/k8s/errors.go +++ b/pkg/k8s/errors.go @@ -233,3 +233,16 @@ func (e *ImmutableFieldsMutationError) Error() string { func NewImmutableFieldsMutationError(immutableFields []string) *ImmutableFieldsMutationError { return &ImmutableFieldsMutationError{immutableFields} } + +type InvalidFormatError struct { + value string + format string +} + +func (e *InvalidFormatError) Error() string { + return fmt.Sprintf("invalid format: %s. Allowed format(s): %s", e.value, e.format) +} + +func NewInvalidFormatError(value, format string) *InvalidFormatError { + return &InvalidFormatError{value, format} +} diff --git a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computeforwardingrule.md b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computeforwardingrule.md index ae572988a7..dd0cf35ab6 100644 --- a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computeforwardingrule.md +++ b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computeforwardingrule.md @@ -238,7 +238,7 @@ internal load balancer.{% endverbatim %}

string

-

{% verbatim %}The ComputeBackendService selflink in the form "projects/{{project}}/global/backendServices/{{name}}" or "projects/{{project}}/regions/{{region}}/backendServices/{{name}}" when not managed by Config Connector.{% endverbatim %}

+

{% verbatim %}The value of an externally managed ComputeBackendService resource. Should be in the format "projects/{{project}}/global/backendServices/{{backendService}}" or "projects/{{project}}/regions/{{region}}/backendServices/{{backendService}}".{% endverbatim %}

@@ -248,7 +248,7 @@ internal load balancer.{% endverbatim %}

string

-

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

+

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

@@ -258,7 +258,7 @@ internal load balancer.{% endverbatim %}

string

-

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

+

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

diff --git a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computetargettcpproxy.md b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computetargettcpproxy.md index ea27c12cb6..6f3930967a 100644 --- a/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computetargettcpproxy.md +++ b/scripts/generate-google3-docs/resource-reference/generated/resource-docs/compute/computetargettcpproxy.md @@ -112,7 +112,7 @@ resourceID: string

string

-

{% verbatim %}The ComputeBackendService selflink in the form "projects/{{project}}/global/backendServices/{{name}}" or "projects/{{project}}/regions/{{region}}/backendServices/{{name}}" when not managed by Config Connector.{% endverbatim %}

+

{% verbatim %}The value of an externally managed ComputeBackendService resource. Should be in the format "projects/{{project}}/global/backendServices/{{backendService}}" or "projects/{{project}}/regions/{{region}}/backendServices/{{backendService}}".{% endverbatim %}

@@ -122,7 +122,7 @@ resourceID: string

string

-

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

+

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

@@ -132,7 +132,7 @@ resourceID: string

string

-

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

+

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