Added support for AWS Credentials profiles and enhanced Ctrl-C handling #8
Conversation
Maybe it is not working because the process is running one of the threads and there is a broad try/except clause like the one we see here: https://github.com/andresriancho/enumerate-iam/blob/master/enumerate_iam/main.py#L144 Instead of implementing the The original code has KeyboardInterrupt handling and I remember testing it at some point. Does it work on some scenarios for you? Never? |
…up/role policies to the stdout
| CLIENT_POOL = {} | ||
| STOP_SIGNAL = False | ||
| MANAGER = Manager() |
There was a problem hiding this comment.
Looks like we can move the manager and stop_signal variables to a different scope? Using variables with global scope should be avoided as much as possible.
There was a problem hiding this comment.
Which particular scope would you think of?
There was a problem hiding this comment.
Where you wrote: global STOP_SIGNAL
Replace with:
manager = Manager()
stop_signal = manager.Value('i', 0)
And remove the old MANAGER and STOP_SIGNAL. Did not check if that works, please confirm :-)
| @@ -319,6 +325,15 @@ def enumerate_role(iam_client, output): | |||
| for policy in role_policies['AttachedPolicies']: | |||
| logger.info('-- Policy "%s" (%s)', policy['PolicyName'], policy['PolicyArn']) | |||
There was a problem hiding this comment.
If the others changed from info to debug, maybe this one should be debug too?
I'm not sure why you changed some of the calls from logger.info to logger.debug but I'll trust you on those changes and that the whole tool will use the same "rules" to decide what is info and what is debug.
There was a problem hiding this comment.
So right, I've seen that running the tool by default on verbose output is fine to the general UX, but as soon as iam's get-account-authorization-details results gets dumped, they will generate such a big JSON that would effectively make reading program's output cumbersome. So I started using logger.debug anywhere we've been dumping JSON contents. Whereas policy's name does not affect programs output to the point of making it being of a debug level.
If we wish to have the user know what's going on during program's non-verbose execution, then we shall go with logger.info only for essential or short enough outputs. Should a user want to learn more what the program learns as it goes, debug would provide him with all we've got to say there.
There was a problem hiding this comment.
Totally agree with your decision, lets keep it like that: debug contains JSON, info contains short messages.
Just one more thing: for the places where you removed the logger.info(), is there a short message we could use? For example:
logger.info('Run for the hills, get_account_authorization_details worked!')
Could still be info.
| if key not in output.keys(): output[key] = [] | ||
| output[key].append(remove_metadata(policy_version)) | ||
| except: | ||
| pass |
There was a problem hiding this comment.
Maybe log error debug log? Applies to all the new try/except that were added.
| CLIENT_POOL = {} | ||
| STOP_SIGNAL = False | ||
| MANAGER = Manager() |
There was a problem hiding this comment.
Where you wrote: global STOP_SIGNAL
Replace with:
manager = Manager()
stop_signal = manager.Value('i', 0)
And remove the old MANAGER and STOP_SIGNAL. Did not check if that works, please confirm :-)
|
getting this with the fork (I was looking for a solution to the CTRL-C issue): Traceback (most recent call last): got rid of import botocore.vendored.requests.packages.urllib3 as urllib3 and the following line in main.py. Not sure if it will have side effects... |
This commit introduced support for
--profileargument for those who has multiple AWS credentials stored creds file with dedicated profile name labels. Also, sincePool.mapis a blocking operation, found Ctrl-C not working anytime I hit it. Therefore a minor work was done to switch parallel execution tomap_asyncapproach, while introducing general timeout of 600 seconds and reinforcing SIGINT handler accoriding to the following recipe:https://stackoverflow.com/questions/11312525/catch-ctrlc-sigint-and-exit-multiprocesses-gracefully-in-python
Best regards,
M.