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.

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

resource/aws_codebuild_project: Add registry_credential argument for environment #9168

Merged
merged 4 commits into from
Jul 8, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
50 changes: 50 additions & 0 deletions aws/resource_aws_codebuild_project.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,26 @@ func resourceAwsCodeBuildProject() *schema.Resource {
Optional: true,
ValidateFunc: validation.StringMatch(regexp.MustCompile(`\.(pem|zip)$`), "must end in .pem or .zip"),
},
"registry_credential": {
Type: schema.TypeSet,
Copy link
Contributor

Choose a reason for hiding this comment

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

When using MaxItems: 1 for configuration block attributes, we prefer to simply the attribute type to TypeList instead. 👍

Suggested change
Type: schema.TypeSet,
Type: schema.TypeList,

Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"credential": {
Type: schema.TypeString,
Required: true,
},
"credential_provider": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
codebuild.CredentialProviderTypeSecretsManager,
}, false),
},
},
},
},
},
},
Set: resourceAwsCodeBuildProjectEnvironmentHash,
Expand Down Expand Up @@ -659,6 +679,22 @@ func expandProjectEnvironment(d *schema.ResourceData) *codebuild.ProjectEnvironm
projectEnv.ImagePullCredentialsType = aws.String(v.(string))
}

if v := envConfig["registry_credential"]; v != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the code around this might not be performing the safer check, but could you please update this to something like the following to perform the existence checking? Thanks!

