-
Notifications
You must be signed in to change notification settings - Fork 378
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
IAMPolicy #197
IAMPolicy #197
Conversation
8b36d18
to
6fedab9
Compare
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.
This one mostly LGTM, but per my comment I think we need to store the ARN in the external-name annotation.
Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
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.
Thanks @sahil-lakhwani! I've added one small suggestion that I'd like to address before I merge this.
Signed-off-by: sahil-lakhwani <sahilakhwani@gmail.com>
Description of your changes
Adds IAM Policy as a managed resource.
Example:
example/iam/policy.yaml
This PR depends on #194 & #196, and should not be merged before those
For the update case:
An update to the policy is actually creating a new version of that policy. The approach in this PR sets the latest version as the default version. Also, because only maximum of 5 versions are allowed, the oldest version is deleted for an update request when there are already 5 versions.
Checklist
I have:
make reviewable
to ensure this PR is ready for review.app.yaml
to include any new role permissions.