Skip to content

fix(nfs): stop naming app-next on targets that no longer ship it - #162

Open
gustavolira wants to merge 6 commits into
mainfrom
fix/app-next-package-name
Open

gustavolira wants to merge 6 commits into
mainfrom
fix/app-next-package-name

Conversation

@gustavolira

Copy link
Copy Markdown
Member

What broke

Every NFS lane applies a secret setting APP_CONFIG_app_packageName: app-next. RHDH read that and called resolvePackagePath("app-next"), which resolved because packages/app-next/package.json declared that name.

redhat-developer/rhdh#5232 merged this morning and renamed that package to packages/app, whose package.json declares app. On any target built from main the resolve now throws inside the app plugin's init and the backend does not start, with a module-resolution error that looks nothing like its cause.

ENABLE_STANDARD_MODULE_FEDERATION, the secret's other key, was removed from RHDH in the same change and no longer does anything.

The fix

The secret is still correct for the 1.x lines, which do ship the separate package, so it is applied only for those. next tracks main and is on the 2.x line, so it no longer gets the secret and falls back to the app plugin's own default of app, which is right there.

export function hasSeparateAppNextPackage(version: string): boolean {
  return Number.parseInt(version, 10) < 2;
}

Verification

yarn check and yarn test green (169 tests). Both assertions were confirmed red first: applying the secret unconditionally, and moving the version cutoff, each fail a test.

Fixes RHIDP-16520.

🤖 Generated with Claude Code

gustavolira and others added 3 commits September 17, 2026 16:14
Every NFS lane applies a secret setting `APP_CONFIG_app_packageName:
app-next`. RHDH read that and called `resolvePackagePath("app-next")`,
which resolved because `packages/app-next/package.json` declared that name.

redhat-developer/rhdh#5232 merged on 2026-09-17 and renamed that package to
`packages/app`, whose package.json declares `app`. On any target built from
main the resolve now throws inside the app plugin's init and the backend
does not start, with a module-resolution error that looks nothing like its
cause. `ENABLE_STANDARD_MODULE_FEDERATION`, the secret's other key, was
removed from RHDH in the same change and no longer does anything.

The secret is still needed for the 1.x lines, which do ship the separate
package, so it is now applied only for those. `next` tracks main and is on
the 2.x line, so it no longer gets it and falls back to the app plugin's
own default of `app`, which is correct there.

Verified by mutation: applying the secret unconditionally, and moving the
version cutoff, each fail a test.

