✨ (kustomize/v2): Add CRD viewer and editor roles in rbac/kustomization.yaml #3800

lunarwhite · 2024-03-01T13:36:04Z

Description

Add the logic to insert generated roles into kustomization file when executing kubebuilder create api.

Cleanup

Remove e2e steps for "applying the CRD Editor/Viewer Role".
Remove outdated content in https://book.kubebuilder.io/getting-started#rbac-generate-under-configrbac.
Generate docs and testdata; pass test-unit and test-e2e-local

k8s-ci-robot · 2024-03-01T13:36:13Z

Welcome @lunarwhite!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-03-01T13:36:14Z

Hi @lunarwhite. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

lunarwhite · 2024-03-01T13:43:10Z

Hi @camilamacedo86, you could take a look whenever you get a chance. Thanks!

camilamacedo86

/ok-to-test

lunarwhite · 2024-03-15T10:17:37Z

Hi @camilamacedo86, I'd appreciate it if you could take a look at this PR^^

camilamacedo86 · 2024-03-29T19:29:04Z

docs/book/src/component-config-tutorial/testdata/project/config/rbac/kustomization.yaml

@@ -16,3 +16,6 @@ resources:
 - auth_proxy_role.yaml
 - auth_proxy_role_binding.yaml
 - auth_proxy_client_clusterrole.yaml
+# Editor and Viewer roles for each CRD to be used by end users.


Can we add a better comment here to clarify why it is scaffold/when it is useful?
Example/Suggestion

For each CRD, "Editor" and "Viewer" roles are scaffolded by default, aiding admins in cluster management. While optional for managers, who can modify or remove them, their removal means they won't be installed with your solution.

camilamacedo86 · 2024-03-29T19:29:34Z

docs/book/src/getting-started.md

  // If there's an error, then restart from the beginning of the reconcile
  if err != nil {
    return reconcile.Result{}, err
  }

-  // Look for Database CR/CRD 
+  // Look for Database CR/CRD


those changes make no part of this PR please rebase with master and if it still
revert

camilamacedo86 · 2024-03-29T19:35:12Z

pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/rbac/kustomization.go

