-
Notifications
You must be signed in to change notification settings - Fork 5.5k
use containerd registry client #13245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Looks like you missed a test-file;
|
pkg/compose/publish.go
Outdated
if host == "registry-1.docker.io" { | ||
host = "https://index.docker.io/v1/" | ||
} | ||
auth, err := config.GetAuthConfig(host) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should use the internal utility to keep it central? Or is the logic different here?
compose/internal/registry/registry.go
Lines 30 to 38 in 032e030
// GetAuthConfigKey special-cases using the full index address of the official | |
// index as the AuthConfig key, and uses the (host)name[:port] for private indexes. | |
func GetAuthConfigKey(reposName reference.Named) string { | |
indexName := reference.Domain(reposName) | |
if indexName == IndexName || indexName == IndexHostname { | |
return IndexServer | |
} | |
return indexName | |
} |
That one doesn't use registry-1.docker.io
though; ISTR we had that at some point, but some users depended on registry-1.docker.io
not to be normalised to docker.io
(and thus "default auth")
(😭 I need to cry every time I run into these)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😅
17817ee
to
499aa34
Compare
"github.com/compose-spec/compose-go/v2/types" | ||
"github.com/docker/compose/v2/internal/ocipush" | ||
"github.com/docker/compose/v2/pkg/api" | ||
v1 "github.com/opencontainers/image-spec/specs-go/v1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(very unrelated) In our other repositories, we alias this import as either ocispec
(or ocispecs
); I can open a follow-up PR to update the aliases in this repo, which may help reading the code.
internal/registry/registry.go
Outdated
IndexServer = "https://index.docker.io/v1/" | ||
// IndexName is the name of the index | ||
IndexName = "docker.io" | ||
// LegacyRegistry is the legacy name of the inde |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually the canonical, only correct hostname of the registry 🫠
https://github.com/moby/moby/blob/2670796a010406b6b35aaff1cdd02f4309e79060/daemon/pkg/registry/config.go#L40-L46
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just 1 small nit, tests ok on my side 👍
internal/ocipush/push.go
Outdated
_, err = push.Write(descriptor.Data) | ||
if err != nil { | ||
return err | ||
} | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_, err = push.Write(descriptor.Data) | |
if err != nil { | |
return err | |
} | |
return nil | |
_, err = push.Write(descriptor.Data) | |
return err |
internal/registry/registry.go
Outdated
// IndexName is the name of the index | ||
IndexName = "docker.io" | ||
) | ||
import "github.com/docker/docker/registry" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OH! Don't import this one; it is removed in the next release / moved internal. It's the package we want to get rid of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮💨
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, while those are used everywhere there's no canonical place where such const are declared ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't even think Docker Hub publicly documents them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't even think Docker Hub publicly documents them
- OH, actually it does now; registry: add reference docs#22497
But yeah, that package was my attempt at having a somewhat documented place for these, and at the time there were hopes that we would finally have the registry running at docker.io
, but that effort didn't go anywhere, and that package was problematic, and didn't fit in "client" or "api" for Moby.
8431002
to
d8a110b
Compare
Signed-off-by: Nicolas De Loof <[email protected]>
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | minor | `v2.39.4` -> `v2.40.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>docker/compose (docker/compose)</summary> ### [`v2.40.0`](https://github.com/docker/compose/releases/tag/v2.40.0) [Compare Source](docker/compose@v2.39.4...v2.40.0) #### What's Changed ##### ✨ Improvements - publish Compose application as compose.yaml + images by [@​ndeloof](https://github.com/ndeloof) in [#​13257](docker/compose#13257) ##### 🐛 Fixes - resolve secrets based on env var before executing bake by [@​ndeloof](https://github.com/ndeloof) in [#​13237](docker/compose#13237) - pass bake secrets by env by [@​ndeloof](https://github.com/ndeloof) in [#​13249](docker/compose#13249) - escape $ in bake.json as interpolation already has been managed by cpmpose by [@​ndeloof](https://github.com/ndeloof) in [#​13259](docker/compose#13259) ##### 🔧 Internal - pkg/compose: remove uses of deprecated mitchellh/mapstructure module by [@​thaJeztah](https://github.com/thaJeztah) in [#​13239](docker/compose#13239) - pkg/watch: remove unused IsWindowsShortReadError by [@​thaJeztah](https://github.com/thaJeztah) in [#​13052](docker/compose#13052) - pkg/compose: build: remove permissions warning on Windows by [@​thaJeztah](https://github.com/thaJeztah) in [#​13236](docker/compose#13236) - pluginMain: remove uses of DockerCLI.Apply by [@​thaJeztah](https://github.com/thaJeztah) in [#​13240](docker/compose#13240) - use containerd registry client by [@​ndeloof](https://github.com/ndeloof) in [#​13245](docker/compose#13245) - provider services: use '--project-name=' notation by [@​glours](https://github.com/glours) in [#​13250](docker/compose#13250) - gha: update test-matrix: remove docker 26.x by [@​thaJeztah](https://github.com/thaJeztah) in [#​13254](docker/compose#13254) - pkg/compose: explicitly map AuthConfig fields instead of a direct cast by [@​thaJeztah](https://github.com/thaJeztah) in [#​13253](docker/compose#13253) - cmd/compose: fix minor linting issues by [@​thaJeztah](https://github.com/thaJeztah) in [#​13252](docker/compose#13252) - use containerd client for OCI operations by [@​ndeloof](https://github.com/ndeloof) in [#​13251](docker/compose#13251) ##### ⚙️ Dependencies - build(deps): bump github.com/docker/docker, docker/cli v28.5.0-rc.1 by [@​thaJeztah](https://github.com/thaJeztah) in [#​13241](docker/compose#13241) - build(deps): bump github.com/docker/docker from 28.5.0-rc.1+incompatible to 28.5.0+incompatible by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13260](docker/compose#13260) - build(deps): bump github.com/docker/cli from 28.5.0-rc.1+incompatible to 28.5.0+incompatible by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13261](docker/compose#13261) **Full Changelog**: <docker/compose@v2.39.4...v2.40.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMzIuNSIsInVwZGF0ZWRJblZlciI6IjQxLjEzMi41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
What I did
replace use of buildx's imagetool with direct use of containerd's registry client
this is related to #13056, removing compose use of buildx codebase
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did