feat: Use rbac from controller repo in the development environment and sync rbac with helm charts #704
Conversation
There was a problem hiding this comment.
LGTM, nice changes!
On the CRDs, I think we should only include policies.kubewarden.io_policyservers.yaml and not the other formatting changes, as they may break the crds-to-docs.kubewarden.io workflow.
For the duplicated rule in manager-role, I can't reproduce from this PR with and without the new changes. Could it be that you have ./bin/controller-gen != 0.8.0?
There was a problem hiding this comment.
I'm conflicted. I understand why you would like to have tilt consume the RBAC rules from this repository, instead of the helm chart.
However there's also the fact that the real (as in the final one for our users) rules are the one from the helm chart. Hence it would make sense to not change the current behavior.
That's something that we can probably solve during one of our meetings (rather then endless conversations on GH)
config/crd/bases/policies.kubewarden.io_clusteradmissionpolicies.yaml
Outdated
Show resolved
Hide resolved
jvanz
force-pushed
the
use-rbac-from-controller-repo
branch
2 times, most recently
from
49bfd4b
to
ed92417
Compare
jvanz
force-pushed
the
use-rbac-from-controller-repo
branch
from
ed92417
to
ad59b48
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #704 +/- ##
==========================================
- Coverage 50.71% 44.97% -5.75%
==========================================
Files 27 22 -5
Lines 2021 1543 -478
==========================================
- Hits 1025 694 -331
+ Misses 889 788 -101
+ Partials 107 61 -46
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
jvanz
force-pushed
the
use-rbac-from-controller-repo
branch
from
ad59b48
to
3e27244
Compare
jvanz
changed the title
viccuad
force-pushed
the
use-rbac-from-controller-repo
branch
from
3e27244
to
5ff9de1
Compare
|
rebased via UI as it was needed. |
|
argh, now it complains about signed commits. @jvanz could you rebase on your own? (and that way remove me as committer, leaving you as authored, which is more fitting). |
Updates the Kubebuilder directives to be in sync with the RBAC used in the Helm charts to install Kuberwanden. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
Updates the Tiltfile to change the Roles and ClusterRoles defined in the Helm charts to use the rules defined in the RBAC defined in the local directory. Therefore, when permissions are added,changed or removed, there is no need to copy the content to the Helm chart directory. Signed-off-by: José Guilherme Vanz <jguilhermevanz@suse.com>
jvanz
force-pushed
the
use-rbac-from-controller-repo
branch
from
5ff9de1
to
aa6b27d
Compare
Description
Updates the Tiltfile to change the Roles and ClusterRoles defined in the Helm charts to use the rules defined in the RBAC defined in the local directory. Therefore, when permissions are added,changed or removed, there is no need to copy the content to the Helm chart directory.
During this process I've found out that some kubebuilder directives does not match what we use in the Helm charts. Thus, I've updated them. Furthermore, for some reason, kubebuilder generates a duplicate rule in the manager-role role. Witch is causing the resultant role to not have create/update access to the deployments resource. A Kustomize patch to remove that rule has been added. This problematic rule is not present in the Helm chart roles as well.
This changes are written during my tests on #698
Test
Additional Information
I found weird that a duplicate rule of a RBAC overwrite a previous rule defined in the same role. As far as I can remember, this should not happen. But it was happening in my local cluster. Furthermore, the same problematic rule is remove in the Helm charts. Maybe this is can be a problem indeed. I've also tried to change the Kubebuilder directives to avoid that, but I could not find a way to workaround that there. If you know why that happen or another workaround, let me know.