Skip to content

feat: migrate to nfs - #5232

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

feat: migrate to nfs#5232
Zaperex wants to merge 49 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

Migrates RHDH to the Backstage New Frontend System (NFS) as the default app shell.

App shell

  • Renamed packages/app-nextpackages/app (NFS); removed legacy OFS packages/app / app-legacy and the DynamicRoot/Scalprum stack
  • Removed ENABLE_STANDARD_MODULE_FEDERATION and APP_CONFIG_app_packageName=app-next opt-in — Module Federation is always on
  • Removed packages/theme-wrapper (NFS app-shell cleanup)

Statically installed in packages/app

  • Upstream NFS plugins: home, catalog, catalog-import, catalog-graph, scaffolder, search, user-settings, RHDH theme
  • RHDH NFS modules: nav sidebar, learning-paths page, user-settings build-metadata card, catalog Created At column, translations, storage/learning-path APIs

Moved to dynamic plugins (DPDY)

  • app-auth, app-integrations, app-defaults (previously statically bundled)
  • global-header, homepage, techdocs (previously OFS mountPoints config)
  • All auth backend providers (previously authProvidersModule)

Removed / deferred

  • OFS application-provider / application-listener renderer and E2E fixtures (no NFS equivalent yet)
  • Legacy OFS mount-point home-page cards (Placeholder/Markdown/Headline with arbitrary props); Random Joke now uses NFS home-page-widget:home/random-joke instead of mountPoints
  • Nested sidebar menuItems config (flat code-defined nav instead)

Backend

  • schemaLocator now searches for dist/.config-schema.json

Config & docs

  • Homepage/global-header/sidebar config migrated from dynamicPlugins.frontend to app.extensions
  • Updated migration guide, auth docs, customization docs, and local E2E harness docs

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

@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

Zaperex added 10 commits August 10, 2026 16:03
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>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
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>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
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>

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
…to-NFS

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>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@github-actions

Copy link
Copy Markdown
Contributor

The container image build workflow finished with status: failure.

…to-NFS

Signed-off-by: Frank Kong <frkong@redhat.com>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@github-actions

Copy link
Copy Markdown
Contributor

Image was built and published successfully. It is available at:

…to-NFS

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>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
…to-NFS

Signed-off-by: Frank Kong <frkong@redhat.com>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
gustavolira pushed a commit to redhat-developer/rhdh-plugin-export-overlays that referenced this pull request Aug 26, 2026
…er PR

Two corrections after main moved.

#3328 merged thirteen backstage projects onto NFS through
configure({ useNewFrontendSystem: true }) without renaming any of them, so the
section as written argued against the largest migration in the repo. It now
describes both mechanisms and what each costs — the in-spec call is invisible to
the namespace, the reports and any inventory — and leaves the choice to
RHIDP-16461, which is the ticket for it. The section's own claim only needs that
both mechanisms replace the legacy lane rather than adding to it, which is true
of either.

The lead sentence said "renames its Playwright project", which was the same
prescription in the first line; it now says "replaces its legacy project".

The cut-over is also no longer abstract: redhat-developer/rhdh#5232 is open and
deletes packages/app-next/package.json, so the paragraph cites it rather than
describing 2.1 in general terms.
gustavolira pushed a commit to redhat-developer/rhdh-plugin-export-overlays that referenced this pull request Aug 26, 2026
Section 2 said app-defaults and keycloak both replaced their legacy lane; the
app-defaults row then called app-defaults "the only workspace whose only project
is -app-next". Those cannot both be true, and the row was the wrong one — it was
already false when written, and is now false five times over.

Re-measured: eight workspaces have no legacy lane left. Five renamed the
project; backstage, github and homepage pass
configure({ useNewFrontendSystem: true }) and keep the legacy names, which the
section did not mention at all and which is what made the original count look
like two. That mechanism also costs the reader something worth stating: 16 of
the 48 projects are NFS while every name still reads as legacy, so an inventory
keyed on the suffix miscounts them.

backstage is 13 projects, not 12.

The paragraph closed by asking for the doubling to be given an end date. It has
one now: redhat-developer/rhdh#5232 removes the legacy frontend, after which
both lanes of an add-beside pair boot the same shell.
gustavolira added a commit to redhat-developer/rhdh-plugin-export-overlays that referenced this pull request Aug 26, 2026
…3291)

* docs(nfs): map every e2e test to the cheapest layer that can hold it

