Skip to content

feat: support configuring multiple catalog index images [RHIDP-12934]#368

Open
rm3l wants to merge 3 commits intoredhat-developer:mainfrom
rm3l:RHIDP-12934--update-helm-chart-to-support-configuring-multiple-catalog-index-images
Open

feat: support configuring multiple catalog index images [RHIDP-12934]#368
rm3l wants to merge 3 commits intoredhat-developer:mainfrom
rm3l:RHIDP-12934--update-helm-chart-to-support-configuring-multiple-catalog-index-images

Conversation

@rm3l
Copy link
Copy Markdown
Member

@rm3l rm3l commented Apr 22, 2026

Description of the change

Followup to redhat-developer/rhdh#4655 (which adds support for extra catalog index images in the install-dynamic-plugins.py script).

This introduces a new global.catalogIndex.extraImages field to allow listing additional catalog index OCI images whose catalog entities are extracted for the Extensions UI. The global.catalogIndex.image is still considered as the primary source for the dynamic-plugins.default.yaml, the extra ones only contribute additional catalog entities for the Extensions UI.

Assisted-by: Claude

Which issue(s) does this PR fix or relate to

Fixes https://redhat.atlassian.net/browse/RHIDP-12934

How to test changes / Special notes to the reviewer

Checklist

  • For each Chart updated, version bumped in the corresponding Chart.yaml according to Semantic Versioning.
  • For each Chart updated, variables are documented in the values.yaml and added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Run pre-commit run --all-files to run the hooks and then push any resulting changes. The pre-commit Workflow will enforce this and warn you if needed.
  • JSON Schema template updated and re-generated the raw schema via the pre-commit hook.
  • Tests pass using the Chart Testing tool and the ct lint command.
  • If you updated the orchestrator-infra chart, make sure the versions of the Knative CRDs are aligned with the versions of the CRDs installed by the OpenShift Serverless operators declared in the values.yaml file. See Installing Knative Eventing and Knative Serving CRDs for more details.

Add global.catalogIndex.extraImages to allow listing additional catalog
index OCI images whose catalog entities are extracted for the Extensions
UI.  A new helper (rhdh.catalogIndex.extraImagesEnvValue) builds the
comma-separated EXTRA_CATALOG_INDEX_IMAGES value, and the vendored
deployment template conditionally injects it into the
install-dynamic-plugins init container only when the list is non-empty.

Assisted-by: Claude
@rm3l rm3l force-pushed the RHIDP-12934--update-helm-chart-to-support-configuring-multiple-catalog-index-images branch from b25ba94 to 78dc7f6 Compare April 22, 2026 19:02
Comment thread docs/catalog-index-configuration.md Outdated
@rm3l
Copy link
Copy Markdown
Member Author

rm3l commented Apr 23, 2026

/agentic_review

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Apr 23, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Duplicate env var entries 🐞 Bug ≡ Correctness
Description
The deployment template unconditionally appends EXTRA_CATALOG_INDEX_IMAGES to the
install-dynamic-plugins init container env when extra images are set, even if the user already
defined that env var. This can result in duplicate env var names and silently override/alter the
effective value at runtime.
Code

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[R119-123]

+        {{- $container := include "common.tplvalues.render" (dict "value" . "context" $) | fromYaml }}
+        {{- if and $extraCatalogImages (eq (index $container "name") "install-dynamic-plugins") }}
+        {{- $_ := set $container "env" (append (default list (index $container "env")) (dict "name" "EXTRA_CATALOG_INDEX_IMAGES" "value" $extraCatalogImages)) }}
+        {{- end }}
+        - {{ $container | toYaml | nindent 10 | trim }}
Relevance

⭐⭐⭐ High

Duplicate env var names in list can change runtime value order-dependently; likely treated as real
correctness bug.

