Skip to content

wasm: add initial support for OCI registries#37635

Closed
jewertow wants to merge 62 commits intoenvoyproxy:mainfrom
jewertow:wasm-oci-registry
Closed

wasm: add initial support for OCI registries#37635
jewertow wants to merge 62 commits intoenvoyproxy:mainfrom
jewertow:wasm-oci-registry

Conversation

@jewertow
Copy link
Copy Markdown
Contributor

@jewertow jewertow commented Dec 12, 2024

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.

{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.wasm.config.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [
    {
      "mediaType": "application/vnd.wasm.content.layer.v1+wasm",
      "digest": "sha256:4c7915b4c1f9b0c13f962998e4199ceb00db39a4a7fa4554f40ae0bed83d9510",
      "size": 1624962
    }
  ]
}

Fetching proces:
According to the OCI spec, Envoy has to send 2 subsequent requests: GET /v2/<repo>/manifests/<tag> and GET /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 httpUri and checks if it starts with oci://, to decide if it should use RemoteDataFetcher or OCI fetcher. Additionally, I added image_pull_secret field 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:

  1. ACR and GCR - this will require testing and making sure that those don't need additional authorization for getting blobs.
  2. Docker - implement callback for bearer authorization.
  3. Compat format.
  4. Quay (requires compat).
  5. Enable other registries - at this point we should have all required building blocks, i.e. authorization callback and appending authz header if not included in temporary location path.

If the direction of this PR is acceptable, I will add missing unit tests.

@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #37635 was opened by jewertow.

see: more, trace.

@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @abeyad
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #37635 was opened by jewertow.

see: more, trace.

@jewertow
Copy link
Copy Markdown
Contributor Author

@arkodg
Copy link
Copy Markdown
Contributor

arkodg commented Dec 12, 2024

from the envoy calendar, looks like kuat is away until March, cc @mpwarres

Copy link
Copy Markdown
Contributor

@abeyad abeyad left a comment

Choose a reason for hiding this comment

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

/lgtm api

but should we add release notes?

@abeyad
Copy link
Copy Markdown
Contributor

abeyad commented Dec 16, 2024

/lgtm api

but should we add release notes?

sorry just realized this is a draft PR

@zhaohuabing
Copy link
Copy Markdown
Member

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

@botengyao botengyao self-assigned this Dec 18, 2024
@github-actions
Copy link
Copy Markdown

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!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jan 16, 2025
@jewertow
Copy link
Copy Markdown
Contributor Author

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?

I didn't know that format, but sounds like a good idea. I will try to implement support for this format.

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Jan 21, 2025
mattklein123 pushed a commit that referenced this pull request Feb 12, 2025
…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]>
jewertow and others added 10 commits February 24, 2025 13:47
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]>
@jewertow jewertow force-pushed the wasm-oci-registry branch from 735c2e4 to 754e2af Compare April 2, 2025 21:25
Copy link
Copy Markdown
Member

@botengyao botengyao left a comment

Choose a reason for hiding this comment

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

/wait

context.lifecycleNotifier(),
context.getTransportSocketFactoryContext(), context.threadLocal(),
remote_data_provider_, oci_manifest_provider_, oci_blob_provider_,
std::move(image_pull_secret_provider_), std::move(callback))) {
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.

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.

Copy link
Copy Markdown
Contributor Author

@jewertow jewertow Apr 16, 2025

Choose a reason for hiding this comment

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

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.

agrawroh pushed a commit to agrawroh/envoy that referenced this pull request Apr 9, 2025
…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]>
@jewertow jewertow force-pushed the wasm-oci-registry branch from 9d4e8eb to 0df3683 Compare April 18, 2025 11:35
@github-actions
Copy link
Copy Markdown

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!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label May 22, 2025
@jewertow
Copy link
Copy Markdown
Contributor Author

Not stale

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label May 28, 2025
@github-actions
Copy link
Copy Markdown

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!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Jun 27, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 4, 2025

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!

@github-actions github-actions bot closed this Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api stale stalebot believes this issue/PR has not been touched recently waiting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support oci_uri as a remote data source for wasm

6 participants