aws - account access-analyzer filter #6075
Conversation
|
@kapilt Any updates on this one? |
c7n/resources/account.py
Outdated
|
|
||
| def process(self, resources, event=None): | ||
| account = resources[0] | ||
| if not account.get('c7n:matched_analyzers'): |
There was a problem hiding this comment.
what's the intent on the condition, if matched is defined, and we don't execute the conditional here, the analyzers will be undefined later. typical use account.setdefault('c7n:matched_analyzers', []) .. the previous version had two separate annotations one for the analyzers, one for api cached analyzers and one for a matched analyzer. this version does away with the api cached analyzers, which is fine, but the condition logic here doesn't cleanly handle both condition states.
There was a problem hiding this comment.
Yep makes sense, that was definitely a bug. I think it should be fixed now - I iterate through each analyzer and if it matches then I add it to matched_analyzers.
There was a problem hiding this comment.
this use of annotation cache by the filter is actually another bug imo, ie it short-circuits the filter evaluation based on the presence of a previous match.
also when using an annotation key (c7n:matched-analyzers) repeatedly its better to define as a class variable to avoid typos in usage.
generally i think we also want to support chaining these filters, ie find active && last analyzed more than 30 days old.
i'll push an update to address.
There was a problem hiding this comment.
Makes sense - thanks for the help on this one!
…upport filter chaining
Addresses #6054.
I'm unsure what the status and trust values should default to if they are not provided, or whether they should be made to be required. Let me know what you think.