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

Design update for EncryptRootVolume to explain IAM Automate Service r… #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

awsandrewpark
Copy link
Contributor

Issue #, if available: N/A

Description of changes: Design documentation update for EncryptRootVolume to explain IAM requirement for the automation service role.

@awsandrewpark
Copy link
Contributor Author

@alemartini here is better PR. please review and approve.

@awsandrewpark
Copy link
Contributor Author

@alemartini just a friendly reminder to approve and merge. Thanks!

@nanalakshmanan
Copy link
Contributor

@awsandrewpark the change contains a recommendation to create a role with some specified policies. How do we ensure correctness of this recommendation? Maybe some tests?

@awsandrewpark
Copy link
Contributor Author

@nanalakshmanan Thanks for your comment. The role and the policies are from my testing during the development phase and they are correct but I am assuming that you're talking about something different. If you can please let me know of acceptable forms of testing method(s), I'd be happy to see what I can do to accommodate it. Please let me know.

@awsandrewpark
Copy link
Contributor Author

@nanalakshmanan Hi Nana. It's been a full week and I've not received any reply from you. Could you please advise? If you do not have a hard requirement(s), perhaps the PR can be just merged? Afterall, it's just a documentation update. Please let me know,

@nanalakshmanan
Copy link
Contributor

@awsandrewpark What is preferred is the following:

  • Design.md references a JSON file with the required IAM policy
  • the JSON file is validated using a test - so when there is a change, the test catches the same

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.

2 participants