From a212f52fbab25ce8b96710c0c075b9052512c80a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20L=C3=A1zni=C4=8Dka?= Date: Wed, 4 Mar 2026 16:34:12 +0100 Subject: [PATCH 1/3] NodeConformance e2e: tests all Ensure Secret Pulled Images policies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stanislav Láznička --- test/e2e_node/image_credential_pulls.go | 119 +++++++++++++++++------- test/e2e_node/util_kubeletconfig.go | 80 ++++++++++++---- 2 files changed, 147 insertions(+), 52 deletions(-) diff --git a/test/e2e_node/image_credential_pulls.go b/test/e2e_node/image_credential_pulls.go index 1cfa7b1304e..43499a504a6 100644 --- a/test/e2e_node/image_credential_pulls.go +++ b/test/e2e_node/image_credential_pulls.go @@ -42,20 +42,19 @@ var _ = SIGDescribe("Ensure Credential Pulled Images", func() { var is internalapi.ImageManagerService framework.Describe("pulling images with credentials", framework.WithFeatureGate(features.KubeletEnsureSecretPulledImages), framework.WithSerial(), func() { + var registryAddress string var testImage string + var testImageID string var testSecret *v1.Secret var testNode string - tempSetCurrentKubeletConfig(f, func(ctx context.Context, initialConfig *kubeletconfig.KubeletConfiguration) { - initialConfig.ImagePullCredentialsVerificationPolicy = string(kubeletconfig.AlwaysVerify) - }) - ginkgo.BeforeEach(func(ctx context.Context) { var err error _, is, err = getCRIClient() framework.ExpectNoError(err) - registryAddress, registryNodeNames, err := e2eregistry.SetupRegistry(ctx, f, true) + var registryNodeNames []string + registryAddress, registryNodeNames, err = e2eregistry.SetupRegistry(ctx, f, true) framework.ExpectNoError(err) gomega.Expect(registryNodeNames).ToNot(gomega.BeEmpty(), "registry should run on at least one node") // this is to wait for the complete removal of all registry pods between tests @@ -72,40 +71,88 @@ var _ = SIGDescribe("Ensure Credential Pulled Images", func() { framework.ExpectNoError(err) // Use the registry node for scheduling - in node e2e tests, this is the single test node testNode = registryNodeNames[0] - origPod := e2ecommonnode.ImagePullTest(ctx, f, testImage, v1.PullIfNotPresent, testSecret, testNode, v1.PodRunning, false) + // PullAlways pull policy will force an ImagePulledRecord to be created + origPod := e2ecommonnode.ImagePullTest(ctx, f, testImage, v1.PullAlways, testSecret, testNode, v1.PodRunning, false) gomega.Expect(origPod.Spec.NodeName).To(gomega.Equal(testNode), "pod should be scheduled on the expected node") + + testImgStatus, err := is.ImageStatus(ctx, &runtimeapi.ImageSpec{Image: testImage}, false) + framework.ExpectNoError(err) + testImageID = testImgStatus.Image.Id }) - for _, pullPolicy := range []v1.PullPolicy{v1.PullIfNotPresent, v1.PullNever} { - framework.Context(string(pullPolicy), func() { - framework.It("pod without PullSecret cannot access previously pulled private image", func(ctx context.Context) { - _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, nil, testNode, v1.PodPending, true) + for _, credsPolicy := range []kubeletconfig.ImagePullCredentialsVerificationPolicy{ + kubeletconfig.NeverVerifyPreloadedImages, + kubeletconfig.AlwaysVerify, + kubeletconfig.NeverVerifyAllowlistedImages, + kubeletconfig.NeverVerify, + } { + framework.Context(string(credsPolicy), func() { + tempSetCurrentKubeletConfig(f, func(ctx context.Context, initialConfig *kubeletconfig.KubeletConfiguration) { + initialConfig.ImagePullCredentialsVerificationPolicy = string(credsPolicy) }) - framework.It("pod with invalid PullSecret cannot access previously pulled private image", func(ctx context.Context) { - invalidSecret, err := f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Create(ctx, &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{GenerateName: f.UniqueName}, - Type: v1.SecretTypeDockerConfigJson, - Data: map[string][]byte{ - v1.DockerConfigJsonKey: []byte(`{"auths":{"somerepo.com": {"auth": "aW52YWxpZHVzZXI6aW52YWxpZHBhc3N3b3Jk"}}}`), - }, - }, metav1.CreateOptions{}) - framework.ExpectNoError(err) - _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, invalidSecret, testNode, v1.PodPending, true) - }) - framework.It("pod with the same PullSecret can access previously pulled image", func(ctx context.Context) { - _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, testSecret, testNode, v1.PodRunning, false) - }) - framework.It("pod with the same credentials in a different secret can access previously pulled image", func(ctx context.Context) { - newSecret := testSecret.DeepCopy() - newSecret.Name = "" - newSecret.ResourceVersion = "" - newSecret.GenerateName = f.UniqueName - newSecret, err := f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Create(ctx, newSecret, metav1.CreateOptions{}) - framework.ExpectNoError(err) - _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, newSecret, testNode, v1.PodRunning, false) - }) - }) - } - }) + for _, pullPolicy := range []v1.PullPolicy{v1.PullIfNotPresent, v1.PullNever} { + framework.Context(string(pullPolicy), func() { + statusOnInvalidCreds := v1.PodPending + pullingOnInvalidCreds := true + if credsPolicy == kubeletconfig.NeverVerify { + statusOnInvalidCreds = v1.PodRunning + pullingOnInvalidCreds = false + } + + framework.It("pod without PullSecret accessing previously pulled private image", func(ctx context.Context) { + _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, nil, testNode, statusOnInvalidCreds, pullingOnInvalidCreds) + }) + framework.It("pod with invalid PullSecret accessing previously pulled private image", func(ctx context.Context) { + invalidSecret, err := f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Create(ctx, &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{GenerateName: f.UniqueName}, + Type: v1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + v1.DockerConfigJsonKey: []byte(`{"auths":{"somerepo.com": {"auth": "aW52YWxpZHVzZXI6aW52YWxpZHBhc3N3b3Jk"}}}`), + }, + }, metav1.CreateOptions{}) + framework.ExpectNoError(err) + + _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, invalidSecret, testNode, statusOnInvalidCreds, pullingOnInvalidCreds) + }) + framework.It("pod with the same PullSecret accessing previously pulled image", func(ctx context.Context) { + _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, testSecret, testNode, v1.PodRunning, false) + }) + framework.It("pod with the same credentials in a different secret accessing previously pulled image", func(ctx context.Context) { + newSecret := testSecret.DeepCopy() + newSecret.Name = "" + newSecret.ResourceVersion = "" + newSecret.GenerateName = f.UniqueName + newSecret, err := f.ClientSet.CoreV1().Secrets(f.Namespace.Name).Create(ctx, newSecret, metav1.CreateOptions{}) + framework.ExpectNoError(err) + _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, newSecret, testNode, v1.PodRunning, false) + }) + + switch credsPolicy { + case kubeletconfig.NeverVerifyAllowlistedImages: + framework.Context("[NeverVerifyAllowListedImages specific tests]", func() { + tempRemoveImagePulledRecord(f, &testImageID) + tempSetCurrentKubeletConfig(f, func(ctx context.Context, initialConfig *kubeletconfig.KubeletConfiguration) { + initialConfig.ImagePullCredentialsVerificationPolicy = string(credsPolicy) + initialConfig.PreloadedImagesVerificationAllowlist = []string{path.Join(registryAddress, "pause")} + }) + + framework.It("pod without PullSecret accessing previously pulled private image which is allowlisted", func(ctx context.Context) { + _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, nil, testNode, v1.PodRunning, false) + }) + }) + case kubeletconfig.NeverVerifyPreloadedImages: + framework.Context("[NeverVerifyPreloadedImages specific tests]", func() { + tempRemoveImagePulledRecord(f, &testImageID) + + framework.It("pod without PullSecret accessing a preloaded image", func(ctx context.Context) { + _ = e2ecommonnode.ImagePullTest(ctx, f, testImage, pullPolicy, nil, testNode, v1.PodRunning, false) + }) + }) + } // switch credsPolicy + }) // framework.Context(pullPolicy) + } // for pullPolicy + }) // framework.Context(credsPolicy) + } // for credsPolicy + }) // framework.Describe() }) diff --git a/test/e2e_node/util_kubeletconfig.go b/test/e2e_node/util_kubeletconfig.go index 1b0b4425ccb..9fe1cce3f1f 100644 --- a/test/e2e_node/util_kubeletconfig.go +++ b/test/e2e_node/util_kubeletconfig.go @@ -18,11 +18,16 @@ package e2enode import ( "context" + "crypto/sha256" + "fmt" + "os" + "path/filepath" "time" v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" "k8s.io/kubernetes/test/e2e/framework" @@ -74,24 +79,16 @@ type updateKubeletOptions struct { func updateKubeletConfigWithOptions(ctx context.Context, f *framework.Framework, kubeletConfig *kubeletconfig.KubeletConfiguration, opts updateKubeletOptions) { ginkgo.GinkgoHelper() - // Update the Kubelet configuration. - restartKubelet := mustStopKubelet(ctx, f) + withStoppedKubelet(ctx, f, opts.ensureConsistentReadyNode, func() { + // Delete CPU and memory manager state files to be sure it will not prevent the kubelet restart + if opts.deleteStateFiles { + deleteStateFile(cpuManagerStateFile) + deleteStateFile(memoryManagerStateFile) + } - // Delete CPU and memory manager state files to be sure it will not prevent the kubelet restart - if opts.deleteStateFiles { - deleteStateFile(cpuManagerStateFile) - deleteStateFile(memoryManagerStateFile) - } + framework.ExpectNoError(e2enodekubelet.WriteKubeletConfigFile(kubeletConfig)) + }) - framework.ExpectNoError(e2enodekubelet.WriteKubeletConfigFile(kubeletConfig)) - - restartKubelet(ctx) - - if opts.ensureConsistentReadyNode { - gomega.Consistently(ctx, func(ctx context.Context) bool { - return getNodeReadyStatus(ctx, f) && kubeletHealthCheck(kubeletHealthCheckURL) - }).WithTimeout(2 * time.Minute).WithPolling(2 * time.Second).Should(gomega.BeTrueBecause("node keeps reporting ready status")) - } } func updateKubeletConfig(ctx context.Context, f *framework.Framework, kubeletConfig *kubeletconfig.KubeletConfiguration, deleteStateFiles bool) { @@ -100,6 +97,57 @@ func updateKubeletConfig(ctx context.Context, f *framework.Framework, kubeletCon }) } +func tempRemoveImagePulledRecord(f *framework.Framework, imageID *string) { + pulledRecordFilenameFromID := func(imageID string) string { + return fmt.Sprintf("sha256-%x", sha256.Sum256([]byte(imageID))) + } + + ginkgo.BeforeEach(func(ctx context.Context) { + pulledRecordsDir := "/var/lib/kubelet/image_manager/pulled" + fileName := pulledRecordFilenameFromID(*imageID) + origPath := filepath.Join(pulledRecordsDir, fileName) + tempPath := filepath.Join(pulledRecordsDir, "temp_"+fileName) + + // wait for the kubelet to create the record + err := wait.PollUntilContextTimeout(ctx, time.Second, 5*time.Second, true, func(_ context.Context) (bool, error) { + if _, err := os.Stat(origPath); err != nil { + return false, nil + } + return true, nil + }) + framework.ExpectNoError(err, "the file with the pulled record for the image ID never appeared") + + withStoppedKubelet(ctx, f, false, func() { + err := os.Rename(origPath, tempPath) + framework.ExpectNoError(err, "failed to move the ImagePulledRecord file") + + ginkgo.DeferCleanup(func(ctx context.Context) { + withStoppedKubelet(ctx, f, false, func() { + err := os.Rename(tempPath, origPath) + framework.ExpectNoError(err, "failed to move the ImagePulledRecord file back to its original location") + }) + }) + }) + }) +} + +// withStoppedKubelet stops the kubelet, runs the `action`, and starts the kubelet again +func withStoppedKubelet(ctx context.Context, f *framework.Framework, ensureConsistentReadyNode bool, action func()) { + ginkgo.GinkgoHelper() + + kubeletStart := mustStopKubelet(ctx, f) + defer func() { + kubeletStart(ctx) + if ensureConsistentReadyNode { + gomega.Consistently(ctx, func(ctx context.Context) bool { + return getNodeReadyStatus(ctx, f) && kubeletHealthCheck(kubeletHealthCheckURL) + }).WithTimeout(2 * time.Minute).WithPolling(2 * time.Second).Should(gomega.BeTrueBecause("node keeps reporting ready status")) + } + }() + + action() +} + func getNodeReadyStatus(ctx context.Context, f *framework.Framework) bool { ginkgo.GinkgoHelper() nodeList, err := f.ClientSet.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) From be5c4d70185da841e798023801c42246a5a785e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20L=C3=A1zni=C4=8Dka?= Date: Mon, 9 Mar 2026 10:42:12 +0100 Subject: [PATCH 2/3] kubelet/imageManager: fix allowlisting absolute path images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an image absolute path was specified in the allowlist, credential reverification would have always been required regardless. Signed-off-by: Stanislav Láznička --- .../images/pullmanager/image_pull_policies.go | 50 +++- .../pullmanager/image_pull_policies_test.go | 245 +++++++++++++++--- 2 files changed, 258 insertions(+), 37 deletions(-) diff --git a/pkg/kubelet/images/pullmanager/image_pull_policies.go b/pkg/kubelet/images/pullmanager/image_pull_policies.go index 2642275a3d5..17af51e0a13 100644 --- a/pkg/kubelet/images/pullmanager/image_pull_policies.go +++ b/pkg/kubelet/images/pullmanager/image_pull_policies.go @@ -20,7 +20,7 @@ import ( "fmt" "strings" - dockerref "github.com/distribution/reference" + imageref "github.com/distribution/reference" "k8s.io/apimachinery/pkg/util/sets" kubeletconfiginternal "k8s.io/kubernetes/pkg/kubelet/apis/config" @@ -110,11 +110,19 @@ func (p *NeverVerifyAllowlistedImages) RequireCredentialVerificationForImage(ima } func (p *NeverVerifyAllowlistedImages) imageMatches(image string) bool { - if p.absoluteURLs.Has(image) { + noTagsName, err := removeTagDigest(image) + if err != nil { + // in a best-effort manner, we should still attempt to match with what + // we've got from the user + noTagsName = image + } + + if p.absoluteURLs.Has(noTagsName) { return true } + for _, prefix := range p.prefixes { - if strings.HasPrefix(image, prefix) { + if strings.HasPrefix(noTagsName, prefix) { return true } } @@ -155,12 +163,12 @@ func getAllowlistImagePattern(pattern string) (string, bool, error) { return "", false, fmt.Errorf("at least registry hostname is required") } } else { // not a wildcard - image, err := dockerref.ParseNormalizedNamed(trimmedPattern) + image, err := removeTagDigest(trimmedPattern) if err != nil { return "", false, fmt.Errorf("failed to parse as an image name: %w", err) } - if trimmedPattern != image.Name() { // image.Name() returns the image name without tag/digest + if trimmedPattern != image { return "", false, fmt.Errorf("neither tag nor digest is accepted in an image reference: %s", pattern) } @@ -169,3 +177,35 @@ func getAllowlistImagePattern(pattern string) (string, bool, error) { return trimmedPattern, true, nil } + +func removeTagDigest(image string) (string, error) { + imageParsed, err := imageref.Parse(image) + if err != nil { + return "", err + } + var tag, digest string + if tagged, ok := imageParsed.(imageref.Tagged); ok { + tag = tagged.Tag() + } + if digested, ok := imageParsed.(imageref.Digested); ok { + digest = digested.Digest().String() + } + + if len(digest) > 0 { + var foundDigest bool + image, foundDigest = strings.CutSuffix(image, "@"+digest) + if !foundDigest { + return "", fmt.Errorf("digest was not specified last, this does not appear to be a valid image spec: %q", image) + } + } + + if len(tag) > 0 { + var tagFound bool + image, tagFound = strings.CutSuffix(image, ":"+tag) + if !tagFound { + return "", fmt.Errorf("tag %q was not specified last (or before the digest), this does not appear to be a valid image spec: %q", tag, image) + } + } + + return image, nil +} diff --git a/pkg/kubelet/images/pullmanager/image_pull_policies_test.go b/pkg/kubelet/images/pullmanager/image_pull_policies_test.go index eb8af5ee0f6..675904a4e71 100644 --- a/pkg/kubelet/images/pullmanager/image_pull_policies_test.go +++ b/pkg/kubelet/images/pullmanager/image_pull_policies_test.go @@ -40,7 +40,7 @@ func TestNeverVerifyPreloadedPullPolicy(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := NeverVerifyPreloadedPullPolicy()("test-image", tt.imageRecordsExist); got != tt.want { + if got := NeverVerifyPreloadedPullPolicy()("test-image:sometag", tt.imageRecordsExist); got != tt.want { t.Errorf("NeverVerifyPreloadedPullPolicy() = %v, want %v", got, tt.want) } }) @@ -48,9 +48,12 @@ func TestNeverVerifyPreloadedPullPolicy(t *testing.T) { } func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { + const basicTestImage = "test.io/test/test-image:tag" + tests := []struct { name string imageRecordsExist bool + inputImage string allowlist []string expectedAbsolutes int expectedWildcards int @@ -60,6 +63,7 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { { name: "there are no records about the image being pulled, not in allowlist", imageRecordsExist: false, + inputImage: basicTestImage, want: true, allowlist: []string{"test.io/test/image1", "test.io/test/image2", "test.io/test/image3"}, expectedAbsolutes: 3, @@ -67,6 +71,7 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { { name: "there are records about the image being pulled, not in allowlist", imageRecordsExist: true, + inputImage: basicTestImage, want: true, allowlist: []string{"test.io/test/image1", "test.io/test/image3", "test.io/test/image2", "test.io/test/image3"}, expectedAbsolutes: 3, @@ -74,6 +79,7 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { { name: "there are no records about the image being pulled, appears in allowlist", imageRecordsExist: false, + inputImage: basicTestImage, want: false, allowlist: []string{"test.io/test/image1", "test.io/test/image2", "test.io/test/test-image", "test.io/test/image3"}, expectedAbsolutes: 4, @@ -81,62 +87,74 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { { name: "there are records about the image being pulled, appears in allowlist", imageRecordsExist: true, + inputImage: basicTestImage, want: false, allowlist: []string{"test.io/test/image1", "test.io/test/image2", "test.io/test/test-image", "test.io/test/image3"}, expectedAbsolutes: 4, }, { - name: "invalid allowlist pattern - wildcard in the middle", - wantErr: true, - allowlist: []string{"image.repo/pokus*/imagename"}, + name: "invalid allowlist pattern - wildcard in the middle", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"image.repo/pokus*/imagename"}, }, { - name: "invalid allowlist pattern - trailing non-segment wildcard middle", - wantErr: true, - allowlist: []string{"image.repo/pokus*"}, + name: "invalid allowlist pattern - trailing non-segment wildcard middle", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"image.repo/pokus*"}, }, { - name: "invalid allowlist pattern - wildcard path segment in the middle", - wantErr: true, - allowlist: []string{"image.repo/*/imagename"}, + name: "invalid allowlist pattern - wildcard path segment in the middle", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"image.repo/*/imagename"}, }, { - name: "invalid allowlist pattern - only wildcard segment", - wantErr: true, - allowlist: []string{"/*"}, + name: "invalid allowlist pattern - only wildcard segment", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"/*"}, }, { - name: "invalid allowlist pattern - ends with a '/'", - wantErr: true, - allowlist: []string{"image.repo/"}, + name: "invalid allowlist pattern - ends with a '/'", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"image.repo/"}, }, { - name: "invalid allowlist pattern - empty", - wantErr: true, - allowlist: []string{""}, + name: "invalid allowlist pattern - empty", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{""}, }, { - name: "invalid allowlist pattern - asterisk", - wantErr: true, - allowlist: []string{"*"}, + name: "invalid allowlist pattern - asterisk", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"*"}, }, { - name: "invalid allowlist pattern - image with a tag", - wantErr: true, - allowlist: []string{"test.io/test/image1:tagged"}, + name: "invalid allowlist pattern - image with a tag", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"test.io/test/image1:tagged"}, }, { - name: "invalid allowlist pattern - image with a digest", - wantErr: true, - allowlist: []string{"test.io/test/image1@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd"}, + name: "invalid allowlist pattern - image with a digest", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"test.io/test/image1@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd"}, }, { - name: "invalid allowlist pattern - trailing whitespace", - wantErr: true, - allowlist: []string{"test.io/test/image1 "}, + name: "invalid allowlist pattern - trailing whitespace", + inputImage: basicTestImage, + wantErr: true, + allowlist: []string{"test.io/test/image1 "}, }, { name: "there are no records about the image being pulled, not in allowlist - different repo wildcard", + inputImage: basicTestImage, imageRecordsExist: false, want: true, allowlist: []string{"test.io/test/image1", "test.io/test/image2", "different.repo/test/*"}, @@ -145,6 +163,7 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { }, { name: "there are no records about the image being pulled, not in allowlist - matches org wildcard", + inputImage: basicTestImage, imageRecordsExist: false, want: false, allowlist: []string{"test.io/test/image1", "test.io/test/image2", "test.io/test/*"}, @@ -153,12 +172,53 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { }, { name: "there are no records about the image being pulled, not in allowlist - matches repo wildcard", + inputImage: basicTestImage, imageRecordsExist: false, want: false, allowlist: []string{"test.io/test/image1", "test.io/test/image2", "test.io/*"}, expectedAbsolutes: 2, expectedWildcards: 1, }, + { + name: "'familiar' image name with docker-disambiguated fqdn in the allowlist - not a match", + inputImage: "ubuntu", + imageRecordsExist: false, + want: true, + allowlist: []string{"docker.io/library/ubuntu"}, + expectedAbsolutes: 1, + }, + { + name: "'familiar' image name with docker-disambiguated wildcard in the allowlist - not a match", + inputImage: "ubuntu", + imageRecordsExist: false, + want: true, + allowlist: []string{"docker.io/library/*"}, + expectedWildcards: 1, + }, + { + name: "'familiar' image name with 'familiar' image matching it in the allowlist", + inputImage: "ubuntu", + imageRecordsExist: false, + want: false, + allowlist: []string{"ubuntu"}, + expectedAbsolutes: 1, + }, + { + name: "'familiar' image name with a tag with 'familiar' image matching it in the allowlist", + inputImage: "ubuntu:tagged", + imageRecordsExist: false, + want: false, + allowlist: []string{"ubuntu"}, + expectedAbsolutes: 1, + }, + { + name: "'familiar' image name with a digest with 'familiar' image matching it in the allowlist", + inputImage: "ubuntu@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + imageRecordsExist: false, + want: false, + allowlist: []string{"ubuntu"}, + expectedAbsolutes: 1, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -179,10 +239,131 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { t.Errorf("expected %d of wildcard image URLs in the allowlist policy, got %d: %v", tt.expectedWildcards, len(policyEnforcer.prefixes), policyEnforcer.prefixes) } - got := policyEnforcer.RequireCredentialVerificationForImage("test.io/test/test-image", tt.imageRecordsExist) + got := policyEnforcer.RequireCredentialVerificationForImage(tt.inputImage, tt.imageRecordsExist) if !reflect.DeepEqual(got, tt.want) { t.Errorf("NewNeverVerifyAllowListedPullPolicy() = %v, want %v", got, tt.want) } }) } } + +func TestRemoveTagDigest(t *testing.T) { + tests := []struct { + name string + image string + want string + wantErr bool + }{ + { + name: "'familiar' image name, no tag or digest", + image: "myimage", + want: "myimage", + }, + { + name: "'familiar' image name with tag", + image: "myimage:latest", + want: "myimage", + }, + { + name: "'familiar' image name with digest", + image: "myimage@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + want: "myimage", + }, + { + name: "'familiar' image name with tag and digest", + image: "myimage:v1@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + want: "myimage", + }, + { + name: "single path segment, no tag", + image: "registry.io/myimage", + want: "registry.io/myimage", + }, + { + name: "single path segment with tag", + image: "registry.io/myimage:v1.0", + want: "registry.io/myimage", + }, + { + name: "single path segment with digest", + image: "registry.io/myimage@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + want: "registry.io/myimage", + }, + { + name: "single path segment with tag and digest", + image: "registry.io/myimage:latest@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + want: "registry.io/myimage", + }, + { + name: "domain and two path segments, no tag", + image: "registry.io/myorg/myimage", + want: "registry.io/myorg/myimage", + }, + { + name: "domain and two path segments with tag", + image: "registry.io/myorg/myimage:v2", + want: "registry.io/myorg/myimage", + }, + { + name: "domain and two path segments with digest", + image: "registry.io/myorg/myimage@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + want: "registry.io/myorg/myimage", + }, + { + name: "domain and two path segments with tag and digest", + image: "registry.io/myorg/myimage:v2@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + want: "registry.io/myorg/myimage", + }, + { + name: "deep path and tag", + image: "registry.io/a/b/c/myimage:latest", + want: "registry.io/a/b/c/myimage", + }, + { + name: "image name ends with slash", + image: "registry.io/myorg/", + wantErr: true, + }, + { + name: "last segment is only a tag", + image: "registry.io/myorg/:tag", + wantErr: true, + }, + { + name: "last segment is only a digest", + image: "registry.io/myorg/@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + wantErr: true, + }, + { + name: "domain with port and tag", + image: "localhost:5000/myimage:v1", + want: "localhost:5000/myimage", + }, + { + name: "domain with port, org, and tag", + image: "myregistry:5000/myorg/myimage:v1", + want: "myregistry:5000/myorg/myimage", + }, + { + name: "multiple occurrences of tag", + image: "docker.io/taghere/image:taghere", + want: "docker.io/taghere/image", + }, + { + name: "multiple occurrences of digest", + image: "registry.io/38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd/myorg/image@sha256:38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd", + want: "registry.io/38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd/myorg/image", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := removeTagDigest(tt.image) + if (err != nil) != tt.wantErr { + t.Fatalf("removeTagDigest(%q) error = %v, wantErr %v", tt.image, err, tt.wantErr) + } + if !tt.wantErr && got != tt.want { + t.Errorf("removeTagDigest(%q) = %q, want %q", tt.image, got, tt.want) + } + }) + } +} From 70a6b73fd8c5ddf3ce8362f79c129ef7ff37a65e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20L=C3=A1zni=C4=8Dka?= Date: Mon, 16 Mar 2026 12:38:26 +0100 Subject: [PATCH 3/3] kubelet/pullmanager: use image name parsers that don't modify the source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `k8s.io/kubernetes/pkg/util/parsers.ParseImageName()` does not only parse the input, but also adds Docker-specific domain/org for images names that exclude these - "familiar" image names, such as "alpine", for example. While this means that previous records _are_ invalid, we should be able to do this change safely as if `imageRef` matches a known record, if we miss on the "image name" level, we will always request a new pull if the Pod does not present any credentials that are already recorded, or it will write the new "image name" record with the valid presented credentials. Signed-off-by: Stanislav Láznička --- .../images/pullmanager/image_pull_manager.go | 7 --- .../pullmanager/image_pull_manager_test.go | 52 +++++++++++++++++++ .../images/pullmanager/image_pull_policies.go | 7 +-- .../pullmanager/image_pull_policies_test.go | 4 +- ...b410dfae9c8047bc6ebe57d642fe264d479e152955 | 1 + ...c4cba501d1aaf06d1481935aa1e160029afd639847 | 1 + 6 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 pkg/kubelet/images/pullmanager/testdata/pulling/sha256-5f3ca9e4d82e32203c2fafb410dfae9c8047bc6ebe57d642fe264d479e152955 create mode 100644 pkg/kubelet/images/pullmanager/testdata/pulling/sha256-7ffc99ef20589dc844b202c4cba501d1aaf06d1481935aa1e160029afd639847 diff --git a/pkg/kubelet/images/pullmanager/image_pull_manager.go b/pkg/kubelet/images/pullmanager/image_pull_manager.go index 9684223e33d..27e97381d87 100644 --- a/pkg/kubelet/images/pullmanager/image_pull_manager.go +++ b/pkg/kubelet/images/pullmanager/image_pull_manager.go @@ -29,7 +29,6 @@ import ( "k8s.io/klog/v2" kubeletconfiginternal "k8s.io/kubernetes/pkg/kubelet/apis/config" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" - "k8s.io/kubernetes/pkg/util/parsers" ) var _ ImagePullManager = &PullManager{} @@ -576,12 +575,6 @@ func imagePullSecretLess(a, b kubeletconfiginternal.ImagePullSecret) int { return strings.Compare(a.UID, b.UID) } -// trimImageTagDigest removes the tag and digest from an image name -func trimImageTagDigest(containerImage string) (string, error) { - imageName, _, _, err := parsers.ParseImageName(containerImage) - return imageName, err -} - // mergePullServiceAccounts merges two slices of ImagePullServiceAccount object into one while // keeping the objects unique per `Namespace, Name, UID` key. // The returned slice is sorted by Namespace, Name and UID (in this order). diff --git a/pkg/kubelet/images/pullmanager/image_pull_manager_test.go b/pkg/kubelet/images/pullmanager/image_pull_manager_test.go index f5ce357a9ee..d10432a6a83 100644 --- a/pkg/kubelet/images/pullmanager/image_pull_manager_test.go +++ b/pkg/kubelet/images/pullmanager/image_pull_manager_test.go @@ -1147,6 +1147,58 @@ func TestFileBasedImagePullManager_RecordImagePulled(t *testing.T) { }, }, }, + { + name: "record with familiar image name - only the last segment", + image: "testing@sha256:f24acc752be18b93b0504c86312bbaf482c9efb0c45e925bbccb0a591cebd7af", + imageRef: "testimageref-familiarlast", + creds: &kubeletconfiginternal.ImagePullCredentials{ + KubernetesServiceAccounts: []kubeletconfiginternal.ImagePullServiceAccount{ + {UID: "test-sa-uid", Namespace: "default", Name: "test-sa"}, + }, + }, + existingPulling: []string{"sha256-7ffc99ef20589dc844b202c4cba501d1aaf06d1481935aa1e160029afd639847"}, + expectPulled: []string{"sha256-f2ba1256df29972c4fe11d8af7d302e37631c6077a2ed29d8bee55ee77cf26ad"}, + pullsInFlight: 1, + expectPullingRemoved: "sha256-7ffc99ef20589dc844b202c4cba501d1aaf06d1481935aa1e160029afd639847", + checkedPullFile: "sha256-f2ba1256df29972c4fe11d8af7d302e37631c6077a2ed29d8bee55ee77cf26ad", + expectUpdated: true, + expectedPullRecord: kubeletconfiginternal.ImagePulledRecord{ + ImageRef: "testimageref-familiarlast", + CredentialMapping: map[string]kubeletconfiginternal.ImagePullCredentials{ + "testing": { + KubernetesServiceAccounts: []kubeletconfiginternal.ImagePullServiceAccount{ + {UID: "test-sa-uid", Namespace: "default", Name: "test-sa"}, + }, + }, + }, + }, + }, + { + name: "record with familiar image name - org and name", + image: "secretorg/myimage:customtag", + imageRef: "testimageref-familiarwithorg", + creds: &kubeletconfiginternal.ImagePullCredentials{ + KubernetesServiceAccounts: []kubeletconfiginternal.ImagePullServiceAccount{ + {UID: "test-sa-uid", Namespace: "default", Name: "test-sa"}, + }, + }, + existingPulling: []string{"sha256-5f3ca9e4d82e32203c2fafb410dfae9c8047bc6ebe57d642fe264d479e152955"}, + expectPulled: []string{"sha256-ff66306c3a398c6e6b64d8f7acdd1aebc37fce76add3849f9d3f59274434234a"}, + pullsInFlight: 1, + expectPullingRemoved: "sha256-5f3ca9e4d82e32203c2fafb410dfae9c8047bc6ebe57d642fe264d479e152955", + checkedPullFile: "sha256-ff66306c3a398c6e6b64d8f7acdd1aebc37fce76add3849f9d3f59274434234a", + expectUpdated: true, + expectedPullRecord: kubeletconfiginternal.ImagePulledRecord{ + ImageRef: "testimageref-familiarwithorg", + CredentialMapping: map[string]kubeletconfiginternal.ImagePullCredentials{ + "secretorg/myimage": { + KubernetesServiceAccounts: []kubeletconfiginternal.ImagePullServiceAccount{ + {UID: "test-sa-uid", Namespace: "default", Name: "test-sa"}, + }, + }, + }, + }, + }, } for _, tt := range tests { diff --git a/pkg/kubelet/images/pullmanager/image_pull_policies.go b/pkg/kubelet/images/pullmanager/image_pull_policies.go index 17af51e0a13..be6cf14dfe2 100644 --- a/pkg/kubelet/images/pullmanager/image_pull_policies.go +++ b/pkg/kubelet/images/pullmanager/image_pull_policies.go @@ -110,7 +110,7 @@ func (p *NeverVerifyAllowlistedImages) RequireCredentialVerificationForImage(ima } func (p *NeverVerifyAllowlistedImages) imageMatches(image string) bool { - noTagsName, err := removeTagDigest(image) + noTagsName, err := trimImageTagDigest(image) if err != nil { // in a best-effort manner, we should still attempt to match with what // we've got from the user @@ -163,7 +163,7 @@ func getAllowlistImagePattern(pattern string) (string, bool, error) { return "", false, fmt.Errorf("at least registry hostname is required") } } else { // not a wildcard - image, err := removeTagDigest(trimmedPattern) + image, err := trimImageTagDigest(trimmedPattern) if err != nil { return "", false, fmt.Errorf("failed to parse as an image name: %w", err) } @@ -178,7 +178,8 @@ func getAllowlistImagePattern(pattern string) (string, bool, error) { return trimmedPattern, true, nil } -func removeTagDigest(image string) (string, error) { +// trimImageTagDigest trims digest and tag from an image name +func trimImageTagDigest(image string) (string, error) { imageParsed, err := imageref.Parse(image) if err != nil { return "", err diff --git a/pkg/kubelet/images/pullmanager/image_pull_policies_test.go b/pkg/kubelet/images/pullmanager/image_pull_policies_test.go index 675904a4e71..d00c6116c8f 100644 --- a/pkg/kubelet/images/pullmanager/image_pull_policies_test.go +++ b/pkg/kubelet/images/pullmanager/image_pull_policies_test.go @@ -247,7 +247,7 @@ func TestNewNeverVerifyAllowListedPullPolicy(t *testing.T) { } } -func TestRemoveTagDigest(t *testing.T) { +func TestTrimImageTagDigest(t *testing.T) { tests := []struct { name string image string @@ -357,7 +357,7 @@ func TestRemoveTagDigest(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := removeTagDigest(tt.image) + got, err := trimImageTagDigest(tt.image) if (err != nil) != tt.wantErr { t.Fatalf("removeTagDigest(%q) error = %v, wantErr %v", tt.image, err, tt.wantErr) } diff --git a/pkg/kubelet/images/pullmanager/testdata/pulling/sha256-5f3ca9e4d82e32203c2fafb410dfae9c8047bc6ebe57d642fe264d479e152955 b/pkg/kubelet/images/pullmanager/testdata/pulling/sha256-5f3ca9e4d82e32203c2fafb410dfae9c8047bc6ebe57d642fe264d479e152955 new file mode 100644 index 00000000000..f79faafc4db --- /dev/null +++ b/pkg/kubelet/images/pullmanager/testdata/pulling/sha256-5f3ca9e4d82e32203c2fafb410dfae9c8047bc6ebe57d642fe264d479e152955 @@ -0,0 +1 @@ +{"kind":"ImagePullIntent","apiVersion":"kubelet.config.k8s.io/v1beta1","image":"secretorg/myimage:customtag"} diff --git a/pkg/kubelet/images/pullmanager/testdata/pulling/sha256-7ffc99ef20589dc844b202c4cba501d1aaf06d1481935aa1e160029afd639847 b/pkg/kubelet/images/pullmanager/testdata/pulling/sha256-7ffc99ef20589dc844b202c4cba501d1aaf06d1481935aa1e160029afd639847 new file mode 100644 index 00000000000..d03b469b77b --- /dev/null +++ b/pkg/kubelet/images/pullmanager/testdata/pulling/sha256-7ffc99ef20589dc844b202c4cba501d1aaf06d1481935aa1e160029afd639847 @@ -0,0 +1 @@ +{"kind":"ImagePullIntent","apiVersion":"kubelet.config.k8s.io/v1beta1","image":"testing@sha256:f24acc752be18b93b0504c86312bbaf482c9efb0c45e925bbccb0a591cebd7af"}