-
Notifications
You must be signed in to change notification settings - Fork 83
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
Rule class #21
Rule class #21
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.
Really excellent PR. Nice Job!
namespace_based_results = harden(resources, rules, "namespace_based") | ||
results = results + namespace_based_results | ||
|
||
print_consolidated_results(results) | ||
|
||
if export_txt: |
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.
Do we want users to be able to add all three export options at once? Or do we only want to allow one at a time?
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 think it is ok for them to do all three export at once? I did it that way assuming if someone needs to see different outputs but run the tool only once
@@ -39,184 +37,216 @@ def _get_policy_documents_for_role(role_name, iam_client): | |||
return actions | |||
|
|||
|
|||
def check_any_cluster_autoscaler_exists(resources: Resources): | |||
class check_any_cluster_autoscaler_exists(Rule): | |||
_type = "cluster_wide" |
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.
So I really like this method of setting the metadata. I'm wondering, should we take it further and put this into the config? each test we run in the config would have its own set of metadata? Possibly could allow for extensions that users could write on their own.
Just a thought maybe another iteration we do something like that.
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.
definitely. That is where I was going :) This is to make the extension easier.
Issue #, if available:
Description of changes: