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

chore: Simplify cert-manager certs and support existingIssuer #442

Closed
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
5 changes: 3 additions & 2 deletions keda/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ their default values.
| `additionalLabels` | object | `{}` | Custom labels to add into metadata |
| `affinity` | object | `{}` | [Affinity] for pod scheduling for both KEDA operator and Metrics API Server |
| `certificates.autoGenerated` | bool | `true` | Enables the self generation for KEDA TLS certificates inside KEDA operator |
| `certificates.certManager.caSecretName` | string | `"kedaorg-ca"` | Secret name where the CA is stored (generatedby cert-manager or user given) |
| `certificates.certManager.enabled` | bool | `false` | Enables Cert-manager for certificate management |
| `certificates.certManager.generateCA` | bool | `true` | Generates a self-signed CA with Cert-manager. If generateCA is false, the secret with the CA has to be annotated with `cert-manager.io/allow-direct-injection: "true"` |
| `certificates.certManager.existingIssuer.enabled` | bool | `false` | Use an existing cert-manager issuer |
| `certificates.certManager.existingIssuer.kind` | string | `"ClusterIssuer"` | Kind of the existing cert-manager issuer |
| `certificates.certManager.existingIssuer.name` | string | `""` | Name of the existing cert-manager issuer |
| `certificates.certManager.secretTemplate` | object | `{}` | Add labels/annotations to secrets created by Certificate resources [docs](https://cert-manager.io/docs/usage/certificate/#creating-certificate-resources) |
| `certificates.mountPath` | string | `"/certs"` | Path where KEDA TLS certificates are mounted |
| `certificates.secretName` | string | `"kedaorg-certs"` | Secret name to be mounted with KEDA TLS certificates |
Expand Down
7 changes: 3 additions & 4 deletions keda/templates/cert-manager/keda-issuer.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{- if .Values.certificates.certManager.enabled }}
{{- if and .Values.certificates.certManager.enabled (not .Values.certificates.certManager.existingIssuer.enabled) }}
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
Expand All @@ -7,6 +7,5 @@ metadata:
name: {{ .Values.operator.name }}-issuer
namespace: {{ .Release.Namespace }}
spec:
ca:
secretName: {{ .Values.certificates.certManager.caSecretName }}
{{- end }}
selfSigned: {}
{{- end }}
5 changes: 5 additions & 0 deletions keda/templates/cert-manager/keda-tls-certificate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ spec:
duration: 8760h0m0s # 1 year
renewBefore: 5840h0m0s # 8 months
issuerRef:
{{- if .Values.certificates.certManager.existingIssuer.enabled }}
name: {{ .Values.certificates.certManager.existingIssuer.name }}
kind: {{ .Values.certificates.certManager.existingIssuer.kind }}
{{- else }}
name: {{ .Values.operator.name }}-issuer
kind: Issuer
{{- end }}
group: cert-manager.io
{{- end }}
22 changes: 0 additions & 22 deletions keda/templates/cert-manager/self-ca.yaml

This file was deleted.

11 changes: 0 additions & 11 deletions keda/templates/cert-manager/self-issuer.yaml

This file was deleted.

6 changes: 1 addition & 5 deletions keda/templates/metrics-server/apiservice.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,7 @@ kind: APIService
metadata:
annotations:
{{- if .Values.certificates.certManager.enabled }}
{{- if .Values.certificates.certManager.generateCA }}
cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Values.operator.name }}-ca
{{- else }}
cert-manager.io/inject-ca-from-secret: {{ .Release.Namespace }}/{{ .Values.certificates.certManager.caSecretName }}
{{- end }}
cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Values.operator.name }}-tls-certificates
Copy link
Contributor Author

@mkilchhofer mkilchhofer May 15, 2023

Choose a reason for hiding this comment

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

This wasn't configured like its documented inside the cert-manager docs.

I'd suggest to follow 1:1 the docs of cert-manager:
https://cert-manager.io/docs/concepts/ca-injector/#injecting-ca-data-from-a-certificate-resource

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean?

Copy link
Contributor Author

@mkilchhofer mkilchhofer May 16, 2023

Choose a reason for hiding this comment

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

The annotation cert-manager.io/inject-ca-from has the value <namespace-name>/<certificate-name>

In the current implementation of the chart the value is <namespace-name>/<secret-name> which seems also working but it does not follow the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

you are right

{{- end }}
{{- if .Values.additionalAnnotations }}
{{- toYaml .Values.additionalAnnotations | nindent 4 }}
Expand Down
6 changes: 1 addition & 5 deletions keda/templates/webhooks/validatingconfiguration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ kind: ValidatingWebhookConfiguration
metadata:
annotations:
{{- if .Values.certificates.certManager.enabled }}
{{- if .Values.certificates.certManager.generateCA }}
cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Values.operator.name }}-ca
{{- else }}
cert-manager.io/inject-ca-from-secret: {{ .Release.Namespace }}/{{ .Values.certificates.certManager.caSecretName }}
{{- end }}
cert-manager.io/inject-ca-from: {{ .Release.Namespace }}/{{ .Values.operator.name }}-tls-certificates
{{- end }}
{{- if .Values.additionalAnnotations }}
{{- toYaml .Values.additionalAnnotations | nindent 4 }}
Expand Down
15 changes: 8 additions & 7 deletions keda/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -594,12 +594,6 @@ certificates:
certManager:
# -- Enables Cert-manager for certificate management
enabled: false
# -- Generates a self-signed CA with Cert-manager.
# If generateCA is false, the secret with the CA
# has to be annotated with `cert-manager.io/allow-direct-injection: "true"`
generateCA: true
# -- Secret name where the CA is stored (generatedby cert-manager or user given)
caSecretName: "kedaorg-ca"
# -- Add labels/annotations to secrets created by Certificate resources
# [docs](https://cert-manager.io/docs/usage/certificate/#creating-certificate-resources)
secretTemplate: {}
Expand All @@ -608,6 +602,13 @@ certificates:
# my-secret-annotation-2: "bar"
# labels:
# my-secret-label: foo
existingIssuer:
# -- Use an existing cert-manager issuer
enabled: false
# -- Kind of the existing cert-manager issuer
kind: ClusterIssuer
# -- Name of the existing cert-manager issuer
name: ""

permissions:
metricServer:
Expand All @@ -631,4 +632,4 @@ extraObjects: []
# provider: aws-eks

# -- Capability to turn on/off ASCII art in Helm installation notes
asciiArt: true
asciiArt: true