wasm: add initial support for OCI registries#37635
wasm: add initial support for OCI registries#37635jewertow wants to merge 62 commits intoenvoyproxy:mainfrom
Conversation
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
from the envoy calendar, looks like kuat is away until March, cc @mpwarres |
abeyad
left a comment
There was a problem hiding this comment.
/lgtm api
but should we add release notes?
sorry just realized this is a draft PR |
|
@jewertow Thanks for working on this! Adding built-in OCI image support WASM plugin would allow EG to remove the shim required to pulll the wasm from a remote image registry. This proposal looks great! I do have one suggestion: should we also support the compat format to accommodate standard tools such as docker, podman, buildah, and standard container registries which don't yet support custom media types? |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
I didn't know that format, but sounds like a good idea. I will try to implement support for this format. |
…actoryContext (#38399) Commit Message: factory context: extend ServerFactoryContext with getTransportSocketFactoryContext Additional Description: This change is necessary to allow upstream filters reading secrets from SDS. Risk Level: low Testing: - Docs Changes: Not needed as it is only an internal change. Release Notes: Not needed as it is only an internal change. Platform Specific Features: - This change is necessary to unblock: 1. injecting credentials to upstream requests #38398 2. reading image pull secret by upstream wasm filters #37635 --------- Signed-off-by: Jacek Ewertowski <[email protected]>
5301b48 to
ecee353
Compare
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
735c2e4 to
754e2af
Compare
| context.lifecycleNotifier(), | ||
| context.getTransportSocketFactoryContext(), context.threadLocal(), | ||
| remote_data_provider_, oci_manifest_provider_, oci_blob_provider_, | ||
| std::move(image_pull_secret_provider_), std::move(callback))) { |
There was a problem hiding this comment.
By moving this unique_ptr, image_pull_secret_provider_ will become nullptr. If oci_manifest_provider_ and oci_blob_provider_ are intended to use it as a shared ptr we can let both extend the provider's life time, since both are owned by the PluginConfig.
There was a problem hiding this comment.
Right, sorry for that. I removed image_pull_secret_provider_ from PluginConfig and now I store it as a shared pointer in OCI fetchers, which are stored as unique pointers in PluginConfig.
…actoryContext (envoyproxy#38399) Commit Message: factory context: extend ServerFactoryContext with getTransportSocketFactoryContext Additional Description: This change is necessary to allow upstream filters reading secrets from SDS. Risk Level: low Testing: - Docs Changes: Not needed as it is only an internal change. Release Notes: Not needed as it is only an internal change. Platform Specific Features: - This change is necessary to unblock: 1. injecting credentials to upstream requests envoyproxy#38398 2. reading image pull secret by upstream wasm filters envoyproxy#37635 --------- Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
9d4e8eb to
0df3683
Compare
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
Signed-off-by: Jacek Ewertowski <[email protected]>
…CI manifest and blob providers Signed-off-by: Jacek Ewertowski <[email protected]>
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
Not stale |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
Commit Message: wasm: add initial support for OCI registries
Additional Description: This is an initial implementation of OCI client for fetching images.
Risk Level: medium
Testing: TODO
Docs Changes: Documentation will be added when the feature will be complete.
Release Notes: TODO
Fixes: #33212
Runtime guard:
envoy.reloadable_features.wasm_plugin_enable_fetching_from_oci_regisitry(disabled by default).Implementation details
Image format:
Currently, it supports image format compatible with WASM Artifact Image Specification v0.0.0. Other formats, like compat, can be added in next iterations.
Fetching proces:
According to the OCI spec, Envoy has to send 2 subsequent requests:
GET /v2/<repo>/manifests/<tag>andGET /v2/<repo>/blobs/<digest>to fetch an image. However, registries may return 307 and redirect blob request to custom location, e.g. S3. So eventually Envoy must send at least 3 requests.Dependency on DFP cluster:
Handling redirects requires dynamic cluster resolution, so the only practical solution is using dynamic forward proxy cluster.
Authorization:
Different registries support different authorization strategies. ECR, ACR and GCR use basic scheme and require the credential from the image pull secret. Docker on the other hand uses bearer scheme and requires additional authorization request to obtain a token.
API changes
I wanted to avoid significant API changes, so currently Envoy uses
httpUriand checks if it starts withoci://, to decide if it should use RemoteDataFetcher or OCI fetcher. Additionally, I addedimage_pull_secretfield that specifies the secret from which Envoy reads credential. Currently, this field is hidden from the documentation.Here you can see example working configuration.
Implementation status
In this iteration, I was focused on introducing scaffolding for the OCI client that has minimal impact on the existing code base, and reuses existing code for fetching from HTTP URI. I wasn't able to test it with GCR and ACR yet, so it works only with ECR for now.
I would like to address missing features in follow-up PRs. I suggest adding support for missing features as follows:
If the direction of this PR is acceptable, I will add missing unit tests.