PR-#280
PR-#293
PR-#325

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The initContainers rendering logic always appends a new EXTRA_CATALOG_INDEX_IMAGES entry and does
not check for an existing one. Because env is a list, this can produce duplicate name entries and
the effective value becomes order-dependent.

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]
charts/backstage/values.yaml[368-403]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EXTRA_CATALOG_INDEX_IMAGES` is appended to the init container env list without checking for an existing entry, which can create duplicate env var names and ambiguous behavior.

### Issue Context
Users can override/extend `upstream.backstage.initContainers[*].env` in values; if they already define `EXTRA_CATALOG_INDEX_IMAGES`, the chart will add a second entry.

### Fix Focus Areas
- charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]

### Suggested fix approach
- Before appending, scan `$container.env` for an item with `name: EXTRA_CATALOG_INDEX_IMAGES`.
- If found, either (a) leave it unchanged (treat user override as authoritative) or (b) replace its `value` with `$extraCatalogImages`.
- Only append a new env var if it does not already exist.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unrendered extraImages templates 🐞 Bug ≡ Correctness
Description
The rhdh.catalogIndex.extraImagesEnvValue helper builds image references from raw values without tpl
rendering, so any templated strings in extraImages fields are emitted literally into
EXTRA_CATALOG_INDEX_IMAGES. This can produce invalid image references passed to
install-dynamic-plugins while other parts of the chart do support templated values.
Code

charts/backstage/templates/_helpers.tpl[R291-301]

+{{- define "rhdh.catalogIndex.extraImagesEnvValue" -}}
+{{- $imgs := list -}}
+{{- range (.Values.global.catalogIndex.extraImages | default list) -}}
+  {{- $ref := printf "%s/%s:%s" .registry .repository .tag -}}
+  {{- if .name -}}
+    {{- $ref = printf "%s=%s" .name $ref -}}
+  {{- end -}}
+  {{- $imgs = append $imgs $ref -}}
+{{- end -}}
+{{- join "," $imgs -}}
+{{- end -}}
Relevance

⭐⭐⭐ High

Team commonly uses common.tplvalues.render for user-provided templated strings; inconsistency likely
fixed.

PR-#280
PR-#293
PR-#307

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The helper directly reads .registry, .repository, .tag, and .name without calling
tpl/common.tplvalues.render. In contrast, initContainers in the deployment are rendered with
common.tplvalues.render, and the default values demonstrate templated strings (e.g., `image: '{{
include "backstage.image" . }}'`), so users may reasonably expect templating to work consistently.

charts/backstage/templates/_helpers.tpl[286-301]
charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[117-124]
charts/backstage/values.yaml[389-401]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rhdh.catalogIndex.extraImagesEnvValue` does not template-render `extraImages` values. If a user uses `{{ ... }}` in `extraImages` fields, the chart will emit the literal braces into `EXTRA_CATALOG_INDEX_IMAGES`, producing invalid image refs.

### Issue Context
The chart already supports templated values elsewhere via `common.tplvalues.render` (notably for initContainers), so this is an inconsistency.

### Fix Focus Areas
- charts/backstage/templates/_helpers.tpl[286-301]

### Suggested fix approach
- Capture the root context before `range` (e.g., `{{$root := .}}`).
- For each item, render the whole object with `common.tplvalues.render` using `$root` as context, then `fromYaml` it, and build the ref from the rendered fields.
 - Example shape:
   - `$item := include "common.tplvalues.render" (dict "value" . "context" $root) | fromYaml`
   - then use `$item.registry`, `$item.repository`, `$item.tag`, `$item.name`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. extraImages.name delimiter unsafe 🐞 Bug ☼ Reliability
Description
extraImages.name is not constrained, but EXTRA_CATALOG_INDEX_IMAGES uses ',' to separate entries and
'=' for named entries, so names containing these delimiters make the env var value
ambiguous/unparseable. This can break extra image parsing at runtime.
Code

charts/backstage/values.schema.json[R43-46]

+                                    "name": {
+                                        "title": "Optional name for the extra catalog index image. Produces cleaner extraction directory names (e.g., /extra/community/).",
+                                        "type": "string"
+                                    },
Relevance

⭐⭐ Medium

Edge-case schema constraint; no history of enforcing delimiter restrictions in values.schema.json.

PR-#225
PR-#280
PR-#303

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Docs explicitly define the env var format as a comma-separated list with optional name=...
prefixes, while the schema allows any string for name and the helper uses name directly to build
name=ref and joins entries with commas. Therefore, delimiter characters in name can produce
malformed values the chart does not prevent.

docs/catalog-index-configuration.md[37-40]
charts/backstage/templates/_helpers.tpl[291-301]
charts/backstage/values.schema.json[35-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`extraImages.name` can contain `,` or `=`, which conflicts with the `EXTRA_CATALOG_INDEX_IMAGES` encoding (`comma`-separated entries and `name=ref` for named entries).

### Issue Context
The docs specify the encoding, but neither the JSON schema nor the templates validate/sanitize the name.

### Fix Focus Areas
- charts/backstage/values.schema.tmpl.json[134-164]
- charts/backstage/values.schema.json[38-66]
- charts/backstage/templates/_helpers.tpl[291-301]

### Suggested fix approach
- Add a JSON schema `pattern` for `name` that excludes `,` and `=` (and optionally `/`), e.g. `^[A-Za-z0-9._-]+$`.
- Optionally add a Helm template guard in the helper:
 - `{{- if (or (contains "," .name) (contains "=" .name)) -}} {{- fail "extraImages.name must not contain ',' or '='" -}} {{- end -}}`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit f41ae10

Results up to commit a9a853c


🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)


Remediation recommended
1. Duplicate env var entries 🐞 Bug ≡ Correctness
Description
The deployment template unconditionally appends EXTRA_CATALOG_INDEX_IMAGES to the
install-dynamic-plugins init container env when extra images are set, even if the user already
defined that env var. This can result in duplicate env var names and silently override/alter the
effective value at runtime.
Code

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[R119-123]

+        {{- $container := include "common.tplvalues.render" (dict "value" . "context" $) | fromYaml }}
+        {{- if and $extraCatalogImages (eq (index $container "name") "install-dynamic-plugins") }}
+        {{- $_ := set $container "env" (append (default list (index $container "env")) (dict "name" "EXTRA_CATALOG_INDEX_IMAGES" "value" $extraCatalogImages)) }}
+        {{- end }}
+        - {{ $container | toYaml | nindent 10 | trim }}
Relevance

⭐⭐⭐ High

Duplicate env var names in list can change runtime value order-dependently; likely treated as real
correctness bug.

PR-#280
PR-#293
PR-#325

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The initContainers rendering logic always appends a new EXTRA_CATALOG_INDEX_IMAGES entry and does
not check for an existing one. Because env is a list, this can produce duplicate name entries and
the effective value becomes order-dependent.

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]
charts/backstage/values.yaml[368-403]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EXTRA_CATALOG_INDEX_IMAGES` is appended to the init container env list without checking for an existing entry, which can create duplicate env var names and ambiguous behavior.

### Issue Context
Users can override/extend `upstream.backstage.initContainers[*].env` in values; if they already define `EXTRA_CATALOG_INDEX_IMAGES`, the chart will add a second entry.

### Fix Focus Areas
- charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]

### Suggested fix approach
- Before appending, scan `$container.env` for an item with `name: EXTRA_CATALOG_INDEX_IMAGES`.
- If found, either (a) leave it unchanged (treat user override as authoritative) or (b) replace its `value` with `$extraCatalogImages`.
- Only append a new env var if it does not already exist.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unrendered extraImages templates 🐞 Bug ≡ Correctness
Description
The rhdh.catalogIndex.extraImagesEnvValue helper builds image references from raw values without tpl
rendering, so any templated strings in extraImages fields are emitted literally into
EXTRA_CATALOG_INDEX_IMAGES. This can produce invalid image references passed to
install-dynamic-plugins while other parts of the chart do support templated values.
Code

charts/backstage/templates/_helpers.tpl[R291-301]

+{{- define "rhdh.catalogIndex.extraImagesEnvValue" -}}
+{{- $imgs := list -}}
+{{- range (.Values.global.catalogIndex.extraImages | default list) -}}
+  {{- $ref := printf "%s/%s:%s" .registry .repository .tag -}}
+  {{- if .name -}}
+    {{- $ref = printf "%s=%s" .name $ref -}}
+  {{- end -}}
+  {{- $imgs = append $imgs $ref -}}
+{{- end -}}
+{{- join "," $imgs -}}
+{{- end -}}
Relevance

⭐⭐⭐ High

Team commonly uses common.tplvalues.render for user-provided templated strings; inconsistency likely
fixed.

