Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add override_artifact_name flag to aws_codebuild_project #7824

Merged
merged 1 commit into from
Jul 31, 2019
Merged

Add override_artifact_name flag to aws_codebuild_project #7824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions aws/resource_aws_codebuild_project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ func resourceAwsCodeBuildProject() *schema.Resource {
codebuild.ArtifactsTypeNoArtifacts,
}, false),
},
"override_artifact_name": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},
},
},
Set: resourceAwsCodeBuildProjectArtifactsHash,
Expand Down Expand Up @@ -565,6 +570,8 @@ func expandProjectArtifactData(data map[string]interface{}) codebuild.ProjectArt
projectArtifacts.EncryptionDisabled = aws.Bool(data["encryption_disabled"].(bool))
}

projectArtifacts.OverrideArtifactName = aws.Bool(data["override_artifact_name"].(bool))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent panics like the below, found with other acceptance tests:

panic: interface conversion: interface {} is nil, not bool

goroutine 8222 [running]:
github.com/terraform-providers/terraform-provider-aws/aws.expandProjectArtifactData(0xc00ec050b0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws-deux/aws/resource_aws_codebuild_project.go:676 +0xa0e
github.com/terraform-providers/terraform-provider-aws/aws.expandProjectSecondaryArtifacts(0xc00045b180, 0x0, 0xc000e26a99, 0xc00beabd20)
	/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws-deux/aws/resource_aws_codebuild_project.go:649 +0x14e
github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsCodeBuildProjectCreate(0xc00045b180, 0x53bf4a0, 0xc00bbf8480, 0x2, 0xada4560)
	/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws-deux/aws/resource_aws_codebuild_project.go:554 +0x16e

This needed a nil check:

	if v, ok := data["override_artifact_name"]; ok {
		projectArtifacts.OverrideArtifactName = aws.Bool(v.(bool))
	}

Copy link
Contributor Author

@jckuester jckuester Jul 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing @bflad. Just wondering for the future: my assumption was that override_artifact_name has a default value and therefore cannot be nil?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there were two separate configuration block schemas (artifacts which had override_artifact_name declared and secondary_artifacts which did not have override_artifact_name declared), you're correct that the Default meant that the map key was always present when working with artifacts, but it was the secondary_artifacts map that did not have the override_artifact_name key since it was not declared in the schema. 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always recommend running the acceptance testing for the whole resource to tease out issues like these 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining. I just realized that updates for secondary_artifacts are not working correctly yet, so I added some more tests to cover this and tried to provide a fix: #9652


if data["artifact_identifier"] != nil && data["artifact_identifier"].(string) != "" {
projectArtifacts.ArtifactIdentifier = aws.String(data["artifact_identifier"].(string))
}
Expand Down Expand Up @@ -955,6 +962,11 @@ func flattenAwsCodeBuildProjectArtifactsData(artifacts codebuild.ProjectArtifact
if artifacts.EncryptionDisabled != nil {
values["encryption_disabled"] = *artifacts.EncryptionDisabled
}

if artifacts.OverrideArtifactName != nil {
values["override_artifact_name"] = *artifacts.OverrideArtifactName
}

if artifacts.Location != nil {
values["location"] = *artifacts.Location
}
Expand Down Expand Up @@ -1064,6 +1076,7 @@ func resourceAwsCodeBuildProjectArtifactsHash(v interface{}) int {
m := v.(map[string]interface{})

buf.WriteString(fmt.Sprintf("%s-", m["type"].(string)))
buf.WriteString(fmt.Sprintf("%t-", m["override_artifact_name"].(bool)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent panics like the below, found with other acceptance tests:

panic: interface conversion: interface {} is nil, not bool

goroutine 3492 [running]:
github.com/terraform-providers/terraform-provider-aws/aws.resourceAwsCodeBuildProjectArtifactsHash(0x58d8340, 0xc0044ac210, 0xc011f4dbd8)
	/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws-deux/aws/resource_aws_codebuild_project.go:1366 +0x39a

This needed a nil check:

	if v, ok := m["override_artifact_name"]; ok {
		buf.WriteString(fmt.Sprintf("%t-", v.(bool)))
	}


if v, ok := m["artifact_identifier"]; ok {
buf.WriteString(fmt.Sprintf("%s:", v.(string)))
Expand Down
58 changes: 58 additions & 0 deletions aws/resource_aws_codebuild_project_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func TestAccAWSCodeBuildProject_basic(t *testing.T) {
testAccCheckResourceAttrRegionalARN(resourceName, "arn", "codebuild", fmt.Sprintf("project/%s", rName)),
resource.TestCheckResourceAttr(resourceName, "artifacts.#", "1"),
resource.TestCheckResourceAttr(resourceName, "artifacts.1178773975.encryption_disabled", "false"),
resource.TestCheckResourceAttr(resourceName, "artifacts.1178773975.override_artifact_name", "false"),
resource.TestCheckResourceAttr(resourceName, "artifacts.1178773975.location", ""),
resource.TestCheckResourceAttr(resourceName, "artifacts.1178773975.name", ""),
resource.TestCheckResourceAttr(resourceName, "artifacts.1178773975.namespace_type", ""),
Expand Down Expand Up @@ -768,6 +769,37 @@ func TestAccAWSCodeBuildProject_Artifacts_EncryptionDisabled(t *testing.T) {
})
}

func TestAccAWSCodeBuildProject_Artifacts_OverrideArtifactName(t *testing.T) {
var project codebuild.Project
rName := acctest.RandomWithPrefix("tf-acc-test")
bName := acctest.RandomWithPrefix("tf-acc-test-bucket")
resourceName := "aws_codebuild_project.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSCodeBuild(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSCodeBuildProjectDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSCodebuildProjectConfig_Artifacts_OverrideArtifactName(rName, bName, true),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSCodeBuildProjectExists(resourceName, &project),
resource.TestCheckResourceAttr(resourceName, "artifacts.#", "1"),
resource.TestCheckResourceAttr(resourceName, "artifacts.1580844383.override_artifact_name", "true"),
),
},
{
Config: testAccAWSCodebuildProjectConfig_Artifacts_OverrideArtifactName(rName, bName, false),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSCodeBuildProjectExists(resourceName, &project),
resource.TestCheckResourceAttr(resourceName, "artifacts.#", "1"),
resource.TestCheckResourceAttr(resourceName, "artifacts.135498304.override_artifact_name", "false"),
),
},
},
})
}

