Repository navigation
fix(chart): render namespace scoping and netobs keys into the Edge config #18
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| name: CI | ||
|
|
||
| on: | ||
| pull_request: | ||
| push: | ||
| branches: [main] | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: azure/setup-helm@v4 | ||
| with: | ||
| version: v3.18.0 | ||
| - name: Install helm-unittest | ||
| run: helm plugin install https://github.com/helm-unittest/helm-unittest --version v1.2.1 | ||
| - name: Install yq | ||
| run: | | ||
| sudo wget -qO /usr/local/bin/yq https://github.com/mikefarah/yq/releases/download/v4.44.3/yq_linux_amd64 | ||
| sudo chmod +x /usr/local/bin/yq | ||
| - run: helm lint . | ||
| - run: helm unittest . | ||
| - run: tests/render/run.sh | ||
| - run: tests/render/harness_test.sh |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,3 +31,6 @@ docs/ | |
| .idea/ | ||
| *.tmproj | ||
| .vscode/ | ||
| # Tests and CI helpers are not chart content | ||
| tests/ | ||
| ci/ | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Values for tests/e2e/kind.sh: a local kind cluster with no backend. | ||
| image: | ||
| pullPolicy: IfNotPresent | ||
|
|
||
| config: | ||
| # Publishing needs a graph URL and API key; the e2e only needs the Edge to | ||
| # accept its config and start. | ||
| enablePublishing: false | ||
| kube: | ||
| namespaceFilter: | ||
| mode: allow | ||
| namespaces: [default] | ||
| netobs: | ||
| excludeNamespaces: [x] | ||
| edgeExistenceTtl: 1h |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,39 @@ data: | |
| {{ $resource | quote }} | ||
| {{- end }} | ||
| ] | ||
| {{- $nfRaw := .Values.config.kube.namespaceFilter }} | ||
| {{- if and (hasKey .Values.config.kube "namespaceFilter") (not (kindIs "invalid" $nfRaw)) (not (kindIs "map" $nfRaw)) }} | ||
| {{- fail "config.kube.namespaceFilter must be a map" }} | ||
| {{- end }} | ||
| {{- range $key, $_ := $nfRaw }} | ||
| {{- if not (has $key (list "mode" "namespaces")) }} | ||
| {{- fail (printf "config.kube.namespaceFilter has unknown key %q; supported keys are mode and namespaces" $key) }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- $nf := dict }} | ||
| {{- $_ := include "nofire-edge.typedPick" (dict "src" $nfRaw "path" "config.kube.namespaceFilter" "out" $nf "spec" (dict "mode" "string" "namespaces" "strlist")) }} | ||
| {{- if $nf }} | ||
| {{- /* Mirror the Edge's validation so a bad filter fails at install, not as a crashloop. */}} | ||
| {{- $mode := get $nf "mode" | default "" }} | ||
| {{- $namespaces := get $nf "namespaces" | default list }} | ||
| {{- if has $mode (list "" "none") }} | ||
| {{- if $namespaces }} | ||
| {{- fail (printf "config.kube.namespaceFilter.namespaces is set but mode %q disables filtering; set config.kube.namespaceFilter.mode to 'allow' or 'deny'" $mode) }} | ||
| {{- end }} | ||
| {{- else if has $mode (list "allow" "deny") }} | ||
| {{- if not $namespaces }} | ||
| {{- fail (printf "config.kube.namespaceFilter.mode is %q but config.kube.namespaceFilter.namespaces is empty" $mode) }} | ||
| {{- end }} | ||
| {{- range $ns := $namespaces }} | ||
| {{- if not (and (le (len $ns) 63) (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$" $ns)) }} | ||
| {{- fail (printf "config.kube.namespaceFilter.namespaces entry %q is not a valid namespace name (a lowercase DNS-1123 label of at most 63 characters)" $ns) }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- else }} | ||
| {{- fail (printf "config.kube.namespaceFilter.mode must be one of '', 'none', 'allow', 'deny' (got %q)" $mode) }} | ||
| {{- end }}, | ||
| "namespaceFilter": {{ $nf | toJson }} | ||
| {{- end }} | ||
| }, | ||
| "enableCausalAnalysis": {{ .Values.config.enableCausalAnalysis }}, | ||
| "causal": { | ||
|
|
@@ -38,38 +71,7 @@ data: | |
| "sampleRate": {{ .Values.config.sentry.sampleRate | default 1.0 }}, | ||
| "tracesSampleRate": {{ .Values.config.sentry.tracesSampleRate | default 0.0 }} | ||
| }, | ||
| "services": {{ include "nofire-edge.servicesJson" . }}, | ||
|
There was a problem hiding this comment. nit: The description says the generated Failure: none at runtime, but Evidence:
Collaborator
Author
There was a problem hiding this comment. Agreed, that was inaccurate. I updated the description: it is semantically identical ( |
||
| "enablePublishing": {{ .Values.config.enablePublishing }}, | ||
| "publisher": { | ||
| "apiKey": {{ if .Values.config.publisher.apiKeySecret.enabled }}"{NOFIRE_API_KEY}"{{ else }}{{ .Values.config.publisher.apiKey | default "" | quote }}{{ end }}, | ||
|
|
@@ -92,22 +94,39 @@ data: | |
| "prometheusUrl": {{ .Values.config.netobs.prometheusUrl | default "" | quote }}, | ||
| "source": {{ .Values.config.netobs.source | default "" | quote }}, | ||
| "prometheusTimeout": {{ .Values.config.netobs.prometheusTimeout | default "10s" | quote }} | ||
| {{- if and (hasKey .Values.config.netobs "edgeExistenceTtl") (not (kindIs "invalid" .Values.config.netobs.edgeExistenceTtl)) }} | ||
| {{- if not (kindIs "string" .Values.config.netobs.edgeExistenceTtl) }} | ||
|
There was a problem hiding this comment. nit: Failure: Evidence: ran Fix: add
Collaborator
Author
There was a problem hiding this comment. Fixed in 206d63b. |
||
| {{- fail (printf "config.netobs.edgeExistenceTtl must be a duration string like 2h (got %v)" .Values.config.netobs.edgeExistenceTtl) }} | ||
| {{- end }} | ||
| {{- /* Go duration syntax: optional sign, then number+unit pairs ("1h30m", "1.5h"), or plain "0". */}} | ||
| {{- if not (regexMatch "^([-+]?(([0-9]+[.]?[0-9]*|[.][0-9]+)(ns|us|µs|μs|ms|s|m|h))+|[-+]?0)$" .Values.config.netobs.edgeExistenceTtl) }} | ||
| {{- fail (printf "config.netobs.edgeExistenceTtl %q is not a valid duration; use Go syntax such as 2h, 90m or 1h30m" .Values.config.netobs.edgeExistenceTtl) }} | ||
| {{- end }}, | ||
| "edgeExistenceTtl": {{ .Values.config.netobs.edgeExistenceTtl | quote }} | ||
| {{- end }} | ||
| {{- $netobsEx := dict }} | ||
| {{- $_ := include "nofire-edge.typedPick" (dict "src" .Values.config.netobs "path" "config.netobs" "out" $netobsEx "spec" (dict "excludeNamespaces" "strlist")) }} | ||
| {{- if hasKey $netobsEx "excludeNamespaces" }}, | ||
| "excludeNamespaces": {{ $netobsEx.excludeNamespaces | toJson }} | ||
| {{- end }} | ||
| }, | ||
| {{- $cmc := .Values.config.configMapCapture | default dict }} | ||
| "configMapCapture": { | ||
| "clearText": {{ $cmc.clearText | default false }}, | ||
| "captureCap": {{ $cmc.captureCap | default 4096 }}, | ||
| "redactKeyPatterns": [ | ||
| {{- range $index, $pattern := $cmc.redactKeyPatterns }} | ||
| {{- if $index }},{{ end }} | ||
| {{ $pattern | quote }} | ||
| {{- end }} | ||
| ] | ||
| }, | ||
| {{- $envc := .Values.config.envCapture | default dict }} | ||
| "envCapture": { | ||
| "clearText": {{ $envc.clearText | default false }}, | ||
| "captureCap": {{ $envc.captureCap | default 4096 }}, | ||
| "redactKeyPatterns": [ | ||
| {{- range $index, $pattern := $envc.redactKeyPatterns }} | ||
| {{- if $index }},{{ end }} | ||
| {{ $pattern | quote }} | ||
| {{- end }} | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| suite: configmap | ||
| templates: | ||
| - templates/configmap.yaml | ||
| tests: | ||
| - it: renders a config.json ConfigMap with defaults | ||
| asserts: | ||
| - isKind: | ||
| of: ConfigMap | ||
| - matchRegex: | ||
| path: metadata.name | ||
| pattern: -config$ | ||
| - exists: | ||
| path: data["config.json"] | ||
|
|
||
| - it: fails when namespaces are set without a mode | ||
| set: | ||
| config.kube.namespaceFilter.namespaces: [a] | ||
| asserts: | ||
| - failedTemplate: | ||
| errorPattern: disables filtering | ||
|
|
||
| - it: fails when mode is allow with no namespaces | ||
| set: | ||
| config.kube.namespaceFilter.mode: allow | ||
| asserts: | ||
| - failedTemplate: | ||
| errorPattern: mode is "allow" but | ||
|
|
||
| - it: fails on an unknown mode | ||
| set: | ||
| config.kube.namespaceFilter.mode: bogus | ||
| config.kube.namespaceFilter.namespaces: [a] | ||
| asserts: | ||
| - failedTemplate: | ||
| errorPattern: must be one of | ||
|
|
||
| - it: fails on a non-string excludeNamespaces entry | ||
| set: | ||
| config.netobs.excludeNamespaces: [1, 2] | ||
| asserts: | ||
| - failedTemplate: | ||
| errorPattern: excludeNamespaces entry 1 must be a string | ||
|
|
||
| - it: keeps excludeNamespaces entries the Edge accepts, such as uppercase | ||
| set: | ||
| config.netobs.excludeNamespaces: ["Prod_1"] | ||
| asserts: | ||
| - matchRegex: | ||
| path: data["config.json"] | ||
| pattern: '"excludeNamespaces": \["Prod_1"\]' | ||
|
|
||
| - it: fails on an edgeExistenceTtl that is not a duration | ||
| set: | ||
| config.netobs.edgeExistenceTtl: not-a-duration | ||
| asserts: | ||
| - failedTemplate: | ||
| errorPattern: edgeExistenceTtl "not-a-duration" is not a valid duration | ||
|
|
||
| - it: fails on an edgeExistenceTtl without a unit | ||
| set: | ||
| config.netobs.edgeExistenceTtl: "30" | ||
| asserts: | ||
| - failedTemplate: | ||
| errorPattern: edgeExistenceTtl "30" is not a valid duration | ||
|
|
||
| - it: accepts Go duration syntax for edgeExistenceTtl | ||
| set: | ||
| config.netobs.edgeExistenceTtl: -1h30m | ||
| asserts: | ||
| - matchRegex: | ||
| path: data["config.json"] | ||
| pattern: '"edgeExistenceTtl": "-1h30m"' | ||
|
|
||
| - it: fails on an unknown namespaceFilter key | ||
| set: | ||
| config.kube.namespaceFilter.node: allow | ||
| asserts: | ||
| - failedTemplate: | ||
| errorPattern: namespaceFilter has unknown key "node" |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
{{- if $nf }}skips both the validation and the render whentypedPickproduced an empty dict, which happens when the user'snamespaceFiltermap contains only keys the spec does not know. The result is a silently ignored namespace filter: the same class of bug this PR fixes.Failure:
config.kube.namespaceFilter: {node: allow}(a plausible typo formode) renders with nonamespaceFilterkey at all and no warning; the Edge graphs the whole cluster. A typo in only one of the two keys, such as{modes: allow, namespaces: [x]}, does fail, so the behaviour is inconsistent.Evidence: ran
helm templatewithnamespaceFilter: {node: allow};jq '.kube | has("namespaceFilter")'returnedfalse.Fix: when the source map is non-empty but
$nfis empty,failwith the unrecognized key names.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 446f0cb. Any key other than
mode/namespacesnow fails the render:{node: allow}givesconfig.kube.namespaceFilter has unknown key "node"; supported keys are mode and namespaces. This applies with or without a validmodealongside (casesns-filter-fail-unknown-key,ns-filter-fail-unknown-key-valid-mode, plus a unittest).