Skip to content
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

aws - session policy support via custodian cli #9416

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

PratMis
Copy link
Collaborator

@PratMis PratMis commented Apr 9, 2024

Closes #9404
Took a stab at it and this is an initial draft. I can go ahead and add tests etc. However, have a few open questions from a design perspective

  1. The CLI in this setup will only allow a json file. Haven't added a step where the json is validated because AWS by default gives an error at the time of assume_role if the session policy json is a MalformedPolicyDocument
    Tested with a bad json policy and the traceback received was
    botocore.errorfactory.MalformedPolicyDocumentException: An error occurred (MalformedPolicyDocument) when calling the AssumeRole operation: Syntax errors in policy.
  2. Is it worth allowing inline policies? Imo, it can get pretty messy
  3. We could potentially add a step for input validation re: whether the input is a json file or not?

I also tested it with a good session policy.

@@ -88,6 +91,10 @@ def assumed_session(role_arn, session_name, session=None, region=None, external_
def refresh():

parameters = {"RoleArn": role_arn, "RoleSessionName": session_name}
if session_policy is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the read from file should take place ideally at a higher level, potentially at the command module wrapper, we also in future want to support auto generation of the session policy, so having this consumer being agnostic to origins would be preferred.

Copy link
Collaborator Author

@PratMis PratMis Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback @kapilt. Understood re: read from file should take place ideally at a higher level,

so having this consumer being agnostic to origins would be preferred

Does this mean the session policy argument should be moved out of provider to config?

Copy link
Collaborator Author

@PratMis PratMis Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the flag outside of the AWS provider and also created a SessionPolicy class. Doing it inside the command module wrapper was causing a potential circular import dependency error. Any suggestions would be appreciated. Thanks

ImportError: cannot import name 'SessionPolicy' from partially initialized module 'c7n.commands' (most likely due to a circular import) (/Users/pratyushmishra/development/cloud-custodian/c7n/commands.py)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you just want to pass the parameter through the same way, but instead of it being the file its the content, then you won't run into the import circular since it wouldn't be instantiating in commands.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see now. We just want to pass in the contents aka the session policies as strings instead of passing a file and reading it so that we could at a later stage also be able to generate it by other means. Current implementation shouldn't care about that

c7n/cli.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leverage IAM session policies to extend custodian actions to users safely
2 participants