service/codebuild: use typelist where maxitems are 1 #13417

anGie44 · 2020-05-20T02:16:54Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #9956

Release note for CHANGELOG:

use typelist for attributes with maxitems of 1 and remove DisableBinaryDriver refs

Output from acceptance testing:

--- SKIP: TestAccAWSCodeBuildProject_SecondaryArtifacts_Name (0.00s)
--- PASS: TestAccAWSCodeBuildProject_basic (47.95s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_GitHubEnterprise (50.96s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_CodeCommit (59.26s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Type (59.34s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodeCommit (63.95s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_Bitbucket (64.51s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodePipeline (78.75s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts (83.83s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHubEnterprise (87.31s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitCloneDepth (94.39s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Path (112.91s)
--- FAIL: TestAccAWSCodeBuildProject_Source_Auth (35.62s) -- unrelated: No Access token found, please visit AWS CodeBuild
--- PASS: TestAccAWSCodeBuildProject_Environment_RegistryCredential (124.81s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Location (125.16s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Packaging (129.07s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_Bitbucket (85.30s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHub (88.50s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_NamespaceType (137.39s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Type (143.18s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_GitSubmodulesConfig_GitHubEnterprise (84.69s)
--- PASS: TestAccAWSCodeBuildProject_Source_InsecureSSL (85.02s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_OverrideArtifactName (147.90s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Path (150.63s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_GitSubmodulesConfig_CodeCommit (88.98s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_ArtifactIdentifier (163.42s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_GitSubmodulesConfig_GitHub (102.21s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitSubmodulesConfig_GitHub (74.30s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_EncryptionDisabled (176.14s)
--- PASS: TestAccAWSCodeBuildProject_SourceVersion (43.61s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitSubmodulesConfig_GitHubEnterprise (100.25s)
--- PASS: TestAccAWSCodeBuildProject_EncryptionKey (61.21s)
--- PASS: TestAccAWSCodeBuildProject_BadgeEnabled (48.29s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitSubmodulesConfig_CodeCommit (70.19s)
--- PASS: TestAccAWSCodeBuildProject_Environment_Certificate (93.34s)
--- PASS: TestAccAWSCodeBuildProject_QueuedTimeout (71.76s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable (154.73s)
--- PASS: TestAccAWSCodeBuildProject_BuildTimeout (88.53s)
--- PASS: TestAccAWSCodeBuildProject_Description (98.00s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Value (130.94s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_CloudWatchLogs (133.60s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSourceInvalid (43.29s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Packaging (118.50s)
--- PASS: TestAccAWSCodeBuildProject_Tags (85.27s)
--- PASS: TestAccAWSCodeBuildProject_WindowsContainer (51.70s)
--- PASS: TestAccAWSCodeBuildProject_ARMContainer (52.90s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Type (150.71s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSource (69.03s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_S3Logs (168.23s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_OverrideArtifactName (117.46s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_ArtifactIdentifier (139.53s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Location (127.58s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_EncryptionDisabled (123.86s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_S3 (68.80s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_NamespaceType (128.18s)
--- PASS: TestAccAWSCodeBuildProject_Cache (182.06s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Name (123.96s)
--- PASS: TestAccAWSCodeBuildProject_VpcConfig (98.21s)

…ryDriver refs

anGie44 · 2020-05-20T02:32:27Z

aws/resource_aws_codebuild_project.go

@@ -178,7 +177,6 @@ func resourceAwsCodeBuildProject() *schema.Resource {
 						"environment_variable": {
 							Type:     schema.TypeList,
 							Optional: true,
-							Computed: true,


@bflad this helped TestAccAWSCodeBuildProject_Environment_EnvironmentVariable pass but I'm not sure if this is the best change; in the original test, updating from 2 env variables -> 0 failed but 1->2 passed 😕

This seems like a good thing to me, especially if the existing testing was expecting it to behave this way (it likely worked previously because of the Set function). It enables proper drift detection if the environment variables are not in the Terraform configuration, but they are configured in the API. The original implementation (hashicorp/terraform#11560) includes this without comment, so I'm willing to say that we should feel okay changing this now given that generally we want to enable drift detection where possible. CodeBuild also supports hiding sensitive values now via SSM and Secrets Manager. Worst case, we can re-enable Computed, if for some reason this must be hidden from the diff in some cases, which should have their own, new acceptance tests to prevent this potential "regression" again. 😄

We'll definitely want to note this particular change in the CHANGELOG though, just in case it might catch folks off guard. 👍

bflad · 2020-05-20T02:58:33Z

aws/resource_aws_codebuild_project.go

@@ -178,7 +177,6 @@ func resourceAwsCodeBuildProject() *schema.Resource {
 						"environment_variable": {
 							Type:     schema.TypeList,
 							Optional: true,
-							Computed: true,


This seems like a good thing to me, especially if the existing testing was expecting it to behave this way (it likely worked previously because of the Set function). It enables proper drift detection if the environment variables are not in the Terraform configuration, but they are configured in the API. The original implementation (hashicorp/terraform#11560) includes this without comment, so I'm willing to say that we should feel okay changing this now given that generally we want to enable drift detection where possible. CodeBuild also supports hiding sensitive values now via SSM and Secrets Manager. Worst case, we can re-enable Computed, if for some reason this must be hidden from the diff in some cases, which should have their own, new acceptance tests to prevent this potential "regression" again. 😄

We'll definitely want to note this particular change in the CHANGELOG though, just in case it might catch folks off guard. 👍

bflad · 2020-05-20T03:00:40Z

aws/resource_aws_codebuild_project.go

@@ -1516,84 +1512,6 @@ func resourceAwsCodeBuildProjectArtifactsHash(v interface{}) int {
 	return hashcode.String(buf.String())
 }

-func resourceAwsCodeBuildProjectEnvironmentHash(v interface{}) int {


aws/resource_aws_codebuild_project_test.go

aws/resource_aws_codebuild_project.go

bflad · 2020-05-20T03:42:52Z

Please note that I've also marked this to close #10615 since the original issue report refers to a problem with the set hashing, that will no longer be present.

…blebinarydriver flag where applicable

anGie44 · 2020-05-20T14:36:25Z

@bflad I also started using the string ptr -> string conversion func in one place then ended up looking at the whole file 😅 lmk if I should change those back as well!

bflad

This looks good to go, thanks for all your effort into this beast of an issue. 🚀 Please reach out if you'd like any help with changelog or issue commenting.

Output from acceptance testing (VPC config one needs an issue opened, OAuth testing is completely manual, which we may want to consider hiding that test behind an environment variable):

--- FAIL: TestAccAWSCodeBuildProject_Source_Auth (11.50s)
--- PASS: TestAccAWSCodeBuildProject_BadgeEnabled (23.67s)
--- PASS: TestAccAWSCodeBuildProject_Description (52.62s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_CloudWatchLogs (55.68s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_GitSubmodulesConfig_GitHub (63.09s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitSubmodulesConfig_GitHub (77.11s)
--- PASS: TestAccAWSCodeBuildProject_basic (85.00s)
--- PASS: TestAccAWSCodeBuildProject_SourceVersion (86.97s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitSubmodulesConfig_GitHubEnterprise (89.06s)
--- PASS: TestAccAWSCodeBuildProject_QueuedTimeout (88.99s)
--- PASS: TestAccAWSCodeBuildProject_Environment_Certificate (92.68s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Type (95.28s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitSubmodulesConfig_CodeCommit (96.34s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_GitSubmodulesConfig_GitHubEnterprise (75.27s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable (106.75s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSourceInvalid (11.68s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_Bitbucket (54.60s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_Bitbucket (32.26s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodeCommit (32.38s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSource (30.63s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitCloneDepth (137.75s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_GitHubEnterprise (48.92s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodePipeline (50.46s)
--- PASS: TestAccAWSCodeBuildProject_Source_InsecureSSL (93.21s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_ArtifactIdentifier (33.25s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_S3Logs (153.71s)
--- PASS: TestAccAWSCodeBuildProject_Tags (58.31s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_S3 (71.52s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHub (96.63s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Name (35.31s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_NamespaceType (37.37s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_EncryptionDisabled (65.58s)
--- SKIP: TestAccAWSCodeBuildProject_SecondaryArtifacts_Name (0.00s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHubEnterprise (110.56s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Packaging (42.54s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts (38.99s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Path (46.35s)
--- PASS: TestAccAWSCodeBuildProject_EncryptionKey (198.18s)
--- PASS: TestAWSCodeBuildProject_nameValidation (0.00s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_OverrideArtifactName (64.06s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Type (70.45s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Packaging (39.19s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_NamespaceType (51.99s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_CodeCommit (40.82s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Location (64.50s)
--- PASS: TestAccAWSCodeBuildProject_ARMContainer (131.31s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Location (116.51s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Path (54.57s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_EncryptionDisabled (72.07s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_OverrideArtifactName (77.64s)
--- PASS: TestAccAWSCodeBuildProject_Environment_RegistryCredential (55.33s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Type (66.01s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Value (263.13s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_ArtifactIdentifier (99.59s)
--- PASS: TestAccAWSCodeBuildProject_BuildTimeout (270.23s)
--- PASS: TestAccAWSCodeBuildProject_Cache (319.79s)
--- PASS: TestAccAWSCodeBuildProject_WindowsContainer (243.82s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_GitSubmodulesConfig_CodeCommit (397.06s)
--- FAIL: TestAccAWSCodeBuildProject_VpcConfig (348.53s)

anGie44 · 2020-05-21T06:31:06Z

thank you @bflad! made the issue covering the 2 test failures here #13450. lmk if it should be split into 2 and/or details should be updated w/more context

ghost · 2020-05-29T00:59:49Z

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2020-06-27T17:11:03Z

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!

anGie44 added 2 commits May 19, 2020 21:11

use typelist for attributes with maxitems of 1 and remove DisableBina…

b9acd21

…ryDriver refs

change attribute type to handle update diffs

cefb059

ghost added size/XL Managed by automation to categorize the size of a PR. service/codebuild Issues and PRs that pertain to the codebuild service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels May 20, 2020

anGie44 changed the title ~~Ap typelist codebuild project~~ resource/codebuildproject: use typelist where maxitems are 1 May 20, 2020

anGie44 changed the title ~~resource/codebuildproject: use typelist where maxitems are 1~~ resource/codebuild: use typelist where maxitems are 1 May 20, 2020

anGie44 marked this pull request as ready for review May 20, 2020 02:26

anGie44 requested a review from a team May 20, 2020 02:26

anGie44 commented May 20, 2020

View reviewed changes

anGie44 changed the title ~~resource/codebuild: use typelist where maxitems are 1~~ service/codebuild: use typelist where maxitems are 1 May 20, 2020

bflad reviewed May 20, 2020

View reviewed changes

bflad self-assigned this May 20, 2020

bflad linked an issue May 20, 2020 that may be closed by this pull request

resource/aws_codebuild_project: Migrate Attributes From TypeSet to TypeList #6427

Closed

bflad reviewed May 20, 2020

View reviewed changes

aws/resource_aws_codebuild_project.go Show resolved Hide resolved

bflad added the bug Addresses a defect in current functionality. label May 20, 2020

bflad linked an issue May 20, 2020 that may be closed by this pull request

Error: Provider produced inconsistent final plan (aws_codebuild_project) #10615

Closed

anGie44 added 2 commits May 20, 2020 01:46

change auth attributes to typelist with maxitems of 1 and remove disa…

dd9af86

…blebinarydriver flag where applicable

update hash strings for secondary_sources

76c5578

anGie44 requested a review from bflad May 20, 2020 14:33

bflad approved these changes May 20, 2020

View reviewed changes

anGie44 merged commit 62fd223 into master May 28, 2020

anGie44 deleted the ap_typelist_codebuild_project branch May 28, 2020 17:00

anGie44 added a commit that referenced this pull request May 28, 2020

Update CHANGELOG for #13417

64fd8eb

anGie44 added this to the v2.64.0 milestone May 28, 2020

ghost locked and limited conversation to collaborators Jun 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

service/codebuild: use typelist where maxitems are 1 #13417

service/codebuild: use typelist where maxitems are 1 #13417

anGie44 commented May 20, 2020 •

edited

Loading

anGie44 May 20, 2020 •

edited

Loading

bflad May 20, 2020

bflad May 20, 2020

bflad May 20, 2020

bflad commented May 20, 2020

anGie44 commented May 20, 2020

bflad left a comment

anGie44 commented May 21, 2020

ghost commented May 29, 2020

ghost commented Jun 27, 2020

service/codebuild: use typelist where maxitems are 1 #13417

service/codebuild: use typelist where maxitems are 1 #13417

Conversation

anGie44 commented May 20, 2020 • edited Loading

Community Note

anGie44 May 20, 2020 • edited Loading

Choose a reason for hiding this comment

bflad May 20, 2020

Choose a reason for hiding this comment

bflad May 20, 2020

Choose a reason for hiding this comment

bflad May 20, 2020

Choose a reason for hiding this comment

bflad commented May 20, 2020

anGie44 commented May 20, 2020

bflad left a comment

Choose a reason for hiding this comment

anGie44 commented May 21, 2020

ghost commented May 29, 2020

ghost commented Jun 27, 2020

anGie44 commented May 20, 2020 •

edited

Loading

anGie44 May 20, 2020 •

edited

Loading