The triage sheet measured the epic's cost — 46 Playwright projects, 246
tests, 24 workspaces. It did not ask the other half of the question the
epic opens: how much of that has to be an e2e test at all.

This maps all 246 tests to a layer. The NFS migration is what makes the
question answerable: under NFS a suite's Scalprum mountPoints config is
inert and the tab comes from the plugin's own EntityContentBlueprint, so
every assertion a suite has to rewrite during this migration is, by
construction, an assertion about declarative wiring rather than about the
external service the suite claims to test.

The result is that roughly a fifth of the suite needs a cluster. The rest
asserts plugin rendering, table pagination, form validation, retry policy
and permission logic. Some examples of the mismatch:

- orchestrator runs three fetch-wrapper unit tests (retry delay, backoff,
  status-code matching) on OpenShift Serverless Logic.
- intelligent-assistant runs file-attachment validation and chat display
  modes against an LLM sidecar.
- scorecard asserts empty, error and invalid-threshold states that cannot
  be produced from live GitHub or Jira at all.
- rbac spends 27 cluster tests on policy enforcement that startTestBackend
  exercises per-role in one process.

Nothing here proposes deleting coverage, and every row needs the plugin
owner's confirmation — it is a proposal per workspace, not a measurement.
The table's own arithmetic is checked against the stated totals.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): ground the layering study in the two governing documents

Rewrites the study after reading the RHDH Test Strategy and the testing
requirements matrix properly, and after investigating each plugin's
upstream tree rather than only its e2e assertions. Four things changed.

Layer numbering was wrong. The matrix defines L2 as integration
(startTestBackend) and L3 as component (RTL); the first revision had them
swapped, which inverted the advice on every backend recommendation.

Support level was ignored. The matrix requires no E2E for Community and
nothing at all for Dev Preview, and scorecard is dev-preview with 16
cluster tests while theme, quay, argocd, github, acr and tekton are
community. The strategy is also explicit that for plugins we do not own
we should not have coverage goals at all, only integration checks. That
splits the 24 workspaces into two halves that get different advice.

The central finding was missed entirely. 16 of the 22 plugins ship an NFS
extension (src/alpha*) that nothing anywhere tests. Only github (7 tests),
tech-radar and scorecard have createExtensionTester coverage. The epic
plans to verify those 16 by deploying to OpenShift and looking for a tab,
which fails ~20 minutes later, in a different repo, saying "heading not
found". So the first task on most tickets is an alpha.test.tsx upstream,
not the e2e migration -- and that test is what makes the migration safe,
since a blueprint that fails to attach leaves the NFS lane green.

The merged migrations were not read. Their diffs size the work precisely
(analytics 3 lines with 0 scalprum keys; topology ~96 with 4) and show
that the -app-next project is added beside the legacy one, so the epic as
executed takes 46 projects toward ~65 rather than reducing them.

Recipes are now concrete: real export symbols, real file paths, and
counts of how many files already use each utility upstream.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): use adoption counts that reproduce, and say how they were taken

The four "already used in N files" figures came from GitHub's code-search
totals. Those count every file type, markdown included, and none of them
reproduces against a checkout: renderInTestApp is 69/256 source files, not
71/283, and startTestBackend is 27/27, not 31/32.

The numbers carry weight in the argument -- they are what turns "use
startTestBackend" from a proposal into "follow what the repo already does"
-- so a reader has to be able to re-derive them. Replaced with grep counts
over .ts/.tsx at the two pinned commits, and added a note recording the
command, the commits, and what the earlier figures had been.

The claims those counts support are unchanged: the pattern is established
in both repos, and registerMswTestHooks (1 vs 32) remains the wider gap.

Re-verified at the same time, and unchanged: the 3/16/3 NFS-surface split,
and all five merged-migration diff sizes (analytics +3, acr +19,
tech-radar +17, tekton +90, topology +96).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): correct the Layer 4a premise, and publish a recipe that was run

Two corrections, both load-bearing.

"Layer 4a does not exist yet" was wrong. It exists in 16 of the 22
workspaces, cluster-free, today. All 10 rhdh-plugins workspaces that also
carry an e2e suite here have a playwright.config.ts with a webServer block
on ports 3000-3002 and testDir e2e-tests -- 209 tests against the 135
here, 17 test names byte-identical, quickstart identical in both of two.
homepage already parameterises legacy versus NFS there through appMode.
The error came from sweeping for *.spec.ts when those specs are named
*.test.ts.

What is actually missing is narrower and worth stating precisely: a
browser lane against the published OCI artifact rather than workspace
source. smoke-tests-native/ is further from that than the previous
revision implied -- it calls backend.stop() immediately so never serves a
request, carries no backend-dynamic-feature-service dependency so never
serves the remotes router, and inspects the frontend bundle as a static
file without executing it.

Recipe A is now a recipe that was executed. Against community-plugins
workspaces/acr, 5 tests, 3 mutations, each caught by exactly one
assertion. Running it falsified the version published earlier:

- `apis` must go on renderInTestApp, not createExtensionTester, when the
  tester's element is nested inside it -- the app registry wins and the
  tester's option is silently ignored, failing with NotImplementedError.
- coreExtensionData.routePath, not coreExtensionData.routing.path.
- createExtensionTester instantiates an extension in ISOLATION, so
  removing it from the plugin's extensions[] leaves the title and route
  assertions green. Since "the plugin forgot the extension" is one of the
  two silent NFS failure modes, the plugin-composition assertion is not
  optional. Added, with $$type.
- frontend-test-utils must be added as a devDependency; several plugins
  carry only @backstage/test-utils.

Also adds the prior question the per-workspace tables cannot answer: is
this test already covered upstream, cluster-free? The two lanes are not
equivalent -- this repo exercises the published artifact and the
dynamic-plugin loading path, upstream exercises source -- so a duplicate
name is a lead, not a verdict, and whether a 4b copy still earns its
cluster is the plugin owner's call. The document states the three honest
readings and leaves the decision there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): link Jira keys instead of leaving them as dead text

GitHub does not auto-link RHIDP keys, and neither does Jira inside an ADF
comment body, so every bare key was text a reader had to copy into a search
box. Linkified in both sheets; the Jira comments were fixed the same way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): the 209 are tests that exist, not tests that pass

I presented that count as available cluster-free coverage. It is not.
global-header reported 48 passed in 1.1m on 2026-08-18; orchestrator's step
on rhdh-plugins#4371 ran 3h11m until the job timed out, with real assertion
failures. Two of ten checked. The mechanism is proven cluster-free, the
state is not uniform, and the distinction matters because the whole point
of citing the number is to argue coverage already exists somewhere cheaper.

Also drops "upstream" as a column label: in RHDH that means
backstage/community-plugins, and 11 of these 24 workspaces come from
rhdh-plugins, which is not upstream of anything.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): replace "two of ten checked" with the actual sweep — 8 of 10 green

Ran the sweep the previous revision said was outstanding, over the last 300
CI runs on rhdh-plugins. Eight lanes are green, from 1m28s (theme) to
25m35s (intelligent-assistant).

Two are not, and both are more interesting than "unhealthy":

orchestrator is persistently red, not an outlier -- three consecutive runs
failed after 195m, 216m and 191m against the job timeout. It burns roughly
3.5 hours of runner time per run and never reports a real result, so its
48 tests are not coverage anyone is currently relying on.

adoption-insights has no signal at all: no PR touched it in the window.
That is a property of the gate, not the lane -- the CI matrix runs only
changed workspaces, so "green" everywhere here means green the last time
that workspace changed, not green today. Worth knowing before treating the
lane as a safety net for work that moves assertions into it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): reconcile the counts, and say what they count

Review of this PR turned up four things that were checkably wrong, all of
them in the numbers the document leans on.

Two tables disagreed with each other about the same quantity: Recipe D had
`scorecard` at 15 and `orchestrator` at 24 where section 5 had 16 and 26.
Section 5 is the column that sums to the 246 carried over from
nfs-e2e-triage.md, so Recipe D moves to match, and its total goes 135 -> 138.

Neither number is what Playwright reports. `npx playwright test --list` gives
19 for `scorecard` and 32 for `orchestrator`, because parametrised tests
expand at collection time. The counting basis was never stated, which left a
reader no way to reproduce the figures or to know they are a floor. Both
tables now say what they count, and say the suites move.

"22 of 22" as the proposed end state was impossible on the document's own
figures: 3 of the 22 have no NFS surface at all, and it says two of those are
backend-only so none is due. The population is 19.

The `github` row claimed 7 upstream tests "across 5 alpha/ directories".
There are 5 such directories, but the tests cover 4 of them —
`github-discussions` ships an NFS surface with nothing testing it, which
weakens rather than supports the "already covered upstream" verdict, so it is
now stated.

Also: the Recipe A snippet referenced `entity` and `fixtureTags` without
defining them, in a document whose claim is that the recipe was run rather
than written from the API docs. Both are now declared, and `getTags` returns
the `{ tags }` shape the real API ref returns. Re-counted `renderInTestApp`
in rhdh-plugins: 70 files, not 69.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): make Recipe B runnable, and fix five miscounts

Second review pass. The worst of it is Recipe B, which was presented as
copy-and-run code and could not run:

- `server.url()` does not exist. `TestBackend.server` is an
  `ExtendedHttpServer`, whose public surface is `start()`, `stop()` and
  `port()`; Backstage builds its own `backend.baseUrl` from `port()`. The
  snippet died on its second line, and the same wrong method was used further
  down to argue that keeping the backend alive is one of the "small" steps.
- The example passed a catalog *module* with no catalog plugin.
  `startTestBackend` stubs a plugin for an orphan module, but the stub
  registers no routes, so `GET /api/catalog/entities` is a 404 — it could
  never reach the 200 it asserts. It now starts the module alongside
  `catalogPlugin`, and says why a module alone cannot answer level 2.

Recipe B also now states that, unlike Recipe A, it was type-checked rather
than executed. That asymmetry is what let the two errors above through.

The miscounts, each contradicted by the document's own tables or by the
companion doc this PR edits:

- "Five workspaces have an -app-next lane today" — six do. The sixth,
  `app-defaults`, is also the one counterexample to the "added beside the
  legacy lane" doubling argument in the same section.
- Section 1.2's heading said four workspaces are over-tested by policy; the
  table under it lists seven.
- "All 10 rhdh-plugins workspaces" is 11; `app-defaults` is the one with no
  upstream lane, which is why it is missing from Recipe D and why section 5
  proposes building one. Section 5's preamble made the same claim over an
  11-row table.
- The ours/not-ours split was keyed on the hosting org, filing `rbac` and
  `topology` under "do not rewrite their coverage" — then recommending
  exactly that two rows later. Both are generally-available and in
  rhdh-supported-packages.txt; the split is really about support tier.
- `quay` shared the `n/a` marker with two backend-only workspaces, collapsing
  "NFS does not apply" and "due, but blocked until a surface exists".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): re-count renderInTestApp in community-plugins, and name the method

256 was stale; a fresh count gives 284. Both figures in that sentence now say
how they were obtained and when, so the next reader can reproduce them rather
than inherit them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): stop restating a count that moves, and reconcile with main

The project count was 46 when this was written, 47 when `nfs-e2e-triage.md`
was updated three days later, and 48 today — `keycloak` and `bulk-import` both
gained a lane while this PR sat open. Restating it in four places guaranteed
the document would contradict its own companion the moment it merged.

It is no longer restated. The intro points at the triage doc's §6, which
already carries the one-line commands that regenerate it, and the two places
that needed a magnitude say "one per project, and rising" instead of a number
that rots. The `-app-next` lane count is dated and comes with the grep.

Two claims were wrong against main rather than merely stale:

- "The `-app-next` project is added beside the legacy one, not in place of it"
  now has two counterexamples. `app-defaults` and `keycloak` have an
  `-app-next` project and no legacy one — which is the shape worth copying, so
  it is called out rather than buried.
- The `keycloak` row said NFS does not apply. That is true of its packages,
  which are both `backend-plugin-module`, but it now runs a `keycloak-app-next`
  lane and no legacy one. That tests something real — the backend modules under
  an NFS shell — and it is a different claim from the one the row made. Noted,
  along with the fact that nothing asserts the shell is actually NFS
  (RHIDP-16457).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): correct Recipe A against a second run of it

Rebuilt the acr test from scratch to submit it upstream, which turned up three
things the first run had not.

`getExtension(id)`, `$$type` and `pluginId` are all public API, so the
plugin-composition assertion needs no `as any` at all — the recipe was reaching
through a cast into `.extensions` for something the interface already exposes.
Ids there are namespaced: `entity-content:acr/acrImagesEntityContent`.

`EntityContentBlueprint.dataRefs` carries `filterFunction` alongside `title`,
and that is the most valuable assertion in the set: it is what an e2e test is
really checking when it navigates to an annotated entity and looks for the tab.
The first draft missed it because the refs were guessed rather than enumerated.
The recipe now asserts both directions of the filter, and says to enumerate.

Recipe A is therefore 6 tests and 5 mutations rather than 5 and 3. The
`pluginId` mutation is caught twice, since the extension ids move with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(nfs): fix a claim that contradicted the same document

Section 2 said app-defaults and keycloak both replaced their legacy lane; the
app-defaults row then called app-defaults "the only workspace whose only project
is -app-next". Those cannot both be true, and the row was the wrong one — it was
already false when written, and is now false five times over.

Re-measured: eight workspaces have no legacy lane left. Five renamed the
project; backstage, github and homepage pass
configure({ useNewFrontendSystem: true }) and keep the legacy names, which the
section did not mention at all and which is what made the original count look
like two. That mechanism also costs the reader something worth stating: 16 of
the 48 projects are NFS while every name still reads as legacy, so an inventory
keyed on the suffix miscounts them.

backstage is 13 projects, not 12.

The paragraph closed by asking for the doubling to be given an end date. It has
one now: redhat-developer/rhdh#5232 removes the legacy frontend, after which
both lanes of an add-beside pair boot the same shell.

* docs(nfs): re-measure the triage counts, and stop counting NFS lanes by suffix

Every figure in §1 was measured before backstage, github and homepage switched
to NFS through configure({ useNewFrontendSystem: true }) without renaming their
projects, and before #3324 renamed intelligent-assistant. Counting by the
-app-next suffix therefore missed 16 of the 27 NFS projects — more than half.

Re-measured on origin/main with the document's own §6 commands:

  projects       46 -> 48
  -app-next      6  -> 11
  NFS total      -  -> 27   (11 by name, 16 by configure)
  legacy         40 -> 21
  spec files     42 -> 43
  static tests   246 -> 261
  backstage      12 -> 13 projects

The framing sentence above the table said the -app-next name "is what switches"
the library into NFS mode; it is one of three things that do, and the only one
visible in a project name. §6 gains the command that finds the other lanes, so
the next person to regenerate these does not repeat the undercount.

Figures are dated and the table says to regenerate rather than quote, since the
whole point of this section is that they move.

---------

Co-authored-by: t <t@example.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frank Kong <frkong@redhat.com>

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@github-actions

Copy link
Copy Markdown
Contributor

Image was built and published successfully. It is available at:

Signed-off-by: Frank Kong <frkong@redhat.com>
Assisted-By: Cursor

rh-pre-commit.version: 2.4.0
rh-pre-commit.check-secrets: ENABLED
@github-actions

Copy link
Copy Markdown
Contributor

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

@sonarqubecloud

Copy link
Copy Markdown

@Zaperex Zaperex changed the title feat: migrate to nfs (WIP) feat: migrate to nfs Aug 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we might want to keep these fonts, checking the old app's index.html it doesn't reference them, but likely our theme plugin might be referencing these, maybe inadvertantly.

Comment thread packages/app/config.d.ts
* Configuration options for your user settings.
* @deepVisibility frontend
*/
userSettings?: {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we remove this, or does anyone actually set this to "browser"? There's a corresponding backend module that this controls here

<link rel="shortcut icon" href="<%= publicPath %>/favicon.ico" />
<link rel="apple-touch-icon" sizes="180x180" href="<%= publicPath %>/apple-touch-icon.png" />
<meta name="apple-mobile-web-app-title" content="<%= config.getString('app.title') %>" />
<title><%= config.getString('app.title') %></title>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<title><%= config.getString('app.title') %></title>
<title><%= config.getOptionalString('app.title') ?? 'Red Hat Developer Hub' %></title>

@openshift-ci

openshift-ci Bot commented Aug 27, 2026

Copy link
Copy Markdown

@Zaperex: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ocp-helm c1601ba link true /test e2e-ocp-helm

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

<CopyTextButton
text={clipboardText}
tooltipText={t('app.userSettings.infoCard.metadataCopied')}
arial-label={t('app.userSettings.infoCard.copyMetadata')}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
aria-label={t('app.userSettings.infoCard.copyMetadata')}

may as well sort this typo now 😄

@gashcrumb

gashcrumb commented Aug 27, 2026

Copy link
Copy Markdown
Member

besides those questions, this is looking really nice! I used this PR image against my existing setup, had to add the oidc and guest provider dynamic plugins, looking good!

image

@openshift-ci

openshift-ci Bot commented Aug 27, 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.

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.

2 participants