From 124ae3cb12c98f4f9a27d16cc9d3a5d25392dc7f Mon Sep 17 00:00:00 2001 From: Anh Le Date: Thu, 19 Dec 2024 22:35:21 +0000 Subject: [PATCH] chore: Split identity and ref for MemoryStoreInstance --- apis/memorystore/v1alpha1/instance_identiy.go | 86 +++++++++++++ .../v1alpha1/instance_reference.go | 114 ++++-------------- .../v1alpha1/zz_generated.deepcopy.go | 25 +++- 3 files changed, 127 insertions(+), 98 deletions(-) create mode 100644 apis/memorystore/v1alpha1/instance_identiy.go diff --git a/apis/memorystore/v1alpha1/instance_identiy.go b/apis/memorystore/v1alpha1/instance_identiy.go new file mode 100644 index 0000000000..ddc66b7c09 --- /dev/null +++ b/apis/memorystore/v1alpha1/instance_identiy.go @@ -0,0 +1,86 @@ +// 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 v1alpha1 + +import ( + "context" + "fmt" + + "github.com/GoogleCloudPlatform/k8s-config-connector/apis/common" + refsv1beta1 "github.com/GoogleCloudPlatform/k8s-config-connector/apis/refs/v1beta1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type MemoryStoreInstanceIdentity struct { + id string + parent *MemorystoreInstanceParent +} + +func (i *MemoryStoreInstanceIdentity) String() string { + return i.parent.String() + "/instances/" + i.id +} + +func (r *MemoryStoreInstanceIdentity) Parent() *MemorystoreInstanceParent { + return r.parent +} + +func (r *MemoryStoreInstanceIdentity) ID() string { + return r.id +} + +func NewMemoryStoreInstanceIdentity(ctx context.Context, reader client.Reader, obj *MemorystoreInstance, u *unstructured.Unstructured) (*MemoryStoreInstanceIdentity, error) { + // Get Parent + projectID, err := refsv1beta1.ResolveProjectID(ctx, reader, u) + if err != nil { + return nil, err + } + + location := obj.Spec.Location + if location == "" { + return nil, fmt.Errorf("cannot resolve location") + } + + // Get desired ID + resourceID := common.ValueOf(obj.Spec.ResourceID) + if resourceID == "" { + resourceID = obj.GetName() + } + if resourceID == "" { + return nil, fmt.Errorf("cannot resolve resource ID") + } + // Use approved External + externalRef := common.ValueOf(obj.Status.ExternalRef) + if externalRef != "" { + actualIdentity, err := ParseMemorystoreInstanceExternal(externalRef) + if err != nil { + return nil, err + } + if actualIdentity.parent.ProjectID != projectID { + return nil, fmt.Errorf("spec.projectRef changed, expect %s, got %s", actualIdentity.parent.ProjectID, projectID) + } + if actualIdentity.id != resourceID { + return nil, fmt.Errorf("cannot reset `metadata.name` or `spec.resourceID` to %s, since it has already assigned to %s", + resourceID, actualIdentity.id) + } + } + return &MemoryStoreInstanceIdentity{ + parent: &MemorystoreInstanceParent{ + ProjectID: projectID, + Location: location, + }, + id: resourceID, + }, nil +} diff --git a/apis/memorystore/v1alpha1/instance_reference.go b/apis/memorystore/v1alpha1/instance_reference.go index bed1c060f4..a0520a6bb7 100644 --- a/apis/memorystore/v1alpha1/instance_reference.go +++ b/apis/memorystore/v1alpha1/instance_reference.go @@ -16,6 +16,7 @@ package v1alpha1 import ( "context" + "errors" "fmt" "strings" @@ -41,8 +42,6 @@ type MemorystoreInstanceRef struct { // The namespace of a MemorystoreInstance resource. Namespace string `json:"namespace,omitempty"` - - parent *MemorystoreInstanceParent } // NormalizedExternal provision the "External" value for other resource that depends on MemorystoreInstance. @@ -54,7 +53,7 @@ func (r *MemorystoreInstanceRef) NormalizedExternal(ctx context.Context, reader } // From given External if r.External != "" { - if _, _, err := parseMemorystoreInstanceExternal(r.External); err != nil { + if _, err := ParseMemorystoreInstanceExternal(r.External); err != nil { return "", err } return r.External, nil @@ -74,9 +73,15 @@ func (r *MemorystoreInstanceRef) NormalizedExternal(ctx context.Context, reader return "", fmt.Errorf("reading referenced %s %s: %w", MemorystoreInstanceGVK, 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) + actualExternalRef, _, err1 := unstructured.NestedString(u.Object, "status", "externalRef") + if err1 != nil { + err1 = fmt.Errorf("MemorystoreInstance `status.externalRef` not configured: %w", err1) + // Backward compatible to Terraform/DCL based resource, which does not have status.externalRef. + var err2 error + actualExternalRef, _, err2 = unstructured.NestedString(u.Object, "status", "name") + if err2 != nil { + return "", errors.Join(err1, err2) + } } if actualExternalRef == "" { return "", k8s.NewReferenceNotReadyError(u.GroupVersionKind(), key) @@ -85,72 +90,6 @@ func (r *MemorystoreInstanceRef) NormalizedExternal(ctx context.Context, reader return r.External, nil } -// New builds a MemorystoreInstanceRef from the Config Connector MemorystoreInstance object. -func NewMemorystoreInstanceRef(ctx context.Context, reader client.Reader, obj *MemorystoreInstance) (*MemorystoreInstanceRef, error) { - id := &MemorystoreInstanceRef{} - - // Get Parent - projectRef, err := refsv1beta1.ResolveProject(ctx, reader, obj.GetNamespace(), obj.Spec.ProjectRef) - if err != nil { - return nil, err - } - projectID := projectRef.ProjectID - if projectID == "" { - return nil, fmt.Errorf("cannot resolve project") - } - location := obj.Spec.Location - id.parent = &MemorystoreInstanceParent{ProjectID: projectID, Location: location} - - // Get desired ID - resourceID := valueOf(obj.Spec.ResourceID) - if resourceID == "" { - resourceID = obj.GetName() - } - if resourceID == "" { - return nil, fmt.Errorf("cannot resolve resource ID") - } - - // Use approved External - externalRef := valueOf(obj.Status.ExternalRef) - if externalRef == "" { - id.External = asMemorystoreInstanceExternal(id.parent, resourceID) - return id, nil - } - - // Validate desired with actual - actualParent, actualResourceID, err := parseMemorystoreInstanceExternal(externalRef) - if err != nil { - return nil, err - } - if actualParent.ProjectID != projectID { - return nil, fmt.Errorf("spec.projectRef changed, expect %s, got %s", actualParent.ProjectID, projectID) - } - if actualParent.Location != location { - return nil, fmt.Errorf("spec.location changed, expect %s, got %s", actualParent.Location, location) - } - if actualResourceID != resourceID { - return nil, fmt.Errorf("cannot reset `metadata.name` or `spec.resourceID` to %s, since it has already assigned to %s", - resourceID, actualResourceID) - } - id.External = externalRef - id.parent = &MemorystoreInstanceParent{ProjectID: projectID, Location: location} - return id, nil -} - -func (r *MemorystoreInstanceRef) Parent() (*MemorystoreInstanceParent, error) { - if r.parent != nil { - return r.parent, nil - } - if r.External != "" { - parent, _, err := parseMemorystoreInstanceExternal(r.External) - if err != nil { - return nil, err - } - return parent, nil - } - return nil, fmt.Errorf("MemorystoreInstanceRef not initialized from `NewMemorystoreInstanceRef` or `NormalizedExternal`") -} - type MemorystoreInstanceParent struct { ProjectID string Location string @@ -160,28 +99,17 @@ func (p *MemorystoreInstanceParent) String() string { return "projects/" + p.ProjectID + "/locations/" + p.Location } -func asMemorystoreInstanceExternal(parent *MemorystoreInstanceParent, resourceID string) (external string) { - return parent.String() + "/instances/" + resourceID -} - -func parseMemorystoreInstanceExternal(external string) (parent *MemorystoreInstanceParent, resourceID string, err error) { +func ParseMemorystoreInstanceExternal(external string) (*MemoryStoreInstanceIdentity, error) { external = strings.TrimPrefix(external, "/") tokens := strings.Split(external, "/") if len(tokens) != 6 || tokens[0] != "projects" || tokens[2] != "locations" || tokens[4] != "instance" { - return nil, "", fmt.Errorf("format of MemorystoreInstance external=%q was not known (use projects/{{projectId}}/locations/{{location}}/instances/{{instanceID}})", external) - } - parent = &MemorystoreInstanceParent{ - ProjectID: tokens[1], - Location: tokens[3], - } - resourceID = tokens[5] - return parent, resourceID, nil -} - -func valueOf[T any](t *T) T { - var zeroVal T - if t == nil { - return zeroVal - } - return *t + return nil, fmt.Errorf("format of MemorystoreInstance external=%q was not known (use projects/{{projectId}}/locations/{{location}}/instances/{{instanceID}})", external) + } + return &MemoryStoreInstanceIdentity{ + parent: &MemorystoreInstanceParent{ + ProjectID: tokens[1], + Location: tokens[3], + }, + id: tokens[5], + }, nil } diff --git a/apis/memorystore/v1alpha1/zz_generated.deepcopy.go b/apis/memorystore/v1alpha1/zz_generated.deepcopy.go index 1463b579f0..c40e4db4d7 100644 --- a/apis/memorystore/v1alpha1/zz_generated.deepcopy.go +++ b/apis/memorystore/v1alpha1/zz_generated.deepcopy.go @@ -222,6 +222,26 @@ func (in *Instance_StateInfo_UpdateInfo) DeepCopy() *Instance_StateInfo_UpdateIn return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MemoryStoreInstanceIdentity) DeepCopyInto(out *MemoryStoreInstanceIdentity) { + *out = *in + if in.parent != nil { + in, out := &in.parent, &out.parent + *out = new(MemorystoreInstanceParent) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MemoryStoreInstanceIdentity. +func (in *MemoryStoreInstanceIdentity) DeepCopy() *MemoryStoreInstanceIdentity { + if in == nil { + return nil + } + out := new(MemoryStoreInstanceIdentity) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MemorystoreInstance) DeepCopyInto(out *MemorystoreInstance) { *out = *in @@ -388,11 +408,6 @@ func (in *MemorystoreInstanceParent) DeepCopy() *MemorystoreInstanceParent { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MemorystoreInstanceRef) DeepCopyInto(out *MemorystoreInstanceRef) { *out = *in - if in.parent != nil { - in, out := &in.parent, &out.parent - *out = new(MemorystoreInstanceParent) - **out = **in - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MemorystoreInstanceRef.