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.

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

examples: add an example to illustrate authorization (authz) support #5920

Merged
merged 18 commits into from Mar 2, 2023
Merged

examples: add an example to illustrate authorization (authz) support #5920

merged 18 commits into from Mar 2, 2023

Conversation

KenxinKun
Copy link
Contributor

@KenxinKun KenxinKun commented Jan 10, 2023 •
edited by dfawley

Partially addresses #5900 using a hard coded string.

RELEASE NOTES:

  • examples: add an example to illustrate authorization (authz) support

@KenxinKun KenxinKun mentioned this pull request Jan 10, 2023
Merged
@KenxinKun KenxinKun marked this pull request as ready for review January 10, 2023 20:42
easwars
easwars requested changes Jan 10, 2023
View reviewed changes
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
@easwars easwars added the Type: Documentation Documentation or examples label Jan 10, 2023
@easwars easwars added this to the 1.53 Release milestone Jan 10, 2023
@KenxinKun
Copy link
Contributor Author

FYI pushed a few changes, not finished with all the feedback yet :)

@KenxinKun
Copy link
Contributor Author

@easwars Do you prefer to resolve the comments yourself on review, or should I mark them msyelf? I've addressed them all but the one on the stream wrapper

@easwars
Copy link
Contributor

easwars commented Jan 11, 2023

@easwars Do you prefer to resolve the comments yourself on review, or should I mark them msyelf? I've addressed them all but the one on the stream wrapper

Thanks for asking. Yes, we prefer to resolve the comments ourselves, because if the author marks it as resolved, then the reviewer will have to go through the extra step of clicking through each of those comments to see if they were resolved appropriately.

Also, please let me know once you have addresses all comments and I will make another pass.

@KenxinKun
Copy link
Contributor Author

Hi @easwars just pushed my last changes and have addressed all comments, please go ahead for another review pass :)

@easwars easwars assigned easwars and unassigned KenxinKun Jan 17, 2023
easwars
easwars requested changes Jan 17, 2023
View reviewed changes
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
easwars
easwars previously approved these changes Jan 17, 2023
View reviewed changes
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
@easwars easwars assigned KenxinKun and unassigned easwars Jan 17, 2023
@easwars easwars dismissed their stale review January 20, 2023 00:19

Approved by mistake. There are still comments to be addressed.

@easwars easwars changed the title Add authz example examples: add an example to illustrate authorization (authz) support Jan 20, 2023
@KenxinKun
Copy link
Contributor Author

Hi @easwars sorry for the delay, I've addressed all your points (left a question in #5920 (comment))
I think I've illustrated the behaviours you requested (and removed the unnecessary bits), let me know what you think :)

easwars
easwars requested changes Jan 25, 2023
View reviewed changes
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/client/main.go Show resolved Hide resolved
examples/features/authz/client/main.go Show resolved Hide resolved
examples/features/authz/client/main.go Show resolved Hide resolved
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/client/main.go Outdated Show resolved Hide resolved
examples/features/authz/server/main.go Outdated Show resolved Hide resolved
examples/features/authz/token/token.go Outdated Show resolved Hide resolved
examples/features/authz/token/token.go Show resolved Hide resolved
@easwars
Copy link
Contributor

easwars commented Jan 25, 2023

I think we are quite close to having a good example now. Just some minor nits this time around. Thanks.

@easwars easwars self-assigned this Jan 25, 2023
@dfawley
Copy link
Member

dfawley commented Jan 25, 2023

cc @temawi

@arvindbr8 arvindbr8 modified the milestones: 1.53 Release, 1.54 Release Jan 26, 2023
@easwars
Copy link
Contributor

easwars commented Feb 2, 2023

@dfawley : Could you please take a look.

@dfawley dfawley self-requested a review February 7, 2023 18:34
dfawley
dfawley requested changes Feb 15, 2023
View reviewed changes
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Thanks for adding the explanatory text! LGTM except for the two small nits.

examples/features/authz/README.md Outdated Show resolved Hide resolved
examples/features/authz/README.md Outdated Show resolved Hide resolved
@dfawley dfawley removed their assignment Feb 15, 2023
@dfawley
Copy link
Member

dfawley commented Feb 15, 2023

cc @temawi FYI

@zasweq zasweq assigned easwars and unassigned KenxinKun Feb 21, 2023
easwars
easwars approved these changes Feb 21, 2023
View reviewed changes
Copy link
Contributor

@easwars easwars left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution.

@easwars
Copy link
Contributor

easwars commented Feb 21, 2023

This looks good, but I'm unable to merge this at this point in time because @dfawley "requested changes" in his most recent review. Although you have addressed those changes, @dfawley is out this week. So, once he come back and marks the requested changes as addressed, we will be able to merge this.

Thanks again.

@easwars easwars assigned dfawley and unassigned easwars Feb 21, 2023
easwars
easwars approved these changes Feb 21, 2023
View reviewed changes
examples/features/authz/README.md Outdated Show resolved Hide resolved
dfawley
dfawley approved these changes Mar 2, 2023
View reviewed changes
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Thanks!

@dfawley dfawley merged commit 20141c2 into grpc:master Mar 2, 2023
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@easwars easwars easwars approved these changes

@dfawley dfawley dfawley approved these changes

Assignees

@dfawley dfawley

Labels
Type: Documentation Documentation or examples
Projects
None yet
Milestone
1.54 Release
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@KenxinKun @easwars @dfawley @arvindbr8