Skip to content

Commit

Permalink
Improve logging information on failed to get manifest error (#126)
Browse files Browse the repository at this point in the history
* Improve logging information on failed to get manifest error

For better understanding of some issues with noe and authentication against some registries, we want to enrich the logging format for when a desired manifest cannot be retrieved.

This small pull request improves that logline.

* fixup! Merge branch 'main' into better-logging

* fixup! fixup! Merge branch 'main' into better-logging

* fixup! fixup! fixup! Merge branch 'main' into better-logging

* fixup! fixup! fixup! fixup! Merge branch 'main' into better-logging

* fixup! Merge branch 'main' into better-logging

---------

Co-authored-by: Sebastian Caldarola <sebastian.caldarola@ådevinta.com>
  • Loading branch information
scaldarola and Sebastian Caldarola authored Jul 17, 2024
1 parent 667744f commit ec066c1
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 6 deletions.
6 changes: 5 additions & 1 deletion pkg/registry/anonymous_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ func (r AnonymousAuthenticator) Authenticate(ctx context.Context, imagePullSecre
}
}
select {
case candidates <- AuthenticationToken{}:
case candidates <- AuthenticationToken{
Ref: AuthenticationSourceRef{
Provider: "anonymous",
},
}:
case <-ctx.Done():
return
}
Expand Down
1 change: 1 addition & 0 deletions pkg/registry/anonymous_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func TestAnonymousAuthenticator(t *testing.T) {
candidate := <-candidates
assert.Empty(t, candidate.Kind)
assert.Empty(t, candidate.Token)
assert.Equal(t, candidate.Ref.Provider, "anonymous")
}

func TestAnonymousAuthenticatorSkipsPrivateRegistries(t *testing.T) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/registry/containerd_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ func (r ContainerDAuthenticator) Authenticate(ctx context.Context, imagePullSecr
case candidates <- AuthenticationToken{
Kind: "Basic",
Token: containerdAuth.Header,
Ref: AuthenticationSourceRef{
Provider: "containerD",
},
}:
case <-ctx.Done():
return
Expand Down
3 changes: 3 additions & 0 deletions pkg/registry/containerd_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ func TestRegistryAuthenticator_GetHeaderOnContainerdFiles(t *testing.T) {
expectedToken := AuthenticationToken{
Kind: "Basic",
Token: "dXNlcjpwYXNz",
Ref: AuthenticationSourceRef{
Provider: "containerD",
},
}

assert.Equal(t, expectedToken, receivedToken)
Expand Down
4 changes: 4 additions & 0 deletions pkg/registry/docker_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type DockerConfig struct {
type DockerConfigAuthenticator struct {
scheme *runtime.Scheme
KubeletAuthenticator Authenticator
Provider string
}

func (r DockerConfigAuthenticator) parseDockerConfig(reader io.ReadCloser) (DockerConfig, error) {
Expand Down Expand Up @@ -96,6 +97,9 @@ func (r DockerConfigAuthenticator) Authenticate(ctx context.Context, cfg DockerC
case candidates <- AuthenticationToken{
Kind: "Basic",
Token: auth,
Ref: AuthenticationSourceRef{
Provider: r.Provider,
},
}:
case <-ctx.Done():
return
Expand Down
10 changes: 8 additions & 2 deletions pkg/registry/docker_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func TestDockerAuthenticatorWithimagePullSecret(t *testing.T) {
image := "myimage"
tag := "latest"

authenticator := ImagePullSecretAuthenticator{} // Create an instance of the RegistryAuthenticator
authenticator := ImagePullSecretAuthenticator{DockerConfigAuthenticator{Provider: "ImagePullSecret"}} // Create an instance of the RegistryAuthenticator
candidates := make(chan AuthenticationToken)
go authenticator.Authenticate(context.Background(), imagePullSecret, registry, image, tag, candidates)

Expand All @@ -24,6 +24,9 @@ func TestDockerAuthenticatorWithimagePullSecret(t *testing.T) {
expectedToken := AuthenticationToken{
Kind: "Basic",
Token: "YXV0aDp1c2VyOnBhc3M=",
Ref: AuthenticationSourceRef{
Provider: "ImagePullSecret",
},
}

assert.Equal(t, expectedToken, receivedToken)
Expand All @@ -39,7 +42,7 @@ func TestDockerConfigFileWithimagePullSecret(t *testing.T) {
fs := afero.NewMemMapFs()
afero.WriteFile(fs, "/var/lib/kubelet/config.json", []byte(imagePullSecret), 0644)

authenticator := DockerConfigFileAuthenticator{fs: fs} // Create an instance of the RegistryAuthenticator
authenticator := DockerConfigFileAuthenticator{fs: fs, DockerConfigAuthenticator: DockerConfigAuthenticator{Provider: "docker-config"}} // Create an instance of the RegistryAuthenticator
candidates := make(chan AuthenticationToken)
go authenticator.Authenticate(context.Background(), imagePullSecret, registry, image, tag, candidates)

Expand All @@ -49,6 +52,9 @@ func TestDockerConfigFileWithimagePullSecret(t *testing.T) {
expectedToken := AuthenticationToken{
Kind: "Basic",
Token: "YXV0aDp1c2VyOnBhc3M=",
Ref: AuthenticationSourceRef{
Provider: "docker-config",
},
}

assert.Equal(t, expectedToken, receivedToken)
Expand Down
3 changes: 3 additions & 0 deletions pkg/registry/kubelet_credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,9 @@ func (r KubeletAuthenticator) tryIndividualKubeletProvider(ctx context.Context,
candidates <- AuthenticationToken{
Kind: "Basic",
Token: base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", value.Username, value.Password))),
Ref: AuthenticationSourceRef{
Provider: "kubelet",
},
}
} else {
log.DefaultLogger.WithContext(ctx).Info("image does not match kubelet credentials provider response, skipping it")
Expand Down
9 changes: 7 additions & 2 deletions pkg/registry/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ import (
type AuthenticationToken struct {
Kind string
Token string
Ref AuthenticationSourceRef
}

type AuthenticationSourceRef struct {
Provider string
}

type Authenticator interface {
Expand All @@ -37,7 +42,7 @@ func NewAuthenticator(kubeletConfigFile, kubeletBinDir string, privateRegistryPa

fs := afero.NewOsFs()
a := Authenticators{
ImagePullSecretAuthenticator{},
ImagePullSecretAuthenticator{DockerConfigAuthenticator: DockerConfigAuthenticator{Provider: "ImagePullSecret"}},
}
if kubeletConfigFile != "" && kubeletBinDir != "" {
a = append(a, KubeletAuthenticator{fs: fs, scheme: newScheme(), BinDir: kubeletBinDir, Config: kubeletConfigFile})
Expand All @@ -46,7 +51,7 @@ func NewAuthenticator(kubeletConfigFile, kubeletBinDir string, privateRegistryPa
}
a = append(a,
ContainerDAuthenticator{fs: fs},
DockerConfigFileAuthenticator{fs: fs},
DockerConfigFileAuthenticator{fs: fs, DockerConfigAuthenticator: DockerConfigAuthenticator{Provider: "docker-config"}},
AnonymousAuthenticator{
PrivateRegistryPatterns: cleanRegistryPatterns(privateRegistryPaterns),
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/registry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ func (r *PlainRegistry) listArchsWithAuth(ctx context.Context, client http.Clien
}

if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("failed to get manifest list. Unexpected status code %d. Expecting %d", resp.StatusCode, http.StatusOK)
return nil, fmt.Errorf("failed to get manifest list for image %s in registry %s using the provider %s. Unexpected status code %d. Expecting %d", image, registry, auth.Ref.Provider, resp.StatusCode, http.StatusOK)
}
r.updateRemaingRateLimits(ctx, registry, resp)

Expand Down

0 comments on commit ec066c1

Please sign in to comment.