Skip to content

Support for the gRPC Docker registry proxy#25

Merged
mikejholly merged 1 commit into
earthly-mainfrom
mh/registry-proxy
Sep 28, 2023
Merged

Support for the gRPC Docker registry proxy#25
mikejholly merged 1 commit into
earthly-mainfrom
mh/registry-proxy

Conversation

@mikejholly

Copy link
Copy Markdown
Contributor

This PR adds a new gRPC service that makes use of the pre-existing gRPC connection to stream registry image data out of the embedded registry and into the local container frontend (docker pull, etc.).

image

Associated with: earthly/earthly#3299

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

LGTM! Perhaps @alexcb should also review this.


// parseHeader parses the incoming request header and extracts the method, path,
// & header values.
func parseHeader(r io.Reader) (method string, path string, header http.Header, err error) {

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.

so you've created your own webserver I see? interesting :)

(if it works, it works.)

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

I think it looks good -- my only concern is if we find ourselves needing to proxy anything else this is tied to the http protocol. I think it's fine for now -- if (and this is a big if) we need to proxy to a different service, i think it would be good to refactor this to be more generic (e.g. Proxy could take some sort of serviceID).

Please rebase all your commits prior to merging this PR

@mikejholly

Copy link
Copy Markdown
Contributor Author

I think it looks good -- my only concern is if we find ourselves needing to proxy anything else this is tied to the http protocol. I think it's fine for now -- if (and this is a big if) we need to proxy to a different service, i think it would be good to refactor this to be more generic (e.g. Proxy could take some sort of serviceID).

Interesting point. This service is purpose-built to speak with the embedded registry, but it's certainly possible to register other services on the gRPC connection for other purposes. We could definitely take a look at making this more generic, like you suggested, as the need arises. I may also take a look at streaming the raw HTTP data over a lower-level TCP connection. I figured the HTTP libraries would help with some of the overhead, but that may not be necessary.

@mikejholly

Copy link
Copy Markdown
Contributor Author

On second thought, I think I may leave this as is for the time being. We can always revisit it later if needed.

@mikejholly mikejholly merged commit af796f0 into earthly-main Sep 28, 2023
@mikejholly mikejholly deleted the mh/registry-proxy branch September 28, 2023 03:23
mikejholly added a commit to earthly/earthly that referenced this pull request Sep 29, 2023
This PR adds support for proxying image registry requests to the
BK-embedded registry using a gRPC stream. A local proxy is used by the
container frontend to "pull" an image from the local or remote BK via
the gRPC proxy.


![image](https://github.com/earthly/earthly/assets/332408/996bd3fd-ef84-4844-8f7e-250e68f4f975)


Associated with: earthly/buildkit#25
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