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.

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

(cloudtrail): (configuration for IsOrganizationTrail is failed) #21578

Closed
2 tasks
haikoschmidt opened this issue Aug 12, 2022 · 6 comments · Fixed by #21625
Closed
2 tasks

(cloudtrail): (configuration for IsOrganizationTrail is failed) #21578

haikoschmidt opened this issue Aug 12, 2022 · 6 comments · Fixed by #21625
Labels
@aws-cdk/aws-cloudtrail Related to AWS CloudTrail effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@haikoschmidt
Copy link

haikoschmidt commented Aug 12, 2022

Describe the feature

I miss the fact that I can use boolean to specify an OrganizationTrail as in CloudFormation.

https://docs.aws.amazon.com/de_de/AWSCloudFormation/latest/UserGuide/aws-resource-cloudtrail-trail.html#cfn-cloudtrail-trail-isorganizationtrail

Use Case

I am currently converting my payer configuration to CDK, at least what is available at all, which unfortunately is still very little at the moment. Using CloudFormation, I can specify whether a trail is an organization trail and whether it is then automatically rolled out throughout the organization.
Unfortunately, this is not yet possible with CDK.

Proposed Solution

Give an option that you can configure this using boolean, the default should be "false".

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.37.1

Environment details (OS name and version, etc.)

none

@haikoschmidt haikoschmidt added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2022
@github-actions github-actions bot added the @aws-cdk/aws-cloudtrail Related to AWS CloudTrail label Aug 12, 2022
@laurelmay
Copy link
Contributor

As a workaround, you should be able to use an escape hatch to override the property:

import * as cloudtrail from "aws-cdk-lib/aws-cloudtrail";

// Create the L2 Trail resource with the preferred configuration
const trail = new cloudtrail.Trail(stack, "OrganizationTrail");
// Get the underlying AWS::CloudTrail::Trail L1 resource
const l1Trail = trail.node.defaultChild as cloudtrail.CfnTrail;
// Use the escape hatch to set the L1 AWS::CloudTrail::Trail resource's property
l1Trail.addPropertyOverride("IsOrganizationTrail", true);

@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2022
@peterwoodworth
Copy link
Contributor

Thanks for the feature request @haikoschmidt,

I am marking this issue as p2, which means that we are unable to work on this immediately.

Check out our contributing guide if you're interested in contributing yourself - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

Link to code where we declare the CfnTrail

const trail = new CfnTrail(this, 'Resource', {

@peterwoodworth peterwoodworth added the good first issue Related to contributions. See CONTRIBUTING.md label Aug 12, 2022
@chinmayv2
Copy link

chinmayv2 commented Aug 14, 2022

Hello All,
I was going through the chain since I am interested to contribute to the cdk. I understood the suggestion/ proposed solution is

Give an option that you can configure this using boolean, the default should be "false".

Excerpts from this link attached in the bug report.

If the trail is an organization trail and this is set to false, the trail will remain in the current AWS account but be deleted from all member accounts in the organization.

1- Do we expect any unintended side-effects or trails being un-intentionally deleted if we default it to false ?
2- Should we have the default to true ?

@peterwoodworth
Copy link
Contributor

Hey @chinmayv2,

The CloudFormation documentation mentions that this prop is set to false by default, so that's what we'd want to do here as well. Someone would have had to explicitly set this to true for the behavior to change, so if we keep the default as false or undefined then that will not affect existing users.

@TheRealAmazonKendra
Copy link
Contributor

It may be worth it to point to to CloudFormation that the documentation there is pretty confusing/misleading. I had to re-read it a few times to get what the actual behavior was.

@mergify mergify bot closed this as completed in #21625 Aug 17, 2022
mergify bot pushed a commit that referenced this issue Aug 17, 2022
Fixes #21578

Please add `pr-linter/exempt-readme` label since this property needs no entry in the README imho.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

josephedward pushed a commit to josephedward/aws-cdk that referenced this issue Aug 30, 2022
Fixes aws#21578

Please add `pr-linter/exempt-readme` label since this property needs no entry in the README imho.

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudtrail Related to AWS CloudTrail effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants