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

Support CodeBuild within VPC #2547

Closed
wants to merge 5 commits into from
Closed

Support CodeBuild within VPC #2547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
56a5b3d
Add VPC config block to aws_codebuild_project resource
Dec 5, 2017
4581628
Documentation for VPC block within aws_codebuild_project resource
Dec 5, 2017
cd23499
Update basic codebuild test to support VPC block. Currently contains …
Dec 5, 2017
0bef8ec
Add further error catching to CodeBuild resource creation to deal wit…
Dec 5, 2017
f417d49
Formatting fix for codepipline environment variables documentation
Dec 5, 2017
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
105 changes: 105 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 @@ -179,6 +179,31 @@ func resourceAwsCodeBuildProject() *schema.Resource {
ValidateFunc: validateAwsCodeBuildTimeout,
},
"tags": tagsSchema(),
"vpc_config": {
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.

I would switch this to Type: schema.TypeList,. There are subtleties between the two, but the gist being that list in this context is much easier to work with when there's only 1 item.

Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"vpc_id": {
Type: schema.TypeString,
Required: true,
},
"subnets": {
Type: schema.TypeList,
Required: true,
Elem: &schema.Schema{Type: schema.TypeString},
MaxItems: 16,
},
"security_group_ids": {
Type: schema.TypeList,
Required: true,
Elem: &schema.Schema{Type: schema.TypeString},
MaxItems: 5,
},
},
},
},
},
}
}
Expand Down Expand Up @@ -213,6 +238,11 @@ func resourceAwsCodeBuildProjectCreate(d *schema.ResourceData, meta interface{})
params.TimeoutInMinutes = aws.Int64(int64(v.(int)))
}

if _, ok := d.GetOk("vpc_config"); ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would change this to (note about function signature update below in another comment):

if v, ok := d.GetOk("vpc_config"); ok {
	params.VpcConfig = expandCodeBuildVpcConfig(v.([]interface{})[0].(map[string]interface{}))
}

vpcConfig := expandVpcConfig(d)
params.VpcConfig = vpcConfig
}

if v, ok := d.GetOk("tags"); ok {
params.Tags = tagsFromMapCodeBuild(v.(map[string]interface{}))
}
Expand All @@ -228,6 +258,10 @@ func resourceAwsCodeBuildProjectCreate(d *schema.ResourceData, meta interface{})
return resource.RetryableError(err)
}

if isAWSErr(err, "InvalidInputException", "Not authorized to perform DescribeSecurityGroups") {
return resource.RetryableError(err)
}

return resource.NonRetryableError(err)
}

Expand Down Expand Up @@ -324,6 +358,29 @@ func expandProjectEnvironment(d *schema.ResourceData) *codebuild.ProjectEnvironm
return projectEnv
}

func expandVpcConfig(d *schema.ResourceData) *codebuild.VpcConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

We really try to limit the usage of *schema.ResourceData in downstream functions unless its necessary in multiple pieces of logic. It helps make the code more testable and reusable (if necessary in the future). Could you update this function's signature to expandCodeBuildVpcConfig(m map[string]interface{}) *codebuild.VpcConfig? Thanks!

configs := d.Get("vpc_config").(*schema.Set).List()
data := configs[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.

This line is generating a panic for me running the acceptance test:

=== RUN   TestAccAWSCodeBuildProject_basic
panic: runtime error: index out of range

goroutine 2579 [running]:
github.com/terraform-providers/terraform-provider-aws/aws.expandVpcConfig(0xc42084b110, 0x3715e47)
	/Users/bflad/go/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_codebuild_project.go:363 +0x790

It will go away with the function signature refactor and TypeList changes mentioned above.


vpcConfig := codebuild.VpcConfig{
VpcId: aws.String(data["vpc_id"].(string)),
}

var vpcConfigSubnets []*string
for _, v := range data["subnets"].([]interface{}) {
vpcConfigSubnets = append(vpcConfigSubnets, aws.String(v.(string)))
}
vpcConfig.Subnets = vpcConfigSubnets

var vpcSecurityGroupIds []*string
for _, s := range data["security_group_ids"].([]interface{}) {
vpcSecurityGroupIds = append(vpcSecurityGroupIds, aws.String(s.(string)))
}
vpcConfig.SecurityGroupIds = vpcSecurityGroupIds

return &vpcConfig
}

func expandProjectSource(d *schema.ResourceData) codebuild.ProjectSource {
configs := d.Get("source").(*schema.Set).List()
projectSource := codebuild.ProjectSource{}
Expand Down Expand Up @@ -390,6 +447,10 @@ func resourceAwsCodeBuildProjectRead(d *schema.ResourceData, meta interface{}) e
return err
}

if err := d.Set("vpc_config", schema.NewSet(resourceAwsCodeBuildVpcConfigHash, flattenAwsCodebuildVpcConfig(project.VpcConfig))); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

When this is TypeList this can become:

if err := d.Set("vpc_config", flattenAwsCodebuildVpcConfig(project.VpcConfig)); err != nil {

return err
}

d.Set("description", project.Description)
d.Set("encryption_key", project.EncryptionKey)
d.Set("name", project.Name)
Expand Down Expand Up @@ -425,6 +486,11 @@ func resourceAwsCodeBuildProjectUpdate(d *schema.ResourceData, meta interface{})
params.Artifacts = &projectArtifacts
}

if d.HasChange("vpc_config") {
vpcConfig := expandVpcConfig(d)
params.VpcConfig = vpcConfig
}

if d.HasChange("description") {
params.Description = aws.String(d.Get("description").(string))
}
Expand Down Expand Up @@ -556,6 +622,26 @@ func flattenAwsCodebuildProjectSource(source *codebuild.ProjectSource) *schema.S

}

func flattenAwsCodebuildVpcConfig(vpcConfig *codebuild.VpcConfig) []interface{} {
values := map[string]interface{}{}

values["vpc_id"] = *vpcConfig.VpcId

var subnets []string
for _, s := range vpcConfig.Subnets {
subnets = append(subnets, *s)
}
values["subnets"] = subnets

var securityGroupIds []string
for _, s := range vpcConfig.SecurityGroupIds {
securityGroupIds = append(securityGroupIds, *s)
}
values["security_group_ids"] = securityGroupIds

return []interface{}{values}
}

