Skip to content

Use lower level TCP for registry proxy - #26

Merged
mikejholly merged 1 commit into
earthly-mainfrom
mh/regproxy-v2
Oct 3, 2023
Merged

Use lower level TCP for registry proxy#26
mikejholly merged 1 commit into
earthly-mainfrom
mh/regproxy-v2

Conversation

@mikejholly

@mikejholly mikejholly commented Sep 29, 2023

Copy link
Copy Markdown
Contributor

Simplifies the registry proxying code by removing the HTTP client & header parsing in favor of lower-level data copying.

Works with earthly/earthly#3317.

@mikejholly mikejholly changed the title Use lower level TCP Use lower level TCP for registry proxy Oct 2, 2023
@mikejholly
mikejholly marked this pull request as ready for review October 2, 2023 19:12

@alexcb alexcb 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.

looks good, please rebase your commits prior to merging.

@mikejholly
mikejholly merged commit 0fde422 into earthly-main Oct 3, 2023
@mikejholly
mikejholly deleted the mh/regproxy-v2 branch October 3, 2023 22:44
mikejholly added a commit to earthly/earthly that referenced this pull request Oct 3, 2023
Removes the HTTP-level code in favor of a lower-level TCP proxy
approach.

The tricky part here was that Docker (and most HTTP clients) leaves the
TCP connection open. Hence, there's no way for the code to detect when
to stop reading the initial request aside from a timeout. Apparently,
the recommended approach is to set
[`SetReadDeadline`](https://cs.opensource.google/go/go/+/refs/tags/go1.21.1:src/net/net.go;l=156)
ahead of each read and appropriately handle any timeout errors.

Overall, I think this is preferable to the original code as the HTTP
code & header parsing is a bit brittle.

Works with: earthly/buildkit#26
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