Merge pull request #133114 from Apptane/early-image-verification-policy

kubelet: invoke part of image verification policy earlier
This commit is contained in:
Kubernetes Prow Robot
2025-11-06 12:42:52 -08:00
committed by GitHub
8 changed files with 320 additions and 224 deletions

View File

@@ -199,48 +199,14 @@ func (m *imageManager) EnsureImageExists(ctx context.Context, objRef *v1.ObjectR
return "", message, err
}
if imageRef != "" && !utilfeature.DefaultFeatureGate.Enabled(features.KubeletEnsureSecretPulledImages) {
imagePullRequired = ptr.To(false)
msg := fmt.Sprintf("Container image %q already present on machine", requestedImage)
m.logIt(objRef, v1.EventTypeNormal, events.PulledImage, logPrefix, msg, klog.Info)
// wrap the lookup in a function to ensure that we only look up credentials once and no earlier than needed
lookupPullCredentials := m.makeLookupPullCredentialsFunc(spec.Image, pod, pullSecrets, podSandboxConfig)
// we need to stop recording if it is still in progress, as the image
// has already been pulled by another pod.
m.podPullingTimeRecorder.RecordImageFinishedPulling(pod.UID)
return imageRef, msg, nil
}
repoToPull, _, _, err := parsers.ParseImageName(spec.Image)
if err != nil {
return "", err.Error(), err
}
// construct the dynamic keyring using the providers we have in the kubelet
var podName, podNamespace, podUID string
if utilfeature.DefaultFeatureGate.Enabled(features.KubeletServiceAccountTokenForCredentialProviders) {
sandboxMetadata := podSandboxConfig.GetMetadata()
podName = sandboxMetadata.Name
podNamespace = sandboxMetadata.Namespace
podUID = sandboxMetadata.Uid
}
externalCredentialProviderKeyring := credentialproviderplugin.NewExternalCredentialProviderDockerKeyring(
podNamespace,
podName,
podUID,
pod.Spec.ServiceAccountName)
keyring, err := credentialprovidersecrets.MakeDockerKeyring(pullSecrets, credentialprovider.UnionDockerKeyring{m.nodeKeyring, externalCredentialProviderKeyring})
if err != nil {
return "", err.Error(), err
}
pullCredentials, _ := keyring.Lookup(repoToPull)
if imageRef != "" {
imagePullRequired = ptr.To(false) // should be overridden right after this if-block in case pull was required
getPodCredentials := func() ([]kubeletconfiginternal.ImagePullSecret, *kubeletconfiginternal.ImagePullServiceAccount, error) {
pullCredentials, err := lookupPullCredentials()
if err != nil {
return nil, nil, err
}
var imagePullSecrets []kubeletconfiginternal.ImagePullSecret
// we don't take the audience of the service account into account, so there can only
@@ -268,8 +234,26 @@ func (m *imageManager) EnsureImageExists(ctx context.Context, objRef *v1.ObjectR
}
}
pullRequired := m.imagePullManager.MustAttemptImagePull(ctx, requestedImage, imageRef, imagePullSecrets, imagePullServiceAccount)
if !pullRequired {
return imagePullSecrets, imagePullServiceAccount, nil
}
if imageRef != "" {
imagePullRequired = ptr.To(false) // should be overridden right after this if-block in case pull was required
if !utilfeature.DefaultFeatureGate.Enabled(features.KubeletEnsureSecretPulledImages) {
msg := fmt.Sprintf("Container image %q already present on machine", requestedImage)
m.logIt(objRef, v1.EventTypeNormal, events.PulledImage, logPrefix, msg, klog.Info)
// we need to stop recording if it is still in progress, as the image
// has already been pulled by another pod.
m.podPullingTimeRecorder.RecordImageFinishedPulling(pod.UID)
return imageRef, msg, nil
}
if pullRequired, err := m.imagePullManager.MustAttemptImagePull(ctx, requestedImage, imageRef, getPodCredentials); err != nil {
imagePullRequired = nil
return "", err.Error(), err
} else if !pullRequired {
msg := fmt.Sprintf("Container image %q already present on machine and can be accessed by the pod", requestedImage)
m.logIt(objRef, v1.EventTypeNormal, events.PulledImage, logPrefix, msg, klog.Info)
@@ -289,9 +273,47 @@ func (m *imageManager) EnsureImageExists(ctx context.Context, objRef *v1.ObjectR
return "", msg, err
}
pullCredentials, err := lookupPullCredentials()
if err != nil {
return "", err.Error(), err
}
return m.pullImage(ctx, logPrefix, objRef, pod.UID, requestedImage, spec, pullCredentials, podSandboxConfig)
}
func (m *imageManager) makeLookupPullCredentialsFunc(image string, pod *v1.Pod, pullSecrets []v1.Secret, podSandboxConfig *runtimeapi.PodSandboxConfig) func() ([]credentialprovider.TrackedAuthConfig, error) {
return sync.OnceValues(func() ([]credentialprovider.TrackedAuthConfig, error) {
repoToPull, _, _, err := parsers.ParseImageName(image)
if err != nil {
return nil, err
}
// construct the dynamic keyring using the providers we have in the kubelet
var podName, podNamespace, podUID string
if utilfeature.DefaultFeatureGate.Enabled(features.KubeletServiceAccountTokenForCredentialProviders) {
sandboxMetadata := podSandboxConfig.GetMetadata()
podName = sandboxMetadata.Name
podNamespace = sandboxMetadata.Namespace
podUID = sandboxMetadata.Uid
}
externalCredentialProviderKeyring := credentialproviderplugin.NewExternalCredentialProviderDockerKeyring(
podNamespace,
podName,
podUID,
pod.Spec.ServiceAccountName)
keyring, err := credentialprovidersecrets.MakeDockerKeyring(pullSecrets, credentialprovider.UnionDockerKeyring{m.nodeKeyring, externalCredentialProviderKeyring})
if err != nil {
return nil, err
}
pullCredentials, _ := keyring.Lookup(repoToPull)
return pullCredentials, nil
})
}
func (m *imageManager) pullImage(ctx context.Context, logPrefix string, objRef *v1.ObjectReference, podUID types.UID, image string, imgSpec kubecontainer.ImageSpec, pullCredentials []credentialprovider.TrackedAuthConfig, podSandboxConfig *runtimeapi.PodSandboxConfig) (imageRef, message string, err error) {
var pullSucceeded bool
var finalPullCredentials *credentialprovider.TrackedAuthConfig

View File

@@ -734,9 +734,14 @@ type mockImagePullManager struct {
config *mockImagePullManagerConfig
}
func (m *mockImagePullManager) MustAttemptImagePull(ctx context.Context, image, _ string, podSecrets []kubeletconfiginternal.ImagePullSecret, podServiceAccount *kubeletconfiginternal.ImagePullServiceAccount) bool {
func (m *mockImagePullManager) MustAttemptImagePull(ctx context.Context, image, _ string, getPodCredentials pullmanager.GetPodCredentials) (bool, error) {
if m.config == nil || m.config.allowAll {
return false
return false, nil
}
podSecrets, podServiceAccount, err := getPodCredentials()
if err != nil {
return true, err
}
// Check secrets
@@ -744,7 +749,7 @@ func (m *mockImagePullManager) MustAttemptImagePull(ctx context.Context, image,
for _, s := range podSecrets {
for _, allowed := range allowedSecrets {
if s.Namespace == allowed.Namespace && s.Name == allowed.Name && s.UID == allowed.UID {
return false
return false, nil
}
}
}
@@ -754,12 +759,12 @@ func (m *mockImagePullManager) MustAttemptImagePull(ctx context.Context, image,
if podServiceAccount != nil {
if allowedServiceAccounts, ok := m.config.allowedServiceAccounts[image]; ok {
if slices.Contains(allowedServiceAccounts, *podServiceAccount) {
return false
return false, nil
}
}
}
return true
return true, nil
}
// mockImagePullManagerWithTracking tracks calls to MustAttemptImagePull for service account testing
@@ -776,19 +781,25 @@ type mockImagePullManagerWithTracking struct {
recordedCredentials *kubeletconfiginternal.ImagePullCredentials
}
func (m *mockImagePullManagerWithTracking) MustAttemptImagePull(ctx context.Context, image, imageRef string, podSecrets []kubeletconfiginternal.ImagePullSecret, podServiceAccount *kubeletconfiginternal.ImagePullServiceAccount) bool {
func (m *mockImagePullManagerWithTracking) MustAttemptImagePull(ctx context.Context, image, imageRef string, getPodCredentials pullmanager.GetPodCredentials) (bool, error) {
m.mustAttemptCalled = true
m.lastImage = image
m.lastImageRef = imageRef
podSecrets, podServiceAccount, err := getPodCredentials()
if err != nil {
return true, err
}
m.lastSecrets = podSecrets
if podServiceAccount != nil {
m.lastServiceAccounts = []kubeletconfiginternal.ImagePullServiceAccount{*podServiceAccount}
}
if m.allowAll {
return false
return false, nil
}
return m.mustAttemptReturn
return m.mustAttemptReturn, nil
}
func (m *mockImagePullManagerWithTracking) RecordImagePulled(ctx context.Context, image, imageRef string, credentials *kubeletconfiginternal.ImagePullCredentials) {

View File

@@ -74,9 +74,11 @@ func directRecordReadFunc(expectHit bool) benchmarkedCheckFunc {
func mustAttemptPullReadFunc(expectHit bool) benchmarkedCheckFunc {
return func(b *testing.B, pullManager PullManager, imgRef string) {
tCtx := ktesting.Init(b)
mustPull := pullManager.MustAttemptImagePull(tCtx, "test.repo/org/"+imgRef, imgRef, nil, nil)
if mustPull != !expectHit {
b.Fatalf("MustAttemptImagePull() expected %t, got %t", !expectHit, mustPull)
mustPull, err := pullManager.MustAttemptImagePull(tCtx, "test.repo/org/"+imgRef, imgRef, func() ([]kubeletconfig.ImagePullSecret, *kubeletconfig.ImagePullServiceAccount, error) {
return nil, nil, nil
})
if mustPull != !expectHit || err != nil {
b.Fatalf("no error expected (got %v); MustAttemptImagePull() expected %t, got %t", err, !expectHit, mustPull)
}
}
}

View File

@@ -182,9 +182,9 @@ func (f *PullManager) decrementImagePullIntent(ctx context.Context, image string
f.decrementIntentCounterForImage(image)
}
func (f *PullManager) MustAttemptImagePull(ctx context.Context, image, imageRef string, podSecrets []kubeletconfiginternal.ImagePullSecret, podServiceAccount *kubeletconfiginternal.ImagePullServiceAccount) bool {
func (f *PullManager) MustAttemptImagePull(ctx context.Context, image, imageRef string, getPodCredentials GetPodCredentials) (bool, error) {
if len(imageRef) == 0 {
return true
return true, nil
}
logger := klog.FromContext(ctx)
@@ -238,42 +238,48 @@ func (f *PullManager) MustAttemptImagePull(ctx context.Context, image, imageRef
if err != nil {
resultForMetrics = checkResultError
logger.Error(err, "Unable to access cache records about image pulls")
return true
return true, nil
}
if !f.imagePolicyEnforcer.RequireCredentialVerificationForImage(image, imagePulledByKubelet) {
resultForMetrics = checkResultCredentialPolicyAllowed
return false
return false, nil
}
if pulledRecord == nil {
// we have no proper records of the image being pulled in the past, we can short-circuit here
resultForMetrics = checkResultMustAuthenticate
return true
return true, nil
}
sanitizedImage, err := trimImageTagDigest(image)
if err != nil {
resultForMetrics = checkResultError
logger.Error(err, "failed to parse image name, forcing image credentials reverification", "image", sanitizedImage)
return true
return true, nil
}
cachedCreds, ok := pulledRecord.CredentialMapping[sanitizedImage]
if !ok {
resultForMetrics = checkResultMustAuthenticate
return true
return true, nil
}
if cachedCreds.NodePodsAccessible {
// anyone on this node can access the image
resultForMetrics = checkResultCredentialRecordFound
return false
return false, nil
}
if len(cachedCreds.KubernetesSecrets) == 0 && len(cachedCreds.KubernetesServiceAccounts) == 0 {
resultForMetrics = checkResultMustAuthenticate
return true
return true, nil
}
podSecrets, podServiceAccount, err := getPodCredentials()
if err != nil {
resultForMetrics = checkResultError
return true, err
}
for _, podSecret := range podSecrets {
@@ -295,7 +301,7 @@ func (f *PullManager) MustAttemptImagePull(ctx context.Context, image, imageRef
}
}
resultForMetrics = checkResultCredentialRecordFound
return false
return false, nil
}
if secretCoordinatesMatch {
@@ -307,7 +313,7 @@ func (f *PullManager) MustAttemptImagePull(ctx context.Context, image, imageRef
logger.Error(err, "failed to write an image pulled record", "image", image, "imageRef", imageRef)
}
resultForMetrics = checkResultCredentialRecordFound
return false
return false, nil
}
}
}
@@ -316,11 +322,11 @@ func (f *PullManager) MustAttemptImagePull(ctx context.Context, image, imageRef
if podServiceAccount != nil && slices.Contains(cachedCreds.KubernetesServiceAccounts, *podServiceAccount) {
// we found a matching service account, no need to pull the image
resultForMetrics = checkResultCredentialRecordFound
return false
return false, nil
}
resultForMetrics = checkResultMustAuthenticate
return true
return true, nil
}
func (f *PullManager) PruneUnknownRecords(ctx context.Context, imageList []string, until time.Time) {

View File

@@ -425,17 +425,18 @@ func Test_pulledRecordMergeNewCreds(t *testing.T) {
func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
tests := []struct {
name string
imagePullPolicy ImagePullPolicyEnforcer
podSecrets []kubeletconfiginternal.ImagePullSecret
podServiceAccount *kubeletconfiginternal.ImagePullServiceAccount
image string
imageRef string
pulledFiles []string
pullingFiles []string
expectedPullRecord *kubeletconfiginternal.ImagePulledRecord
want bool
expectedCacheWrite bool
name string
imagePullPolicy ImagePullPolicyEnforcer
podSecrets []kubeletconfiginternal.ImagePullSecret
podServiceAccount *kubeletconfiginternal.ImagePullServiceAccount
image string
imageRef string
pulledFiles []string
pullingFiles []string
expectedPullRecord *kubeletconfiginternal.ImagePulledRecord
want bool
wantPodCredsRequested bool
expectedCacheWrite bool
}{
{
name: "image exists and is recorded with pod's exact secret",
@@ -445,18 +446,20 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "testsecrethash",
},
},
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: false,
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: false,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded, no pod secrets",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
name: "image exists and is recorded, no pod secrets",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded with the same secret but different credential hash",
@@ -479,8 +482,9 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
},
},
},
want: false,
expectedCacheWrite: true,
want: false,
wantPodCredsRequested: true,
expectedCacheWrite: true,
},
{
name: "image exists and is recorded with a different secret with a different UID",
@@ -490,10 +494,11 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "different uid", Namespace: "default", Name: "pull-secret", CredentialHash: "differenthash",
},
},
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded with a different secret",
@@ -503,10 +508,11 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "differentns", Name: "pull-secret", CredentialHash: "differenthash",
},
},
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded with a different secret with the same credential hash",
@@ -530,8 +536,9 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
},
},
},
want: false,
expectedCacheWrite: true,
want: false,
wantPodCredsRequested: true,
expectedCacheWrite: true,
},
{
name: "image exists but the pull is recorded with a different image name but with the exact same secret",
@@ -541,39 +548,44 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "testsecrethash",
},
},
image: "docker.io/testing/different:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
image: "docker.io/testing/different:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
wantPodCredsRequested: false,
},
{
name: "image exists and is recorded with empty credential mapping",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testemptycredmapping",
pulledFiles: []string{"sha256-f8778b6393eaf39315e767a58cbeacf2c4b270d94b4d6926ee993d9e49444991"},
want: true,
name: "image exists and is recorded with empty credential mapping",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testemptycredmapping",
pulledFiles: []string{"sha256-f8778b6393eaf39315e767a58cbeacf2c4b270d94b4d6926ee993d9e49444991"},
want: true,
wantPodCredsRequested: false,
},
{
name: "image does not exist and there are no records of it",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "",
want: true,
name: "image does not exist and there are no records of it",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "",
want: true,
wantPodCredsRequested: false,
},
{
name: "image exists and there are no records of it with NeverVerifyPreloadedImages pull policy",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
want: false,
name: "image exists and there are no records of it with NeverVerifyPreloadedImages pull policy",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
want: false,
wantPodCredsRequested: false,
},
{
name: "image exists and there are no records of it with AlwaysVerify pull policy",
imagePullPolicy: AlwaysVerifyImagePullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
want: true,
name: "image exists and there are no records of it with AlwaysVerify pull policy",
imagePullPolicy: AlwaysVerifyImagePullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
want: true,
wantPodCredsRequested: false,
},
{
name: "image exists but is only recorded via pulling intent",
@@ -583,10 +595,11 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "testsecrethash",
},
},
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: true,
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: true,
wantPodCredsRequested: false,
},
{
name: "image exists but is only recorded via pulling intent - NeverVerify policy",
@@ -596,18 +609,20 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "testsecrethash",
},
},
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: false,
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: false,
wantPodCredsRequested: false,
},
{
name: "image exists and is recorded as node-accessible, no pod secrets",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testimage-anonpull",
pulledFiles: []string{"sha256-a2eace2182b24cdbbb730798e47b10709b9ef5e0f0c1624a3bc06c8ca987727a"},
want: false,
name: "image exists and is recorded as node-accessible, no pod secrets",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testimage-anonpull",
pulledFiles: []string{"sha256-a2eace2182b24cdbbb730798e47b10709b9ef5e0f0c1624a3bc06c8ca987727a"},
want: false,
wantPodCredsRequested: false,
},
{
name: "image exists and is recorded as node-accessible, request with pod secrets",
@@ -617,10 +632,11 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "testsecrethash",
},
},
image: "docker.io/testing/test:latest",
imageRef: "testimage-anonpull",
pulledFiles: []string{"sha256-a2eace2182b24cdbbb730798e47b10709b9ef5e0f0c1624a3bc06c8ca987727a"},
want: false,
image: "docker.io/testing/test:latest",
imageRef: "testimage-anonpull",
pulledFiles: []string{"sha256-a2eace2182b24cdbbb730798e47b10709b9ef5e0f0c1624a3bc06c8ca987727a"},
want: false,
wantPodCredsRequested: false,
},
{
name: "image exists and is recorded with empty hash as its hashing originally failed, the same fail for a different pod secret",
@@ -630,10 +646,11 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "differentns", Name: "pull-secret", CredentialHash: "",
},
},
image: "docker.io/testing/test:latest",
imageRef: "test-brokenhash",
pulledFiles: []string{"sha256-38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd"},
want: true,
image: "docker.io/testing/test:latest",
imageRef: "test-brokenhash",
pulledFiles: []string{"sha256-38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd"},
want: true,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded with empty hash as its hashing originally failed, the same fail for the same pod secret",
@@ -643,82 +660,91 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "",
},
},
image: "docker.io/testing/test:latest",
imageRef: "test-brokenhash",
pulledFiles: []string{"sha256-38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd"},
want: false,
image: "docker.io/testing/test:latest",
imageRef: "test-brokenhash",
pulledFiles: []string{"sha256-38a8906435c4dd5f4258899d46621bfd8eea3ad6ff494ee3c2f17ef0321625bd"},
want: false,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded with pod's exact service account",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: false,
name: "image exists and is recorded with pod's exact service account",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: false,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded, no pod service accounts",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
name: "image exists and is recorded, no pod service accounts",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
image: "docker.io/testing/test:latest",
imageRef: "testimageref",
pulledFiles: []string{"sha256-b3c0cc4278800b03a308ceb2611161430df571ca733122f0a40ac8b9792a9064"},
want: true,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded with a different service account with different UID",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "different-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: true,
name: "image exists and is recorded with a different service account with different UID",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "different-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: true,
wantPodCredsRequested: true,
},
{
name: "image exists and is recorded with a different service account with different namespace",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "different-ns", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: true,
name: "image exists and is recorded with a different service account with different namespace",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "different-ns", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: true,
wantPodCredsRequested: true,
},
{
name: "image exists but the pull is recorded with a different image name but with the exact same service account",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/different:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: true,
name: "image exists but the pull is recorded with a different image name but with the exact same service account",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/different:latest",
imageRef: "testimageref-sa",
pulledFiles: []string{"sha256-917e8b3439bf8a7a6f37ffd2d2ddfdfafac8a251bf214a0be39675742b420b1a"},
want: true,
wantPodCredsRequested: false,
},
{
name: "image exists but is only recorded via pulling intent with service account",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: true,
name: "image exists but is only recorded via pulling intent with service account",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: true,
wantPodCredsRequested: false,
},
{
name: "image exists but is only recorded via pulling intent with service account - NeverVerify policy",
imagePullPolicy: NeverVerifyImagePullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: false,
name: "image exists but is only recorded via pulling intent with service account - NeverVerify policy",
imagePullPolicy: NeverVerifyImagePullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testexistingref",
pullingFiles: []string{"sha256-aef2af226629a35d5f3ef0fdbb29fdbebf038d0acd8850590e8c48e1e283aa56"},
want: false,
wantPodCredsRequested: false,
},
{
name: "image exists and is recorded as node-accessible, request with pod service accounts",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimage-anonpull",
pulledFiles: []string{"sha256-a2eace2182b24cdbbb730798e47b10709b9ef5e0f0c1624a3bc06c8ca987727a"},
want: false,
name: "image exists and is recorded as node-accessible, request with pod service accounts",
imagePullPolicy: NeverVerifyPreloadedPullPolicy(),
podServiceAccount: &kubeletconfiginternal.ImagePullServiceAccount{UID: "test-sa-uid", Namespace: "default", Name: "test-sa"},
image: "docker.io/testing/test:latest",
imageRef: "testimage-anonpull",
pulledFiles: []string{"sha256-a2eace2182b24cdbbb730798e47b10709b9ef5e0f0c1624a3bc06c8ca987727a"},
want: false,
wantPodCredsRequested: false,
},
}
@@ -744,6 +770,11 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
},
}
var podCredsRequested bool
s := func() ([]kubeletconfiginternal.ImagePullSecret, *kubeletconfiginternal.ImagePullServiceAccount, error) {
podCredsRequested = true
return tt.podSecrets, tt.podServiceAccount, nil
}
f := &PullManager{
recordsAccessor: fsRecordAccessor,
imagePolicyEnforcer: tt.imagePullPolicy,
@@ -752,8 +783,14 @@ func TestFileBasedImagePullManager_MustAttemptImagePull(t *testing.T) {
pulledAccessors: NewStripedLockSet(10),
}
if got := f.MustAttemptImagePull(tCtx, tt.image, tt.imageRef, tt.podSecrets, tt.podServiceAccount); got != tt.want {
if got, err := f.MustAttemptImagePull(tCtx, tt.image, tt.imageRef, s); err != nil {
t.Errorf("FileBasedImagePullManager.MustAttemptImagePull() unexpected error %v", err)
} else if got != tt.want {
t.Errorf("FileBasedImagePullManager.MustAttemptImagePull() = %v, want %v", got, tt.want)
} else if podCredsRequested && !tt.wantPodCredsRequested {
t.Error("expected FileBasedImagePullManager.MustAttemptImagePull() to not look up pod credentials, but it did")
} else if !podCredsRequested && tt.wantPodCredsRequested {
t.Error("expected FileBasedImagePullManager.MustAttemptImagePull() to look up pod credentials, but it did not")
}
if tt.expectedCacheWrite != (fsRecordAccessor.imagePulledRecordsWrites != 0) {

View File

@@ -23,6 +23,9 @@ import (
kubeletconfiginternal "k8s.io/kubernetes/pkg/kubelet/apis/config"
)
// GetPodCredentials lazily looks up and returns a set of pull credentials.
type GetPodCredentials func() ([]kubeletconfiginternal.ImagePullSecret, *kubeletconfiginternal.ImagePullServiceAccount, error)
// ImagePullManager keeps the state of images that were pulled and which are
// currently still being pulled.
// It should keep an internal state of images currently being pulled by the kubelet
@@ -59,11 +62,16 @@ type ImagePullManager interface {
// set of credentials or if the image can be accessed by any of the node's pods
// or if the image can be accessed by the specified service account.
//
// Returns true if the policy demands verification and no record of the pull
// `getPodCredentials` is invoked to retrieve the set of credentials after
// MustAttemptImagePull evaluates the parts of the policy that do not depend on them.
//
// Returns true and an error if `getPodCredentials` returns an error.
//
// Returns true and nil if the policy demands verification and no record of the pull
// was found in the cache.
//
// `image` is the content of the pod's container `image` field.
MustAttemptImagePull(ctx context.Context, image, imageRef string, credentials []kubeletconfiginternal.ImagePullSecret, serviceAccount *kubeletconfiginternal.ImagePullServiceAccount) bool
MustAttemptImagePull(ctx context.Context, image, imageRef string, getPodCredentials GetPodCredentials) (bool, error)
// PruneUnknownRecords deletes all of the cache ImagePulledRecords for each of the images
// whose imageRef does not appear in the `imageList` iff such an record was last updated
// _before_ the `until` timestamp.

View File

@@ -17,6 +17,7 @@ limitations under the License.
package pullmanager
import (
"context"
"fmt"
"path/filepath"
"strings"
@@ -200,43 +201,52 @@ func TestMustAttemptPullMetrics(t *testing.T) {
})
expectedMetrics := make(map[string]int)
require.True(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/broken", "testbrokenrecord", nil, nil))
testMustAttemptImagePull(t, true, imageManager, ctx, "docker.io/testing/broken", "testbrokenrecord", nil)
expectedMetrics[string(checkResultError)]++
cmpMustAttemptPullMetrics(t, expectedMetrics)
require.True(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/broken", "testbrokenrecord", nil, nil))
require.True(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/broken", "testbrokenrecord", nil, nil))
testMustAttemptImagePull(t, true, imageManager, ctx, "docker.io/testing/broken", "testbrokenrecord", nil)
testMustAttemptImagePull(t, true, imageManager, ctx, "docker.io/testing/broken", "testbrokenrecord", nil)
expectedMetrics[string(checkResultError)] += 2
require.False(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/test", "testimage-anonpull", nil, nil))
testMustAttemptImagePull(t, false, imageManager, ctx, "docker.io/testing/test", "testimage-anonpull", nil)
expectedMetrics[string(checkResultCredentialRecordFound)]++
cmpMustAttemptPullMetrics(t, expectedMetrics)
require.False(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/test", "testimage-anonpull", nil, nil))
require.False(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/test", "testimage-anonpull", nil, nil))
require.False(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/test", "testimage-anonpull", nil, nil))
testMustAttemptImagePull(t, false, imageManager, ctx, "docker.io/testing/test", "testimage-anonpull", nil)
testMustAttemptImagePull(t, false, imageManager, ctx, "docker.io/testing/test", "testimage-anonpull", nil)
testMustAttemptImagePull(t, false, imageManager, ctx, "docker.io/testing/test", "testimage-anonpull", nil)
expectedMetrics[string(checkResultCredentialRecordFound)] += 3
cmpMustAttemptPullMetrics(t, expectedMetrics)
require.False(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/policyexempt", "policyallowed", nil, nil))
testMustAttemptImagePull(t, false, imageManager, ctx, "docker.io/testing/policyexempt", "policyallowed", nil)
expectedMetrics[string(checkResultCredentialPolicyAllowed)]++
cmpMustAttemptPullMetrics(t, expectedMetrics)
require.False(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/policyexempt", "policyallowed", nil, nil))
testMustAttemptImagePull(t, false, imageManager, ctx, "docker.io/testing/policyexempt", "policyallowed", nil)
expectedMetrics[string(checkResultCredentialPolicyAllowed)]++
cmpMustAttemptPullMetrics(t, expectedMetrics)
require.True(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/norecords", "somewhatunknown", nil, nil))
testMustAttemptImagePull(t, true, imageManager, ctx, "docker.io/testing/norecords", "somewhatunknown", nil)
expectedMetrics[string(checkResultMustAuthenticate)]++
cmpMustAttemptPullMetrics(t, expectedMetrics)
require.False(t, imageManager.MustAttemptImagePull(ctx, "docker.io/testing/test", "testimageref", []kubeletconfig.ImagePullSecret{
{UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "testsecrethash"},
}, nil))
testMustAttemptImagePull(t, false, imageManager, ctx, "docker.io/testing/test", "testimageref",
func() ([]kubeletconfig.ImagePullSecret, *kubeletconfig.ImagePullServiceAccount, error) {
return []kubeletconfig.ImagePullSecret{
{UID: "testsecretuid", Namespace: "default", Name: "pull-secret", CredentialHash: "testsecrethash"},
}, nil, nil
})
expectedMetrics[string(checkResultCredentialRecordFound)]++
cmpMustAttemptPullMetrics(t, expectedMetrics)
}
func testMustAttemptImagePull(t *testing.T, expected bool, f *PullManager, ctx context.Context, image, imageRef string, getPodCredentials GetPodCredentials) {
mustPull, err := f.MustAttemptImagePull(ctx, image, imageRef, getPodCredentials)
require.NoError(t, err)
require.Equal(t, expected, mustPull)
}
func cmpFSIntents(t *testing.T, expected uint) {
t.Helper()
const metricFormat = `

View File

@@ -31,7 +31,7 @@ func (m *NoopImagePullManager) RecordPullIntent(string) error { return nil }
func (m *NoopImagePullManager) RecordImagePulled(context.Context, string, string, *kubeletconfiginternal.ImagePullCredentials) {
}
func (m *NoopImagePullManager) RecordImagePullFailed(context.Context, string) {}
func (m *NoopImagePullManager) MustAttemptImagePull(context.Context, string, string, []kubeletconfiginternal.ImagePullSecret, *kubeletconfiginternal.ImagePullServiceAccount) bool {
return false
func (m *NoopImagePullManager) MustAttemptImagePull(context.Context, string, string, GetPodCredentials) (bool, error) {
return false, nil
}
func (m *NoopImagePullManager) PruneUnknownRecords(context.Context, []string, time.Time) {}