Merge pull request #5603 from TimeIncOSS/fix/4913

aws: Fix CloudFormation update for parameters & capabilities if not modified

builtin/providers/aws/resource_aws_cloudformation_stack.go

@@ -268,7 +268,7 @@ func resourceAwsCloudFormationStackUpdate(d *schema.ResourceData, meta interface
 		StackName: aws.String(d.Get("name").(string)),
 	}
 
-	// Either TemplateBody or TemplateURL are required for each change
+	// Either TemplateBody, TemplateURL or UsePreviousTemplate are required
 	if v, ok := d.GetOk("template_url"); ok {
 		input.TemplateURL = aws.String(v.(string))
 	}
@@ -276,15 +276,20 @@ func resourceAwsCloudFormationStackUpdate(d *schema.ResourceData, meta interface
 		input.TemplateBody = aws.String(normalizeJson(v.(string)))
 	}
 
-	if d.HasChange("capabilities") {
+	// Capabilities must be present whether they are changed or not
-		input.Capabilities = expandStringList(d.Get("capabilities").(*schema.Set).List())
+	if v, ok := d.GetOk("capabilities"); ok {
+		input.Capabilities = expandStringList(v.(*schema.Set).List())
 	}
+
 	if d.HasChange("notification_arns") {
 		input.NotificationARNs = expandStringList(d.Get("notification_arns").(*schema.Set).List())
 	}
-	if d.HasChange("parameters") {
+
-		input.Parameters = expandCloudFormationParameters(d.Get("parameters").(map[string]interface{}))
+	// Parameters must be present whether they are changed or not
+	if v, ok := d.GetOk("parameters"); ok {
+		input.Parameters = expandCloudFormationParameters(v.(map[string]interface{}))
 	}
+
 	if d.HasChange("policy_body") {
 		input.StackPolicyBody = aws.String(normalizeJson(d.Get("policy_body").(string)))
 	}

builtin/providers/aws/resource_aws_cloudformation_stack_test.go

@@ -51,15 +51,46 @@ func TestAccAWSCloudFormation_defaultParams(t *testing.T) {
 func TestAccAWSCloudFormation_allAttributes(t *testing.T) {
 	var stack cloudformation.Stack
 
+	expectedPolicyBody := "{\"Statement\":[{\"Action\":\"Update:*\",\"Effect\":\"Deny\",\"Principal\":\"*\",\"Resource\":\"LogicalResourceId/StaticVPC\"},{\"Action\":\"Update:*\",\"Effect\":\"Allow\",\"Principal\":\"*\",\"Resource\":\"*\"}]}"
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSCloudFormationDestroy,
 		Steps: []resource.TestStep{
 			resource.TestStep{
-				Config: testAccAWSCloudFormationConfig_allAttributes,
+				Config: testAccAWSCloudFormationConfig_allAttributesWithBodies,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckCloudFormationStackExists("aws_cloudformation_stack.full", &stack),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "name", "tf-full-stack"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "capabilities.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "capabilities.1328347040", "CAPABILITY_IAM"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "disable_rollback", "false"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "notification_arns.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "parameters.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "parameters.VpcCIDR", "10.0.0.0/16"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "policy_body", expectedPolicyBody),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "tags.#", "2"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "tags.First", "Mickey"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "tags.Second", "Mouse"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "timeout_in_minutes", "10"),
+				),
+			},
+			resource.TestStep{
+				Config: testAccAWSCloudFormationConfig_allAttributesWithBodies_modified,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckCloudFormationStackExists("aws_cloudformation_stack.full", &stack),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "name", "tf-full-stack"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "capabilities.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "capabilities.1328347040", "CAPABILITY_IAM"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "disable_rollback", "false"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "notification_arns.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "parameters.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "parameters.VpcCIDR", "10.0.0.0/16"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "policy_body", expectedPolicyBody),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "tags.#", "2"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "tags.First", "Mickey"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "tags.Second", "Mouse"),
+					resource.TestCheckResourceAttr("aws_cloudformation_stack.full", "timeout_in_minutes", "10"),
 				),
 			},
 		},
@@ -251,29 +282,79 @@ BODY
 }
 `
 
-var testAccAWSCloudFormationConfig_allAttributes = `
+var testAccAWSCloudFormationConfig_allAttributesWithBodies_tpl = `
 resource "aws_cloudformation_stack" "full" {
   name = "tf-full-stack"
   template_body = <<STACK
 {
+  "Parameters" : {
+    "VpcCIDR" : {
+      "Description" : "CIDR to be used for the VPC",
+      "Type" : "String"
+    }
+  },
   "Resources" : {
     "MyVPC": {
       "Type" : "AWS::EC2::VPC",
       "Properties" : {
-        "CidrBlock" : "10.0.0.0/16",
+        "CidrBlock" : {"Ref": "VpcCIDR"},
         "Tags" : [
-          {"Key": "Name", "Value": "Primary_CF_VPC"}
+          {"Key": "Name", "Value": "%s"}
         ]
       }
+    },
+    "StaticVPC": {
+      "Type" : "AWS::EC2::VPC",
+      "Properties" : {
+        "CidrBlock" : {"Ref": "VpcCIDR"},
+        "Tags" : [
+          {"Key": "Name", "Value": "Static_CF_VPC"}
+        ]
+      }
+    },
+    "InstanceRole" : {
+      "Type" : "AWS::IAM::Role",
+      "Properties" : {
+        "AssumeRolePolicyDocument": {
+          "Version": "2012-10-17",
+          "Statement": [ {
+            "Effect": "Allow",
+            "Principal": { "Service": "ec2.amazonaws.com" },
+            "Action": "sts:AssumeRole"
+          } ]
+        },
+        "Path" : "/",
+        "Policies" : [ {
+          "PolicyName": "terraformtest",
+          "PolicyDocument": {
+            "Version": "2012-10-17",
+            "Statement": [ {
+              "Effect": "Allow",
+              "Action": [ "ec2:DescribeSnapshots" ],
+              "Resource": [ "*" ]
+            } ]
+          }
+        } ]
+      }
     }
   }
 }
 STACK
+  parameters {
+    VpcCIDR = "10.0.0.0/16"
+  }
 
+  policy_body = <<POLICY
+%s
+POLICY
   capabilities = ["CAPABILITY_IAM"]
   notification_arns = ["${aws_sns_topic.cf-updates.arn}"]
   on_failure = "DELETE"
-  timeout_in_minutes = 1
+  timeout_in_minutes = 10
+  tags {
+    First = "Mickey"
+    Second = "Mouse"
+  }
 }
 
 resource "aws_sns_topic" "cf-updates" {
@@ -281,6 +362,34 @@ resource "aws_sns_topic" "cf-updates" {
 }
 `
 
+var policyBody = `
+{
+  "Statement" : [
+    {
+      "Effect" : "Deny",
+      "Action" : "Update:*",
+      "Principal": "*",
+      "Resource" : "LogicalResourceId/StaticVPC"
+    },
+    {
+      "Effect" : "Allow",
+      "Action" : "Update:*",
+      "Principal": "*",
+      "Resource" : "*"
+    }
+  ]
+}
+`
+
+var testAccAWSCloudFormationConfig_allAttributesWithBodies = fmt.Sprintf(
+	testAccAWSCloudFormationConfig_allAttributesWithBodies_tpl,
+	"Primary_CF_VPC",
+	policyBody)
+var testAccAWSCloudFormationConfig_allAttributesWithBodies_modified = fmt.Sprintf(
+	testAccAWSCloudFormationConfig_allAttributesWithBodies_tpl,
+	"Primary_CloudFormation_VPC",
+	policyBody)
+
 var tpl_testAccAWSCloudFormationConfig_withParams = `
 resource "aws_cloudformation_stack" "with_params" {
   name = "tf-stack-with-params"