Fix secondary_artifacts hash function for aws_codebuild_project

[jckuester]

Relates to #6643

This PR fixes the hash function for secondary_artifacts of the aws_codebuild_project resource. The symptom was that changes weren't detected by Terraform when changing some of secondary_artifacts's attributes.

Unfortunately, I found no good way yet to correctly detect and apply updates for the following attributes: name, namespace_type, and packaging. The reason is that those three attributes (if not set explicitly through Terraform configuration) are set by AWS internally and cause the hash value of secondary_artifacts to change after the apply. The tests complain that their is some unapplied diff in the plan.

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCodeBuildProject'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSCodeBuildProject -timeout 120m
...
--- SKIP: TestAccAWSCodeBuildProject_SecondaryArtifacts_Name (0.00s)
    resource_aws_codebuild_project_test.go:1418: Currently no solution to allow updates on name attribute
...
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSourceInvalid (20.68s)
=== CONT  TestAccAWSCodeBuildProject_Source_Type_NoSource
--- PASS: TestAccAWSCodeBuildProject_Source_Type_Bitbucket (37.90s)
=== CONT  TestAccAWSCodeBuildProject_Source_Type_S3
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_CodeCommit (39.46s)
=== CONT  TestAccAWSCodeBuildProject_Source_Type_GitHubEnterprise
--- PASS: TestAccAWSCodeBuildProject_importBasic (40.21s)
=== CONT  TestAccAWSCodeBuildProject_Source_Type_CodePipeline
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts (41.89s)
=== CONT  TestAccAWSCodeBuildProject_Source_Type_CodeCommit
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Type (42.38s)
=== CONT  TestAccAWSCodeBuildProject_Artifacts_Location
--- PASS: TestAccAWSCodeBuildProject_Tags (51.68s)
=== CONT  TestAccAWSCodeBuildProject_Source_InsecureSSL
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSource (36.49s)
=== CONT  TestAccAWSCodeBuildProject_Artifacts_EncryptionDisabled
--- PASS: TestAccAWSCodeBuildProject_Environment_RegistryCredential (58.28s)
=== CONT  TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHubEnterprise
--- PASS: TestAccAWSCodeBuildProject_Artifacts_NamespaceType (60.80s)
=== CONT  TestAccAWSCodeBuildProject_Artifacts_ArtifactIdentifier
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Path (61.32s)
=== CONT  TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHub
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Type (61.47s)
=== CONT  TestAccAWSCodeBuildProject_WindowsContainer
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Path (61.54s)
=== CONT  TestAccAWSCodeBuildProject_VpcConfig
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_ArtifactIdentifier (61.78s)
=== CONT  TestAccAWSCodeBuildProject_Environment_Certificate
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Name (61.80s)
=== CONT  TestAccAWSCodeBuildProject_Source_Auth
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Packaging (61.95s)
=== CONT  TestAccAWSCodeBuildProject_LogsConfig_S3Logs
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_NamespaceType (61.97s)
=== CONT  TestAccAWSCodeBuildProject_Source_GitCloneDepth
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_EncryptionDisabled (62.00s)
=== CONT  TestAccAWSCodeBuildProject_LogsConfig_CloudWatchLogs
--- PASS: TestAccAWSCodeBuildProject_Artifacts_OverrideArtifactName (62.00s)
=== CONT  TestAccAWSCodeBuildProject_Description
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_OverrideArtifactName (62.65s)
=== CONT  TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Type
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Location (65.90s)
=== CONT  TestAccAWSCodeBuildProject_Environment_EnvironmentVariable
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodePipeline (28.37s)
=== CONT  TestAccAWSCodeBuildProject_EncryptionKey
--- PASS: TestAccAWSCodeBuildProject_Source_Type_S3 (35.88s)
=== CONT  TestAccAWSCodeBuildProject_BuildTimeout
--- FAIL: TestAccAWSCodeBuildProject_Source_Auth (13.61s)
    testing.go:568: Step 1 error: errors during apply:
        
        Error: Error creating CodeBuild project: InvalidInputException: No Access token found, please visit AWS CodeBuild console to connect to GitHub
        	status code: 400, request id: d9e431a0-4172-47d6-97f1-c7d627f5a8be
        
          on /tmp/tf-test886635756/main.tf line 64:
          (source code not available)
        
        
