provider/aws: Fix update of environment_variable in codebuild_project (#12169)

* Fix for environment_variable not updating

* Add update test

* Fix for tags not passing update test

* Fix for nil in statefile

builtin/providers/aws/resource_aws_codebuild_project.go

@@ -200,6 +200,10 @@ func resourceAwsCodeBuildProjectCreate(d *schema.ResourceData, meta interface{})
 		params.TimeoutInMinutes = aws.Int64(int64(v.(int)))
 	}
 
+	if v, ok := d.GetOk("tags"); ok {
+		params.Tags = tagsFromMapCodeBuild(v.(map[string]interface{}))
+	}
+
 	var resp *codebuild.CreateProjectOutput
 	err := resource.Retry(2*time.Minute, func() *resource.RetryError {
 		var err error
@@ -416,9 +420,9 @@ func resourceAwsCodeBuildProjectUpdate(d *schema.ResourceData, meta interface{})
 		params.TimeoutInMinutes = aws.Int64(int64(d.Get("timeout").(int)))
 	}
 
-	if d.HasChange("tags") {
-		params.Tags = tagsFromMapCodeBuild(d.Get("tags").(map[string]interface{}))
-	}
+	// The documentation clearly says "The replacement set of tags for this build project."
+	// But its a slice of pointers so if not set for every update, they get removed.
+	params.Tags = tagsFromMapCodeBuild(d.Get("tags").(map[string]interface{}))
 
 	_, err := conn.UpdateProject(params)
 
@@ -548,10 +552,16 @@ func resourceAwsCodeBuildProjectEnvironmentHash(v interface{}) int {
 	environmentType := m["type"].(string)
 	computeType := m["compute_type"].(string)
 	image := m["image"].(string)
-
+	environmentVariables := m["environment_variable"].([]interface{})
 	buf.WriteString(fmt.Sprintf("%s-", environmentType))
 	buf.WriteString(fmt.Sprintf("%s-", computeType))
 	buf.WriteString(fmt.Sprintf("%s-", image))
+	for _, e := range environmentVariables {
+		if e != nil { // Old statefiles might have nil values in them
+			ev := e.(map[string]interface{})
+			buf.WriteString(fmt.Sprintf("%s:%s-", ev["name"].(string), ev["value"].(string)))
+		}
+	}
 
 	return hashcode.String(buf.String())
 }
@@ -584,14 +594,12 @@ func resourceAwsCodeBuildProjectSourceAuthHash(v interface{}) int {
 	return hashcode.String(buf.String())
 }
 
-func environmentVariablesToMap(environmentVariables []*codebuild.EnvironmentVariable) []map[string]interface{} {
-
-	envVariables := make([]map[string]interface{}, len(environmentVariables))
+func environmentVariablesToMap(environmentVariables []*codebuild.EnvironmentVariable) []interface{} {
 
+	envVariables := []interface{}{}
 	if len(environmentVariables) > 0 {
-		for i := 0; i < len(environmentVariables); i++ {
-			env := environmentVariables[i]
-			item := make(map[string]interface{})
+		for _, env := range environmentVariables {
+			item := map[string]interface{}{}
 			item["name"] = *env.Name
 			item["value"] = *env.Value
 			envVariables = append(envVariables, item)

builtin/providers/aws/resource_aws_codebuild_project_test.go

@@ -21,12 +21,18 @@ func TestAccAWSCodeBuildProject_basic(t *testing.T) {
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSCodeBuildProjectDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccAWSCodeBuildProjectConfig_basic(name),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSCodeBuildProjectExists("aws_codebuild_project.foo"),
 				),
 			},
+			{
+				Config: testAccAWSCodeBuildProjectConfig_basicUpdated(name),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSCodeBuildProjectExists("aws_codebuild_project.foo"),
+				),
+			},
 		},
 	})
 }
@@ -369,3 +375,90 @@ resource "aws_codebuild_project" "foo" {
 }
 `, rName, rName, rName, rName)
 }
+
+func testAccAWSCodeBuildProjectConfig_basicUpdated(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_iam_role" "codebuild_role" {
+  name = "codebuild-role-%s"
+  assume_role_policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Principal": {
+        "Service": "codebuild.amazonaws.com"
+      },
+      "Action": "sts:AssumeRole"
+    }
+  ]
+}
+EOF
+}
+
+resource "aws_iam_policy" "codebuild_policy" {
+    name        = "codebuild-policy-%s"
+    path        = "/service-role/"
+    description = "Policy used in trust relationship with CodeBuild"
+    policy      = <<POLICY
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Resource": [
+        "*"
+      ],
+      "Action": [
+        "logs:CreateLogGroup",
+        "logs:CreateLogStream",
+        "logs:PutLogEvents"
+      ]
+    }
+  ]
+}
+POLICY
+}
+
+resource "aws_iam_policy_attachment" "codebuild_policy_attachment" {
+  name       = "codebuild-policy-attachment-%s"
+  policy_arn = "${aws_iam_policy.codebuild_policy.arn}"
+  roles      = ["${aws_iam_role.codebuild_role.id}"]
+}
+
+resource "aws_codebuild_project" "foo" {
+  name         = "test-project-%s"
+  description  = "test_codebuild_project"
+  timeout      = "5"
+	service_role = "${aws_iam_role.codebuild_role.arn}"
+
+	artifacts {
+		type = "NO_ARTIFACTS"
+	}
+
+  environment {
+    compute_type = "BUILD_GENERAL1_SMALL"
+    image        = "2"
+    type         = "LINUX_CONTAINER"
+
+		environment_variable = {
+			"name"  = "SOME_OTHERKEY"
+			"value" = "SOME_OTHERVALUE"
+		}
+  }
+
+  source {
+    auth {
+      type = "OAUTH"
+    }
+
+    type     = "GITHUB"
+    location = "https://github.com/mitchellh/packer.git"
+  }
+
+  tags {
+    "Environment" = "Test"
+  }
+}
+`, rName, rName, rName, rName)
+}

builtin/providers/aws/tagsCodeBuild.go

@@ -32,7 +32,7 @@ func diffTagsCodeBuild(oldTags, newTags []*codebuild.Tag) ([]*codebuild.Tag, []*
 }
 
 func tagsFromMapCodeBuild(m map[string]interface{}) []*codebuild.Tag {
-	result := make([]*codebuild.Tag, 0, len(m))
+	result := []*codebuild.Tag{}
 	for k, v := range m {
 		result = append(result, &codebuild.Tag{
 			Key:   aws.String(k),
@@ -44,7 +44,7 @@ func tagsFromMapCodeBuild(m map[string]interface{}) []*codebuild.Tag {
 }
 
 func tagsToMapCodeBuild(ts []*codebuild.Tag) map[string]string {
-	result := make(map[string]string)
+	result := map[string]string{}
 	for _, t := range ts {
 		result[*t.Key] = *t.Value
 	}