From b6b5ba44695bd906575b75cd9d98d1cfc8e8c7e2 Mon Sep 17 00:00:00 2001 From: Gemma Hou Date: Tue, 24 Sep 2024 01:35:17 +0000 Subject: [PATCH] handle labels for psc forwarding rule --- .../mockcompute/globalforwardingrulesv1.go | 5 +- .../mockcompute/regionalforwardingrulev1.go | 3 + .../compute/forwardingrule_controller.go | 17 ++- ...balforwardingrulepscgoogleapis.golden.yaml | 2 +- .../_http.log | 101 ++++++++---------- .../create.yaml | 2 +- .../dependencies.yaml | 2 + .../update.yaml | 2 +- tests/e2e/unified_test.go | 1 + 9 files changed, 70 insertions(+), 65 deletions(-) diff --git a/mockgcp/mockcompute/globalforwardingrulesv1.go b/mockgcp/mockcompute/globalforwardingrulesv1.go index 3ffd299e8d..58784486c7 100644 --- a/mockgcp/mockcompute/globalforwardingrulesv1.go +++ b/mockgcp/mockcompute/globalforwardingrulesv1.go @@ -82,6 +82,9 @@ func (s *GlobalForwardingRulesV1) Insert(ctx context.Context, req *pb.InsertGlob if obj.NetworkTier == nil { obj.NetworkTier = PtrTo("PREMIUM") } + if *obj.LoadBalancingScheme == "" { + obj.LoadBalancingScheme = nil + } if isPSCForwardingRule(obj) { var num uint64 = 111111111111 obj.PscConnectionId = &num @@ -158,7 +161,7 @@ func (s *GlobalForwardingRulesV1) Delete(ctx context.Context, req *pb.DeleteGlob var opType *string if isPSCForwardingRule(deleted) { - opType = PtrTo("deletePSCServiceEndpoint") + opType = PtrTo("deletePscForwardingRule") } else { opType = PtrTo("delete") } diff --git a/mockgcp/mockcompute/regionalforwardingrulev1.go b/mockgcp/mockcompute/regionalforwardingrulev1.go index 4c8a5e5172..c1696d8198 100644 --- a/mockgcp/mockcompute/regionalforwardingrulev1.go +++ b/mockgcp/mockcompute/regionalforwardingrulev1.go @@ -82,6 +82,9 @@ func (s *RegionalForwardingRulesV1) Insert(ctx context.Context, req *pb.InsertFo if obj.NetworkTier == nil { obj.NetworkTier = PtrTo("PREMIUM") } + if *obj.LoadBalancingScheme == "" { + obj.LoadBalancingScheme = nil + } // pattern: \d+(?:-\d+)? if obj.PortRange != nil { diff --git a/pkg/controller/direct/compute/forwardingrule_controller.go b/pkg/controller/direct/compute/forwardingrule_controller.go index d9ab853b07..2eeee4a8a6 100644 --- a/pkg/controller/direct/compute/forwardingrule_controller.go +++ b/pkg/controller/direct/compute/forwardingrule_controller.go @@ -294,12 +294,15 @@ func (a *forwardingRuleAdapter) Create(ctx context.Context, createOp *directbase return mapCtx.Err() } forwardingRule.Name = direct.LazyPtr(a.id.forwardingRule) - target := direct.ValueOf(forwardingRule.Target) + forwardingRule.Labels = desired.Labels - // API restriction: Labels are invalid in Private Service Connect Forwarding Rule. - // Config Connector workaround on TF-based resource: https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/944 - if target != "all-apis" && target != "vpc-sc" && !strings.Contains(target, "/serviceAttachments/") { - forwardingRule.Labels = desired.Labels + // API restriction: Cannot set labels during creation(by POST). But it can be set later by PATCH SetLabels. + // API error message: Labels are invalid in Private Service Connect Forwarding Rule. + // See GH issue for details: https://github.com/hashicorp/terraform-provider-google/issues/16255 + target := direct.ValueOf(forwardingRule.Target) + // Remove labels for psc forwarding rule + if target == "all-apis" || target == "vpc-sc" || strings.Contains(target, "/serviceAttachments/") { + forwardingRule.Labels = nil } // Create forwarding rule(labels are not set during Insert) @@ -338,6 +341,10 @@ func (a *forwardingRuleAdapter) Create(ctx context.Context, createOp *directbase } // Set labels for the created forwarding rule + // Add labels back for psc forwarding rule + if target == "all-apis" || target == "vpc-sc" || strings.Contains(target, "/serviceAttachments/") { + forwardingRule.Labels = desired.Labels + } if forwardingRule.Labels != nil { op, err := a.setLabels(ctx, created.LabelFingerprint, forwardingRule.Labels) if err != nil { diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_generated_object_globalforwardingrulepscgoogleapis.golden.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_generated_object_globalforwardingrulepscgoogleapis.golden.yaml index 7728a7326c..83c9032dc8 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_generated_object_globalforwardingrulepscgoogleapis.golden.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_generated_object_globalforwardingrulepscgoogleapis.golden.yaml @@ -17,7 +17,7 @@ spec: description: A global forwarding rule ipAddress: addressRef: - name: default + external: https://www.googleapis.com/compute/v1/projects/${projectId}/global/addresses/default loadBalancingScheme: "" location: global networkRef: diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_http.log b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_http.log index 04f02ccf6f..866189773e 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_http.log +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/_http.log @@ -559,7 +559,6 @@ X-Xss-Protection: 0 "id": "000000000000000000000", "kind": "compute#forwardingRule", "labelFingerprint": "abcdef0123A=", - "loadBalancingScheme": "", "name": "rule${uniqueId}", "network": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/${networkID}", "networkTier": "PREMIUM", @@ -585,7 +584,7 @@ x-goog-request-params: project=${projectId}&resource=rule${uniqueId} "labelFingerprint": "abcdef0123A=", "labels": { "cnrm-test": "true", - "label-one": "value-two", + "label-one": "value-one", "managed-by-cnrm": "true" } } @@ -646,10 +645,9 @@ X-Xss-Protection: 0 "labelFingerprint": "abcdef0123A=", "labels": { "cnrm-test": "true", - "label-one": "value-two", + "label-one": "value-one", "managed-by-cnrm": "true" }, - "loadBalancingScheme": "", "name": "rule${uniqueId}", "network": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/${networkID}", "networkTier": "PREMIUM", @@ -666,44 +664,20 @@ X-Xss-Protection: 0 --- -DELETE https://compute.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/${forwardingRuleID} +POST https://compute.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/${forwardingRuleID}/setLabels Content-Type: application/json User-Agent: kcc/controller-manager -x-goog-request-params: project=${projectId}&forwarding_rule=rule${uniqueId} - -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 +x-goog-request-params: project=${projectId}&resource=rule${uniqueId} { - "id": "000000000000000000000", - "insertTime": "2024-04-01T12:34:56.123456Z", - "kind": "compute#operation", - "name": "${operationID}", - "operationType": "deletePSCServiceEndpoint", - "progress": 0, - "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}", - "startTime": "2024-04-01T12:34:56.123456Z", - "status": "RUNNING", - "targetId": "${forwardingRulesId}", - "targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/rule${uniqueId}", - "user": "user@example.com" + "labelFingerprint": "abcdef0123A=", + "labels": { + "cnrm-test": "true", + "label-one": "value-two", + "managed-by-cnrm": "true" + } } ---- - -GET https://compute.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID} -Content-Type: application/json -User-Agent: kcc/controller-manager -x-goog-request-params: project=${projectId}&operation=${operationID} - 200 OK Cache-Control: private Content-Type: application/json; charset=UTF-8 @@ -721,7 +695,7 @@ X-Xss-Protection: 0 "insertTime": "2024-04-01T12:34:56.123456Z", "kind": "compute#operation", "name": "${operationID}", - "operationType": "deletePSCServiceEndpoint", + "operationType": "setLabels", "progress": 100, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}", "startTime": "2024-04-01T12:34:56.123456Z", @@ -733,9 +707,10 @@ X-Xss-Protection: 0 --- -GET https://compute.googleapis.com/compute/v1/projects/${projectId}/global/addresses/${networkID}?alt=json +GET https://compute.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/${forwardingRuleID} Content-Type: application/json -User-Agent: Terraform/ (+https://www.terraform.io) Terraform-Plugin-SDK/2.10.1 terraform-provider-google-beta/kcc/controller-manager +User-Agent: kcc/controller-manager +x-goog-request-params: project=${projectId}&forwarding_rule=rule${uniqueId} 200 OK Cache-Control: private @@ -749,27 +724,39 @@ X-Frame-Options: SAMEORIGIN X-Xss-Protection: 0 { - "address": "100.100.100.106", - "addressType": "INTERNAL", + "IPAddress": "8.8.8.8", + "IPProtocol": "TCP", "creationTimestamp": "2024-04-01T12:34:56.123456Z", + "description": "A global forwarding rule", + "fingerprint": "abcdef0123A=", "id": "000000000000000000000", - "kind": "compute#address", + "kind": "compute#forwardingRule", "labelFingerprint": "abcdef0123A=", "labels": { "cnrm-test": "true", + "label-one": "value-two", "managed-by-cnrm": "true" }, - "name": "${networkID}", - "network": "projects/${projectId}/global/networks/${networkID}", - "purpose": "PRIVATE_SERVICE_CONNECT", - "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/addresses/${networkID}" + "name": "rule${uniqueId}", + "network": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/${networkID}", + "networkTier": "PREMIUM", + "pscConnectionId": "111111111111", + "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/rule${uniqueId}", + "serviceDirectoryRegistrations": [ + { + "namespace": "goog-psc-${networkID}-${networkID}", + "serviceDirectoryRegion": "us-central1" + } + ], + "target": "all-apis" } --- -DELETE https://compute.googleapis.com/compute/v1/projects/${projectId}/global/addresses/${networkID}?alt=json +DELETE https://compute.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/${forwardingRuleID} Content-Type: application/json -User-Agent: Terraform/ (+https://www.terraform.io) Terraform-Plugin-SDK/2.10.1 terraform-provider-google-beta/kcc/controller-manager +User-Agent: kcc/controller-manager +x-goog-request-params: project=${projectId}&forwarding_rule=rule${uniqueId} 200 OK Cache-Control: private @@ -787,20 +774,22 @@ X-Xss-Protection: 0 "insertTime": "2024-04-01T12:34:56.123456Z", "kind": "compute#operation", "name": "${operationID}", - "operationType": "delete", + "operationType": "deletePscForwardingRule", "progress": 0, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}", "startTime": "2024-04-01T12:34:56.123456Z", "status": "RUNNING", - "targetId": "${addressesId}", - "targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/addresses/${networkID}", + "targetId": "${forwardingRulesId}", + "targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/rule${uniqueId}", "user": "user@example.com" } --- -GET https://compute.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}?alt=json&prettyPrint=false -User-Agent: google-api-go-client/0.5 Terraform/ (+https://www.terraform.io) Terraform-Plugin-SDK/2.10.1 terraform-provider-google-beta/kcc/controller-manager +GET https://compute.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID} +Content-Type: application/json +User-Agent: kcc/controller-manager +x-goog-request-params: project=${projectId}&operation=${operationID} 200 OK Cache-Control: private @@ -819,12 +808,12 @@ X-Xss-Protection: 0 "insertTime": "2024-04-01T12:34:56.123456Z", "kind": "compute#operation", "name": "${operationID}", - "operationType": "delete", + "operationType": "deletePscForwardingRule", "progress": 100, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/operations/${operationID}", "startTime": "2024-04-01T12:34:56.123456Z", "status": "DONE", - "targetId": "${addressesId}", - "targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/addresses/${networkID}", + "targetId": "${forwardingRulesId}", + "targetLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/forwardingRules/rule${uniqueId}", "user": "user@example.com" } \ No newline at end of file diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/create.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/create.yaml index 8a8a2bfd9d..e57b030596 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/create.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/create.yaml @@ -27,6 +27,6 @@ spec: loadBalancingScheme: "" ipAddress: addressRef: - name: default + external: https://www.googleapis.com/compute/v1/projects/${projectId}/global/addresses/default networkRef: name: default diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/dependencies.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/dependencies.yaml index e2c273e5e0..db4d2de894 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/dependencies.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/dependencies.yaml @@ -24,6 +24,8 @@ spec: apiVersion: compute.cnrm.cloud.google.com/v1beta1 kind: ComputeAddress metadata: + annotations: + cnrm.cloud.google.com/deletion-policy: "abandon" name: default spec: location: global diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/update.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/update.yaml index c13ea8658b..ad252bb6db 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/update.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computeforwardingrule/globalforwardingrulepscgoogleapis/update.yaml @@ -27,6 +27,6 @@ spec: loadBalancingScheme: "" ipAddress: addressRef: - name: default + external: https://www.googleapis.com/compute/v1/projects/${projectId}/global/addresses/default networkRef: name: default diff --git a/tests/e2e/unified_test.go b/tests/e2e/unified_test.go index 93dbdf9a22..15da8ebf33 100644 --- a/tests/e2e/unified_test.go +++ b/tests/e2e/unified_test.go @@ -529,6 +529,7 @@ func runScenario(ctx context.Context, t *testing.T, testPause bool, fixture reso addReplacement("fingerprint", "abcdef0123A=") // Matches the mock ip address of Compute forwarding rule addReplacement("IPAddress", "8.8.8.8") + addReplacement("pscConnectionId", "111111111111") // Extract resource targetID numbers from compute operations for _, event := range events {