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
azure - parent filter for child resources & bugfix #4611
azure - parent filter for child resources & bugfix #4611
Conversation
self.parent_manager) | ||
|
||
def process(self, resources, event=None): | ||
parent_resources = self.parent_filter.process(self.parent_manager.resources()) |
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.
it would be much more efficient to use get_resources() with the unique parent ids then fetching all, unless its expected they are already cached.
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 for non-event based executions we had to fully enumerate the parents already to get the children, so in that case that is OK as long as we add the cache read in to the def resources
(I know it was missing since I created the resource manager years ago :( )
But for event based we'd certainly need some special handling to call get_resources
with the right ID(s). If we don't have any children that support events right now we can punt on it (but I think we do?)
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.
Good point on events.. I will take a look
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.
Created a work item to ensure child resources work properly with event mode. I think today both ARM\non-arm children have issues.
tools/c7n_azure/c7n_azure/filters.py
Outdated
filters: | ||
- type: parent | ||
filter: | ||
"tag:owner": ProjectA |
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 not be good to use the special case as the example here, as it still isn't clear how to use a normal filter. Presumably I'd put a -type: filtername
under filter - but for 99.99% of people this will be the first time they have seen this tag syntax and they'll have no idea what is going on.
Or just have two examples, a value filter is fine, or any thing that works on kv/cosmos/sql
cache_key = self.get_cache_key(query) | ||
|
||
resources = None | ||
if self._cache.load(): |
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.
Cool yeah we needed to fix this, but I admit I am a little worried about what new issues we'll discover now, as none of us have ever used the caching in Azure.
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.
Changed default cache for Azure to memory because of the way Azure SDK works..
15 minute old cache can introduce regressions in the resources if they were change after cache was saved (via the portal e.g.)
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.
Just looking to understand if we need to do something for event based lookups of parents efficiently - otherwise looks great.
QueryResourceManager
not handlingcache
at allparent
filter for child resources. Allows you to apply any parent filter to the child parent.Example of use: filter Key Vault keys only from specific Key Vaults (tags, names etc) or CosmosDB collections from specific CosmosDBs.
closes #4390