CodeBuild in VPC continuing Issue #2547 #3324

moofish32 · 2018-02-11T07:50:55Z

updating 2547

@bflad -- I have made your requested changes here.
@DrSolaris happy to hand this back over, just trying to get it merged. I did squash your commits down during the conflict resolution.

@bflad -- the sourceAuth tests obviously fail locally, I'm not sure if it's worth making that test more intelligent.

moofish32 · 2018-02-11T09:07:23Z

aws/resource_aws_codebuild_project.go

+		for _, s := range vpcConfig.Subnets {
+			subnets = append(subnets, *s)
+		}
+		sort.Strings(subnets)


@bflad -- I'm still getting random sort ordering issues. Where am I going wrong?

moofish32 · 2018-02-11T23:32:16Z

aws/resource_aws_codebuild_project.go

+		return err
+	}
+
+	if err := d.Set("vpc_config", flattenAwsCodeBuildVpcConfig(project.VpcConfig)); err != nil {


No matter how I sort subnets (and I've tried a few). I seem to get failing scenarios, but this line seems to be the indicator of the failure:

vpc_config is set to [map[security_group_ids:[sg-19a81766] vpc_id:vpc-61a22318 subnets:[subnet-d503a08f subnet-953c9fcf]]] vpc_config is set to [map[vpc_id:vpc-61a22318 subnets:[subnet-d503a08f subnet-953c9fcf] security_group_ids:[sg-19a81766]]]

The object has the same values. The subnets are in the same order, but I get this error:

--- FAIL: TestAccAWSCodeBuildProject_basic (43.42s) testing.go:513: Step 0 error: After applying this step, the plan was not empty: DIFF: UPDATE: aws_codebuild_project.foo vpc_config.0.subnets.0: "subnet-d503a08f" => "subnet-953c9fcf" vpc_config.0.subnets.1: "subnet-953c9fcf" => "subnet-d503a08f" STATE:

Is it easier just go back to Set and calculate the hash for the difference?

The subnets and security_group_ids can be schema.TypeSet, but you don't need a special hash function for them. Terraform core will "do the right thing" when comparing simple string sets.

bflad · 2018-02-12T00:10:01Z

@bflad -- the sourceAuth tests obviously fail locally, I'm not sure if it's worth making that test more intelligent.

Are you on the latest master code? What is the error you are seeing? We should address this separately if its an issue as it passes in our daily acceptance testing.

=== RUN   TestAccAWSCodeBuildProject_sourceAuth
--- PASS: TestAccAWSCodeBuildProject_sourceAuth (30.17s)

moofish32 · 2018-02-12T16:36:32Z

@bflad sourceAuth is generating this for me:

=== RUN   TestAccAWSCodeBuildProject_sourceAuth
--- FAIL: TestAccAWSCodeBuildProject_sourceAuth (9.65s)
        testing.go:513: Step 1 error: Error applying: 1 error(s) occurred:

                * aws_codebuild_project.foo: 1 error(s) occurred:

                * aws_codebuild_project.foo: [ERROR] Error creating CodeBuild project: InvalidInputException: No Access token found, please visit AWS CodeBuild console to connect to GitHub
                        status code: 400, request id: 1be10e94-1012-11e8-91d8-f7a26fdbda1c
        testing.go:573: Error destroying resource! WARNING: Dangling resources
                may exist. The full state and error is shown below.

                Error: Check failed: Default error in CodeBuild Test

                State: <no state>

I expect this is due to my git setup having some unique-ness because I have public github, work github enterprise, and experimental Code Commit (work). All being used in various flavors on my laptop. Is this worth a separate issue?

moofish32 · 2018-02-12T16:38:02Z

Regarding the in VPC config. I've made the changes back to string and Set please let me know if we need any other work to get this into the merge queue.

bflad

@moofish32 this is looking really good! Thanks for picking up the torch here. Can you split the testing for vpc_config into its own test then I think this is ready to 🚢 !

make testacc TEST=./aws TESTARGS='-run=TestAccAWSCodeBuildProject'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSCodeBuildProject -timeout 120m
=== RUN   TestAccAWSCodeBuildProject_basic
--- PASS: TestAccAWSCodeBuildProject_basic (69.94s)
=== RUN   TestAccAWSCodeBuildProject_sourceAuth
--- PASS: TestAccAWSCodeBuildProject_sourceAuth (30.96s)
=== RUN   TestAccAWSCodeBuildProject_default_build_timeout
--- PASS: TestAccAWSCodeBuildProject_default_build_timeout (36.86s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	137.806s

bflad · 2018-02-12T18:57:29Z

aws/resource_aws_codebuild_project_test.go

 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSCodeBuildProjectExists("aws_codebuild_project.foo"),
 					resource.TestCheckResourceAttr(
 						"aws_codebuild_project.foo", "build_timeout", "5"),
+					resource.TestCheckResourceAttrSet("aws_codebuild_project.foo", "vpc_config.0.vpc_id"),


Can you please move the vpc_config testing to its own acceptance test? Its an optional attribute and we really prefer to keep the _basic tests only for required attributes. It should continue to test removing the configuration 👍

bflad · 2018-02-12T19:00:22Z

aws/resource_aws_codebuild_project.go

+							Type:     schema.TypeSet,
+							Required: true,
+							Elem:     &schema.Schema{Type: schema.TypeString},
+							Set:      schema.HashString,


Set function should not be required for a simple TypeString set (same with security_group_ids attribute). Although you might have to do this since its nested, I think I ran into something similar recently but would love confirmation.

bflad · 2018-02-12T19:01:49Z

aws/resource_aws_codebuild_project.go

+		values := map[string]interface{}{}
+
+		values["vpc_id"] = *vpcConfig.VpcId
+		values["subnets"] = schema.NewSet(schema.HashString, flattenStringList(vpcConfig.Subnets))


Does this work with just values["subnets"] = flattenStringList(vpcConfig.Subnets)?

testing.go:513: Step 0 error: Error applying: 1 error(s) occurred: * aws_codebuild_project.foo: 1 error(s) occurred: * aws_codebuild_project.foo: Invalid address to set: []string{"vpc_config", "0", "subnets"} testing.go:573: Error destroying resource! WARNING: Dangling resources may exist. The full state and error is shown below. Error: Error refreshing: 1 error(s) occurred: * aws_codebuild_project.foo: 1 error(s) occurred: * aws_codebuild_project.foo: aws_codebuild_project.foo: Invalid address to set: []string{"vpc_config", "0", "subnets"} State: <nil> FAIL

I don't think so sadly :(

It appears d.Set has the magic buried in there to make this work. I tried a few tricks to pull it out so it would work for subresources, but I couldn't quite make it work. I've stopped and decided that perhaps core should add some features to set and get with subresources instead of my hack-ish attempts. If you think we should invest more here let me know, but right now I didn't want to block this with that.

bflad · 2018-02-12T19:03:36Z

aws/resource_aws_codebuild_project_test.go

@@ -350,8 +365,27 @@ func testAccCheckAWSCodeBuildProjectDestroy(s *terraform.State) error {
 	return fmt.Errorf("Default error in CodeBuild Test")
 }

-func testAccAWSCodeBuildProjectConfig_basic(rName string) string {
+func testAccAWSCodeBuildProjectConfig_basic(rName string, subnet string) string {


Same with the mention above about the test itself, the VPC configuration should be separated as well. Thanks!

Documentation for VPC block within aws_codebuild_project resource Update basic codebuild test to support VPC block. Currently contains IAM race condition Add further error catching to CodeBuild resource creation to deal with more IAM eventual consistency. Remove unnecessary depends_on clause from CodeBuild basic acceptance test. Formatting fix for codepipline environment variables documentation

… and removal

moofish32 · 2018-02-17T20:35:58Z

@bflad -- one more review please? Thanks for the feedback so far.

moofish32 · 2018-02-22T19:40:01Z

@bflad -- did we lose momentum?

bflad

LGTM -- Sorry I wasn't able to respond quickly, I'm out of office this week, but this is good to get in. Thanks for the work here. 👍

=== RUN   TestAccAWSCodeBuildProject_sourceAuth
--- PASS: TestAccAWSCodeBuildProject_sourceAuth (27.48s)
=== RUN   TestAccAWSCodeBuildProject_basic
--- PASS: TestAccAWSCodeBuildProject_basic (27.63s)
=== RUN   TestAccAWSCodeBuildProject_default_build_timeout
--- PASS: TestAccAWSCodeBuildProject_default_build_timeout (34.57s)
=== RUN   TestAccAWSCodeBuildProject_vpc
--- PASS: TestAccAWSCodeBuildProject_vpc (45.92s)

bflad · 2018-02-22T22:43:11Z

aws/resource_aws_codebuild_project_test.go

@@ -686,3 +737,40 @@ resource "aws_codebuild_project" "foo" {
 }
 `, rName, authResource, authType)
 }
+
+func hclVpcResources() string {


FYI these function names are global across the entire provider so we would generally prefer sticking with the long for testAccAWSCodeBuildProjectConfig_X

bflad · 2018-02-27T12:46:19Z

This has been released in version 1.10.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-07T17:54:08Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 11, 2018

moofish32 force-pushed the f-codebuild-in-vpc-2495 branch from dbc8d5b to b1207c8 Compare February 11, 2018 08:58

ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 11, 2018

moofish32 changed the title ~~Continuing Issue #2495~~ CodeBuild in VPC continuing Issue #2547 Feb 11, 2018

moofish32 commented Feb 11, 2018

View reviewed changes

radeksimko added enhancement Requests to existing resources that expand the functionality or scope. service/codebuild Issues and PRs that pertain to the codebuild service. labels Feb 11, 2018

moofish32 commented Feb 11, 2018

View reviewed changes

bflad self-assigned this Feb 12, 2018

ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 12, 2018

bflad suggested changes Feb 12, 2018

View reviewed changes

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Feb 12, 2018

Ryan Moore and others added 2 commits February 17, 2018 12:31

adding and separating tests for vpc_config codebuild projects changes…

eb9bf45

… and removal

moofish32 force-pushed the f-codebuild-in-vpc-2495 branch from 0b57406 to eb9bf45 Compare February 17, 2018 20:31

ghost added the size/L Managed by automation to categorize the size of a PR. label Feb 17, 2018

bflad approved these changes Feb 22, 2018

View reviewed changes

bflad removed the waiting-response Maintainers are waiting on response from community or contributor. label Feb 22, 2018

bflad added this to the v1.10.0 milestone Feb 22, 2018

bflad merged commit 173342c into hashicorp:master Feb 22, 2018

This was referenced Feb 22, 2018

Support CodeBuild within VPC #2547

Closed

Add support for CodeBuild with VPC #2495

Closed

bflad added a commit that referenced this pull request Feb 22, 2018

Update CHANGELOG for #2547 and #3324

a842695

moofish32 mentioned this pull request Feb 24, 2018

Fix Naming on Test Methods in Codebuild VPC #3517

Merged

emmanuel mentioned this pull request Oct 31, 2018

In Terraform 0.11.8, aws_codebuild_project resource does not expose logsConfig from AWS API #6312

Closed

ghost locked and limited conversation to collaborators Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CodeBuild in VPC continuing Issue #2547 #3324

CodeBuild in VPC continuing Issue #2547 #3324

moofish32 commented Feb 11, 2018 •

edited

Loading

moofish32 Feb 11, 2018

moofish32 Feb 11, 2018

bflad Feb 12, 2018

bflad commented Feb 12, 2018

moofish32 commented Feb 12, 2018

moofish32 commented Feb 12, 2018

bflad left a comment

bflad Feb 12, 2018

bflad Feb 12, 2018

bflad Feb 12, 2018

moofish32 Feb 14, 2018

moofish32 Feb 17, 2018 •

edited

Loading

bflad Feb 12, 2018

moofish32 commented Feb 17, 2018

moofish32 commented Feb 22, 2018

bflad left a comment

bflad Feb 22, 2018

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

CodeBuild in VPC continuing Issue #2547 #3324

CodeBuild in VPC continuing Issue #2547 #3324

Conversation

moofish32 commented Feb 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad commented Feb 12, 2018

moofish32 commented Feb 12, 2018

moofish32 commented Feb 12, 2018

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moofish32 Feb 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moofish32 commented Feb 17, 2018

moofish32 commented Feb 22, 2018

bflad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

moofish32 commented Feb 11, 2018 •

edited

Loading

moofish32 Feb 17, 2018 •

edited

Loading