Fixes RHIDP-16520.

Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Gustavo Lira e Silva <[email protected]>
The repo's Version Bump Check requires one when src/ changes.

Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Gustavo Lira e Silva <[email protected]>
Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Gustavo Lira e Silva <[email protected]>
Comment thread src/deployment/rhdh/deployment.ts Outdated
* `app-next` makes `resolvePackagePath` throw and the backend never starts.
* `next` tracks main, which is on the 2.x line.
*/
export function hasSeparateAppNextPackage(version: string): boolean {

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.

I believe that the rhdh-e2e-test-utils version is tied to a given release (see the last bump on the release line for RHDH 1.10), so I don't think this check is necessary. This version will only be consumed by RHDH 2.1

Review finding: the version check protected a case that cannot happen.
The overlay branches pin this package exactly, main on 2.1.11 and
release-1.10 on 1.10.1, so a release branch never receives a 2.x release
and its one NFS lane keeps the behaviour it has today. The gate was inert
and only added a branch to reason about.

The whole file is now wrong for anything that does receive this version:
RHDH 2.x renamed `packages/app-next` to `packages/app` and removed
`ENABLE_STANDARD_MODULE_FEDERATION` entirely, so both keys name things that
no longer exist. Removed, and the app plugin's own default of `app` applies.

This is the smallest change that unbreaks the lanes. Collapsing the rest of
the NFS layer, the toggle, the auto-detection and the remaining config,
stays with RHIDP-15081.

Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Gustavo Lira e Silva <[email protected]>
@gustavolira

Copy link
Copy Markdown
Member Author

Good call, dropped it. Overlays pins this package exactly (main 2.1.11, release-1.10 1.10.1), so a release branch never gets a 2.x release and the check could never fire. Removed the secret outright.

On RHIDP-15081: I'd keep them separate for timing only. Every NFS lane is failing to boot right now and this is the smallest fix. The collapse is bigger (the toggle, the -app-next auto-detection, the env var, the remaining config files). Happy to take that next with this PR as the first slice, or do it all here if you prefer.

@hopehadfield

Copy link
Copy Markdown
Member

That makes sense to me, let's get this one in and create a follow-up PR to address the relevant action items in RHIDP-15081.

@hopehadfield hopehadfield left a comment

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.

The runtime fix is correct: removing the NFS secret prevents app-next from being passed to RHDH 2.x, and the clean build will not retain the deleted YAML asset. Before merging, please update the user-facing NFS documentation, which still says that the package injects APP_CONFIG_app_packageName: app-next and ENABLE_STANDARD_MODULE_FEDERATION. Those statements are now incorrect and could cause consumers to reintroduce the removed configuration manually. Please also update the VitePress version navigation from 2.1.14 to 2.1.15.

Review finding from @hopehadfield. Two pages still told consumers the
package injects `APP_CONFIG_app_packageName: app-next` and
`ENABLE_STANDARD_MODULE_FEDERATION`, which would have led someone to
reintroduce by hand exactly what this PR removes.

The merge list in the deployment guide and the NFS section in the overlay
configuration reference now start at dynamic plugins, and the
`useNewFrontendSystem` row no longer claims it merges those secrets. The
auto-detection and namespace-suffix behaviour is unchanged and still
documented.

Also bumps the VitePress version nav and the five snippets that pin this
package from 2.1.14 to 2.1.15.

Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Gustavo Lira e Silva <[email protected]>
@gustavolira

Copy link
Copy Markdown
Member Author

Both done in dc07247, thanks.

Docs: the merge list in the deployment guide and the NFS section in the overlay configuration reference no longer mention the two keys, and I also caught the useNewFrontendSystem row in the options table, which still said it merges app-next secrets. The auto-detection and namespace-suffix behaviour is unchanged and still documented.

Version nav bumped to 2.1.15, along with the five doc snippets that pin this package.

And you were right about the version check, it is already gone. Overlays pins the package exactly (main 2.1.11, release-1.10 1.10.1), so a release branch never receives a 2.x release and the check could never fire.

@hopehadfield

Copy link
Copy Markdown
Member

A couple more things:

P1 — Deployment guide still describes the old merge behavior

docs/guide/deployment/rhdh-deployment.md:152 says:

Merges configuration files ... for app-config, secrets, and dynamic plugins

That is no longer accurate. NFS defaults now apply only to dynamic plugins and Helm values. _applyAppConfig() and _applySecrets() merge common, auth, and workspace files only.

P2 — Additional stale NFS secret references

These still describe the deleted secret layer:

  • docs/guide/configuration/environment-variables.md:70
    • Says config/new-frontend-system/ contains “secrets, dynamic plugins, Helm values”.
  • CLAUDE.md:98
    • Says secrets use the NFS layer order.

They should state that NFS contributes dynamic-plugin and Helm-value defaults only.

Three more places described the deleted secret layer, all found by
@hopehadfield:

- The deployment guide's `deploy()` steps said it merges NFS defaults for
  app-config, secrets and dynamic plugins. `_applyAppConfig` and
  `_applySecrets` merge common, auth and workspace files only.
- The environment-variables page listed `config/new-frontend-system/` as
  carrying secrets.
- CLAUDE.md said secrets use the NFS layer order, and its cascade diagram
  implied the layer applies to every config.

Co-Authored-By: Claude Opus 5 <[email protected]>
Signed-off-by: Gustavo Lira e Silva <[email protected]>
@gustavolira

Copy link
Copy Markdown
Member Author

All three fixed in the latest commit, thanks for catching them.

P1: the deploy() step now says common → auth → project overrides for app-config and secrets, with NFS defaults applying to dynamic plugins.

P2: the environment-variables page no longer lists secrets under config/new-frontend-system/, and CLAUDE.md now says the layer contributes dynamic-plugin and Helm-value defaults only. I also adjusted its cascade diagram, which implied the layer applies to every config.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants