diff --git a/apis/cloudbuild/v1alpha1/workerpool_types.go b/apis/cloudbuild/v1alpha1/workerpool_types.go index 6397bfdf3a6..8ee3eb6c9ce 100644 --- a/apis/cloudbuild/v1alpha1/workerpool_types.go +++ b/apis/cloudbuild/v1alpha1/workerpool_types.go @@ -68,6 +68,11 @@ type CloudBuildWorkerPoolStatus struct { // +optional ObservedGeneration *int64 `json:"observedGeneration,omitempty"` + /* The GCP URL to the cloudbuild workerpool resource. + The cloudbuild workerpool does not have selfLink. */ + // +optional + ExternalRef *string `json:"externalRef,omitempty"` + /* ObservedState is the state of the resource as most recently observed in GCP. */ // +optional ObservedState *CloudBuildWorkerPoolObservedState `json:"observedState,omitempty"` diff --git a/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go b/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go index 2e9aed9c881..8d095bc6ec0 100644 --- a/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go +++ b/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go @@ -147,6 +147,11 @@ func (in *CloudBuildWorkerPoolStatus) DeepCopyInto(out *CloudBuildWorkerPoolStat *out = new(int64) **out = **in } + if in.ExternalRef != nil { + in, out := &in.ExternalRef, &out.ExternalRef + *out = new(string) + **out = **in + } if in.ObservedState != nil { in, out := &in.ObservedState, &out.ObservedState *out = new(CloudBuildWorkerPoolObservedState) diff --git a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_cloudbuildworkerpools.cloudbuild.cnrm.cloud.google.com.yaml b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_cloudbuildworkerpools.cloudbuild.cnrm.cloud.google.com.yaml index f072f3b0bb2..c063d1655fa 100644 --- a/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_cloudbuildworkerpools.cloudbuild.cnrm.cloud.google.com.yaml +++ b/config/crds/resources/apiextensions.k8s.io_v1_customresourcedefinition_cloudbuildworkerpools.cloudbuild.cnrm.cloud.google.com.yaml @@ -164,6 +164,10 @@ spec: type: string type: object type: array + externalRef: + description: The GCP URL to the cloudbuild workerpool resource. The + cloudbuild workerpool does not have selfLink. + type: string observedGeneration: description: ObservedGeneration is the generation of the resource that was most recently observed by the Config Connector controller. diff --git a/go.mod b/go.mod index df959dae6be..f97489071a8 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ replace github.com/GoogleCloudPlatform/k8s-config-connector/mockgcp => ./mockgcp require ( cloud.google.com/go/apikeys v1.1.5 cloud.google.com/go/cloudbuild v1.16.1 + cloud.google.com/go/compute v1.25.1 cloud.google.com/go/monitoring v1.19.0 cloud.google.com/go/profiler v0.1.0 cloud.google.com/go/resourcemanager v1.9.7 @@ -75,7 +76,6 @@ require ( cloud.google.com/go/auth v0.3.0 // indirect cloud.google.com/go/auth/oauth2adapt v0.2.2 // indirect cloud.google.com/go/bigtable v1.19.0 // indirect - cloud.google.com/go/compute v1.25.1 // indirect cloud.google.com/go/compute/metadata v0.3.0 // indirect cloud.google.com/go/iam v1.1.7 // indirect cloud.google.com/go/longrunning v0.5.6 // indirect diff --git a/pkg/clients/generated/apis/cloudbuild/v1alpha1/cloudbuildworkerpool_types.go b/pkg/clients/generated/apis/cloudbuild/v1alpha1/cloudbuildworkerpool_types.go index 73d62f9f33c..31040b0ae3c 100644 --- a/pkg/clients/generated/apis/cloudbuild/v1alpha1/cloudbuildworkerpool_types.go +++ b/pkg/clients/generated/apis/cloudbuild/v1alpha1/cloudbuildworkerpool_types.go @@ -117,6 +117,10 @@ type CloudBuildWorkerPoolStatus struct { /* Conditions represent the latest available observations of the CloudBuildWorkerPool's current state. */ Conditions []v1alpha1.Condition `json:"conditions,omitempty"` + /* The GCP URL to the cloudbuild workerpool resource. The cloudbuild workerpool does not have selfLink. */ + // +optional + ExternalRef *string `json:"externalRef,omitempty"` + /* ObservedGeneration is the generation of the resource that was most recently observed by the Config Connector controller. If this is equal to metadata.generation, then that means that the current reported status reflects the most recent desired state of the resource. */ // +optional ObservedGeneration *int64 `json:"observedGeneration,omitempty"` diff --git a/pkg/clients/generated/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go b/pkg/clients/generated/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go index 12492f78c11..33ea3b77250 100644 --- a/pkg/clients/generated/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/clients/generated/apis/cloudbuild/v1alpha1/zz_generated.deepcopy.go @@ -131,6 +131,11 @@ func (in *CloudBuildWorkerPoolStatus) DeepCopyInto(out *CloudBuildWorkerPoolStat *out = make([]k8sv1alpha1.Condition, len(*in)) copy(*out, *in) } + if in.ExternalRef != nil { + in, out := &in.ExternalRef, &out.ExternalRef + *out = new(string) + **out = **in + } if in.ObservedGeneration != nil { in, out := &in.ObservedGeneration, &out.ObservedGeneration *out = new(int64) diff --git a/pkg/controller/direct/cloudbuild/externalresource.go b/pkg/controller/direct/cloudbuild/externalresource.go new file mode 100644 index 00000000000..149a7e0ddb8 --- /dev/null +++ b/pkg/controller/direct/cloudbuild/externalresource.go @@ -0,0 +1,51 @@ +/* +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 cloudbuild + +import ( + "strings" + + cloudbuildpb "cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb" + "github.com/GoogleCloudPlatform/k8s-config-connector/pkg/controller/direct/externalresource" +) + +func NewResourceRef(gcpObj *cloudbuildpb.WorkerPool) *ResourceRef { + return &ResourceRef{ + externalresource.New("https://cloudbuild.googleapis.com/v1/", gcpObj), + } +} + +type ResourceRef struct { + *externalresource.ExternalResourceReference +} + +func (e *ResourceRef) GetResourceID() string { + segments := strings.Split(*e.Get(), "/workerPools/") + return segments[1] +} + +func (e *ResourceRef) GetLocation() string { + segments := strings.Split(*e.Get(), "/locations/") + segment := strings.Split(segments[1], "/workerPools/") + return segment[0] +} + +func (e *ResourceRef) GetProject() string { + segments := strings.Split(*e.Get(), "/projects/") + segment := strings.Split(segments[1], "/locations/") + return segment[0] +} diff --git a/pkg/controller/direct/cloudbuild/workerpool_controller.go b/pkg/controller/direct/cloudbuild/workerpool_controller.go index 41e998dace1..e3c680ded2c 100644 --- a/pkg/controller/direct/cloudbuild/workerpool_controller.go +++ b/pkg/controller/direct/cloudbuild/workerpool_controller.go @@ -197,10 +197,14 @@ func (a *Adapter) Create(ctx context.Context, u *unstructured.Unstructured) erro } status.ObservedState.CreateTime = ToOpenAPIDateTime(created.GetCreateTime()) status.ObservedState.UpdateTime = ToOpenAPIDateTime(created.GetUpdateTime()) + status.ExternalRef = NewResourceRef(created).Get() return setStatus(u, status) } func (a *Adapter) Update(ctx context.Context, u *unstructured.Unstructured) error { + if err := a.ValidateExternalResource(); err != nil { + return err + } updateMask := &fieldmaskpb.FieldMask{} @@ -283,6 +287,8 @@ func (a *Adapter) Update(ctx context.Context, u *unstructured.Unstructured) erro } status.ObservedState.CreateTime = ToOpenAPIDateTime(updated.GetCreateTime()) status.ObservedState.UpdateTime = ToOpenAPIDateTime(updated.GetUpdateTime()) + // This value should not be updated. Just in case. + status.ExternalRef = NewResourceRef(updated).Get() return setStatus(u, status) } @@ -292,9 +298,10 @@ func (a *Adapter) Export(ctx context.Context) (*unstructured.Unstructured, error // Delete implements the Adapter interface. func (a *Adapter) Delete(ctx context.Context) (bool, error) { - if a.resourceID == "" { - return false, nil + if err := a.ValidateExternalResource(); err != nil { + return false, err } + req := &cloudbuildpb.DeleteWorkerPoolRequest{Name: a.fullyQualifiedName(), AllowMissing: true} op, err := a.gcpClient.DeleteWorkerPool(ctx, req) if err != nil { @@ -313,6 +320,38 @@ func (a *Adapter) Delete(ctx context.Context) (bool, error) { return true, nil } +// ValidateExternalResource compares the `status.externalRef` with the `spec` Project, Location and +// (external) resourceID to make sure those fields are immutable and matches the previous deployed value. +func (a *Adapter) ValidateExternalResource() error { + actualExternalRef := NewResourceRef(a.actual) + + desiredExternalRef := "https://cloudbuild.googleapis.com/v1/" + a.fullyQualifiedName() + if *actualExternalRef.Get() == desiredExternalRef { + return nil + } + + // Give user guidance on how to fix the CloudBuildWorkerPool spec. + if a.desired.Spec.ResourceID != nil && *a.desired.Spec.ResourceID != actualExternalRef.GetResourceID() { + return fmt.Errorf("`spec.resourceID` is immutable field, expect %s, got %s", + actualExternalRef.GetResourceID(), *a.desired.Spec.ResourceID) + } + if a.desired.Spec.Location != actualExternalRef.GetLocation() { + return fmt.Errorf("`spec.location` is immutable field, expect %s, got %s", + actualExternalRef.GetLocation(), a.desired.Spec.Location) + } + // TODO: Some Selflink may change the project from projectID to projectNum. + /* + if a.desired.Spec.ProjectRef.Name != "" { + return fmt.Errorf("`spec.projectRef.name` is immutable field, expect project %s", + actualExternalRef.GetProject()) + } + if a.desired.Spec.ProjectRef.External != "" { + return fmt.Errorf("`spec.projectRef.external` is immutable field, expect project %s", + actualExternalRef.GetProject()) + }*/ + return nil +} + func (a *Adapter) fullyQualifiedName() string { return fmt.Sprintf("projects/%s/locations/%s/workerPools/%s", a.projectID, a.location, a.resourceID) } diff --git a/pkg/controller/direct/directbase/directbase_controller.go b/pkg/controller/direct/directbase/directbase_controller.go index f0884b4feff..938320754ae 100644 --- a/pkg/controller/direct/directbase/directbase_controller.go +++ b/pkg/controller/direct/directbase/directbase_controller.go @@ -225,6 +225,18 @@ func (r *reconcileContext) doReconcile(ctx context.Context, u *unstructured.Unst return false, r.handleUpdateFailed(ctx, u, err) } + // To create, update or delete the GCP object, we need to get the GCP object first. + // Because the object contains the cloud service information like `selfLink` `ID` required to validate + // the resource uniqueness before updating/deleting. + existsAlready, err := adapter.Find(ctx) + if err != nil { + if unwrappedErr, ok := lifecyclehandler.CausedByUnresolvableDeps(err); ok { + logger.Info(unwrappedErr.Error(), "resource", k8s.GetNamespacedName(u)) + return r.handleUnresolvableDeps(ctx, u, unwrappedErr) + } + return false, r.handleUpdateFailed(ctx, u, err) + } + defer execution.RecoverWithInternalError(&err) if !u.GetDeletionTimestamp().IsZero() { if !k8s.HasFinalizer(u, k8s.ControllerFinalizerName) { @@ -255,14 +267,6 @@ func (r *reconcileContext) doReconcile(ctx context.Context, u *unstructured.Unst return false, r.handleDeleted(ctx, u) } - existsAlready, err := adapter.Find(ctx) - if err != nil { - if unwrappedErr, ok := lifecyclehandler.CausedByUnresolvableDeps(err); ok { - logger.Info(unwrappedErr.Error(), "resource", k8s.GetNamespacedName(u)) - return r.handleUnresolvableDeps(ctx, u, unwrappedErr) - } - return false, r.handleUpdateFailed(ctx, u, err) - } k8s.EnsureFinalizers(u, k8s.ControllerFinalizerName, k8s.DeletionDefenderFinalizerName) // set the etag to an empty string, since IAMPolicy is the authoritative intent, KCC wants to overwrite the underlying policy regardless diff --git a/pkg/controller/direct/externalresource/resource.go b/pkg/controller/direct/externalresource/resource.go new file mode 100644 index 00000000000..f9280674f72 --- /dev/null +++ b/pkg/controller/direct/externalresource/resource.go @@ -0,0 +1,64 @@ +/* +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 externalresource + +import ( + "reflect" +) + +type ExternalResourceReference struct { + hasSelfLink bool + externalRef string +} + +type GCPResource interface { + GetName() string +} + +// New builds the External Reference for CloudBuildWorkerPool when the object already exists on the GCP server. +// Ideally, it expects to use the GCP object's selfLink value as the external reference. However, if +// the resource does not have `selfLink` (like CloudBuildWorkerPool), it builds the URL from the GCP object. +func New(host string, gcpObj GCPResource) *ExternalResourceReference { + baseExternalRef := &ExternalResourceReference{ + hasSelfLink: false, + externalRef: host + gcpObj.GetName(), + } + // Ignore the actual GCP proto, get the `selfLink` in general. + s := reflect.ValueOf(gcpObj).Elem() + if s.Kind() != reflect.Struct { + return baseExternalRef + } + + selfLink := s.FieldByName("SelfLink") + + if !selfLink.IsValid() || selfLink.IsZero() { + return baseExternalRef + } + switch selfLink.Kind() { + case reflect.String: + baseExternalRef.hasSelfLink = true + baseExternalRef.externalRef = selfLink.String() + case reflect.Pointer: + baseExternalRef.hasSelfLink = true + baseExternalRef.externalRef = selfLink.Elem().String() + } + return baseExternalRef +} + +func (e *ExternalResourceReference) Get() *string { + return &e.externalRef +} diff --git a/pkg/controller/direct/externalresource/resource_test.go b/pkg/controller/direct/externalresource/resource_test.go new file mode 100644 index 00000000000..07466f70b15 --- /dev/null +++ b/pkg/controller/direct/externalresource/resource_test.go @@ -0,0 +1,61 @@ +/* +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 externalresource + +import ( + "testing" + + "cloud.google.com/go/cloudbuild/apiv1/v2/cloudbuildpb" + "cloud.google.com/go/compute/apiv1/computepb" +) + +var ( + networkSelfLink = "https://www.googleapis.com/mockservice/v1/projects/yuwenma-gke-dev/global/networks/computenetwork-52ldcmpbgxfmizhrk74q" +) + +func TestNew(t *testing.T) { + tests := []struct { + name string + gcpResource GCPResource + expectedHasSelfLink bool + expectedExternalRef string + }{ + { + name: "cloudbuild workerpool, no selfLink", + gcpResource: &cloudbuildpb.WorkerPool{Name: "projects/mock-project/locations/us-central1/workerPools/mock-pool"}, + expectedHasSelfLink: false, + expectedExternalRef: "https://www.googleapis.com/mockservice/v1/projects/mock-project/locations/us-central1/workerPools/mock-pool", + }, + { + name: "compute network, has seflLink as string pointer", + gcpResource: &computepb.Network{SelfLink: &networkSelfLink}, + expectedHasSelfLink: true, + expectedExternalRef: networkSelfLink, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + gcpObj := test.gcpResource + actualRef := New("https://www.googleapis.com/mockservice/v1/", gcpObj) + if actualRef.hasSelfLink != test.expectedHasSelfLink { + t.Errorf("expected %v, got %v", test.expectedHasSelfLink, actualRef.hasSelfLink) + } + if actualRef.externalRef != test.expectedExternalRef { + t.Errorf("expected %v, got %v", test.expectedExternalRef, actualRef.externalRef) + } + }) + } +} diff --git a/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_generated_object_cloudbuildworkerpool.golden.yaml b/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_generated_object_cloudbuildworkerpool.golden.yaml index 3acbb4ad51a..e33ebcdb173 100644 --- a/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_generated_object_cloudbuildworkerpool.golden.yaml +++ b/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_generated_object_cloudbuildworkerpool.golden.yaml @@ -30,6 +30,7 @@ status: reason: UpToDate status: "True" type: Ready + externalRef: https://cloudbuild.googleapis.com/v1/projects/${projectId}/locations/us-central1/workerPools/cloudbuildworkerpool-${uniqueId} observedGeneration: 2 observedState: createTime: "1970-01-01T00:00:00Z" diff --git a/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_http.log b/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_http.log index d38bcce2cff..269e5d2a883 100644 --- a/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_http.log +++ b/pkg/test/resourcefixture/testdata/basic/cloudbuild/v1alpha1/cloudbuildworkerpool/_http.log @@ -734,6 +734,40 @@ X-Xss-Protection: 0 --- +GET https://cloudbuild.googleapis.com/v1/projects/${projectId}/locations/us-central1/workerPools/cloudbuildworkerpool-${uniqueId}?%24alt=json%3Benum-encoding%3Dint +Content-Type: application/json +x-goog-request-params: location=us-central1 + +200 OK +Cache-Control: private +Content-Type: application/json; charset=UTF-8 +Server: ESF +Vary: Origin +Vary: X-Origin +Vary: Referer +X-Content-Type-Options: nosniff +X-Frame-Options: SAMEORIGIN +X-Xss-Protection: 0 + +{ + "displayName": "New CloudBuild WorkerPool", + "name": "projects/${projectId}/locations/us-central1/workerPools/cloudbuildworkerpool-${uniqueId}", + "privatePoolV1Config": { + "networkConfig": { + "egressOption": "NO_PUBLIC_EGRESS", + "peeredNetwork": "projects/${projectId}/global/networks/computenetwork-${uniqueId}", + "peeredNetworkIpRange": "/29" + }, + "workerConfig": { + "diskSizeGb": "100", + "machineType": "e2-medium" + } + }, + "updateTime": "2024-04-01T12:34:56.123456Z" +} + +--- + DELETE https://cloudbuild.googleapis.com/v1/projects/${projectId}/locations/us-central1/workerPools/cloudbuildworkerpool-${uniqueId}?%24alt=json%3Benum-encoding%3Dint&allowMissing=true Content-Type: application/json x-goog-request-params: location=us-central1