Merge pull request #114373 from TommyStarK/unit-tests/kubelet-kuberuntime

kubelet/kuberuntime: Improving test coverage

pkg/kubelet/kuberuntime/helpers_test.go

@@ -148,6 +148,12 @@ func TestToKubeContainer(t *testing.T) {
 	got, err := m.toKubeContainer(c)
 	assert.NoError(t, err)
 	assert.Equal(t, expect, got)
+
+	// unable to convert a nil pointer to a runtime container
+	_, err = m.toKubeContainer(nil)
+	assert.Error(t, err)
+	_, err = m.sandboxToKubeContainer(nil)
+	assert.Error(t, err)
 }
 
 func TestGetImageUser(t *testing.T) {
@@ -233,3 +239,78 @@ func TestGetImageUser(t *testing.T) {
 		assert.Equal(t, test.expectedImageUserValues.username, username, "TestCase[%d]", j)
 	}
 }
+
+func TestToRuntimeProtocol(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		protocol string
+		expected runtimeapi.Protocol
+	}{
+		{
+			name:     "TCP protocol",
+			protocol: "TCP",
+			expected: runtimeapi.Protocol_TCP,
+		},
+		{
+			name:     "UDP protocol",
+			protocol: "UDP",
+			expected: runtimeapi.Protocol_UDP,
+		},
+		{
+			name:     "SCTP protocol",
+			protocol: "SCTP",
+			expected: runtimeapi.Protocol_SCTP,
+		},
+		{
+			name:     "unknown protocol",
+			protocol: "unknown",
+			expected: runtimeapi.Protocol_TCP,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			if result := toRuntimeProtocol(v1.Protocol(test.protocol)); result != test.expected {
+				t.Errorf("expected %d but got %d", test.expected, result)
+			}
+		})
+	}
+}
+
+func TestToKubeContainerState(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		state    int32
+		expected kubecontainer.State
+	}{
+		{
+			name:     "container created",
+			state:    0,
+			expected: kubecontainer.ContainerStateCreated,
+		},
+		{
+			name:     "container running",
+			state:    1,
+			expected: kubecontainer.ContainerStateRunning,
+		},
+		{
+			name:     "container exited",
+			state:    2,
+			expected: kubecontainer.ContainerStateExited,
+		},
+		{
+			name:     "unknown state",
+			state:    3,
+			expected: kubecontainer.ContainerStateUnknown,
+		},
+		{
+			name:     "not supported state",
+			state:    4,
+			expected: kubecontainer.ContainerStateUnknown,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			if result := toKubeContainerState(runtimeapi.ContainerState(test.state)); result != test.expected {
+				t.Errorf("expected %s but got %s", test.expected, result)
+			}
+		})
+	}
+}

pkg/kubelet/kuberuntime/kuberuntime_image_test.go

@@ -52,8 +52,13 @@ func TestPullImageWithError(t *testing.T) {
 	_, fakeImageService, fakeManager, err := createTestRuntimeManager()
 	assert.NoError(t, err)
 
+	// trying to pull an image with an invalid name should return an error
+	imageRef, err := fakeManager.PullImage(ctx, kubecontainer.ImageSpec{Image: ":invalid"}, nil, nil)
+	assert.Error(t, err)
+	assert.Equal(t, "", imageRef)
+
 	fakeImageService.InjectError("PullImage", fmt.Errorf("test-error"))
-	imageRef, err := fakeManager.PullImage(ctx, kubecontainer.ImageSpec{Image: "busybox"}, nil, nil)
+	imageRef, err = fakeManager.PullImage(ctx, kubecontainer.ImageSpec{Image: "busybox"}, nil, nil)
 	assert.Error(t, err)
 	assert.Equal(t, "", imageRef)
 
@@ -272,6 +277,62 @@ func TestPullWithSecrets(t *testing.T) {
 	}
 }
 
