From 23207436cdc06f9a4174a7146f1c5dd2d1d8b646 Mon Sep 17 00:00:00 2001 From: Saikat Roychowdhury Date: Tue, 3 Nov 2020 05:41:15 +0000 Subject: [PATCH 1/2] Enable ConfigurableFSGroupPolicy feature gate --- pkg/features/kube_features.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 53c8bdf19c3..1487ffeb118 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -725,7 +725,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS CSIMigrationOpenStack: {Default: false, PreRelease: featuregate.Beta}, // Off by default (requires OpenStack Cinder CSI driver) CSIMigrationOpenStackComplete: {Default: false, PreRelease: featuregate.Alpha}, VolumeSubpath: {Default: true, PreRelease: featuregate.GA}, - ConfigurableFSGroupPolicy: {Default: false, PreRelease: featuregate.Alpha}, + ConfigurableFSGroupPolicy: {Default: true, PreRelease: featuregate.Beta}, BalanceAttachedNodeVolumes: {Default: false, PreRelease: featuregate.Alpha}, CSIBlockVolume: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.20 CSIInlineVolume: {Default: true, PreRelease: featuregate.Beta}, From a07096952bb91795d5d37f8e5df3f5e9a9d06d28 Mon Sep 17 00:00:00 2001 From: Saikat Roychowdhury Date: Wed, 4 Nov 2020 07:03:09 +0000 Subject: [PATCH 2/2] FsgroupChange policy test suite --- test/e2e/framework/pod/create.go | 29 +- test/e2e/storage/testsuites/BUILD | 2 + test/e2e/storage/testsuites/base.go | 1 + .../storage/testsuites/fsgroupchangepolicy.go | 275 ++++++++++++++++++ test/e2e/storage/utils/utils.go | 19 ++ 5 files changed, 314 insertions(+), 12 deletions(-) create mode 100644 test/e2e/storage/testsuites/fsgroupchangepolicy.go diff --git a/test/e2e/framework/pod/create.go b/test/e2e/framework/pod/create.go index 353fb8cb9f2..623d307cf5d 100644 --- a/test/e2e/framework/pod/create.go +++ b/test/e2e/framework/pod/create.go @@ -37,18 +37,19 @@ var ( // Config is a struct containing all arguments for creating a pod. // SELinux testing requires to pass HostIPC and HostPID as boolean arguments. type Config struct { - NS string - PVCs []*v1.PersistentVolumeClaim - PVCsReadOnly bool - InlineVolumeSources []*v1.VolumeSource - IsPrivileged bool - Command string - HostIPC bool - HostPID bool - SeLinuxLabel *v1.SELinuxOptions - FsGroup *int64 - NodeSelection NodeSelection - ImageID int + NS string + PVCs []*v1.PersistentVolumeClaim + PVCsReadOnly bool + InlineVolumeSources []*v1.VolumeSource + IsPrivileged bool + Command string + HostIPC bool + HostPID bool + SeLinuxLabel *v1.SELinuxOptions + FsGroup *int64 + NodeSelection NodeSelection + ImageID int + PodFSGroupChangePolicy *v1.PodFSGroupChangePolicy } // CreateUnschedulablePod with given claims based on node selector @@ -219,6 +220,10 @@ func MakeSecPod(podConfig *Config) (*v1.Pod, error) { RestartPolicy: v1.RestartPolicyOnFailure, }, } + if podConfig.PodFSGroupChangePolicy != nil { + podSpec.Spec.SecurityContext.FSGroupChangePolicy = podConfig.PodFSGroupChangePolicy + } + var volumeMounts = make([]v1.VolumeMount, 0) var volumeDevices = make([]v1.VolumeDevice, 0) var volumes = make([]v1.Volume, len(podConfig.PVCs)+len(podConfig.InlineVolumeSources)) diff --git a/test/e2e/storage/testsuites/BUILD b/test/e2e/storage/testsuites/BUILD index 127e187b531..508772790ab 100644 --- a/test/e2e/storage/testsuites/BUILD +++ b/test/e2e/storage/testsuites/BUILD @@ -7,6 +7,7 @@ go_library( "disruptive.go", "driveroperations.go", "ephemeral.go", + "fsgroupchangepolicy.go", "multivolume.go", "provisioning.go", "snapshottable.go", @@ -63,6 +64,7 @@ go_library( "//vendor/github.com/onsi/ginkgo:go_default_library", "//vendor/github.com/onsi/gomega:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", ], ) diff --git a/test/e2e/storage/testsuites/base.go b/test/e2e/storage/testsuites/base.go index 87a2f5c236f..96d8fc922a4 100644 --- a/test/e2e/storage/testsuites/base.go +++ b/test/e2e/storage/testsuites/base.go @@ -85,6 +85,7 @@ var BaseSuites = []func() TestSuite{ InitVolumeLimitsTestSuite, InitTopologyTestSuite, InitVolumeStressTestSuite, + InitFsGroupChangePolicyTestSuite, } // CSISuites is a list of storage test suites that work only for CSI drivers diff --git a/test/e2e/storage/testsuites/fsgroupchangepolicy.go b/test/e2e/storage/testsuites/fsgroupchangepolicy.go new file mode 100644 index 00000000000..380185d891c --- /dev/null +++ b/test/e2e/storage/testsuites/fsgroupchangepolicy.go @@ -0,0 +1,275 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testsuites + +import ( + "fmt" + "strconv" + + "github.com/onsi/ginkgo" + v1 "k8s.io/api/core/v1" + errors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/kubernetes/test/e2e/framework" + e2epod "k8s.io/kubernetes/test/e2e/framework/pod" + e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" + e2evolume "k8s.io/kubernetes/test/e2e/framework/volume" + "k8s.io/kubernetes/test/e2e/storage/testpatterns" + storageutils "k8s.io/kubernetes/test/e2e/storage/utils" + utilpointer "k8s.io/utils/pointer" +) + +const ( + rootDir = "/mnt/volume1" + rootDirFile = "file1" + rootDirFilePath = rootDir + "/" + rootDirFile + subdir = "/mnt/volume1/subdir" + subDirFile = "file2" + subDirFilePath = subdir + "/" + subDirFile +) + +type fsGroupChangePolicyTestSuite struct { + tsInfo TestSuiteInfo +} + +var _ TestSuite = &fsGroupChangePolicyTestSuite{} + +// InitFsGroupChangePolicyTestSuite returns fsGroupChangePolicyTestSuite that implements TestSuite interface +func InitFsGroupChangePolicyTestSuite() TestSuite { + return &fsGroupChangePolicyTestSuite{ + tsInfo: TestSuiteInfo{ + Name: "fsgroupchangepolicy", + TestPatterns: []testpatterns.TestPattern{ + testpatterns.DefaultFsDynamicPV, + }, + SupportedSizeRange: e2evolume.SizeRange{ + Min: "1Mi", + }, + }, + } +} + +func (s *fsGroupChangePolicyTestSuite) GetTestSuiteInfo() TestSuiteInfo { + return s.tsInfo +} + +func (s *fsGroupChangePolicyTestSuite) SkipRedundantSuite(driver TestDriver, pattern testpatterns.TestPattern) { + skipVolTypePatterns(pattern, driver, testpatterns.NewVolTypeMap(testpatterns.CSIInlineVolume, testpatterns.GenericEphemeralVolume)) +} + +func (s *fsGroupChangePolicyTestSuite) DefineTests(driver TestDriver, pattern testpatterns.TestPattern) { + type local struct { + config *PerTestConfig + driverCleanup func() + driver TestDriver + resource *VolumeResource + } + var l local + ginkgo.BeforeEach(func() { + dInfo := driver.GetDriverInfo() + if !dInfo.Capabilities[CapFsGroup] { + e2eskipper.Skipf("Driver %q does not support FsGroup - skipping", dInfo.Name) + } + + if pattern.VolMode == v1.PersistentVolumeBlock { + e2eskipper.Skipf("Test does not support non-filesystem volume mode - skipping") + } + + if pattern.VolType != testpatterns.DynamicPV { + e2eskipper.Skipf("Suite %q does not support %v", s.tsInfo.Name, pattern.VolType) + } + + _, ok := driver.(DynamicPVTestDriver) + if !ok { + e2eskipper.Skipf("Driver %s doesn't support %v -- skipping", dInfo.Name, pattern.VolType) + } + }) + + // This intentionally comes after checking the preconditions because it + // registers its own BeforeEach which creates the namespace. Beware that it + // also registers an AfterEach which renders f unusable. Any code using + // f must run inside an It or Context callback. + f := framework.NewDefaultFramework("fsgroupchangepolicy") + + init := func() { + e2eskipper.SkipIfNodeOSDistroIs("windows") + l = local{} + l.driver = driver + l.config, l.driverCleanup = driver.PrepareTest(f) + testVolumeSizeRange := s.GetTestSuiteInfo().SupportedSizeRange + l.resource = CreateVolumeResource(l.driver, l.config, pattern, testVolumeSizeRange) + } + + cleanup := func() { + var errs []error + if l.resource != nil { + if err := l.resource.CleanupResource(); err != nil { + errs = append(errs, err) + } + l.resource = nil + } + + if l.driverCleanup != nil { + errs = append(errs, tryFunc(l.driverCleanup)) + l.driverCleanup = nil + } + + framework.ExpectNoError(errors.NewAggregate(errs), "while cleanup resource") + } + + tests := []struct { + name string // Test case name + podfsGroupChangePolicy string // 'Always' or 'OnRootMismatch' + initialPodFsGroup int // FsGroup of the initial pod + changedRootDirFileOwnership int // Change the ownership of the file in the root directory (/mnt/volume1/file1), as part of the initial pod + changedSubDirFileOwnership int // Change the ownership of the file in the sub directory (/mnt/volume1/subdir/file2), as part of the initial pod + secondPodFsGroup int // FsGroup of the second pod + finalExpectedRootDirFileOwnership int // Final expcted ownership of the file in the root directory (/mnt/volume1/file1), as part of the second pod + finalExpectedSubDirFileOwnership int // Final expcted ownership of the file in the sub directory (/mnt/volume1/subdir/file2), as part of the second pod + }{ + // Test cases for 'Always' policy + { + name: "pod created with an initial fsgroup, new pod fsgroup applied to volume contents", + podfsGroupChangePolicy: "Always", + initialPodFsGroup: 1000, + secondPodFsGroup: 2000, + finalExpectedRootDirFileOwnership: 2000, + finalExpectedSubDirFileOwnership: 2000, + }, + { + name: "pod created with an initial fsgroup, volume contents ownership changed in first pod, new pod with same fsgroup applied to the volume contents", + podfsGroupChangePolicy: "Always", + initialPodFsGroup: 1000, + changedRootDirFileOwnership: 2000, + changedSubDirFileOwnership: 3000, + secondPodFsGroup: 1000, + finalExpectedRootDirFileOwnership: 1000, + finalExpectedSubDirFileOwnership: 1000, + }, + { + name: "pod created with an initial fsgroup, volume contents ownership changed in first pod, new pod with different fsgroup applied to the volume contents", + podfsGroupChangePolicy: "Always", + initialPodFsGroup: 1000, + changedRootDirFileOwnership: 2000, + changedSubDirFileOwnership: 3000, + secondPodFsGroup: 4000, + finalExpectedRootDirFileOwnership: 4000, + finalExpectedSubDirFileOwnership: 4000, + }, + // Test cases for 'OnRootMismatch' policy + { + name: "pod created with an initial fsgroup, new pod fsgroup applied to volume contents", + podfsGroupChangePolicy: "OnRootMismatch", + initialPodFsGroup: 1000, + secondPodFsGroup: 2000, + finalExpectedRootDirFileOwnership: 2000, + finalExpectedSubDirFileOwnership: 2000, + }, + { + name: "pod created with an initial fsgroup, volume contents ownership changed in first pod, new pod with same fsgroup skips ownership changes to the volume contents", + podfsGroupChangePolicy: "OnRootMismatch", + initialPodFsGroup: 1000, + changedRootDirFileOwnership: 2000, + changedSubDirFileOwnership: 3000, + secondPodFsGroup: 1000, + finalExpectedRootDirFileOwnership: 2000, + finalExpectedSubDirFileOwnership: 3000, + }, + { + name: "pod created with an initial fsgroup, volume contents ownership changed in first pod, new pod with different fsgroup applied to the volume contents", + podfsGroupChangePolicy: "OnRootMismatch", + initialPodFsGroup: 1000, + changedRootDirFileOwnership: 2000, + changedSubDirFileOwnership: 3000, + secondPodFsGroup: 4000, + finalExpectedRootDirFileOwnership: 4000, + finalExpectedSubDirFileOwnership: 4000, + }, + } + + for _, t := range tests { + test := t + testCaseName := fmt.Sprintf("(%s)[LinuxOnly], %s", test.podfsGroupChangePolicy, test.name) + ginkgo.It(testCaseName, func() { + init() + defer cleanup() + + policy := v1.PodFSGroupChangePolicy(test.podfsGroupChangePolicy) + podConfig := e2epod.Config{ + NS: f.Namespace.Name, + NodeSelection: l.config.ClientNodeSelection, + PVCs: []*v1.PersistentVolumeClaim{l.resource.Pvc}, + FsGroup: utilpointer.Int64Ptr(int64(test.initialPodFsGroup)), + PodFSGroupChangePolicy: &policy, + } + // Create initial pod and create files in root and sub-directory and verify ownership. + pod := createPodAndVerifyContentGid(l.config.Framework, &podConfig, true /* createInitialFiles */, "" /* expectedRootDirFileOwnership */, "" /* expectedSubDirFileOwnership */) + + // Change the ownership of files in the initial pod. + if test.changedRootDirFileOwnership != 0 { + ginkgo.By(fmt.Sprintf("Changing the root directory file ownership to %s", strconv.Itoa(test.changedRootDirFileOwnership))) + storageutils.ChangeFilePathGidInPod(f, rootDirFilePath, strconv.Itoa(test.changedRootDirFileOwnership), pod) + } + + if test.changedSubDirFileOwnership != 0 { + ginkgo.By(fmt.Sprintf("Changing the sub-directory file ownership to %s", strconv.Itoa(test.changedSubDirFileOwnership))) + storageutils.ChangeFilePathGidInPod(f, subDirFilePath, strconv.Itoa(test.changedSubDirFileOwnership), pod) + } + + ginkgo.By(fmt.Sprintf("Deleting Pod %s/%s", pod.Namespace, pod.Name)) + framework.ExpectNoError(e2epod.DeletePodWithWait(f.ClientSet, pod)) + + // Create a second pod with existing volume and verify the contents ownership. + podConfig.FsGroup = utilpointer.Int64Ptr(int64(test.secondPodFsGroup)) + pod = createPodAndVerifyContentGid(l.config.Framework, &podConfig, false /* createInitialFiles */, strconv.Itoa(test.finalExpectedRootDirFileOwnership), strconv.Itoa(test.finalExpectedSubDirFileOwnership)) + ginkgo.By(fmt.Sprintf("Deleting Pod %s/%s", pod.Namespace, pod.Name)) + framework.ExpectNoError(e2epod.DeletePodWithWait(f.ClientSet, pod)) + }) + } +} + +func createPodAndVerifyContentGid(f *framework.Framework, podConfig *e2epod.Config, createInitialFiles bool, expectedRootDirFileOwnership, expectedSubDirFileOwnership string) *v1.Pod { + podFsGroup := strconv.FormatInt(*podConfig.FsGroup, 10) + ginkgo.By(fmt.Sprintf("Creating Pod in namespace %s with fsgroup %s", podConfig.NS, podFsGroup)) + pod, err := e2epod.CreateSecPodWithNodeSelection(f.ClientSet, podConfig, framework.PodStartTimeout) + framework.ExpectNoError(err) + framework.Logf("Pod %s/%s started successfully", pod.Namespace, pod.Name) + + if createInitialFiles { + ginkgo.By(fmt.Sprintf("Creating a sub-directory and file, and verifying their ownership is %s", podFsGroup)) + cmd := fmt.Sprintf("touch %s", rootDirFilePath) + var err error + _, _, err = storageutils.PodExec(f, pod, cmd) + framework.ExpectNoError(err) + storageutils.VerifyFilePathGidInPod(f, rootDirFilePath, podFsGroup, pod) + + cmd = fmt.Sprintf("mkdir %s", subdir) + _, _, err = storageutils.PodExec(f, pod, cmd) + framework.ExpectNoError(err) + cmd = fmt.Sprintf("touch %s", subDirFilePath) + _, _, err = storageutils.PodExec(f, pod, cmd) + framework.ExpectNoError(err) + storageutils.VerifyFilePathGidInPod(f, subDirFilePath, podFsGroup, pod) + return pod + } + + // Verify existing contents of the volume + ginkgo.By(fmt.Sprintf("Verifying the ownership of root directory file is %s", expectedRootDirFileOwnership)) + storageutils.VerifyFilePathGidInPod(f, rootDirFilePath, expectedRootDirFileOwnership, pod) + ginkgo.By(fmt.Sprintf("Verifying the ownership of sub directory file is %s", expectedSubDirFileOwnership)) + storageutils.VerifyFilePathGidInPod(f, subDirFilePath, expectedSubDirFileOwnership, pod) + return pod +} diff --git a/test/e2e/storage/utils/utils.go b/test/e2e/storage/utils/utils.go index 17f9713d7b4..e3a4fec83ec 100644 --- a/test/e2e/storage/utils/utils.go +++ b/test/e2e/storage/utils/utils.go @@ -841,3 +841,22 @@ func WaitForGVRFinalizer(ctx context.Context, c dynamic.Interface, gvr schema.Gr } return err } + +// VerifyFilePathGidInPod verfies expected GID of the target filepath +func VerifyFilePathGidInPod(f *framework.Framework, filePath, expectedGid string, pod *v1.Pod) { + cmd := fmt.Sprintf("ls -l %s", filePath) + stdout, stderr, err := PodExec(f, pod, cmd) + framework.ExpectNoError(err) + framework.Logf("pod %s/%s exec for cmd %s, stdout: %s, stderr: %s", pod.Namespace, pod.Name, cmd, stdout, stderr) + ll := strings.Fields(stdout) + framework.Logf("stdout split: %v, expected gid: %v", ll, expectedGid) + framework.ExpectEqual(ll[3], expectedGid) +} + +// ChangeFilePathGidInPod changes the GID of the target filepath. +func ChangeFilePathGidInPod(f *framework.Framework, filePath, targetGid string, pod *v1.Pod) { + cmd := fmt.Sprintf("chgrp %s %s", targetGid, filePath) + _, _, err := PodExec(f, pod, cmd) + framework.ExpectNoError(err) + VerifyFilePathGidInPod(f, filePath, targetGid, pod) +}