Skip to content

Stop publishing the codegen node_modules - #30

Open
francoisferrand wants to merge 6 commits into
development/1.0from
bugfix/CLDSRVCLT-19
Open

francoisferrand wants to merge 6 commits into
development/1.0from
bugfix/CLDSRVCLT-19

Conversation

@francoisferrand

Copy link
Copy Markdown
Contributor

1.0.12 publishes each Smithy typescript-codegen directory wholesale, including its node_modules. That comes to ~400 MB installed, with build-only tools and an old @aws-sdk/core that brings a vulnerable fast-xml-parser. Consumers (cloudserver, backbeat) can't de-duplicate any of it, and only image scanners like trivy notice it.

What changes:

  • Codegen bumped to 0.54.0. The generated clients now depend on a few caret ranges on current @aws-sdk/core and @smithy/core, instead of ~40 exact, outdated pins. The new serde no longer keeps error bodies, so the custom error handler now sits inside the deserializer. It turns cloudserver's XML errors and the S3C proxy's HTML errors into exceptions using the raw response.
  • Packaging. The generated clients' runtime deps are declared in the root package.json, and the package only ships each client's package.json and dist-*. No more node_modules.
  • CI packaging check. CI packs the build, rejects any node_modules in the tarball, and installs it into a scratch consumer with prod deps only. It then checks that the package and each generated client load, send a request and type-check. The release workflow runs the same check on the tarball it publishes.
  • Dependency check. The build fails if a dependency of a generated client isn't declared at the root, so a future codegen bump can't silently break consumers.

Local results:

Before (1.0.12) After
Tarball 52 MB, 46,681 files 71 KB, 345 files
Installed 402 MB 23 MB, 3,870 files

trivy reports no findings on the installed consumer after the change.

The version isn't bumped here. Once this lands, releasing 1.0.13 and bumping it in cloudserver and backbeat will remove the nested trees from their images.

Issue: CLDSRVCLT-19

The 0.34.0 codegen pinned exact, now outdated @aws-sdk/* versions in the
generated clients (@aws-sdk/core 3.856.0 brings a vulnerable
fast-xml-parser). 0.54.0 emits a handful of caret ranges on current
@aws-sdk/core and @smithy/core instead of ~40 packages. It is built
against Smithy 1.73.0, so smithy-aws-traits (and the CLI used in CI)
follows.

The new schema-based serde reads error bodies itself and only surfaces a
JSON SyntaxError for cloudserver's XML errors, so the custom error
handler now sits inside the deserializer and maps XML/HTML error
responses from the raw response. The generated *FilterSensitiveLog
helpers are gone; nothing uses them.

Issue: CLDSRVCLT-19
The package listed each typescript-codegen directory wholesale in
`files`, so their node_modules got published too: ~400 MB installed,
including build-only tools and an old @aws-sdk/core pulling a vulnerable
fast-xml-parser. Consumers could not de-duplicate any of it, and yarn
audit / osv did not see it either, only image scanners did.

The generated clients now resolve their runtime dependencies from the
root package.json, which declares them, and the package only ships each
client's package.json and dist-* output. The build drops the nested
node_modules once the clients are compiled so nothing resolves from
there by accident.

Issue: CLDSRVCLT-19
Packaging regressions have slipped through twice now: 1.0.10 shipped
without the generated clients, 1.0.12 with their node_modules. Neither
showed up in our tests, which run from the repository.

CI now packs the build, rejects it if it contains node_modules, installs
it in a scratch consumer with production dependencies only, and checks
the package and each generated client load, can send a request and
type-check. The release workflow runs the same check on the tarball it
publishes.

Issue: CLDSRVCLT-19
The generated clients load their dependencies from our package.json, and
the set they need changes with the Smithy codegen version. A codegen bump
that adds a dependency would otherwise only break consumers at runtime.

The build now checks every dependency of the generated package.json files
is declared at the root, and prints the `yarn add` line to fix it.

Issue: CLDSRVCLT-19
@bert-e

bert-e commented Sep 30, 2026

Copy link
Copy Markdown

Hello francoisferrand,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval ⭐
/bypass_build_status Bypass the build and test status ⭐
/bypass_commit_size Bypass the check on the size of the changeset TBA ⭐
/bypass_incompatible_branch Bypass the check on the source branch prefix ⭐
/bypass_jira_check Bypass the Jira issue check ⭐
/bypass_peer_approval Bypass the pull request peers' approval ⭐
/bypass_leader_approval Bypass the pull request leaders' approval ⭐
/bypass_source_branch_lineage Bypass the cross-branch contamination check ⭐
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request.
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@francoisferrand
francoisferrand marked this pull request as ready for review October 1, 2026 20:32
The error middleware had to wrap the deserializer and rebuild exceptions
by hand from the raw response. Parsing these bodies in the protocol
instead hands them to the SDK's own error handling, so modeled errors
come back as their generated classes with their header members, the
same as for JSON responses.

createCustomErrorMiddleware is removed: nothing outside this package
uses it.

Issue: CLDSRVCLT-19
@scality scality deleted a comment from bert-e Oct 2, 2026
@bert-e

bert-e commented Oct 2, 2026

Copy link
Copy Markdown

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • 2 peers

npm install --omit=dev --ignore-scripts --no-audit --no-fund --silent "$TARBALL"
node smoke.js

npm install --no-save --ignore-scripts --no-audit --no-fund --silent typescript @types/node@20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be node@24 ? And so in package json also node >= 24

@SylvainSenechal SylvainSenechal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we consider node >= 24 instead of 20 ?

The last commit says release 1.1.13, but the code bumps to 1.0.13 and the targeted branch is dev/1.0, I think we wanna go with 1.1.13, right ?

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.

3 participants