Skip to content

fix(base-cluster/monitoring): alertmanager condition#1781

Merged
cwrau merged 1 commit intomainfrom
fix/base-cluster/alertmanager-condition
Nov 19, 2025
Merged

fix(base-cluster/monitoring): alertmanager condition#1781
cwrau merged 1 commit intomainfrom
fix/base-cluster/alertmanager-condition

Conversation

@cwrau
Copy link
Copy Markdown
Member

@cwrau cwrau commented Nov 10, 2025

Summary by CodeRabbit

  • Chores
    • Improved alertmanager configuration validation handling
    • Enhanced network policy applicability conditions to better align with monitoring enablement state

Copilot AI review requested due to automatic review settings November 10, 2025 13:56
@cwrau cwrau enabled auto-merge November 10, 2025 13:56
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 10, 2025

Walkthrough

Two Alertmanager-related Helm templates in the base-cluster chart are modified. The first changes the enabled field rendering to emit true or empty string via ternary logic. The second strengthens the CiliumNetworkPolicy condition to require both alertmanager enabled and network policy type "cilium".

Changes

Cohort / File(s) Change Summary
Alertmanager Template Logic
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
Wrapped non-empty receivers check with ternary expression to emit true or empty string instead of direct boolean value
Alertmanager Network Policy
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
Combined condition requiring both alertmanager enabled (base-cluster.prometheus-stack.alertmanager.enabled) and network policy type "cilium"; adjusted end block whitespace trimming

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Both changes involve straightforward template conditional refinements
  • No complex business logic or structural changes
  • Limited scope (2 files with focused modifications)

Possibly related PRs

Suggested labels

base-cluster

Suggested reviewers

  • tasches
  • marvinWolff
  • teutonet-bot

Poem

🐰 A template tale in YAML form,
Where booleans dance and conditions transform,
True or empty, the choice now precise,
Alertmanagers gated with ternary spice! 🎪

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(base-cluster/monitoring): alertmanager condition' directly relates to the main changes, which involve fixing alertmanager-related conditional logic in monitoring templates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/base-cluster/alertmanager-condition

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5769237 and 8780c1a.

📒 Files selected for processing (2)
  • charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1 hunks)
  • charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: cwrau
Repo: teutonet/teutonet-helm-charts PR: 1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:55-61
Timestamp: 2025-07-24T13:42:05.473Z
Learning: In charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml, the receiver-type parsing logic that splits keys by space and only handles exactly two tokens is intentional and matches the schema design. The schema pattern `^email($| \S+$)` specifically allows "email" or "email <suffix>" format, not arbitrary multi-space patterns. The current parsing implementation correctly enforces this constraint.
📚 Learning: 2025-07-24T13:42:05.473Z
Learnt from: cwrau
Repo: teutonet/teutonet-helm-charts PR: 1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:55-61
Timestamp: 2025-07-24T13:42:05.473Z
Learning: In charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml, the receiver-type parsing logic that splits keys by space and only handles exactly two tokens is intentional and matches the schema design. The schema pattern `^email($| \S+$)` specifically allows "email" or "email <suffix>" format, not arbitrary multi-space patterns. The current parsing implementation correctly enforces this constraint.

Applied to files:

  • charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml
  • charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
📚 Learning: 2025-07-24T13:42:05.473Z
Learnt from: cwrau
Repo: teutonet/teutonet-helm-charts PR: 1600
File: charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml:55-61
Timestamp: 2025-07-24T13:42:05.473Z
Learning: In charts/base-cluster alertmanager receivers schema, email receiver keys follow the pattern `^email($| \\S+$)`, which allows only "email" or "email <suffix>" where suffix is `\S+` (no spaces allowed in suffix). The current parsing logic correctly enforces this by only handling exactly two tokens: the receiver type and the single non-whitespace suffix that becomes the receiver name.

Applied to files:

  • charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
📚 Learning: 2025-07-21T14:04:07.149Z
Learnt from: cwrau
Repo: teutonet/teutonet-helm-charts 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.

Applied to files:

  • charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml
🪛 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)

⏰ 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: lint helm chart (base-cluster)
🔇 Additional comments (3)
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml (1)

2-2: Ternary expression correctly renders enabled field based on receiver presence.

The logic chain—checking receiver emptiness, negating it, and outputting "true" or ""—ensures the alertmanager config block is only fully rendered when receivers are defined. The output works correctly in the if condition on line 17.

charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml (2)

1-1: Condition correctly requires both alertmanager enablement and cilium policy type.

The AND logic ensures the CiliumNetworkPolicy is only rendered when alertmanager is enabled (reusing the template from _alertmanager-config.yaml) and when cilium is the active network policy type. This prevents orphaned resources and aligns policy deployment with operational intent.


33-33: Whitespace trimming is a minor, correct adjustment.

