Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add non-root end-to-end test cases to credentials test suite #327

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+109 −25
Open

Add non-root end-to-end test cases to credentials test suite #327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68377c3
Add test-cases to credentials e2e suite for running Pods as non-root
unexge Dec 24, 2024
01cd7f7
Fix non-root cache end-to-end test
unexge Dec 27, 2024
12d1737
Use `defaultNonRoot{User, Group}` constants in all tests
unexge Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions tests/e2e-kubernetes/testsuites/cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

const volumeName1 = "volume1"
const root = int64(0)
const defaultNonRootGroup = int64(2000)

type s3CSICacheTestSuite struct {
tsInfo storageframework.TestSuiteInfo
Expand Down Expand Up @@ -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),
muddyfish marked this conversation as resolved.
Show resolved Hide resolved
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([]func(*v1.Pod){podModifierNonRoot}, basePodModifiers...)
muddyfish marked this conversation as resolved.
Show resolved Hide resolved

pod, bucketName := createPod(ctx, mountOptions, podModifiers...)
checkBasicFileOperations(ctx, pod, bucketName, e2epod.VolumeMountPath1)
Expand Down Expand Up @@ -332,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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we always changing this even if the test explicitly set the security context?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you instantiate Kubernetes testing framework with LevelRestricted, it sets this to true, meaning all containers in the Pod must run as non-root (i.e., not 0).

Since this is a generic helper function and gets called from different security levels, we're just making sure to set it to false, so we can spawn our privileged container needed to create cache directory. The other containers created for this Pod would probably have their RunAsUser set to non-root (due to security level), so we just need to remove this restriction at Pod-level for our container.

This whole ensureCacheDirExistsInNode is very hacky due to fact Mountpoint running in the host and hopefully this will go away with running Mountpoint inside container.


// 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{
Expand Down
72 changes: 61 additions & 11 deletions tests/e2e-kubernetes/testsuites/credentials.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) })

Expand All @@ -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"
Expand Down Expand Up @@ -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() {
Expand All @@ -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)
Expand All @@ -385,14 +412,20 @@ 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)
})
})
})

Describe("Pod level", func() {
Describe("Pod Level", func() {
enablePodLevelIdentity := func(ctx context.Context) context.Context {
return contextWithAuthenticationSource(ctx, "pod")
}
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to pass in keyword arguments here? Right now I can't understand what these boolean arguments are for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go doesn't support named arguments, usually structs are used if such a thing is desired, but I think it'd make this more verbose. Since this is a local helper function and not used outside this test-case, I think I'd prefer current one.

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

Expand Down Expand Up @@ -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)
})

Expand Down
18 changes: 12 additions & 6 deletions tests/e2e-kubernetes/testsuites/mountoptions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
28 changes: 27 additions & 1 deletion tests/e2e-kubernetes/testsuites/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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-"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Loading