From d9abcd8a9f2a5c8fc03cf7cb1fa56e2c4cc8c91c Mon Sep 17 00:00:00 2001 From: Vincent Roseberry Date: Wed, 19 Jul 2017 13:49:14 -0700 Subject: [PATCH] Support path-only when referencing ssl certificate in compute_target_https_proxy (#210) --- google/resource_compute_target_https_proxy.go | 84 +++--------- ...esource_compute_target_https_proxy_test.go | 120 ++++++++++++++---- .../compute_target_https_proxy.html.markdown | 3 +- 3 files changed, 117 insertions(+), 90 deletions(-) diff --git a/google/resource_compute_target_https_proxy.go b/google/resource_compute_target_https_proxy.go index 7ba080e4..a65dce47 100644 --- a/google/resource_compute_target_https_proxy.go +++ b/google/resource_compute_target_https_proxy.go @@ -7,6 +7,12 @@ import ( "github.com/hashicorp/terraform/helper/schema" "google.golang.org/api/compute/v1" + "regexp" +) + +const ( + sslCertificateRegex = "projects/(.+)/global/sslCertificates/(.+)$" + canonicalSslCertificateTemplate = "https://www.googleapis.com/compute/v1/projects/%s/global/sslCertificates/%s" ) func resourceComputeTargetHttpsProxy() *schema.Resource { @@ -26,7 +32,11 @@ func resourceComputeTargetHttpsProxy() *schema.Resource { "ssl_certificates": &schema.Schema{ Type: schema.TypeList, Required: true, - Elem: &schema.Schema{Type: schema.TypeString}, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validateRegexp(sslCertificateRegex), + StateFunc: toCanonicalSslCertificate, + }, }, "url_map": &schema.Schema{ @@ -129,51 +139,9 @@ func resourceComputeTargetHttpsProxyUpdate(d *schema.ResourceData, meta interfac } if d.HasChange("ssl_certificates") { - proxy, err := config.clientCompute.TargetHttpsProxies.Get( - project, d.Id()).Do() - - _old, _new := d.GetChange("ssl_certificates") - _oldCerts := _old.([]interface{}) - _newCerts := _new.([]interface{}) - current := proxy.SslCertificates - - _oldMap := make(map[string]bool) - _newMap := make(map[string]bool) - - for _, v := range _oldCerts { - _oldMap[v.(string)] = true - } - - for _, v := range _newCerts { - _newMap[v.(string)] = true - } - - sslCertificates := make([]string, 0) - // Only modify certificates in one of our old or new states - for _, v := range current { - _, okOld := _oldMap[v] - _, okNew := _newMap[v] - - // we deleted the certificate - if okOld && !okNew { - continue - } - - sslCertificates = append(sslCertificates, v) - - // Keep track of the fact that we have added this certificate - if okNew { - delete(_newMap, v) - } - } - - // Add fresh certificates - for k, _ := range _newMap { - sslCertificates = append(sslCertificates, k) - } - + certs := convertStringArr(d.Get("ssl_certificates").([]interface{})) cert_ref := &compute.TargetHttpsProxiesSetSslCertificatesRequest{ - SslCertificates: sslCertificates, + SslCertificates: certs, } op, err := config.clientCompute.TargetHttpsProxies.SetSslCertificates( project, d.Id(), cert_ref).Do() @@ -208,24 +176,7 @@ func resourceComputeTargetHttpsProxyRead(d *schema.ResourceData, meta interface{ return handleNotFoundError(err, d, fmt.Sprintf("Target HTTPS proxy %q", d.Get("name").(string))) } - _certs := d.Get("ssl_certificates").([]interface{}) - current := proxy.SslCertificates - - _certMap := make(map[string]bool) - _newCerts := make([]interface{}, 0) - - for _, v := range _certs { - _certMap[v.(string)] = true - } - - // Store intersection of server certificates and user defined certificates - for _, v := range current { - if _, ok := _certMap[v]; ok { - _newCerts = append(_newCerts, v) - } - } - - d.Set("ssl_certificates", _newCerts) + d.Set("ssl_certificates", proxy.SslCertificates) d.Set("self_link", proxy.SelfLink) d.Set("id", strconv.FormatUint(proxy.Id, 10)) @@ -256,3 +207,10 @@ func resourceComputeTargetHttpsProxyDelete(d *schema.ResourceData, meta interfac d.SetId("") return nil } + +func toCanonicalSslCertificate(v interface{}) string { + value := v.(string) + m := regexp.MustCompile(sslCertificateRegex).FindStringSubmatch(value) + + return fmt.Sprintf(canonicalSslCertificateTemplate, m[1], m[2]) +} diff --git a/google/resource_compute_target_https_proxy_test.go b/google/resource_compute_target_https_proxy_test.go index f8d731f0..573fcab8 100644 --- a/google/resource_compute_target_https_proxy_test.go +++ b/google/resource_compute_target_https_proxy_test.go @@ -7,9 +7,13 @@ import ( "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + "google.golang.org/api/compute/v1" + "regexp" ) func TestAccComputeTargetHttpsProxy_basic(t *testing.T) { + var proxy compute.TargetHttpsProxy + resourceSuffix := acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -17,10 +21,12 @@ func TestAccComputeTargetHttpsProxy_basic(t *testing.T) { CheckDestroy: testAccCheckComputeTargetHttpsProxyDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccComputeTargetHttpsProxy_basic1, + Config: testAccComputeTargetHttpsProxy_basic1(resourceSuffix), Check: resource.ComposeTestCheckFunc( testAccCheckComputeTargetHttpsProxyExists( - "google_compute_target_https_proxy.foobar"), + "google_compute_target_https_proxy.foobar", &proxy), + testAccComputeTargetHttpsProxyDescription("Resource created for Terraform acceptance testing", &proxy), + testAccComputeTargetHttpsProxyHasSslCertificate("httpsproxy-test-cert1-"+resourceSuffix, &proxy), ), }, }, @@ -28,6 +34,8 @@ func TestAccComputeTargetHttpsProxy_basic(t *testing.T) { } func TestAccComputeTargetHttpsProxy_update(t *testing.T) { + var proxy compute.TargetHttpsProxy + resourceSuffix := acctest.RandString(10) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -35,24 +43,45 @@ func TestAccComputeTargetHttpsProxy_update(t *testing.T) { CheckDestroy: testAccCheckComputeTargetHttpsProxyDestroy, Steps: []resource.TestStep{ resource.TestStep{ - Config: testAccComputeTargetHttpsProxy_basic1, + Config: testAccComputeTargetHttpsProxy_basic1(resourceSuffix), Check: resource.ComposeTestCheckFunc( testAccCheckComputeTargetHttpsProxyExists( - "google_compute_target_https_proxy.foobar"), + "google_compute_target_https_proxy.foobar", &proxy), + testAccComputeTargetHttpsProxyDescription("Resource created for Terraform acceptance testing", &proxy), + testAccComputeTargetHttpsProxyHasSslCertificate("httpsproxy-test-cert1-"+resourceSuffix, &proxy), ), }, resource.TestStep{ - Config: testAccComputeTargetHttpsProxy_basic2, + Config: testAccComputeTargetHttpsProxy_basic2(resourceSuffix), Check: resource.ComposeTestCheckFunc( testAccCheckComputeTargetHttpsProxyExists( - "google_compute_target_https_proxy.foobar"), + "google_compute_target_https_proxy.foobar", &proxy), + testAccComputeTargetHttpsProxyDescription("Resource created for Terraform acceptance testing (updated)", &proxy), + testAccComputeTargetHttpsProxyHasSslCertificate("httpsproxy-test-cert1-"+resourceSuffix, &proxy), + testAccComputeTargetHttpsProxyHasSslCertificate("httpsproxy-test-cert2-"+resourceSuffix, &proxy), ), }, }, }) } +func TestAccComputeTargetHttpsProxy_invalidCertificate(t *testing.T) { + resourceSuffix := acctest.RandString(10) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckComputeTargetHttpsProxyDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccComputeTargetHttpsProxy_invalidCertificate(resourceSuffix), + ExpectError: regexp.MustCompile("ssl_certificate"), + }, + }, + }) +} + func testAccCheckComputeTargetHttpsProxyDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) @@ -71,7 +100,7 @@ func testAccCheckComputeTargetHttpsProxyDestroy(s *terraform.State) error { return nil } -func testAccCheckComputeTargetHttpsProxyExists(n string) resource.TestCheckFunc { +func testAccCheckComputeTargetHttpsProxyExists(n string, proxy *compute.TargetHttpsProxy) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -94,11 +123,38 @@ func testAccCheckComputeTargetHttpsProxyExists(n string) resource.TestCheckFunc return fmt.Errorf("TargetHttpsProxy not found") } + *proxy = *found + return nil } } -var testAccComputeTargetHttpsProxy_basic1 = fmt.Sprintf(` +func testAccComputeTargetHttpsProxyDescription(description string, proxy *compute.TargetHttpsProxy) resource.TestCheckFunc { + return func(s *terraform.State) error { + if proxy.Description != description { + return fmt.Errorf("Wrong description: expected '%s' got '%s'", description, proxy.Description) + } + return nil + } +} + +func testAccComputeTargetHttpsProxyHasSslCertificate(cert string, proxy *compute.TargetHttpsProxy) resource.TestCheckFunc { + return func(s *terraform.State) error { + config := testAccProvider.Meta().(*Config) + certUrl := fmt.Sprintf(canonicalSslCertificateTemplate, config.Project, cert) + + for _, sslCertificate := range proxy.SslCertificates { + if sslCertificate == certUrl { + return nil + } + } + + return fmt.Errorf("Ssl certificate not found: expected'%s'", certUrl) + } +} + +func testAccComputeTargetHttpsProxy_basic1(id string) string { + return fmt.Sprintf(` resource "google_compute_target_https_proxy" "foobar" { description = "Resource created for Terraform acceptance testing" name = "httpsproxy-test-%s" @@ -107,19 +163,19 @@ resource "google_compute_target_https_proxy" "foobar" { } resource "google_compute_backend_service" "foobar" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-backend-%s" health_checks = ["${google_compute_http_health_check.zero.self_link}"] } resource "google_compute_http_health_check" "zero" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-health-check-%s" request_path = "/" check_interval_sec = 1 timeout_sec = 1 } resource "google_compute_url_map" "foobar" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-url-map-%s" default_service = "${google_compute_backend_service.foobar.self_link}" host_rule { hosts = ["mysite.com", "myothersite.com"] @@ -141,43 +197,47 @@ resource "google_compute_url_map" "foobar" { } resource "google_compute_ssl_certificate" "foobar1" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-cert1-%s" description = "very descriptive" private_key = "${file("test-fixtures/ssl_cert/test.key")}" certificate = "${file("test-fixtures/ssl_cert/test.crt")}" } resource "google_compute_ssl_certificate" "foobar2" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-cert2-%s" description = "very descriptive" private_key = "${file("test-fixtures/ssl_cert/test.key")}" certificate = "${file("test-fixtures/ssl_cert/test.crt")}" } -`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10), - acctest.RandString(10), acctest.RandString(10), acctest.RandString(10)) +`, id, id, id, id, id, id) +} -var testAccComputeTargetHttpsProxy_basic2 = fmt.Sprintf(` +func testAccComputeTargetHttpsProxy_basic2(id string) string { + return fmt.Sprintf(` resource "google_compute_target_https_proxy" "foobar" { - description = "Resource created for Terraform acceptance testing" + description = "Resource created for Terraform acceptance testing (updated)" name = "httpsproxy-test-%s" url_map = "${google_compute_url_map.foobar.self_link}" - ssl_certificates = ["${google_compute_ssl_certificate.foobar1.self_link}"] + ssl_certificates = [ + "${google_compute_ssl_certificate.foobar1.self_link}", + "${google_compute_ssl_certificate.foobar2.self_link}", + ] } resource "google_compute_backend_service" "foobar" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-backend-%s" health_checks = ["${google_compute_http_health_check.zero.self_link}"] } resource "google_compute_http_health_check" "zero" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-health-check-%s" request_path = "/" check_interval_sec = 1 timeout_sec = 1 } resource "google_compute_url_map" "foobar" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-url-map-%s" default_service = "${google_compute_backend_service.foobar.self_link}" host_rule { hosts = ["mysite.com", "myothersite.com"] @@ -199,17 +259,27 @@ resource "google_compute_url_map" "foobar" { } resource "google_compute_ssl_certificate" "foobar1" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-cert1-%s" description = "very descriptive" private_key = "${file("test-fixtures/ssl_cert/test.key")}" certificate = "${file("test-fixtures/ssl_cert/test.crt")}" } resource "google_compute_ssl_certificate" "foobar2" { - name = "httpsproxy-test-%s" + name = "httpsproxy-test-cert2-%s" description = "very descriptive" private_key = "${file("test-fixtures/ssl_cert/test.key")}" certificate = "${file("test-fixtures/ssl_cert/test.crt")}" } -`, acctest.RandString(10), acctest.RandString(10), acctest.RandString(10), - acctest.RandString(10), acctest.RandString(10), acctest.RandString(10)) +`, id, id, id, id, id, id) +} + +func testAccComputeTargetHttpsProxy_invalidCertificate(id string) string { + return fmt.Sprintf(` +resource "google_compute_target_https_proxy" "foobar" { +name = "httpsproxy-test-%s" +url_map = "some-url-map" +ssl_certificates = ["invalid-certificate-reference"] +} +`, id) +} diff --git a/website/docs/r/compute_target_https_proxy.html.markdown b/website/docs/r/compute_target_https_proxy.html.markdown index c8c2e398..1a71273e 100644 --- a/website/docs/r/compute_target_https_proxy.html.markdown +++ b/website/docs/r/compute_target_https_proxy.html.markdown @@ -78,8 +78,7 @@ The following arguments are supported: this forces a new resource to be created. * `ssl_certificates` - (Required) The URLs of the SSL Certificate resources that - authenticate connections between users and load balancing. Currently exactly - one must be specified. + authenticate connections between users and load balancing. * `url_map` - (Required) The URL of a URL Map resource that defines the mapping from the URL to the BackendService.