containers / image

Work with containers' images
Apache License 2.0
843 stars 365 forks source link

[Additional Layer Store] Enable Additional Layer Store to perform registry authentication #2417

Closed ktock closed 4 weeks ago

ktock commented 1 month ago

Implements https://github.com/openshift/enhancements/pull/1600#issuecomment-2099607551

Currently, c/image doesn't share creds to Additional Layer Store so it requires additional configurations for registry authentication. And the current means of registry authentication for Additional Layer Store doesn't seem to meet requirements for the platform: https://github.com/openshift/enhancements/pull/1600#pullrequestreview-1990049051

This commit fixes that issue by allowing c/image directly sharing creds to Additional Layer Store. Additional Layer Store doesn't need to have its own logic to fetch registry creds but it can receive them from c/image.

Additional Layer Store needs to provide a helper binary that is executed from c/image. This helper binary is registered to c/image using registries.conf with the following field (store-helper can be any command name of the helper binary). It receives registry creds via stdin and Additional Layer Store can use that creds for registry authentication.

additional-layer-store-auth-helper = "store-helper"

An example draft implementation of the helper binary is store-helper of stargz-store: https://github.com/containerd/stargz-snapshotter/pull/1674 This binary is executed by c/image and receives the registry creds from stdin and shares them to stargz-store via the unix socket of stargz-store. Then stargz-store uses these creds for registry authentication.

c/image passes DockerAuthConfig structures with keying them with the image reference.

{
  "image-reference": {
    "username": "username",
    "password": "password",
    "identitytoken": "identitytoken"
  }
}
TomSweeneyRedHat commented 1 month ago

@ktock Skopeo test is failing and you need a rebase.

ktock commented 1 month ago

Rebased. And skopeo error seems to be resolved.

rhatdan commented 1 month ago

@mtrmac PTAL

ktock commented 1 month ago

Added docs to docs/containers-registries.conf.5.md

ktock commented 1 month ago

Could we move this forward?

TomSweeneyRedHat commented 1 month ago

@giuseppe thoughts?

@ktock, looks like you need a rebase.

ktock commented 1 month ago

@TomSweeneyRedHat Thanks for the review. FIxed and rebased the patch.

ktock commented 4 weeks ago

@giuseppe PTAL

mtrmac commented 4 weeks ago

Thanks again!