Skip to content

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Jul 28, 2025

What does this PR do?

introduce a generic credentialsFn to retrieve credentials for pull operation
offers WithCredentialsFromConfig to get credentials from docker/cli config (current mechanism)
this makes it possible to plug an alternative func that can resolve credentials from a TBD mechanism use by the docker/cli after cli/config cleanup (see discussion on docker/cli#6210), as storing credentials in config is not the desired way.

This mkes legacyadapter to convert docker/cli Config <-> go-sdk/config useless as (AFAICT) it is only used by Pull operation to retrieve username/password. Here we model this requirement as a minimal lookup method.

@ndeloof ndeloof force-pushed the pull_credentials branch 2 times, most recently from 141fa25 to beebe0d Compare July 28, 2025 14:38
@ndeloof ndeloof marked this pull request as ready for review October 1, 2025 12:50
@Copilot Copilot AI review requested due to automatic review settings October 1, 2025 12:50
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a generic credentials function mechanism to make the credential retrieval process pluggable for Docker image pull operations. The change replaces the hardcoded Docker CLI config credential lookup with a configurable function, enabling alternative credential resolution mechanisms.

  • Adds a configurable credentialsFn field to pullOptions struct for credential retrieval
  • Introduces WithCredentialsFn option to set custom credential functions
  • Moves existing config-based credential logic to WithCredentialsFromConfig function
  • Sets WithCredentialsFromConfig as the default when no custom credential function is provided

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
image/pull.go Replaces direct config-based auth logic with pluggable credential function calls
image/options.go Adds credential function field and new option functions for configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ndeloof ndeloof force-pushed the pull_credentials branch 3 times, most recently from 410ada9 to 0680a59 Compare October 1, 2025 13:00
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

I added some unit tests, other than that LGTM!! 🙇

Will merge it as soon as the CI passes.

@mdelapenya mdelapenya self-assigned this Oct 3, 2025
@mdelapenya mdelapenya added the enhancement New feature or request label Oct 3, 2025
@mdelapenya mdelapenya merged commit 998c10e into docker:main Oct 3, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants