feat(base-cluster/monitoring/alertmanager)!: add receiver and route configuration capabilities #1600
Conversation
tasches
requested review from
cwrau,
marvinWolff and
teutonet-bot
as code owners
|
""" WalkthroughThe changes overhaul the Alertmanager configuration in the Helm chart, introducing support for multiple receiver types (PagerDuty, email), stricter validation and schema constraints, dynamic receiver and routing configuration, and removal of explicit enablement flags. The configuration files and schema are updated to reflect these new structures and validation requirements. Additionally, a new Cilium network policy manifest for Alertmanager email egress is added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm Chart
participant Alertmanager
User->>Helm Chart: Provide values.yaml with receivers/routes/defaultReceiver
Helm Chart->>Helm Chart: Validate receivers and routes (recursive check)
Helm Chart->>Alertmanager: Render config with dynamic receivers and routes
Alertmanager-->>Helm Chart: Accepts config if validation passes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
tasches
changed the title
cwrau
changed the title
cwrau
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
charts/base-cluster/ci/pagerduty-values.yaml (1)
4-7: Same 32-char length issue as in artifacthub valuesThe placeholder is 30 chars – adjust to keep CI green.
- integrationKey: INTEGRATION_KEY__INTEGRATION_KEY + integrationKey: 00000000000000000000000000000000
🧹 Nitpick comments (4)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
81-83:route.receiveremitted even when only the “null” sink exists
$receiversalways contains the pre-seeded"null"entry, so theif $receiversguard is always true and a route with receiver"null"is emitted even when no real receivers are configured.
Consider switching the condition toif gt (len $receivers) 1or checking.Values.monitoring.prometheus.alertmanager.receiversinstead to avoid suprisingly enabled Alertmanager.charts/base-cluster/values.yaml (1)
298-300: Default receiver falls back to literal"null"If no PagerDuty receiver exists,
.defaultReceiverbecomes the string"null".
Because a dummy"null"receiver is injected, this works technically, but a clearer fallback could be the first configured receiver or an empty string to surface mis-configuration early.charts/base-cluster/values.schema.json (2)
723-730: PagerDutyintegrationKeyshould restrict the character setLength is enforced, but not content. PagerDuty keys are URL-safe base-32 (A-Z, a-z, 0-9).
Consider tightening the schema with a pattern so accidental whitespace or separators are caught at validation time.- "minLength": 32, - "maxLength": 32 + "minLength": 32, + "maxLength": 32, + "pattern": "^[A-Za-z0-9]{32}$"
1713-1715: External$defsdependency might introduce brittle buildsDepending on a remote GitHub raw link ties chart installation to network availability and the exact upstream schema structure. Consider vendoring the snippet or pinning to a commit SHA to prevent silent breakage if the file layout changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
charts/base-cluster/ci/artifacthub-values.yaml(1 hunks)charts/base-cluster/ci/pagerduty-values.yaml(1 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)charts/base-cluster/values.schema.json(2 hunks)charts/base-cluster/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Validate Title
- GitHub Check: Validate README changes (base-cluster)
- GitHub Check: wait-for-checks
- GitHub Check: Update release-please config file for a possibly new chart
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
🔇 Additional comments (2)
charts/base-cluster/ci/artifacthub-values.yaml (1)
41-44: Placeholder integration key violates new 32-char schema constraint
INTEGRATION_KEY__INTEGRATION_KEYis only 30 characters, so Helm-schema validation will now fail.
Provide a dummy value with exactly 32 printable characters or fetch a real key via CI secrets.- integrationKey: INTEGRATION_KEY__INTEGRATION_KEY +# 32-character placeholder, adjust in CI + integrationKey: 00000000000000000000000000000000Likely an incorrect or invalid review comment.
charts/base-cluster/values.schema.json (1)
774-776: Verify external$refpath forroutesThe schema defers to
#/$defs/alertmanagerConfigRoute, which in turn references a remote URL. Please double-check that the referenced path (…/values.schema.json#/properties/config/properties/route) still exists in the upstream repo; breaking URLs cause Helm-lint failures.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Outdated
Show resolved
Hide resolved
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Show resolved
Hide resolved
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
43-50: Receiver type extraction –$namestill mutated, risking inconsistenciesInside the loop
$nameis reassigned after determining$type.
Down-stream template logic (e.g.$receivers = set … $name) relies on the mutated value, while other places (see lines 87-89) expect the original key.
Keeping two variables but mutating one of them is error-prone and hard to reason about.A safer pattern is to keep the original key untouched and work exclusively with
$type/$displayNamederived from it:- {{- range $name, $config := .Values.monitoring.prometheus.alertmanager.receivers -}} - {{- $type := $name -}} - {{- $splitted := splitList $name " " -}} - {{- if ge (len $splitted) 2 -}} - {{- $type = index $splitted 0 -}} - {{- $name = index $splitted 1 -}} - {{- end -}} + {{- range $rawName, $config := .Values.monitoring.prometheus.alertmanager.receivers -}} + {{- $parts := splitList $rawName " " -}} + {{- $type := index $parts 0 -}} + {{- $displayName := ternary (index $parts 1) $type (ge (len $parts) 2) -}}This removes side-effects and makes it clear which variable represents what.
(Subsequent template lines must of course switch from$nameto$displayName.)
🧹 Nitpick comments (2)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
62-75: Parity between e-mail and PagerDuty receiversAfter applying the above change, remember to align the e-mail branch (
eq $type "email") so both receiver types follow the same$displayNameconvention.
This ensuresdefaultReceivervalues and custom routes can reference either type uniformly.
83-85:if $receiversguard is redundant – dict is never empty
$receiversis initialised with a"null"receiver (line 31), therefore theif $receiverscondition is always truthy.
The extra guard adds noise without any functional benefit and can be removed:- {{- if $receivers }} receiver: {{ include "common.tplvalues.render" (dict "value" .Values.monitoring.prometheus.alertmanager.defaultReceiver "context" .) | quote }} - {{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)charts/base-cluster/values.schema.json(2 hunks)charts/base-cluster/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/base-cluster/values.yaml
- charts/base-cluster/values.schema.json
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Show resolved
Hide resolved
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Outdated
Show resolved
Hide resolved
tasches
force-pushed
the
feat/alertmanager-config
branch
from
0939bf7 to
b817202
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
43-49: Fix variable-mutation side-effects by keeping$nameimmutableInside the loop
$nameis overwritten after the split which (a) makes the code harder to reason about and (b) risks subtle bugs later if$nameis reused for anything other than the final receiver key.
Keep the original iterator variable immutable and work with distinct locals instead.-{{- range $name, $config := .Values.monitoring.prometheus.alertmanager.receivers -}} - {{- $type := $name -}} - {{- $splitted := splitList $name " " -}} - {{- if ge (len $splitted) 2 -}} - {{- $type = index $splitted 0 -}} - {{- $name = index $splitted 1 -}} - {{- end -}} +{{- range $rawName, $config := .Values.monitoring.prometheus.alertmanager.receivers -}} + {{- $parts := splitList $rawName " " -}} + {{- $type := index $parts 0 -}} + {{- $displayName := (ge (len $parts) 2 | ternary (index $parts 1) $type) -}}Follow-up lines need to use
$displayNameinstead of$name.
This removes hidden mutation and makes the intent explicit.
🧹 Nitpick comments (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
83-85:if $receiverscondition is always truthy – drop the pointless guard
$receiversis initialised with a"null"stub before the loop, therefore the
{{- if $receivers }}guard can never be false.
Removing the check simplifies the template and avoids future confusion.-{{- $receiversList := list -}} -... - receivers: {{- toYaml $receiversList | nindent 4 }} - route: {{- if $receivers }} +{{- $receiversList := list -}} +... + receivers: {{- toYaml $receiversList | nindent 4 }} + route:(Indentation unchanged.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
charts/base-cluster/ci/artifacthub-values.yaml(1 hunks)charts/base-cluster/ci/pagerduty-values.yaml(1 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)charts/base-cluster/values.schema.json(2 hunks)charts/base-cluster/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- charts/base-cluster/ci/artifacthub-values.yaml
- charts/base-cluster/ci/pagerduty-values.yaml
- charts/base-cluster/values.yaml
- charts/base-cluster/values.schema.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
🔇 Additional comments (3)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (3)
51-61: Quote constant keeps single PagerDuty receiver – OK by schemaPagerDuty is deliberately stored under the fixed key
"pagerduty"so additional entries would overwrite the previous one. Given the schema only allows one PagerDuty object, that’s correct and the implementation now matches the contract.
No action required.
87-89: ExacthasKeyworks given the schemaBecause the values schema restricts PagerDuty to a single top-level key named
pagerduty, the exact-matchhasKeycheck is appropriate and won’t miss any
legitimate configurations.
Good to keep things simple here.
1-2: YAMLlint false positive – template delimiters confuse the linterThe YAMLlint error reported in CI is a known limitation when raw Helm
templates are linted as plain YAML. No change is necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 4
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 4-4: wrong indentation: expected 0 but found 4
(indentation)
[warning] 5-5: wrong indentation: expected 0 but found 2
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Update release-please config file for a possibly new chart
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
charts/base-cluster/templates/monitoring/kube-prometheus-stack/validation.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
55-61: Stop mutating$nameinside the loop – readability & safety
$nameis reused to store the display name after the split, which makes the loop harder to follow and was the root cause of past bugs. Keep the original key immutable:- {{- range $name, $config := .Values.monitoring.prometheus.alertmanager.receivers -}} - {{- $type := $name -}} - {{- $splitted := splitList " " $name -}} - {{- if eq (len $splitted) 2 -}} - {{- $type = index $splitted 0 -}} - {{- $name = index $splitted 1 -}} - {{- end -}} + {{- range $rawName, $config := .Values.monitoring.prometheus.alertmanager.receivers -}} + {{- $parts := splitList " " $rawName -}} + {{- $type := index $parts 0 -}} + {{- $displayName := (eq (len $parts) 2) | ternary (index $parts 1) $type -}}Then use
$displayNameinstead of$namein thesetcalls below.
This removes side-effects and makes future modifications less error-prone.
🧹 Nitpick comments (2)
charts/base-cluster/ci/alertmanager-values.yaml (1)
4-5: Quote keys that contain spaces for YAML clarityWhile unquoted keys like
email testmail:are technically valid, quoting avoids surprises with linters and editors:- email testmail: + "email testmail":Also applies to: 14-15
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
95-109: Receiver existence check happens after user routes are appendedIf a user route references a receiver that is added later in the loop (rare but possible when the values file’s key order differs), validation will incorrectly fail.
Compute$receiverscompletely before building$routes/calling the validator to avoid order-dependence.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/base-cluster/ci/alertmanager-values.yaml(1 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)charts/base-cluster/values.schema.json(2 hunks)
🧠 Learnings (3)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/ci/alertmanager-values.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 GitHub Actions: Lint Helm Charts
charts/base-cluster/ci/alertmanager-values.yaml
[warning] 1-1: coalesce.go warnings: cannot overwrite table with non table for monitoring.prometheus.alertmanager.routes.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[warning] 1-1: coalesce.go warnings: cannot overwrite table with non table for monitoring.prometheus.alertmanager.routes.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 6
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/base-cluster/values.schema.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/ci/alertmanager-values.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 GitHub Actions: Lint Helm Charts
charts/base-cluster/ci/alertmanager-values.yaml
[warning] 1-1: coalesce.go warnings: cannot overwrite table with non table for monitoring.prometheus.alertmanager.routes.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[warning] 1-1: coalesce.go warnings: cannot overwrite table with non table for monitoring.prometheus.alertmanager.routes.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 6
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check licenses
- GitHub Check: wait-for-checks
🔇 Additional comments (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
62-73: Schema allows only one PagerDuty receiver – short-circuit further iterationsWhen multiple
pagerduty …keys are supplied, later iterations silently overwrite the earlier one.
Given the one-receiver restriction, consider adding an explicit guard:{{- if and (eq $type "pagerduty") (hasKey $receivers "pagerduty") -}} {{- fail "Only one PagerDuty receiver is supported by the schema" -}} {{- end -}}Failing fast prevents confusing “last one wins” behaviour.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
43-50: “null” placeholder might collide with a real receiver name
If a user legitimately defines a receiver callednull, the placeholder will be overwritten and the InfoInhibitor / Watchdog route will suddenly hit a non-existent receiver. Consider using something that is virtually impossible to clash with (e.g.__blackhole__) or make the placeholder configurable.- {{- $receivers := dict "null" (dict) -}} + {{- $receivers := dict "__blackhole__" (dict) -}} ... - "receiver" "null" + "receiver" "__blackhole__"
55-61: Stop mutating$name– keep type, display name, and original key separate
Re-assigning$nameafter the split is functional but makes the block harder to reason about and risks future bugs. Capture a dedicated$displayNameinstead:- {{- $type := $name -}} - {{- $splitted := splitList " " $name -}} - {{- if eq (len $splitted) 2 -}} - {{- $type = index $splitted 0 -}} - {{- $name = index $splitted 1 -}} - {{- end -}} + {{- $parts := splitList " " $name -}} + {{- $type := index $parts 0 -}} + {{- $displayName := (len $parts | eq 2 | ternary (index $parts 1) $type) -}}…and then use
$displayNamewhen you store the receiver. Improves readability and removes hidden state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 6
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 6
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (2)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
5-14: Recursive “receiver-exists” validation is spot-on
Neat, fails fast, and prevents mis-configured routes from ever reaching Alertmanager.
63-73: PagerDuty receiver – single-key approach aligns with schema
Keeping the key fixed to"pagerduty"enforces the “only one PagerDuty receiver” rule baked into the values schema. Looks good.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml(1 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
Show resolved
Hide resolved
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
Show resolved
Hide resolved
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
charts/base-cluster/values.schema.json (2)
735-774: Email receiver schema still missing"type": "object"
Same comment as earlier – without the explicit type the validation is weaker than the rest of the file."^email($| \\S+$)": { + "type": "object", "description": "Sets up an email receiver …",
710-712:defaultReceiverlacks validation & documentationThe field is a plain string; typos silently pass schema validation but will break Alertmanager at runtime.
Add a description and at least enforce a non-empty string.- "defaultReceiver": { - "type": "string" - }, + "defaultReceiver": { + "type": "string", + "description": "Name of the receiver used when no route matches. Must equal one of the keys in .receivers.", + "minLength": 1 + },
🧹 Nitpick comments (3)
charts/base-cluster/ci/alertmanager-values.yaml (1)
10-21: Plaintext SMTP credentials are acceptable for CI, but mark them as dummyConsider explicitly flagging them as placeholders to avoid confusion in code-search results.
- password: "1234" + password: "<dummy>" ... - password: abcd + password: "<dummy>"charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (1)
24-31:protocolfield omitted – add for stricter Cilium validationAlthough Cilium treats missing protocol as “ANY”, explicitly setting
TCPavoids lint warnings and makes intent clear.- - ports: + - ports: {{- range $port := $ports | uniq | sortAlpha }} - - port: {{ $port | quote }} + - port: {{ $port | quote }} + protocol: TCP {{- end }}charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)
55-61: Avoid mutating$nameinside the loopChanging the range key
$namein-place makes the block harder to reason about and is easy to break once a third suffix component (e.g."email prod ops") appears.
Keep the original key immutable and introduce a separate$displayNamevariable instead.-{{- $type := $name -}} -{{- $splitted := splitList " " $name -}} -{{- if eq (len $splitted) 2 -}} - {{- $type = index $splitted 0 -}} - {{- $name = index $splitted 1 -}} -{{- end -}} +{{- $parts := splitList " " $name -}} +{{- $type := index $parts 0 -}} +{{- $displayName := (gt (len $parts) 1 | ternary (index $parts 1) $type) -}}Then use
$displayNamewhere you currently use the potentially mutated$name.
This keeps the intent crystal-clear and prevents accidental key collisions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
charts/base-cluster/ci/alertmanager-values.yaml(1 hunks)charts/base-cluster/ci/artifacthub-values.yaml(1 hunks)charts/base-cluster/ci/pagerduty-values.yaml(1 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml(1 hunks)charts/base-cluster/values.schema.json(2 hunks)charts/base-cluster/values.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/ci/artifacthub-values.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/ci/alertmanager-values.yaml (2)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.334Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/base-cluster/ci/pagerduty-values.yaml (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/values.yaml (3)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.334Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.614Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (5)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml:16-23
Timestamp: 2025-07-24T11:32:00.003Z
Learning: The Helm/Sprig dig function syntax is dig key1 key2 ... keyN default dict, where keys come first, then the default value, and finally the dictionary to search. For example, dig $receiver.host (list) $fqdns correctly looks up $receiver.host in $fqdns with (list) as default.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml:16-23
Timestamp: 2025-07-24T11:32:00.003Z
Learning: The Helm/Sprig dig function syntax is dig "key1" "key2" ... "defaultValue" $dict, where keys come first, then the default value, and finally the dictionary to search. The dictionary argument must come last. For example, dig $receiver.host (list) $fqdns correctly looks up $receiver.host in $fqdns with (list) as the default value.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.614Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.334Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.614Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/base-cluster/values.schema.json (1)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 6
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
🔇 Additional comments (4)
charts/base-cluster/ci/artifacthub-values.yaml (1)
41-44:enabledflag removal + integration key patch LGTMDropping the obsolete
enabledflag and switching to a 32-char placeholder key aligns with the new schema and keeps CI fixtures valid. No further action required.charts/base-cluster/ci/pagerduty-values.yaml (1)
4-7: Consistent PagerDuty placeholder – looks goodThe file mirrors the new schema exactly; length and format of the placeholder key satisfy the 32-character constraint.
charts/base-cluster/ci/alertmanager-values.yaml (1)
4-6: Possible default-receiver mismatch
defaultReceiver: testmaildoes not match any exact key inreceivers(email testmail).
If the template expects an exact key, Alertmanager’s top-level route will reference a non-existent receiver and fail validation.- defaultReceiver: testmail + defaultReceiver: "email testmail"Please verify how
_alertmanager-config.yamlresolves the default receiver.charts/base-cluster/values.yaml (1)
298-300:nullsentinel might still surface in rendered configThe ternary falls back to the literal string
"null".
If the Alertmanager template inserts this value unchanged, the generated route will again point to a non-existent receiver.Confirm that the template substitutes
"null"with an empty value or omits the receiver field entirely; otherwise consider:- defaultReceiver: '{{ hasKey .Values.monitoring.prometheus.alertmanager.receivers "pagerduty" | ternary "pagerduty" "" }}' + defaultReceiver: '{{ hasKey .Values.monitoring.prometheus.alertmanager.receivers "pagerduty" | ternary "pagerduty" "" }}'
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Show resolved
Hide resolved
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Show resolved
Hide resolved
tasches
force-pushed
the
feat/alertmanager-config
branch
from
8644dbb to
39ea3f4
Compare
…onfiguration capabilities
tasches
force-pushed
the
feat/alertmanager-config
branch
from
39ea3f4 to
7527121
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (1)
24-32: Broken indentation & malformedtoPortsblock – policy will be rejected by Cilium
toPortsmust be a sibling oftoFQDNs(same indent level) and has to contain a list of{ port, protocol }maps.
The current indentation neststoPortsundertoFQDNs, andports:is emitted as a scalar instead of a list. This produces invalid YAML and Cilium will refuse the manifest.- - toFQDNs: - - matchName: {{ $host }} - toPorts: - - ports: - {{- range $port := $ports | uniq | sortAlpha }} - - port: {{ $port | quote }} - {{- end }} + - toFQDNs: + - matchName: {{ $host }} + toPorts: + - ports: +{{- range $port := $ports | uniq | sortAlpha }} + - port: {{ $port | quote }} + protocol: TCP +{{- end }}After applying, run
helm template+kubectl apply --dry-run=clientto confirm the CRD validates.
🧹 Nitpick comments (2)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
55-61: Receiver name parsing is limited to exactly two tokens
splitList " " $namefollowed byif eq (len $splitted) 2means keys like
"email team prod"(3 tokens) or accidental double-spaces silently fall back totype = name, causing the receiver to be ignored.- {{- if eq (len $splitted) 2 -}} + {{- if ge (len $splitted) 2 -}}This keeps the first element as the type and joins the rest back together (or just takes index 1) to form the display name, making the template tolerant to extra qualifiers.
1-3: Alertmanager disabled when only Dead-Mans-Switch is configured
base-cluster.prometheus-stack.alertmanager.enabledreturnsfalsewhen the user receiver map is empty, even though the template later injects ahealthchecks.ioreceiver.
Consider expanding the predicate to include the Dead-Mans-Switch toggle, otherwise operators must always define at least one dummy receiver.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
charts/base-cluster/ci/alertmanager-values.yaml(1 hunks)charts/base-cluster/ci/artifacthub-values.yaml(1 hunks)charts/base-cluster/ci/pagerduty-values.yaml(1 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml(1 hunks)charts/base-cluster/values.schema.json(3 hunks)charts/base-cluster/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- charts/base-cluster/ci/pagerduty-values.yaml
- charts/base-cluster/ci/alertmanager-values.yaml
- charts/base-cluster/ci/artifacthub-values.yaml
- charts/base-cluster/values.yaml
- charts/base-cluster/values.schema.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.614Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (5)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.110Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml:16-23
Timestamp: 2025-07-24T11:32:00.003Z
Learning: The Helm/Sprig dig function syntax is dig key1 key2 ... keyN default dict, where keys come first, then the default value, and finally the dictionary to search. For example, dig $receiver.host (list) $fqdns correctly looks up $receiver.host in $fqdns with (list) as default.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml:16-23
Timestamp: 2025-07-24T11:32:00.003Z
Learning: The Helm/Sprig dig function syntax is dig "key1" "key2" ... "defaultValue" $dict, where keys come first, then the default value, and finally the dictionary to search. The dictionary argument must come last. For example, dig $receiver.host (list) $fqdns correctly looks up $receiver.host in $fqdns with (list) as the default value.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.614Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.334Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 6
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check licenses
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (1)
28-31: Be explicit about the protocolCilium defaults the protocol to TCP when it is omitted, but being explicit
avoids surprises and improves readability.- - port: {{ $port | quote }} + - port: {{ $port | quote }} + protocol: TCP
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
charts/base-cluster/ci/alertmanager-values.yaml(1 hunks)charts/base-cluster/ci/artifacthub-values.yaml(1 hunks)charts/base-cluster/ci/pagerduty-values.yaml(1 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml(2 hunks)charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml(1 hunks)charts/base-cluster/values.schema.json(3 hunks)charts/base-cluster/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- charts/base-cluster/ci/pagerduty-values.yaml
- charts/base-cluster/ci/artifacthub-values.yaml
- charts/base-cluster/values.yaml
- charts/base-cluster/values.schema.json
- charts/base-cluster/ci/alertmanager-values.yaml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cwrau
PR: teutonet/teutonet-helm-charts#1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.149Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as `pagerduty` object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.149Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (5)
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:50-61
Timestamp: 2025-07-21T14:04:07.149Z
Learning: In base-cluster alertmanager configuration, the schema only supports a single PagerDuty receiver (defined as pagerduty object), but supports multiple email receivers via pattern properties (e.g., "email", "email prod", "email staging"). Multiple PagerDuty receivers are not supported by design.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml:16-23
Timestamp: 2025-07-24T11:32:00.011Z
Learning: The Helm/Sprig dig function syntax is dig key1 key2 ... keyN default dict, where keys come first, then the default value, and finally the dictionary to search. For example, dig $receiver.host (list) $fqdns correctly looks up $receiver.host in $fqdns with (list) as default.
Learnt from: cwrau
PR: #1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml:16-23
Timestamp: 2025-07-24T11:32:00.011Z
Learning: The Helm/Sprig dig function syntax is dig "key1" "key2" ... "defaultValue" $dict, where keys come first, then the default value, and finally the dictionary to search. The dictionary argument must come last. For example, dig $receiver.host (list) $fqdns correctly looks up $receiver.host in $fqdns with (list) as the default value.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:30-32
Timestamp: 2025-07-24T09:55:53.655Z
Learning: In charts/base-cluster/templates/dns/external-dns.yaml, the dns.provider field in values.yaml has always been expected to be a map format (e.g., { cloudflare: {} }), never a string format. The template correctly uses {{ .Values.dns.provider | keys | first }} to extract the provider name from the map keys.
Learnt from: cwrau
PR: #1601
File: charts/base-cluster/templates/dns/external-dns.yaml:33-39
Timestamp: 2025-07-24T09:56:41.380Z
Learning: In the teutonet-helm-charts base-cluster chart, secret names like "external-dns" for Cloudflare provider are intentionally hard-coded. Users who need custom secret names should use Helm's valuesFrom feature to override values rather than expecting dedicated fields in values.yaml. This design keeps the values.yaml clean while still allowing full customization flexibility.
🪛 YAMLlint (1.37.1)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
[error] 2-2: syntax error: expected '', but found '{'
(syntax)
[warning] 6-6: wrong indentation: expected 0 but found 2
(indentation)
[warning] 7-7: wrong indentation: expected 0 but found 4
(indentation)
[warning] 8-8: wrong indentation: expected 0 but found 6
(indentation)
[warning] 9-9: wrong indentation: expected 0 but found 4
(indentation)
[warning] 10-10: wrong indentation: expected 0 but found 2
(indentation)
[warning] 11-11: wrong indentation: expected 0 but found 2
(indentation)
[warning] 12-12: wrong indentation: expected 0 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 0 but found 2
(indentation)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint helm chart (base-cluster)
- GitHub Check: wait-for-checks
🔇 Additional comments (2)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (2)
81-83: Double-checksendResolvedlookup
digexpects key(s) … default … dict. IfsendResolvedis a top-level key
the current call is fine; if it is nested (e.g. underemail_configs) the key
path must be split into separate arguments.Please verify the field location; otherwise the default
falsewill always be
returned.
112-114: Template hard-fails whendefaultReceiveris emptyDuring upgrades an existing values file may omit
defaultReceiver.
validateReceiverInRoutewill then fail with “Receiver '' is not configured”.Consider a safe fallback (first configured receiver) before validation:
- "receiver" (include "common.tplvalues.render" (dict "value" .Values.monitoring.prometheus.alertmanager.defaultReceiver "context" .)) + {{- $def := include "common.tplvalues.render" (dict "value" .Values.monitoring.prometheus.alertmanager.defaultReceiver "context" .) -}} + "receiver" (empty $def | ternary (keys $receivers | first) $def)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Show resolved
Hide resolved
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
Show resolved
Hide resolved
🤖 I have created a release *beep* *boop* --- ## [9.0.0](base-cluster-v8.2.1...base-cluster-v9.0.0) (2025-07-25) ### ⚠ BREAKING CHANGES * **base-cluster/monitoring/alertmanager:** add receiver and route configuration capabilities ([#1600](#1600)) ### Features * **base-cluster/monitoring/alertmanager:** add receiver and route configuration capabilities ([#1600](#1600)) ([7f549fb](7f549fb)) * **base-cluster/monitoring:** don't automount the ServiceAccountToken ([#1615](#1615)) ([52386b7](52386b7)) ### Miscellaneous Chores * **base-cluster/monitoring:** deploy log collector to potential control-plane ([#1616](#1616)) ([e021845](e021845)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configuration options for monitoring alertmanager receiver and route. * Enabled the ability to disable automounting of the ServiceAccountToken in the monitoring component. * **Breaking Changes** * Introduced breaking changes related to monitoring alertmanager configuration. * **Chores** * Deployed a log collector to potential control-plane nodes. * **Documentation** * Updated changelog to reflect new features and breaking changes in version 9.0.0. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Refactor
Chores