From fddafd2b96540bf97ef7b0751d920c1ae9ae57f5 Mon Sep 17 00:00:00 2001 From: clint shryock Date: Thu, 12 Nov 2015 11:10:52 -0600 Subject: [PATCH 1/3] providers/aws: Document and validate ELB ssl_cert and protocol requirements --- builtin/providers/aws/structure.go | 18 ++++++++++++- builtin/providers/aws/structure_test.go | 25 +++++++++++++++++++ .../docs/providers/aws/r/elb.html.markdown | 12 ++++++--- 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/builtin/providers/aws/structure.go b/builtin/providers/aws/structure.go index 3afebbad7..86ccdb483 100644 --- a/builtin/providers/aws/structure.go +++ b/builtin/providers/aws/structure.go @@ -44,7 +44,23 @@ func expandListeners(configured []interface{}) ([]*elb.Listener, error) { l.SSLCertificateId = aws.String(v.(string)) } - listeners = append(listeners, l) + var valid bool + if l.SSLCertificateId != nil && *l.SSLCertificateId != "" { + // validate the protocol is correct + for _, p := range []string{"https", "ssl"} { + if (*l.InstanceProtocol == p) || (*l.Protocol == p) { + valid = true + } + } + } else { + valid = true + } + + if valid { + listeners = append(listeners, l) + } else { + return nil, fmt.Errorf("[ERR] Invalid ssl_certificate_id / Protocol combination. Must be either HTTPS or SSL") + } } return listeners, nil diff --git a/builtin/providers/aws/structure_test.go b/builtin/providers/aws/structure_test.go index 7447801ee..38abfbfbb 100644 --- a/builtin/providers/aws/structure_test.go +++ b/builtin/providers/aws/structure_test.go @@ -2,6 +2,7 @@ package aws import ( "reflect" + "strings" "testing" "github.com/aws/aws-sdk-go/aws" @@ -314,7 +315,31 @@ func TestExpandListeners(t *testing.T) { listeners[0], expected) } +} +// this test should produce an error from expandlisteners on an invalid +// combination +func TestExpandListeners_invalid(t *testing.T) { + expanded := []interface{}{ + map[string]interface{}{ + "instance_port": 8000, + "lb_port": 80, + "instance_protocol": "http", + "lb_protocol": "http", + "ssl_certificate_id": "something", + }, + } + _, err := expandListeners(expanded) + if err != nil { + // Check the error we got + if !strings.Contains(err.Error(), "Protocol combination") { + t.Fatalf("Got error in TestExpandListeners_invalid, but not what we expected: %s", err) + } + } + + if err == nil { + t.Fatalf("Expected TestExpandListeners_invalid to fail, but passed") + } } func TestFlattenHealthCheck(t *testing.T) { diff --git a/website/source/docs/providers/aws/r/elb.html.markdown b/website/source/docs/providers/aws/r/elb.html.markdown index 401f4cabe..dde90e54d 100644 --- a/website/source/docs/providers/aws/r/elb.html.markdown +++ b/website/source/docs/providers/aws/r/elb.html.markdown @@ -33,7 +33,7 @@ resource "aws_elb" "bar" { listener { instance_port = 8000 - instance_protocol = "http" + instance_protocol = "https" lb_port = 443 lb_protocol = "https" ssl_certificate_id = "arn:aws:iam::123456789012:server-certificate/certName" @@ -90,10 +90,14 @@ Access Logs support the following: Listeners support the following: * `instance_port` - (Required) The port on the instance to route to -* `instance_protocol` - (Required) The protocol to use to the instance. +* `instance_protocol` - (Required) The protocol to use to the instance. Valid + values are `HTTP`, `HTTPS`, `TCP`, or `SSL` * `lb_port` - (Required) The port to listen on for the load balancer -* `lb_protocol` - (Required) The protocol to listen on. -* `ssl_certificate_id` - (Optional) The id of an SSL certificate you have uploaded to AWS IAM. +* `lb_protocol` - (Required) The protocol to listen on. Valid values are `HTTP`, + `HTTPS`, `TCP`, or `SSL` +* `ssl_certificate_id` - (Optional) The id of an SSL certificate you have +uploaded to AWS IAM. **Only valid when `instance_protocol` and + `lb_protocol` are either HTTPS or SSL** Health Check supports the following: From 1b2e068b19d57e85e0733d52fd9a4cada458acc0 Mon Sep 17 00:00:00 2001 From: clint shryock Date: Thu, 12 Nov 2015 11:14:33 -0600 Subject: [PATCH 2/3] add extra test block --- builtin/providers/aws/structure_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/builtin/providers/aws/structure_test.go b/builtin/providers/aws/structure_test.go index 38abfbfbb..9f6ce9ff1 100644 --- a/builtin/providers/aws/structure_test.go +++ b/builtin/providers/aws/structure_test.go @@ -296,6 +296,13 @@ func TestExpandListeners(t *testing.T) { "instance_protocol": "http", "lb_protocol": "http", }, + map[string]interface{}{ + "instance_port": 8000, + "lb_port": 80, + "instance_protocol": "https", + "lb_protocol": "https", + "ssl_certificate_id": "something", + }, } listeners, err := expandListeners(expanded) if err != nil { From 5cafe740ff6ef308de9d19f96a3adbedd3628b25 Mon Sep 17 00:00:00 2001 From: clint shryock Date: Thu, 12 Nov 2015 14:25:41 -0600 Subject: [PATCH 3/3] update wording on ssl cert error --- builtin/providers/aws/structure.go | 4 ++-- builtin/providers/aws/structure_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/providers/aws/structure.go b/builtin/providers/aws/structure.go index 86ccdb483..d74064db9 100644 --- a/builtin/providers/aws/structure.go +++ b/builtin/providers/aws/structure.go @@ -59,9 +59,9 @@ func expandListeners(configured []interface{}) ([]*elb.Listener, error) { if valid { listeners = append(listeners, l) } else { - return nil, fmt.Errorf("[ERR] Invalid ssl_certificate_id / Protocol combination. Must be either HTTPS or SSL") + return nil, fmt.Errorf("[ERR] ELB Listener: ssl_certificate_id may be set only when protocol is 'https' or 'ssl'") } - } + } return listeners, nil } diff --git a/builtin/providers/aws/structure_test.go b/builtin/providers/aws/structure_test.go index 9f6ce9ff1..65d56bb93 100644 --- a/builtin/providers/aws/structure_test.go +++ b/builtin/providers/aws/structure_test.go @@ -339,7 +339,7 @@ func TestExpandListeners_invalid(t *testing.T) { _, err := expandListeners(expanded) if err != nil { // Check the error we got - if !strings.Contains(err.Error(), "Protocol combination") { + if !strings.Contains(err.Error(), "ssl_certificate_id may be set only when protocol") { t.Fatalf("Got error in TestExpandListeners_invalid, but not what we expected: %s", err) } }