PR-#280
PR-#293
PR-#307

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The helper directly reads .registry, .repository, .tag, and .name without calling
tpl/common.tplvalues.render. In contrast, initContainers in the deployment are rendered with
common.tplvalues.render, and the default values demonstrate templated strings (e.g., `image: '{{
include "backstage.image" . }}'`), so users may reasonably expect templating to work consistently.

charts/backstage/templates/_helpers.tpl[286-301]
charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[117-124]
charts/backstage/values.yaml[389-401]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rhdh.catalogIndex.extraImagesEnvValue` does not template-render `extraImages` values. If a user uses `{{ ... }}` in `extraImages` fields, the chart will emit the literal braces into `EXTRA_CATALOG_INDEX_IMAGES`, producing invalid image refs.

### Issue Context
The chart already supports templated values elsewhere via `common.tplvalues.render` (notably for initContainers), so this is an inconsistency.

### Fix Focus Areas
- charts/backstage/templates/_helpers.tpl[286-301]

### Suggested fix approach
- Capture the root context before `range` (e.g., `{{$root := .}}`).
- For each item, render the whole object with `common.tplvalues.render` using `$root` as context, then `fromYaml` it, and build the ref from the rendered fields.
 - Example shape:
   - `$item := include "common.tplvalues.render" (dict "value" . "context" $root) | fromYaml`
   - then use `$item.registry`, `$item.repository`, `$item.tag`, `$item.name`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. extraImages.name delimiter unsafe 🐞 Bug ☼ Reliability
Description
extraImages.name is not constrained, but EXTRA_CATALOG_INDEX_IMAGES uses ',' to separate entries and
'=' for named entries, so names containing these delimiters make the env var value
ambiguous/unparseable. This can break extra image parsing at runtime.
Code

charts/backstage/values.schema.json[R43-46]

+                                    "name": {
+                                        "title": "Optional name for the extra catalog index image. Produces cleaner extraction directory names (e.g., /extra/community/).",
+                                        "type": "string"
+                                    },
Relevance

⭐⭐ Medium

Edge-case schema constraint; no history of enforcing delimiter restrictions in values.schema.json.

PR-#225
PR-#280
PR-#303

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Docs explicitly define the env var format as a comma-separated list with optional name=...
prefixes, while the schema allows any string for name and the helper uses name directly to build
name=ref and joins entries with commas. Therefore, delimiter characters in name can produce
malformed values the chart does not prevent.

docs/catalog-index-configuration.md[37-40]
charts/backstage/templates/_helpers.tpl[291-301]
charts/backstage/values.schema.json[35-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`extraImages.name` can contain `,` or `=`, which conflicts with the `EXTRA_CATALOG_INDEX_IMAGES` encoding (`comma`-separated entries and `name=ref` for named entries).

### Issue Context
The docs specify the encoding, but neither the JSON schema nor the templates validate/sanitize the name.

### Fix Focus Areas
- charts/backstage/values.schema.tmpl.json[134-164]
- charts/backstage/values.schema.json[38-66]
- charts/backstage/templates/_helpers.tpl[291-301]

### Suggested fix approach
- Add a JSON schema `pattern` for `name` that excludes `,` and `=` (and optionally `/`), e.g. `^[A-Za-z0-9._-]+$`.
- Optionally add a Helm template guard in the helper:
 - `{{- if (or (contains "," .name) (contains "=" .name)) -}} {{- fail "extraImages.name must not contain ',' or '='" -}} {{- end -}}`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@sonarqubecloud
Copy link
Copy Markdown

@rm3l
Copy link
Copy Markdown
Member Author

rm3l commented Apr 24, 2026

/agentic_review

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Apr 24, 2026

Persistent review updated to latest commit f41ae10

@rm3l rm3l marked this pull request as ready for review April 24, 2026 09:04
@rm3l rm3l requested a review from a team as a code owner April 24, 2026 09:04
@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Apr 24, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (1)

Grey Divider


Remediation recommended

1. EXTRA_CATALOG_INDEX_IMAGES not enforced 📎 Requirement gap ≡ Correctness ⭐ New
Description
When global.catalogIndex.extraImages is set, the template skips setting
EXTRA_CATALOG_INDEX_IMAGES if the initContainer already defines it, so the rendered value may not
reflect the configured extraImages list. This can silently ignore the Helm values-based
configuration and break multi-image discovery behavior.
Code

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[R121-125]

+        {{- $hasExisting := false }}
+        {{- range (default list (index $container "env")) }}
+          {{- if eq (default "" .name) "EXTRA_CATALOG_INDEX_IMAGES" }}{{ $hasExisting = true }}{{ end }}
+        {{- end }}
+        {{- if not $hasExisting }}
Relevance

⭐⭐⭐ High

Team previously rejected “skip if exists” patterns; values-based env var should be
enforced/overridden (see PR #186).

PR-#186
PR-#280
PR-#293

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2 requires the init container’s EXTRA_CATALOG_INDEX_IMAGES value to be
constructed from global.catalogIndex.extraImages when set. The template explicitly detects an
existing EXTRA_CATALOG_INDEX_IMAGES env var and then does not set/override it, so the env var can
differ from (and not represent) the configured extraImages list.

Inject EXTRA_CATALOG_INDEX_IMAGES env var into the RHDH init container based on extraImages
charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[121-125]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
When `global.catalogIndex.extraImages` is configured, the Helm template should ensure `EXTRA_CATALOG_INDEX_IMAGES` in the `install-dynamic-plugins` init container reflects that list. Currently, if the user already defines `EXTRA_CATALOG_INDEX_IMAGES` in `.Values.backstage.initContainers[].env`, the chart skips setting it, which can make `extraImages` ineffective.

## Issue Context
Compliance requires the chart to construct/inject the env var value from `global.catalogIndex.extraImages` when it is set.

## Fix Focus Areas
- charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[121-125]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Duplicate env var entries 🐞 Bug ≡ Correctness
Description
The deployment template unconditionally appends EXTRA_CATALOG_INDEX_IMAGES to the
install-dynamic-plugins init container env when extra images are set, even if the user already
defined that env var. This can result in duplicate env var names and silently override/alter the
effective value at runtime.
Code

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[R119-123]

+        {{- $container := include "common.tplvalues.render" (dict "value" . "context" $) | fromYaml }}
+        {{- if and $extraCatalogImages (eq (index $container "name") "install-dynamic-plugins") }}
+        {{- $_ := set $container "env" (append (default list (index $container "env")) (dict "name" "EXTRA_CATALOG_INDEX_IMAGES" "value" $extraCatalogImages)) }}
+        {{- end }}
+        - {{ $container | toYaml | nindent 10 | trim }}
Relevance

⭐⭐⭐ High

Duplicate env var names in list can change runtime value order-dependently; likely treated as real
correctness bug.

PR-#280
PR-#293
PR-#325

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The initContainers rendering logic always appends a new EXTRA_CATALOG_INDEX_IMAGES entry and does
not check for an existing one. Because env is a list, this can produce duplicate name entries and
the effective value becomes order-dependent.

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]
charts/backstage/values.yaml[368-403]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EXTRA_CATALOG_INDEX_IMAGES` is appended to the init container env list without checking for an existing entry, which can create duplicate env var names and ambiguous behavior.

