iam-user - add ability to pick what parts of the iam-user to delete #2454
Conversation
c7n/resources/iam.py
Outdated
| client.delete_user(UserName=r['UserName']) | ||
|
|
||
| OPTIONS = { |
There was a problem hiding this comment.
can you move the class vars above the methods in the class def
There was a problem hiding this comment.
I originally had them above but the reference to OPTIONS.keys in schema required it to be below the method definitions. I'm thinking of moving the delete_* outside of the class itself, as delete_access_keys could actually be reused in the remove-keys
There was a problem hiding this comment.
ended up switching to use getattr and using a dict of {str:str} to be able to move the class vars to the top while keeping the methods inside of the class for encapsulation
c7n/resources/iam.py
Outdated
| 'ssh-keys': delete_ssh_keys, | ||
| 'signing-certificates': delete_signing_certificates, | ||
| 'services-specific-credentials': delete_service_specific_credentials, | ||
| 'user': delete_user, |
There was a problem hiding this comment.
we should omit delete user as a config option doing it in isolation is error prone without its deps and its the default action.
There was a problem hiding this comment.
Yeah, I was in between including/removing user as an option as the order of ops requires it to be last, I'll remove and change to an ordered dict and include special handling for when options is None
There was a problem hiding this comment.
If we don't include user in options then supplying options means that we should not delete the user?
c7n/resources/iam.py
Outdated
| def process(self, resources): | ||
| client = local_session(self.manager.session_factory).client('iam') | ||
| for r in resources: | ||
| self.log.debug('Deleting user %s options: %s' % |
There was a problem hiding this comment.
per resource logging on the happy path, is a bit verbose even at the debug level
c7n/resources/iam.py
Outdated
| self.process_user(client, r) | ||
|
|
||
| def process_user(self, client, r): | ||
| options = self.data.get('options', list(self.OPTIONS.keys())) |
There was a problem hiding this comment.
in python2 dictionary ordering is random, so you could get the remove user first which would error out due to dependencies, the options dict needs to be an ordered dict to handle the order. it looks like your already handling this by hardcoding the 'user' option and then removing it separately, but using an ordered dict would resolve this more simply.
c7n/resources/iam.py
Outdated
| @@ -1129,61 +1258,18 @@ class UserDelete(BaseAction): | |||
| def process(self, resources): | |||
| client = local_session(self.manager.session_factory).client('iam') | |||
| for r in resources: | |||
| self.log.debug('Deleting user %s' % r['UserName']) | |||
| self.log.debug('Deleting user %s options: %s' % | |||
There was a problem hiding this comment.
can you pull this up out of the loop to say deleting n users with x options. the per resource logging even at the verbose level is a bit much.
thisisshi
force-pushed
the
iam-user-delete-extend
branch
from
43e2ea0 to
0978c40
Compare
…s (keys, console login, etc) (cloud-custodian#2454)
related to: #2450
New Functionality:
optionsdeletewith no options (previous implementation):