Merge pull request #130763 from aramase/aramase/t/kep_4412_alpha_plugin_unit_tests

Add unit tests for credential provider in service account mode

pkg/credentialprovider/plugin/plugin.go

@@ -73,7 +73,7 @@ var (
 	}
 )
 
-// GetServiceAccountFunc is a function type that returns a service account token for the given namespace and name.
+// GetServiceAccountFunc is a function type that returns a service account for the given namespace and name.
 type GetServiceAccountFunc func(namespace, name string) (*v1.ServiceAccount, error)
 
 // getServiceAccountTokenFunc is a function type that returns a service account token for the given namespace and name.
@@ -240,6 +240,11 @@ func (e requiredAnnotationNotFoundError) Error() string {
 	return fmt.Sprintf("required annotation %s not found", string(e))
 }
 
+func isRequiredAnnotationNotFoundError(err error) bool {
+	var requiredAnnotationNotFoundErr requiredAnnotationNotFoundError
+	return errors.As(err, &requiredAnnotationNotFoundErr)
+}
+
 // getServiceAccountData returns the service account UID and required annotations for the service account.
 // If the service account does not exist, an error is returned.
 // saAnnotations is a map of annotation keys and values that the plugin requires to generate credentials
@@ -370,8 +375,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
 		// to pull images for pods without service accounts (e.g., static pods).
 		if len(serviceAccountName) > 0 {
 			if serviceAccountUID, saAnnotations, err = p.serviceAccountProvider.getServiceAccountData(podNamespace, serviceAccountName); err != nil {
-				var requiredAnnotationNotFoundErr requiredAnnotationNotFoundError
+				if isRequiredAnnotationNotFoundError(err) {
-				if errors.As(err, &requiredAnnotationNotFoundErr) {
 					// The required annotation could be a mechanism for individual workloads to opt in to using service account tokens
 					// for image pull. If any of the required annotation is missing, we will not invoke the plugin. We will log the error
 					// at higher verbosity level as it could be noisy.

pkg/credentialprovider/plugin/plugin_test.go

@@ -19,8 +19,11 @@ package plugin
 import (
 	"bytes"
 	"context"
+	"errors"
+	"flag"
 	"fmt"
 	"reflect"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -35,6 +38,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/klog/v2"
 	credentialproviderapi "k8s.io/kubelet/pkg/apis/credentialprovider"
 	credentialproviderv1 "k8s.io/kubelet/pkg/apis/credentialprovider/v1"
 	credentialproviderv1alpha1 "k8s.io/kubelet/pkg/apis/credentialprovider/v1alpha1"
@@ -188,12 +192,19 @@ func TestSingleflightProvide(t *testing.T) {
 }
 
 func Test_Provide(t *testing.T) {
+	klog.InitFlags(nil)
+	if err := flag.Set("v", "6"); err != nil {
+		t.Fatalf("failed to set log level: %v", err)
+	}
+	flag.Parse()
+
 	tclock := clock.RealClock{}
 	testcases := []struct {
 		name           string
 		pluginProvider *perPodPluginProvider
 		image          string
 		dockerconfig   credentialprovider.DockerConfig
+		wantLog        string
 	}{
 		{
 			name: "exact image match, with Registry cache key",
@@ -357,17 +368,162 @@ func Test_Provide(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "[service account mode] sa name required but empty",
+			pluginProvider: &perPodPluginProvider{
+				provider: &pluginProvider{
+					matchImages: []string{"test.registry.io"},
+					serviceAccountProvider: &serviceAccountProvider{
+						requireServiceAccount: true,
+					},
+				},
+				podName:      "pod-name",
+				podNamespace: "ns",
+			},
+			image:        "test.registry.io/foo/bar",
+			dockerconfig: credentialprovider.DockerConfig{},
+			wantLog:      "Service account name is empty for pod ns/pod-name",
+		},
+		{
+			name: "[service account mode] sa does not have required annotations",
+			pluginProvider: &perPodPluginProvider{
+				provider: &pluginProvider{
+					matchImages: []string{"test.registry.io"},
+					serviceAccountProvider: &serviceAccountProvider{
+						requireServiceAccount: true,
+						requiredServiceAccountAnnotationKeys: []string{
+							"domain.io/identity-id",
+						},
+						getServiceAccountFunc: func(_, _ string) (*v1.ServiceAccount, error) {
+							return &v1.ServiceAccount{
+								ObjectMeta: metav1.ObjectMeta{
+									Name:        "sa-name",
+									Namespace:   "ns",
+									Annotations: map[string]string{},
+								},
+							}, nil
+						},
+					},
+				},
+				podName:            "pod-name",
+				podNamespace:       "ns",
+				serviceAccountName: "sa-name",
+			},
+			image:        "test.registry.io/foo/bar",
+			dockerconfig: credentialprovider.DockerConfig{},
+			wantLog:      "Failed to get service account data ns/sa-name: required annotation domain.io/identity-id not found",
+		},
+		{
+			name: "[service account mode] failed to get service account token",
+			pluginProvider: &perPodPluginProvider{
+				provider: &pluginProvider{
+					matchImages: []string{"test.registry.io"},
+					serviceAccountProvider: &serviceAccountProvider{
+						requireServiceAccount: true,
+						requiredServiceAccountAnnotationKeys: []string{
+							"domain.io/identity-id",
+						},
+						optionalServiceAccountAnnotationKeys: []string{
+							"domain.io/identity-type",
+						},
+						getServiceAccountFunc: func(_, _ string) (*v1.ServiceAccount, error) {
+							return &v1.ServiceAccount{
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "sa-name",
+									Namespace: "ns",
+									Annotations: map[string]string{
+										"domain.io/identity-id": "id",
+									},
+								},
+							}, nil
+						},
+						getServiceAccountTokenFunc: func(_, _ string, tr *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
+							return nil, errors.New("failed to get token")
+						},
+					},
+				},
+				podName:            "pod-name",
+				podNamespace:       "ns",
+				serviceAccountName: "sa-name",
+			},
+			image:        "test.registry.io/foo/bar",
+			dockerconfig: credentialprovider.DockerConfig{},
+			wantLog:      "Error getting service account token ns/sa-name: failed to get token",
+		},
+		{
+			name: "[service account mode] exact image match",
+			pluginProvider: &perPodPluginProvider{
+				provider: &pluginProvider{
+					clock:          tclock,
+					lastCachePurge: tclock.Now(),
+					matchImages:    []string{"test.registry.io"},
+					cache:          cache.NewExpirationStore(cacheKeyFunc, &cacheExpirationPolicy{clock: tclock}),
+					plugin: &fakeExecPlugin{
+						cacheKeyType: credentialproviderapi.ImagePluginCacheKeyType,
+						auth: map[string]credentialproviderapi.AuthConfig{
+							"test.registry.io/foo/bar": {
+								Username: "user",
+								Password: "password",
+							},
+						},
+					},
+					serviceAccountProvider: &serviceAccountProvider{
+						requireServiceAccount: true,
+						requiredServiceAccountAnnotationKeys: []string{
+							"domain.io/identity-id",
+						},
+						optionalServiceAccountAnnotationKeys: []string{
+							"domain.io/identity-type",
+						},
+						getServiceAccountFunc: func(_, _ string) (*v1.ServiceAccount, error) {
+							return &v1.ServiceAccount{
+								ObjectMeta: metav1.ObjectMeta{
+									Name:      "sa-name",
+									Namespace: "ns",
+									Annotations: map[string]string{
+										"domain.io/identity-id":   "id",
+										"domain.io/identity-type": "type",
+									},
+								},
+							}, nil
+						},
+						getServiceAccountTokenFunc: func(_, _ string, tr *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
+							return &authenticationv1.TokenRequest{Status: authenticationv1.TokenRequestStatus{Token: "token"}}, nil
+						},
+					},
+				},
+				podName:            "pod-name",
+				podNamespace:       "ns",
+				serviceAccountName: "sa-name",
+			},
+			image: "test.registry.io/foo/bar",
+			dockerconfig: credentialprovider.DockerConfig{
+				"test.registry.io/foo/bar": credentialprovider.DockerConfigEntry{
+					Username: "user",
+					Password: "password",
+				},
+			},
+		},
 	}
 
 	for _, testcase := range testcases {
 		testcase := testcase
 		t.Run(testcase.name, func(t *testing.T) {
-			t.Parallel()
+			var buf bytes.Buffer
-			dockerconfig := testcase.pluginProvider.Provide(testcase.image)
+			klog.SetOutput(&buf)
-			if !reflect.DeepEqual(dockerconfig, testcase.dockerconfig) {
+			klog.LogToStderr(false)
-				t.Logf("actual docker config: %v", dockerconfig)
+			defer klog.LogToStderr(true)
-				t.Logf("expected docker config: %v", testcase.dockerconfig)
+
-				t.Error("unexpected docker config")
+			if got := testcase.pluginProvider.Provide(testcase.image); !reflect.DeepEqual(got, testcase.dockerconfig) {
+				t.Errorf("unexpected docker config: %v, expected: %v", got, testcase.dockerconfig)
+			}
+
+			klog.Flush()
+			klog.SetOutput(&bytes.Buffer{}) // prevent further writes into buf
+			capturedOutput := buf.String()
+
+			if len(testcase.wantLog) > 0 && !strings.Contains(capturedOutput, testcase.wantLog) {
+				t.Errorf("expected log message %q not found in captured output: %q", testcase.wantLog, capturedOutput)
 			}
 		})
 	}
@@ -1568,3 +1724,30 @@ func TestGenerateCacheKey(t *testing.T) {
 		})
 	}
 }
+
+func TestRequiredAnnotationNotFoundErr(t *testing.T) {
+	tests := []struct {
+		name     string
+		err      error
+		expected bool
+	}{
+		{
+			name:     "required annotation not found error",
+			err:      requiredAnnotationNotFoundError("something"),
+			expected: true,
+		},
+		{
+			name:     "other error",
+			err:      errors.New("some other error"),
+			expected: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if got := isRequiredAnnotationNotFoundError(test.err); got != test.expected {
+				t.Errorf("expected %v, got %v", test.expected, got)
+			}
+		})
+	}
+}