func TestAccAWSCodeBuildProject_SecondaryArtifacts(t *testing.T) {
var project codebuild.Project
rName := acctest.RandomWithPrefix("tf-acc-test")
Expand Down Expand Up @@ -1692,6 +1724,32 @@ resource "aws_codebuild_project" "test" {
`, rName, encryptionDisabled)
}

func testAccAWSCodebuildProjectConfig_Artifacts_OverrideArtifactName(rName string, bName string, overrideArtifactName bool) string {
return testAccAWSCodeBuildProjectConfig_Base_ServiceRole(rName) + testAccAWSCodeBuildProjectConfig_Base_Bucket(bName) + fmt.Sprintf(`
resource "aws_codebuild_project" "test" {
name = "%s"
service_role = "${aws_iam_role.test.arn}"

artifacts {
override_artifact_name = %t
location = "${aws_s3_bucket.test.bucket}"
type = "S3"
}

environment {
compute_type = "BUILD_GENERAL1_SMALL"
image = "2"
type = "LINUX_CONTAINER"
}

source {
type = "GITHUB"
location = "https://github.com/hashicorp/packer.git"
}
}
`, rName, overrideArtifactName)
}

func testAccAWSCodebuildProjectConfig_SecondaryArtifacts(rName string, bName string) string {
return testAccAWSCodeBuildProjectConfig_Base_ServiceRole(rName) + testAccAWSCodeBuildProjectConfig_Base_Bucket(bName) + fmt.Sprintf(`
resource "aws_codebuild_project" "test" {
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/codebuild_project.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ The following arguments are supported:

* `type` - (Required) The build output artifact's type. Valid values for this parameter are: `CODEPIPELINE`, `NO_ARTIFACTS` or `S3`.
* `encryption_disabled` - (Optional) If set to true, output artifacts will not be encrypted. If `type` is set to `NO_ARTIFACTS` then this value will be ignored. Defaults to `false`.
* `override_artifact_name` (Optional) If set to true, a name specified in the build spec file overrides the artifact name.
* `location` - (Optional) Information about the build output artifact location. If `type` is set to `CODEPIPELINE` or `NO_ARTIFACTS` then this value will be ignored. If `type` is set to `S3`, this is the name of the output bucket. If `path` is not also specified, then `location` can also specify the path of the output artifact in the output bucket.
* `name` - (Optional) The name of the project. If `type` is set to `S3`, this is the name of the output artifact object
* `namespace_type` - (Optional) The namespace to use in storing build artifacts. If `type` is set to `S3`, then valid values for this parameter are: `BUILD_ID` or `NONE`.
Expand Down Expand Up @@ -217,6 +218,7 @@ The following arguments are supported:
* `type` - (Required) The build output artifact's type. Valid values for this parameter are: `CODEPIPELINE`, `NO_ARTIFACTS` or `S3`.
* `artifact_identifier` - (Required) The artifact identifier. Must be the same specified inside AWS CodeBuild buildspec.
* `encryption_disabled` - (Optional) If set to true, output artifacts will not be encrypted. If `type` is set to `NO_ARTIFACTS` then this value will be ignored. Defaults to `false`.
* `override_artifact_name` (Optional) If set to true, a name specified in the build spec file overrides the artifact name.
* `location` - (Optional) Information about the build output artifact location. If `type` is set to `CODEPIPELINE` or `NO_ARTIFACTS` then this value will be ignored. If `type` is set to `S3`, this is the name of the output bucket. If `path` is not also specified, then `location` can also specify the path of the output artifact in the output bucket.
* `name` - (Optional) The name of the project. If `type` is set to `S3`, this is the name of the output artifact object
* `namespace_type` - (Optional) The namespace to use in storing build artifacts. If `type` is set to `S3`, then valid values for this parameter are: `BUILD_ID` or `NONE`.
Expand Down