Skip to content

Commit

Permalink
Address further comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@maqiuyujoyce
maqiuyujoyce committed Dec 19, 2024
1 parent 7c7cc39 commit e33dba7
Show file tree
Hide file tree
Showing 17 changed files with 95 additions and 208 deletions.
27 changes: 11 additions & 16 deletions apis/alloydb/v1alpha1/instance_identity.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

const serviceDomain = "//alloydb.googleapis.com"

// InstanceIdentity defines the resource reference to AlloyDBInstance, which "External" field
// holds the GCP identifier for the KRM object.
// InstanceIdentity defines the resource reference to AlloyDBInstance.
type InstanceIdentity struct {
parent *InstanceParent
id string
Expand All @@ -45,18 +42,14 @@ func (i *InstanceIdentity) Parent() *InstanceParent {
return i.parent
}

// AsExternalRef builds a externalRef from a PrivilegedAccessManagerEntitlement.
func (i *InstanceIdentity) AsExternalRef() *string {
er := serviceDomain + "/" + i.String()
return &er
}

type InstanceParent struct {
clusterName string
projectID string
location string
clusterID string
}

func (p *InstanceParent) String() string {
return p.clusterName
return "projects/" + p.projectID + "/locations/" + p.location + "/clusters/" + p.clusterID
}

// New builds a InstanceIdentity from the Config Connector Instance object.
Expand All @@ -81,12 +74,12 @@ func NewInstanceIdentity(ctx context.Context, reader client.Reader, obj *AlloyDB
externalRef := common.ValueOf(obj.Status.ExternalRef)
if externalRef != "" {
// Validate desired with actual
actualParent, actualResourceID, err := ParseInstanceExternalRef(externalRef)
actualParent, actualResourceID, err := ParseInstanceExternal(externalRef)
if err != nil {
return nil, err
}
if actualParent.clusterName != clusterRef.String() {
return nil, fmt.Errorf("spec.clusterRef changed, expect %s, got %s", actualParent.clusterName, clusterRef)
if actualParent.String() != clusterRef.String() {
return nil, fmt.Errorf("spec.clusterRef changed, expect %s, got %s", actualParent.String(), clusterRef)
}
if actualResourceID != resourceID {
return nil, fmt.Errorf("cannot reset `metadata.name` or `spec.resourceID` to %s, since it has already assigned to %s",
Expand All @@ -95,7 +88,9 @@ func NewInstanceIdentity(ctx context.Context, reader client.Reader, obj *AlloyDB
}
return &InstanceIdentity{
parent: &InstanceParent{
clusterName: clusterRef.String(),
projectID: clusterRef.ProjectID,
location: clusterRef.Location,
clusterID: clusterRef.ClusterID,
},
id: resourceID,
}, nil
Expand Down
12 changes: 3 additions & 9 deletions apis/alloydb/v1alpha1/instance_reference.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,15 @@ func (r *InstanceRef) NormalizedExternal(ctx context.Context, reader client.Read
return r.External, nil
}

func ParseInstanceExternalRef(externalRef string) (parent *InstanceParent, resourceID string, err error) {
if !strings.HasPrefix(externalRef, serviceDomain) {
return nil, "", fmt.Errorf("externalRef should have prefix %s, got %s", serviceDomain, externalRef)
}
path := strings.TrimPrefix(externalRef, serviceDomain+"/")
return ParseInstanceExternal(path)
}

func ParseInstanceExternal(external string) (parent *InstanceParent, resourceID string, err error) {
tokens := strings.Split(external, "/")
if len(tokens) != 8 || tokens[0] != "projects" || tokens[2] != "locations" || tokens[4] != "clusters" || tokens[6] != "instances" {
return nil, "", fmt.Errorf("format of AlloyDBInstance external=%q was not known (use projects/<projectId>/locations/<location>/clusters/<clusterID>/instances/<instanceID>)", external)
}
parent = &InstanceParent{
clusterName: fmt.Sprintf("%s/%s/%s/%s/%s/%s", tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]),
projectID: tokens[1],
location: tokens[3],
clusterID: tokens[5],
}
resourceID = tokens[7]
return parent, resourceID, nil
Expand Down
2 changes: 0 additions & 2 deletions apis/alloydb/v1alpha1/instance_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ type AlloyDBInstanceSpec struct {
// +required
ClusterRef *refs.AlloyDBClusterRef `json:"clusterRef,omitempty"`

// Immutable.
// Optional. The instanceId of the resource. If not given, the metadata.name will be used.
ResourceID *string `json:"resourceID,omitempty"`

Expand Down Expand Up @@ -160,7 +159,6 @@ type AlloyDBInstanceObservedState struct {

// +genclient
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// TODO(user): make sure the pluralizaiton below is correct
// +kubebuilder:resource:categories=gcp,shortName=gcpalloydbinstance;gcpalloydbinstances
// +kubebuilder:subresource:status
// +kubebuilder:metadata:labels="cnrm.cloud.google.com/tf2crd=true";"cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/stability-level=stable";"cnrm.cloud.google.com/system=true"
Expand Down
27 changes: 11 additions & 16 deletions apis/alloydb/v1beta1/instance_identity.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

const serviceDomain = "//alloydb.googleapis.com"

// InstanceIdentity defines the resource reference to AlloyDBInstance, which "External" field
// holds the GCP identifier for the KRM object.
// InstanceIdentity defines the resource reference to AlloyDBInstance.
type InstanceIdentity struct {
parent *InstanceParent
id string
Expand All @@ -45,18 +42,14 @@ func (i *InstanceIdentity) Parent() *InstanceParent {
return i.parent
}

// AsExternalRef builds a externalRef from a PrivilegedAccessManagerEntitlement.
func (i *InstanceIdentity) AsExternalRef() *string {
er := serviceDomain + "/" + i.String()
return &er
}

type InstanceParent struct {
clusterName string
projectID string
location string
clusterID string
}

func (p *InstanceParent) String() string {
return p.clusterName
return "projects/" + p.projectID + "/locations/" + p.location + "/clusters/" + p.clusterID
}

// New builds a InstanceIdentity from the Config Connector Instance object.
Expand All @@ -81,12 +74,12 @@ func NewInstanceIdentity(ctx context.Context, reader client.Reader, obj *AlloyDB
externalRef := common.ValueOf(obj.Status.ExternalRef)
if externalRef != "" {
// Validate desired with actual
actualParent, actualResourceID, err := ParseInstanceExternalRef(externalRef)
actualParent, actualResourceID, err := ParseInstanceExternal(externalRef)
if err != nil {
return nil, err
}
if actualParent.clusterName != clusterRef.String() {
return nil, fmt.Errorf("spec.clusterRef changed, expect %s, got %s", actualParent.clusterName, clusterRef)
if actualParent.String() != clusterRef.String() {
return nil, fmt.Errorf("spec.clusterRef changed, expect %s, got %s", actualParent.String(), clusterRef)
}
if actualResourceID != resourceID {
return nil, fmt.Errorf("cannot reset `metadata.name` or `spec.resourceID` to %s, since it has already assigned to %s",
Expand All @@ -95,7 +88,9 @@ func NewInstanceIdentity(ctx context.Context, reader client.Reader, obj *AlloyDB
}
return &InstanceIdentity{
parent: &InstanceParent{
clusterName: clusterRef.String(),
projectID: clusterRef.ProjectID,
location: clusterRef.Location,
clusterID: clusterRef.ClusterID,
},
id: resourceID,
}, nil
Expand Down
12 changes: 3 additions & 9 deletions apis/alloydb/v1beta1/instance_reference.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,15 @@ func (r *InstanceRef) NormalizedExternal(ctx context.Context, reader client.Read
return r.External, nil
}

func ParseInstanceExternalRef(externalRef string) (parent *InstanceParent, resourceID string, err error) {
if !strings.HasPrefix(externalRef, serviceDomain) {
return nil, "", fmt.Errorf("externalRef should have prefix %s, got %s", serviceDomain, externalRef)
}
path := strings.TrimPrefix(externalRef, serviceDomain+"/")
return ParseInstanceExternal(path)
}

func ParseInstanceExternal(external string) (parent *InstanceParent, resourceID string, err error) {
tokens := strings.Split(external, "/")
if len(tokens) != 8 || tokens[0] != "projects" || tokens[2] != "locations" || tokens[4] != "clusters" || tokens[6] != "instances" {
return nil, "", fmt.Errorf("format of AlloyDBInstance external=%q was not known (use projects/<projectId>/locations/<location>/clusters/<clusterID>/instances/<instanceID>)", external)
}
parent = &InstanceParent{
clusterName: fmt.Sprintf("%s/%s/%s/%s/%s/%s", tokens[0], tokens[1], tokens[2], tokens[3], tokens[4], tokens[5]),
projectID: tokens[1],
location: tokens[3],
clusterID: tokens[5],
}
resourceID = tokens[7]
return parent, resourceID, nil
Expand Down
2 changes: 0 additions & 2 deletions apis/alloydb/v1beta1/instance_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ type AlloyDBInstanceSpec struct {
// +required
ClusterRef *refs.AlloyDBClusterRef `json:"clusterRef,omitempty"`

// Immutable.
// Optional. The instanceId of the resource. If not given, the metadata.name will be used.
ResourceID *string `json:"resourceID,omitempty"`

Expand Down Expand Up @@ -160,7 +159,6 @@ type AlloyDBInstanceObservedState struct {

// +genclient
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// TODO(user): make sure the pluralizaiton below is correct
// +kubebuilder:resource:categories=gcp,shortName=gcpalloydbinstance;gcpalloydbinstances
// +kubebuilder:subresource:status
// +kubebuilder:metadata:labels="cnrm.cloud.google.com/tf2crd=true";"cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/stability-level=stable";"cnrm.cloud.google.com/system=true"
Expand Down
8 changes: 8 additions & 0 deletions apis/common/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,11 @@ func ValueOf[T any](t *T) T {
}
return *t
}

func LazyPtr[V comparable](v V) *V {
var defaultV V
if v == defaultV {
return nil
}
return &v
}
Loading

0 comments on commit e33dba7

Please sign in to comment.