diff --git a/mockgcp/mock_http_roundtrip.go b/mockgcp/mock_http_roundtrip.go index 77bd0c28bc..7b8fc0e6f8 100644 --- a/mockgcp/mock_http_roundtrip.go +++ b/mockgcp/mock_http_roundtrip.go @@ -324,6 +324,15 @@ func (m *mockRoundTripper) prefilterRequest(req *http.Request) error { req.Body = io.NopCloser(bytes.NewBuffer(b)) } + } else { + // When sending a delete request for a ComputeFirewallPolicyRule resource, + // The request URL looks like POST https://compute.googleapis.com/compute/v1/locations/global/firewallPolicies/${firewallPolicyID}/removeRule. + // It's uncommon to use POST requests for delete operations, and a nil request body for POST method is unexpected, + // I got the "missing form body" error. Ref: https://go.dev/src/net/http/request.go?s=41070:41129 line 1340 + // So instead of sending a nil request body, send an empty request body to ensure successful processing of the remove rule request. + body := &bytes.Buffer{} + b := body.Bytes() + req.Body = io.NopCloser(bytes.NewBuffer(b)) } return nil } diff --git a/mockgcp/mockcompute/firewallpoliciesv1.go b/mockgcp/mockcompute/firewallpoliciesv1.go index 4c9ec1a0c9..d9ea6768d0 100644 --- a/mockgcp/mockcompute/firewallpoliciesv1.go +++ b/mockgcp/mockcompute/firewallpoliciesv1.go @@ -263,6 +263,7 @@ func (s *FirewallPoliciesV1) PatchRule(ctx context.Context, req *pb.PatchRuleFir return obj, nil }) } + func (s *FirewallPoliciesV1) RemoveRule(ctx context.Context, req *pb.RemoveRuleFirewallPolicyRequest) (*pb.Operation, error) { reqName := "locations/global/firewallPolicies/" + req.GetFirewallPolicy() name, err := s.parseFirewallPolicyName(reqName) diff --git a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go index 115a71e84f..b811338249 100644 --- a/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go +++ b/pkg/controller/direct/compute/firewallpolicyrule/firewallpolicyrule_controller.go @@ -255,17 +255,14 @@ func (a *firewallPolicyRuleAdapter) Export(ctx context.Context) (*unstructured.U // Delete implements the Adapter interface. func (a *firewallPolicyRuleAdapter) Delete(ctx context.Context, deleteOp *directbase.DeleteOperation) (bool, error) { - log := klog.FromContext(ctx).WithName(ctrlName) log.V(2).Info("deleting ComputeFirewallPolicyRule", "priority", a.priority) - var err error - op := &gcp.Operation{} - req := &computepb.RemoveRuleFirewallPolicyRequest{ + delReq := &computepb.RemoveRuleFirewallPolicyRequest{ FirewallPolicy: a.firewallPolicy, Priority: direct.PtrTo(int32(a.priority)), } - op, err = a.firewallPoliciesClient.RemoveRule(ctx, req) + op, err := a.firewallPoliciesClient.RemoveRule(ctx, delReq) if err != nil { return false, fmt.Errorf("deleting ComputeFirewallPolicyRule %d: %w", a.priority, err) diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml index 95335ff087..4893b524e3 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_generated_object_computefirewallpolicyrule-egress-full.golden.yaml @@ -48,4 +48,4 @@ status: type: Ready kind: compute#firewallPolicyRule observedGeneration: 2 - ruleTupleCount: 110 + ruleTupleCount: 4 diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_http.log b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_http.log index 5817ee2cd4..246637e3be 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_http.log +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-egress-full/_http.log @@ -304,7 +304,6 @@ X-Xss-Protection: 0 "name": "network-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-${uniqueId}", @@ -440,7 +439,6 @@ X-Xss-Protection: 0 "name": "network-2-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-2-${uniqueId}", @@ -536,99 +534,6 @@ X-Xss-Protection: 0 --- -GET https://iam.googleapis.com/v1/projects/${projectId}/serviceAccounts/sa-${uniqueId}@${projectId}.iam.gserviceaccount.com?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 - -404 Not Found -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 - -{ - "error": { - "code": 404, - "errors": [ - { - "domain": "global", - "message": "Service account projects/${projectId}/serviceAccounts/sa-${uniqueId}@${projectId}.iam.gserviceaccount.com does not exist.", - "reason": "notFound" - } - ], - "message": "Service account projects/${projectId}/serviceAccounts/sa-${uniqueId}@${projectId}.iam.gserviceaccount.com does not exist.", - "status": "NOT_FOUND" - } -} - ---- - -POST https://iam.googleapis.com/v1/projects/${projectId}/serviceAccounts?alt=json&prettyPrint=false -Content-Type: application/json -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 - -{ - "accountId": "sa-${uniqueId}", - "serviceAccount": {} -} - -409 Conflict -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 - -{ - "error": { - "code": 409, - "errors": [ - { - "domain": "global", - "message": "Service account sa-${uniqueId} already exists within project projects/${projectId}.", - "reason": "alreadyExists" - } - ], - "message": "Service account sa-${uniqueId} already exists within project projects/${projectId}.", - "status": "ALREADY_EXISTS" - } -} - ---- - -GET https://iam.googleapis.com/v1/projects/${projectId}/serviceAccounts/sa-${uniqueId}@${projectId}.iam.gserviceaccount.com?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 - -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 - -{ - "email": "sa-${uniqueId}@${projectId}.iam.gserviceaccount.com", - "etag": "abcdef0123A=", - "name": "projects/${projectId}/serviceAccounts/sa-${uniqueId}@${projectId}.iam.gserviceaccount.com", - "oauth2ClientId": "888888888888888888888", - "projectId": "${projectId}", - "uniqueId": "111111111111111111111" -} - ---- - GET https://iam.googleapis.com/v1/projects/${projectId}/serviceAccounts/sa-2-${uniqueId}@${projectId}.iam.gserviceaccount.com?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 @@ -735,13 +640,6 @@ X-Xss-Protection: 0 { "error": { "code": 400, - "errors": [ - { - "domain": "global", - "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000.", - "reason": "invalid" - } - ], "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000." } } @@ -905,7 +803,7 @@ X-Xss-Protection: 0 ] }, "priority": 9000, - "ruleTupleCount": 109, + "ruleTupleCount": 4, "targetResources": [ "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-${uniqueId}" ], @@ -1074,7 +972,7 @@ X-Xss-Protection: 0 ] }, "priority": 9000, - "ruleTupleCount": 110, + "ruleTupleCount": 4, "targetResources": [ "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-2-${uniqueId}" ], @@ -1171,13 +1069,6 @@ X-Xss-Protection: 0 { "error": { "code": 400, - "errors": [ - { - "domain": "global", - "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000.", - "reason": "invalid" - } - ], "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000." } } @@ -1293,7 +1184,6 @@ X-Xss-Protection: 0 "name": "network-2-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-2-${uniqueId}", @@ -1389,7 +1279,6 @@ X-Xss-Protection: 0 "name": "network-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-${uniqueId}", diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml index 65e5585f7f..0d2386e0df 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_generated_object_computefirewallpolicyrule-ingress-full.golden.yaml @@ -48,4 +48,4 @@ status: type: Ready kind: compute#firewallPolicyRule observedGeneration: 2 - ruleTupleCount: 110 + ruleTupleCount: 4 diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_http.log b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_http.log index 17b6fb2fbf..7a13ce286b 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_http.log +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-ingress-full/_http.log @@ -304,7 +304,6 @@ X-Xss-Protection: 0 "name": "network-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-${uniqueId}", @@ -440,7 +439,6 @@ X-Xss-Protection: 0 "name": "network-2-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-2-${uniqueId}", @@ -642,13 +640,6 @@ X-Xss-Protection: 0 { "error": { "code": 400, - "errors": [ - { - "domain": "global", - "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000.", - "reason": "invalid" - } - ], "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000." } } @@ -812,7 +803,7 @@ X-Xss-Protection: 0 ] }, "priority": 9000, - "ruleTupleCount": 109, + "ruleTupleCount": 4, "targetResources": [ "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-${uniqueId}" ], @@ -981,7 +972,7 @@ X-Xss-Protection: 0 ] }, "priority": 9000, - "ruleTupleCount": 110, + "ruleTupleCount": 4, "targetResources": [ "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-2-${uniqueId}" ], @@ -1078,13 +1069,6 @@ X-Xss-Protection: 0 { "error": { "code": 400, - "errors": [ - { - "domain": "global", - "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000.", - "reason": "invalid" - } - ], "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000." } } @@ -1200,7 +1184,6 @@ X-Xss-Protection: 0 "name": "network-2-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-2-${uniqueId}", @@ -1296,7 +1279,6 @@ X-Xss-Protection: 0 "name": "network-${uniqueId}", "networkFirewallPolicyEnforcementOrder": "AFTER_CLASSIC_FIREWALL", "routingConfig": { - "bgpBestPathSelectionMode": "LEGACY", "routingMode": "REGIONAL" }, "selfLink": "https://www.googleapis.com/compute/v1/projects/${projectId}/global/networks/network-${uniqueId}", diff --git a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_http.log b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_http.log index b9c2b34321..c08bdcdb0b 100644 --- a/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_http.log +++ b/pkg/test/resourcefixture/testdata/basic/compute/v1beta1/computefirewallpolicyrule/computefirewallpolicyrule-minimal/_http.log @@ -196,13 +196,6 @@ X-Xss-Protection: 0 { "error": { "code": 400, - "errors": [ - { - "domain": "global", - "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000.", - "reason": "invalid" - } - ], "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000." } } @@ -346,8 +339,7 @@ x-goog-request-params: firewall_policy=${firewallPolicyId} "srcIpRanges": [ "10.100.0.1/32" ] - }, - "priority": 9000 + } } 200 OK @@ -535,13 +527,6 @@ X-Xss-Protection: 0 { "error": { "code": 400, - "errors": [ - { - "domain": "global", - "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000.", - "reason": "invalid" - } - ], "message": "Invalid value for field 'priority': '9000'. The firewall policy does not contain a rule at priority 9000." } }