Skip to content

Conversation

@rm3l
Copy link
Member

@rm3l rm3l commented Jan 9, 2026

Description

As discussed in https://redhat-internal.slack.com/archives/C04CUSD4JSG/p1767790419980379, we need to extract the catalog entities from the index image to the /marketplace (to be replaced by /extensions in redhat-developer/rhdh-plugins#2006) folder, so that the extensions backend providers can automatically discover them. Otherwise, there are no plugins displayed in the RHDH Extensions UI.

redhat-developer/rhdh#3970 added support for specifying the extraction dir via a new CATALOG_ENTITIES_EXTRACT_DIR env var, which we now need to set in the Install Methods (and additionally add the right volume mounts - we cannot create that folder right in the main container because the root filesystem is read-only for security purposes).

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

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

Before

image

With the changes here

image

@openshift-ci
Copy link

openshift-ci bot commented Jan 9, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kim-tsao for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rm3l rm3l changed the title fix: add volume for extension catalog entities in 1.9+ [RHIDP-11292] fix: add volume for extension catalog entities [RHIDP-11292] Jan 9, 2026
@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHIDP-11292 - Partially compliant

Compliant requirements:

  • Add an additional /marketplace volume mount into the RHDH container.
  • Ensure catalog entities extraction directory is set to the new volume/mount (so extracted entities land on that mounted storage).

Non-compliant requirements:

(none)

Requires further human verification:

  • Validate at runtime that catalog entity extraction actually writes to CATALOG_ENTITIES_EXTRACT_DIR=/extensions and that /marketplace alias behavior remains backward compatible for existing extensions.
  • Validate that the volume is mounted in all operator-managed deployment variants (e.g., upgrades, different profiles) and not only in the default-config templates/manifests.
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Backward-compat risk

The same volume extensions-catalog is mounted to both /extensions and legacy /marketplace with readOnly: true on the legacy path. Confirm that anything still expecting to write under /marketplace will not break (even indirectly), and that all writers now use /extensions (via CATALOG_ENTITIES_EXTRACT_DIR).

- name: extensions-catalog
  mountPath: /extensions
- name: extensions-catalog
  # TODO(asoro): legacy path for backward compatibility. Will be removed in a near future.
  mountPath: /marketplace
  readOnly: true
- mountPath: /tmp
Generated metadata

The createdAt field changed; if this file is expected to be generated, ensure it is updated by the correct release/bundle tooling and won’t cause noisy diffs or conflicts in downstream automation.

createdAt: "2026-01-09T20:08:15Z"
description: Red Hat Developer Hub is a Red Hat supported version of Backstage.
📚 Focus areas based on broader codebase context

Consistency

The new CATALOG_ENTITIES_EXTRACT_DIR env var value is single-quoted ('/extensions') while other env var path values in similar manifests are left unquoted. Align the quoting style (prefer unquoted or consistent double quotes) to avoid accidental escaping/formatting differences when these manifests are embedded or templated. (Ref 2)

  - name: CATALOG_ENTITIES_EXTRACT_DIR
    value: '/extensions'
volumeMounts:

Reference reasoning: Similar deployment/config YAML in the repo sets filesystem path env vars as plain scalars (e.g., /opt/app-root/src/.npmrc.dynamic-plugins), indicating a consistent convention that avoids introducing unnecessary quoting differences.

📄 References
  1. redhat-developer/rhdh-chart/charts/backstage/values.yaml [160-178]
  2. redhat-developer/rhdh-operator/pkg/model/testdata/janus-deployment.yaml [36-58]
  3. redhat-developer/rhdh-operator/pkg/model/testdata/janus-deployment.yaml [94-96]
  4. redhat-developer/rhdh-operator/examples/pvc-dp-cache.yaml [60-83]
  5. redhat-developer/rhdh-operator/examples/pvc-dp-cache.yaml [84-94]
  6. redhat-developer/rhdh-operator/integration_tests/testdata/rhdh-replace-dynaplugin-root.yaml [1-14]
  7. redhat-developer/rhdh-chart/charts/backstage/ci/with-custom-dynamic-pvc-claim-spec-values.yaml [2-27]
  8. redhat-developer/rhdh-chart/charts/backstage/values.yaml [37-44]

@rhdh-qodo-merge rhdh-qodo-merge bot added Review effort 3/5 enhancement New feature or request labels Jan 9, 2026
@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Jan 9, 2026

PR Type

(Describe updated until commit d7c78fd)

Enhancement


Description

  • Add extensions-catalog emptyDir volume to RHDH pod for catalog entity extraction

  • Set CATALOG_ENTITIES_EXTRACT_DIR environment variable to /extensions in init container

  • Mount extensions volume in both init and main containers with legacy /marketplace path support

  • Update tests and manifests to reflect new volume configuration


File Walkthrough

Relevant files
Tests
rhdh-config_test.go
Update tests for extensions catalog volume                             

integration_tests/rhdh-config_test.go

  • Add extensions-catalog volume mount assertion to expected container
    mounts
  • Update volume count expectation from 7 to 8 volumes
  • Add /extensions and /marketplace (read-only) mount path validations
+12/-1   
Formatting
dynamic-plugins_test.go
Minor formatting cleanup                                                                 

pkg/model/dynamic-plugins_test.go

  • Remove extra blank line for code formatting
+0/-1     
Configuration changes
backstage-operator.clusterserviceversion.yaml
Update operator manifest timestamp                                             

bundle/rhdh/manifests/backstage-operator.clusterserviceversion.yaml

  • Update createdAt timestamp to reflect manifest regeneration
+1/-1     
rhdh-default-config_v1_configmap.yaml
Configure extensions catalog volume and extraction directory

bundle/rhdh/manifests/rhdh-default-config_v1_configmap.yaml

  • Add extensions-catalog emptyDir volume definition
  • Add CATALOG_ENTITIES_EXTRACT_DIR environment variable set to
    /extensions
  • Mount extensions-catalog volume in init container at /extensions
  • Mount extensions-catalog volume in main container at both /extensions
    and /marketplace (read-only for backward compatibility)
+12/-0   
deployment.yaml
Configure extensions catalog volume and extraction directory

config/profile/rhdh/default-config/deployment.yaml

  • Add extensions-catalog emptyDir volume definition
  • Add CATALOG_ENTITIES_EXTRACT_DIR environment variable set to
    /extensions
  • Mount extensions-catalog volume in init container at /extensions
  • Mount extensions-catalog volume in main container at both /extensions
    and /marketplace (read-only for backward compatibility)
+12/-0   
install.yaml
Configure extensions catalog volume and extraction directory

dist/rhdh/install.yaml

  • Add extensions-catalog emptyDir volume definition
  • Add CATALOG_ENTITIES_EXTRACT_DIR environment variable set to
    /extensions
  • Mount extensions-catalog volume in init container at /extensions
  • Mount extensions-catalog volume in main container at both /extensions
    and /marketplace (read-only for backward compatibility)
+12/-0   

@rhdh-qodo-merge
Copy link

rhdh-qodo-merge bot commented Jan 9, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Avoid dual volume mounts for compatibility

Instead of mounting the same volume to both /extensions and /marketplace for
backward compatibility, consider using a symbolic link. This simplifies the
Kubernetes manifests and avoids a confusing dual-mount pattern.

Examples:

config/profile/rhdh/default-config/deployment.yaml [158-163]
            - name: extensions-catalog
              mountPath: /extensions
            - name: extensions-catalog
              # TODO(asoro): legacy path for backward compatibility. Will be removed in a near future.
              mountPath: /marketplace
              readOnly: true
bundle/rhdh/manifests/rhdh-default-config_v1_configmap.yaml [334-339]
                - name: extensions-catalog
                  mountPath: /extensions
                - name: extensions-catalog
                  # TODO(asoro): legacy path for backward compatibility. Will be removed in a near future.
                  mountPath: /marketplace
                  readOnly: true

Solution Walkthrough:

Before:

# In deployment.yaml
spec:
  template:
    spec:
      containers:
      - name: backstage
        volumeMounts:
        - name: extensions-catalog
          mountPath: /extensions
        - name: extensions-catalog
          # TODO: legacy path for backward compatibility
          mountPath: /marketplace
          readOnly: true
        ...
      volumes:
      - name: extensions-catalog
        emptyDir: {}

After:

# In deployment.yaml
spec:
  template:
    spec:
      containers:
      - name: backstage
        command: ["/bin/sh", "-c"]
        args:
          - ln -s /extensions /marketplace && <original_command>
        volumeMounts:
        - name: extensions-catalog
          mountPath: /extensions
        ...
      volumes:
      - name: extensions-catalog
        emptyDir: {}
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies an unusual dual-volume-mount pattern for backward compatibility and proposes cleaner, more idiomatic alternatives like using a symbolic link, which improves the maintainability of the Kubernetes manifests.

Medium
Security
Make extension mount read-only

Improve security by making the extensions-catalog volume mount at /extensions
read-only for the main container.

config/profile/rhdh/default-config/deployment.yaml [158-159]

 - name: extensions-catalog
   mountPath: /extensions
+  readOnly: true
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the /extensions volume mount could be made read-only for the main container, which is a good security practice.

Low
Organization
best practice
Strengthen volume mount assertions

Include the expected Name (and ReadOnly where applicable) for the new mounts and
assert against it, so the test ensures /extensions and /marketplace are backed
by extensions-catalog as intended.

integration_tests/rhdh-config_test.go [122-142]

 mainContainerExpectedVolumeMounts := []corev1.VolumeMount{
 	{
+		Name:      "dynamic-plugins-root",
 		MountPath: "/opt/app-root/src/dynamic-plugins-root",
 		SubPath:   "",
 	},
 	{
+		Name:      utils.GenerateVolumeNameFromCmOrSecret(model.AppConfigDefaultName(backstageName)),
 		MountPath: "/opt/app-root/src/default.app-config.yaml",
 		SubPath:   "default.app-config.yaml",
 	},
 	{
+		Name:      "extensions-catalog",
 		MountPath: "/extensions",
+		SubPath:   "",
 	},
 	{
+		Name:      "extensions-catalog",
 		MountPath: "/marketplace",
+		SubPath:   "",
 		ReadOnly:  true,
 	},
 	{
+		Name:      "temp",
 		MountPath: "/tmp",
 		SubPath:   "",
 	},
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Write assertions that fully validate critical configuration (e.g., volume mount Name/ReadOnly) to avoid false positives and improve test robustness.

Low
Document new extension catalog volume

Document the new extensions-catalog volume and the CATALOG_ENTITIES_EXTRACT_DIR
default (including the 1.9+ behavior and the /marketplace legacy mount and its
planned removal) so users understand where catalog entities are extracted and
how to override/disable it.

docs/configuration.md [1]

-...
+## Extension catalog entities (1.9+)
 
+Starting with **RHDH 1.9**, the Operator mounts an `emptyDir` volume named `extensions-catalog` and configures dynamic plugin catalog extraction into it.
+
+- Default extract directory: `CATALOG_ENTITIES_EXTRACT_DIR=/extensions`
+- Mounted paths:
+  - `/extensions` (read/write)
+  - `/marketplace` (read-only **legacy** path for backward compatibility; will be removed in a future release)
+
+If you customize the Deployment/Pod template, ensure the `extensions-catalog` volume and corresponding mounts are preserved, or set `CATALOG_ENTITIES_EXTRACT_DIR` to match your desired mount path.
+

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Keep documentation synchronized with implementation changes, specifying versions, defaults, and behavior changes to avoid user confusion when behavior evolves.

Low
General
Assert extension volume exists

Enhance the integration test by adding an explicit assertion to verify the
presence and configuration of the extensions-catalog volume.

integration_tests/rhdh-config_test.go [113]

 g.Expect(deploy.PodSpec().Volumes).To(HaveLen(8))
+{
+    found := false
+    for _, v := range deploy.PodSpec().Volumes {
+        if v.Name == "extensions-catalog" {
+            found = true
+            g.Expect(v.EmptyDir).NotTo(BeNil())
+        }
+    }
+    g.Expect(found).To(BeTrue())
+}
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: This suggestion improves the test by adding a more specific assertion for the extensions-catalog volume, making the test more robust against future changes.

Low
  • Update

@rm3l rm3l force-pushed the rhidp-11292-operator-add-a-new-marketplace-volume-into-the-rhdh-pod-and-set-catalog-entities-extraction-dir-to-it branch from a11ebb9 to d7c78fd Compare January 12, 2026 12:26
@sonarqubecloud
Copy link

Comment on lines +166 to +169
- name: extensions-catalog
# TODO(asoro): legacy path for backward compatibility. Will be removed in a near future.
mountPath: /marketplace
readOnly: true
Copy link
Member Author

@rm3l rm3l Jan 12, 2026

Choose a reason for hiding this comment

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

Might need to be removed when redhat-developer/rhdh-plugins#2006 is merged and a new catalog index image is built. or we can still keep it for now for backward compatibility.

@rm3l rm3l marked this pull request as ready for review January 12, 2026 15:54
@openshift-ci openshift-ci bot requested review from gazarenkov and zdrapela January 12, 2026 15:54
@rhdh-qodo-merge
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

RHIDP-11292 - Partially compliant

Compliant requirements:

  • Add an additional /marketplace volume mount into the RHDH container.

Non-compliant requirements:

(empty)

Requires further human verification:

  • Confirm at runtime that the extracted catalog entities are written to the intended directory and that the Extensions UI discovers them correctly with the new mount(s).
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Brittle Test

The test asserts an exact volume count (HaveLen(8)), which can become flaky as unrelated volumes are added/removed. Consider asserting presence of the new volume (extensions-catalog) and expected mounts/paths instead of a strict total count.

g.Expect(deploy.PodSpec().Volumes).To(HaveLen(8))
g.Expect(deploy.PodSpec().Containers).To(HaveLen(1))
mainCont := backstageContainer(*deploy.PodSpec())
g.Expect(mainCont.Args).To(HaveLen(4))
📚 Focus areas based on broader codebase context

VolumeMount Duplication

The same volume (extensions-catalog) is mounted twice in the same container at two different paths (/extensions and /marketplace), with only one of them marked readOnly. Validate that this duplication is intentional and doesn’t create confusing semantics (e.g., writes via /extensions still affect the data visible via /marketplace despite it being readOnly). (Ref 3, Ref 4)

- name: extensions-catalog
  mountPath: /extensions
- name: extensions-catalog
  # TODO(asoro): legacy path for backward compatibility. Will be removed in a near future.
  mountPath: /marketplace
  readOnly: true

Reference reasoning: Existing operator-managed deployment YAML consistently mounts each volume a single time per container (e.g., default-config and plugin-deps), avoiding multiple mount points to the same backing volume. Aligning with that pattern reduces ambiguity and makes access mode expectations clearer.

📄 References
  1. redhat-developer/rhdh-operator/pkg/model/testdata/janus-deployment.yaml [36-58]
  2. redhat-developer/rhdh-operator/dist/rhdh/install.yaml [2994-3005]
  3. redhat-developer/rhdh-operator/dist/backstage.io/install.yaml [2096-2100]
  4. redhat-developer/rhdh-operator/config/manager/deployment.yaml [91-95]
  5. redhat-developer/rhdh-operator/integration_tests/testdata/spec-deployment.yaml [1-23]
  6. redhat-developer/rhdh-operator/pkg/model/testdata/janus-deployment.yaml [94-96]
  7. redhat-developer/rhdh-operator/examples/pvc-dp-cache.yaml [60-83]
  8. redhat-developer/rhdh-operator/integration_tests/testdata/rhdh-replace-dynaplugin-root.yaml [1-14]

@rhdh-qodo-merge
Copy link

PR Code Suggestions ✨

No code suggestions found for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Review effort 3/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant