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

Add user-facing role definitions for Envoy Gateway and Gateway API #4532

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

evankanderson
Copy link
Contributor

What type of PR is this?

feat: Add admin/edit/view namespaced rolebindings for gateway.networking.k8s.io and gateway.envoyproxy.io resources

What this PR does / why we need it:

Adds cluster roles which aggregate to the built-in user-facing cluster roles to allow users with namespace-level admin, edit, or view permissions to view the appropriate Gateway API resources.

Which issue(s) this PR fixes:

I didn't open an issue, but with the default helm chart install and view on a namespace, I get the following error:

$ kubectl get backendtrafficpolicy --context staging -n foo
Error from server (Forbidden): backendtrafficpolicies.gateway.envoyproxy.io is forbidden: User "engineering" cannot list resource "backendtrafficpolicies" in API group "gateway.envoyproxy.io" in the namespace "foo"

Release Notes: Yes

@evankanderson evankanderson requested a review from a team as a code owner October 25, 2024 17:47
Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.45%. Comparing base (6f5ae8e) to head (5468f84).
Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4532      +/-   ##
==========================================
- Coverage   65.72%   65.45%   -0.27%     
==========================================
  Files         211      211              
  Lines       31669    31858     +189     
==========================================
+ Hits        20813    20854      +41     
- Misses       9656     9759     +103     
- Partials     1200     1245      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zhaohuabing
Copy link
Member

zhaohuabing commented Oct 28, 2024

@evankanderson Thanks for adding this. The gen-check check failed. To fix it, could you please run make gen-check loacally and submitted the generate files?

rules:
- apiGroups: ["gateway.networking.k8s.io", "gateway.envoyproxy.io"]
resources: ["*"]
verbs: ["create", "update", "patch", "delete"]
Copy link
Member

@zhaohuabing zhaohuabing Oct 28, 2024

Choose a reason for hiding this comment

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

Should the edit role also be able to view(get list watch) the resources?

Copy link
Member

Choose a reason for hiding this comment

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

Should we also allow deletecollection ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good points! Added.

@evankanderson
Copy link
Contributor Author

Updated, sorry for the delay!

@evankanderson evankanderson force-pushed the add-viewer-editor branch 2 times, most recently from 0997956 to 2c9a80b Compare October 29, 2024 20:21
@evankanderson
Copy link
Contributor Author

... and fixed whatever happened with merges and DCO that made the DCO-bot mad.

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.

2 participants