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

add option to reuse previously auto-generated cert #936

Merged
merged 8 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 1 addition & 1 deletion charts/opentelemetry-operator/Chart.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
apiVersion: v2
name: opentelemetry-operator
version: 0.42.2
version: 0.42.3
description: OpenTelemetry Operator Helm chart for Kubernetes
type: application
home: https://opentelemetry.io/
Expand Down
5 changes: 5 additions & 0 deletions charts/opentelemetry-operator/UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Upgrade guidelines

## <0.42.3 to 0.42.3

A type of flag `autoGenerateCert` has been changed, now it is an object with two attributes `enabled` and `recreate`.
If you previously set `autoGenerateCert` to `true` or `false` you have to set `autoGenerateCert.enabled` accordingly.
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved

## <0.35.0 to 0.35.0
OpenTelemetry Operator [0.82.0](https://github.com/open-telemetry/opentelemetry-operator/releases/tag/v0.82.0) includes a change that allows setting the management state of custom resources [PR 1888](https://github.com/open-telemetry/opentelemetry-operator/pull/1888). Since helm doesn't upgrade CRDs ([documented](https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-operator#upgrade-chart)) it is critical to manually update CRDs from chart `0.35.0` or above, possibly using [this procedure](https://github.com/open-telemetry/opentelemetry-helm-charts/issues/69#issuecomment-1567285625). If this step isn't taken existing otelcol CRs won't be reconciled by the operator.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
annotations:
cert-manager.io/inject-ca-from: default/example-opentelemetry-operator-serving-cert
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -88,7 +88,7 @@ metadata:
annotations:
cert-manager.io/inject-ca-from: default/example-opentelemetry-operator-serving-cert
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand All @@ -29,7 +29,7 @@ apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -253,7 +253,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand All @@ -271,7 +271,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand All @@ -25,7 +25,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: apps/v1
kind: Deployment
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ apiVersion: v1
kind: Service
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand All @@ -31,7 +31,7 @@ apiVersion: v1
kind: Service
metadata:
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
name: opentelemetry-operator
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
name: "example-opentelemetry-operator-cert-manager"
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
name: "example-opentelemetry-operator-metrics"
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down Expand Up @@ -43,7 +43,7 @@ metadata:
name: "example-opentelemetry-operator-webhook"
namespace: default
labels:
helm.sh/chart: opentelemetry-operator-0.42.2
helm.sh/chart: opentelemetry-operator-0.42.3
app.kubernetes.io/name: opentelemetry-operator
app.kubernetes.io/version: "0.88.0"
app.kubernetes.io/managed-by: Helm
Expand Down
36 changes: 36 additions & 0 deletions charts/opentelemetry-operator/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,39 @@ Create an ordered name of the MutatingWebhookConfiguration
{{- define "opentelemetry-operator.MutatingWebhookName" -}}
{{- printf "%s-%s" (.Values.admissionWebhooks.namePrefix | toString) (include "opentelemetry-operator.fullname" .) | trimPrefix "-" }}
{{- end }}

{{/*
Return certificate and CA for Webhooks.
It handles variants when a cert has to be generated by Helm,
a cert is loaded from an existing secret or is provided via `.Values`
*/}}
{{- define "opentelemetry-operator.WebhookCert" -}}
{{- $caCertEnc := "" }}
{{- $certCrtEnc := "" }}
{{- $certKeyEnc := "" }}
{{- if .Values.admissionWebhooks.autoGenerateCert.enabled }}
{{- $prevSecret := (lookup "v1" "Secret" .Release.Namespace (default (printf "%s-controller-manager-service-cert" (include "opentelemetry-operator.fullname" .)) .Values.admissionWebhooks.secretName )) }}
{{- if and (not .Values.admissionWebhooks.autoGenerateCert.recreate) $prevSecret }}
{{- $certCrtEnc = index $prevSecret "data" "tls.crt" }}
{{- $certKeyEnc = index $prevSecret "data" "tls.key" }}
{{- $caCertEnc = index $prevSecret "data" "ca.crt" }}
{{- if not $caCertEnc }}
{{- $prevHook := (lookup "admissionregistration.k8s.io/v1" "MutatingWebhookConfiguration" .Release.Namespace (print (include "opentelemetry-operator.MutatingWebhookName" . ) "-mutation")) }}
{{- $caCertEnc = (first $prevHook.webhooks).clientConfig.caBundle }}
{{- end }}
{{- else }}
{{- $altNames := list ( printf "%s-webhook.%s" (include "opentelemetry-operator.fullname" .) .Release.Namespace ) ( printf "%s-webhook.%s.svc" (include "opentelemetry-operator.fullname" .) .Release.Namespace ) -}}
{{- $ca := genCA "opentelemetry-operator-operator-ca" 365 }}
{{- $cert := genSignedCert (include "opentelemetry-operator.fullname" .) nil $altNames 365 $ca }}
{{- $certCrtEnc = b64enc $cert.Cert }}
{{- $certKeyEnc = b64enc $cert.Key }}
{{- $caCertEnc = b64enc $ca.Cert }}
{{- end }}
{{- else }}
{{- $certCrtEnc = b64enc .Values.admissionWebhooks.cert_file }}
Copy link
Contributor

@jinja2 jinja2 Nov 3, 2023

Choose a reason for hiding this comment

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

Not directly related to this PR, since this variable/logic is not newly introduced. Is the variable cert_file meant to be the file path to cert? If that's the intention, we should be using Files.Get to read the content. Same for the key/ca_file vars. We can fix this in a different issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've noticed it too. In values.yaml there are comments that it's a path but it's treated as data.

{{- $certKeyEnc = b64enc .Values.admissionWebhooks.key_file }}
{{- $caCertEnc = b64enc .Values.admissionWebhooks.ca_file }}
{{- end }}
{{- $result := dict "crt" $certCrtEnc "key" $certKeyEnc "ca" $caCertEnc }}
{{- $result | toYaml }}
{{- end }}
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{{- if and (.Values.admissionWebhooks.create) (not .Values.admissionWebhooks.certManager.enabled) }}
{{- $altNames := list ( printf "%s-webhook.%s" (include "opentelemetry-operator.fullname" .) .Release.Namespace ) ( printf "%s-webhook.%s.svc" (include "opentelemetry-operator.fullname" .) .Release.Namespace ) -}}
{{- $ca := genCA "opentelemetry-operator-operator-ca" 365 -}}
{{- $cert := genSignedCert (include "opentelemetry-operator.fullname" .) nil $altNames 365 $ca -}}
{{- $cert := fromYaml (include "opentelemetry-operator.WebhookCert" .) }}
{{- $caCertEnc := $cert.ca }}
{{- $certCrtEnc := $cert.crt }}
{{- $certKeyEnc := $cert.key }}
apiVersion: v1
kind: Secret
type: kubernetes.io/tls
Expand All @@ -21,13 +22,9 @@ metadata:
name: {{ default (printf "%s-controller-manager-service-cert" (include "opentelemetry-operator.fullname" .)) .Values.admissionWebhooks.secretName }}
namespace: {{ .Release.Namespace }}
data:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
tls.crt: {{ $cert.Cert | b64enc }}
tls.key: {{ $cert.Key | b64enc }}
{{- else }}
tls.crt: {{ .Values.admissionWebhooks.cert_file | b64enc }}
tls.key: {{ .Values.admissionWebhooks.key_file | b64enc }}
{{- end }}
tls.crt: {{ $certCrtEnc }}
tls.key: {{ $certKeyEnc }}
ca.crt: {{ $caCertEnc }}
---
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
Expand All @@ -40,11 +37,7 @@ webhooks:
- admissionReviewVersions:
- v1
clientConfig:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
caBundle: {{ $ca.Cert | b64enc }}
{{- else }}
caBundle: {{ .Values.admissionWebhooks.ca_file | b64enc }}
{{- end }}
caBundle: {{ $caCertEnc }}
service:
name: {{ template "opentelemetry-operator.fullname" . }}-webhook
namespace: {{ .Release.Namespace }}
Expand Down Expand Up @@ -75,11 +68,7 @@ webhooks:
- admissionReviewVersions:
- v1
clientConfig:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
caBundle: {{ $ca.Cert | b64enc }}
{{- else }}
caBundle: {{ .Values.admissionWebhooks.ca_file | b64enc }}
{{- end }}
caBundle: {{ $caCertEnc }}
service:
name: {{ template "opentelemetry-operator.fullname" . }}-webhook
namespace: {{ .Release.Namespace }}
Expand Down Expand Up @@ -110,11 +99,7 @@ webhooks:
- admissionReviewVersions:
- v1
clientConfig:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
caBundle: {{ $ca.Cert | b64enc }}
{{- else }}
caBundle: {{ .Values.admissionWebhooks.ca_file | b64enc }}
{{- end }}
caBundle: {{ $caCertEnc }}
service:
name: {{ template "opentelemetry-operator.fullname" . }}-webhook
namespace: {{ .Release.Namespace }}
Expand Down Expand Up @@ -156,11 +141,7 @@ webhooks:
- admissionReviewVersions:
- v1
clientConfig:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
caBundle: {{ $ca.Cert | b64enc }}
{{- else }}
caBundle: {{ .Values.admissionWebhooks.ca_file | b64enc }}
{{- end }}
caBundle: {{ $caCertEnc }}
service:
name: {{ template "opentelemetry-operator.fullname" . }}-webhook
namespace: {{ .Release.Namespace }}
Expand Down Expand Up @@ -191,11 +172,7 @@ webhooks:
- admissionReviewVersions:
- v1
clientConfig:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
caBundle: {{ $ca.Cert | b64enc }}
{{- else }}
caBundle: {{ .Values.admissionWebhooks.ca_file | b64enc }}
{{- end }}
caBundle: {{ $caCertEnc }}
service:
name: {{ template "opentelemetry-operator.fullname" . }}-webhook
namespace: {{ .Release.Namespace }}
Expand Down Expand Up @@ -225,11 +202,7 @@ webhooks:
- admissionReviewVersions:
- v1
clientConfig:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
caBundle: {{ $ca.Cert | b64enc }}
{{- else }}
caBundle: {{ .Values.admissionWebhooks.ca_file | b64enc }}
{{- end }}
caBundle: {{ $caCertEnc }}
service:
name: {{ template "opentelemetry-operator.fullname" . }}-webhook
namespace: {{ .Release.Namespace }}
Expand Down Expand Up @@ -260,11 +233,7 @@ webhooks:
- admissionReviewVersions:
- v1
clientConfig:
{{- if .Values.admissionWebhooks.autoGenerateCert }}
caBundle: {{ $ca.Cert | b64enc }}
{{- else }}
caBundle: {{ .Values.admissionWebhooks.ca_file | b64enc }}
{{- end }}
caBundle: {{ $caCertEnc }}
service:
name: {{ template "opentelemetry-operator.fullname" . }}-webhook
namespace: {{ .Release.Namespace }}
Expand Down
31 changes: 25 additions & 6 deletions charts/opentelemetry-operator/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1336,12 +1336,31 @@
}]
},
"autoGenerateCert": {
"type": "boolean",
"default": true,
"title": "The autoGenerateCert schema",
"examples": [
true
]
"type": "object",
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
"default": {},
"title": "The autoGenerateCert Schema",
"required": [
"enabled",
"recreate"
],
"properties": {
"enabled": {
"type": "boolean",
"default": true,
"title": "The enabled Schema",
"examples": [
true
]
},
"recreate": {
"type": "boolean",
"default": true,
"title": "The recreate Schema",
"examples": [
true
]
}
}
},
"secretAnnotations": {
"type": "object",
Expand Down
5 changes: 4 additions & 1 deletion charts/opentelemetry-operator/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,10 @@ admissionWebhooks:
## TLS Certificate Option 2: Use Helm to automatically generate self-signed certificate.
## certManager must be disabled and autoGenerateCert must be enabled.
## If true and certManager.enabled is false, Helm will automatically create a self-signd cert and secret for you.
autoGenerateCert: true
autoGenerateCert:
enabled: true
# If set to true, new webhook key/certificate is generated on helm upgrade.
recreate: true

## TLS Certificate Option 3: Use your own self-signed certificate.
## certManager and autoGenerateCert must be disabled and cert_file, key_file, and ca_file must be set.
Expand Down