Skip to content

refactor and simplify various code-paths related to distribution / authentication #49696

Merged
thaJeztah merged 15 commits intomoby:masterfrom
thaJeztah:registry_cleanup_step1
Mar 28, 2025
Merged

refactor and simplify various code-paths related to distribution / authentication #49696
thaJeztah merged 15 commits intomoby:masterfrom
thaJeztah:registry_cleanup_step1

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Mar 24, 2025

distribution: remove vars that shadowed imports or types

distribution: newRepository: don't require RepositoryInfo

This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.

distribution: layerDescriptor: don't require RepositoryInfo

This type only uses the name / reference, and doesn't use any of the
other properties provided in RepositoryInfo.

distribution: newPuller: don't require RepositoryInfo

This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo

distribution: pullEndpoints: don't require RepositoryInfo

The callback only used the name / reference

distribution: pullEndpoints: don't return RepositoryInfo

we're only consuming the name returned.

distribution: pullEndpoints: skip using Service.ResolveRepository

Service.ResolveRepository is a shallow wrapper around newRepositoryInfo,
from which we only consume the Name field. That field is a direct result
of reference.TrimNamed, so we can replace this with that.

distribution; newPusher: don't require RepositoryInfo

This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.

distribution: Push: skip using Service.ResolveRepository

Service.ResolveRepository is a shallow wrapper around newRepositoryInfo,
from which we only consume the Name field. That field is a direct result
of reference.TrimNamed, so we can replace this with that.

distribution: GetRepositories skip using Service.ResolveRepository

Service.ResolveRepository is a shallow wrapper around newRepositoryInfo,
from which we only consume the Name field. That field is a direct result
of reference.TrimNamed, so we can replace this with that.

daemon/containerd: registryResolver: remove IsInsecureRegistry

It's not called anywhere, so we can remove it from this interface.

lookup auth-config without depending on RepositoryInfo

Simplify how we lookup auth-config, as we don't need the
additional information provided by RepositoryInfo. There's
still more layers to peel off, which will be done in follow-ups.

registry: ResolveAuthConfig: inline newIndexInfo code

inline a simplified version of "newIndexInfo" without handling of
insecure registries and mirrors, as we don't need that information
to resolve the auth-config.

daemon/containerd: remove registryResolver interface

While it's generally better to define interfaces locally, this one
now duplicated distribution.RegistryResolver, and it's passed on
to other types which expect that interface.

Remove this (un-exported) interface to make it easier to discover
what's used where.

distribution: pushDescriptor: rename repoInfo to repoName

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah added area/distribution Image Distribution status/2-code-review kind/refactor PR's that refactor, or clean-up code area/authentication labels Mar 24, 2025
@thaJeztah thaJeztah marked this pull request as ready for review March 24, 2025 18:13
@thaJeztah thaJeztah force-pushed the registry_cleanup_step1 branch from 84dec33 to 9f09cc2 Compare March 25, 2025 08:31
@thaJeztah thaJeztah added this to the 28.0.3 milestone Mar 25, 2025
@vvoland vvoland modified the milestones: 28.0.3, 28.0.4 Mar 25, 2025
@thaJeztah thaJeztah requested review from robmry and vvoland March 25, 2025 10:27
@vvoland vvoland modified the milestones: 28.0.4, 28.0.5 Mar 25, 2025
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
// TODO(thaJeztah): do we need p.repoInfo at all, as it would probably be same as ref?
p.repo, err = newRepository(ctx, p.repoInfo.Name, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "pull")
p.repo, err = newRepository(ctx, p.repoName, p.endpoint, p.config.MetaHeaders, p.config.AuthConfig, "pull")
Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like I forgot to update the comment here; let me do a quick rebase and fix that

This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This type only uses the name / reference, and doesn't use any of the
other properties provided in RepositoryInfo.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The callback only used the name / reference

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
we're only consuming the name returned.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[Service.ResolveRepository] is a shallow wrapper around [newRepositoryInfo],
from which we only consume the `Name` field. That field is a direct result
of `reference.TrimNamed`, so we can replace this with that.

[Service.ResolveRepository]: https://github.com/moby/moby/blob/ecb03c4cdae6f323150fc11b303dcc5dc4d82416/registry/service.go#L106-L111
[newRepositoryInfo]: https://github.com/moby/moby/blob/ecb03c4cdae6f323150fc11b303dcc5dc4d82416/registry/config.go#L392-L408

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This constructor only uses the name / reference, and doesn't
use any of the other properties provided in RepositoryInfo.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[Service.ResolveRepository] is a shallow wrapper around [newRepositoryInfo],
from which we only consume the `Name` field. That field is a direct result
of `reference.TrimNamed`, so we can replace this with that.

[Service.ResolveRepository]: https://github.com/moby/moby/blob/ecb03c4cdae6f323150fc11b303dcc5dc4d82416/registry/service.go#L106-L111
[newRepositoryInfo]: https://github.com/moby/moby/blob/ecb03c4cdae6f323150fc11b303dcc5dc4d82416/registry/config.go#L392-L408

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
[Service.ResolveRepository] is a shallow wrapper around [newRepositoryInfo],
from which we only consume the `Name` field. That field is a direct result
of `reference.TrimNamed`, so we can replace this with that.

[Service.ResolveRepository]: https://github.com/moby/moby/blob/ecb03c4cdae6f323150fc11b303dcc5dc4d82416/registry/service.go#L106-L111
[newRepositoryInfo]: https://github.com/moby/moby/blob/ecb03c4cdae6f323150fc11b303dcc5dc4d82416/registry/config.go#L392-L408

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
It's not called anywhere, so we can remove it from this interface.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Simplify how we lookup auth-config, as we don't need the
additional information provided by RepositoryInfo. There's
still more layers to peel off, which will be done in follow-ups.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
inline a simplified version of "newIndexInfo" without handling of
insecure registries and mirrors, as we don't need that information
to resolve the auth-config.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
While it's generally better to define interfaces locally, this one
now duplicated distribution.RegistryResolver, and it's passed on
to other types which expect that interface.

Remove this (un-exported) interface to make it easier to discover
what's used where.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the registry_cleanup_step1 branch from 9f09cc2 to ec7fe73 Compare March 28, 2025 09:56
@thaJeztah thaJeztah merged commit 81e267c into moby:master Mar 28, 2025
150 checks passed
@thaJeztah thaJeztah deleted the registry_cleanup_step1 branch March 28, 2025 14:48
@thompson-shaun thompson-shaun modified the milestones: 28.0.5, 28.1.0 Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/authentication area/distribution Image Distribution kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants