Skip to content

Commit

Permalink
fix: Set empty origin auth if no credentials configured (#306)
Browse files Browse the repository at this point in the history
* test(e2e): Switch to config path mirror config for containerd

* fix: Set empty origin auth if no credentials configured

This fixes an incorrect 401 returned from registry if no credentials are
returned from the kubelet credential provider.

* fixup! test: Add empty origin auth info

* fixup! test: Tweak test config
  • Loading branch information
jimmidyson authored May 8, 2024
1 parent 1ae26e9 commit 662d158
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 38 deletions.
5 changes: 1 addition & 4 deletions pkg/credentialprovider/dynamic/dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,9 @@ func (p *dynamicProvider) GetCredentials(
if err != nil {
return nil, fmt.Errorf("failed to get origin credentials: %w", err)
}
}

if originAuthFound {
authMap[img] = originAuthConfig

if !mirrorAuthFound || cacheDuration > originCacheDuration {
if originAuthFound && (!mirrorAuthFound || cacheDuration > originCacheDuration) {
cacheDuration = originCacheDuration
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/credentialprovider/dynamic/dynamic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ func Test_dynamicProvider_GetCredentials(t *testing.T) {
CacheKeyType: credentialproviderv1.ImagePluginCacheKeyType,
CacheDuration: &metav1.Duration{Duration: expectedDummyDuration},
Auth: map[string]credentialproviderv1.AuthConfig{
wildcardDomain: {Username: mirrorUser, Password: mirrorPassword},
wildcardDomain: {Username: mirrorUser, Password: mirrorPassword},
"noorigin/image:v1.2.3.4": {Username: "", Password: ""},
},
},
},
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/cluster/kind.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"time"

"github.com/docker/docker/pkg/namesgenerator"
"github.com/onsi/ginkgo/v2"
yaml "gopkg.in/yaml.v3"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -77,6 +78,8 @@ func NewKinDCluster(
"--config", cfgFile,
"--retain",
)
cmd.Stdout = ginkgo.GinkgoWriter
cmd.Stderr = ginkgo.GinkgoWriter

err = cmd.Run()
if err != nil {
Expand Down
9 changes: 5 additions & 4 deletions test/e2e/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/image"
"github.com/docker/docker/api/types/registry"
"github.com/docker/docker/client"
)
Expand Down Expand Up @@ -82,7 +83,7 @@ func RunContainerInBackground(
}
}

out, err := dClient.ImagePull(ctx, containerCfg.Image, types.ImagePullOptions{})
out, err := dClient.ImagePull(ctx, containerCfg.Image, image.PullOptions{})
defer func() { _ = out.Close() }()
if err != nil {
_, _ = io.Copy(os.Stderr, out)
Expand Down Expand Up @@ -161,7 +162,7 @@ func RetagAndPushImage( //nolint:revive // Lots of args is fine in these tests.
out, err := dClient.ImagePull(
ctx,
srcImage,
types.ImagePullOptions{RegistryAuth: authString(pullUsername, pullPassword)},
image.PullOptions{RegistryAuth: authString(pullUsername, pullPassword)},
)
defer func() {
if out != nil {
Expand All @@ -184,12 +185,12 @@ func RetagAndPushImage( //nolint:revive // Lots of args is fine in these tests.
if err := dClient.ImageTag(ctx, srcImage, destImage); err != nil {
return fmt.Errorf("failed to retag image: %w", err)
}
defer func() { _, _ = dClient.ImageRemove(ctx, destImage, types.ImageRemoveOptions{}) }()
defer func() { _, _ = dClient.ImageRemove(ctx, destImage, image.RemoveOptions{}) }()

out, err := dClient.ImagePush(
ctx,
destImage,
types.ImagePushOptions{RegistryAuth: authString(pushUsername, pushPassword)},
image.PushOptions{RegistryAuth: authString(pushUsername, pushPassword)},
)
defer func() {
if out != nil {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/suites/mirror/dynamic_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ var _ = Describe("Successful",
if len(pod.Status.ContainerStatuses) == 0 {
return ""
}
return pod.Status.ContainerStatuses[0].State.Waiting.Reason
return ptr.Deref(pod.Status.ContainerStatuses[0].State.Waiting, corev1.ContainerStateWaiting{}).Reason
}, time.Minute, time.Second).WithContext(ctx).
Should(Or(Equal("ErrImagePull"), Equal("ImagePullBackOff")))
})
Expand Down
61 changes: 40 additions & 21 deletions test/e2e/suites/mirror/mirror_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ type imageCredentialProviderConfigData struct {
MirrorAddress string
}

type containerdMirrorHostsConfigData struct {
MirrorAddress string
MirrorCACertPath string
}

func testdataPath(f string) string {
return filepath.Join("testdata", f)
}
Expand Down Expand Up @@ -169,39 +174,53 @@ var _ = SynchronizedBeforeSuite(
filepath.Join(providerBinDir, "static-credential-provider"),
)).To(Succeed())

By("Setting up containerd mirror hosts configuration")
containerdMirrorHostsConfigDir := GinkgoT().TempDir()

containerdMirrorMirrorHostsConfigDir := filepath.Join(containerdMirrorHostsConfigDir, mirrorRegistry.Address())
Expect(copy.Copy(
mirrorRegistry.CACertFile(),
filepath.Join(containerdMirrorMirrorHostsConfigDir, "ca.crt"),
)).To(Succeed())

containerdMirrorDefaultHostsConfigDir := filepath.Join(containerdMirrorHostsConfigDir, "_default")
Expect(os.MkdirAll(containerdMirrorDefaultHostsConfigDir, 0o755)).To(Succeed())
templatedFile, err = os.Create(
filepath.Join(containerdMirrorDefaultHostsConfigDir, "hosts.toml"),
)
Expect(err).NotTo(HaveOccurred())
Expect(configTemplates.ExecuteTemplate(
templatedFile,
"hosts.yaml.tmpl",
containerdMirrorHostsConfigData{
MirrorAddress: mirrorRegistry.Address(),
MirrorCACertPath: fmt.Sprintf("/etc/containerd/certs.d/%s/ca.crt", mirrorRegistry.Address()),
},
)).To(Succeed())

By("Starting KinD cluster")
kindCluster, kcName, kubeconfig, err := cluster.NewKinDCluster(
ctx,
&v1alpha4.Cluster{
KubeadmConfigPatches: []string{
kubeadmInitPatchKubeletCredentialProviderExtraArgs,
kubeadmJoinPatchKubeletCredentialProviderExtraArgs,
},
Nodes: []v1alpha4.Node{{
Role: v1alpha4.ControlPlaneRole,
Image: "ghcr.io/mesosphere/kind-node:v1.26.4",
Image: "ghcr.io/mesosphere/kind-node:v1.30.0",
ExtraMounts: []v1alpha4.Mount{{
HostPath: mirrorRegistry.CACertFile(),
ContainerPath: "/etc/containerd/mirror-registry-ca.pem",
Readonly: true,
}, {
HostPath: providerBinDir,
ContainerPath: "/etc/kubernetes/image-credential-provider/",
}, {
HostPath: containerdMirrorHostsConfigDir,
ContainerPath: "/etc/containerd/certs.d/",
Readonly: true,
}},
}},
KubeadmConfigPatches: []string{
kubeadmInitPatchKubeletCredentialProviderExtraArgs,
kubeadmJoinPatchKubeletCredentialProviderExtraArgs,
},
ContainerdConfigPatches: []string{
fmt.Sprintf(
`[plugins."io.containerd.grpc.v1.cri".registry.mirrors."docker.io"]
endpoint = ["https://%[1]s"]
[plugins."io.containerd.grpc.v1.cri".registry.mirrors."k8s.gcr.io"]
endpoint = ["https://%[1]s"]
[plugins."io.containerd.grpc.v1.cri".registry.mirrors."*"]
endpoint = ["https://%[1]s"]
[plugins."io.containerd.grpc.v1.cri".registry.configs."%[1]s".tls]
ca_file = "/etc/containerd/mirror-registry-ca.pem"
`,
mirrorRegistry.Address(),
),
`[plugins."io.containerd.grpc.v1.cri".registry]
config_path = "/etc/containerd/certs.d"`,
},
},
)
Expand Down
15 changes: 10 additions & 5 deletions test/e2e/suites/mirror/mirror_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ var _ = Describe("Successful",
Expect(
pod.Status.ContainerStatuses[0].State.Waiting.Message,
).To(HaveSuffix("not found"))
})
},
)

It("pull image from origin when it does not exist in mirror",
func(ctx SpecContext) {
Expand All @@ -61,7 +62,8 @@ var _ = Describe("Successful",
return objStatus(pod, scheme.Scheme)
}, time.Minute, time.Second).WithContext(ctx).
Should(Equal(status.CurrentStatus))
})
},
)

It("pull image that only exists in mirror using origin style address",
func(ctx SpecContext) {
Expand Down Expand Up @@ -90,7 +92,8 @@ var _ = Describe("Successful",
return objStatus(pod, scheme.Scheme)
}, time.Minute, time.Second).WithContext(ctx).
Should(Equal(status.CurrentStatus))
})
},
)

It("pull image that only exists in mirror using mirror address",
func(ctx SpecContext) {
Expand Down Expand Up @@ -120,5 +123,7 @@ var _ = Describe("Successful",
return objStatus(pod, scheme.Scheme)
}, time.Minute, time.Second).WithContext(ctx).
Should(Equal(status.CurrentStatus))
})
})
},
)
},
)
3 changes: 3 additions & 0 deletions test/e2e/suites/mirror/testdata/hosts.yaml.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[host."{{ .MirrorAddress }}"]
capabilities = ["pull", "resolve"]
ca = "{{ .MirrorCACertPath }}"
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
"cacheDuration":"0s",
"auth":{
{{- if .MirrorAddress }}
{{ printf "%q" .MirrorAddress }}: {"username": {{ printf "%q" .MirrorUsername }}, "password": {{ printf "%q" .MirrorPassword }}},
{{- end }}
{{ printf "%q" .MirrorAddress }}: {"username": {{ printf "%q" .MirrorUsername }}, "password": {{ printf "%q" .MirrorPassword }}}{{ if .DockerHubUsername }},{{ end }}{{ end }}
{{- if .DockerHubUsername }}
"docker.io": {"username": {{ printf "%q" .DockerHubUsername }}, "password": {{ printf "%q" .DockerHubPassword }}}
{{- end }}
}
}

0 comments on commit 662d158

Please sign in to comment.