From 68377c3b746023c636ea58d55e412d0b7a9ee269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Varl=C4=B1?= Date: Tue, 24 Dec 2024 16:33:04 +0000 Subject: [PATCH 1/3] Add test-cases to credentials e2e suite for running Pods as non-root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Burak Varlı --- tests/e2e-kubernetes/testsuites/cache.go | 9 +-- .../e2e-kubernetes/testsuites/credentials.go | 72 ++++++++++++++++--- tests/e2e-kubernetes/testsuites/util.go | 28 +++++++- 3 files changed, 90 insertions(+), 19 deletions(-) diff --git a/tests/e2e-kubernetes/testsuites/cache.go b/tests/e2e-kubernetes/testsuites/cache.go index ff99a257..a8bebcdd 100644 --- a/tests/e2e-kubernetes/testsuites/cache.go +++ b/tests/e2e-kubernetes/testsuites/cache.go @@ -26,7 +26,6 @@ import ( const volumeName1 = "volume1" const root = int64(0) -const defaultNonRootGroup = int64(2000) type s3CSICacheTestSuite struct { tsInfo storageframework.TestSuiteInfo @@ -183,13 +182,9 @@ func (t *s3CSICacheTestSuite) DefineTests(driver storageframework.TestDriver, pa mountOptions := append(baseMountOptions, "allow-delete", "allow-other", - fmt.Sprintf("uid=%d", *e2epod.GetDefaultNonRootUser()), + fmt.Sprintf("uid=%d", defaultNonRootUser), fmt.Sprintf("gid=%d", defaultNonRootGroup)) - podModifiers := append(basePodModifiers, func(pod *v1.Pod) { - pod.Spec.Containers[0].SecurityContext.RunAsUser = e2epod.GetDefaultNonRootUser() - pod.Spec.Containers[0].SecurityContext.RunAsGroup = ptr.To(defaultNonRootGroup) - pod.Spec.Containers[0].SecurityContext.RunAsNonRoot = ptr.To(true) - }) + podModifiers := append(basePodModifiers, podModifierNonRoot) pod, bucketName := createPod(ctx, mountOptions, podModifiers...) checkBasicFileOperations(ctx, pod, bucketName, e2epod.VolumeMountPath1) diff --git a/tests/e2e-kubernetes/testsuites/credentials.go b/tests/e2e-kubernetes/testsuites/credentials.go index a413e05e..0186e567 100644 --- a/tests/e2e-kubernetes/testsuites/credentials.go +++ b/tests/e2e-kubernetes/testsuites/credentials.go @@ -137,8 +137,13 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv return vol } - createPod := func(ctx context.Context, vol *storageframework.VolumeResource) *v1.Pod { - pod, err := e2epod.CreateClientPod(ctx, f.ClientSet, f.Namespace.Name, vol.Pvc) + createPod := func(ctx context.Context, vol *storageframework.VolumeResource, podModifiers ...func(*v1.Pod)) *v1.Pod { + pod := e2epod.MakePod(f.Namespace.Name, nil, []*v1.PersistentVolumeClaim{vol.Pvc}, admissionapi.LevelBaseline, "") + for _, pm := range podModifiers { + pm(pod) + } + + pod, err := createPod(ctx, f.ClientSet, f.Namespace.Name, pod) framework.ExpectNoError(err) deferCleanup(func(ctx context.Context) error { return e2epod.DeletePodWithWait(ctx, f.ClientSet, pod) }) @@ -156,6 +161,17 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv return createPod(ctx, vol) } + createPodAllowsDeleteNonRoot := func(ctx context.Context) *v1.Pod { + vol := createVolumeResourceWithMountOptions(ctx, l.config, pattern, []string{ + "allow-delete", + "allow-other", + fmt.Sprintf("uid=%d", defaultNonRootUser), + fmt.Sprintf("gid=%d", defaultNonRootGroup), + }) + deferCleanup(vol.CleanupResource) + return createPod(ctx, vol, podModifierNonRoot) + } + const ( testVolumePath = e2epod.VolumeMountPath1 testFilePath = testVolumePath + "/file.txt" @@ -345,6 +361,11 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv pod := createPodAllowsDelete(ctx) expectFullAccess(pod) }) + + It("should use ec2 instance profile's full access role as non-root", func(ctx context.Context) { + pod := createPodAllowsDeleteNonRoot(ctx) + expectFullAccess(pod) + }) }) Context("IAM Roles for Service Accounts (IRSA)", Ordered, func() { @@ -366,6 +387,12 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv expectFullAccess(pod) }) + It("should use service account's full access role as non-root", func(ctx context.Context) { + updateCSIDriversServiceAccountRole(ctx, iamPolicyS3FullAccess) + pod := createPodAllowsDeleteNonRoot(ctx) + expectFullAccess(pod) + }) + It("should fail to mount if service account's role does not allow s3::ListObjectsV2", func(ctx context.Context) { updateCSIDriversServiceAccountRole(ctx, iamPolicyS3NoAccess) expectFailToMount(ctx, "", nil) @@ -385,6 +412,12 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv expectFullAccess(pod) }) + It("should use full access aws credentials as non-root", func(ctx context.Context) { + updateDriverLevelKubernetesSecret(ctx, iamPolicyS3FullAccess) + pod := createPodAllowsDeleteNonRoot(ctx) + expectFullAccess(pod) + }) + It("should fail to mount if aws credentials does not allow s3::ListObjectsV2", func(ctx context.Context) { updateDriverLevelKubernetesSecret(ctx, iamPolicyS3NoAccess) expectFailToMount(ctx, "", nil) @@ -392,7 +425,7 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv }) }) - Describe("Pod level", func() { + Describe("Pod Level", func() { enablePodLevelIdentity := func(ctx context.Context) context.Context { return contextWithAuthenticationSource(ctx, "pod") } @@ -424,32 +457,49 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv return assignPolicyToServiceAccount(ctx, sa, policyARN) } - createPodWithServiceAccountAndPolicy := func(ctx context.Context, policyARN string, allowDelete bool) (*v1.Pod, *v1.ServiceAccount) { + createPodWithServiceAccountAndPolicy := func(ctx context.Context, policyARN string, allowDelete bool, asNonRoot bool) (*v1.Pod, *v1.ServiceAccount) { By("Creating Pod with ServiceAccount") mountOptions := []string{fmt.Sprintf("region %s", DefaultRegion)} if allowDelete { mountOptions = append(mountOptions, "allow-delete") } + if asNonRoot { + mountOptions = append(mountOptions, + "allow-other", + fmt.Sprintf("uid=%d", defaultNonRootUser), + fmt.Sprintf("gid=%d", defaultNonRootGroup)) + } vol := createVolumeResourceWithMountOptions(enablePodLevelIdentity(ctx), l.config, pattern, mountOptions) deferCleanup(vol.CleanupResource) sa := createServiceAccountWithPolicy(ctx, policyARN) - pod, err := createPodWithServiceAccount(ctx, f.ClientSet, f.Namespace.Name, []*v1.PersistentVolumeClaim{vol.Pvc}, sa.Name) - framework.ExpectNoError(err) - deferCleanup(func(ctx context.Context) error { return e2epod.DeletePodWithWait(ctx, f.ClientSet, pod) }) + var podModifiers []func(*v1.Pod) + podModifiers = append(podModifiers, func(pod *v1.Pod) { + pod.Spec.ServiceAccountName = sa.Name + }) + if asNonRoot { + podModifiers = append(podModifiers, podModifierNonRoot) + } + + pod := createPod(ctx, vol, podModifiers...) return pod, sa } It("should use pod's service account's read-only role", func(ctx context.Context) { - pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3ReadOnlyAccess, false) + pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3ReadOnlyAccess, false, false) expectReadOnly(pod) }) It("should use pod's service account's full access role", func(ctx context.Context) { - pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3FullAccess, true) + pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3FullAccess, true, false) + expectFullAccess(pod) + }) + + It("should use pod's service account's full access role as non-root", func(ctx context.Context) { + pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3FullAccess, true, true) expectFullAccess(pod) }) @@ -502,14 +552,14 @@ func (t *s3CSICredentialsTestSuite) DefineTests(driver storageframework.TestDriv It("should not use csi driver's service account tokens", func(ctx context.Context) { updateCSIDriversServiceAccountRole(ctx, iamPolicyS3FullAccess) - pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3ReadOnlyAccess, true) + pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3ReadOnlyAccess, true, false) expectReadOnly(pod) }) It("should not use driver-level kubernetes secrets", func(ctx context.Context) { updateDriverLevelKubernetesSecret(ctx, iamPolicyS3FullAccess) - pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3ReadOnlyAccess, true) + pod, _ := createPodWithServiceAccountAndPolicy(ctx, iamPolicyS3ReadOnlyAccess, true, false) expectReadOnly(pod) }) diff --git a/tests/e2e-kubernetes/testsuites/util.go b/tests/e2e-kubernetes/testsuites/util.go index 1bdb8f8d..30e7d5a4 100644 --- a/tests/e2e-kubernetes/testsuites/util.go +++ b/tests/e2e-kubernetes/testsuites/util.go @@ -24,8 +24,12 @@ import ( e2evolume "k8s.io/kubernetes/test/e2e/framework/volume" storageframework "k8s.io/kubernetes/test/e2e/storage/framework" admissionapi "k8s.io/pod-security-admission/api" + "k8s.io/utils/ptr" ) +const defaultNonRootUser = int64(e2epod.DefaultNonRootUser) +const defaultNonRootGroup = int64(2000) + type jsonMap = map[string]interface{} const NamespacePrefix = "aws-s3-csi-e2e-" @@ -146,7 +150,11 @@ func bucketNameFromVolumeResource(vol *storageframework.VolumeResource) string { } func createPod(ctx context.Context, client clientset.Interface, namespace string, pod *v1.Pod) (*v1.Pod, error) { - framework.Logf("Creating Pod %s in %s (SA: %s)", pod.Name, namespace, pod.Spec.ServiceAccountName) + serviceAccount := pod.Spec.ServiceAccountName + if serviceAccount == "" { + serviceAccount = "default" + } + framework.Logf("Creating Pod %s in %s (SA: %s)", pod.Name, namespace, serviceAccount) pod, err := client.CoreV1().Pods(namespace).Create(ctx, pod, metav1.CreateOptions{}) if err != nil { return nil, fmt.Errorf("pod Create API error: %w", err) @@ -170,6 +178,24 @@ func createPodWithServiceAccount(ctx context.Context, client clientset.Interface return createPod(ctx, client, namespace, pod) } +func podModifierNonRoot(pod *v1.Pod) { + if pod.Spec.SecurityContext == nil { + pod.Spec.SecurityContext = &v1.PodSecurityContext{} + } + pod.Spec.SecurityContext.RunAsUser = ptr.To(defaultNonRootUser) + pod.Spec.SecurityContext.RunAsGroup = ptr.To(defaultNonRootGroup) + pod.Spec.SecurityContext.RunAsNonRoot = ptr.To(true) + + for _, container := range pod.Spec.Containers { + if container.SecurityContext == nil { + container.SecurityContext = &v1.SecurityContext{} + } + container.SecurityContext.RunAsUser = ptr.To(defaultNonRootUser) + container.SecurityContext.RunAsGroup = ptr.To(defaultNonRootGroup) + container.SecurityContext.RunAsNonRoot = ptr.To(true) + } +} + func copySmallFileToPod(_ context.Context, f *framework.Framework, pod *v1.Pod, hostPath, podPath string) { data, err := os.ReadFile(hostPath) framework.ExpectNoError(err) From 01cd7f793982ca5905fbea2aa4a75a36bc392093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Varl=C4=B1?= Date: Fri, 27 Dec 2024 17:13:42 +0000 Subject: [PATCH 2/3] Fix non-root cache end-to-end test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Burak Varlı --- tests/e2e-kubernetes/testsuites/cache.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/e2e-kubernetes/testsuites/cache.go b/tests/e2e-kubernetes/testsuites/cache.go index a8bebcdd..f2100e9e 100644 --- a/tests/e2e-kubernetes/testsuites/cache.go +++ b/tests/e2e-kubernetes/testsuites/cache.go @@ -184,7 +184,7 @@ func (t *s3CSICacheTestSuite) DefineTests(driver storageframework.TestDriver, pa "allow-other", fmt.Sprintf("uid=%d", defaultNonRootUser), fmt.Sprintf("gid=%d", defaultNonRootGroup)) - podModifiers := append(basePodModifiers, podModifierNonRoot) + podModifiers := append([]func(*v1.Pod){podModifierNonRoot}, basePodModifiers...) pod, bucketName := createPod(ctx, mountOptions, podModifiers...) checkBasicFileOperations(ctx, pod, bucketName, e2epod.VolumeMountPath1) @@ -327,6 +327,13 @@ func ensureCacheDirExistsInNode(pod *v1.Pod, cacheDir string) { MountPath: "/cache", } + if pod.Spec.SecurityContext == nil { + pod.Spec.SecurityContext = &v1.PodSecurityContext{} + } + // We need to set this false at Pod-level as `chmod-cache-dir` needs to run as `root` and this + // would prevent container creation if its true. + pod.Spec.SecurityContext.RunAsNonRoot = ptr.To(false) + // The directory created with `DirectoryOrCreate` will have 0755 permissions and will be owned by kubelet. // Unless we change permissions here, non-root containers won't be able to access to the cache dir. pod.Spec.InitContainers = append(pod.Spec.DeepCopy().InitContainers, v1.Container{ From 12d1737054ff32405bb9058b8975b6b803481186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Burak=20Varl=C4=B1?= Date: Fri, 3 Jan 2025 14:13:06 +0000 Subject: [PATCH 3/3] Use `defaultNonRoot{User, Group}` constants in all tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Burak Varlı --- .../e2e-kubernetes/testsuites/mountoptions.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/e2e-kubernetes/testsuites/mountoptions.go b/tests/e2e-kubernetes/testsuites/mountoptions.go index 4af8b84b..d64e8024 100644 --- a/tests/e2e-kubernetes/testsuites/mountoptions.go +++ b/tests/e2e-kubernetes/testsuites/mountoptions.go @@ -85,11 +85,17 @@ func (t *s3CSIMountOptionsTestSuite) DefineTests(driver storageframework.TestDri }) validateWriteToVolume := func(ctx context.Context) { - resource := createVolumeResourceWithMountOptions(ctx, l.config, pattern, []string{"uid=1000", "gid=2000", "allow-other", "debug", "debug-crt"}) + resource := createVolumeResourceWithMountOptions(ctx, l.config, pattern, []string{ + fmt.Sprintf("uid=%d", defaultNonRootUser), + fmt.Sprintf("gid=%d", defaultNonRootGroup), + "allow-other", + "debug", + "debug-crt", + }) l.resources = append(l.resources, resource) ginkgo.By("Creating pod with a volume") pod := e2epod.MakePod(f.Namespace.Name, nil, []*v1.PersistentVolumeClaim{resource.Pvc}, admissionapi.LevelRestricted, "") - pod.Spec.SecurityContext.RunAsGroup = ptr.To(int64(2000)) + pod.Spec.SecurityContext.RunAsGroup = ptr.To(defaultNonRootGroup) var err error pod, err = createPod(ctx, f.ClientSet, f.Namespace.Name, pod) framework.ExpectNoError(err) @@ -105,11 +111,11 @@ func (t *s3CSIMountOptionsTestSuite) DefineTests(driver storageframework.TestDri ginkgo.By("Checking read from a volume") checkReadFromPath(f, pod, fileInVol, toWrite, seed) ginkgo.By("Checking file group owner") - e2evolume.VerifyExecInPodSucceed(f, pod, fmt.Sprintf("stat -L -c '%%a %%g %%u' %s | grep '644 2000 1000'", fileInVol)) + e2evolume.VerifyExecInPodSucceed(f, pod, fmt.Sprintf("stat -L -c '%%a %%g %%u' %s | grep '644 %d %d'", fileInVol, defaultNonRootGroup, defaultNonRootUser)) ginkgo.By("Checking dir group owner") - e2evolume.VerifyExecInPodSucceed(f, pod, fmt.Sprintf("stat -L -c '%%a %%g %%u' %s | grep '755 2000 1000'", volPath)) + e2evolume.VerifyExecInPodSucceed(f, pod, fmt.Sprintf("stat -L -c '%%a %%g %%u' %s | grep '755 %d %d'", volPath, defaultNonRootGroup, defaultNonRootUser)) ginkgo.By("Checking pod identity") - e2evolume.VerifyExecInPodSucceed(f, pod, "id | grep 'uid=1000 gid=2000 groups=2000'") + e2evolume.VerifyExecInPodSucceed(f, pod, fmt.Sprintf("id | grep 'uid=%d gid=%d groups=%d'", defaultNonRootUser, defaultNonRootGroup, defaultNonRootGroup)) } ginkgo.It("should access volume as a non-root user", func(ctx context.Context) { validateWriteToVolume(ctx) @@ -124,7 +130,7 @@ func (t *s3CSIMountOptionsTestSuite) DefineTests(driver storageframework.TestDri l.resources = append(l.resources, resource) ginkgo.By("Creating pod with a volume") pod := e2epod.MakePod(f.Namespace.Name, nil, []*v1.PersistentVolumeClaim{resource.Pvc}, admissionapi.LevelRestricted, "") - pod.Spec.SecurityContext.RunAsGroup = ptr.To(int64(2000)) + pod.Spec.SecurityContext.RunAsGroup = ptr.To(defaultNonRootGroup) var err error pod, err = createPod(ctx, f.ClientSet, f.Namespace.Name, pod) framework.ExpectNoError(err)