Suggested change
if v := envConfig["registry_credential"]; v != nil {
if v, ok := envConfig["registry_credential"]; ok && len(v.([]interface{})) > 0 {

config := v.(*schema.Set).List()[0].(map[string]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

With the switch to TypeList and the length check added above for safety, this simplifies to:

Suggested change
config := v.(*schema.Set).List()[0].(map[string]interface{})
config := v.([]interface{}).[0].(map[string]interface{})


projectRegistryCredential := &codebuild.RegistryCredential{}

if v := config["credential"].(string); v != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

For additional safety we should use existence checking here as well. 👍

Suggested change
if v := config["credential"].(string); v != "" {
if v, ok := config["credential"]; ok && v.(string) != "" {

projectRegistryCredential.Credential = &v
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally when setting AWS Go SDK parameters, we prefer (by convention) to use the provided conversion functions (e.g. aws.String()) instead of raw Go pointer referencing

Suggested change
projectRegistryCredential.Credential = &v
projectRegistryCredential.Credential = aws.String(v.(string))

}

if v := config["credential_provider"].(string); v != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar change here:

Suggested change
if v := config["credential_provider"].(string); v != "" {
if v, ok := config["credential_provider"]; ok && v.(string) != "" {

projectRegistryCredential.CredentialProvider = &v
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar change here:

Suggested change
projectRegistryCredential.CredentialProvider = &v
projectRegistryCredential.CredentialProvider = aws.String(v.(string))

}

projectEnv.RegistryCredential = projectRegistryCredential
}

if v := envConfig["environment_variable"]; v != nil {
envVariables := v.([]interface{})
if len(envVariables) > 0 {
Expand Down Expand Up @@ -1027,12 +1063,26 @@ func flattenAwsCodeBuildProjectEnvironment(environment *codebuild.ProjectEnviron
envConfig["privileged_mode"] = *environment.PrivilegedMode
envConfig["image_pull_credentials_type"] = *environment.ImagePullCredentialsType

envConfig["registry_credential"] = flattenAwsCodebuildRegistryCredential(environment.RegistryCredential)

if environment.EnvironmentVariables != nil {
envConfig["environment_variable"] = environmentVariablesToMap(environment.EnvironmentVariables)
}

return []interface{}{envConfig}
}

func flattenAwsCodebuildRegistryCredential(registryCredential *codebuild.RegistryCredential) []interface{} {
if registryCredential == nil {
return []interface{}{}
}

values := map[string]interface{}{
"credential": aws.StringValue(registryCredential.Credential),
"credential_provider": aws.StringValue(registryCredential.CredentialProvider),
}

return []interface{}{values}
}

func flattenAwsCodeBuildProjectSecondarySources(sourceList []*codebuild.ProjectSource) []interface{} {
Expand Down
60 changes: 60 additions & 0 deletions aws/resource_aws_codebuild_project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,26 @@ func TestAWSCodeBuildProject_nameValidation(t *testing.T) {
}
}

func TestAccAWSCodeBuildProject_Environment_RegistryCredential(t *testing.T) {
var project codebuild.Project
rName := acctest.RandomWithPrefix("tf-acc-test")
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_Environment_RegistryCredential(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSCodeBuildProjectExists(resourceName, &project),
),
},
},
})
}

func testAccCheckAWSCodeBuildProjectExists(n string, project *codebuild.Project) resource.TestCheckFunc {
return func(s *terraform.State) error {
rs, ok := s.RootModule().Resources[n]
Expand Down Expand Up @@ -1399,6 +1419,46 @@ resource "aws_codebuild_project" "test" {
`, oName, rName)
}

func testAccAWSCodeBuildProjectConfig_Environment_RegistryCredential(rName string) string {
return testAccAWSCodeBuildProjectConfig_Base_ServiceRole(rName) + fmt.Sprintf(`
resource "aws_codebuild_project" "test" {
name = %[1]q
service_role = "${aws_iam_role.test.arn}"

artifacts {
type = "NO_ARTIFACTS"
}

environment {
compute_type = "BUILD_GENERAL1_SMALL"
image = "2"
type = "LINUX_CONTAINER"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please fix the formatting here? Looks like spaces versus tabs. 😄

Suggested change
type = "LINUX_CONTAINER"
type = "LINUX_CONTAINER"

image_pull_credentials_type = "SERVICE_ROLE"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
image_pull_credentials_type = "SERVICE_ROLE"
image_pull_credentials_type = "SERVICE_ROLE"


registry_credential {
credential = "${aws_secretsmanager_secret_version.test.arn}"
credential_provider = "SECRETS_MANAGER"
}
}

source {
type = "GITHUB"
location = "https://github.com/hashicorp/packer.git"
}
}

resource "aws_secretsmanager_secret" "test" {
name = "test"
recovery_window_in_days = 0
}

resource "aws_secretsmanager_secret_version" "test" {
secret_id = "${aws_secretsmanager_secret.test.id}"
secret_string = "${jsonencode(map("username", "user", "password", "pass"))}"
}
`, rName)
}

func testAccAWSCodeBuildProjectConfig_Source_Auth(rName, authResource, authType string) string {
return testAccAWSCodeBuildProjectConfig_Base_ServiceRole(rName) + fmt.Sprintf(`
resource "aws_codebuild_project" "test" {
Expand Down
5 changes: 5 additions & 0 deletions website/docs/r/codebuild_project.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ The following arguments are supported:
* `environment_variable` - (Optional) A set of environment variables to make available to builds for this build project.
* `privileged_mode` - (Optional) If set to true, enables running the Docker daemon inside a Docker container. Defaults to `false`.
* `certificate` - (Optional) The ARN of the S3 bucket, path prefix and object key that contains the PEM-encoded certificate.
* `registry_credential` - (Optional) Information about credentials for access to a private Docker registry. Registry Credential config blocks are documented below.

`environment_variable` supports the following:

Expand Down Expand Up @@ -252,6 +253,10 @@ The following arguments are supported:
* `subnets` - (Required) The subnet IDs within which to run builds.
* `vpc_id` - (Required) The ID of the VPC within which to run builds.

`registry_credential` supports the following:

* `credential` - (Required) The Amazon Resource Name (ARN) or name of credentials created using AWS Secrets Manager.
* `credential_provider` - (Required) The service that created the credentials to access a private Docker registry. The valid value, SECRETS_MANAGER, is for AWS Secrets Manager.

`secondary_artifacts` supports the following:

Expand Down