From efe3cd655febb5f19babf07d6d2ed4550a698055 Mon Sep 17 00:00:00 2001 From: Pete Wall Date: Thu, 31 Oct 2024 11:16:09 -0500 Subject: [PATCH] Fix typo, improve destination validations Signed-off-by: Pete Wall --- charts/k8s-monitoring/Makefile | 2 +- .../invalid-destination.schema.json | 8 ++ .../schema-mods/destination-list.json | 1 + charts/k8s-monitoring/templates/_helpers.tpl | 17 +++- .../collectors/_collector_validations.tpl | 2 +- .../destinations/_destination_validations.tpl | 9 ++- .../k8s-monitoring/templates/validations.yaml | 3 +- .../tests/cluster_events_test.yaml | 4 +- .../tests/destination_validations_test.yaml | 77 +++++++++++++++++++ .../tests/validations_test.yaml | 5 +- charts/k8s-monitoring/values.schema.json | 11 +++ 11 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 charts/k8s-monitoring/schema-mods/definitions/invalid-destination.schema.json create mode 100644 charts/k8s-monitoring/tests/destination_validations_test.yaml diff --git a/charts/k8s-monitoring/Makefile b/charts/k8s-monitoring/Makefile index 8dc0cea5c..e618639bb 100644 --- a/charts/k8s-monitoring/Makefile +++ b/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 diff --git a/charts/k8s-monitoring/schema-mods/definitions/invalid-destination.schema.json b/charts/k8s-monitoring/schema-mods/definitions/invalid-destination.schema.json new file mode 100644 index 000000000..7aa0a34e5 --- /dev/null +++ b/charts/k8s-monitoring/schema-mods/definitions/invalid-destination.schema.json @@ -0,0 +1,8 @@ +{ + "type": "object", + "properties": { + "type": { + "type": "string" + } + } +} diff --git a/charts/k8s-monitoring/schema-mods/destination-list.json b/charts/k8s-monitoring/schema-mods/destination-list.json index a51cf775d..aa040d757 100644 --- a/charts/k8s-monitoring/schema-mods/destination-list.json +++ b/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"}, diff --git a/charts/k8s-monitoring/templates/_helpers.tpl b/charts/k8s-monitoring/templates/_helpers.tpl index adf1dd9c3..3437dfba6 100644 --- a/charts/k8s-monitoring/templates/_helpers.tpl +++ b/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 := . }} diff --git a/charts/k8s-monitoring/templates/collectors/_collector_validations.tpl b/charts/k8s-monitoring/templates/collectors/_collector_validations.tpl index 149d6aa17..e333b1335 100644 --- a/charts/k8s-monitoring/templates/collectors/_collector_validations.tpl +++ b/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" }} diff --git a/charts/k8s-monitoring/templates/destinations/_destination_validations.tpl b/charts/k8s-monitoring/templates/destinations/_destination_validations.tpl index bf9910375..5cc616aaf 100644 --- a/charts/k8s-monitoring/templates/destinations/_destination_validations.tpl +++ b/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 }} - diff --git a/charts/k8s-monitoring/templates/validations.yaml b/charts/k8s-monitoring/templates/validations.yaml index c373ff122..d80adbabc 100644 --- a/charts/k8s-monitoring/templates/validations.yaml +++ b/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) }} diff --git a/charts/k8s-monitoring/tests/cluster_events_test.yaml b/charts/k8s-monitoring/tests/cluster_events_test.yaml index 2923971f7..21fdd9e37 100644 --- a/charts/k8s-monitoring/tests/cluster_events_test.yaml +++ b/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: diff --git a/charts/k8s-monitoring/tests/destination_validations_test.yaml b/charts/k8s-monitoring/tests/destination_validations_test.yaml new file mode 100644 index 000000000..276647b8b --- /dev/null +++ b/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: {} diff --git a/charts/k8s-monitoring/tests/validations_test.yaml b/charts/k8s-monitoring/tests/validations_test.yaml index 12364355d..64d95ac80 100644 --- a/charts/k8s-monitoring/tests/validations_test.yaml +++ b/charts/k8s-monitoring/tests/validations_test.yaml @@ -7,11 +7,12 @@ 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: @@ -19,7 +20,7 @@ tests: 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 diff --git a/charts/k8s-monitoring/values.schema.json b/charts/k8s-monitoring/values.schema.json index 47d66b334..4178307a7 100644 --- a/charts/k8s-monitoring/values.schema.json +++ b/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" },