### Issue Context
Users can override/extend `upstream.backstage.initContainers[*].env` in values; if they already define `EXTRA_CATALOG_INDEX_IMAGES`, the chart will add a second entry.

### Fix Focus Areas
- charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]

### Suggested fix approach
- Before appending, scan `$container.env` for an item with `name: EXTRA_CATALOG_INDEX_IMAGES`.
- If found, either (a) leave it unchanged (treat user override as authoritative) or (b) replace its `value` with `$extraCatalogImages`.
- Only append a new env var if it does not already exist.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Unrendered extraImages templates 🐞 Bug ≡ Correctness
Description
The rhdh.catalogIndex.extraImagesEnvValue helper builds image references from raw values without tpl
rendering, so any templated strings in extraImages fields are emitted literally into
EXTRA_CATALOG_INDEX_IMAGES. This can produce invalid image references passed to
install-dynamic-plugins while other parts of the chart do support templated values.
Code

charts/backstage/templates/_helpers.tpl[R291-301]

+{{- define "rhdh.catalogIndex.extraImagesEnvValue" -}}
+{{- $imgs := list -}}
+{{- range (.Values.global.catalogIndex.extraImages | default list) -}}
+  {{- $ref := printf "%s/%s:%s" .registry .repository .tag -}}
+  {{- if .name -}}
+    {{- $ref = printf "%s=%s" .name $ref -}}
+  {{- end -}}
+  {{- $imgs = append $imgs $ref -}}
+{{- end -}}
+{{- join "," $imgs -}}
+{{- end -}}
Relevance

