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

Fix helm install without cluster admin priv #369

Merged
merged 6 commits into from
Apr 11, 2023

Conversation

spilchen
Copy link
Collaborator

@spilchen spilchen commented Apr 6, 2023

Normally, when installing the operator via a helm chart you need full cluster admin privileges. There is a path that you can take install the operator if you don't have cluster admin priv. That was broken a few months ago. This PR is fixing that.

The root cause was that cluster roles/rolebindings were added to the operator so that it could patch the webhook config objects. This is only needed when the operator generates its own cert for the webhook. We are adding conditional logic in those manifests to only apply them if the operator is generating the certs.

New e2e testcase was added to verify install without cluster admin priv.

@spilchen spilchen requested a review from roypaulin April 6, 2023 17:24
@spilchen spilchen self-assigned this Apr 6, 2023
Normally, when installing the operator via a helm chart you need full
cluster admin privileges. There is a path that you can take install the
operator if you don't have cluster admin priv. That was broken that this
PR is fixing.

The root cause was that cluster roles/rolebindings were added to the
operator so that it could patch the webhook config objects. This is only
needed when the operator generates its own cert for the webhook. We are
adding conditional logic in those manifests to only apply them if the
operator is generating the certs.

New e2e testcase was added to verify install without cluster admin priv.
Copy link
Collaborator

@roypaulin roypaulin left a comment

Choose a reason for hiding this comment

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

I have a few questions for now.

Matt Spilchen and others added 4 commits April 7, 2023 16:52
I missed a few things in my first commit. We patch the webhook CA bundle
for other use cases. Not just when we have the operator generate the
self-signed cert. So we need to broaden when the clusterroles and
clusterrolebindings are added.

The rules to add clusterroles and clusterrolebindings are:
- webhook must be enabled
- webhook cert source is internal
- webhook cert source is a user provided secret
Co-authored-by: Roy Paulin <paulin.nguetsop@yahoo.com>
@spilchen spilchen merged commit 41e3c9a into vertica:main Apr 11, 2023
@spilchen spilchen deleted the deploy-wo-admin branch April 11, 2023 12:19
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.

2 participants