Install the GSS fork in-tree so the acs-admin build works and #674 takes effect#676
Merged
Conversation
PR #674 vendored lib/js-gssapi (gssapi.js without GSS_C_DELEG_FLAG) and taught deps.js to prefer it, but declared it as a registry dependency, "@amrc-factoryplus/gssapi": "^2.1.0". That package was never published to npm and will not be. The reference 404s, npm and bun skip it silently because it is optional, and deps.js falls back to upstream gssapi.js - with delegation. #674 has therefore been inert since it landed, and the services still storm the KDC with TGT NOT FORWARDABLE. Point the reference at the vendored copy instead. The lib tree is already supplied as a build context to every service image (mk/acs.docker.mk), and the js-build base image already installs krb5-dev and cmake precisely to compile gssapi.js, so the fork builds where the services build. Declaring the fork in js-service-client alone is not enough: a file: reference made BY a package which is itself installed via file: is never materialised. Under `npm install --install-links`, which every service Dockerfile runs, npm resolves the nested "../js-gssapi" relative to the COPIED package, looks for <app>/node_modules/@amrc-factoryplus/js-gssapi, does not find it, and skips it without a warning because it is optional. So each service needing GSS declares the fork itself; that is what puts it in node_modules, where deps.js resolves it by hoisting. This matches the existing file:../lib/js-* convention. It stays in optionalDependencies, exactly like gssapi.js: the addon needs krb5 headers, and deps.js is written to tolerate its absence (no GSS on Windows) and fall back. acs-admin is deliberately excluded, and instead stubs the specifier to emptyModule in vite.config.js just as it already does for gssapi.js, got and got-fetch. That alias is also what fixes the v6.0.0-rc.2 release failure: #674 introduced a second bare GSS specifier into a module which is bundled for the browser, without a matching alias, so rollup failed with "Failed to resolve import @amrc-factoryplus/gssapi". The npm 404 was only a warning, not the cause. Without the alias the browser bundle would drag in the native addon loader instead. acs-edge is excluded: it takes GSS from @amrc-factoryplus/utilities and never imports service-client, and its image has no lib build context. Note that js-service-client must not now be published to npm while it carries a file: reference: an external consumer's `npm ci` fails hard on it ("Cannot destructure property 'package' of 'node.target'"). Consumers resolving service-client from the registry (acs-mcp, acs-visualiser, historian-uns, js-sparkplug-app) keep the published copy, and so keep upstream gssapi.js with delegation.
A file: dependency declared by a file:-linked package does not install: npm resolves the nested ../js-gssapi relative to the copied location under <app>/node_modules/@amrc-factoryplus/, hits ENOENT, and skips it silently because it is optional. The per-service declarations are what actually install the fork, so this entry never did anything. It is also a publish landmine: an external consumer running npm ci against a published service-client carrying a file: spec fails outright. Removing it keeps the npm 404 fixed with no hazard. External consumers continue to fall back to gssapi.js via the catch in deps.js.
This reverts commit cca1d63.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
v6.0.0-rc.2's release failed: theacs-adminimage build errored withwhich fail-fast-cancelled the other 18 JS image builds and skipped the chart release.
Root cause
Two separate defects, both introduced by 42a88e0 ("Prefer @amrc-factoryplus/gssapi over gssapi.js for GSS"):
The browser build.
acs-adminis the only bundled build in the tree, andvite.config.jsaliases native GSS out of it ('gssapi.js': EMPTY). 42a88e0 introduced a second bare specifier,@amrc-factoryplus/gssapi, intodeps.js— a module that gets bundled — without adding it to that alias list.v5.1.0andv6.0.0-rc.1are byte-identical here and built fine; rc.1 predates the vendoring by 3h40m and itsacs-adminjob passed.The fork was never installed. It was declared as a registry dependency (
^2.1.0) of a package that is not on npm.bun install/npm installlogged the 404 as a warning and skipped it (it is optional), sodeps.jsfell through its.catch()to upstreamgssapi.js— with credential delegation. Stop requesting GSS credential delegation (TGT NOT FORWARDABLE storm) #674's delegation fix has therefore been inert since it merged.The npm 404 was never the cause of the build failure. In the same rc.2 run,
acs-edge(npm, samedeps.jsviafile:) built successfully.The fix
acs-admin/vite.config.js: alias@amrc-factoryplus/gssapito the empty stub, next to the existinggssapi.jsentry. This alone unblocks the build — aliases intercept the specifier before node resolution, so the package need not be installed. It is also correct on its own merits: without it, an installed fork would drag the native addon loader (bindings,file-uri-to-path) into the browser bundle.service-clientdeclare"@amrc-factoryplus/gssapi": "file:../lib/js-gssapi"inoptionalDependencies, which is what actually installs the fork and makes Stop requesting GSS credential delegation (TGT NOT FORWARDABLE storm) #674 live. Afile:ref declared by afile:-linked package does not install — npm resolves the nested path relative to the copied location, hits ENOENT, and silently skips it.optionalDependencies(notdependencies) matches howgssapi.jsis declared and keepsnpm installworking without krb5 headers.acs-admin(browser; the alias handles it),acs-edge(never importsservice-client),acs-example-service(not built).js/gssapi/v2.1.0did not fail on missing Kerberos headers, it failed in 2s at workflow evaluation withSecret NPM_TOKEN is required, but not provided. No step ever ran.js/gssapi/v2.1.1, cut with the libkrb5-dev step, failed identically. The repo has no Actions secrets, and the js-lib workflow has never published anything — it was created 2025-06-16, four days after service-client 1.6.0 went to npm. The npm route is abandoned; the fork stays in-tree.Verification
built in 22.14swith the alias. The bundled GSS import compiles toimport('./emptyModule-...js'), giving the browser the sameGSS === {}it has had since before the fork — no behaviour change.node-gssapiandfile-uri-to-pathboth 0 matches.npm install --install-links(build/Release/node-gssapi.nodepresent, true GSS surface). The js-build base image already installskrb5-devexplicitly "to build gssapi.js", so the fork builds wherevergssapi.jsalready does.deps.jsresolves the fork rather than upstream, even withgssapi.jsalso installed.acs-i3xjest: 13 suites, 342 tests passing.How to test
v6.0.0-rc.3.acs-adminbuild passes and the chart release runs.node_modules/@amrc-factoryplus/gssapi/build/Release/node-gssapi.nodeexists.TGT NOT FORWARDABLErequest per token/MQTT connect.Known gaps
acs-edgeimports@amrc-factoryplus/utilities, notservice-client, so edge agents keep the upstream delegatinggssapi.js. If edge principals contribute to the KDC storm, this does not resolve it for them.service-client(acs-mcp,acs-visualiser,historian-uns,lib/js-sparkplug-app) resolve the published copy and keep delegating.Release notes
Fixes the
acs-adminimage build, which broke in v6.0.0-rc.2. The GSS fork that removes Kerberos credential delegation is now actually installed in the services that use it, so the fix released in #674 takes effect.