@@ -60,4 +60,5 @@ const kustomizeRBACTemplate = `resources:
 - auth_proxy_role.yaml
 - auth_proxy_role_binding.yaml
 - auth_proxy_client_clusterrole.yaml
+# Editor and Viewer roles for each CRD to be used by end users.


Lets just add this line when we create/scaffold an API and not via the init by default.
WDYT?

Agree, it would be more clear.

Do we still need to add a marker (such as #+kubebuilder:scaffold:..) as the InsertCode's target in this kustomization file? Or is there any better approach? I'd appreciate it if you could shed some more light on this!

No, just insert this line when we create the first API. Because today we create the file when we run kb init. So, if nobody run create API project than would not make sense have the comment without any API/CRD bellow right?

If you see that is to hard and get stuck that is fine we can move with it as it is.

camilamacedo86

Hi @lunarwhite

Really great work 🥇
It is amazing !!!

Just a few details / small nits suggestions and then I think we are ready to get merged this one. Could you please give a look?

camilamacedo86 · 2024-04-02T21:13:56Z

docs/book/src/component-config-tutorial/testdata/project/config/rbac/kustomization.yaml

@@ -16,3 +16,9 @@ resources:
 - auth_proxy_role.yaml


@joelanford

You request this one. What do you think?
Is that addressing your need?

lunarwhite · 2024-04-02T21:41:19Z

@camilamacedo86, just found that make generate/make check-testdata would fail to execute after rebase.

...
make[1]: Entering directory '/workspaces/kubebuilder/testdata/project-v4-multigroup'
/workspaces/kubebuilder/testdata/project-v4-multigroup/bin/controller-gen-v0.14.0 rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/workspaces/kubebuilder/testdata/project-v4-multigroup/bin/controller-gen-v0.14.0 object:headerFile="hack/boilerplate.go.txt" paths="./..."
Downloading sigs.k8s.io/kustomize/kustomize/v5@v5.3.0
mkdir -p dist
cd config/manager && /workspaces/kubebuilder/testdata/project-v4-multigroup/bin/kustomize-v5.3.0 edit set image controller=controller:latest
/workspaces/kubebuilder/testdata/project-v4-multigroup/bin/kustomize-v5.3.0 build config/default > dist/install.yaml
Error: accumulating resources: accumulation err='accumulating resources from '../rbac': '/workspaces/kubebuilder/testdata/project-v4-multigroup/config/rbac' must resolve to a file': recursed accumulation of path '/workspaces/kubebuilder/testdata/project-v4-multigroup/config/rbac': accumulating resources: accumulation err='merging resources from 'foo_bar_editor_role.yaml': may not add resource with an already registered id: ClusterRole.v1.rbac.authorization.k8s.io/bar-editor-role.[noNs]': must build at directory: '/workspaces/kubebuilder/testdata/project-v4-multigroup/config/rbac/foo_bar_editor_role.yaml': file is not directory
make[1]: *** [Makefile:121: build-installer] Error 1
make[1]: Leaving directory '/workspaces/kubebuilder/testdata/project-v4-multigroup'
make: *** [Makefile:80: generate-testdata] Error 2

One is:

accumulating resources: accumulation err='merging resources from 'foo_bar_editor_role.yaml': may not add resource with an already registered id: ClusterRole.v1.rbac.authorization.k8s.io/bar-editor-role.[noNs]'

It may indicate that there is more than one ClusterRole named "bar-editor-role" under project-v4-multigroup:

https://github.com/lunarwhite/kubebuilder/blob/4e206d87bfa13a9f71efefe55de379ed1aa436bd/testdata/project-v4-multigroup/config/rbac/fiz_bar_editor_role.yaml#L12

https://github.com/lunarwhite/kubebuilder/blob/4e206d87bfa13a9f71efefe55de379ed1aa436bd/testdata/project-v4-multigroup/config/rbac/foo_bar_editor_role.yaml#L12

Do you have any idea? Should we ensure that the names of scaffolded ClusterRoles are unique?

camilamacedo86 · 2024-04-05T05:42:25Z

Hi @lunarwhite

Before you do the rebase the dist/install.yaml was not been generating to multi-group layouts due a bug in the scaffold that was solved with the PR: https://github.com/kubernetes-sigs/kubebuilder/pull/3838/files

So, that means we probably have an issue with the edit/view roles generated in this scenario which requires a fix.

The issue is:

foo_bar_editor_role.yaml and fiz_bar_editor_role.yaml both define a ClusterRole with the name bar-editor-role. Although they target different API groups (foo.testproject.org and fiz.testproject.org respectively), the reuse of the name could be causing the conflict reported by Kustomize

Then, what we need to do is start to: if is multigroup layour and the resource has a group we should use in the name of the RBAC rule. That is the issue Edit and View Rules are not using the Group name when the project has multi-group layout support.

See the fix: #3845
After the above one get merged you can rebase with master and you will see that all will work

camilamacedo86 · 2024-04-06T05:27:03Z

docs/book/src/component-config-tutorial/testdata/project/config/rbac/kustomization.yaml

+# For each CRD, "Editor" and "Viewer" roles are scaffolded by
+# default, aiding admins in cluster management. While optional
+# for managers, who can modify or remove them, their removal
+# means they won't be installed with your solution.


@lunarwhite

Sorry, now I check the comment here better.
See that Manager means the "cmd/main.go" -> The Project Itself
We call Manager because it Manages Controllers
So, i think we need to improve this comment here.

# For each CRD, "Editor" and "Viewer" roles are scaffolded by # default, aiding admins in cluster management. Those roles are # not used by the Project itself. You can comment the following lines # if you do not want those helpers be installed with your Project.

camilamacedo86

@lunarwhite

Sorry, I did not check properly the comment
See we just need to address this nit: https://github.com/kubernetes-sigs/kubebuilder/pull/3800/files#r1554527783

Then, I am happy to approve this one for we get this one merged

camilamacedo86

/lgtm
/approved

k8s-ci-robot · 2024-04-06T06:24:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, lunarwhite

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…on.yaml

camilamacedo86 · 2024-04-06T06:36:08Z

/override pull-kubebuilder-e2e-k8s-1-29-2

k8s-ci-robot · 2024-04-06T06:36:10Z

@camilamacedo86: Overrode contexts on behalf of camilamacedo86: pull-kubebuilder-e2e-k8s-1-29-2

In response to this:

/override pull-kubebuilder-e2e-k8s-1-29-2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

camilamacedo86 · 2024-04-06T06:38:44Z

/override pull-kubebuilder-e2e-k8s-1-29-2

Reason:https://kubernetes.slack.com/archives/CCK68P2Q2/p1712378775102839

k8s-ci-robot · 2024-04-06T06:38:47Z

@camilamacedo86: Overrode contexts on behalf of camilamacedo86: pull-kubebuilder-e2e-k8s-1-29-2

In response to this:

/override pull-kubebuilder-e2e-k8s-1-29-2

Reason:https://kubernetes.slack.com/archives/CCK68P2Q2/p1712378775102839

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

camilamacedo86 · 2024-04-06T06:49:10Z

/lgtm

camilamacedo86 · 2024-04-06T07:16:12Z

/override pull-kubebuilder-e2e-k8s-1-29-2

Reason:https://kubernetes.slack.com/archives/CCK68P2Q2/p1712378775102839

k8s-ci-robot · 2024-04-06T07:16:15Z

@camilamacedo86: Overrode contexts on behalf of camilamacedo86: pull-kubebuilder-e2e-k8s-1-29-2

In response to this:

/override pull-kubebuilder-e2e-k8s-1-29-2

Reason:https://kubernetes.slack.com/archives/CCK68P2Q2/p1712378775102839

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

camilamacedo86 · 2024-04-06T07:57:28Z

/override pull-kubebuilder-e2e-k8s-1-29-2

k8s-ci-robot · 2024-04-06T07:57:32Z

@camilamacedo86: Overrode contexts on behalf of camilamacedo86: pull-kubebuilder-e2e-k8s-1-29-2

In response to this:

/override pull-kubebuilder-e2e-k8s-1-29-2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 1, 2024

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 1, 2024

k8s-ci-robot requested review from camilamacedo86 and varshaprasad96 March 1, 2024 13:36

lunarwhite marked this pull request as ready for review March 1, 2024 13:42

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 1, 2024

k8s-ci-robot requested a review from rashmigottipati March 1, 2024 13:42

camilamacedo86 reviewed Mar 1, 2024

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 1, 2024

camilamacedo86 reviewed Mar 29, 2024

View reviewed changes

lunarwhite force-pushed the add-roles branch from 60b0e18 to 53f8587 Compare April 2, 2024 19:33

camilamacedo86 requested a review from joelanford April 2, 2024 21:13

camilamacedo86 reviewed Apr 2, 2024

View reviewed changes

lunarwhite force-pushed the add-roles branch 2 times, most recently from 94952ab to bc40b32 Compare April 2, 2024 21:26

camilamacedo86 mentioned this pull request Apr 5, 2024

🐛 (go/v4,kustomize/v2): fix roles names for projects with multi-group layout #3845

Merged

lunarwhite force-pushed the add-roles branch from bc40b32 to 22ac247 Compare April 5, 2024 17:38

k8s-ci-robot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 5, 2024

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 5, 2024

camilamacedo86 reviewed Apr 6, 2024

View reviewed changes

camilamacedo86 requested changes Apr 6, 2024

View reviewed changes

lunarwhite force-pushed the add-roles branch from 22ac247 to 9a80ebc Compare April 6, 2024 06:23

camilamacedo86 approved these changes Apr 6, 2024

View reviewed changes

k8s-ci-robot assigned camilamacedo86 Apr 6, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 6, 2024

Add scaffolded CRD viewer and editor roles in config/rbac/kustomizati…

a1d8c2c

…on.yaml

lunarwhite force-pushed the add-roles branch from 9a80ebc to a1d8c2c Compare April 6, 2024 06:35

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2024

k8s-ci-robot merged commit 32e0fdc into kubernetes-sigs:master Apr 6, 2024
18 checks passed

lunarwhite deleted the add-roles branch April 6, 2024 08:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ (kustomize/v2): Add CRD viewer and editor roles in rbac/kustomization.yaml #3800

✨ (kustomize/v2): Add CRD viewer and editor roles in rbac/kustomization.yaml #3800

lunarwhite commented Mar 1, 2024

k8s-ci-robot commented Mar 1, 2024

k8s-ci-robot commented Mar 1, 2024

lunarwhite commented Mar 1, 2024

camilamacedo86 left a comment

lunarwhite commented Mar 15, 2024

camilamacedo86 Mar 29, 2024

camilamacedo86 Mar 29, 2024

camilamacedo86 Mar 29, 2024

lunarwhite Apr 2, 2024

camilamacedo86 Apr 2, 2024 •

edited

Loading

camilamacedo86 left a comment

camilamacedo86 Apr 2, 2024

lunarwhite commented Apr 2, 2024

camilamacedo86 commented Apr 5, 2024 •

edited

Loading

camilamacedo86 Apr 6, 2024 •

edited

Loading

camilamacedo86 left a comment

camilamacedo86 left a comment

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

✨ (kustomize/v2): Add CRD viewer and editor roles in rbac/kustomization.yaml #3800

✨ (kustomize/v2): Add CRD viewer and editor roles in rbac/kustomization.yaml #3800

Conversation

lunarwhite commented Mar 1, 2024

Description

k8s-ci-robot commented Mar 1, 2024

k8s-ci-robot commented Mar 1, 2024

lunarwhite commented Mar 1, 2024

camilamacedo86 left a comment

Choose a reason for hiding this comment

lunarwhite commented Mar 15, 2024

camilamacedo86 Mar 29, 2024

Choose a reason for hiding this comment

camilamacedo86 Mar 29, 2024

Choose a reason for hiding this comment

camilamacedo86 Mar 29, 2024

Choose a reason for hiding this comment

lunarwhite Apr 2, 2024

Choose a reason for hiding this comment

camilamacedo86 Apr 2, 2024 • edited Loading

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

camilamacedo86 Apr 2, 2024

Choose a reason for hiding this comment

lunarwhite commented Apr 2, 2024

camilamacedo86 commented Apr 5, 2024 • edited Loading

camilamacedo86 Apr 6, 2024 • edited Loading

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 commented Apr 6, 2024

k8s-ci-robot commented Apr 6, 2024

camilamacedo86 Apr 2, 2024 •

edited

Loading

camilamacedo86 commented Apr 5, 2024 •

edited

Loading

camilamacedo86 Apr 6, 2024 •

edited

Loading