feat: Merging metrics and service monitor services (#4356)

Signed-off-by: Rémy Jacquin <remy@remyj.fr>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

deploy/charts/external-secrets/templates/_helpers.tpl

@@ -66,6 +66,9 @@ app.kubernetes.io/managed-by: {{ .Release.Service }}
 {{- with .Values.commonLabels }}
 {{ toYaml . }}
 {{- end }}
+{{- if and ( .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" ) .Values.serviceMonitor.enabled }}
+app.kubernetes.io/metrics: "webhook"
+{{- end }}
 {{- end }}
 
 {{- define "external-secrets-webhook-metrics.labels" -}}
@@ -86,6 +89,9 @@ app.kubernetes.io/managed-by: {{ .Release.Service }}
 {{- with .Values.commonLabels }}
 {{ toYaml . }}
 {{- end }}
+{{- if and ( .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" ) .Values.serviceMonitor.enabled }}
+app.kubernetes.io/metrics: "cert-controller"
+{{- end }}
 {{- end }}
 
 {{- define "external-secrets-cert-controller-metrics.labels" -}}

deploy/charts/external-secrets/templates/cert-controller-service.yaml

@@ -1,11 +1,11 @@
-{{- if and .Values.certController.create .Values.certController.metrics.service.enabled (not .Values.webhook.certManager.enabled) }}
+{{- if and .Values.certController.create ( or .Values.certController.metrics.service.enabled ( and ( .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" ) .Values.serviceMonitor.enabled )) (not .Values.webhook.certManager.enabled) }}
 apiVersion: v1
 kind: Service
 metadata:
   name: {{ include "external-secrets.fullname" . }}-cert-controller-metrics
   namespace: {{ template "external-secrets.namespace" . }}
   labels:
-    {{- include "external-secrets.labels" . | nindent 4 }}
+    {{- include "external-secrets-cert-controller.labels" . | nindent 4 }}
   {{- with .Values.metrics.service.annotations }}
   annotations:
     {{- toYaml . | nindent 4 }}

deploy/charts/external-secrets/templates/service.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.metrics.service.enabled }}
+{{- if or (and ( .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" ) .Values.serviceMonitor.enabled) .Values.metrics.service.enabled -}}
 apiVersion: v1
 kind: Service
 metadata:

deploy/charts/external-secrets/templates/servicemonitor.yaml

@@ -1,26 +1,4 @@
 {{- if and ( .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" ) .Values.serviceMonitor.enabled -}}
-apiVersion: v1
-kind: Service
-metadata:
-  name: {{ include "external-secrets.fullname" . }}-metrics
-  namespace: {{ template "external-secrets.namespace" . }}
-  labels:
-    {{- include "external-secrets.labels" . | nindent 4 }}
-spec:
-  type: ClusterIP
-  {{- if .Values.service.ipFamilyPolicy }}
-  ipFamilyPolicy: {{ .Values.service.ipFamilyPolicy }}
-  {{- end }}
-  {{- if .Values.service.ipFamilies }}
-  ipFamilies: {{ .Values.service.ipFamilies | toYaml | nindent 2 }}
-  {{- end }}
-  ports:
-    - port: {{ .Values.metrics.service.port }}
-      protocol: TCP
-      name: metrics
-  selector:
-    {{- include "external-secrets.selectorLabels" . | nindent 4 }}
----
 apiVersion: "monitoring.coreos.com/v1"
 kind: ServiceMonitor
 metadata:
@@ -53,28 +31,6 @@ spec:
     {{- end }}
 ---
 {{- if .Values.webhook.create }}
-apiVersion: v1
-kind: Service
-metadata:
-  name: {{ include "external-secrets.fullname" . }}-webhook-metrics
-  namespace: {{ template "external-secrets.namespace" . }}
-  labels:
-    {{- include "external-secrets-webhook-metrics.labels" . | nindent 4 }}
-spec:
-  type: ClusterIP
-  {{- if .Values.service.ipFamilyPolicy }}
-  ipFamilyPolicy: {{ .Values.service.ipFamilyPolicy }}
-  {{- end }}
-  {{- if .Values.service.ipFamilies }}
-  ipFamilies: {{ .Values.service.ipFamilies | toYaml | nindent 2 }}
-  {{- end }}
-  ports:
-    - port: {{ .Values.webhook.metrics.service.port }}
-      protocol: TCP
-      name: metrics
-  selector:
-    {{- include "external-secrets-webhook.selectorLabels" . | nindent 4 }}
----
 apiVersion: "monitoring.coreos.com/v1"
 kind: ServiceMonitor
 metadata:
@@ -108,28 +64,6 @@ spec:
 {{- end }}
 ---
 {{- if .Values.certController.create }}
-apiVersion: v1
-kind: Service
-metadata:
-  name: {{ include "external-secrets.fullname" . }}-cert-controller-metrics
-  namespace: {{ template "external-secrets.namespace" . }}
-  labels:
-    {{- include "external-secrets-cert-controller-metrics.labels" . | nindent 4 }}
-spec:
-  type: ClusterIP
-  {{- if .Values.service.ipFamilyPolicy }}
-  ipFamilyPolicy: {{ .Values.service.ipFamilyPolicy }}
-  {{- end }}
-  {{- if .Values.service.ipFamilies }}
-  ipFamilies: {{ .Values.service.ipFamilies | toYaml | nindent 2 }}
-  {{- end }}
-  ports:
-    - port: {{ .Values.certController.metrics.listen.port }}
-      protocol: TCP
-      name: metrics
-  selector:
-    {{- include "external-secrets-cert-controller.selectorLabels" . | nindent 4 }}
----
 apiVersion: "monitoring.coreos.com/v1"
 kind: ServiceMonitor
 metadata:

deploy/charts/external-secrets/templates/webhook-service.yaml

@@ -26,7 +26,7 @@ spec:
     targetPort: {{ .Values.webhook.port }}
     protocol: TCP
     name: webhook
-  {{- if .Values.webhook.metrics.service.enabled }}
+  {{- if or .Values.webhook.metrics.service.enabled ( and ( .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" ) .Values.serviceMonitor.enabled ) }}
   - port: {{ .Values.webhook.metrics.service.port }}
     protocol: TCP
     targetPort: metrics

deploy/charts/external-secrets/tests/cert_controller_test.yaml

@@ -1,18 +1,25 @@
 suite: test cert controller deployment
 templates:
   - cert-controller-deployment.yaml
+  - cert-controller-service.yaml
 tests:
   - it: should match snapshot of default values
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - matchSnapshot: {}
   - it: should set imagePullPolicy to Always
     set:
       certController.image.pullPolicy: Always
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.containers[0].imagePullPolicy
           value: Always
   - it: should imagePullPolicy to be default value IfNotPresent
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.containers[0].imagePullPolicy
@@ -23,6 +30,8 @@ tests:
         runAsUser: 2000
       certController.securityContext:
         runAsUser: 3000
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.securityContext
@@ -43,6 +52,8 @@ tests:
   - it: should override hostNetwork
     set:
       certController.hostNetwork: true
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.hostNetwork
@@ -50,6 +61,8 @@ tests:
   - it: should override readinessProbe port
     set:
       certController.readinessProbe.port: 8082
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.containers[0].args[7]
@@ -57,6 +70,8 @@ tests:
   - it: should override metrics port
     set:
       certController.metrics.listen.port: 8888
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.containers[0].args[6]
@@ -66,6 +81,8 @@ tests:
       certController.image.repository: ghcr.io/external-secrets/external-secrets
       certController.image.tag: v0.9.8
       certController.image.flavour: ubi-boringssl
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.containers[0].image
@@ -74,7 +91,60 @@ tests:
     set:
       certController.image.repository: example.com/external-secrets/external-secrets
       certController.image.tag: v0.9.9-ubi
+    templates:
+      - cert-controller-deployment.yaml
     asserts:
       - equal:
           path: spec.template.spec.containers[0].image
           value: example.com/external-secrets/external-secrets:v0.9.9-ubi
+  - it: should render service without metrics label when metrics is enabled
+    set:
+      certController.metrics.service.enabled: true
+    templates:
+      - cert-controller-service.yaml
+    asserts:
+      - hasDocuments:
+          count: 1
+      - isNull:
+          path: metadata.labels["app.kubernetes.io/metrics"]
+  - it: should render service with metrics label when APIVersions are present and serviceMonitor is enabled
+    set:
+      serviceMonitor.enabled: true
+    capabilities:
+      apiVersions:
+        - "monitoring.coreos.com/v1"
+    templates:
+      - cert-controller-service.yaml
+    asserts:
+      - hasDocuments:
+          count: 1
+      - equal:
+          path: metadata.labels["app.kubernetes.io/metrics"]
+          value: "cert-controller"
+  - it: should not render service when APIVersions is not present but serviceMonitor is enabled
+    set:
+      serviceMonitor.enabled: true
+    templates:
+      - cert-controller-service.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: should not render service when APIVersions is present and serviceMonitor is disabled
+    set:
+      serviceMonitor.enabled: false
+    capabilities:
+      apiVersions:
+        - "monitoring.coreos.com/v1"
+    templates:
+      - cert-controller-service.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: should not render service when APIVersions is not present and serviceMonitor is disabled
+    set:
+      serviceMonitor.enabled: false
+    templates:
+      - cert-controller-service.yaml
+    asserts:
+      - hasDocuments:
+          count: 0

deploy/charts/external-secrets/tests/service_monitor_test.yaml

@@ -1,5 +1,5 @@
 suite: test service monitor
-templates: 
+templates:
   - servicemonitor.yaml
 tests:
   - it: should render service monitor when APIVersions is present and serviceMonitor is enabled
@@ -10,7 +10,7 @@ tests:
         - "monitoring.coreos.com/v1"
     asserts:
       - hasDocuments:
-          count: 6
+          count: 3
   - it: should not render service monitor when APIVersions is not present but serviceMonitor is enabled
     set:
       serviceMonitor.enabled: true
@@ -31,4 +31,4 @@ tests:
       serviceMonitor.enabled: false
     asserts:
       - hasDocuments:
-          count: 0
+          count: 0

deploy/charts/external-secrets/tests/service_test.yaml

@@ -0,0 +1,50 @@
+suite: test service
+templates:
+  - service.yaml
+tests:
+  - it: should render service when metrics are enabled
+    set:
+      metrics.service.enabled: true
+    templates:
+      - service.yaml
+    asserts:
+      - hasDocuments:
+          count: 1
+  - it: should render service when APIVersions is present and serviceMonitor is enabled
+    set:
+      serviceMonitor.enabled: true
+    capabilities:
+      apiVersions:
+        - "monitoring.coreos.com/v1"
+    templates:
+      - service.yaml
+    asserts:
+      - hasDocuments:
+          count: 1
+  - it: should not render service when APIVersions is not present but serviceMonitor is enabled
+    set:
+      serviceMonitor.enabled: true
+    templates:
+      - service.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: should not render service when APIVersions is present and serviceMonitor is disabled
+    set:
+      serviceMonitor.enabled: false
+    capabilities:
+      apiVersions:
+        - "monitoring.coreos.com/v1"
+    templates:
+      - service.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: should not render service when APIVersions is not present and serviceMonitor is disabled
+    set:
+      serviceMonitor.enabled: false
+    templates:
+      - service.yaml
+    asserts:
+      - hasDocuments:
+          count: 0

deploy/charts/external-secrets/tests/webhook_test.yaml

@@ -2,6 +2,7 @@ suite: test webhook deployment
 templates:
   - webhook-deployment.yaml
   - webhook-secret.yaml
+  - webhook-service.yaml
   - webhook-certificate.yaml
   - validatingwebhook.yaml
   - crds/externalsecret.yaml
@@ -203,3 +204,63 @@ tests:
       - equal:
           path: spec.template.spec.containers[0].image
           value: example.com/external-secrets/external-secrets:v0.9.9-ubi
+  - it: should expose metrics port when metrics are enabled
+    set:
+      webhook.metrics.service.enabled: true
+    templates:
+      - webhook-service.yaml
+    asserts:
+      - equal:
+          path: spec.ports[1].name
+          value: metrics
+  - it: should expose metrics port and metrics label when APIVersions is present and serviceMonitor is enabled
+    set:
+      serviceMonitor.enabled: true
+    capabilities:
+      apiVersions:
+        - "monitoring.coreos.com/v1"
+    templates:
+      - webhook-service.yaml
+    asserts:
+      - equal:
+          path: spec.ports[1].name
+          value: metrics
+      - equal:
+          path: metadata.labels["app.kubernetes.io/metrics"]
+          value: "webhook"
+  - it: should not expose metrics port nor metrics label when APIVersions is not present but serviceMonitor is enabled
+    set:
+      serviceMonitor.enabled: true
+    templates:
+      - webhook-service.yaml
+    asserts:
+      - lengthEqual:
+          path: spec.ports
+          count: 1
+      - isNull:
+          path: metadata.labels["app.kubernetes.io/metrics"]
+  - it: should not expose metrics port nor metrics label when APIVersions is present and serviceMonitor is disabled
+    set:
+      serviceMonitor.enabled: false
+    capabilities:
+      apiVersions:
+        - "monitoring.coreos.com/v1"
+    templates:
+      - webhook-service.yaml
+    asserts:
+      - lengthEqual:
+          path: spec.ports
+          count: 1
+      - isNull:
+          path: metadata.labels["app.kubernetes.io/metrics"]
+  - it: should not expose metrics port nor metrics annotation when APIVersions is not present and serviceMonitor is disabled
+    set:
+      serviceMonitor.enabled: false
+    templates:
+      - webhook-service.yaml
+    asserts:
+      - lengthEqual:
+          path: spec.ports
+          count: 1
+      - isNull:
+          path: metadata.labels["app.kubernetes.io/metrics"]