Skip to content

Stream layer artifact hashing to avoid buffering whole zip#206

Merged
GrahamCampbell merged 2 commits intomainfrom
fix/stream-layer-artifact-hash
Apr 29, 2026
Merged

Stream layer artifact hashing to avoid buffering whole zip#206
GrahamCampbell merged 2 commits intomainfrom
fix/stream-layer-artifact-hash

Conversation

@GrahamCampbell
Copy link
Copy Markdown
Contributor

Replaces fsp.readFile() in compileLayer() with a streaming SHA-1 over fs.createReadStream(). The previous code materialized the entire layer zip in memory before feeding it to crypto.createHash('sha1'), which remained an OOM risk for large layers after PR #205 fixed the function-artifact and change-detection hashing paths.

The hash bytes are unchanged: crypto.Hash.update() is incremental, so update(JSON.stringify(layerForHash)) + per-chunk update(chunk) produces the same digest as a single update of the full buffer. A new unit test locks this in by computing the expected hash with the original buffered shape and asserting equality against the streaming output, while also asserting fs.createReadStream is used and fs.promises.readFile is not.

This addresses the last whole-artifact buffering site flagged during the #189 review.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Eliminates an OOM risk in AWS layer packaging by switching layer artifact hashing from buffered reads to a streaming SHA-1 over the layer zip file.

Changes:

  • Replace fs.promises.readFile()-based hashing in compileLayer() with a streaming hash via fs.createReadStream()
  • Add a unit test to assert streaming behavior (uses createReadStream, does not use fs.promises.readFile) and to lock in hash equivalence vs the prior buffered approach

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
lib/plugins/aws/package/compile/layers.js Streams the layer zip into the SHA-1 hash instead of buffering the entire artifact in memory.
test/unit/lib/plugins/aws/package/compile/layers.test.js Adds a regression test that validates streaming is used and the resulting digest matches the prior buffered hashing behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/plugins/aws/package/compile/layers.js
@GrahamCampbell GrahamCampbell merged commit 241b53a into main Apr 29, 2026
8 checks passed
@GrahamCampbell GrahamCampbell deleted the fix/stream-layer-artifact-hash branch April 29, 2026 22:08
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