Skip to content

r/aws_codepipeline: support removal of trigger arguments #42008

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.

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

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

jar-b
Copy link
Member

@jar-b jar-b commented Mar 27, 2025

Description

Previously the trigger argument was optional and computed, meaning that removal of a configured trigger definition would not result in planned changes as the value would be interpreted as computed only. This change switches the trigger argument back to optional only, and adds a computed trigger_all block which is used to track both custom trigger configurations provided by the user and default configurations added by AWS for V2 pipelines. The downside of this approach is that drift between the remote and configured trigger definition can no longer be detected. This was deemed an acceptable trade-off in order to ensure trigger removals operate as intended without manual intervention.

Relations

Closes #39347

References

Output from Acceptance Testing

% make testacc PKG=codepipeline TESTS="TestAccCodePipeline_"
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.7 test ./internal/service/codepipeline/... -v -count 1 -parallel 20 -run='TestAccCodePipeline_'  -timeout 360m -vet=off
2025/03/27 11:28:59 Initializing Terraform AWS Provider...

--- PASS: TestAccCodePipeline_disappears (36.97s)
--- PASS: TestAccCodePipeline_withNamespace (37.27s)
--- PASS: TestAccCodePipeline_deployWithServiceRole (42.22s)
--- PASS: TestAccCodePipeline_emptyStageArtifacts (42.87s)
--- PASS: TestAccCodePipeline_MultiRegion_basic (46.63s)
--- PASS: TestAccCodePipeline_trigger (50.36s)
--- PASS: TestAccCodePipeline_manualApprovalTimeoutInMinutes (59.18s)
--- PASS: TestAccCodePipeline_conditions (59.98s)
--- PASS: TestAccCodePipeline_basic (60.17s)
--- PASS: TestAccCodePipeline_MultiRegion_update (64.77s)
--- PASS: TestAccCodePipeline_ecr (71.32s)
--- PASS: TestAccCodePipeline_tags (77.72s)
--- PASS: TestAccCodePipeline_MultiRegion_convertSingleRegion (79.22s)
--- PASS: TestAccCodePipeline_pipelineType (100.41s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/codepipeline       107.207s

Copy link

Community Guidelines

This comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀

Voting for Prioritization

  • Please vote on this Pull Request by adding a 👍 reaction to the original post to help the community and maintainers prioritize it.
  • Please see our prioritization guide for additional information on how the maintainers handle prioritization.
  • Please do not leave +1 or other comments that do not add relevant new information or questions; they generate extra noise for others following the Pull Request and do not help prioritize the request.

Pull Request Authors

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/codepipeline Issues and PRs that pertain to the codepipeline service. size/XL Managed by automation to categorize the size of a PR. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. labels Mar 27, 2025
@jar-b jar-b force-pushed the f-codebuild-triggers-rm branch from 370cb1c to fa188ba Compare March 27, 2025 16:42
Previously the `trigger` argument was optional and computed, meaning that removal of a configured trigger definition would not result in planned changes as the value would be interpreted as computed only. This change switches the `trigger` argument back to optional only, and adds a computed `trigger_all` block which is used to track both custom trigger configurations provided by the user and default configurations added by AWS for `V2` pipelines. The downside of this approach is that drift between the remote and configured `trigger` definition can no longer be detected. This was deemed an acceptable trade-off in order to ensure trigger removals operate as intended without manual intervention.

```console
% make testacc PKG=codepipeline TESTS="TestAccCodePipeline_"
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.7 test ./internal/service/codepipeline/... -v -count 1 -parallel 20 -run='TestAccCodePipeline_'  -timeout 360m -vet=off
2025/03/27 11:28:59 Initializing Terraform AWS Provider...

--- PASS: TestAccCodePipeline_disappears (36.97s)
--- PASS: TestAccCodePipeline_withNamespace (37.27s)
--- PASS: TestAccCodePipeline_deployWithServiceRole (42.22s)
--- PASS: TestAccCodePipeline_emptyStageArtifacts (42.87s)
--- PASS: TestAccCodePipeline_MultiRegion_basic (46.63s)
--- PASS: TestAccCodePipeline_trigger (50.36s)
--- PASS: TestAccCodePipeline_manualApprovalTimeoutInMinutes (59.18s)
--- PASS: TestAccCodePipeline_conditions (59.98s)
--- PASS: TestAccCodePipeline_basic (60.17s)
--- PASS: TestAccCodePipeline_MultiRegion_update (64.77s)
--- PASS: TestAccCodePipeline_ecr (71.32s)
--- PASS: TestAccCodePipeline_tags (77.72s)
--- PASS: TestAccCodePipeline_MultiRegion_convertSingleRegion (79.22s)
--- PASS: TestAccCodePipeline_pipelineType (100.41s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/codepipeline       107.207s
```
@jar-b jar-b force-pushed the f-codebuild-triggers-rm branch from fa188ba to 8074223 Compare March 27, 2025 16:58
@jar-b jar-b marked this pull request as ready for review March 27, 2025 17:28
@jar-b jar-b requested a review from a team as a code owner March 27, 2025 17:28
},
},
"trigger": triggerSchema(),
"trigger_all": sdkv2.DataSourcePropertyFromResourceProperty(triggerSchema()),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably rename DataSourcePropertyFromResourceProperty to capture the fact that it can be used outside of data sources to make al;l attributes Computed only.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I'll open a tech debt issue for follow up.

Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccCodePipeline_' PKG=codepipeline ACCTEST_PARALLELISM=3
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.7 test ./internal/service/codepipeline/... -v -count 1 -parallel 3  -run=TestAccCodePipeline_ -timeout 360m -vet=off
2025/03/27 13:43:01 Initializing Terraform AWS Provider...
=== RUN   TestAccCodePipeline_basic
=== PAUSE TestAccCodePipeline_basic
=== RUN   TestAccCodePipeline_disappears
=== PAUSE TestAccCodePipeline_disappears
=== RUN   TestAccCodePipeline_emptyStageArtifacts
=== PAUSE TestAccCodePipeline_emptyStageArtifacts
=== RUN   TestAccCodePipeline_deployWithServiceRole
=== PAUSE TestAccCodePipeline_deployWithServiceRole
=== RUN   TestAccCodePipeline_tags
=== PAUSE TestAccCodePipeline_tags
=== RUN   TestAccCodePipeline_MultiRegion_basic
=== PAUSE TestAccCodePipeline_MultiRegion_basic
=== RUN   TestAccCodePipeline_MultiRegion_update
=== PAUSE TestAccCodePipeline_MultiRegion_update
=== RUN   TestAccCodePipeline_MultiRegion_convertSingleRegion
=== PAUSE TestAccCodePipeline_MultiRegion_convertSingleRegion
=== RUN   TestAccCodePipeline_withNamespace
=== PAUSE TestAccCodePipeline_withNamespace
=== RUN   TestAccCodePipeline_withGitHubV1SourceAction
    codepipeline_test.go:480: Environment variable GITHUB_TOKEN is not set, skipping test
--- SKIP: TestAccCodePipeline_withGitHubV1SourceAction (0.00s)
=== RUN   TestAccCodePipeline_ecr
=== PAUSE TestAccCodePipeline_ecr
=== RUN   TestAccCodePipeline_pipelineType
=== PAUSE TestAccCodePipeline_pipelineType
=== RUN   TestAccCodePipeline_manualApprovalTimeoutInMinutes
=== PAUSE TestAccCodePipeline_manualApprovalTimeoutInMinutes
=== RUN   TestAccCodePipeline_conditions
=== PAUSE TestAccCodePipeline_conditions
=== RUN   TestAccCodePipeline_trigger
=== PAUSE TestAccCodePipeline_trigger
=== CONT  TestAccCodePipeline_basic
=== CONT  TestAccCodePipeline_MultiRegion_convertSingleRegion
=== CONT  TestAccCodePipeline_tags
--- PASS: TestAccCodePipeline_basic (43.68s)
=== CONT  TestAccCodePipeline_manualApprovalTimeoutInMinutes
--- PASS: TestAccCodePipeline_tags (50.69s)
=== CONT  TestAccCodePipeline_trigger
--- PASS: TestAccCodePipeline_MultiRegion_convertSingleRegion (64.30s)
=== CONT  TestAccCodePipeline_conditions
--- PASS: TestAccCodePipeline_manualApprovalTimeoutInMinutes (37.74s)
=== CONT  TestAccCodePipeline_MultiRegion_update
--- PASS: TestAccCodePipeline_trigger (39.23s)
=== CONT  TestAccCodePipeline_ecr
--- PASS: TestAccCodePipeline_conditions (38.72s)
=== CONT  TestAccCodePipeline_pipelineType
--- PASS: TestAccCodePipeline_ecr (23.04s)
=== CONT  TestAccCodePipeline_withNamespace
--- PASS: TestAccCodePipeline_MultiRegion_update (45.88s)
=== CONT  TestAccCodePipeline_emptyStageArtifacts
--- PASS: TestAccCodePipeline_withNamespace (26.69s)
=== CONT  TestAccCodePipeline_deployWithServiceRole
--- PASS: TestAccCodePipeline_emptyStageArtifacts (23.82s)
=== CONT  TestAccCodePipeline_disappears
--- PASS: TestAccCodePipeline_deployWithServiceRole (26.88s)
=== CONT  TestAccCodePipeline_MultiRegion_basic
--- PASS: TestAccCodePipeline_disappears (20.40s)
--- PASS: TestAccCodePipeline_pipelineType (87.16s)
--- PASS: TestAccCodePipeline_MultiRegion_basic (33.76s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/codepipeline	205.848s

@jar-b jar-b merged commit 8d6a160 into main Mar 27, 2025
46 checks passed
@jar-b jar-b deleted the f-codebuild-triggers-rm branch March 27, 2025 18:00
Copy link

Warning

This Issue has been closed, meaning that any additional comments are much easier for the maintainers to miss. Please assume that the maintainers will not see them.

Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.

Copy link

This functionality has been released in v5.93.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. Thank you!

@github-actions github-actions bot removed the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Mar 27, 2025
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/codepipeline Issues and PRs that pertain to the codepipeline service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: No changes detected when adding/removing aws_codepipeline trigger
2 participants