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.

Sign up for GitHub

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

Jump to bottom

Fix webhook certname in helm template #7775

Merged
merged 3 commits into from
May 7, 2024
Merged

Fix webhook certname in helm template #7775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba0c04e
fix webhook secret name
kvalliyurnatt May 6, 2024
6e02fea
fix the configmap as well
kvalliyurnatt May 6, 2024
76cfe22
add unit test for helm chart
kvalliyurnatt May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions deploy/eck-operator/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ Determine the name for the webhook secret
{{- define "eck-operator.webhookSecretName" -}}
{{- if .Values.global.manifestGen -}}
elastic-webhook-server-cert
{{- else if .Values.webhook.certsSecret -}}
{{- .Values.webhook.certsSecret }}
{{- else -}}
{{- $name := include "eck-operator.name" . -}}
{{ printf "%s-webhook-cert" $name | trunc 63 }}
Expand Down
2 changes: 1 addition & 1 deletion deploy/eck-operator/templates/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,6 @@ data:
{{- if not .Values.config.containerSuffix }}
ubi-only: {{ .Values.config.ubiOnly }}
{{- end }}
{{- with .Values.webhook.secret }}
{{- with .Values.webhook.certsSecret }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure we should keep this as webhook.secret and change the docs ? since the operator config itself is named webhook-secret

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if we fix this I don't understand how this can be used today since the WEBHOOK_SECRET env. variable seems to take precedence over webhook-secret in the configmap :

cloud-on-k8s/deploy/eck-operator/templates/statefulset.yaml

Lines 73 to 76 in 233cbd0

{{- if .Values.webhook.enabled }}
- name: WEBHOOK_SECRET
value: {{ include "eck-operator.webhookSecretName" . }}
{{- end }}

webhook-secret: {{ . }}
{{- end }}
22 changes: 22 additions & 0 deletions deploy/eck-operator/templates/tests/statefulset_test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,25 @@ tests:
app.kubernetes.io/version: 2.14.0-SNAPSHOT
helm.sh/chart: eck-operator-2.14.0-SNAPSHOT
key2: value2
- it: should use the specified webhook secret name
set:
webhook:
manageCerts: false
certsSecret: "my-webhook-server-cert"
asserts:
- template: statefulset.yaml
equal:
path: spec.template.spec.volumes[1].name
value: cert
- template: statefulset.yaml
equal:
path: spec.template.spec.volumes[1].secret.secretName
value: my-webhook-server-cert
- template: statefulset.yaml
equal:
path: spec.template.spec.containers[0].env[2].name
value: WEBHOOK_SECRET
- template: statefulset.yaml
equal:
path: spec.template.spec.containers[0].env[2].value
value: my-webhook-server-cert
2 changes: 1 addition & 1 deletion deploy/eck-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ webhook:
# port is the port that the validating webhook binds to.
port: 9443
# secret specifies the Kubernetes secret to be mounted into the path designated by the certsDir value to be used for webhook certificates.
secret: ""
certsSecret: ""

# hostNetwork allows a Pod to use the Node network namespace.
# This is required to allow for communication with the kube API when using some alternate CNIs in conjunction with webhook enabled.
Expand Down