The trailing dash in {{- end -}} removes any trailing newline, consistent with Helm best practices and other blocks in this template.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves the alertmanager template logic to conditionally render the network policy based on whether alertmanager is enabled, preventing the creation of an unnecessary CiliumNetworkPolicy resource when alertmanager is disabled.

  • Modified the base-cluster.prometheus-stack.alertmanager.enabled template to explicitly return true or empty string for proper boolean evaluation in conditional contexts
  • Updated the alertmanager network policy to only be rendered when both alertmanager is enabled and the network policy type is "cilium"

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
charts/base-cluster/templates/monitoring/kube-prometheus-stack/_alertmanager-config.yaml Modified the enabled template to use ternary for explicit boolean-like return values
charts/base-cluster/templates/monitoring/kube-prometheus-stack/alertmanager-networkpolicy.yaml Added alertmanager enabled check to the conditional and improved whitespace control

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@cwrau cwrau added this pull request to the merge queue Nov 19, 2025
Merged via the queue into main with commit b6abed0 Nov 19, 2025
38 of 39 checks passed
@cwrau cwrau deleted the fix/base-cluster/alertmanager-condition branch November 19, 2025 12:37
github-merge-queue Bot pushed a commit that referenced this pull request Nov 28, 2025
🤖 I have created a release *beep* *boop*
---


##
[10.1.0](base-cluster-v10.0.3...base-cluster-v10.1.0)
(2025-11-28)


### Features

* **base-cluster/logging:** enable automatic resizing
([#1785](#1785))
([167e5e0](167e5e0))
* **base-cluster/tracing:** add gateway to enable tail sampling
([#1736](#1736))
([7c1bd9a](7c1bd9a))


### Bug Fixes

* **base-cluster/backup:** fix secret creation for velero
([#1816](#1816))
([04a8ca0](04a8ca0))
* **base-cluster/monitoring:** alertmanager condition
([#1781](#1781))
([b6abed0](b6abed0))


### Miscellaneous Chores

* **base-cluster/dependencies:** update common docker tag to v1.6.0
([#1796](#1796))
([f1d8f05](f1d8f05))
* **base-cluster/dependencies:** update docker.io/curlimages/curl docker
tag to v8.17.0
([#1797](#1797))
([86362fe](86362fe))
* **base-cluster/dependencies:** update docker.io/fluxcd/flux-cli docker
tag to v2.7.3
([#1798](#1798))
([f7b42d1](f7b42d1))
* **base-cluster/dependencies:** update docker.io/fluxcd/flux-cli docker
tag to v2.7.4
([#1818](#1818))
([6a318a1](6a318a1))
* **base-cluster/dependencies:** update docker.io/fluxcd/flux-cli docker
tag to v2.7.5
([#1823](#1823))
([bcd266e](bcd266e))
* **base-cluster/dependencies:** update
docker.io/grafana/grafana-image-renderer docker tag to v3.12.9
([#1639](#1639))
([e99101a](e99101a))
* **base-cluster/dependencies:** update helm release alloy to v1.2.1
([#1771](#1771))
([87df788](87df788))
* **base-cluster/dependencies:** update helm release alloy to v1.4.0
([#1799](#1799))
([9bc1aaa](9bc1aaa))
* **base-cluster/dependencies:** update helm release descheduler to
v0.34.0
([#1800](#1800))
([33f9a53](33f9a53))
* **base-cluster/dependencies:** update helm release external-dns to
v1.19.0
([#1801](#1801))
([c1f24a4](c1f24a4))
* **base-cluster/dependencies:** update helm release
kube-prometheus-stack to v75.15.2
([#1772](#1772))
([0cc66b2](0cc66b2))
* **base-cluster/dependencies:** update helm release
kube-prometheus-stack to v75.18.1
([#1802](#1802))
([b096b58](b096b58))
* **base-cluster/dependencies:** update helm release loki to v6.46.0
([#1727](#1727))
([ec1b906](ec1b906))
* **base-cluster/dependencies:** update helm release metrics-server to
v3.13.0
([#1805](#1805))
([6ba8633](6ba8633))
* **base-cluster/dependencies:** update helm release oauth2-proxy to
v7.14.2
([#1635](#1635))
([d88c7c0](d88c7c0))
* **base-cluster/dependencies:** update helm release oauth2-proxy to
v7.18.0
([#1806](#1806))
([636a585](636a585))
* **base-cluster/dependencies:** update helm release reflector to
v9.1.39
([#1790](#1790))
([5b032af](5b032af))
* **base-cluster/dependencies:** update helm release reflector to
v9.1.40
([#1819](#1819))
([da8be9d](da8be9d))
* **base-cluster/dependencies:** update helm release tempo-distributed
to v1.48.1
([#1791](#1791))
([d00ac00](d00ac00))
* **base-cluster/dependencies:** update helm release tempo-distributed
to v1.56.2
([#1807](#1807))
([80c67d1](80c67d1))
* **base-cluster/dependencies:** update helm release tetragon to v1.6.0
([#1808](#1808))
([c3fb92d](c3fb92d))
* **base-cluster/dependencies:** update helm release trivy-operator to
v0.31.0
([#1809](#1809))
([59976f6](59976f6))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants