Conversation
Summary of ChangesHello @cwrau, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Helm templates to track base-cluster chart versions and alert on updates: FluxCD ImageRepository/ImagePolicy, a kube-state-metrics config entry to export ImagePolicy metrics, a Prometheus recording rule for current version, and a Prometheus alert rule triggered when a newer chart version is available. Changes
Sequence DiagramsequenceDiagram
participant IR as ImageRepository / ImagePolicy (FluxCD)
participant KSM as Kube-State-Metrics
participant PR as Prometheus Recording Rule
participant PA as Prometheus Alert Rule
IR->>IR: Poll registry (24h)
IR->>KSM: ImagePolicy status (latestRef.tag)
KSM->>PR: Expose metric image_policy_latest_version{tag}
PR->>PR: Record base_cluster_current_version:info
PA->>PR: Query recording rule + image_policy metrics
PA->>PA: Evaluate alert expression
PA->>PA: Fire BaseClusterUpdateAvailable if condition met
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request adds a Prometheus alert to notify about available updates for the base-cluster chart. The implementation has a few critical issues that will prevent the alert from working as intended.
- The Prometheus alert expression in
update-alert.yamlis logically flawed and will fire constantly, regardless of whether an update is available. - The mechanism for comparing the current and latest versions in
current-source.yamlis not robust, as it doesn't handle common version tag variations (like a 'v' prefix), which will lead to false alerts. This aligns with the repository's rule on semver validation. - Resource names and namespaces are hardcoded in
update-source.yamlandupdate-alert.yaml. This will cause conflicts and failures if the chart is deployed multiple times or outside thedefaultnamespace.
I've provided detailed comments and suggestions to fix these issues, which involve correcting the PromQL query, making the version comparison more robust, and templating resource names and namespaces for uniqueness and correctness.
d5cf93f to
cfa9095
Compare
There was a problem hiding this comment.
Pull request overview
Adds monitoring support to detect when a newer base-cluster chart version is available, based on Flux ImagePolicy state exposed via kube-state-metrics.
Changes:
- Extend kube-state-metrics custom resource metrics to export the latest ImagePolicy tag.
- Add Flux ImageRepository/ImagePolicy resources to track
base-clusterchart versions. - Add a recording rule for the currently deployed chart version and a Prometheus alert for available updates.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| charts/base-cluster/templates/monitoring/kube-prometheus-stack/_kube-state-metrics-config.yaml | Adds a customResourceState metric to expose ImagePolicy status.latestRef.tag as an Info metric. |
| charts/base-cluster/templates/base-cluster-update/update-source.yaml | Introduces Flux ImageRepository/ImagePolicy resources to discover the latest base-cluster version. |
| charts/base-cluster/templates/base-cluster-update/current-source.yaml | Adds a recording rule emitting the current chart version as a Prometheus metric label. |
| charts/base-cluster/templates/base-cluster-update/update-alert.yaml | Adds a PrometheusRule intended to alert when a newer version is available. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@charts/base-cluster/templates/base-cluster-update/update-alert.yaml`:
- Around line 13-24: The BaseClusterUpdateAvailable alert expression is
incorrect: change the namespace selector from the hard-coded
customresource_namespace="default" to {{ .Release.Namespace }}, remove the
standalone equality check that forces firing, and replace the subtraction with a
label-aware comparison using unless on(tag) between
max(image_policy_latest_version{customresource_namespace="{{ .Release.Namespace
}}",customresource_name="base-cluster-chart"}) by (tag) and
base_cluster_current_version:info so series are matched by tag; update the
BaseClusterUpdateAvailable expr to use this unless-on(tag) pattern to only fire
when a newer tag exists.
In `@charts/base-cluster/templates/base-cluster-update/update-source.yaml`:
- Around line 7-19: The ImagePolicy resource named base-cluster-chart has an
invalid semver value at spec.policy.semver.range ("x.x.x"); update
spec.policy.semver.range in the ImagePolicy (metadata.name: base-cluster-chart)
to a valid Flux semver constraint such as "1.x", ">=1.0.0", or a caret range
like "^1.0.0" (and include "-0" e.g. "^1.x-0" if you need to match prerelease
tags) so the policy will correctly match image tags.
6a36630 to
558daa4
Compare
…1937) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added Prometheus recording rule and critical alert to detect available base-cluster chart updates. * Added image repository and policy automation to detect new base-cluster chart versions. * Exposed ImagePolicy "latest version" metrics for improved observability. *Notes*: All monitoring changes apply only when Prometheus integration is enabled. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
8597ef0 to
41ea3ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/scripts/splitYamlIntoDir:
- Around line 42-49: The check/usage of helmreleaseDir only uses the basename
and tests in the current working directory; change to derive and check the full
target directory path (e.g. build a helmreleaseDirPath by joining dirname
"$helmrelease" and basename -s .yaml "$helmrelease") and then use that full path
for the -d test and as the second argument to splitYamlIntoDir (keep the
existing helmrelease and templating logic intact). Ensure you update references
to helmreleaseDir in the conditional and the splitYamlIntoDir call to use this
full path so existing dirs next to the HelmRelease file are detected correctly.
b17d1f7 to
dede829
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/scripts/splitYamlIntoDir:
- Around line 44-48: The recursive call constructs a malformed path by
prepending dirname to an already computed helmreleaseDir; update the call to
splitYamlIntoDir to pass helmreleaseDir (not "$(dirname
"$helmrelease")/$helmreleaseDir") so it uses the correct directory computed
above (refer to variables helmreleaseDir and helmrelease and the
splitYamlIntoDir invocation).
dede829 to
0656ae1
Compare
|
Needs to wait until #1948 is merged |
0656ae1 to
d10e1e4
Compare
d10e1e4 to
1005a9f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@charts/base-cluster/templates/base-cluster-update/current-source.yaml`:
- Around line 19-21: The current use of {{- with semver .Chart.Version }} can
silently omit the tag label when .Chart.Version isn't valid semver; change the
block so the tag label is always emitted: replace the with-block with an if/else
that tries semver (.Chart.Version) and emits tag: formatted "%d.%d.%d" when
semver succeeds (same code currently inside the with), and in the else branch
emit a safe fallback tag (e.g., the raw .Chart.Version quoted or a fixed "dev"
string) so the recording rule base_cluster_current_version:info always has a tag
label even in development snapshots.
🧹 Nitpick comments (2)
charts/base-cluster/templates/base-cluster-update/update-source.yaml (1)
1-1: Redundantandoperator with a single condition.
{{- if and .Values.monitoring.prometheus.enabled }}can be simplified to{{- if .Values.monitoring.prometheus.enabled }}, which is what the other templates in this PR already use.Proposed fix
-{{- if and .Values.monitoring.prometheus.enabled }} +{{- if .Values.monitoring.prometheus.enabled }}charts/base-cluster/templates/base-cluster-update/update-alert.yaml (1)
31-34:severity: criticalwithfor: 1wmay confuse responders.A 1-week pending period implies low urgency, but
severity: criticaltypically signals immediate action. Consider usingwarningorinfoseverity to align with the actual urgency, or add a comment explaining the rationale if this is intentional (e.g., critical only after being unaddressed for a week).
🤖 I have created a release *beep* *boop* --- ## [11.1.0](base-cluster-v11.0.1...base-cluster-v11.1.0) (2026-03-16) ### Features * **base-cluster/ingress:** add auto detection of need for proxy protocol ([#1951](#1951)) ([a94de1a](a94de1a)) * **base-cluster/ingress:** allow external ingress controller ([#1859](#1859)) ([1442431](1442431)) * **base-cluster/kyverno:** allow setting kyverno resources ([#1986](#1986)) ([71b9db4](71b9db4)) * **base-cluster/monitoring:** add alert about base-cluster updates ([#1937](#1937)) ([a3c63a7](a3c63a7)) * **base-cluster/monitoring:** add alert about deprecated APIs ([#2021](#2021)) ([cb334dd](cb334dd)) ### Bug Fixes * **base-cluster/deadMansSwitch:** fix alertmanager healthchecks URL ([#2019](#2019)) ([d874a56](d874a56)) * **base-cluster/ingress:** disable traefik apiCheck ([#1902](#1902)) ([d45bd69](d45bd69)) * **base-cluster/ingress:** they now have the redirections nested under http ([#1952](#1952)) ([dca2502](dca2502)) * **base-cluster/monitoring:** adjust for short-lived certificates ([#1921](#1921)) ([41062b2](41062b2)) * **base-cluster/monitoring:** only roll out alloy tracing ports if enabled ([#2005](#2005)) ([ea44c4d](ea44c4d)) * **base-cluster:** Revert "chore(base-cluster/dependencies): update helm release traefik to v39 ([#1936](#1936))" ([#1954](#1954)) ([5d2ae36](5d2ae36)) ### Miscellaneous Chores * **base-cluster/dependencies:** update common docker tag to v1.8.0 ([#1939](#1939)) ([38b1c7e](38b1c7e)) * **base-cluster/dependencies:** update docker.io/curlimages/curl docker tag to v8.18.0 ([#1896](#1896)) ([f046977](f046977)) * **base-cluster/dependencies:** update docker.io/grafana/grafana-image-renderer docker tag to v5.0.13 ([#1885](#1885)) ([474e903](474e903)) * **base-cluster/dependencies:** update docker.io/grafana/grafana-image-renderer docker tag to v5.2.3 ([#1897](#1897)) ([84b647b](84b647b)) * **base-cluster/dependencies:** update docker.io/grafana/grafana-image-renderer docker tag to v5.3.0 ([#1922](#1922)) ([ef6f80f](ef6f80f)) * **base-cluster/dependencies:** update docker.io/grafana/grafana-image-renderer docker tag to v5.4.0 ([#1931](#1931)) ([50171d8](50171d8)) * **base-cluster/dependencies:** update docker.io/grafana/grafana-image-renderer docker tag to v5.5.0 ([#1968](#1968)) ([ee276e2](ee276e2)) * **base-cluster/dependencies:** update docker.io/grafana/grafana-image-renderer docker tag to v5.5.1 ([#1988](#1988)) ([f765f5e](f765f5e)) * **base-cluster/dependencies:** update docker.io/vladgh/gpg docker tag to v1.3.7 ([#1886](#1886)) ([4b2c33b](4b2c33b)) * **base-cluster/dependencies:** update helm release alloy to v1.5.2 ([#1891](#1891)) ([41b25e9](41b25e9)) * **base-cluster/dependencies:** update helm release alloy to v1.5.3 ([#1949](#1949)) ([d8bda90](d8bda90)) * **base-cluster/dependencies:** update helm release alloy to v1.6.0 ([#1975](#1975)) ([76632e4](76632e4)) * **base-cluster/dependencies:** update helm release external-dns to v1.20.0 ([#1905](#1905)) ([ff53477](ff53477)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v80.13.3 ([#1892](#1892)) ([9775868](9775868)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v80.14.4 ([#1906](#1906)) ([f62458d](f62458d)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v81 ([#1923](#1923)) ([9e9915d](9e9915d)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v81.2.1 ([#1934](#1934)) ([30fa0dd](30fa0dd)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v81.3.2 ([#1950](#1950)) ([95a9398](95a9398)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v81.5.0 ([#1962](#1962)) ([1a9bab8](1a9bab8)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v81.5.2 ([#1982](#1982)) ([07c2249](07c2249)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v81.6.6 ([#1989](#1989)) ([2bf4f3c](2bf4f3c)) * **base-cluster/dependencies:** update helm release kube-prometheus-stack to v82 ([#1995](#1995)) ([45ef213](45ef213)) * **base-cluster/dependencies:** update helm release loki to v6.49.0 ([#1908](#1908)) ([f36dd6d](f36dd6d)) * **base-cluster/dependencies:** update helm release loki to v6.51.0 ([#1928](#1928)) ([6ac27d6](6ac27d6)) * **base-cluster/dependencies:** update helm release loki to v6.53.0 ([#1974](#1974)) ([0bc6e68](0bc6e68)) * **base-cluster/dependencies:** update helm release oauth2-proxy to v10 ([#1913](#1913)) ([7e551b5](7e551b5)) * **base-cluster/dependencies:** update helm release oauth2-proxy to v10.1.1 ([#1944](#1944)) ([3f97108](3f97108)) * **base-cluster/dependencies:** update helm release oauth2-proxy to v10.1.2 ([#1961](#1961)) ([c0bc91d](c0bc91d)) * **base-cluster/dependencies:** update helm release oauth2-proxy to v10.1.3 ([#1979](#1979)) ([9b95c4b](9b95c4b)) * **base-cluster/dependencies:** update helm release oauth2-proxy to v10.1.4 ([#2001](#2001)) ([8ffa211](8ffa211)) * **base-cluster/dependencies:** update helm release reflector to v10 ([#1924](#1924)) ([0051c34](0051c34)) * **base-cluster/dependencies:** update helm release reflector to v10.0.19 ([#1999](#1999)) ([a2b5189](a2b5189)) * **base-cluster/dependencies:** update helm release reflector to v10.0.2 ([#1935](#1935)) ([333393e](333393e)) * **base-cluster/dependencies:** update helm release reflector to v10.0.4 ([#1956](#1956)) ([3eef9a0](3eef9a0)) * **base-cluster/dependencies:** update helm release reflector to v10.0.8 ([#1978](#1978)) ([b2f97f9](b2f97f9)) * **base-cluster/dependencies:** update helm release reflector to v9.1.45 ([#1893](#1893)) ([ff100d9](ff100d9)) * **base-cluster/dependencies:** update helm release tempo to v1.24.3 ([#1904](#1904)) ([99099bf](99099bf)) * **base-cluster/dependencies:** update helm release tempo to v1.24.4 ([#1957](#1957)) ([7d67bf3](7d67bf3)) * **base-cluster/dependencies:** update helm release tempo to v1.26.1 ([#1976](#1976)) ([517da93](517da93)) * **base-cluster/dependencies:** update helm release tempo to v1.26.7 ([#2000](#2000)) ([6cabd54](6cabd54)) * **base-cluster/dependencies:** update helm release traefik to v38 ([#1914](#1914)) ([106c7cf](106c7cf)) * **base-cluster/dependencies:** update helm release traefik to v39 ([#1936](#1936)) ([5b39257](5b39257)) * **base-cluster/dependencies:** update helm release traefik to v39 ([#1959](#1959)) ([6efe111](6efe111)) * **base-cluster/dependencies:** update helm release traefik to v39.0.1 ([#1992](#1992)) ([27d7316](27d7316)) * **base-cluster/monitoring:** migrate helm repo to new URL ([#1955](#1955)) ([9263d6a](9263d6a)) * **base-cluster/tetragon:** update flux apiVersion ([#1900](#1900)) ([ff93afb](ff93afb)) * **base-cluster:** update kyverno ([#1918](#1918)) ([a503ef6](a503ef6)) * migrate kyverno config ([71b9db4](71b9db4)) --- 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>
Summary by CodeRabbit