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

feat: Add support for custom ClusterRoles (controller,server) for cluster scoped instances #1357

Merged
merged 2 commits into from
May 24, 2024

Conversation

jparsai
Copy link
Collaborator

@jparsai jparsai commented May 16, 2024

What type of PR is this?

/kind enhancement

What does this PR do / why we need it:

This PR enables user to define custom roles for cluster scoped instances of Argo CD. This is becoming increasingly important with the addition of the auto-respect RBAC feature added in Argo CD 2.10.

Have you updated the necessary documentation?

  • Documentation update is required by this PR.
  • Documentation has been updated.

Which issue(s) this PR fixes:

Fixes #1275
Note this also tracked in Red Hat's issue database here: https://issues.redhat.com/browse/GITOPS-2614

How to test changes / Special notes to the reviewer:

  • Create a namespace and to make it cluster scoped namespace update ARGOCD_CLUSTER_CONFIG_NAMESPACES environment variable of Subscription resource.
  • Create ClusterRoles and ClusterRoleBindings for Application controller and server.
  • Create Argo CD CR in namespace and set Spec.DefaultClusterScopedRoleCreationDisabled to true.
  • All Argo CD components should be healthy.

Copy link
Collaborator

@svghadi svghadi left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @jparsai. I have some concerns regarding backward compatibility. Are we removing the ability to specify cluster roles via subscriptions?

controllers/argocd/rolebinding.go Outdated Show resolved Hide resolved
@jparsai jparsai changed the title feat: Add support for custom ClusterRoles (controller,server) for cluster scoped instances [WIP] feat: Add support for custom ClusterRoles (controller,server) for cluster scoped instances May 22, 2024
@jparsai jparsai changed the title [WIP] feat: Add support for custom ClusterRoles (controller,server) for cluster scoped instances feat: Add support for custom ClusterRoles (controller,server) for cluster scoped instances May 22, 2024
Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

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

Looks great, just some minor stuff I could think of re: expected user behaviour.

EDIT: kuttl tests for this use case are blocked due to need to set env var, so ignore this: One more request: can you add a simple kuttl test that sets DefaultClusterScopedRoleCreationDisabled to true, and ensures that ClusterRole/ClusterRoleBinding are not created?

controllers/argocd/role_test.go Outdated Show resolved Hide resolved
controllers/argocd/role_test.go Show resolved Hide resolved
controllers/argocd/rolebinding_test.go Show resolved Hide resolved
controllers/argocd/role.go Outdated Show resolved Hide resolved
controllers/argocd/rolebinding.go Outdated Show resolved Hide resolved
Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jparsai!

@jgwest jgwest merged commit 4047a69 into argoproj-labs:master May 24, 2024
7 checks passed
reginapizza pushed a commit to reginapizza/argocd-operator that referenced this pull request Jun 5, 2024
…ster scoped instances (argoproj-labs#1357)

* feat: Enable use of alternate cluster roles for cluster scoped instances

Signed-off-by: Jayendra Parsai <[email protected]>

* Update role/binding unit tests, add a new line to the doc

Signed-off-by: Jonathan West <[email protected]>

---------

Signed-off-by: Jayendra Parsai <[email protected]>
Signed-off-by: Jonathan West <[email protected]>
Co-authored-by: Jayendra Parsai <[email protected]>
Co-authored-by: Jonathan West <[email protected]>
@svghadi svghadi added the backport-to-redesign Changes which need to be backported to operator-redesign branch label Jun 19, 2024
@svghadi svghadi removed the backport-to-redesign Changes which need to be backported to operator-redesign branch label Aug 13, 2024
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

Successfully merging this pull request may close these issues.

Support defining custom cluster roles for cluster scoped instances
3 participants