libimage: Fix getDockerAuthConfig() for authentication #1342
Conversation
|
I'm waiting to pass the podman CI tests which include this commit. |
|
This is ready for review. |
libimage/search.go
Outdated
| @@ -348,6 +331,41 @@ func searchRepositoryTags(ctx context.Context, sys *types.SystemContext, registr | |||
| return paramsArr, nil | |||
| } | |||
|
|
|||
| // getDockerAuthConfig extracts a docker auth config from the CopyOptions. Returns | |||
| // nil if no credentials are set. | |||
| func (options *SearchOptions) getDockerAuthConfig() (*types.DockerAuthConfig, error) { | |||
There was a problem hiding this comment.
This is a code clone of https://github.com/containers/common/blob/main/libimage/copier.go#L172.
It.s probably worth refactoring the two functions into one that would username, credetials and ID token as arguments? The getDockerAuthConfig() methods could call this function.
sstosh
force-pushed
the
search-fix
branch
3 times, most recently
from
32703e1 to
585e07e
Compare
|
The podman CI tests are passed and ready for review. |
sstosh
changed the title
libimage/copier.go
Outdated
| } | ||
|
|
||
| return nil, nil | ||
| // Return nil if there is no credential source. | ||
| if numCredsSources == 0 { |
There was a problem hiding this comment.
(Absolutely non-blocking: Alternatively, move the numCredsSources checks before creating authConf, so that we don’t create an authConf with invalid data at all.
Then this can be
switch numCredsSources {
case 0: return nil, nil
case 1: return &types.DockerAuthConfig{…}, nil
default: /* error */)
- We need to set DockerAuthConfig even if we use of non-IdentityToken credentials. - This function should be failed when there are multiple credential sources, not only `Username` + `Credentials`. Signed-off-by: Toshiki Sonoda <sonoda.toshiki@fujitsu.com>
|
Thank you for your review. I fixed it. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mtrmac, sstosh, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
We need to set DockerAuthConfig even if we use of
non-IdentityToken credentials.
This function should be failed when there are multiple
credential sources, not only
Username+Credentials.Signed-off-by: Toshiki Sonoda sonoda.toshiki@fujitsu.com