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

Support additional CodeBuild settings, retry for IAM errors #3929

Merged
merged 8 commits into from
May 24, 2018
Merged

Support additional CodeBuild settings, retry for IAM errors #3929

merged 8 commits into from
May 24, 2018

Conversation

errriclee
Copy link
Contributor

Support CodeBuild environment variable type, git clone depth, insecure SSL options
Retry UpdateProject similar to CreateProject

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Mar 27, 2018
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. service/codebuild Issues and PRs that pertain to the codebuild service. labels Mar 27, 2018
@bflad
Copy link
Contributor

bflad commented Mar 27, 2018

Supporting Git clone depth already has a pending PR: #3467

Can you please work with the original author or implement on top of their commits? The main issue with adding these new attributes inside source is that since source is a TypeSet, it requires a state migration (either to TypeList or an updated TypeSet hash)

@bflad bflad mentioned this pull request Mar 27, 2018
Closed
@kwent
Copy link
Contributor

kwent commented Mar 30, 2018

Looking forward to this !

@kwent
Copy link
Contributor

kwent commented Mar 30, 2018

They also added branchFilter aws/aws-sdk-go@c0ea8ae#diff-4ac32a78649ca5bdd8e0ba38b7006a1eR8

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Apr 2, 2018
@errriclee
Copy link
Contributor Author

I don't plan on supporting the webhook branch filter in this PR, but it seems separate enough it could be in a separate PR without stepping on toes.

@errriclee
Copy link
Contributor Author

@bflad is this good to go?

bflad
bflad reviewed Apr 17, 2018
View reviewed changes
aws/resource_aws_codebuild_project.go
@@ -527,7 +557,22 @@ func resourceAwsCodeBuildProjectUpdate(d *schema.ResourceData, meta interface{})
// 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)
err := resource.Retry(5*time.Minute, func() *resource.RetryError {
Copy link
Contributor

Choose a reason for hiding this comment

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

We tend to only retry for 1 minute unless there is heavy evidence it requires longer. Can you please also move this to its own PR so it can get merged separately of the other changes? Reference: #4233

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now #4238

@bflad bflad mentioned this pull request Apr 17, 2018
Closed
@errriclee errriclee mentioned this pull request Apr 17, 2018
Merged
@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Apr 17, 2018
@errriclee
Revert "Revert "Retry UpdateProject similar to CreateProject since IA…
da75f28 
…M is eventually consistent""

This reverts commit 0a75b77, now that
f-codebuild-updateproj-retry was merged.
@salvianreynaldi
Copy link
Contributor

@kwent branch filter is already created here #2814

@salvianreynaldi salvianreynaldi mentioned this pull request Apr 25, 2018
Merged
@joestump joestump mentioned this pull request May 7, 2018
Closed
@bflad bflad mentioned this pull request May 24, 2018
Merged
1 task
@bflad
Copy link
Contributor

bflad commented May 24, 2018

After looking through the various duplicate pull requests, I'm going to try to get the commits merged in the following order here (resolving the merge conflicts and test changes along the way, which could be tedious): #2811 -> #4021 -> #3929. 🤞

In the end the following new arguments will be supported:

  • environment > environment_variable > type
  • source > git_clone_depth
  • source > insecure_ssl

@bflad
Copy link
Contributor

bflad commented May 24, 2018

After merging together the above pull requests and refactoring the test suite, we should be good to go:

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 (31.87s)
=== RUN   TestAccAWSCodeBuildProject_BadgeEnabled
--- PASS: TestAccAWSCodeBuildProject_BadgeEnabled (30.05s)
=== RUN   TestAccAWSCodeBuildProject_BuildTimeout
--- PASS: TestAccAWSCodeBuildProject_BuildTimeout (38.35s)
=== RUN   TestAccAWSCodeBuildProject_Cache
--- PASS: TestAccAWSCodeBuildProject_Cache (59.77s)
=== RUN   TestAccAWSCodeBuildProject_Description
--- PASS: TestAccAWSCodeBuildProject_Description (38.08s)
=== RUN   TestAccAWSCodeBuildProject_EncryptionKey
--- PASS: TestAccAWSCodeBuildProject_EncryptionKey (53.94s)
=== RUN   TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Type
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Type (30.31s)
=== RUN   TestAccAWSCodeBuildProject_Source_Auth
--- PASS: TestAccAWSCodeBuildProject_Source_Auth (20.33s)
=== RUN   TestAccAWSCodeBuildProject_Source_GitCloneDepth
--- PASS: TestAccAWSCodeBuildProject_Source_GitCloneDepth (38.51s)
=== RUN   TestAccAWSCodeBuildProject_Source_InsecureSSL
--- PASS: TestAccAWSCodeBuildProject_Source_InsecureSSL (37.43s)
=== RUN   TestAccAWSCodeBuildProject_Tags
--- PASS: TestAccAWSCodeBuildProject_Tags (28.91s)
=== RUN   TestAccAWSCodeBuildProject_VpcConfig
--- PASS: TestAccAWSCodeBuildProject_VpcConfig (57.63s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	465.209s

This will release with v1.21.0 of the AWS provider, likely middle of next week.

@bflad bflad added this to the v1.21.0 milestone May 24, 2018
@bflad bflad merged commit da75f28 into hashicorp:master May 24, 2018
bflad added a commit that referenced this pull request May 24, 2018
@bflad
Update CHANGELOG for #2811, #4021, and #3929
88c73a7
@bflad
Copy link
Contributor

bflad commented May 31, 2018

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

@ghost
Copy link

ghost commented Apr 5, 2020

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 ghost locked and limited conversation to collaborators Apr 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bflad bflad bflad left review comments

Assignees

@bflad bflad

Labels
enhancement Requests to existing resources that expand the functionality or scope. service/codebuild Issues and PRs that pertain to the codebuild service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Milestone
v1.21.0
Development

Successfully merging this pull request may close these issues.
4 participants
@errriclee @bflad @kwent @salvianreynaldi