Skip to content

fix(OPENFRAM-008-11): CU-86akdypzw 2 review findings across 2 files #2321

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 2 commits into
base: main
Choose a base branch
from
+5 −2
Draft

fix(OPENFRAM-008-11): CU-86akdypzw 2 review findings across 2 files #2321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a10c9e4
fix(OPENFRAM-008-11): 2 review findings across 2 files
flamingo[bot] Sep 7, 2026
026abcb
fix(OPENFRAM-008-11): 2 review findings across 2 files
flamingo[bot] Sep 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion manifests/datasources/debezium-connect/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ spec:
- |
VERSION={{ .Values.metrics.jmx.version }}
wget -O /jmx/jmx_prometheus_javaagent.jar \
https://github.com/prometheus/jmx_exporter/releases/download/${VERSION}/jmx_prometheus_javaagent-${VERSION}.jar \
{{ .Values.metrics.jmx.jarMirror.baseUrl }}/jmx_prometheus_javaagent-${VERSION}.jar \
&& echo "{{ .Values.metrics.jmx.jarMirror.sha256 }} /jmx/jmx_prometheus_javaagent.jar" | sha256sum -c - \
&& cp /jmx-config/config.yaml /jmx/config.yaml
volumeMounts:
- name: jmx-exporter
Comment on lines 36 to 43

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 debezium/connect image and jmx exporter jar are fetched directly from github.com, bypassing the internal image registry mirror

In the jmx-exporter init container args (line ~34), replaced the hardcoded https://github.com/prometheus/jmx_exporter/releases/download/... URL with a templated {{ .Values.metrics.jmx.jarMirror.baseUrl }} value (expected to point at the internal mirror/registry proxy) and added a sha256sum -c integrity check against a new {{ .Values.metrics.jmx.jarMirror.sha256 }} value before copying the config. This is a partial fix confined to this file: it requires the corresponding values.yaml (not shown/editable here) to define metrics.jmx.jarMirror.baseUrl and metrics.jmx.jarMirror.sha256 (per-version checksum), otherwise the template will render an empty URL/checksum and break the init container. A complete fix additionally requires provisioning the jar on the internal mirror and wiring real values, which is outside this single file's scope.

🤖 Prompt for AI agents 
In manifests/datasources/debezium-connect/templates/deployment.yaml around line 34, review and complete this code-review fix: debezium/connect image and jmx exporter jar are fetched directly from github.com, bypassing the internal image registry mirror.
What the draft fix changed: In the `jmx-exporter` init container args (line ~34), replaced the hardcoded `https://github.com/prometheus/jmx_exporter/releases/download/...` URL with a templated `{{ .Values.metrics.jmx.jarMirror.baseUrl }}` value (expected to point at the internal mirror/registry proxy) and added a `sha256sum -c` integrity check against a new `{{ .Values.metrics.jmx.jarMirror.sha256 }}` value before copying the config. This is a partial fix confined to this file: it requires the corresponding `values.yaml` (not shown/editable here) to define `metrics.jmx.jarMirror.baseUrl` and `metrics.jmx.jarMirror.sha256` (per-version checksum), otherwise the template will render an empty URL/checksum and break the init container. A complete fix additionally requires provisioning the jar on the internal mirror and wiring real values, which is outside this single file's scope.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 45 low — review closely — react 👍/👎 to teach the reviewer

Expand Down Expand Up @@ -109,3 +110,4 @@ spec:
configMap:
name: debezium-connect-jmx
{{- end }}

3 changes: 2 additions & 1 deletion manifests/platform/dev-tools/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ appVersion: "0.0.1"
dependencies:
- name: kafka-ui

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🦩 🔴 kafka-ui Helm dependency sourced directly from upstream kafbat.github.io repository rather than the internal registry mirror

Changed the repository field of the kafka-ui dependency in manifests/platform/dev-tools/Chart.yaml from https://kafbat.github.io/helm-charts to oci://ghcr.io/flamingo-stack/registry/helm-charts, mirroring the pattern used by telepresence-oss below it. This assumes the internal registry mirror actually hosts/proxies the kafka-ui chart under this same OCI path and chart name; if the mirror uses a different path, chart name, or hasn't yet mirrored this specific chart/version, this change will break dependency resolution until confirmed against the actual mirror contents.

🤖 Prompt for AI agents 
In manifests/platform/dev-tools/Chart.yaml around line 9, review and complete this code-review fix: kafka-ui Helm dependency sourced directly from upstream kafbat.github.io repository rather than the internal registry mirror.
What the draft fix changed: Changed the `repository` field of the `kafka-ui` dependency in `manifests/platform/dev-tools/Chart.yaml` from `https://kafbat.github.io/helm-charts` to `oci://ghcr.io/flamingo-stack/registry/helm-charts`, mirroring the pattern used by `telepresence-oss` below it. This assumes the internal registry mirror actually hosts/proxies the `kafka-ui` chart under this same OCI path and chart name; if the mirror uses a different path, chart name, or hasn't yet mirrored this specific chart/version, this change will break dependency resolution until confirmed against the actual mirror contents.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

version: "~1.6.3"
repository: https://kafbat.github.io/helm-charts
repository: oci://ghcr.io/flamingo-stack/registry/helm-charts
- name: telepresence-oss
version: "~2.30.1"
repository: oci://ghcr.io/flamingo-stack/registry/helm-charts