⭐⭐⭐ High

Team commonly uses common.tplvalues.render for user-provided templated strings; inconsistency likely
fixed.

PR-#280
PR-#293
PR-#307

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The helper directly reads .registry, .repository, .tag, and .name without calling
tpl/common.tplvalues.render. In contrast, initContainers in the deployment are rendered with
common.tplvalues.render, and the default values demonstrate templated strings (e.g., `image: '{{
include "backstage.image" . }}'`), so users may reasonably expect templating to work consistently.

charts/backstage/templates/_helpers.tpl[286-301]
charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[117-124]
charts/backstage/values.yaml[389-401]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rhdh.catalogIndex.extraImagesEnvValue` does not template-render `extraImages` values. If a user uses `{{ ... }}` in `extraImages` fields, the chart will emit the literal braces into `EXTRA_CATALOG_INDEX_IMAGES`, producing invalid image refs.

### Issue Context
The chart already supports templated values elsewhere via `common.tplvalues.render` (notably for initContainers), so this is an inconsistency.

### Fix Focus Areas
- charts/backstage/templates/_helpers.tpl[286-301]

### Suggested fix approach
- Capture the root context before `range` (e.g., `{{$root := .}}`).
- For each item, render the whole object with `common.tplvalues.render` using `$root` as context, then `fromYaml` it, and build the ref from the rendered fields.
 - Example shape:
   - `$item := include "common.tplvalues.render" (dict "value" . "context" $root) | fromYaml`
   - then use `$item.registry`, `$item.repository`, `$item.tag`, `$item.name`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. extraImages.name delimiter unsafe 🐞 Bug ☼ Reliability
Description
extraImages.name is not constrained, but EXTRA_CATALOG_INDEX_IMAGES uses ',' to separate entries and
'=' for named entries, so names containing these delimiters make the env var value
ambiguous/unparseable. This can break extra image parsing at runtime.
Code

charts/backstage/values.schema.json[R43-46]

+                                    "name": {
+                                        "title": "Optional name for the extra catalog index image. Produces cleaner extraction directory names (e.g., /extra/community/).",
+                                        "type": "string"
+                                    },
Relevance

⭐⭐ Medium

Edge-case schema constraint; no history of enforcing delimiter restrictions in values.schema.json.

PR-#225
PR-#280
PR-#303

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Docs explicitly define the env var format as a comma-separated list with optional name=...
prefixes, while the schema allows any string for name and the helper uses name directly to build
name=ref and joins entries with commas. Therefore, delimiter characters in name can produce
malformed values the chart does not prevent.

docs/catalog-index-configuration.md[37-40]
charts/backstage/templates/_helpers.tpl[291-301]
charts/backstage/values.schema.json[35-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`extraImages.name` can contain `,` or `=`, which conflicts with the `EXTRA_CATALOG_INDEX_IMAGES` encoding (`comma`-separated entries and `name=ref` for named entries).

### Issue Context
The docs specify the encoding, but neither the JSON schema nor the templates validate/sanitize the name.

### Fix Focus Areas
- charts/backstage/values.schema.tmpl.json[134-164]
- charts/backstage/values.schema.json[38-66]
- charts/backstage/templates/_helpers.tpl[291-301]

### Suggested fix approach
- Add a JSON schema `pattern` for `name` that excludes `,` and `=` (and optionally `/`), e.g. `^[A-Za-z0-9._-]+$`.
- Optionally add a Helm template guard in the helper:
 - `{{- if (or (contains "," .name) (contains "=" .name)) -}} {{- fail "extraImages.name must not contain ',' or '='" -}} {{- end -}}`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit f41ae10

Results up to commit a9a853c


🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)


Remediation recommended
1. Duplicate env var entries 🐞 Bug ≡ Correctness
Description
The deployment template unconditionally appends EXTRA_CATALOG_INDEX_IMAGES to the
install-dynamic-plugins init container env when extra images are set, even if the user already
defined that env var. This can result in duplicate env var names and silently override/alter the
effective value at runtime.
Code

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[R119-123]

+        {{- $container := include "common.tplvalues.render" (dict "value" . "context" $) | fromYaml }}
+        {{- if and $extraCatalogImages (eq (index $container "name") "install-dynamic-plugins") }}
+        {{- $_ := set $container "env" (append (default list (index $container "env")) (dict "name" "EXTRA_CATALOG_INDEX_IMAGES" "value" $extraCatalogImages)) }}
+        {{- end }}
+        - {{ $container | toYaml | nindent 10 | trim }}
Relevance

⭐⭐⭐ High

Duplicate env var names in list can change runtime value order-dependently; likely treated as real
correctness bug.

PR-#280
PR-#293
PR-#325

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The initContainers rendering logic always appends a new EXTRA_CATALOG_INDEX_IMAGES entry and does
not check for an existing one. Because env is a list, this can produce duplicate name entries and
the effective value becomes order-dependent.

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]
charts/backstage/values.yaml[368-403]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EXTRA_CATALOG_INDEX_IMAGES` is appended to the init container env list without checking for an existing entry, which can create duplicate env var names and ambiguous behavior.

