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

RBAC should be auto-generated #1105

Open
Tracked by #138
skitt opened this issue Feb 23, 2021 · 12 comments
Open
Tracked by #138

RBAC should be auto-generated #1105

skitt opened this issue Feb 23, 2021 · 12 comments
Assignees
Labels
blocked confirmed For issues and PRs which we definitely want (disables the stale bot) enhancement New feature or request open-cluster-management Of interest to open-cluster-management/submariner-addon priority:high size:large

Comments

@skitt
Copy link
Member

skitt commented Feb 23, 2021

We should generate the RBAC declarations from code (see the TODO in controllers/submariner/broker_controller.go). The resulting YAML files could also be re-used for the operator bundle, and the RBAC-declaration code in pkg/broker/rbac.go would no longer be necessary.

@skitt skitt added the enhancement New feature or request label Feb 23, 2021
@SteveMattar SteveMattar self-assigned this Feb 23, 2021
@mangelajo
Copy link
Contributor

@skitt @SteveMattar shall this be an item for 0.9.0? sounds risky to me.

@skitt skitt self-assigned this Mar 18, 2021
@skitt
Copy link
Member Author

skitt commented Mar 18, 2021

I’ve got a patch in progress for this, but it is indeed somewhat risky (although the code generates files we can compare with the existing setup, so it’s easy enough to verify). I’ve moved it to 0.10.

@SteveMattar
Copy link

I agree let's wait with this

skitt added a commit to skitt/submariner-operator that referenced this issue Mar 18, 2021
This allows RBAC settings to be declared as close as possible to their
point of use, which means that, as functions are added and deleted,
permissions will be adjusted "automatically" and we'll avoid keeping
no-longer-needed permissions.

As generated by the operator SDK, the operator ends up with only
cluster roles, but this makes sense since the operator is supposed to
be able to act in any namespace.

Fixes: submariner-io#1105
Signed-off-by: Stephen Kitt <[email protected]>
skitt added a commit to skitt/submariner-operator that referenced this issue Mar 19, 2021
This allows RBAC settings to be declared as close as possible to their
point of use, which means that, as functions are added and deleted,
permissions will be adjusted "automatically" and we'll avoid keeping
no-longer-needed permissions.

As generated by the operator SDK, the operator ends up with only
cluster roles, but this makes sense since the operator is supposed to
be able to act in any namespace.

Fixes: submariner-io#1105
Signed-off-by: Stephen Kitt <[email protected]>
skitt added a commit to skitt/submariner-operator that referenced this issue Mar 19, 2021
This allows RBAC settings to be declared as close as possible to their
point of use, which means that, as functions are added and deleted,
permissions will be adjusted "automatically" and we'll avoid keeping
no-longer-needed permissions.

As generated by the operator SDK, the operator ends up with only
cluster roles, but this makes sense since the operator is supposed to
be able to act in any namespace.

Fixes: submariner-io#1105
Signed-off-by: Stephen Kitt <[email protected]>
skitt added a commit to skitt/submariner-operator that referenced this issue Mar 19, 2021
This allows RBAC settings to be declared as close as possible to their
point of use, which means that, as functions are added and deleted,
permissions will be adjusted "automatically" and we'll avoid keeping
no-longer-needed permissions.

As generated by the operator SDK, the operator ends up with only
cluster roles, but this makes sense since the operator is supposed to
be able to act in any namespace.

Fixes: submariner-io#1105
Signed-off-by: Stephen Kitt <[email protected]>
skitt added a commit to skitt/submariner-operator that referenced this issue Mar 19, 2021
This allows RBAC settings to be declared as close as possible to their
point of use, which means that, as functions are added and deleted,
permissions will be adjusted "automatically" and we'll avoid keeping
no-longer-needed permissions.

As generated by the operator SDK, the operator ends up with only
cluster roles, but this makes sense since the operator is supposed to
be able to act in any namespace.

Fixes: submariner-io#1105
Signed-off-by: Stephen Kitt <[email protected]>
skitt added a commit to skitt/submariner-operator that referenced this issue Mar 19, 2021
This allows RBAC settings to be declared as close as possible to their
point of use, which means that, as functions are added and deleted,
permissions will be adjusted "automatically" and we'll avoid keeping
no-longer-needed permissions.

As generated by the operator SDK, the operator ends up with only
cluster roles, but this makes sense since the operator is supposed to
be able to act in any namespace.

Fixes: submariner-io#1105
Signed-off-by: Stephen Kitt <[email protected]>
@nyechiel nyechiel added the open-cluster-management Of interest to open-cluster-management/submariner-addon label May 6, 2021
@SteveMattar SteveMattar self-assigned this May 24, 2021
@SteveMattar
Copy link

The kustomize part is handled in this patch: #1375

@stale
Copy link

stale bot commented Sep 21, 2021

This issue has been automatically marked as stale because it has not had activity for 60 days. It will be closed if no further activity occurs. Please make a comment if this issue/pr is still valid. Thank you for your contributions.

@stale stale bot added the wontfix This will not be worked on label Sep 21, 2021
@dfarrell07
Copy link
Member

@skitt Do you have any pointers about how this should be done?

@skitt
Copy link
Member Author

skitt commented Dec 12, 2022

The relevant section in the Operator SDK documentation is https://sdk.operatorframework.io/docs/building-operators/golang/operator-scope/#restricting-roles-and-permissions

The kubebuilder reference is https://book.kubebuilder.io/reference/markers/rbac.html

See skitt#16 where I started working on this a while back — my process then was to compare the generated RBAC with the RBAC we had in our YAML files, determine whether a missing permission was significant, and add the corresponding annotation.

@dfarrell07
Copy link
Member

dfarrell07 commented Feb 7, 2023

Relates to #1240 and #1241

@skitt
Copy link
Member Author

skitt commented Mar 9, 2023

See operator-framework/operator-sdk#6100 for namespace permissions instead of cluster permissions.

@dfarrell07
Copy link
Member

I should stop beating my head against this and ask...what about the non-operator roles? I went through some ideas that turned out obviously-in-retrospect bad, like annotating some calls with extra permissions to try to cover them or cloning the other repos during generation.

@dfarrell07
Copy link
Member

I should stop beating my head against this and ask...what about the non-operator roles? I went through some ideas that turned out obviously-in-retrospect bad, like annotating some calls with extra permissions to try to cover them or cloning the other repos during generation.

Still not sure what to do about roles that are currently hosted in the operator repo but with code-to-annotate in other repos, how to connect them to the automated generation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked confirmed For issues and PRs which we definitely want (disables the stale bot) enhancement New feature or request open-cluster-management Of interest to open-cluster-management/submariner-addon priority:high size:large
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants