Fix typo, improve destination validations

Signed-off-by: Pete Wall <pete.wall@grafana.com>

charts/k8s-monitoring/Makefile

@@ -148,7 +148,7 @@ lint-helm: build
 	helm lint .
 	ct lint --config .ct.yaml --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --charts .
 
-unittest: build
+unittest: values.schema.json templates/destinations/_destination_types.tpl
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/k8s-monitoring/schema-mods/definitions/invalid-destination.schema.json

@@ -0,0 +1,8 @@
+{
+    "type": "object",
+    "properties": {
+        "type": {
+          "type": "string"
+        }
+    }
+}

charts/k8s-monitoring/schema-mods/destination-list.json

@@ -4,6 +4,7 @@
       "type": "array",
       "items": {
         "anyOf": [
+          { "$ref": "#/definitions/invalid-destination"},
           { "$ref": "#/definitions/loki-destination"},
           { "$ref": "#/definitions/otlp-destination"},
           { "$ref": "#/definitions/prometheus-destination"},

charts/k8s-monitoring/templates/_helpers.tpl

@@ -1,9 +1,9 @@
 {{ define "helper.k8s_name" }}
-{{- . | lower -}}
+{{- . | lower | replace " " "-" -}}
 {{ end }}
 
 {{ define "helper.alloy_name" }}
-{{- . | lower | replace "-" "_" -}}
+{{- . | lower | replace " " "-" | replace "-" "_" -}}
 {{ end }}
 
 {{- define "helper.fullname" -}}
@@ -32,6 +32,19 @@
 {{- end }}
 {{- end }}
 
+{{- define "english_list_or" }}
+{{- if eq (len .) 0 }}
+{{- else if eq (len .) 1 }}
+{{- index . 0 }}
+{{- else if eq (len .) 2 }}
+{{- index . 0 }} and {{ index . 1 }}
+{{- else }}
+{{- $last := index . (sub (len .) 1) }}
+{{- $rest := slice . 0 (sub (len .) 1) }}
+{{- join ", " $rest }}, or {{ $last }}
+{{- end }}
+{{- end }}
+
 {{- define "label_list" }}
 {{- $labels := list }}
 {{- range $key, $value := . }}

charts/k8s-monitoring/templates/collectors/_collector_validations.tpl

@@ -62,7 +62,7 @@
 {{- if (index .Values .collectorName).enabled }}
   {{- if (index .Values .collectorName).liveDebugging.enabled }}
     {{- if not (eq (index .Values .collectorName).alloy.stabilityLevel "experimental") }}
-      {{- $msg := list "" "The live debugging feature requires Alloy to use the \"experiemenal\" stability level. Please set:" }}
+      {{- $msg := list "" "The live debugging feature requires Alloy to use the \"experimental\" stability level. Please set:" }}
       {{- $msg = append $msg (printf "%s:" .collectorName ) }}
       {{- $msg = append $msg "  alloy:" }}
       {{- $msg = append $msg "    stabilityLevel: experimental" }}

charts/k8s-monitoring/templates/destinations/_destination_validations.tpl

@@ -6,14 +6,17 @@
       {{ fail (printf "\nDestination #%d does not have a name.\nPlease set:\ndestinations:\n  - name: my-destination-name" $i) }}
     {{- end }}
 
+    {{- if (regexFind "[^-_ a-zA-Z0-9]" $destination.name) }}
+      {{ fail (printf "\nDestination #%d (%s) has invalid characters in its name.\nPlease only use alphanumeric, underscores, dashes, or spaces" $i $destination.name) }}
+    {{- end }}
+
     {{- $types := (include "destinations.types" . ) | fromYamlArray }}
     {{- if not $destination.type }}
-      {{ fail (printf "\nDestination \"%s\" does not have a type.\nPlease set:\ndestinations:\n  - name: %s\n    type: %s" $destination.name $destination.name ($types | join "|")) }}
+      {{ fail (printf "\nDestination #%d (%s) does not have a type.\nPlease set:\ndestinations:\n  - name: %s\n    type: %s" $i $destination.name $destination.name (include "english_list_or" $types)) }}
     {{- end }}
 
     {{- if not (has $destination.type $types) }}
-      {{ fail (printf "\nDestination \"%s\" is using an unknown type (%s).\nPlease set:\ndestinations:\n  - name: %s\n    type: \"[%s]\"" $destination.name $destination.type $destination.name ($types | join "|")) }}
+      {{ fail (printf "\nDestination #%d (%s) is using an unknown type (%s).\nPlease set:\ndestinations:\n  - name: %s\n    type: \"[%s]\"" $i $destination.name $destination.type $destination.name (include "english_list_or" $types)) }}
     {{- end }}
   {{- end }}
 {{- end }}
-

charts/k8s-monitoring/templates/validations.yaml

@@ -1,13 +1,14 @@
 {{- include "validations.cluster_name" . }}
 {{- include "validations.platform" . }}
 
+{{- include "destinations.validate" . -}}
+
 {{- /* Feature Validations*/}}
 {{- include "validations.features_enabled" . }}
 {{- range $feature := ((include "features.list" .) | fromYamlArray) }}
   {{- include (printf "features.%s.validate" $feature) (dict "Values" $.Values) }}
 {{- end }}
 
-{{- include "destinations.validate" . -}}
 
 {{- include "collectors.validate.featuresEnabled" . }}
 {{- range $collectorName := ((include "collectors.list.enabled" .) | fromYamlArray) }}

charts/k8s-monitoring/tests/cluster_events_test.yaml

@@ -13,7 +13,7 @@ tests:
     asserts:
       - failedTemplate:
           errorMessage: |-
-            execution error at (k8s-monitoring/templates/validations.yaml:7:6): 
+            execution error at (k8s-monitoring/templates/validations.yaml:9:6): 
             No destinations found that can accept logs from Kubernetes Cluster events
             Please add a destination with logs support.
             See https://github.com/grafana/k8s-monitoring-helm/blob/main/charts/k8s-monitoring/docs/destinations/README.md for more details.
@@ -31,7 +31,7 @@ tests:
     asserts:
       - failedTemplate:
           errorMessage: |-
-            execution error at (k8s-monitoring/templates/validations.yaml:7:6): 
+            execution error at (k8s-monitoring/templates/validations.yaml:9:6): 
             The Kubernetes Cluster events feature requires the use of the alloy-singleton collector.
             
             Please enable it by setting:

charts/k8s-monitoring/tests/destination_validations_test.yaml

@@ -0,0 +1,77 @@
+# yamllint disable rule:document-start rule:line-length rule:trailing-spaces
+suite: Test destination validations
+templates:
+  - validations.yaml
+tests:
+  - it: asks you to set the name of a destination
+    set:
+      cluster: {name: test-cluster}
+      clusterMetrics: {enabled: true}
+      destinations:
+        - type: prometheus
+    asserts:
+      - failedTemplate:
+          errorMessage: |-
+            execution error at (k8s-monitoring/templates/validations.yaml:4:4):
+            Destination #0 does not have a name.
+            Please set:
+            destinations:
+              - name: my-destination-name
+
+  - it: asks you to set a valid name for the destination
+    set:
+      cluster: {name: test-cluster}
+      clusterMetrics: {enabled: true}
+      destinations:
+        - name: prøm3thüs!
+          type: prometheus
+    asserts:
+      - failedTemplate:
+          errorMessage: |-
+            execution error at (k8s-monitoring/templates/validations.yaml:4:4):
+            Destination #0 (prøm3thüs!) has invalid characters in its name.
+            Please only use alphanumeric, underscores, dashes, or spaces
+
+  - it: requires a destination type
+    set:
+      cluster: {name: test-cluster}
+      clusterMetrics: {enabled: true}
+      destinations:
+        - name: a destination with no type
+    asserts:
+      - failedTemplate:
+          errorMessage: |-
+            execution error at (k8s-monitoring/templates/validations.yaml:4:4):
+            Destination #0 (a destination with no type) does not have a type.
+            Please set:
+            destinations:
+              - name: a destination with no type
+                type: loki, otlp, prometheus, or pyroscope
+
+  - it: validates the destination type
+    set:
+      cluster: {name: test-cluster}
+      clusterMetrics: {enabled: true}
+      destinations:
+        - name: a destination with an invalid type
+          type: invalidType
+    asserts:
+      - failedTemplate:
+          errorMessage: |-
+            execution error at (k8s-monitoring/templates/validations.yaml:4:4):
+            Destination #0 (a destination with an invalid type) is using an unknown type (invalidType).
+            Please set:
+            destinations:
+              - name: a destination with an invalid type
+                type: "[loki, otlp, prometheus, or pyroscope]"
+
+  - it: allows destination names with alphanumeric, underscores, dashes, and spaces
+    set:
+      cluster: {name: test-cluster}
+      clusterMetrics: {enabled: true}
+      alloy-metrics: {enabled: true}
+      destinations:
+        - name: This is my _Prometheus_ destination-1
+          type: prometheus
+    asserts:
+      - notFailedTemplate: {}

charts/k8s-monitoring/tests/validations_test.yaml

@@ -7,19 +7,20 @@ tests:
     asserts:
       - failedTemplate:
           errorMessage: |-
-            execution error at (k8s-monitoring/templates/validations.yaml:1:4): 
+            execution error at (k8s-monitoring/templates/validations.yaml:1:4):
             A Cluster name is required!
             Please set:
             cluster:
               name: my-cluster-name
+
   - it: asks you to set a feature
     set:
       cluster:
         name: test-cluster
     asserts:
       - failedTemplate:
           errorMessage: |-
-            execution error at (k8s-monitoring/templates/validations.yaml:5:4): 
+            execution error at (k8s-monitoring/templates/validations.yaml:7:4):
             No features are enabled. Please choose a feature to start monitoring. For example:
             clusterMetrics:
               enabled: true

charts/k8s-monitoring/values.schema.json

@@ -1013,6 +1013,14 @@
         }
     },
     "definitions": {
+        "invalid-destination": {
+            "type": "object",
+            "properties": {
+                "type": {
+                    "type": "string"
+                }
+            }
+        },
         "loki-destination": {
             "type": "object",
             "properties": {
@@ -1669,6 +1677,9 @@
             "type": "array",
             "items": {
                 "anyOf": [
+                    {
+                        "$ref": "#/definitions/invalid-destination"
+                    },
                     {
                         "$ref": "#/definitions/loki-destination"
                     },