### Issue Context
Users can override/extend `upstream.backstage.initContainers[*].env` in values; if they already define `EXTRA_CATALOG_INDEX_IMAGES`, the chart will add a second entry.

### Fix Focus Areas
- charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[115-125]

### Suggested fix approach
- Before appending, scan `$container.env` for an item with `name: EXTRA_CATALOG_INDEX_IMAGES`.
- If found, either (a) leave it unchanged (treat user override as authoritative) or (b) replace its `value` with `$extraCatalogImages`.
- Only append a new env var if it does not already exist.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Unrendered extraImages templates 🐞 Bug ≡ Correctness
Description
The rhdh.catalogIndex.extraImagesEnvValue helper builds image references from raw values without tpl
rendering, so any templated strings in extraImages fields are emitted literally into
EXTRA_CATALOG_INDEX_IMAGES. This can produce invalid image references passed to
install-dynamic-plugins while other parts of the chart do support templated values.
Code

charts/backstage/templates/_helpers.tpl[R291-301]

+{{- define "rhdh.catalogIndex.extraImagesEnvValue" -}}
+{{- $imgs := list -}}
+{{- range (.Values.global.catalogIndex.extraImages | default list) -}}
+  {{- $ref := printf "%s/%s:%s" .registry .repository .tag -}}
+  {{- if .name -}}
+    {{- $ref = printf "%s=%s" .name $ref -}}
+  {{- end -}}
+  {{- $imgs = append $imgs $ref -}}
+{{- end -}}
+{{- join "," $imgs -}}
+{{- end -}}
Relevance

⭐⭐⭐ High

Team commonly uses common.tplvalues.render for user-provided templated strings; inconsistency likely
fixed.

PR-#280
PR-#293
PR-#307

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The helper directly reads .registry, .repository, .tag, and .name without calling
tpl/common.tplvalues.render. In contrast, initContainers in the deployment are rendered with
common.tplvalues.render, and the default values demonstrate templated strings (e.g., `image: '{{
include "backstage.image" . }}'`), so users may reasonably expect templating to work consistently.

charts/backstage/templates/_helpers.tpl[286-301]
charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[117-124]
charts/backstage/values.yaml[389-401]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`rhdh.catalogIndex.extraImagesEnvValue` does not template-render `extraImages` values. If a user uses `{{ ... }}` in `extraImages` fields, the chart will emit the literal braces into `EXTRA_CATALOG_INDEX_IMAGES`, producing invalid image refs.

### Issue Context
The chart already supports templated values elsewhere via `common.tplvalues.render` (notably for initContainers), so this is an inconsistency.

### Fix Focus Areas
- charts/backstage/templates/_helpers.tpl[286-301]

### Suggested fix approach
- Capture the root context before `range` (e.g., `{{$root := .}}`).
- For each item, render the whole object with `common.tplvalues.render` using `$root` as context, then `fromYaml` it, and build the ref from the rendered fields.
 - Example shape:
   - `$item := include "common.tplvalues.render" (dict "value" . "context" $root) | fromYaml`
   - then use `$item.registry`, `$item.repository`, `$item.tag`, `$item.name`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. extraImages.name delimiter unsafe 🐞 Bug ☼ Reliability
Description
extraImages.name is not constrained, but EXTRA_CATALOG_INDEX_IMAGES uses ',' to separate entries and
'=' for named entries, so names containing these delimiters make the env var value
ambiguous/unparseable. This can break extra image parsing at runtime.
Code

charts/backstage/values.schema.json[R43-46]

+                                    "name": {
+                                        "title": "Optional name for the extra catalog index image. Produces cleaner extraction directory names (e.g., /extra/community/).",
+                                        "type": "string"
+                                    },
Relevance

⭐⭐ Medium

Edge-case schema constraint; no history of enforcing delimiter restrictions in values.schema.json.

PR-#225
PR-#280
PR-#303

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Docs explicitly define the env var format as a comma-separated list with optional name=...
prefixes, while the schema allows any string for name and the helper uses name directly to build
name=ref and joins entries with commas. Therefore, delimiter characters in name can produce
malformed values the chart does not prevent.

docs/catalog-index-configuration.md[37-40]
charts/backstage/templates/_helpers.tpl[291-301]
charts/backstage/values.schema.json[35-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`extraImages.name` can contain `,` or `=`, which conflicts with the `EXTRA_CATALOG_INDEX_IMAGES` encoding (`comma`-separated entries and `name=ref` for named entries).

### Issue Context
The docs specify the encoding, but neither the JSON schema nor the templates validate/sanitize the name.

### Fix Focus Areas
- charts/backstage/values.schema.tmpl.json[134-164]
- charts/backstage/values.schema.json[38-66]
- charts/backstage/templates/_helpers.tpl[291-301]

### Suggested fix approach
- Add a JSON schema `pattern` for `name` that excludes `,` and `=` (and optionally `/`), e.g. `^[A-Za-z0-9._-]+$`.
- Optionally add a Helm template guard in the helper:
 - `{{- if (or (contains "," .name) (contains "=" .name)) -}} {{- fail "extraImages.name must not contain ',' or '='" -}} {{- end -}}`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Support configuring multiple catalog index images for plugin discovery

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add global.catalogIndex.extraImages field for multiple catalog sources
• Implement helper template to build comma-separated environment variable
• Inject EXTRA_CATALOG_INDEX_IMAGES into install-dynamic-plugins container
• Update documentation and schema with extra images configuration
• Bump chart version from 5.8.0 to 5.9.0
Diagram
flowchart LR
  A["global.catalogIndex.extraImages"] -->|"Helper template"| B["rhdh.catalogIndex.extraImagesEnvValue"]
  B -->|"Comma-separated list"| C["EXTRA_CATALOG_INDEX_IMAGES env var"]
  C -->|"Injected into"| D["install-dynamic-plugins init container"]
  D -->|"Extracts catalog entities"| E["Extensions UI"]
Loading

Grey Divider

File Changes

1. charts/backstage/Chart.yaml ⚙️ Configuration changes +1/-1

Bump chart version to 5.9.0

charts/backstage/Chart.yaml


2. charts/backstage/README.md 📝 Documentation +7/-4

Update version and document extra catalog images

charts/backstage/README.md


3. charts/backstage/README.md.gotmpl 📝 Documentation +3/-1

Add extra catalog images configuration documentation

charts/backstage/README.md.gotmpl


View more (6)
4. charts/backstage/templates/_helpers.tpl ✨ Enhancement +22/-0

Add helper template for extra catalog images

charts/backstage/templates/_helpers.tpl


5. charts/backstage/values.schema.json ⚙️ Configuration changes +33/-0

Add JSON schema for extra catalog images

charts/backstage/values.schema.json


6. charts/backstage/values.schema.tmpl.json ⚙️ Configuration changes +33/-0

Add schema template for extra catalog images

charts/backstage/values.schema.tmpl.json


7. charts/backstage/values.yaml ⚙️ Configuration changes +12/-0

Add extraImages configuration with examples

charts/backstage/values.yaml


8. charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml ✨ Enhancement +14/-1

Conditionally inject EXTRA_CATALOG_INDEX_IMAGES environment variable

charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml


9. docs/catalog-index-configuration.md 📝 Documentation +25/-0

Document extra catalog index images configuration

docs/catalog-index-configuration.md


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant