-
Notifications
You must be signed in to change notification settings - Fork 64
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: add NamespacedPolicy CRD [multi-tenancy PR 7] #1402
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1402 +/- ##
======================================
Coverage ? 66.52%
======================================
Files ? 114
Lines ? 5987
Branches ? 0
======================================
Hits ? 3983
Misses ? 1621
Partials ? 383 ☔ View full report in Codecov by Sentry. |
538b2da
to
d8ce82d
Compare
a65b110
to
ecafdef
Compare
e927eb1
to
3b7a1a3
Compare
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.
thanks for the PR binbin, left some comments.
@@ -0,0 +1,27 @@ | |||
# permissions for end users to view namespacedpolicies. | |||
apiVersion: rbac.authorization.k8s.io/v1 | |||
kind: ClusterRole |
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.
Want to confirm the role here. Do we need a cluster role or role? is there a difference when rolebinding when it comes to ClusterScoped policy vs namespacedpolicy?
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.
Good question! This is actually generated by kubebuilder command, and ClusterRole
is always assigned to it. Since it's not actually used anywhere it does not make a difference. IMO, it's like a template for devs to follow in Helm template which could be updated accordingly.
config/samples/namespaced/policy/config_v1alpha1_policy_rego.yaml
Outdated
Show resolved
Hide resolved
Hi Binbin, does this PR enable customer use Cluster store/verifier , with a name spaced policy? just wondering if we should highlight this scenario in a release note for v1.2 |
3b7a1a3
to
80dc17e
Compare
synced offline, will need to merge all following CRD PRs to enable multi-tenancy. |
9046b7c
to
b163ded
Compare
b163ded
to
cf7c563
Compare
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.
LGTM
Description
What this PR does / why we need it:
This is the 7th PR for multi-tenancy support. Please review #1395 first. Check diff between PR 6 and PR 7 at: binbin-li#123
ActivePolicies
so that it fully supports accessing Policies per namespace.Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #
Type of change
Please delete options that are not relevant.
main
branch)How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration
Checklist:
Post Merge Requirements
Helm Chart Change