-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Description
Describe the bug:
I've upgraded the installation of the Cert-Manager Chart from v1.12.4 to v1.13.0, and I've noticed the creation of an empty ConfigMap. Here is the output of the Helm Diff plugin :
+ # Source: cert-manager/charts/cert-manager/templates/controller-config.yaml
+ apiVersion: v1
+ kind: ConfigMap
+ metadata:
+ name: cert-manager
+ namespace: cert-manager
+ labels:
+ app: cert-manager
+ app.kubernetes.io/name: cert-manager
+ app.kubernetes.io/instance: cert-manager
+ app.kubernetes.io/component: "controller"
+ app.kubernetes.io/version: "v1.13.0"
+ app.kubernetes.io/managed-by: Helm
+ helm.sh/chart: cert-manager-v1.13.0
+ data:
Apparently, it is related to this commit that allowed the "ability to start controller with config file".
Expected behavior:
In my opinion, the ConfigMap should not be created if the .Values.config
map is empty, which is not currently the case because the ConfigMap creation is not scoped within the {{- if .Values.config -}}
block.
Moreover, we can also see this block of code in the controller-config.yaml
file that checks the existence of the .Values.config.apiVersion
value :
{{- if not .Values.config.apiVersion -}}
{{- fail "config.apiVersion must be set" -}}
{{- end -}}
In my opinion, it should be replaced with required
rather than fail
because fail
should be used to check conformity of a value, rather than existence that should be checked with required
.
Or we could completely remove this check and add some abstraction by already including these two lines by default in the ConfigMap data :
config.yaml: |
apiVersion: controller.config.cert-manager.io/v1alpha1
kind: ControllerConfiguration
{{ .Values.config | toYaml | nindent 4 }}
Steps to reproduce the bug:
Install the v1.13.0 Chart and let .Values.config
be empty.
Anything else we need to know?:
I'd be glad to implement the PR related to this issue :)
Environment details::
- Kubernetes version: 1.25
- Cloud-provider/provisioner: GCP
- cert-manager version: 1.12.4 -> 1.13.0
- Install method: Helm
/kind bug