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

Add attributes s3_key_prefix & organization_trail to aws_cloudtrail_trail resource #967

Merged
merged 3 commits into from
Mar 13, 2023

Conversation

rezen
Copy link

@rezen rezen commented Feb 17, 2023

Description

Added attributes s3_key_prefix & organization_trail to aws_cloudtrail_trail resource

Check List

Please fill box or appropriate ([x]) or mark N/A.

@rezen rezen requested a review from a team as a code owner February 17, 2023 23:12
@netlify
Copy link

netlify bot commented Feb 17, 2023

Deploy Preview for inspec-aws canceled.

Name Link
🔨 Latest commit a42b16a
🔍 Latest deploy log https://app.netlify.com/sites/inspec-aws/deploys/64028b6ab743da000874364a

@soumyo13
Copy link
Contributor

@rezen Please check DCO is failing. Please commit the code with the following command with the following command

git commit -s -m "<reason-of-the-commit>"


alias multi_region_trail? is_multi_region_trail
alias log_file_validation_enabled? log_file_validation_enabled
alias has_log_file_validation_enabled? log_file_validation_enabled
alias organization_trail? is_organization_trail
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this? We dont require the alias. is_organization_trail looks good.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to run with the control with organization_trail, it failed. It is not taking the alias.

Code that I have used is

control "test1" do
describe aws_cloudtrail_trail('testbucketname') do
its("is_organization_trail") { should eq false }
its("organization_trail") { should eq false }
its("organization_trail") { should eq "" }
end
end

output

× test1: CloudTrail testbucketname (2 failed)
✔ CloudTrail testbucketname is_organization_trail is expected to eq false
× CloudTrail testbucketname organization_trail is expected to eq false

 expected: false
      got: #<#<Class:0x000000010fc0d860>::NullResponse:0x000000010fa83198>

 (compared using ==)
 ×  CloudTrail testbucketname organization_trail is expected to eq false

 expected: false
      got: #<#<Class:0x000000010fc0d860>::NullResponse:0x000000010fa83198>

 (compared using ==)

Copy link
Contributor

Choose a reason for hiding this comment

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

@rezen Please check this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Try it { should be_organizational_trail } . It makes good English.

Copy link
Contributor

Choose a reason for hiding this comment

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

it { should be_organizational_trail }

This is working fine. So we can merge this PR.

@rezen
Copy link
Author

rezen commented Feb 28, 2023

@soumyo13 I did the sign off, I think it's unhappy because I didn't include my email in Signed-off-by: Andres Hermosilla <>

@soumyo13
Copy link
Contributor

soumyo13 commented Mar 1, 2023

@rezen Please add the email in the sign off. I think that will work and DCO will pass.

Andres Hermosilla added 3 commits March 3, 2023 16:05
…ttributes

Signed-off-by: Andres Hermosilla <an2dres5m@gmail.com>
Signed-off-by: Andres Hermosilla <an2dre5m@gmail.com>
Signed-off-by: Andres Hermosilla <an2dres5m@gmail.com>
@rezen
Copy link
Author

rezen commented Mar 4, 2023

@soumyo13 Fixed the DCO 👍

Copy link
Contributor

@soumyo13 soumyo13 left a comment

Choose a reason for hiding this comment

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

LGTM

@soumyo13 soumyo13 merged commit 2c69985 into inspec:main Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants