Use trail `arn` instead of `name` to get organization trails from a delegated account #30758

sbldevnet · 2023-04-16T10:40:39Z

Description

Getting the list of existing CloudTrail organization trails from a delegated account only works using the arn. This change will use trail arn instead of name as ID during creation to avoid that.
Updating docs basic example adding the recommended AWS S3 policy condition to avoid confused deputy issue.
Updating docs basic example adding depends_on to avoid race condition.

Relations

Closes #15553
Closes #27945
Closes #28440
Closes #30387

References

https://docs.aws.amazon.com/awscloudtrail/latest/userguide/create-s3-bucket-policy-for-cloudtrail.html

Output from Acceptance Testing

$ make testacc TESTS=TestAccCloudTrail_ PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20 -run='TestAccCloudTrail_'  -timeout 180m
=== RUN   TestAccCloudTrail_serial
=== PAUSE TestAccCloudTrail_serial
=== CONT  TestAccCloudTrail_serial
=== RUN   TestAccCloudTrail_serial/Trail
=== RUN   TestAccCloudTrail_serial/Trail/organization
    acctest.go:883: skipping tests; this AWS account must not be an existing member of an AWS Organization
=== RUN   TestAccCloudTrail_serial/Trail/eventSelectorDynamoDB
=== RUN   TestAccCloudTrail_serial/Trail/eventSelectorExclude
=== RUN   TestAccCloudTrail_serial/Trail/insightSelector
=== RUN   TestAccCloudTrail_serial/Trail/advancedEventSelector
=== RUN   TestAccCloudTrail_serial/Trail/disappears
=== RUN   TestAccCloudTrail_serial/Trail/multiRegion
=== RUN   TestAccCloudTrail_serial/Trail/logValidation
=== RUN   TestAccCloudTrail_serial/Trail/eventSelector
=== RUN   TestAccCloudTrail_serial/Trail/cloudwatch
=== RUN   TestAccCloudTrail_serial/Trail/kmsKey
=== RUN   TestAccCloudTrail_serial/Trail/tags
=== RUN   TestAccCloudTrail_serial/Trail/basic
=== RUN   TestAccCloudTrail_serial/Trail/enableLogging
=== RUN   TestAccCloudTrail_serial/Trail/globalServiceEvents
--- PASS: TestAccCloudTrail_serial (1353.81s)
    --- PASS: TestAccCloudTrail_serial/Trail (1353.81s)
        --- SKIP: TestAccCloudTrail_serial/Trail/organization (1.73s)
        --- PASS: TestAccCloudTrail_serial/Trail/eventSelectorDynamoDB (47.41s)
        --- PASS: TestAccCloudTrail_serial/Trail/eventSelectorExclude (123.43s)
        --- PASS: TestAccCloudTrail_serial/Trail/insightSelector (150.39s)
        --- PASS: TestAccCloudTrail_serial/Trail/advancedEventSelector (50.84s)
        --- PASS: TestAccCloudTrail_serial/Trail/disappears (45.74s)
        --- PASS: TestAccCloudTrail_serial/Trail/multiRegion (118.79s)
        --- PASS: TestAccCloudTrail_serial/Trail/logValidation (83.23s)
        --- PASS: TestAccCloudTrail_serial/Trail/eventSelector (172.92s)
        --- PASS: TestAccCloudTrail_serial/Trail/cloudwatch (111.31s)
        --- PASS: TestAccCloudTrail_serial/Trail/kmsKey (54.16s)
        --- PASS: TestAccCloudTrail_serial/Trail/tags (117.78s)
        --- PASS: TestAccCloudTrail_serial/Trail/basic (94.77s)
        --- PASS: TestAccCloudTrail_serial/Trail/enableLogging (130.06s)
        --- PASS: TestAccCloudTrail_serial/Trail/globalServiceEvents (51.24s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail	1358.005s
...

…delegated account

github-actions · 2023-04-16T10:40:51Z

Community Note

Voting for Prioritization

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

For Submitters

Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

github-actions

Welcome @sbldevnet 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

…abled

…ty issue

sherifkayad · 2023-04-25T15:48:15Z

Can we get this one merged soon? we are kinda blocked by it 😄

lostdignity · 2023-04-26T06:27:11Z

Can we get this one merged soon? we are kinda blocked by it 😄

same for us... I've built an workaround with Lambda but I'm longing for this fix.

ahembree · 2023-05-16T13:20:13Z

Any update on this?

Dgadavin · 2023-06-12T16:49:10Z

Can someone merge this. This is super usefull thing.

NaelMachal · 2023-06-22T08:59:25Z

Hello there, is this prioritised? There is currently no other way to deploy an organisation trail from a delegated admin account from terraform which is a very basic aws feature.

twelsh-aw · 2023-07-06T15:50:12Z

Bump. Share sentiment above that we need this

angelmaroco · 2023-07-11T16:04:39Z

Please, someone merge it!

twelsh-aw · 2023-07-19T12:59:45Z

@YakDriver sorry to ping you out of band, but can you please give this one a look. 🙇

A few simple fixes that address some important issues.

popsicleslayer · 2023-07-31T11:28:13Z

Please merge this 🥺 It's extremely annoying to work with as of now and it just blocks work. You can't create it with the resource and if you create it with the CLI it can't be imported

schroehe · 2023-08-07T12:58:24Z

Having the same issue. Upvoting for fix/PR

andredms · 2023-09-18T08:19:03Z

@justinretzolk do we have a timeline for getting this merged in? This is blocking our workflows and its not a change we'd like to revert.

popsicleslayer · 2023-10-18T13:35:15Z

A month later I would like to ask the same question as @andredms: @justinretzolk is there a timeline for merging this? I have been stuck with this problem since July and am constantly having faith in this being merged.

rapides · 2023-11-02T17:09:18Z

Bump! Also upvoting for this! ✋🏻

twelsh-aw · 2023-11-02T19:13:20Z

@ewbankkit It seems like you are mostly maintaining this repo now. Can you give a quick review?

ewbankkit

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccCloudTrail_serial/Trail' PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20  -run=TestAccCloudTrail_serial/Trail -timeout 360m
=== RUN   TestAccCloudTrail_serial
=== PAUSE TestAccCloudTrail_serial
=== CONT  TestAccCloudTrail_serial
=== RUN   TestAccCloudTrail_serial/Trail
=== RUN   TestAccCloudTrail_serial/Trail/kmsKey
=== RUN   TestAccCloudTrail_serial/Trail/tags
=== RUN   TestAccCloudTrail_serial/Trail/eventSelector
=== RUN   TestAccCloudTrail_serial/Trail/eventSelectorExclude
=== RUN   TestAccCloudTrail_serial/Trail/disappears
=== RUN   TestAccCloudTrail_serial/Trail/cloudwatch
=== RUN   TestAccCloudTrail_serial/Trail/globalServiceEvents
=== RUN   TestAccCloudTrail_serial/Trail/organization
    acctest.go:989: this AWS account must be the management account of an AWS Organization
=== RUN   TestAccCloudTrail_serial/Trail/migrateV0
=== RUN   TestAccCloudTrail_serial/Trail/basic
=== RUN   TestAccCloudTrail_serial/Trail/logValidation
=== RUN   TestAccCloudTrail_serial/Trail/enableLogging
    acctest.go:1475: skipping test for aws/us-west-2: Error running apply: exit status 1
        
        Error: stopping CloudTrail Trail (arn:aws:cloudtrail:us-west-2:123456789012:trail/tf-acc-test-1022396321005908609) logging: AccessDeniedException: User: arn:aws:sts::123456789012:assumed-role/terraform_team1_dev-developer/[email protected] is not authorized to perform: cloudtrail:StopLogging on resource: arn:aws:cloudtrail:us-west-2:123456789012:trail/tf-acc-test-1022396321005908609 with an explicit deny in a service control policy
                status code: 400, request id: d13ddd18-ea14-424d-8d6f-c23c0e2fcc7d
        
          with aws_cloudtrail.test,
          on terraform_plugin_test.tf line 51, in resource "aws_cloudtrail" "test":
          51: resource "aws_cloudtrail" "test" {
        
=== RUN   TestAccCloudTrail_serial/Trail/insightSelector
=== RUN   TestAccCloudTrail_serial/Trail/multiRegion
=== RUN   TestAccCloudTrail_serial/Trail/eventSelectorDynamoDB
=== RUN   TestAccCloudTrail_serial/Trail/advancedEventSelector
--- PASS: TestAccCloudTrail_serial (1116.89s)
    --- PASS: TestAccCloudTrail_serial/Trail (1116.89s)
        --- PASS: TestAccCloudTrail_serial/Trail/kmsKey (49.37s)
        --- PASS: TestAccCloudTrail_serial/Trail/tags (84.98s)
        --- PASS: TestAccCloudTrail_serial/Trail/eventSelector (123.85s)
        --- PASS: TestAccCloudTrail_serial/Trail/eventSelectorExclude (88.77s)
        --- PASS: TestAccCloudTrail_serial/Trail/disappears (45.66s)
        --- PASS: TestAccCloudTrail_serial/Trail/cloudwatch (84.26s)
        --- PASS: TestAccCloudTrail_serial/Trail/globalServiceEvents (46.91s)
        --- SKIP: TestAccCloudTrail_serial/Trail/organization (0.34s)
        --- PASS: TestAccCloudTrail_serial/Trail/migrateV0 (109.15s)
        --- PASS: TestAccCloudTrail_serial/Trail/basic (72.46s)
        --- PASS: TestAccCloudTrail_serial/Trail/logValidation (67.26s)
        --- SKIP: TestAccCloudTrail_serial/Trail/enableLogging (56.25s)
        --- PASS: TestAccCloudTrail_serial/Trail/insightSelector (106.26s)
        --- PASS: TestAccCloudTrail_serial/Trail/multiRegion (86.43s)
        --- PASS: TestAccCloudTrail_serial/Trail/eventSelectorDynamoDB (47.88s)
        --- PASS: TestAccCloudTrail_serial/Trail/advancedEventSelector (47.05s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail 1122.001s

% make testacc TESTARGS='-run=TestAccCloudTrail_serial/Trail/organization' PKG=cloudtrail
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudtrail/... -v -count 1 -parallel 20  -run=TestAccCloudTrail_serial/Trail/organization -timeout 360m
=== RUN   TestAccCloudTrail_serial
=== PAUSE TestAccCloudTrail_serial
=== CONT  TestAccCloudTrail_serial
=== RUN   TestAccCloudTrail_serial/Trail
=== RUN   TestAccCloudTrail_serial/Trail/organization
--- PASS: TestAccCloudTrail_serial (70.63s)
    --- PASS: TestAccCloudTrail_serial/Trail (70.63s)
        --- PASS: TestAccCloudTrail_serial/Trail/organization (70.63s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/cloudtrail 75.943s

ewbankkit · 2023-11-06T15:18:40Z

@sbldevnet Thanks for the contribution 🎉 👏.
We went ahead and changed the id to always be the trail ARN which simplified the changes somewhat.

github-actions · 2023-11-10T07:15:47Z

This functionality has been released in v5.25.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 · 2023-12-11T02:10:01Z

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.

fix: use trail arn instead of name to get organization trails from a …

36ad1ac

…delegated account

add changelog

c791d08

github-actions bot reviewed Apr 16, 2023

View reviewed changes

sbldevnet added 3 commits April 18, 2023 10:59

fix: restrict S3 policy in tests to avoid error if the account has en…

2f074b7

…abled

docs: update basic example iam policy to avoid confused deputy securi…

be71d06

…ty issue

docs: adding in basic example to avoid race condition

e115718

github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels Apr 18, 2023

sbldevnet marked this pull request as ready for review April 18, 2023 09:03

justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Apr 25, 2023

Merge branch 'main' into b-aws_cloudtrail-fix-delegated-trail

1aecf9d

github-actions bot added size/S Managed by automation to categorize the size of a PR. and removed tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/M Managed by automation to categorize the size of a PR. labels Jun 9, 2023

Merge branch 'main' into b-aws_cloudtrail-fix-delegated-trail

dd00eea

Merge branch 'main' into b-aws_cloudtrail-fix-delegated-trail

58530ec

jake-naughton approved these changes Jul 31, 2023

View reviewed changes

zahorniak approved these changes Aug 14, 2023

View reviewed changes

justinretzolk added the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Aug 18, 2023

ewbankkit self-assigned this Nov 3, 2023

ewbankkit added 10 commits November 3, 2023 14:42

Merge branch 'main' into HEAD

228b91c

r/aws_cloudtrail: Tidy up Create.

3dca691

r/aws_cloudtrail: Tidy up Delete.

beaa29d

r/aws_cloudtrail: Tidy up Update.

da4032a

r/aws_cloudtrail: Tidy up Read.

813b75b

r/aws_cloudtrail: Simplify findTrail(s).

76f279a

r/aws_cloudtrail: Document that 'id changes from trail name to its ARN.

7c0347b

r/aws_cloudtrail: Add StateUpgrader for 'id'.

f8a24a0

r/aws_cloudtrail: Tidy up acceptance tests.

f268cf1

Fix semgrep 'ci.cloudtrail-in-func-name '.

61d3bb6

ewbankkit approved these changes Nov 6, 2023

View reviewed changes

ewbankkit merged commit 2485513 into hashicorp:main Nov 6, 2023
43 checks passed

github-actions bot added this to the v5.25.0 milestone Nov 6, 2023

github-actions bot removed the bug Addresses a defect in current functionality. label Nov 10, 2023

github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2023

justinretzolk added the bug Addresses a defect in current functionality. label Feb 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use trail `arn` instead of `name` to get organization trails from a delegated account #30758

Use trail `arn` instead of `name` to get organization trails from a delegated account #30758

sbldevnet commented Apr 16, 2023 •

edited

Loading

github-actions bot commented Apr 16, 2023

github-actions bot left a comment

sherifkayad commented Apr 25, 2023

lostdignity commented Apr 26, 2023

ahembree commented May 16, 2023

Dgadavin commented Jun 12, 2023

NaelMachal commented Jun 22, 2023

twelsh-aw commented Jul 6, 2023

angelmaroco commented Jul 11, 2023

twelsh-aw commented Jul 19, 2023

popsicleslayer commented Jul 31, 2023

schroehe commented Aug 7, 2023

andredms commented Sep 18, 2023

popsicleslayer commented Oct 18, 2023

rapides commented Nov 2, 2023

twelsh-aw commented Nov 2, 2023

ewbankkit left a comment

ewbankkit commented Nov 6, 2023

github-actions bot commented Nov 10, 2023

github-actions bot commented Dec 11, 2023

Use trail arn instead of name to get organization trails from a delegated account #30758

Use trail arn instead of name to get organization trails from a delegated account #30758

Conversation

sbldevnet commented Apr 16, 2023 • edited Loading

Description

Relations

References

Output from Acceptance Testing

github-actions bot commented Apr 16, 2023

Community Note

github-actions bot left a comment

Choose a reason for hiding this comment

sherifkayad commented Apr 25, 2023

lostdignity commented Apr 26, 2023

ahembree commented May 16, 2023

Dgadavin commented Jun 12, 2023

NaelMachal commented Jun 22, 2023

twelsh-aw commented Jul 6, 2023

angelmaroco commented Jul 11, 2023

twelsh-aw commented Jul 19, 2023

popsicleslayer commented Jul 31, 2023

schroehe commented Aug 7, 2023

andredms commented Sep 18, 2023

popsicleslayer commented Oct 18, 2023

rapides commented Nov 2, 2023

twelsh-aw commented Nov 2, 2023

ewbankkit left a comment

Choose a reason for hiding this comment

ewbankkit commented Nov 6, 2023

github-actions bot commented Nov 10, 2023

github-actions bot commented Dec 11, 2023

Use trail `arn` instead of `name` to get organization trails from a delegated account #30758

Use trail `arn` instead of `name` to get organization trails from a delegated account #30758

sbldevnet commented Apr 16, 2023 •

edited

Loading