Merge pull request #137629 from stlaz/ensure-secret-images-allowlist-fix

Image Pulling Authorization: fix the allowlisting policy

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).

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 {

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 := trimImageTagDigest(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 := trimImageTagDigest(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,36 @@ func getAllowlistImagePattern(pattern string) (string, bool, error) {
 
 	return trimmedPattern, true, nil
 }
+
+// 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
+	}
+	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
+}

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 TestTrimImageTagDigest(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 := trimImageTagDigest(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)
+			}
+		})
+	}
+}

pkg/kubelet/images/pullmanager/testdata/pulling/sha256-5f3ca9e4d82e32203c2fafb410dfae9c8047bc6ebe57d642fe264d479e152955

@@ -0,0 +1 @@
+{"kind":"ImagePullIntent","apiVersion":"kubelet.config.k8s.io/v1beta1","image":"secretorg/myimage:customtag"}

pkg/kubelet/images/pullmanager/testdata/pulling/sha256-7ffc99ef20589dc844b202c4cba501d1aaf06d1481935aa1e160029afd639847

@@ -0,0 +1 @@
+{"kind":"ImagePullIntent","apiVersion":"kubelet.config.k8s.io/v1beta1","image":"testing@sha256:f24acc752be18b93b0504c86312bbaf482c9efb0c45e925bbccb0a591cebd7af"}

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(ctx)
 			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()
 })

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{})