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

feat(iam): allow changing the effect to 'deny' for policy statements #3165

Merged
merged 1 commit into from
Jul 3, 2019

Conversation

SanderKnape
Copy link

I just noticed that I can not set a policy statement to "deny". Maybe I'm missing something, but otherwise this should be an easy fix.


Please read the contribution guidelines and follow the pull-request checklist.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@SanderKnape SanderKnape requested a review from a team as a code owner July 2, 2019 09:26
@ghost ghost requested a review from eladb July 2, 2019 09:26
@rix0rrr rix0rrr merged commit 6679e86 into aws:master Jul 3, 2019
@skinny85
Copy link
Contributor

skinny85 commented Jul 3, 2019

I have a general question about the class PolicyStatement.

Should we make it immutable? I feel like if we keep it mutable, it prevents us from ever being able to implement statement normalization in policies (to make them smaller), which we might want to do in the future.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 3, 2019

That's a good question.

I feel like maybe so, but some APIs we have now explicitly assume that they're mutable, and that they can be freely modified by adding conditions, for example. In fact, we document this somewhere.

@skinny85
Copy link
Contributor

skinny85 commented Jul 3, 2019

@rix0rrr would it be possible to implement statement normalization using CDK lifecycle callbacks? Like we now do for simple de-duplication of identical statements?

@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 3, 2019

Yes.

@skinny85
Copy link
Contributor

skinny85 commented Jul 3, 2019

Yes.

So perhaps leaving PolicyStatement mutable is not atas problematic as I initially thought.

Carry on.

Kaixiang-AWS pushed a commit to Kaixiang-AWS/aws-cdk that referenced this pull request Jul 3, 2019
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.

3 participants