From 55fec567f92fc3725a522d230a141dc13cea9ec9 Mon Sep 17 00:00:00 2001 From: Gonzalo Servat Date: Fri, 23 Aug 2024 09:19:44 +1000 Subject: [PATCH] Improve handling when enabling/disabling server TLS policy in global target HTTPS proxies (#11496) --- mmv1/products/compute/TargetHttpsProxy.yaml | 7 +++- .../compute_target_https_proxy.go.erb | 10 ++++- ...rce_compute_target_https_proxy_test.go.erb | 41 ++++++++----------- 3 files changed, 32 insertions(+), 26 deletions(-) diff --git a/mmv1/products/compute/TargetHttpsProxy.yaml b/mmv1/products/compute/TargetHttpsProxy.yaml index 0698327cfe7b..2c215dd3c298 100644 --- a/mmv1/products/compute/TargetHttpsProxy.yaml +++ b/mmv1/products/compute/TargetHttpsProxy.yaml @@ -221,7 +221,7 @@ properties: load balancer (classic), this option is not available publicly. - !ruby/object:Api::Type::ResourceRef name: 'serverTlsPolicy' - resource: 'ServerSslPolicy' + resource: 'ServerTlsPolicy' imports: 'selfLink' description: | A URL referring to a networksecurity.ServerTlsPolicy @@ -233,6 +233,11 @@ properties: INTERNAL_SELF_MANAGED and which with EXTERNAL, EXTERNAL_MANAGED loadBalancingScheme consult ServerTlsPolicy documentation. If left blank, communications are not encrypted. + + If you remove this field from your configuration at the same time as + deleting or recreating a referenced ServerTlsPolicy resource, you will + receive a resourceInUseByAnotherResource error. Use lifecycle.create_before_destroy + within the ServerTlsPolicy resource to avoid this. update_verb: :PATCH update_url: 'projects/{{project}}/global/targetHttpsProxies/{{name}}' fingerprint_name: 'fingerprint' diff --git a/mmv1/templates/terraform/encoders/compute_target_https_proxy.go.erb b/mmv1/templates/terraform/encoders/compute_target_https_proxy.go.erb index 168d4a65c5ef..bcf2aa79977f 100644 --- a/mmv1/templates/terraform/encoders/compute_target_https_proxy.go.erb +++ b/mmv1/templates/terraform/encoders/compute_target_https_proxy.go.erb @@ -7,4 +7,12 @@ if _, ok := obj["certificateManagerCertificates"]; ok { obj["sslCertificates"] = obj["certificateManagerCertificates"] delete(obj, "certificateManagerCertificates") } -return obj, nil \ No newline at end of file + +// Send null if serverTlsPolicy is not set. Without this, Terraform would not send any value for `serverTlsPolicy` +// in the "PATCH" payload so if you were to remove a server TLS policy from a target HTTPS proxy, it would NOT remove +// the association. +if _, ok := obj["serverTlsPolicy"]; !ok { + obj["serverTlsPolicy"] = nil +} + +return obj, nil diff --git a/mmv1/third_party/terraform/services/compute/resource_compute_target_https_proxy_test.go.erb b/mmv1/third_party/terraform/services/compute/resource_compute_target_https_proxy_test.go.erb index fabd406d4208..edca74ff66cd 100644 --- a/mmv1/third_party/terraform/services/compute/resource_compute_target_https_proxy_test.go.erb +++ b/mmv1/third_party/terraform/services/compute/resource_compute_target_https_proxy_test.go.erb @@ -95,7 +95,7 @@ func TestAccComputeTargetHttpsProxyServerTlsPolicy_update(t *testing.T) { CheckDestroy: testAccCheckComputeTargetHttpsProxyDestroyProducer(t), Steps: []resource.TestStep{ { - Config: testAccComputeTargetHttpsProxyServerTlsPolicy_full(resourceSuffix), + Config: testAccComputeTargetHttpsProxyWithoutServerTlsPolicy(resourceSuffix), Check: resource.ComposeTestCheckFunc( testAccCheckComputeTargetHttpsProxyExists( t, "google_compute_target_https_proxy.foobar", &proxy), @@ -103,13 +103,21 @@ func TestAccComputeTargetHttpsProxyServerTlsPolicy_update(t *testing.T) { ), }, { - Config: testAccComputeTargetHttpsProxyServerTlsPolicy_update(resourceSuffix), + Config: testAccComputeTargetHttpsProxyWithServerTlsPolicy(resourceSuffix), Check: resource.ComposeTestCheckFunc( testAccCheckComputeTargetHttpsProxyExists( t, "google_compute_target_https_proxy.foobar", &proxy), testAccComputeTargetHttpsProxyHasServerTlsPolicy(t, "tf-test-server-tls-policy-"+resourceSuffix, &proxy), ), }, + { + Config: testAccComputeTargetHttpsProxyWithoutServerTlsPolicy(resourceSuffix), + Check: resource.ComposeTestCheckFunc( + testAccCheckComputeTargetHttpsProxyExists( + t, "google_compute_target_https_proxy.foobar", &proxy), + testAccComputeTargetHttpsProxyHasNullServerTlsPolicy(t, &proxy), + ), + }, }, }) } @@ -422,7 +430,7 @@ resource "google_certificate_manager_dns_authorization" "instance" { `, id, id, id, id, id, id, id, id) } -func testAccComputeTargetHttpsProxyServerTlsPolicy_full(id string) string { +func testAccComputeTargetHttpsProxyWithoutServerTlsPolicy(id string) string { return fmt.Sprintf(` data "google_project" "project" {} @@ -431,7 +439,6 @@ resource "google_compute_target_https_proxy" "foobar" { name = "tf-test-httpsproxy-%s" url_map = google_compute_url_map.foobar.self_link ssl_certificates = [google_compute_ssl_certificate.foobar.self_link] - server_tls_policy = null } resource "google_compute_backend_service" "foobar" { @@ -457,28 +464,10 @@ resource "google_compute_ssl_certificate" "foobar" { private_key = file("test-fixtures/test.key") certificate = file("test-fixtures/test.crt") } - -resource "google_certificate_manager_trust_config" "trust_config" { - name = "tf-test-trust-config-%s" - location = "global" - - allowlisted_certificates { - pem_certificate = file("test-fixtures/cert.pem") - } -} - -resource "google_network_security_server_tls_policy" "server_tls_policy" { - name = "tf-test-server-tls-policy-%s" - - mtls_policy { - client_validation_trust_config = "projects/${data.google_project.project.number}/locations/global/trustConfigs/${google_certificate_manager_trust_config.trust_config.name}" - client_validation_mode = "ALLOW_INVALID_OR_MISSING_CLIENT_CERT" - } -} -`, id, id, id, id, id, id, id) +`, id, id, id, id, id) } -func testAccComputeTargetHttpsProxyServerTlsPolicy_update(id string) string { +func testAccComputeTargetHttpsProxyWithServerTlsPolicy(id string) string { return fmt.Sprintf(` data "google_project" "project" {} @@ -530,6 +519,10 @@ resource "google_network_security_server_tls_policy" "server_tls_policy" { client_validation_trust_config = "projects/${data.google_project.project.number}/locations/global/trustConfigs/${google_certificate_manager_trust_config.trust_config.name}" client_validation_mode = "ALLOW_INVALID_OR_MISSING_CLIENT_CERT" } + + lifecycle { + create_before_destroy = true + } } `, id, id, id, id, id, id, id) }