Skip to content

security policy graduation to v1 #2553

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 22, 2022
Merged

security policy graduation to v1 #2553

merged 1 commit into from
Nov 22, 2022

Conversation

aryan16
Copy link
Contributor

@aryan16 aryan16 commented Nov 7, 2022 •
edited
Loading

This PR involves API changes required to graduate security policies from V1beta1 to V1 and some documentation changes for authorization policy.

@aryan16 aryan16 requested a review from a team as a code owner November 7, 2022 19:42
@istio-policy-bot
Copy link

😊 Welcome @aryan16! This is either your first contribution to the Istio api repo, or it's been
awhile since you've been here.

You can learn more about the Istio working groups, code of conduct, and contributing guidelines
by referring to Contributing to Istio.

Thanks for contributing!

Courtesy of your friendly welcome wagon.

@istio-testing istio-testing added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 7, 2022
@aryan16 aryan16 added the release-notes-none Indicates a PR that does not require release notes. label Nov 7, 2022
@aryan16 aryan16 force-pushed the policies-graduation branch from 544c11d to d6e7a49 Compare November 7, 2022 19:46
howardjohn
howardjohn reviewed Nov 7, 2022
View reviewed changes
howardjohn
howardjohn reviewed Nov 7, 2022
View reviewed changes
Copy link
Member

@howardjohn howardjohn left a comment

Choose a reason for hiding this comment

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

Can we split out new docs from the rest of the changes? They are ~unrelated and make it harder to review

@aryan16 aryan16 force-pushed the policies-graduation branch 3 times, most recently from 46dc5ec to cc24475 Compare November 7, 2022 22:35
@istio-testing istio-testing added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 7, 2022
@aryan16 aryan16 added the do-not-merge/hold Block automatic merging of a PR. label Nov 8, 2022
@aryan16 aryan16 force-pushed the policies-graduation branch from cc24475 to 1985f0f Compare November 18, 2022 21:45
@aryan16 aryan16 removed the do-not-merge/hold Block automatic merging of a PR. label Nov 18, 2022
@aryan16 aryan16 requested a review from howardjohn November 18, 2022 21:46
@aryan16 aryan16 requested a review from a team November 21, 2022 18:36
@aryan16 aryan16 force-pushed the policies-graduation branch from 1985f0f to bbd4a03 Compare November 21, 2022 19:16
linsun
linsun requested changes Nov 21, 2022
View reviewed changes
Copy link
Member

@linsun linsun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Only concern I have is the JWT.proto - some of the changes are too recent, for example ClaimToHeaders were changed few days ago and this feature is still relatively new IMHO, I don't feel comfortable promoting them to stable. Others are fine.

Thoughts?

@aryan16
Copy link
Contributor Author

aryan16 commented Nov 21, 2022

Only concern I have is the JWT.proto - some of the changes are too recent, for example ClaimToHeaders were changed few days ago and this feature is still relatively new IMHO, I don't feel comfortable promoting them to stable. Others are fine.

Thoughts?

I completely agree with your statement, but not sure if we should block the promotion because of one new feature. I will add a separate feature.md file for ClaimToHeader in enhancement which will track the status of new feature. And if we are blocking the promotion because of this then how are we planning to add new features in future once we promote the apis ? IMO we should not block the promotion for this and instead track new features via istio/enhancement

@aryan16 aryan16 force-pushed the policies-graduation branch from bbd4a03 to 7566a5b Compare November 22, 2022 04:38
@aryan16 aryan16 force-pushed the policies-graduation branch from 7566a5b to a2d9e83 Compare November 22, 2022 18:56
@aryan16 aryan16 requested a review from liminw November 22, 2022 18:58
@aryan16
Copy link
Contributor Author

aryan16 commented Nov 22, 2022

cc : @liminw

linsun
linsun approved these changes Nov 22, 2022
View reviewed changes
Copy link
Member

@linsun linsun left a comment

Choose a reason for hiding this comment

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

Thanks @aryan16 for clarifying the claimtoheader. Given we don't have a better way to track each field other than the comment, LGTM

@istio-testing istio-testing merged commit bd9c37f into master Nov 22, 2022
@istio-testing istio-testing deleted the policies-graduation branch November 22, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@howardjohn howardjohn howardjohn approved these changes

@linsun linsun linsun approved these changes

@liminw liminw Awaiting requested review from liminw

Assignees
No one assigned
Labels
release-notes-none Indicates a PR that does not require release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aryan16 @istio-policy-bot @howardjohn @linsun @istio-testing