Enforce expected tags.monitoring=false behavior (#1927)

Chart.yaml

@@ -6,6 +6,9 @@ description: Helm chart to deploy the Astronomer Platform
 icon: https://www.astronomer.io/static/iconforLIGHTbackground.svg
 keywords:
   - astronomer
+
+# In order for tags to function, the referenced condition must not be set to True
+# anywhere else in the chart. Helm values take precedence over tag conditions.
 dependencies:
   # Platform components
   - name: astronomer
@@ -16,35 +19,37 @@ dependencies:
     condition: global.nginxEnabled
     tags:
       - platform
+
   # Monitoring stack
-  - name: grafana
-    condition: global.grafanaEnabled
+  - name: alertmanager
+    condition: global.alertmanagerEnabled
     tags:
       - monitoring
-  - name: prometheus
-    condition: global.prometheusEnabled
+  - name: grafana
+    condition: global.grafanaEnabled
     tags:
       - monitoring
-  - name: prometheus-postgres-exporter
-    condition: global.prometheusPostgresExporterEnabled
+  - name: kube-state
+    condition: global.kubeStateEnabled
     tags:
       - monitoring
-  - name: alertmanager
-    condition: global.alertmanagerEnabled
+  - name: prometheus-blackbox-exporter
+    condition: global.blackboxExporter.Enabled
     tags:
       - monitoring
-  - name: kube-state
-    condition: global.kubeStateEnabled
+  - name: prometheus-node-exporter
+    condition: global.nodeExporter.Enabled
     tags:
       - monitoring
-  - name: prometheus-node-exporter
-    condition: global.nodeExporterEnabled
+  - name: prometheus-postgres-exporter
+    condition: global.prometheusPostgresExporter.Enabled
     tags:
       - monitoring
-  - name: prometheus-blackbox-exporter
-    condition: global.blackboxExporterEnabled
+  - name: prometheus
+    condition: global.prometheusEnabled
     tags:
       - monitoring
+
   # Logging stack
   - name: elasticsearch
     condition: global.elasticsearchEnabled
@@ -58,27 +63,32 @@ dependencies:
     condition: global.fluentdEnabled
     tags:
       - logging
-  # In-cluster DB, not recommended
-  # for production
+
+  # In-cluster DB
+  # !! Not recommended for production !!
   - name: postgresql
     condition: global.postgresqlEnabled
     tags:
       - postgresql
+
   # Pgbouncer
   - name: pgbouncer
     condition: global.pgbouncer.enabled
     tags:
       - pgbouncer
+
   # Nats-server
   - name: nats
     condition: global.nats.enabled
     tags:
       - nats
+
   # Nats-streaming
   - name: stan
     condition: global.stan.enabled
     tags:
       - stan
+
   # external-es-proxy
   - name: external-es-proxy
     condition: global.customLogging.enabled

charts/postgresql/README.md

@@ -146,11 +146,6 @@ The following tables lists the configurable parameters of the PostgreSQL chart a
 | `service.clusterIP`                           | Static clusterIP or None for headless services                                                                         | `nil`                                                       |
 | `metrics.service.annotations`                 | Additional annotations for metrics exporter pod                                                                        | `{ prometheus.io/scrape: "true", prometheus.io/port: "9187"}` |
 | `metrics.service.loadBalancerIP`              | loadBalancerIP if redis metrics service type is `LoadBalancer`                                                         | `nil`                                                       |
-| `metrics.serviceMonitor.enabled`              | Set this to `true` to create ServiceMonitor for Prometheus operator                                                    | `false`                                                     |
-| `metrics.serviceMonitor.additionalLabels`     | Additional labels that can be used so ServiceMonitor will be discovered by Prometheus                                  | `{}`                                                        |
-| `metrics.serviceMonitor.namespace`            | Optional namespace in which to create ServiceMonitor                                                                   | `nil`                                                       |
-| `metrics.serviceMonitor.interval`             | Scrape interval. If not set, the Prometheus default scrape interval is used                                            | `nil`                                                       |
-| `metrics.serviceMonitor.scrapeTimeout`        | Scrape timeout. If not set, the Prometheus default scrape timeout is used                                              | `nil`                                                       |
 | `metrics.image.registry`                      | PostgreSQL Image registry                                                                                              | `docker.io`                                                 |
 | `metrics.image.repository`                    | PostgreSQL Image name                                                                                                  | `bitnami/postgres-exporter`                               |
 | `metrics.image.tag`                           | PostgreSQL Image tag                                                                                                   | `{TAG_NAME}`                                                    |

charts/postgresql/values-production.yaml

@@ -339,12 +339,6 @@ metrics:
       prometheus.io/scrape: "true"
       prometheus.io/port: "9187"
     loadBalancerIP:
-  serviceMonitor:
-    enabled: false
-    additionalLabels: {}
-    # namespace: monitoring
-    # interval: 30s
-    # scrapeTimeout: 10s
   image:
     registry: docker.io
     repository: bitnami/postgres-exporter

charts/postgresql/values.yaml

@@ -346,12 +346,6 @@ metrics:
       prometheus.io/scrape: "true"
       prometheus.io/port: "9187"
     loadBalancerIP:
-  serviceMonitor:
-    enabled: false
-    additionalLabels: {}
-    # namespace: monitoring
-    # interval: 30s
-    # scrapeTimeout: 10s
   image:
     registry: docker.io
     repository: bitnami/postgres-exporter

charts/prometheus-blackbox-exporter/README.md

@@ -72,18 +72,6 @@ The following table lists the configurable parameters of the Blackbox-Exporter c
 | `service.type`                            | type of service to create                                                        | `ClusterIP`                                                                  |
 | `service.port`                            | port for the blackbox http service                                               | `9115`                                                                       |
 | `service.externalIPs`                     | list of external ips                                                             | []                                                                           |
-| `serviceMonitor.enabled`                  | If true, a ServiceMonitor CRD is created for a prometheus operator               | `false`                                                                      |
-| `serviceMonitor.defaults.labels`          | Labels for prometheus operator                                                   | `{}`                                                                         |
-| `serviceMonitor.defaults.interval`        | Interval for prometheus operator endpoint                                        | `30s`                                                                        |
-| `serviceMonitor.defaults.scrapeTimeout`   | Scrape timeout for prometheus operator endpoint                                  | `30s`                                                                        |
-| `serviceMonitor.defaults.module`          | The module that blackbox will use if serviceMonitor is enabled                   | `http_2xx`                                                                   |
-| `serviceMonitor.targets.[]`               | List of targets to scrape                                                        | `[]`                                                                         |
-| `serviceMonitor.targets.[].name`          | Human readable name for the job. It will also appear in job labels in Prometheus | `example`                                                                    |
-| `serviceMonitor.targets.[].url`           | The URL that blackbox will scrape if serviceMonitor is enabled                   | `http://example.com/healthz`                                                 |
-| `serviceMonitor.targets.[].labels`        | See above in `serviceMonitor.defaults`                                           | `{{ serviceMonitor.defaults.labels }`                                        |
-| `serviceMonitor.targets.[].interval`      | See above in `serviceMonitor.defaults`                                           | `{{ serviceMonitor.defaults.interval }}`                                     |
-| `serviceMonitor.targets.[].scrapeTimeout` | See above in `serviceMonitor.defaults`                                           | `{{ serviceMonitor.defaults.scrateTimeout }}`                                |
-| `serviceMonitor.targets.[].module`        | See above in `serviceMonitor.defaults`                                           | `{{ serviceMonitor.defaults.module }}`                                       |
 | `strategy`                                | strategy used to replace old Pods with new ones                                  | `{"rollingUpdate":{"maxSurge":1,"maxUnavailable":0},"type":"RollingUpdate"}` |
 
 Specify each parameter using the `--set key=value[,key=value]` argument to `helm install`. For example,

charts/prometheus-node-exporter/README.md

@@ -34,11 +34,6 @@ The following table lists the configurable parameters of the Node Exporter chart
 | `priorityClassName`                   | Name of Priority Class to assign pods                                                                                         | `nil`                                            |
 | `endpoints`                           | list of addresses that have node exporter deployed outside of the cluster                                                     | `[]`                                             |
 | `hostNetwork`                         | Whether to expose the service to the host network                                                                             | `true`                                           |
-| `prometheus.monitor.enabled`          | Set this to `true` to create ServiceMonitor for Prometheus operator                                                           | `false`                                          |
-| `prometheus.monitor.additionalLabels` | Additional labels that can be used so ServiceMonitor will be discovered by Prometheus                                         | `{}`                                             |
-| `prometheus.monitor.namespace`        | namespace where servicemonitor resource should be created                                                                     | `the same namespace as prometheus node exporter` |
-| `prometheus.monitor.relabelings`      | Relabelings that should be applied on the ServerMonitor                                                                       | `{}` |
-| `prometheus.monitor.scrapeTimeout`    | Timeout after which the scrape is ended                                                                                       | `10s`                                            |
 | `configmaps`                          | Allow mounting additional configmaps.                                                                                         | `[]`                                             |
 | `namespaceOverride`                   | Override the deployment namespace                                                                                             | `""` (`Release.Namespace`)                       |
 | `updateStrategy`                      | Configure a custom update strategy for the daemonset                                                                          | `Rolling update with 1 max unavailable`          |

charts/prometheus-postgres-exporter/values.yaml

@@ -21,20 +21,6 @@ service:
   labels: {}
   annotations: {}
 
-serviceMonitor:
-  # When set true then use a ServiceMonitor to configure scraping
-  enabled: false
-  # Set the namespace the ServiceMonitor should be deployed
-  # namespace: monitoring
-  # Set how frequently Prometheus should scrape
-  # interval: 30s
-  # Set path to cloudwatch-exporter telemetry-path
-  # telemetryPath: /metrics
-  # Set labels for the ServiceMonitor, use this to define your scrape label for Prometheus Operator
-  # labels:
-  # Set timeout for scrape
-  # timeout: 10s
-
 resources:
   limits:
     cpu: 100m

tests/chart_tests/test_argo_sync-wave.py

@@ -22,7 +22,7 @@
     # "charts/postgresql/templates/extended-config-configmap.yaml", # esoteric configurations
     # "charts/postgresql/templates/initialization-configmap.yaml", # esoteric configurations
     "charts/postgresql/templates/networkpolicy.yaml",
-    "charts/postgresql/templates/secrets.yaml",
+    "charts/postgresql/templates/secret.yaml",
     "charts/postgresql/templates/serviceaccount.yaml",
     "charts/postgresql/templates/statefulset.yaml",
     "charts/postgresql/templates/svc.yaml",

tests/chart_tests/test_tags.py

@@ -0,0 +1,68 @@
+from tests.chart_tests.helm_template_generator import render_chart
+import pytest
+import tests.chart_tests as chart_tests
+import subprocess
+from pathlib import Path
+
+
+component_paths = [
+    "charts/alertmanager/templates",
+    "charts/grafana/templates",
+    "charts/kube-state/templates",
+    "charts/prometheus-blackbox-exporter/templates",
+    "charts/prometheus-node-exporter/templates",
+    "charts/prometheus-postgres-exporter/templates",
+    "charts/prometheus/templates",
+]
+
+# Some charts have configs that are hard to test when parametrizing with get_all_features()
+# eg: password vs passwordSecret
+edge_cases = [
+    "charts/prometheus-postgres-exporter/templates/secret.yaml",
+]
+
+show_only = [
+    str(y)
+    for x in component_paths
+    for y in list(Path(x).glob("*.yaml"))
+    if not y.name.startswith("_") and str(y) not in edge_cases
+]
+
+chart_values = chart_tests.get_all_features()
+
+
+# We use kube_version=1.24 here because 1.25 removes psp, and we need to test psp. Once 1.24
+# is deprecated we can set this to something higher, and will likely have to solve similar
+# problems for newer api differences.
+@pytest.mark.parametrize("template", show_only)
+def test_tags_monitoring_enabled(
+    template, chart_values=chart_values, kube_version="1.24.0"
+):
+    """Test that when monitoring is disabled, the monitoring components are not present."""
+    chart_values["tags"] = {"monitoring": True}
+    docs = render_chart(
+        kube_version=kube_version, values=chart_values, show_only=template
+    )
+
+    assert len(docs) >= 1
+    assert (
+        template.split("/")[-1]
+        .split("-")[-1]
+        .removesuffix(".yaml")
+        .replace("psp", "podsecuritypolicy")
+        in docs[0]["kind"].lower()
+    )
+
+
+# We use kube_version=1.24 here because 1.25 removes psp, and we need to test psp. Once 1.24
+# is deprecated we can set this to something higher, and will likely have to solve similar
+# problems for newer api differences.
+@pytest.mark.parametrize("template", show_only)
+def test_tags_monitoring_disabled(
+    template, chart_values=chart_values, kube_version="1.24.0"
+):
+    """Test that when monitoring is disabled, the monitoring components are not present."""
+    chart_values["tags"] = {"monitoring": False}
+
+    with pytest.raises(subprocess.CalledProcessError):
+        render_chart(kube_version=kube_version, values=chart_values, show_only=template)

tests/enable_all_features.yaml

@@ -15,3 +15,8 @@ global:
   pspEnabled: true
   taskUsageMetricsEnabled: True
   veleroEnabled: true
+prometheus-node-exporter:
+  rbac:
+    create: true
+  endpoints:
+    - example.com