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

Implemented first usable version of the RBAC Operator #1

Merged
merged 5 commits into from
Jun 20, 2024

Conversation

ggkhrmv
Copy link
Collaborator

@ggkhrmv ggkhrmv commented Jun 10, 2024

The first version of the RBAC Operator has been implemented. The make deploy command doesn't work yet, since I couldn't push the docker image to the repo.

To run the controller locally:

make manifests
make install
export ENABLE_WEBHOOKS=false
make run

@ggkhrmv ggkhrmv requested a review from leoluz June 10, 2024 21:19
@ggkhrmv ggkhrmv added the enhancement New feature or request label Jun 11, 2024
config/crd/bases/argoproj.io_roles.yaml Outdated Show resolved Hide resolved
internal/controller/common/values.go Show resolved Hide resolved
internal/controller/role_controller.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Show resolved Hide resolved
Copy link
Member

@agaudreault agaudreault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it seems like you have used the latest version of https://github.com/kubernetes-sigs/kubebuilder. However, I think the controller might try to do too much already. Can we limit the first version to manage policy.*.csv keys only in configurable namespaces? This seems to be the main use case.

On another note, @sbeginCoveo @ggkhrmv if you want to discuss on the CNCF Slack, just send me a DM. I would like to have a chat to understand why this operator is a must for you. In almost all ArgoCD deployments I have come across, the global rbac policy configuration don't change often. Usually, it is the policies in the AppProject resource that are harder to define and manage. So I am struggling to grasp the long-term value of this operator, even though it is a nicer way to write the policies.

config/rbac/leader_election_role.yaml Show resolved Hide resolved
internal/controller/common/defaults.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Outdated Show resolved Hide resolved
Copy link

@sbeginCoveo sbeginCoveo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing too deep to address, well done implementing the bindings reconciler
Great job giving users feedback though the status 🚀 That's really awesome
I'm gonna give it a spin on my cluster

internal/controller/argocdrolebinding_controller.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Outdated Show resolved Hide resolved
internal/controller/argocdrole_controller.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Outdated Show resolved Hide resolved
internal/controller/configmap.go Outdated Show resolved Hide resolved
@ggkhrmv ggkhrmv removed the request for review from leoluz June 17, 2024 14:13
api/v1alpha1/groupversion_info.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@leoluz leoluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a minor comment.
LGTM regardless

Signed-off-by: Georgy Khromov <[email protected]>
@ggkhrmv ggkhrmv merged commit 25df298 into main Jun 20, 2024
1 check passed
@ggkhrmv ggkhrmv deleted the feat/RBAC-Operator branch June 20, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants