Skip to content

feat: migrate to nfs (WIP) - #5232

Open
Zaperex wants to merge 4 commits into
redhat-developer:mainfrom
Zaperex:migrate-to-NFS
Open

feat: migrate to nfs (WIP)#5232
Zaperex wants to merge 4 commits into
redhat-developer:mainfrom
Zaperex:migrate-to-NFS

Conversation

@Zaperex

@Zaperex Zaperex commented Aug 10, 2026

Copy link
Copy Markdown
Member

Description

Migrates statically installed OFS elements to the new frontend system

Which issue(s) does this PR fix

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

How to test changes / Special notes to the reviewer

Zaperex added 4 commits August 6, 2026 10:08
Assisted-By: Cursor
Signed-off-by: Frank Kong <frkong@redhat.com>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Assisted-By: Cursor
Signed-off-by: Frank Kong <frkong@redhat.com>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Assisted-By: Cursor
Signed-off-by: Frank Kong <frkong@redhat.com>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Frank Kong <frkong@redhat.com>
Assisted-By: Cursor

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@openshift-ci

openshift-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
11.2% Duplication on New Code (required ≤ 3%)
D Reliability Rating on New Code (required ≥ A)
C Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@Zaperex

Zaperex commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

/test all

@Zaperex Zaperex changed the title chore: migrate to nfs feat: migrate to nfs Aug 10, 2026
@Zaperex
Zaperex marked this pull request as ready for review August 10, 2026 15:32
@Zaperex Zaperex changed the title feat: migrate to nfs feat: migrate to nfs (WIP) Aug 10, 2026
@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Migrate default frontend to Backstage NFS and retain OFS as app-legacy

✨ Enhancement ⚙️ Configuration changes 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Make Backstage New Frontend System (NFS) packages/app the default shipped frontend.
• Rename the legacy OFS frontend to packages/app-legacy and add explicit legacy run scripts.
• Update dynamic plugin schema resolution and cluster-free Playwright harness/docs for NFS + Module
 Federation.
Diagram

graph TD
  H["Dev/E2E harness"] --> B["Backend"] --> SL["schemaLocator"] --> DP{{"Dynamic plugins"}}
  H --> A["NFS app (app)"] --> DFL["Dynamic features loader"] --> B
  B --> L["Legacy app (app-legacy)"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Hard cutover (remove OFS + legacy fallbacks in this PR)
  • ➕ Less long-term maintenance (single frontend, single schema layout).
  • ➕ Avoids keeping compatibility code paths that can mask stale plugins.
  • ➖ High rollout risk: breaks older dynamic plugins and any OFS-dependent environments.
  • ➖ Harder to debug issues without a quick rollback path.
2. Make schema path a build-time contract (no runtime fs probing)
  • ➕ Removes runtime existsSync checks and ambiguity; schema location is explicit.
  • ➕ Improves portability (e.g., different filesystem layouts) and reduces edge cases.
  • ➖ Requires coordinated change across plugin packaging/tooling and published artifacts.
  • ➖ Slower incremental adoption while both old and new plugin builds exist.
3. Explicit feature-flagged default selection for a deprecation window
  • ➕ Keeps a clear operator-controlled rollback during rollout.
  • ➕ Makes deprecation/removal timeline explicit and testable.
  • ➖ More permutations to document/test; extends dual-frontend overhead.

Recommendation: The incremental strategy in this PR is the right approach for a platform migration: NFS becomes the default, but app-legacy and schema fallback paths reduce disruption while the plugin ecosystem catches up. Follow up with a time-boxed plan to remove the legacy schema fallback once all supported plugins publish dist/.config-schema.json.

Files changed (22) +734 / -244

Enhancement (5) +403 / -27
App.tsxReplace Scalprum root with NFS createApp() feature composition +37/-27

Replace Scalprum root with NFS createApp() feature composition

• Builds the app root using 'createApp' + 'dynamicFrontendFeaturesLoader', composing upstream plugins and RHDH frontend modules (nav, user-settings, APIs, translations, theme).

packages/app/src/App.tsx

apisModule.tsAdd NFS frontend module registering core RHDH APIs +98/-0

Add NFS frontend module registering core RHDH APIs

• Introduces API blueprints for storage selection, learning-path client, and catalog graph with custom relations, exposed as an NFS frontend module.

packages/app/src/apis/apisModule.ts

Sidebar.tsxAdd NFS nav content blueprint for RHDH-branded sidebar +70/-0

Add NFS nav content blueprint for RHDH-branded sidebar

• Implements NFS navigation composition (search, menu ordering, settings group, and drawer toggle integration).

packages/app/src/modules/nav/Sidebar.tsx

InfoCard.tsxAdd build metadata InfoCard for Settings/General (NFS) +164/-0

Add build metadata InfoCard for Settings/General (NFS)

• Implements the Settings/General build-info card with copy-to-clipboard and show-more/show-less behavior, using config overrides and translations.

packages/app/src/modules/user-settings/InfoCard.tsx

schemaLocator.tsPrefer modern config schema output with legacy fallback +34/-0

Prefer modern config schema output with legacy fallback

• Adds a schema locator that prefers 'dist/.config-schema.json' (Backstage CLI) and falls back to legacy 'dist[-scalprum]/configSchema.json' during the transition.

packages/backend/src/schemaLocator.ts

Refactor (2) +8 / -11
index.tsxUpdate frontend entrypoint for NFS root rendering +6/-1

Update frontend entrypoint for NFS root rendering

• Switches to NFS-compatible startup: imports Backstage asset types/styles and renders the NFS app root via ReactDOM.

packages/app/src/index.tsx

index.tsUse extracted schemaLocator in dynamic plugin feature loader +2/-10

Use extracted schemaLocator in dynamic plugin feature loader

• Removes the inline schemaLocator implementation and replaces it with the shared helper.

packages/backend/src/index.ts

Tests (5) +96 / -36
playwright.legacy-local.config.tsRun cluster-free harness against NFS app and enable MF env +10/-8

Run cluster-free harness against NFS app and enable MF env

• Switches the frontend start command to 'backstage-cli' and injects 'ENABLE_STANDARD_MODULE_FEDERATION=true' for the backend process. Adds a temporary InfoCard validation spec to the suite.

e2e-tests/playwright.legacy-local.config.ts

tmp-infocard-check.spec.tsAdd temporary Playwright check for Settings/General InfoCard +16/-0

Add temporary Playwright check for Settings/General InfoCard

• Adds a small cluster-free spec validating the build-info InfoCard renders and expands on Settings/General.

e2e-tests/playwright/e2e/plugins/tmp-infocard-check.spec.ts

navigation.tsClarify legacy sidebar behavior comments +2/-2

Clarify legacy sidebar behavior comments

• Updates comments to refer to the legacy OFS sidebar behavior in navigation helpers.

e2e-tests/playwright/utils/ui-helper/navigation.ts

App.test.tsxUpdate app smoke test to render NFS app root +16/-26

Update app smoke test to render NFS app root

• Replaces Scalprum-focused test setup with 'renderWithEffects' against the NFS app root and updates required test config.

packages/app/src/App.test.tsx

schemaLocator.test.tsAdd unit tests for schemaLocator behavior +52/-0

Add unit tests for schemaLocator behavior

• Covers modern-path preference and backend/frontend legacy fallback paths based on plugin role/platform.

packages/backend/src/schemaLocator.test.ts

Documentation (2) +13 / -27
migrating-config-to-new-frontend-system.mdDocument NFS as default and MF enablement requirement +4/-15

Document NFS as default and MF enablement requirement

• Rewrites the guidance to treat NFS ('packages/app') as default and focuses on enabling standard Module Federation on the backend.

docs/dynamic-plugins/migrating-config-to-new-frontend-system.md

local-e2e-harness.mdUpdate harness docs for NFS + standard Module Federation +9/-12

Update harness docs for NFS + standard Module Federation

• Updates the cluster-free harness description to target NFS 'packages/app' and notes the backend must run with 'ENABLE_STANDARD_MODULE_FEDERATION=true'.

docs/e2e-tests/local-e2e-harness.md

Other (8) +214 / -143
e2e-cluster-free.yamlRename cluster-free E2E workflow labeling to NFS +1/-1

Rename cluster-free E2E workflow labeling to NFS

• Updates the workflow step name to reflect the harness targeting the NFS app.

.github/workflows/e2e-cluster-free.yaml

app-config.local-e2e.yamlUpdate local E2E overlay docs to NFS +1/-1

Update local E2E overlay docs to NFS

• Adjusts the header comment to document that the overlay targets the NFS 'packages/app' frontend.

app-config.local-e2e.yaml

ContainerfileRemove app-next package.json from container build inputs +0/-1

Remove app-next package.json from container build inputs

• Stops copying 'packages/app-next/package.json', aligning the container build with the new package layout.

build/containerfiles/Containerfile

package.jsonAdd dev script for running app-legacy alongside backend +1/-0

Add dev script for running app-legacy alongside backend

• Introduces 'dev:legacy' to run the backend filtered with the legacy frontend package.

package.json

package.jsonSwitch app workspace to NFS deps and build/start scripts +62/-47

Switch app workspace to NFS deps and build/start scripts

• Moves 'packages/app' to Backstage CLI NFS wiring, enables experimental Module Federation build, and updates dependencies to include NFS frontend packages and RHDH alpha modules.

packages/app/package.json

package.jsonEnable standard MF by default and add start:legacy +2/-3

Enable standard MF by default and add start:legacy

• Makes 'start' always set 'ENABLE_STANDARD_MODULE_FEDERATION=true' and introduces 'start:legacy' to serve 'app-legacy' via 'APP_CONFIG_app_packageName'. Removes the 'app-next' workspace dependency.

packages/backend/package.json

package.jsonRename app-next to app-legacy and restore OFS toolchain wiring +53/-49

Rename app-next to app-legacy and restore OFS toolchain wiring

• Renames the legacy package to 'app-legacy' and restores janus-cli-based start/build scripts and OFS dependency shape consistent with the legacy frontend.

packages/app-legacy/package.json

yarn.lockLockfile refresh for workspace rename and NFS dependencies +94/-41

Lockfile refresh for workspace rename and NFS dependencies

• Updates dependency resolutions to reflect the new NFS app dependencies and removal/rename of the app-next workspace.

yarn.lock

@rhdh-qodo-merge

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 🔗 Cross-repo conflicts (1) 📜 Skill insights (0)

Grey Divider


Action required

1. Spec imports from @playwright/test 📘 Rule violation ≡ Correctness
Description
The new E2E spec imports test/expect from @playwright/test instead of
@support/coverage/test, which bypasses the required coverage test wrapper. This breaks the
repository’s E2E coverage instrumentation requirements.
Code

e2e-tests/playwright/e2e/plugins/tmp-infocard-check.spec.ts[1]

+import { test, expect } from "@playwright/test";
Relevance

●●● Strong

Clear precedent enforcing @support/coverage/test imports; @playwright/test bypass is corrected.

PR-#4680
PR-#4798

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1798 requires importing test/expect from @support/coverage/test, but the
added spec imports them from @playwright/test on line 1.

Rule 1798: E2E specs must import test/expect from @support/coverage/test, not @playwright/test
e2e-tests/playwright/e2e/plugins/tmp-infocard-check.spec.ts[1-1]

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

## Issue description
The new Playwright spec file imports `test`/`expect` from `@playwright/test` instead of the required `@support/coverage/test` wrapper.

## Issue Context
Compliance requires all Playwright spec files under `e2e-tests/playwright/` to use the coverage-enabled test wrapper.

## Fix Focus Areas
- e2e-tests/playwright/e2e/plugins/tmp-infocard-check.spec.ts[1-3]

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


2. Missing test.beforeAll component annotation 📘 Rule violation ⚙ Maintainability
Description
The new E2E spec file defines tests without a test.beforeAll hook that adds a component
annotation. This reduces test traceability/ownership and violates the required component annotation
policy.
Code

e2e-tests/playwright/e2e/plugins/tmp-infocard-check.spec.ts[R5-8]

+test("tmp: settings general renders InfoCard", { tag: "@cluster-free" }, async ({ page }) => {
+  const common = new Common(page);
+  await common.loginAsGuest();
+
Relevance

●●● Strong

Repo accepts E2E hygiene/policy updates; adding suite-level hooks is common in their specs.

PR-#3482

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1834 requires a test.beforeAll hook that pushes a component annotation, but the
new spec file contains only a test(...) block and no test.beforeAll annotation setup.

Rule 1834: Every e2e spec file must have a component annotation in test.beforeAll
e2e-tests/playwright/e2e/plugins/tmp-infocard-check.spec.ts[1-16]

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

## Issue description
The new spec file does not include a `test.beforeAll` that pushes a `{ type: "component", description: "..." }` annotation.

## Issue Context
Compliance requires every `*.spec.ts` file under `e2e-tests/` to add a component annotation in a `test.beforeAll` hook.

## Fix Focus Areas
- e2e-tests/playwright/e2e/plugins/tmp-infocard-check.spec.ts[1-16]

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


3. Module federation env missing 🔗 Cross-repo conflict ☼ Reliability
Description
This PR documents NFS (packages/app) as the default and requires
ENABLE_STANDARD_MODULE_FEDERATION=true for dynamic frontend plugins to load, but rhdh-chart,
rhdh-operator, and rhdh-local snapshots don’t set that env var by default. As a result, upgraded
deployments can silently disable the backend’s Module Federation asset router and dynamic frontend
plugins may not load.
Code

docs/dynamic-plugins/migrating-config-to-new-frontend-system.md[R9-12]

+## Default: new frontend system (NFS)

-RHDH still ships the legacy `app` frontend package by default. The new frontend system lives in the `app-next` package and will become the default after the app-shell switch. Until then, enable **both** of the following on your RHDH **backend** deployment (OpenShift, Helm, Operator, [rhdh-local](https://github.com/redhat-developer/rhdh-local), or any environment where the backend runs as a container):
+RHDH ships the Backstage new frontend system (`packages/app`) by default. Enable standard Module Federation on the **backend** deployment so dynamic frontend plugins load correctly (OpenShift, Helm, Operator, [rhdh-local](https://github.com/redhat-developer/rhdh-local), or any environment where the backend runs as a container):
Relevance

●●● Strong

They usually fix docs that drift from real chart/operator defaults to avoid upgrade breakage.

PR-#3737
PR-#3834

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR explicitly states NFS is default and that ENABLE_STANDARD_MODULE_FEDERATION=true must be
set on backend deployments, while the backend code still disables the Module Federation router
unless the env var is true. The referenced chart/operator/local deployment manifests show their
backend containers’ env lists and do not include ENABLE_STANDARD_MODULE_FEDERATION, meaning
upgraded deployments won’t meet the new requirement by default.

docs/dynamic-plugins/migrating-config-to-new-frontend-system.md[9-21]
packages/backend/src/index.ts[85-105]
External repo: redhat-developer/rhdh-operator, config/profile/rhdh/default-config/deployment.yaml [161-170]
External repo: redhat-developer/rhdh-chart, charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml [217-236]
External repo: redhat-developer/rhdh-local, compose.yaml [24-40]
External repo: redhat-developer/rhdh-chart, charts/backstage/values.yaml [347-382]

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 backend currently disables the `dynamicPluginsFrontendServiceRef` (which serves standard Module Federation assets for dynamic frontend plugins) unless `ENABLE_STANDARD_MODULE_FEDERATION=true` is set.

This PR declares the New Frontend System (NFS) as default, which makes dynamic frontend plugins depend on standard Module Federation in typical deployments. However, the deployment repos (chart/operator/local) do not inject this env var by default, so upgrading the RHDH image can cause dynamic frontend plugins to stop loading.

## Issue Context
Downstream deployment repos typically treat the RHDH image as a black box; if the image requires a new env var for correct default behavior, those repos must be updated in lockstep, or the image should set a safe default.

## Fix Focus Areas
- docs/dynamic-plugins/migrating-config-to-new-frontend-system.md[9-21]
- packages/backend/src/index.ts[85-105]
- /cross_repos/rhdh-operator/config/profile/rhdh/default-config/deployment.yaml[161-164]
- /cross_repos/rhdh-chart/charts/backstage/vendor/backstage/charts/backstage/templates/backstage-deployment.yaml[217-235]
- /cross_repos/rhdh-local/compose.yaml[24-40]

### Suggested approach
1) **Preferred (avoid lockstep releases):** update the RHDH image/runtime defaults so `ENABLE_STANDARD_MODULE_FEDERATION` effectively defaults to `true` when using NFS (or always). For example:
  - remove/adjust the conditional noop override in `packages/backend/src/index.ts`, or
  - set `ENV ENABLE_STANDARD_MODULE_FEDERATION=true` in the container build/entrypoint for NFS images.

2) **If keeping opt-in behavior:** open coordinated PRs in:
  - `rhdh-chart` to add `ENABLE_STANDARD_MODULE_FEDERATION=true` to the default backend container env (or set it via default `extraEnvVars`).
  - `rhdh-operator` to add the env var to the `backstage-backend` container in the rhdh profile deployment template.
  - `rhdh-local` to add the env var to the `rhdh` service environment.

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



Remediation recommended

4. Unguarded localStorage read 🐞 Bug ☼ Reliability
Description
In InfoCard, the initial state reads localStorage.getItem(...) without guarding for environments
where storage access is unavailable or throws (e.g., blocked storage), which can crash rendering of
the Settings/General page. The same component already wraps localStorage writes/removes in a
try/catch, but the initial read is still unprotected.
Code

packages/app/src/modules/user-settings/InfoCard.tsx[R23-26]

+  const [showBuildInformation, setShowBuildInformation] = useState<boolean>(
+    () =>
+      localStorage.getItem('rhdh-infocard-show-build-information') === 'true',
+  );
Relevance

●●● Strong

Small defensive reliability change in a React component is typically accepted.

PR-#2594

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The state initializer reads localStorage with no error handling, while localStorage writes/removes
are already wrapped in try/catch within the same component, indicating localStorage operations may
fail and should be guarded consistently.

packages/app/src/modules/user-settings/InfoCard.tsx[23-38]

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

### Issue description
`InfoCard` initializes state by directly calling `localStorage.getItem(...)` during render. In environments where localStorage access throws (privacy mode, blocked storage), this can break rendering. Writes/removes are already guarded, but the read is not.

### Issue Context
The component uses localStorage to persist the “show more” state. The write path is wrapped in a try/catch, suggesting failures are expected and should be handled consistently.

### Fix Focus Areas
- packages/app/src/modules/user-settings/InfoCard.tsx[23-26]

### Suggested change
Wrap the initializer in a try/catch (and default to `false` on failure), e.g.:

```ts
const [showBuildInformation, setShowBuildInformation] = useState<boolean>(() => {
 try {
   return localStorage.getItem('rhdh-infocard-show-build-information') === 'true';
 } catch {
   return false;
 }
});
```

(Optionally also use `globalThis.localStorage`/feature checks if you want to be extra defensive.)

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



Informational

5. Typo breaks aria-label 🐞 Bug ⚙ Maintainability
Description
InfoCard passes arial-label (misspelled) to CopyTextButton, so the intended aria-label is
not applied to the rendered button and may lead to an unlabeled control for assistive technologies.
Code

packages/app/src/modules/user-settings/InfoCard.tsx[R129-132]

+              text={clipboardText}
+              tooltipText={t('app.userSettings.infoCard.metadataCopied')}
+              arial-label={t('app.userSettings.infoCard.copyMetadata')}
+            />
Relevance

●●● Strong

Obvious deterministic typo affecting accessibility/labeling; such fixes are routinely accepted.

PR-#2808

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The JSX attribute is visibly misspelled as arial-label in the new InfoCard, which is not a
standard ARIA attribute name.

packages/app/src/modules/user-settings/InfoCard.tsx[128-132]

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

### Issue description
`CopyTextButton` is given a misspelled ARIA attribute name (`arial-label`), which will not set the standard `aria-label` and may leave the control without the intended accessible name.

### Issue Context
This is in the newly introduced NFS app Settings/General build metadata InfoCard.

### Fix Focus Areas
- packages/app/src/modules/user-settings/InfoCard.tsx[129-132]

### Suggested change
Change:
```tsx
arial-label={...}
```
To:
```tsx
aria-label={...}
```

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


Grey Divider

Context used
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
✅ Compliance rules (platform): 46 rules
✅ Cross-repo context
  Explored: repo: redhat-developer/rhdh-operator (sha: a425373c)
  Explored: repo: redhat-developer/rhdh-local (sha: a1776caa)
  Explored: repo: redhat-developer/rhdh-plugins (sha: af6eb5ae)
  Explored: repo: redhat-developer/rhdh-chart (sha: 94e45eac)

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

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.

1 participant