refactor: Envgroup API and controller

apis/apigee/v1alpha1/environmentgroup_identity.go

@@ -101,7 +101,7 @@ func NewGoogleCloudApigeeV1EnvironmentGroupIdentity(ctx context.Context, reader
 func ParseGoogleCloudApigeeV1EnvironmentGroupExternal(external string) (parent *GoogleCloudApigeeV1EnvironmentGroupParent, resourceID string, err error) {
 	tokens := strings.Split(external, "/")
 	if len(tokens) != 4 || tokens[0] != "organizations" || tokens[2] != "envgroups" {
-		return nil, "", fmt.Errorf("external should be organizations/<organization>/envgroups/<envgroup>, got %s",
+		return nil, "", fmt.Errorf("format of ApigeeEnvgroup external=%q was not known (use organizations/{{organization}}/envgroups/{{envgroup}})",
 			external)
 	}
 	parent = &GoogleCloudApigeeV1EnvironmentGroupParent{

apis/apigee/v1alpha1/environmentgroup_reference.go

@@ -32,7 +32,7 @@ var _ refsv1beta1.ExternalNormalizer = &GoogleCloudApigeeV1EnvironmentGroupRef{}
 // holds the GCP identifier for the KRM object.
 type GoogleCloudApigeeV1EnvironmentGroupRef struct {
 	// A reference to an externally managed ApigeeEnvgroup resource.
-	// Should be in the format "projects/<projectID>/locations/<location>/googlecloudapigeev1environmentgroups/<googlecloudapigeev1environmentgroupID>".
+	// Should be in the format "organizations/{{organization}}/envgroups/{{envgroup}}".
 	External string `json:"external,omitempty"`
 
 	// The name of a ApigeeEnvgroup resource.

apis/apigee/v1alpha1/environmentgroup_types.go

@@ -31,8 +31,6 @@ type Parent struct {
 // +kcc:proto=mockgcp.cloud.apigee.v1.GoogleCloudApigeeV1EnvironmentGroup
 type ApigeeEnvgroupSpec struct {
 	Parent `json:",inline"`
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="ResourceID field is immutable"
-	// Immutable.
 	// The ApigeeEnvgroup name. If not given, the metadata.name will be used.
 	ResourceID *string `json:"resourceID,omitempty"`
 	// Host names for this environment group.
@@ -71,7 +69,6 @@ type ApigeeEnvgroupObservedState 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=gcpapigeeenvgroup;gcpapigeeenvgroups
 // +kubebuilder:subresource:status
 // +kubebuilder:metadata:labels="cnrm.cloud.google.com/managed-by-kcc=true";"cnrm.cloud.google.com/system=true"

apis/refs/v1alpha1/organizationref.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"strings"
 
+	"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/runtime/schema"
@@ -84,7 +85,7 @@ func ResolveOrganization(ctx context.Context, reader client.Reader, src client.O
 
 	if err := reader.Get(ctx, key, organization); err != nil {
 		if apierrors.IsNotFound(err) {
-			return nil, fmt.Errorf("referenced Organization %v not found", key)
+			return nil, k8s.NewReferenceNotFoundError(organization.GroupVersionKind(), key)
 		}
 		return nil, fmt.Errorf("error reading referenced Organization %v: %w", key, err)
 	}

config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_apigeeenvgroups.apigee.cnrm.cloud.google.com.yaml

@@ -92,12 +92,9 @@ spec:
                     type: string
                 type: object
               resourceID:
-                description: Immutable. The ApigeeEnvgroup name. If not given, the
-                  metadata.name will be used.
+                description: The ApigeeEnvgroup name. If not given, the metadata.name
+                  will be used.
                 type: string
-                x-kubernetes-validations:
-                - message: ResourceID field is immutable
-                  rule: self == oldSelf
             required:
             - organizationRef
             type: object

dev/tools/controllerbuilder/pkg/commands/generatetypes/generatetypescommand.go

@@ -173,10 +173,6 @@ func RunGenerateCRD(ctx context.Context, o *GenerateCRDOptions) error {
 			return err
 		}
 
-		if err := typeGenerator.WriteVisitedMessages(); err != nil {
-			return err
-		}
-
 		if o.SkipScaffoldFiles {
 			log.Info("skipping scaffolding type, refs and identity files", "resource", resource.ProtoName)
 		} else {
@@ -208,6 +204,10 @@ func RunGenerateCRD(ctx context.Context, o *GenerateCRDOptions) error {
 		}
 	}
 
+	if err := typeGenerator.WriteVisitedMessages(); err != nil {
+		return err
+	}
+
 	addCopyright := true
 	if err := typeGenerator.WriteFiles(addCopyright); err != nil {
 		return err

pkg/controller/direct/apigee/envgroup_controller.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"sort"
 	"strings"
 	"time"
 
@@ -158,17 +159,23 @@ func (a *Adapter) Create(ctx context.Context, createOp *directbase.CreateOperati
 func (a *Adapter) Update(ctx context.Context, updateOp *directbase.UpdateOperation) error {
 	log := klog.FromContext(ctx).WithName(ctrlName)
 	log.V(2).Info("patching ApigeeEnvgroup", a.fullyQualifiedName())
-
+	mapCtx := &direct.MapContext{}
 	updateMask := fieldmaskpb.FieldMask{}
 
-	if !reflect.DeepEqual(a.desired.Hostnames, a.actual.Hostnames) {
+	// Sorts the Hostname lists so that the comparison is deterministic
+	if !reflect.DeepEqual(asSortedCopy(a.desired.Hostnames), asSortedCopy(a.actual.Hostnames)) {
 		log.V(2).Info("change detected: hostnames")
 		updateMask.Paths = append(updateMask.Paths, "hostnames")
 	}
 
 	if len(updateMask.Paths) == 0 {
 		log.V(2).Info("no field needs update", "name", a.id)
-		return nil
+		status := &krm.ApigeeEnvgroupStatus{}
+		status.ObservedState = ApigeeEnvgroupObservedState_FromApi(mapCtx, a.actual)
+		if mapCtx.Err() != nil {
+			return mapCtx.Err()
+		}
+		return updateOp.UpdateStatus(ctx, status, nil)
 	}
 
 	clusterName := a.id.String()
@@ -187,7 +194,6 @@ func (a *Adapter) Update(ctx context.Context, updateOp *directbase.UpdateOperati
 	log.V(2).Info("successfully updated ApigeeEnvgroup", "ApigeeEnvgroup", updated)
 
 	status := &krm.ApigeeEnvgroupStatus{}
-	mapCtx := &direct.MapContext{}
 	status.ObservedState = ApigeeEnvgroupObservedState_FromApi(mapCtx, updated)
 	if mapCtx.Err() != nil {
 		return mapCtx.Err()
@@ -262,3 +268,11 @@ func (a *Adapter) waitForOp(ctx context.Context, op *api.GoogleLongrunningOperat
 func (a *Adapter) fullyQualifiedName() string {
 	return a.id.String()
 }
+
+func asSortedCopy(in []string) []string {
+	out := make([]string, len(in))
+	copy(out, in)
+	sort.Strings(out)
+
+	return out
+}

pkg/controller/direct/apigee/envgroup_mappings.go

@@ -32,6 +32,18 @@ func ApigeeEnvgroupObservedState_FromApi(mapCtx *direct.MapContext, in *api.Goog
 	return out
 }
 
+func ApigeeEnvgroupObservedState_ToApi(mapCtx *direct.MapContext, in *krm.ApigeeEnvgroupObservedState) *api.GoogleCloudApigeeV1EnvironmentGroup {
+	if in == nil {
+		return nil
+	}
+	out := &api.GoogleCloudApigeeV1EnvironmentGroup{}
+	out.CreatedAt = direct.ValueOf(in.CreatedAt)
+	out.LastModifiedAt = direct.ValueOf(in.LastModifiedAt)
+	out.Name = direct.ValueOf(in.Name)
+	out.State = direct.ValueOf(in.State)
+	return out
+}
+
 func ApigeeEnvgroupSpec_FromApi(mapCtx *direct.MapContext, in *api.GoogleCloudApigeeV1EnvironmentGroup) *krm.ApigeeEnvgroupSpec {
 	if in == nil {
 		return nil