=== CONT  TestAccAWSCodeBuildProject_Source_ReportBuildStatus_Bitbucket
--- PASS: TestAccAWSCodeBuildProject_Source_Type_GitHubEnterprise (36.64s)
=== CONT  TestAccAWSCodeBuildProject_Cache
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodeCommit (37.10s)
=== CONT  TestAccAWSCodeBuildProject_BadgeEnabled
--- PASS: TestAccAWSCodeBuildProject_WindowsContainer (36.79s)
=== CONT  TestAccAWSCodeBuildProject_SecondaryArtifacts_Packaging
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHubEnterprise (41.67s)
=== CONT  TestAccAWSCodeBuildProject_basic
--- PASS: TestAccAWSCodeBuildProject_Environment_Certificate (38.86s)
--- PASS: TestAccAWSCodeBuildProject_Source_InsecureSSL (50.37s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Location (61.54s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHub (49.91s)
--- PASS: TestAccAWSCodeBuildProject_Description (50.23s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitCloneDepth (50.45s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Type (49.91s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_EncryptionDisabled (56.54s)
--- PASS: TestAccAWSCodeBuildProject_BuildTimeout (41.60s)
--- PASS: TestAccAWSCodeBuildProject_BadgeEnabled (36.59s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_Bitbucket (41.45s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_ArtifactIdentifier (58.11s)
--- PASS: TestAccAWSCodeBuildProject_EncryptionKey (51.02s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_CloudWatchLogs (65.57s)
--- PASS: TestAccAWSCodeBuildProject_VpcConfig (71.60s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable (69.16s)
--- PASS: TestAccAWSCodeBuildProject_basic (38.72s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_S3Logs (81.69s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Packaging (59.46s)
--- PASS: TestAccAWSCodeBuildProject_Cache (106.52s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	182.697s
...

[bflad]

Thanks for taking this on, @jckuester! If we get this to the right spot, we should add Fixes #6643 to the PR description so it properly references the open issue to close it.

Unfortunately, I found no good way yet to correctly detect and apply updates for the following attributes: name, namespace_type, and packaging. The reason is that those three attributes (if not set explicitly through Terraform configuration) are set by AWS internally and cause the hash value of secondary_artifacts to change after the apply.

While glancing through your changes, it appears S3 is the only valid type for secondary artifacts? If so, is it safe to add Default: codebuild.ArtifactNamespaceNone for namespace_type and Default: codebuild.ArtifactPackagingNone for packaging? This should keep the hash function setup correctly for those two attributes. 🤞

For name, we can pass the CodeBuild project name into the hash function as the default value for hashing, then only use m["name"] to override that if it is configured. That should workaround that issue.

[jckuester]

While glancing through your changes, it appears S3 is the only valid type for secondary artifacts? If so, is it safe to add Default: codebuild.ArtifactNamespaceNone for namespace_type and Default: codebuild.ArtifactPackagingNone for packaging? This should keep the hash function setup correctly for those two attributes. crossed_fingers

Good idea, that works!

For name, we can pass the CodeBuild project name into the hash function as the default value for hashing, then only use m["name"] to override that if it is configured. That should workaround that issue.

I am not sure how I can access/pass the CodeBuild project name into the hash function. How do I get access to d *schema.ResourceData to do a d.Get("name") in the schema?

[bflad]

I am not sure how I can access/pass the CodeBuild project name into the hash function. How do I get access to d *schema.ResourceData to do a d.Get("name") in the schema?

Arg! You are right. The set function only receives an interface{} containing the map of the current attribute. I cannot immediately think of a good answer for this problem.

[jckuester]

If you have a good idea, let me know :-)

[bflad]

@jckuester if you could push up what you have, we can at least fix the other secondary_artifacts arguments 👍

Our options are fairly limited for the name situation with the current Terraform Provider SDK, it only supports a few things, which cannot work for us in all situations:

• Default - which we cannot use because its dynamically based on another value in the schema
• DefaultFunc - similar issue and it doesn't have any additional insight into the schema
• CustomizeDiff - where we dynamically update the value during the final difference phase, however if the project name is a computed value from another resource in anyway, this approach cannot work either

We might need to mark the name argument as Required in a future major version of the provider to work around these limitations. 🙁 In a future version of the Terraform Provider SDK that is compatible with only Terraform 0.12 and later, we should have support for a "map" configuration block of sorts:

# Future Terraform functionality -- not yet implemented
resource "aws_codebuild_project" "example" {
  # ... other configuration ...

  source_artifact "name1" {
    # ... other configuration ...
  }
}

However that is much farther away.

[jckuester]

@bflad pushed my changes and skipped the test for the name attribute with a comment for now.

There is a linter error, but I am not aware that I touched any related code:

aws/config.go:631:6: `parseDNSSuffixFromEndpoint` is unused (deadcode)
func parseDNSSuffixFromEndpoint(endpoint string, region string) (*string, error) {
     ^

Do you want me to remove that func?

[bflad]

Hi @jckuester 👋 Thank you very much for this submission and all the extra testing. I think we are currently at a block for fixing up the secondary_artifacts configuration block name handling since we won't be able to correctly get the default in there, however the rest of the changes here are excellent so going to merge this pull request in its current form so we can get all those fixes in. We can discuss ideas for name further in #6643 🚀

--- PASS: TestAccAWSCodeBuildProject_basic (35.04s)
--- PASS: TestAccAWSCodeBuildProject_BadgeEnabled (35.79s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodeCommit (36.85s)
--- PASS: TestAccAWSCodeBuildProject_importBasic (38.37s)
--- PASS: TestAccAWSCodeBuildProject_Environment_Certificate (38.14s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_Bitbucket (38.54s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_CodePipeline (28.55s)
--- PASS: TestAccAWSCodeBuildProject_Source_InsecureSSL (42.12s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_Bitbucket (44.24s)
--- PASS: TestAccAWSCodeBuildProject_Source_GitCloneDepth (46.56s)
--- PASS: TestAccAWSCodeBuildProject_BuildTimeout (47.35s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHub (47.89s)
--- PASS: TestAccAWSCodeBuildProject_Source_ReportBuildStatus_GitHubEnterprise (48.17s)
--- PASS: TestAccAWSCodeBuildProject_Description (48.45s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable_Type (48.45s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSourceInvalid (14.37s)
--- PASS: TestAccAWSCodeBuildProject_EncryptionKey (55.02s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_CloudWatchLogs (56.54s)
--- PASS: TestAccAWSCodeBuildProject_LogsConfig_S3Logs (57.35s)
--- PASS: TestAccAWSCodeBuildProject_Environment_EnvironmentVariable (57.30s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_S3 (23.13s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_GitHubEnterprise (27.86s)
--- SKIP: TestAccAWSCodeBuildProject_SecondaryArtifacts_Name (0.00s)
--- PASS: TestAccAWSCodeBuildProject_Source_Type_NoSource (28.28s)
--- PASS: TestAccAWSCodeBuildProject_WindowsContainer (29.13s)
--- PASS: TestAccAWSCodeBuildProject_Tags (38.30s)
--- PASS: TestAccAWSCodeBuildProject_Cache (80.17s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_ArtifactIdentifier (40.98s)
--- PASS: TestAWSCodeBuildProject_nameValidation (0.00s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_EncryptionDisabled (39.73s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts (31.67s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Name (40.51s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Location (42.65s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_NamespaceType (41.46s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Path (41.82s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_OverrideArtifactName (42.45s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Packaging (42.64s)
--- PASS: TestAccAWSCodeBuildProject_VpcConfig (52.26s)
--- PASS: TestAccAWSCodeBuildProject_Artifacts_Type (40.22s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_ArtifactIdentifier (39.63s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_OverrideArtifactName (39.58s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_EncryptionDisabled (40.84s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Location (40.07s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_NamespaceType (37.83s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Packaging (36.39s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Path (35.54s)
--- PASS: TestAccAWSCodeBuildProject_SecondaryArtifacts_Type (30.38s)
--- PASS: TestAccAWSCodeBuildProject_SecondarySources_CodeCommit (28.84s)
--- PASS: TestAccAWSCodeBuildProject_Environment_RegistryCredential (36.23s)

[ghost]

This has been released in version 2.24.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!

[JonathanReiss15]

While glancing through your changes, it appears S3 is the only valid type for secondary artifacts?

This is incorrect. According to both the Terraform documentation and AWS's CodeBuild documentation, the following types are valid for a secondary_artifact:

• "NO_ARTIFACT"
• "S3"
• "CODEPIPELINE"

This Pull Request breaks adding a secondary_artifact with type="NO_ARTIFACT" | "CODEPIPELINE"

[jckuester]

@JonathanReiss15 @bflad I added two temporary tests (#10198) to show, why I assumed S3 is the only allowed artifact type for secondary artifacts: . I got this error from the AWS API:

  Error: Error creating CodeBuild project: InvalidInputException: Invalid input: artifactType CODEPIPELINE is not allowed for secondaryArtifacts
        	status code: 400, request id: 37d70abf-ef59-4c93-bbdd-7887fea18761

and

        Error: Error creating CodeBuild project: InvalidInputException: Invalid input: artifactType NO_ARTIFACTS is not allowed for secondaryArtifacts
        	status code: 400, request id: eda4699b-2ddc-4483-90aa-8129be3d8aa2

[ghost]

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!