+func TestPullWithSecretsWithError(t *testing.T) {
+	ctx := context.Background()
+
+	dockerCfg := map[string]map[string]map[string]string{
+		"auths": {
+			"index.docker.io/v1/": {
+				"email": "passed-email",
+				"auth":  "cGFzc2VkLXVzZXI6cGFzc2VkLXBhc3N3b3Jk",
+			},
+		},
+	}
+
+	dockerConfigJSON, err := json.Marshal(dockerCfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, test := range []struct {
+		name              string
+		imageName         string
+		passedSecrets     []v1.Secret
+		shouldInjectError bool
+	}{
+		{
+			name:          "invalid docker secret",
+			imageName:     "ubuntu",
+			passedSecrets: []v1.Secret{{Type: v1.SecretTypeDockercfg, Data: map[string][]byte{v1.DockerConfigKey: []byte("invalid")}}},
+		},
+		{
+			name:      "secret provided, pull failed",
+			imageName: "ubuntu",
+			passedSecrets: []v1.Secret{
+				{Type: v1.SecretTypeDockerConfigJson, Data: map[string][]byte{v1.DockerConfigKey: dockerConfigJSON}},
+			},
+			shouldInjectError: true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			_, fakeImageService, fakeManager, err := createTestRuntimeManager()
+			assert.NoError(t, err)
+
+			if test.shouldInjectError {
+				fakeImageService.InjectError("PullImage", fmt.Errorf("test-error"))
+			}
+
+			imageRef, err := fakeManager.PullImage(ctx, kubecontainer.ImageSpec{Image: test.imageName}, test.passedSecrets, nil)
+			assert.Error(t, err)
+			assert.Equal(t, "", imageRef)
+
+			images, err := fakeManager.ListImages(ctx)
+			assert.NoError(t, err)
+			assert.Equal(t, 0, len(images))
+		})
+	}
+}
+
 func TestPullThenListWithAnnotations(t *testing.T) {
 	ctx := context.Background()
 	_, _, fakeManager, err := createTestRuntimeManager()

pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go

@@ -37,6 +37,49 @@ import (
 	"k8s.io/utils/pointer"
 )
 
+func TestGeneratePodSandboxConfig(t *testing.T) {
+	_, _, m, err := createTestRuntimeManager()
+	require.NoError(t, err)
+	pod := newTestPod()
+
+	expectedLogDirectory := filepath.Join(podLogsRootDirectory, pod.Namespace+"_"+pod.Name+"_12345678")
+	expectedLabels := map[string]string{
+		"io.kubernetes.pod.name":      pod.Name,
+		"io.kubernetes.pod.namespace": pod.Namespace,
+		"io.kubernetes.pod.uid":       string(pod.UID),
+	}
+	expectedLinuxPodSandboxConfig := &runtimeapi.LinuxPodSandboxConfig{
+		SecurityContext: &runtimeapi.LinuxSandboxSecurityContext{
+			SelinuxOptions: &runtimeapi.SELinuxOption{
+				User: "qux",
+			},
+			RunAsUser:  &runtimeapi.Int64Value{Value: 1000},
+			RunAsGroup: &runtimeapi.Int64Value{Value: 10},
+		},
+	}
+	expectedMetadata := &runtimeapi.PodSandboxMetadata{
+		Name:      pod.Name,
+		Namespace: pod.Namespace,
+		Uid:       string(pod.UID),
+		Attempt:   uint32(1),
+	}
+	expectedPortMappings := []*runtimeapi.PortMapping{
+		{
+			HostPort: 8080,
+		},
+	}
+
+	podSandboxConfig, err := m.generatePodSandboxConfig(pod, 1)
+	assert.NoError(t, err)
+	assert.Equal(t, expectedLabels, podSandboxConfig.Labels)
+	assert.Equal(t, expectedLogDirectory, podSandboxConfig.LogDirectory)
+	assert.Equal(t, expectedMetadata, podSandboxConfig.Metadata)
+	assert.Equal(t, expectedPortMappings, podSandboxConfig.PortMappings)
+	assert.Equal(t, expectedLinuxPodSandboxConfig.SecurityContext.SelinuxOptions, podSandboxConfig.Linux.SecurityContext.SelinuxOptions)
+	assert.Equal(t, expectedLinuxPodSandboxConfig.SecurityContext.RunAsUser, podSandboxConfig.Linux.SecurityContext.RunAsUser)
+	assert.Equal(t, expectedLinuxPodSandboxConfig.SecurityContext.RunAsGroup, podSandboxConfig.Linux.SecurityContext.RunAsGroup)
+}
+
 // TestCreatePodSandbox tests creating sandbox and its corresponding pod log directory.
 func TestCreatePodSandbox(t *testing.T) {
 	ctx := context.Background()
@@ -57,7 +100,8 @@ func TestCreatePodSandbox(t *testing.T) {
 	sandboxes, err := fakeRuntime.ListPodSandbox(ctx, &runtimeapi.PodSandboxFilter{Id: id})
 	assert.NoError(t, err)
 	assert.Equal(t, len(sandboxes), 1)
-	// TODO Check pod sandbox configuration
+	assert.Equal(t, sandboxes[0].Id, fmt.Sprintf("%s_%s_%s_1", pod.Name, pod.Namespace, pod.UID))
+	assert.Equal(t, sandboxes[0].State, runtimeapi.PodSandboxState_SANDBOX_READY)
 }
 
 func TestGeneratePodSandboxLinuxConfigSeccomp(t *testing.T) {
@@ -141,6 +185,8 @@ func TestCreatePodSandbox_RuntimeClass(t *testing.T) {
 }
 
 func newTestPod() *v1.Pod {
+	anyGroup := int64(10)
+	anyUser := int64(1000)
 	return &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			UID:       "12345678",
@@ -148,11 +194,23 @@ func newTestPod() *v1.Pod {
 			Namespace: "new",
 		},
 		Spec: v1.PodSpec{
+			SecurityContext: &v1.PodSecurityContext{
+				SELinuxOptions: &v1.SELinuxOptions{
+					User: "qux",
+				},
+				RunAsUser:  &anyUser,
+				RunAsGroup: &anyGroup,
+			},
 			Containers: []v1.Container{
 				{
 					Name:            "foo",
 					Image:           "busybox",
 					ImagePullPolicy: v1.PullIfNotPresent,
+					Ports: []v1.ContainerPort{
+						{
+							HostPort: 8080,
+						},
+					},
 				},
 			},
 		},

pkg/kubelet/kuberuntime/legacy_test.go

@@ -56,3 +56,52 @@ func TestLegacyLogSymLink(t *testing.T) {
 	expectedPath := filepath.Join(legacyContainerLogsDir, fmt.Sprintf("%s_%s_%s-%s", podName, podNamespace, containerName, containerID)[:251]+".log")
 	as.Equal(expectedPath, legacyLogSymlink(containerID, containerName, podName, podNamespace))
 }
+
+func TestGetContainerIDFromLegacyLogSymLink(t *testing.T) {
+	containerID := randStringBytes(80)
+	containerName := randStringBytes(70)
+	podName := randStringBytes(128)
+	podNamespace := randStringBytes(10)
+
+	for _, test := range []struct {
+		name        string
+		logSymLink  string
+		expected    string
+		shouldError bool
+	}{
+		{
+			name:        "unable to find separator",
+			logSymLink:  "dummy.log",
+			expected:    "",
+			shouldError: true,
+		},
+		{
+			name:        "invalid suffix",
+			logSymLink:  filepath.Join(legacyContainerLogsDir, fmt.Sprintf("%s_%s_%s-%s", podName, podNamespace, containerName, containerID)[:251]+".invalidsuffix"),
+			expected:    "",
+			shouldError: true,
+		},
+		{
+			name:        "container ID too short",
+			logSymLink:  filepath.Join(legacyContainerLogsDir, fmt.Sprintf("%s_%s_%s-%s", podName, podNamespace, containerName, containerID[:5])+".log"),
+			expected:    "",
+			shouldError: true,
+		},
+		{
+			name:        "valid path",
+			logSymLink:  filepath.Join(legacyContainerLogsDir, fmt.Sprintf("%s_%s_%s-%s", podName, podNamespace, containerName, containerID)[:251]+".log"),
+			expected:    containerID[:40],
+			shouldError: false,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			containerID, err := getContainerIDFromLegacyLogSymlink(test.logSymLink)
+			if test.shouldError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+			assert.Equal(t, test.expected, containerID)
+		})
+	}
+}

pkg/kubelet/kuberuntime/security_context_others_test.go

@@ -20,11 +20,12 @@ limitations under the License.
 package kuberuntime
 
 import (
-	"k8s.io/api/core/v1"
+	"testing"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/stretchr/testify/assert"
-	"testing"
+
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestVerifyRunAsNonRoot(t *testing.T) {
@@ -64,6 +65,14 @@ func TestVerifyRunAsNonRoot(t *testing.T) {
 			uid:  &rootUser,
 			fail: false,
 		},
+		{
+			desc: "Pass if RunAsUser is non-root and RunAsNonRoot is true",
+			sc: &v1.SecurityContext{
+				RunAsNonRoot: &runAsNonRootTrue,
+				RunAsUser:    &anyUser,
+			},
+			fail: false,
+		},
 		{
 			desc: "Pass if RunAsNonRoot is not set",
 			sc: &v1.SecurityContext{