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

Support RBAC in a generic way #696

Closed
Tracked by #668
csviri opened this issue Aug 23, 2023 · 15 comments
Closed
Tracked by #668

Support RBAC in a generic way #696

csviri opened this issue Aug 23, 2023 · 15 comments
Assignees

Comments

@csviri
Copy link
Contributor

csviri commented Aug 23, 2023

Currently roles and role bindings for secondary resources are generated based on managed dependent resources. While this is good, there are cases when managed dependent resources are not sufficient to cover the logic of the controller, but the roles / bindings for secondary resource we still want to generate.

One way to handle this is to introduce annotations for the reconciler that will provide all the necessary information to generate rbac roles and bindings.

See how it works in kubebuilder: https://book.kubebuilder.io/reference/markers/rbac.html?highlight=rbac#rbac

@csviri csviri changed the title Support RBAC in a generic way (discuss) Support RBAC in a generic way Aug 23, 2023
@csviri csviri self-assigned this Aug 23, 2023
@csviri
Copy link
Contributor Author

csviri commented Aug 23, 2023

Note that this is not just for helm, for all the generated artifacts, like kubernetes.yaml and possibly OLM

@Jeansen
Copy link

Jeansen commented Aug 28, 2023

I have a practical issue where I have a very simple controller which approves server-side certificates -> https://github.com/Jeansen/cert-operator .

Generated permissions are as follows:

      clusterPermissions:
      - rules:
        - apiGroups:
          - apiextensions.k8s.io
          resources:
          - customresourcedefinitions
          verbs:
          - get
          - list
        - apiGroups:
          - certificates.k8s.io
          resources:
          - certificatesigningrequests
          - certificatesigningrequests/status
          - certificatesigningrequests/finalizers
          verbs:
          - get
          - list
          - watch
          - patch
          - update
          - create
          - delete

But I had to add:

- apiGroups:
  - certificates.k8s.io
  resources:
  - certificatesigningrequests/approval
  verbs:
  - update
- apiGroups:
  - certificates.k8s.io
  resourceNames:
  - kubernetes.io/kubelet-serving
  resources:
  - signers
  verbs:
  - approve

So, having a way to add,patch,merge additional requirements would be great. Maybe even in a kustomize manner.

metacosm added a commit that referenced this issue Aug 31, 2023
See SimpleReconciler for a usage example.
Fixes #696
@metacosm
Copy link
Member

metacosm commented Aug 31, 2023

@Jeansen could you try #701 to see if this fits your needs, please? Take a look at the SimpleReconciler class for an example of how to use the feature.

@Jeansen
Copy link

Jeansen commented Aug 31, 2023

@metacosm Oh, great. I'll check it out and report back.

@Jeansen
Copy link

Jeansen commented Aug 31, 2023

@metacosm Hm..I do not see any effect. I've cloned the repo. switched to the additional-rbac branch and did mvn clean install. I then changed the versions in my project accordingly. Gradle will pick them up from the local .m2 folder. I could add the annotations from your example and compile. But I do not see any effect in the generated bundle or manifest files. Maybe I am missing something? I've pushed my changes in https://github.com/Jeansen/cert-operator .

@metacosm
Copy link
Member

metacosm commented Sep 1, 2023

I'm not familiar with Gradle but trying it out your project, it didn't seem to pick up my just built snapshot from my local maven repository… or at least, not completely, since it did indeed find the annotations module (at least, I think it did). So there's something weird, going on. Step debugging the configuration steps in dev mode resulted in the debugger clearly not using the right version of the code.

@metacosm
Copy link
Member

metacosm commented Sep 1, 2023

Found the error: you're using the 6.3.0 version of the BOM. Changing that version to 6.3.1-SNAPSHOT fixes the problem and the additional rules do properly show up. I don't know what's Gradle way to deal with this but you should import the BOM and not have explicit versions on the QOSDK dependencies anywhere else, otherwise this causes issues as just seen.

@metacosm
Copy link
Member

metacosm commented Sep 1, 2023

That said, checking things out again allowed me to find an issue so it's all good :)

@Jeansen
Copy link

Jeansen commented Sep 1, 2023

Found the error: you're using the 6.3.0 version of the BOM. Changing that version to 6.3.1-SNAPSHOT fixes the problem and the additional rules do properly show up. I don't know what's Gradle way to deal with this but you should import the BOM and not have explicit versions on the QOSDK dependencies anywhere else, otherwise this causes issues as just seen.

Well, actually - to my understanding - I gave the versions only to those dependencies I wanted to test and keep the rest as is. But well ... lessons learned ;-)

@Jeansen
Copy link

Jeansen commented Sep 1, 2023

That said, checking things out again allowed me to find an issue so it's all good :)

OK. I've no compiled again with the BOM set correctly and it works. I'll tinker a bit more in the evening.

What issue did you find? Anything I should look out for?

@metacosm
Copy link
Member

metacosm commented Sep 1, 2023

Found the error: you're using the 6.3.0 version of the BOM. Changing that version to 6.3.1-SNAPSHOT fixes the problem and the additional rules do properly show up. I don't know what's Gradle way to deal with this but you should import the BOM and not have explicit versions on the QOSDK dependencies anywhere else, otherwise this causes issues as just seen.

Well, actually - to my understanding - I gave the versions only to those dependencies I wanted to test and keep the rest as is. But well ... lessons learned ;-)

Looking at it again: the problem is that you weren't overriding all the dependencies that needed to be replaced, most importantly the core-deployment one, which is where the "magic" happens and which is why you should always use the BOM versions so that you get all the versions aligned correctly :)

@metacosm
Copy link
Member

metacosm commented Sep 1, 2023

That said, checking things out again allowed me to find an issue so it's all good :)

OK. I've no compiled again with the BOM set correctly and it works. I'll tinker a bit more in the evening.

What issue did you find? Anything I should look out for?

It should already be fixed: the problem was that the code wasn't handling a single RBACRule annotation properly, it only worked if you had multiple ones. This is fixed by d3bbadb

@Jeansen
Copy link

Jeansen commented Sep 1, 2023

OK then. I'll play with it more this evening and try out more. Thank you very much for your input, help and efforts.

@Jeansen
Copy link

Jeansen commented Sep 1, 2023

So, I've successfully added some more annotations, build and bundled everything and deployed it via OPM. Rocks like a guitar :-D

@metacosm
Copy link
Member

metacosm commented Sep 2, 2023

Thanks for the update!

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

No branches or pull requests

3 participants