From 3bcf407d2fc33b80da14a6f87657d5080c92897f Mon Sep 17 00:00:00 2001 From: Christian Huffman Date: Fri, 17 Jul 2020 12:24:08 -0400 Subject: [PATCH] Addresses nitpicks for FSGroupPolicy --- api/openapi-spec/swagger.json | 4 +- pkg/apis/storage/types.go | 11 ++-- pkg/volume/csi/csi_plugin_test.go | 53 +++++++++++++++++++ .../src/k8s.io/api/storage/v1/generated.proto | 4 +- staging/src/k8s.io/api/storage/v1/types.go | 11 ++-- .../storage/v1/types_swagger_doc_generated.go | 2 +- .../api/storage/v1beta1/generated.proto | 4 +- .../src/k8s.io/api/storage/v1beta1/types.go | 10 ++-- .../v1beta1/types_swagger_doc_generated.go | 2 +- 9 files changed, 84 insertions(+), 17 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 60cc68f14be..9bdf62242ff 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -15774,7 +15774,7 @@ "type": "boolean" }, "fsGroupPolicy": { - "description": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is alpha-level, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.", + "description": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is beta, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.\n\nDefaults to ReadWriteOnceWithFSType, which will use heuristics to determine if Kubernetes should modify ownership and permissions of the volume.", "type": "string" }, "podInfoOnMount": { @@ -16509,7 +16509,7 @@ "type": "boolean" }, "fsGroupPolicy": { - "description": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is alpha-level, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.", + "description": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is beta, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.\n\nDefaults to ReadWriteOnceWithFSType, which will use heuristics to determine if Kubernetes should modify ownership and permissions of the volume.", "type": "string" }, "podInfoOnMount": { diff --git a/pkg/apis/storage/types.go b/pkg/apis/storage/types.go index 09e8cd4700c..3bd32c3fe7e 100644 --- a/pkg/apis/storage/types.go +++ b/pkg/apis/storage/types.go @@ -283,11 +283,13 @@ type CSIDriverSpec struct { // Defines if the underlying volume supports changing ownership and // permission of the volume before being mounted. // Refer to the specific FSGroupPolicy values for additional details. - // This field is alpha-level, and is only honored by servers + // This field is beta, and is only honored by servers // that enable the CSIVolumeFSGroupPolicy feature gate. // // This field is immutable. // + // Defaults to ReadWriteOnceWithFSType, which will use heuristics to + // determine if Kubernetes should modify ownership and permissions of the volume. // +optional FSGroupPolicy *FSGroupPolicy @@ -415,10 +417,11 @@ const ( ReadWriteOnceWithFSTypeFSGroupPolicy FSGroupPolicy = "ReadWriteOnceWithFSType" // FileFSGroupPolicy indicates that CSI driver supports volume ownership - // and permission change via fsGroup, and Kubernetes may use fsGroup - // to change permissions and ownership of the volume to match user requested fsGroup in + // and permission change via fsGroup, and Kubernetes will change the permissions + // and ownership of every file in the volume to match the user requested fsGroup in // the pod's SecurityPolicy regardless of fstype or access mode. - // This mode should be defined if the fsGroup is expected to always change on mount + // Use this mode if Kubernetes should modify the permissions and ownership + // of the volume. FileFSGroupPolicy FSGroupPolicy = "File" // NoneFSGroupPolicy indicates that volumes will be mounted without performing diff --git a/pkg/volume/csi/csi_plugin_test.go b/pkg/volume/csi/csi_plugin_test.go index 3632df8a3d6..662fda51a42 100644 --- a/pkg/volume/csi/csi_plugin_test.go +++ b/pkg/volume/csi/csi_plugin_test.go @@ -26,6 +26,7 @@ import ( api "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" + storage "k8s.io/api/storage/v1" storagev1 "k8s.io/api/storage/v1" meta "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -164,6 +165,58 @@ func TestPluginGetPluginName(t *testing.T) { } } +func TestPluginGetFSGroupPolicy(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIVolumeFSGroupPolicy, true)() + defaultPolicy := storage.ReadWriteOnceWithFSTypeFSGroupPolicy + testCases := []struct { + name string + defined bool + expectedFSGroupPolicy storage.FSGroupPolicy + }{ + { + name: "no FSGroupPolicy defined, expect default", + defined: false, + expectedFSGroupPolicy: storage.ReadWriteOnceWithFSTypeFSGroupPolicy, + }, + { + name: "File FSGroupPolicy defined, expect File", + defined: true, + expectedFSGroupPolicy: storage.FileFSGroupPolicy, + }, + { + name: "None FSGroupPolicy defined, expected None", + defined: true, + expectedFSGroupPolicy: storage.NoneFSGroupPolicy, + }, + } + for _, tc := range testCases { + t.Logf("testing: %s", tc.name) + // Define the driver and set the FSGroupPolicy + driver := getTestCSIDriver(testDriver, nil, nil, nil) + if tc.defined { + driver.Spec.FSGroupPolicy = &tc.expectedFSGroupPolicy + } else { + driver.Spec.FSGroupPolicy = &defaultPolicy + } + + // Create the client and register the resources + fakeClient := fakeclient.NewSimpleClientset(driver) + plug, tmpDir := newTestPlugin(t, fakeClient) + defer os.RemoveAll(tmpDir) + registerFakePlugin(testDriver, "endpoint", []string{"1.3.0"}, t) + + // Check to see if we can obtain the CSIDriver, along with examining its FSGroupPolicy + fsGroup, err := plug.getFSGroupPolicy(testDriver) + if err != nil { + t.Fatalf("Error attempting to obtain FSGroupPolicy: %v", err) + } + if fsGroup != *driver.Spec.FSGroupPolicy { + t.Fatalf("FSGroupPolicy doesn't match expected value: %v, %v", fsGroup, tc.expectedFSGroupPolicy) + } + } + +} + func TestPluginGetVolumeName(t *testing.T) { plug, tmpDir := newTestPlugin(t, nil) defer os.RemoveAll(tmpDir) diff --git a/staging/src/k8s.io/api/storage/v1/generated.proto b/staging/src/k8s.io/api/storage/v1/generated.proto index 0e9a2e1da3f..9ba3b8d2e60 100644 --- a/staging/src/k8s.io/api/storage/v1/generated.proto +++ b/staging/src/k8s.io/api/storage/v1/generated.proto @@ -154,11 +154,13 @@ message CSIDriverSpec { // Defines if the underlying volume supports changing ownership and // permission of the volume before being mounted. // Refer to the specific FSGroupPolicy values for additional details. - // This field is alpha-level, and is only honored by servers + // This field is beta, and is only honored by servers // that enable the CSIVolumeFSGroupPolicy feature gate. // // This field is immutable. // + // Defaults to ReadWriteOnceWithFSType, which will use heuristics to + // determine if Kubernetes should modify ownership and permissions of the volume. // +optional optional string fsGroupPolicy = 5; diff --git a/staging/src/k8s.io/api/storage/v1/types.go b/staging/src/k8s.io/api/storage/v1/types.go index 6a7bf492920..3b46c8ce8db 100644 --- a/staging/src/k8s.io/api/storage/v1/types.go +++ b/staging/src/k8s.io/api/storage/v1/types.go @@ -352,11 +352,13 @@ type CSIDriverSpec struct { // Defines if the underlying volume supports changing ownership and // permission of the volume before being mounted. // Refer to the specific FSGroupPolicy values for additional details. - // This field is alpha-level, and is only honored by servers + // This field is beta, and is only honored by servers // that enable the CSIVolumeFSGroupPolicy feature gate. // // This field is immutable. // + // Defaults to ReadWriteOnceWithFSType, which will use heuristics to + // determine if Kubernetes should modify ownership and permissions of the volume. // +optional FSGroupPolicy *FSGroupPolicy `json:"fsGroupPolicy,omitempty" protobuf:"bytes,5,opt,name=fsGroupPolicy"` @@ -414,10 +416,11 @@ const ( ReadWriteOnceWithFSTypeFSGroupPolicy FSGroupPolicy = "ReadWriteOnceWithFSType" // FileFSGroupPolicy indicates that CSI driver supports volume ownership - // and permission change via fsGroup, and Kubernetes may use fsGroup - // to change permissions and ownership of the volume to match user requested fsGroup in + // and permission change via fsGroup, and Kubernetes will change the permissions + // and ownership of every file in the volume to match the user requested fsGroup in // the pod's SecurityPolicy regardless of fstype or access mode. - // This mode should be defined if the fsGroup is expected to always change on mount + // Use this mode if Kubernetes should modify the permissions and ownership + // of the volume. FileFSGroupPolicy FSGroupPolicy = "File" // NoneFSGroupPolicy indicates that volumes will be mounted without performing diff --git a/staging/src/k8s.io/api/storage/v1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/storage/v1/types_swagger_doc_generated.go index a9c7cc9afdd..edcd7953a7d 100644 --- a/staging/src/k8s.io/api/storage/v1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/storage/v1/types_swagger_doc_generated.go @@ -53,7 +53,7 @@ var map_CSIDriverSpec = map[string]string{ "podInfoOnMount": "If set to true, podInfoOnMount indicates this CSI volume driver requires additional pod information (like podName, podUID, etc.) during mount operations. If set to false, pod information will not be passed on mount. Default is false. The CSI driver specifies podInfoOnMount as part of driver deployment. If true, Kubelet will pass pod information as VolumeContext in the CSI NodePublishVolume() calls. The CSI driver is responsible for parsing and validating the information passed in as VolumeContext. The following VolumeConext will be passed if podInfoOnMount is set to true. This list might grow, but the prefix will be used. \"csi.storage.k8s.io/pod.name\": pod.Name \"csi.storage.k8s.io/pod.namespace\": pod.Namespace \"csi.storage.k8s.io/pod.uid\": string(pod.UID) \"csi.storage.k8s.io/ephemeral\": \"true\" if the volume is an ephemeral inline volume\n defined by a CSIVolumeSource, otherwise \"false\"\n\n\"csi.storage.k8s.io/ephemeral\" is a new feature in Kubernetes 1.16. It is only required for drivers which support both the \"Persistent\" and \"Ephemeral\" VolumeLifecycleMode. Other drivers can leave pod info disabled and/or ignore this field. As Kubernetes 1.15 doesn't support this field, drivers can only support one mode when deployed on such a cluster and the deployment determines which mode that is, for example via a command line parameter of the driver.\n\nThis field is immutable.", "volumeLifecycleModes": "volumeLifecycleModes defines what kind of volumes this CSI volume driver supports. The default if the list is empty is \"Persistent\", which is the usage defined by the CSI specification and implemented in Kubernetes via the usual PV/PVC mechanism. The other mode is \"Ephemeral\". In this mode, volumes are defined inline inside the pod spec with CSIVolumeSource and their lifecycle is tied to the lifecycle of that pod. A driver has to be aware of this because it is only going to get a NodePublishVolume call for such a volume. For more information about implementing this mode, see https://kubernetes-csi.github.io/docs/ephemeral-local-volumes.html A driver can support one or more of these modes and more modes may be added in the future. This field is beta.\n\nThis field is immutable.", "storageCapacity": "If set to true, storageCapacity indicates that the CSI volume driver wants pod scheduling to consider the storage capacity that the driver deployment will report by creating CSIStorageCapacity objects with capacity information.\n\nThe check can be enabled immediately when deploying a driver. In that case, provisioning new volumes with late binding will pause until the driver deployment has published some suitable CSIStorageCapacity object.\n\nAlternatively, the driver can be deployed with the field unset or false and it can be flipped later when storage capacity information has been published.\n\nThis field is immutable.\n\nThis is a beta field and only available when the CSIStorageCapacity feature is enabled. The default is false.", - "fsGroupPolicy": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is alpha-level, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.", + "fsGroupPolicy": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is beta, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.\n\nDefaults to ReadWriteOnceWithFSType, which will use heuristics to determine if Kubernetes should modify ownership and permissions of the volume.", "tokenRequests": "TokenRequests indicates the CSI driver needs pods' service account tokens it is mounting volume for to do necessary authentication. Kubelet will pass the tokens in VolumeContext in the CSI NodePublishVolume calls. The CSI driver should parse and validate the following VolumeContext: \"csi.storage.k8s.io/serviceAccount.tokens\": {\n \"\": {\n \"token\": ,\n \"expirationTimestamp\": ,\n },\n ...\n}\n\nNote: Audience in each TokenRequest should be different and at most one token is empty string. To receive a new token after expiry, RequiresRepublish can be used to trigger NodePublishVolume periodically.\n\nThis is a beta feature and only available when the CSIServiceAccountToken feature is enabled.", "requiresRepublish": "RequiresRepublish indicates the CSI driver wants `NodePublishVolume` being periodically called to reflect any possible change in the mounted volume. This field defaults to false.\n\nNote: After a successful initial NodePublishVolume call, subsequent calls to NodePublishVolume should only update the contents of the volume. New mount points will not be seen by a running container.\n\nThis is a beta feature and only available when the CSIServiceAccountToken feature is enabled.", } diff --git a/staging/src/k8s.io/api/storage/v1beta1/generated.proto b/staging/src/k8s.io/api/storage/v1beta1/generated.proto index 5ed6413ec0e..a7411fcf577 100644 --- a/staging/src/k8s.io/api/storage/v1beta1/generated.proto +++ b/staging/src/k8s.io/api/storage/v1beta1/generated.proto @@ -156,11 +156,13 @@ message CSIDriverSpec { // Defines if the underlying volume supports changing ownership and // permission of the volume before being mounted. // Refer to the specific FSGroupPolicy values for additional details. - // This field is alpha-level, and is only honored by servers + // This field is beta, and is only honored by servers // that enable the CSIVolumeFSGroupPolicy feature gate. // // This field is immutable. // + // Defaults to ReadWriteOnceWithFSType, which will use heuristics to + // determine if Kubernetes should modify ownership and permissions of the volume. // +optional optional string fsGroupPolicy = 5; diff --git a/staging/src/k8s.io/api/storage/v1beta1/types.go b/staging/src/k8s.io/api/storage/v1beta1/types.go index d1ffe7a0ec5..6d108becc2d 100644 --- a/staging/src/k8s.io/api/storage/v1beta1/types.go +++ b/staging/src/k8s.io/api/storage/v1beta1/types.go @@ -373,11 +373,13 @@ type CSIDriverSpec struct { // Defines if the underlying volume supports changing ownership and // permission of the volume before being mounted. // Refer to the specific FSGroupPolicy values for additional details. - // This field is alpha-level, and is only honored by servers + // This field is beta, and is only honored by servers // that enable the CSIVolumeFSGroupPolicy feature gate. // // This field is immutable. // + // Defaults to ReadWriteOnceWithFSType, which will use heuristics to + // determine if Kubernetes should modify ownership and permissions of the volume. // +optional FSGroupPolicy *FSGroupPolicy `json:"fsGroupPolicy,omitempty" protobuf:"bytes,5,opt,name=fsGroupPolicy"` @@ -433,9 +435,11 @@ const ( ReadWriteOnceWithFSTypeFSGroupPolicy FSGroupPolicy = "ReadWriteOnceWithFSType" // FileFSGroupPolicy indicates that CSI driver supports volume ownership - // and permission change via fsGroup, and Kubernetes may use fsGroup - // to change permissions and ownership of the volume to match user requested fsGroup in + // and permission change via fsGroup, and Kubernetes will change the permissions + // and ownership of every file in the volume to match the user requested fsGroup in // the pod's SecurityPolicy regardless of fstype or access mode. + // Use this mode if Kubernetes should modify the permissions and ownership + // of the volume. FileFSGroupPolicy FSGroupPolicy = "File" // None indicates that volumes will be mounted without performing diff --git a/staging/src/k8s.io/api/storage/v1beta1/types_swagger_doc_generated.go b/staging/src/k8s.io/api/storage/v1beta1/types_swagger_doc_generated.go index 0d03a30b4f7..6021f960924 100644 --- a/staging/src/k8s.io/api/storage/v1beta1/types_swagger_doc_generated.go +++ b/staging/src/k8s.io/api/storage/v1beta1/types_swagger_doc_generated.go @@ -53,7 +53,7 @@ var map_CSIDriverSpec = map[string]string{ "podInfoOnMount": "If set to true, podInfoOnMount indicates this CSI volume driver requires additional pod information (like podName, podUID, etc.) during mount operations. If set to false, pod information will not be passed on mount. Default is false. The CSI driver specifies podInfoOnMount as part of driver deployment. If true, Kubelet will pass pod information as VolumeContext in the CSI NodePublishVolume() calls. The CSI driver is responsible for parsing and validating the information passed in as VolumeContext. The following VolumeConext will be passed if podInfoOnMount is set to true. This list might grow, but the prefix will be used. \"csi.storage.k8s.io/pod.name\": pod.Name \"csi.storage.k8s.io/pod.namespace\": pod.Namespace \"csi.storage.k8s.io/pod.uid\": string(pod.UID) \"csi.storage.k8s.io/ephemeral\": \"true\" if the volume is an ephemeral inline volume\n defined by a CSIVolumeSource, otherwise \"false\"\n\n\"csi.storage.k8s.io/ephemeral\" is a new feature in Kubernetes 1.16. It is only required for drivers which support both the \"Persistent\" and \"Ephemeral\" VolumeLifecycleMode. Other drivers can leave pod info disabled and/or ignore this field. As Kubernetes 1.15 doesn't support this field, drivers can only support one mode when deployed on such a cluster and the deployment determines which mode that is, for example via a command line parameter of the driver.\n\nThis field is immutable.", "volumeLifecycleModes": "VolumeLifecycleModes defines what kind of volumes this CSI volume driver supports. The default if the list is empty is \"Persistent\", which is the usage defined by the CSI specification and implemented in Kubernetes via the usual PV/PVC mechanism. The other mode is \"Ephemeral\". In this mode, volumes are defined inline inside the pod spec with CSIVolumeSource and their lifecycle is tied to the lifecycle of that pod. A driver has to be aware of this because it is only going to get a NodePublishVolume call for such a volume. For more information about implementing this mode, see https://kubernetes-csi.github.io/docs/ephemeral-local-volumes.html A driver can support one or more of these modes and more modes may be added in the future.\n\nThis field is immutable.", "storageCapacity": "If set to true, storageCapacity indicates that the CSI volume driver wants pod scheduling to consider the storage capacity that the driver deployment will report by creating CSIStorageCapacity objects with capacity information.\n\nThe check can be enabled immediately when deploying a driver. In that case, provisioning new volumes with late binding will pause until the driver deployment has published some suitable CSIStorageCapacity object.\n\nAlternatively, the driver can be deployed with the field unset or false and it can be flipped later when storage capacity information has been published.\n\nThis field is immutable.\n\nThis is a beta field and only available when the CSIStorageCapacity feature is enabled. The default is false.", - "fsGroupPolicy": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is alpha-level, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.", + "fsGroupPolicy": "Defines if the underlying volume supports changing ownership and permission of the volume before being mounted. Refer to the specific FSGroupPolicy values for additional details. This field is beta, and is only honored by servers that enable the CSIVolumeFSGroupPolicy feature gate.\n\nThis field is immutable.\n\nDefaults to ReadWriteOnceWithFSType, which will use heuristics to determine if Kubernetes should modify ownership and permissions of the volume.", "tokenRequests": "TokenRequests indicates the CSI driver needs pods' service account tokens it is mounting volume for to do necessary authentication. Kubelet will pass the tokens in VolumeContext in the CSI NodePublishVolume calls. The CSI driver should parse and validate the following VolumeContext: \"csi.storage.k8s.io/serviceAccount.tokens\": {\n \"\": {\n \"token\": ,\n \"expirationTimestamp\": ,\n },\n ...\n}\n\nNote: Audience in each TokenRequest should be different and at most one token is empty string. To receive a new token after expiry, RequiresRepublish can be used to trigger NodePublishVolume periodically.\n\nThis is a beta feature and only available when the CSIServiceAccountToken feature is enabled.", "requiresRepublish": "RequiresRepublish indicates the CSI driver wants `NodePublishVolume` being periodically called to reflect any possible change in the mounted volume. This field defaults to false.\n\nNote: After a successful initial NodePublishVolume call, subsequent calls to NodePublishVolume should only update the contents of the volume. New mount points will not be seen by a running container.\n\nThis is a beta feature and only available when the CSIServiceAccountToken feature is enabled.", }