func resourceAwsCodeBuildProjectArtifactsHash(v interface{}) int {
var buf bytes.Buffer
m := v.(map[string]interface{})
Expand All @@ -567,6 +653,25 @@ func resourceAwsCodeBuildProjectArtifactsHash(v interface{}) int {
return hashcode.String(buf.String())
}

func resourceAwsCodeBuildVpcConfigHash(v interface{}) int {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is extraneous with switch to TypeList 👍

var buf bytes.Buffer
m := v.(map[string]interface{})

buf.WriteString(fmt.Sprintf("%s-", m["vpc_id"].(string)))

for _, s := range m["subnets"].([]string) {
buf.WriteString(fmt.Sprintf("%s-", s))
}

if m["security_group_ids"] != nil {
for _, s := range m["security_group_ids"].([]string) {
buf.WriteString(fmt.Sprintf("%s-", s))
}
}

return hashcode.String(buf.String())
}

func resourceAwsCodeBuildProjectEnvironmentHash(v interface{}) int {
var buf bytes.Buffer
m := v.(map[string]interface{})
Expand Down
69 changes: 61 additions & 8 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 @@ -324,6 +324,19 @@ func testAccCheckAWSCodeBuildProjectDestroy(s *terraform.State) error {

func testAccAWSCodeBuildProjectConfig_basic(rName string) string {
return fmt.Sprintf(`
resource "aws_vpc" "codebuild_vpc" {
cidr_block = "10.0.0.0/16"
}

resource "aws_subnet" "codebuild_subnet" {
vpc_id = "${aws_vpc.codebuild_vpc.id}"
cidr_block = "10.0.0.0/24"
}

resource "aws_security_group" "codebuild_security_group" {
vpc_id = "${aws_vpc.codebuild_vpc.id}"
}

resource "aws_iam_role" "codebuild_role" {
name = "codebuild-role-%s"
assume_role_policy = <<EOF
Expand All @@ -343,10 +356,10 @@ EOF
}

resource "aws_iam_policy" "codebuild_policy" {
name = "codebuild-policy-%s"
path = "/service-role/"
description = "Policy used in trust relationship with CodeBuild"
policy = <<POLICY
name = "codebuild-policy-%s"
path = "/service-role/"
description = "Policy used in trust relationship with CodeBuild"
policy = <<POLICY
{
"Version": "2012-10-17",
"Statement": [
Expand All @@ -360,6 +373,34 @@ resource "aws_iam_policy" "codebuild_policy" {
"logs:CreateLogStream",
"logs:PutLogEvents"
]
},
{
"Effect": "Allow",
"Action": [
"ec2:CreateNetworkInterface",
"ec2:DescribeDhcpOptions",
"ec2:DescribeNetworkInterfaces",
"ec2:DeleteNetworkInterface",
"ec2:DescribeSubnets",
"ec2:DescribeSecurityGroups",
"ec2:DescribeVpcs"
],
"Resource": "*"
},
{
"Effect": "Allow",
"Action": [
"ec2:CreateNetworkInterfacePermission"
],
"Resource": "arn:aws:ec2::*:network-interface/*",
"Condition": {
"StringEquals": {
"ec2:Subnet": [
"arn:aws:ec2::*:subnet/${aws_subnet.codebuild_subnet.id}"
],
"ec2:AuthorizedService": "codebuild.amazonaws.com"
}
}
}
]
}
Expand All @@ -373,10 +414,10 @@ resource "aws_iam_policy_attachment" "codebuild_policy_attachment" {
}

resource "aws_codebuild_project" "foo" {
name = "test-project-%s"
description = "test_codebuild_project"
build_timeout = "5"
service_role = "${aws_iam_role.codebuild_role.arn}"
name = "test-project-%s"
description = "test_codebuild_project"
build_timeout = "5"
service_role = "${aws_iam_role.codebuild_role.arn}"

artifacts {
type = "NO_ARTIFACTS"
Expand All @@ -401,6 +442,18 @@ resource "aws_codebuild_project" "foo" {
tags {
"Environment" = "Test"
}

vpc_config {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this new attribute is optional and has some complexity, we should probably split it out into its own acceptance test. We need to verify the resource creation (what's done here already), the update of vpc_config attributes (e.g. add/remove subnet), and the removal of the vpc_config altogether from a configuration (if removing VPC configuration is supported). It would also be good to check that the new attributes are at least making it correctly into the state. e.g.

resource.TestCheckResourceSet("aws_codebuild_project.foo", "vpc_config.0.vpc_id"),
resource.TestCheckResource("aws_codebuild_project.foo", "vpc_config.0.subnets.#", "1"),
resource.TestCheckResource("aws_codebuild_project.foo", "vpc_config.0.security_group_ids.#", "1"),

vpc_id = "${aws_vpc.codebuild_vpc.id}"

subnets = [
"${aws_subnet.codebuild_subnet.id}"
]

security_group_ids = [
"${aws_security_group.codebuild_security_group.id}"
]
}
}
`, rName, rName, rName, rName)
}
Expand Down
23 changes: 22 additions & 1 deletion website/docs/r/codebuild_project.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,20 @@ resource "aws_codebuild_project" "foo" {
location = "https://github.com/mitchellh/packer.git"
}

vpc_config {
vpc_id = "vpc-725fca"

subnets = [
"subnet-ba35d2e0",
"subnet-ab129af1",
]

security_group_ids = [
"sg-f9f27d91",
"sg-e4f48g23",
]
}

tags {
"Environment" = "Test"
}
Expand All @@ -113,6 +127,7 @@ The following arguments are supported:
* `artifacts` - (Required) Information about the project's build output artifacts. Artifact blocks are documented below.
* `environment` - (Required) Information about the project's build environment. Environment blocks are documented below.
* `source` - (Required) Information about the project's input source code. Source blocks are documented below.
* `vpc_config` - (Optional) Configuration for the builds to run inside a VPC. VPC config blocks are documented below.

`artifacts` supports the following:

Expand All @@ -132,6 +147,7 @@ The following arguments are supported:
* `environment_variable` - (Optional) A set of environment variables to make available to builds for this build project.

`environment_variable` supports the following:

* `name` - (Required) The environment variable's name or key.
* `value` - (Required) The environment variable's value.

Expand All @@ -147,6 +163,12 @@ The following arguments are supported:
* `type` - (Required) The authorization type to use. The only valid value is `OAUTH`
* `resource` - (Optional) The resource value that applies to the specified authorization type.

`vpc_config` supports the following:

* `vpc_id` - (Required) The ID of the VPC within which to run builds.
* `subnets` - (Required) The subnet IDs within which to run builds.
* `security_group_ids` - (Required) The security group IDs to assign to running builds.

## Attributes Reference

The following attributes are exported:
Expand All @@ -156,4 +178,3 @@ The following attributes are exported:
* `encryption_key` - The AWS Key Management Service (AWS KMS) customer master key (CMK) that was used for encrypting the build project's build output artifacts.
* `name` - The projects name.
* `service_role` - The ARN of the IAM service role.