Skip to content

Conversation

wagoodman
Copy link
Contributor

Addresses anchore/syft#936

This PR builds on #120, where we tailor URLs for dockerhub when authenticating against the registry (via the docker daemon), however, this original change was meant to fix behavior when using the credential helpers. This change regressed behavior for users that have auth entries directly in the ~/.docker/config.json. This PR changes the auth behavior to search for credentials with the credentials workaround first, and if not found, then fallback behavior is to try the credential search again with the original url.

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@github-actions
Copy link

github-actions bot commented Apr 6, 2022

Benchmark Test Results

Benchmark results from the latest changes vs base branch
name                                             old time/op    new time/op    delta
pkg:github.com/anchore/stereoscope/pkg/file goos:linux goarch:amd64
TarIndex-2                                         38.9µs ± 2%    49.7µs ± 1%  +27.76%  (p=0.029 n=4+4)
pkg:github.com/anchore/stereoscope/test/integration goos:linux goarch:amd64
SimpleImage_GetImage/FromTarball-2                 1.34ms ± 7%    1.62ms ± 6%  +20.96%  (p=0.008 n=5+5)
SimpleImage_GetImage/FromOciTarball-2              1.25ms ±10%    1.54ms ±11%  +22.83%  (p=0.008 n=5+5)
SimpleImage_GetImage/FromOciDirectory-2             741µs ± 5%     909µs ± 3%  +22.67%  (p=0.008 n=5+5)
SimpleImage_FetchSquashedContents/FromTarball-2    17.0µs ± 1%    20.4µs ± 1%  +19.85%  (p=0.016 n=4+5)

name                                             old alloc/op   new alloc/op   delta
pkg:github.com/anchore/stereoscope/pkg/file goos:linux goarch:amd64
TarIndex-2                                         5.69kB ± 0%    5.69kB ± 0%   +0.03%  (p=0.029 n=4+4)
pkg:github.com/anchore/stereoscope/test/integration goos:linux goarch:amd64
SimpleImage_GetImage/FromTarball-2                  355kB ± 0%     350kB ± 0%   -1.29%  (p=0.008 n=5+5)
SimpleImage_GetImage/FromOciTarball-2               641kB ± 0%     641kB ± 0%     ~     (p=0.095 n=5+5)
SimpleImage_GetImage/FromOciDirectory-2             403kB ± 0%     403kB ± 0%     ~     (p=0.167 n=5+5)
SimpleImage_FetchSquashedContents/FromTarball-2    2.71kB ± 0%    2.71kB ± 0%     ~     (all equal)

name                                             old allocs/op  new allocs/op  delta
pkg:github.com/anchore/stereoscope/pkg/file goos:linux goarch:amd64
TarIndex-2                                           93.0 ± 0%      93.0 ± 0%     ~     (all equal)
pkg:github.com/anchore/stereoscope/test/integration goos:linux goarch:amd64
SimpleImage_GetImage/FromTarball-2                  2.60k ± 0%     2.47k ± 0%   -5.01%  (p=0.029 n=4+4)
SimpleImage_GetImage/FromOciTarball-2               1.44k ± 0%     1.44k ± 0%     ~     (all equal)
SimpleImage_GetImage/FromOciDirectory-2             1.23k ± 0%     1.23k ± 0%     ~     (all equal)
SimpleImage_FetchSquashedContents/FromTarball-2      21.0 ± 0%      21.0 ± 0%     ~     (all equal)

ref, err := name.ParseReference(imageStr)
if err != nil {
return "", err
}

url := ref.Context().RegistryStr()
if url == "index.docker.io" {
if dockerhubWorkaround && url == "index.docker.io" {
// why do this? There is an upstream issue here: https://github.com/docker/docker-credential-helpers/blob/e595cd69465c6b0f7af2d49582b82fdeddecbf75/wincred/wincred_windows.go#L113-L127
Copy link
Contributor

Choose a reason for hiding this comment

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

For people coming to this PR. This comment gives a lot of the context as to why this whole daemon provider works the way it does.

@wagoodman wagoodman merged commit c03a18a into main Apr 6, 2022
@wagoodman wagoodman deleted the docker-config-auths-fix branch April 6, 2022 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants