feat: migrate to nfs - #5232
Conversation
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
|
Skipping CI for Draft Pull Request. |
|
/test all |
PR Summary by QodoMigrate default frontend to Backstage NFS and retain OFS as app-legacy
AI Description
Diagram
High-Level Assessment
Files changed (22)
|
Code Review by Qodo
1. Spec imports from @playwright/test
|
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
|
The container image build workflow finished with status: |
…to-NFS 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
…to-NFS Signed-off-by: Frank Kong <frkong@redhat.com> rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
…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.
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.
…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
Signed-off-by: Frank Kong <frkong@redhat.com> Assisted-By: Cursor rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED
|
There was a problem hiding this comment.
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.
| * Configuration options for your user settings. | ||
| * @deepVisibility frontend | ||
| */ | ||
| userSettings?: { |
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
| <title><%= config.getString('app.title') %></title> | |
| <title><%= config.getOptionalString('app.title') ?? 'Red Hat Developer Hub' %></title> |
|
@Zaperex: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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')} |
There was a problem hiding this comment.
| aria-label={t('app.userSettings.infoCard.copyMetadata')} |
may as well sort this typo now 😄
|
PR needs rebase. DetailsInstructions 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. |




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
packages/app-next→packages/app(NFS); removed legacy OFSpackages/app/app-legacyand the DynamicRoot/Scalprum stackENABLE_STANDARD_MODULE_FEDERATIONandAPP_CONFIG_app_packageName=app-nextopt-in — Module Federation is always onpackages/theme-wrapper(NFS app-shell cleanup)Statically installed in
packages/appMoved to dynamic plugins (DPDY)
app-auth,app-integrations,app-defaults(previously statically bundled)global-header,homepage,techdocs(previously OFSmountPointsconfig)authProvidersModule)Removed / deferred
application-provider/application-listenerrenderer and E2E fixtures (no NFS equivalent yet)home-page-widget:home/random-jokeinstead ofmountPointsmenuItemsconfig (flat code-defined nav instead)Backend
schemaLocatornow searches fordist/.config-schema.jsonConfig & docs
dynamicPlugins.frontendtoapp.extensionsWhich issue(s) does this PR fix
PR acceptance criteria
Please make sure that the following steps are complete:
How to test changes / Special notes to the reviewer