Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
fix(iam): IAM Policies are too large to deploy #19114
fix(iam): IAM Policies are too large to deploy #19114
Changes from 10 commits
f8208e9
777c24d
e9b6cdc
68d3940
243f9c0
1f40baa
ef260c2
8b896d5
281df8b
a00582a
aeb902f
adf50cb
ccb192b
79f17d1
09cdce1
02274c3
ef354b4
00d0266
cc54755
e4c3a2a
8fbee24
625b2a7
8875096
3e570f0
fe4be84
812d4fb
fac4952
bdb113f
1f0b3c4
4988a90
2d2f16b
e1f701c
8eb3cfa
80d7a07
793da77
4fb02ef
37eaa56
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Maybe it makes sense to add three kinds of each since there are some checks at the end of the file with 3+ distinct statements?
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.
I don't think it makes a difference. Wouldn't 2 of each be enough to represent "different from the other one"? We don't need 3 to show that.
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.
While this is very cool 🙂 What's assuring that the algorithm described here is the same as the one in the TS code?
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.
Isn't that always the problem with modeling languages?
Careful review, I suppose
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.
I’ll be happy to explain this in detail btw
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.
Maybe good topic for a team time even?
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.
It would be nice if we could ensure they are side-by-side able... Like if we could annotate the TypeScript with the Alloy model pieces, and then have a tool that extracts the alloy model from the code.
This way would reduce the likelihood that one side is changed without the other side being changed to match; and would hopefully make it easier to validate that the code is expressing the same thing as the model.
Now - this is obviously work for a different PR, that is already more than enough things to look at in this one...
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.
Do we really need this property? Is the feature flag not enough?
I would get rid of it for now. We can always come back later, and add it.
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.
I like this better on principle. The boolean toggles the feature, and the default value for the boolean is taken from the flag for cases where you don't control the Policy initialization.