-
Notifications
You must be signed in to change notification settings - Fork 2
AWS Tag compliance Codebundle #15
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code under .test doesn't appear to be valid for this codebundle. At a minumum the .test folder needs a README.md that specifies how to set up a scenario to verify the codebundle. This appears to not be updated for this scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a note like this:
- Build Test Infrastructure:
- Note: By default, the test environment leverages existing AWS resources such as VPCs and Security Groups that are untagged. These resources are sufficient to test the codebundle's tagging compliance functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you think this function is useful across other CodeBundles, we may consider moving it into the keyword.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen or needed this in other CodeBundles so far, if I see anywhere it's required will definitely move to cloudcustodian core lib
... pattern=^[a-zA-Z0-9,]+$ | ||
... example=ec2,rds,vpc,iam-group,iam-policy,iam-user,security-group | ||
... default=ec2,rds,vpc,iam-group,iam-policy,iam-user,security-group | ||
${AWS_RESOURCE_PROVIDERS_ID_MAPPINGS}= RW.Core.Import User Variable AWS_RESOURCE_PROVIDERS_ID_MAPPINGS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this? If it's needed, why doesn't it exist in the SLI? If it isn't needed, or is predictable and well known or assumed, can we remove it from the configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated it now loading it through a json file that contains resource-id mapping.
…C, subnets, security groups, and EC2 instance
…d from external JSON file
92aee75
to
8943b5c
Compare
@stewartshea we can review it, added simple EC2 infra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might need a rebase, this will likely cause merge conflicts
A few overall comments @saurabh3460 :
For scope / Issue output
|
…ng for missing tags
e213078
to
674ef07
Compare
Sure, I have tried to improve it, please have a look |
a641fcf
to
8681375
Compare
The SLI produces a score of 0 (bad), 1(good), or a value in between. This score is generated by capturing the following:
${AWS_RESOURCE_PROVIDERS}