Merge pull request #6124 from terraform-providers/f-elbv2-automatic-o…

…rder resource/aws_lb_listener(_rule): Allow order to be based on Terraform configuration ordering
hashicorp · Oct 12, 2018 · 60316b1 · 60316b1
2 parents 6caeae3 + 6438f19
commit 60316b1
Show file tree

Hide file tree

Showing 6 changed files with 364 additions and 43 deletions.
diff --git a/aws/resource_aws_lb_listener.go b/aws/resource_aws_lb_listener.go
@@ -356,15 +356,13 @@ func resourceAwsLbListenerCreate(d *schema.ResourceData, meta interface{}) error
 		defaultActionMap := defaultAction.(map[string]interface{})
 
 		action := &elbv2.Action{
-			Type: aws.String(defaultActionMap["type"].(string)),
+			Order: aws.Int64(int64(i + 1)),
+			Type:  aws.String(defaultActionMap["type"].(string)),
 		}
 
-		if order, ok := defaultActionMap["order"]; ok && order != 0 {
+		if order, ok := defaultActionMap["order"]; ok && order.(int) != 0 {
 			action.Order = aws.Int64(int64(order.(int)))
 		}
-		if len(defaultActions) != 1 && action.Order == nil {
-			return errors.New("when using more then one action, you need to specify 'order' for each action")
-		}
 
 		switch defaultActionMap["type"].(string) {
 		case "forward":
@@ -664,15 +662,13 @@ func resourceAwsLbListenerUpdate(d *schema.ResourceData, meta interface{}) error
 			defaultActionMap := defaultAction.(map[string]interface{})
 
 			action := &elbv2.Action{
-				Type: aws.String(defaultActionMap["type"].(string)),
+				Order: aws.Int64(int64(i + 1)),
+				Type:  aws.String(defaultActionMap["type"].(string)),
 			}
 
-			if order, ok := defaultActionMap["order"]; ok && order != 0 {
+			if order, ok := defaultActionMap["order"]; ok && order.(int) != 0 {
 				action.Order = aws.Int64(int64(order.(int)))
 			}
-			if len(defaultActions) != 1 && action.Order == nil {
-				return errors.New("when using more then one action, you need to specify 'order' for each action")
-			}
 
 			switch defaultActionMap["type"].(string) {
 			case "forward":

diff --git a/aws/resource_aws_lb_listener_rule.go b/aws/resource_aws_lb_listener_rule.go
@@ -332,15 +332,13 @@ func resourceAwsLbListenerRuleCreate(d *schema.ResourceData, meta interface{}) e
 		actionMap := action.(map[string]interface{})
 
 		action := &elbv2.Action{
-			Type: aws.String(actionMap["type"].(string)),
+			Order: aws.Int64(int64(i + 1)),
+			Type:  aws.String(actionMap["type"].(string)),
 		}
 
-		if order, ok := actionMap["order"]; ok && order != 0 {
+		if order, ok := actionMap["order"]; ok && order.(int) != 0 {
 			action.Order = aws.Int64(int64(order.(int)))
 		}
-		if len(actions) != 1 && action.Order == nil {
-			return errors.New("when using more then one action, you need to specify 'order' for each action")
-		}
 
 		switch actionMap["type"].(string) {
 		case "forward":
@@ -677,8 +675,12 @@ func resourceAwsLbListenerRuleUpdate(d *schema.ResourceData, meta interface{}) e
 			actionMap := action.(map[string]interface{})
 
 			action := &elbv2.Action{
+				Order: aws.Int64(int64(i + 1)),
 				Type:  aws.String(actionMap["type"].(string)),
-				Order: aws.Int64(int64(actionMap["order"].(int))), // TODO, optional
+			}
+
+			if order, ok := actionMap["order"]; ok && order.(int) != 0 {
+				action.Order = aws.Int64(int64(order.(int)))
 			}
 
 			switch actionMap["type"].(string) {

diff --git a/aws/resource_aws_lb_listener_rule_test.go b/aws/resource_aws_lb_listener_rule_test.go
@@ -73,6 +73,7 @@ func TestAccAWSLBListenerRule_basic(t *testing.T) {
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.static", "listener_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "priority", "100"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.#", "1"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.order", "1"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.type", "forward"),
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.static", "action.0.target_group_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.redirect.#", "0"),
@@ -106,6 +107,7 @@ func TestAccAWSLBListenerRuleBackwardsCompatibility(t *testing.T) {
 					resource.TestCheckResourceAttrSet("aws_alb_listener_rule.static", "listener_arn"),
 					resource.TestCheckResourceAttr("aws_alb_listener_rule.static", "priority", "100"),
 					resource.TestCheckResourceAttr("aws_alb_listener_rule.static", "action.#", "1"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.order", "1"),
 					resource.TestCheckResourceAttr("aws_alb_listener_rule.static", "action.0.type", "forward"),
 					resource.TestCheckResourceAttrSet("aws_alb_listener_rule.static", "action.0.target_group_arn"),
 					resource.TestCheckResourceAttr("aws_alb_listener_rule.static", "action.0.redirect.#", "0"),
@@ -138,6 +140,7 @@ func TestAccAWSLBListenerRule_redirect(t *testing.T) {
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.static", "listener_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "priority", "100"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.#", "1"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.order", "1"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.type", "redirect"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.target_group_arn", ""),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.redirect.#", "1"),
@@ -176,6 +179,7 @@ func TestAccAWSLBListenerRule_fixedResponse(t *testing.T) {
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.static", "listener_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "priority", "100"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.#", "1"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.order", "1"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.type", "fixed-response"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.target_group_arn", ""),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.static", "action.0.redirect.#", "0"),
@@ -362,16 +366,15 @@ func TestAccAWSLBListenerRule_cognito(t *testing.T) {
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.cognito", "listener_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "priority", "100"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.#", "2"),
-					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.0.type", "authenticate-cognito"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.0.order", "1"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.0.type", "authenticate-cognito"),
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.cognito", "action.0.authenticate_cognito.0.user_pool_arn"),
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.cognito", "action.0.authenticate_cognito.0.user_pool_client_id"),
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.cognito", "action.0.authenticate_cognito.0.user_pool_domain"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.0.authenticate_cognito.0.authentication_request_extra_params.%", "1"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.0.authenticate_cognito.0.authentication_request_extra_params.param", "test"),
-					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.0.order", "1"),
-					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.1.type", "forward"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.1.order", "2"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "action.1.type", "forward"),
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.cognito", "action.1.target_group_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "condition.#", "1"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.cognito", "condition.1366281676.field", "path-pattern"),
@@ -403,8 +406,8 @@ func TestAccAWSLBListenerRule_oidc(t *testing.T) {
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.oidc", "listener_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "priority", "100"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.#", "2"),
-					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.type", "authenticate-oidc"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.order", "1"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.type", "authenticate-oidc"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.authenticate_oidc.0.authorization_endpoint", "https://example.com/authorization_endpoint"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.authenticate_oidc.0.client_id", "s6BhdRkqt3"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.authenticate_oidc.0.client_secret", "7Fjfp0ZBr1KtDRbnfVdmIw"),
@@ -413,9 +416,8 @@ func TestAccAWSLBListenerRule_oidc(t *testing.T) {
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.authenticate_oidc.0.user_info_endpoint", "https://example.com/user_info_endpoint"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.authenticate_oidc.0.authentication_request_extra_params.%", "1"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.authenticate_oidc.0.authentication_request_extra_params.param", "test"),
-					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.0.order", "1"),
-					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.1.type", "forward"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.1.order", "2"),
+					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "action.1.type", "forward"),
 					resource.TestCheckResourceAttrSet("aws_lb_listener_rule.oidc", "action.1.target_group_arn"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "condition.#", "1"),
 					resource.TestCheckResourceAttr("aws_lb_listener_rule.oidc", "condition.1366281676.field", "path-pattern"),
@@ -427,6 +429,29 @@ func TestAccAWSLBListenerRule_oidc(t *testing.T) {
 	})
 }
 
+func TestAccAWSLBListenerRule_Action_Order(t *testing.T) {
+	var rule elbv2.Rule
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+	resourceName := "aws_lb_listener_rule.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProvidersWithTLS,
+		CheckDestroy: testAccCheckAWSLBListenerRuleDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSLBListenerRuleConfig_Action_Order(rName),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckAWSLBListenerRuleExists(resourceName, &rule),
+					resource.TestCheckResourceAttr(resourceName, "action.#", "2"),
+					resource.TestCheckResourceAttr(resourceName, "action.0.order", "1"),
+					resource.TestCheckResourceAttr(resourceName, "action.1.order", "2"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSLbListenerRuleRecreated(t *testing.T,
 	before, after *elbv2.Rule) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
@@ -1520,7 +1545,6 @@ func testAccAWSLBListenerRuleConfig_cognito(lbName string, targetGroupName strin
   priority = 100
 
   action {
-    order = 1
     type = "authenticate-cognito"
     authenticate_cognito {
       user_pool_arn = "${aws_cognito_user_pool.test.arn}"
@@ -1534,7 +1558,6 @@ func testAccAWSLBListenerRuleConfig_cognito(lbName string, targetGroupName strin
   }
 
   action {
-    order = 2
     type = "forward"
     target_group_arn = "${aws_lb_target_group.test.arn}"
   }
@@ -1697,7 +1720,6 @@ func testAccAWSLBListenerRuleConfig_oidc(lbName string, targetGroupName string,
   priority = 100
 
   action {
-    order = 1
     type = "authenticate-oidc"
     authenticate_oidc {
       authorization_endpoint =  "https://example.com/authorization_endpoint"
@@ -1714,7 +1736,6 @@ func testAccAWSLBListenerRuleConfig_oidc(lbName string, targetGroupName string,
   }
 
   action {
-    order = 2
     type = "forward"
     target_group_arn = "${aws_lb_target_group.test.arn}"
   }
@@ -1849,3 +1870,154 @@ resource "aws_security_group" "alb_test" {
   }
 }`, lbName, targetGroupName, certificateName)
 }
+
+func testAccAWSLBListenerRuleConfig_Action_Order(rName string) string {
+	return fmt.Sprintf(`
+variable "rName" {
+  default = %q
+}
+
+data "aws_availability_zones" "available" {}
+
+resource "aws_lb_listener_rule" "test" {
+  listener_arn = "${aws_lb_listener.test.arn}"
+
+  action {
+    order = 1
+    type = "authenticate-oidc"
+
+    authenticate_oidc {
+      authorization_endpoint = "https://example.com/authorization_endpoint"
+      client_id              = "s6BhdRkqt3"
+      client_secret          = "7Fjfp0ZBr1KtDRbnfVdmIw"
+      issuer                 = "https://example.com"
+      token_endpoint         = "https://example.com/token_endpoint"
+      user_info_endpoint     = "https://example.com/user_info_endpoint"
+
+      authentication_request_extra_params {
+        param  = "test"
+      }
+    }
+  }
+
+  action {
+    order            = 2
+    type             = "forward"
+    target_group_arn = "${aws_lb_target_group.test.arn}"
+  }
+
+  condition {
+    field = "path-pattern"
+    values = ["/static/*"]
+  }
+}
+
+resource "aws_iam_server_certificate" "test" {
+  certificate_body = "${tls_self_signed_cert.test.cert_pem}"
+  name             = "${var.rName}"
+  private_key      = "${tls_private_key.test.private_key_pem}"
+}
+
+resource "tls_private_key" "test" {
+  algorithm = "RSA"
+}
+
+resource "tls_self_signed_cert" "test" {
+  key_algorithm         = "RSA"
+  private_key_pem       = "${tls_private_key.test.private_key_pem}"
+  validity_period_hours = 12
+
+  subject {
+    common_name  = "example.com"
+    organization = "ACME Examples, Inc"
+  }
+
+  allowed_uses = [
+    "key_encipherment",
+    "digital_signature",
+    "server_auth",
+  ]
+}
+
+resource "aws_lb_listener" "test" {
+  load_balancer_arn = "${aws_lb.test.id}"
+  protocol          = "HTTPS"
+  port              = "443"
+  ssl_policy        = "ELBSecurityPolicy-2015-05"
+  certificate_arn   = "${aws_iam_server_certificate.test.arn}"
+
+  default_action {
+    target_group_arn = "${aws_lb_target_group.test.id}"
+    type             = "forward"
+  }
+}
+
+resource "aws_lb" "test" {
+  internal                   = true
+  name                       = "${var.rName}"
+  security_groups            = ["${aws_security_group.test.id}"]
+  subnets                    = ["${aws_subnet.test.*.id}"]
+}
+
+resource "aws_lb_target_group" "test" {
+  name     = "${var.rName}"
+  port     = 8080
+  protocol = "HTTP"
+  vpc_id   = "${aws_vpc.test.id}"
+
+  health_check {
+    path = "/health"
+    interval = 60
+    port = 8081
+    protocol = "HTTP"
+    timeout = 3
+    healthy_threshold = 3
+    unhealthy_threshold = 3
+    matcher = "200-299"
+  }
+}
+
+resource "aws_vpc" "test" {
+  cidr_block = "10.0.0.0/16"
+
+  tags {
+    Name = "${var.rName}"
+  }
+}
+
+resource "aws_subnet" "test" {
+  count = 2
+
+  availability_zone       = "${data.aws_availability_zones.available.names[count.index]}"
+  cidr_block              = "10.0.${count.index}.0/24"
+  map_public_ip_on_launch = true
+  vpc_id                  = "${aws_vpc.test.id}"
+
+  tags {
+    Name = "${var.rName}"
+  }
+}
+
+resource "aws_security_group" "test" {
+  name        = "${var.rName}"
+  vpc_id      = "${aws_vpc.test.id}"
+
+  ingress {
+    from_port   = 0
+    to_port     = 0
+    protocol    = "-1"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+
+  egress {
+    from_port   = 0
+    to_port     = 0
+    protocol    = "-1"
+    cidr_blocks = ["0.0.0.0/0"]
+  }
+
+  tags {
+    Name = "${var.rName}"
+  }
+}`, rName)
+}