From 029fd47757518ee525eb679001479a7aa99e4f89 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 24 Jul 2019 08:16:43 +0200 Subject: [PATCH] storage: introduce CSIDriver.Spec.VolumeLifecycleModes Using a "normal" CSI driver for an inline ephemeral volume may have unexpected and potentially harmful effects when the driver gets a NodePublishVolume call that it isn't expecting. To prevent that mistake, driver deployments for a driver that supports such volumes must: - deploy a CSIDriver object for the driver - list "ephemeral" as one of the supported modes The default is "persistent", so existing deployments continue to work and are automatically protected against incorrect usage. This commit contains the API change. Generated code and manual code which uses the new API follow. --- api/api-rules/violation_exceptions.list | 1 + pkg/apis/storage/fuzzer/fuzzer.go | 37 ++++ pkg/apis/storage/types.go | 39 ++++ pkg/apis/storage/v1beta1/defaults.go | 5 + pkg/apis/storage/v1beta1/defaults_test.go | 30 ++++ pkg/registry/storage/csidriver/strategy.go | 17 +- .../storage/csidriver/strategy_test.go | 167 ++++++++++++++++++ .../src/k8s.io/api/storage/v1beta1/types.go | 49 +++++ 8 files changed, 343 insertions(+), 2 deletions(-) diff --git a/api/api-rules/violation_exceptions.list b/api/api-rules/violation_exceptions.list index b39a57e9212..be1f4356458 100644 --- a/api/api-rules/violation_exceptions.list +++ b/api/api-rules/violation_exceptions.list @@ -344,6 +344,7 @@ API rule violation: list_type_missing,k8s.io/api/storage/v1,StorageClassList,Ite API rule violation: list_type_missing,k8s.io/api/storage/v1,VolumeAttachmentList,Items API rule violation: list_type_missing,k8s.io/api/storage/v1alpha1,VolumeAttachmentList,Items API rule violation: list_type_missing,k8s.io/api/storage/v1beta1,CSIDriverList,Items +API rule violation: list_type_missing,k8s.io/api/storage/v1beta1,CSIDriverSpec,VolumeLifecycleModes API rule violation: list_type_missing,k8s.io/api/storage/v1beta1,CSINodeDriver,TopologyKeys API rule violation: list_type_missing,k8s.io/api/storage/v1beta1,CSINodeList,Items API rule violation: list_type_missing,k8s.io/api/storage/v1beta1,CSINodeSpec,Drivers diff --git a/pkg/apis/storage/fuzzer/fuzzer.go b/pkg/apis/storage/fuzzer/fuzzer.go index 2081ffe846f..ea1de3ddacf 100644 --- a/pkg/apis/storage/fuzzer/fuzzer.go +++ b/pkg/apis/storage/fuzzer/fuzzer.go @@ -17,6 +17,7 @@ limitations under the License. package fuzzer import ( + "fmt" fuzz "github.com/google/gofuzz" runtimeserializer "k8s.io/apimachinery/pkg/runtime/serializer" @@ -37,6 +38,37 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { func(obj *storage.CSIDriver, c fuzz.Continue) { c.FuzzNoCustom(obj) // fuzz self without calling this function again + // Custom fuzzing for volume modes. + switch c.Rand.Intn(7) { + case 0: + obj.Spec.VolumeLifecycleModes = nil + case 1: + obj.Spec.VolumeLifecycleModes = []storage.VolumeLifecycleMode{} + case 2: + // Invalid mode. + obj.Spec.VolumeLifecycleModes = []storage.VolumeLifecycleMode{ + storage.VolumeLifecycleMode(fmt.Sprintf("%d", c.Rand.Int31())), + } + case 3: + obj.Spec.VolumeLifecycleModes = []storage.VolumeLifecycleMode{ + storage.VolumeLifecyclePersistent, + } + case 4: + obj.Spec.VolumeLifecycleModes = []storage.VolumeLifecycleMode{ + storage.VolumeLifecycleEphemeral, + } + case 5: + obj.Spec.VolumeLifecycleModes = []storage.VolumeLifecycleMode{ + storage.VolumeLifecyclePersistent, + storage.VolumeLifecycleEphemeral, + } + case 6: + obj.Spec.VolumeLifecycleModes = []storage.VolumeLifecycleMode{ + storage.VolumeLifecycleEphemeral, + storage.VolumeLifecyclePersistent, + } + } + // match defaulting if obj.Spec.AttachRequired == nil { obj.Spec.AttachRequired = new(bool) @@ -46,6 +78,11 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { obj.Spec.PodInfoOnMount = new(bool) *(obj.Spec.PodInfoOnMount) = false } + if obj.Spec.VolumeLifecycleModes == nil { + obj.Spec.VolumeLifecycleModes = []storage.VolumeLifecycleMode{ + storage.VolumeLifecyclePersistent, + } + } }, } } diff --git a/pkg/apis/storage/types.go b/pkg/apis/storage/types.go index 9a77746a2be..70851be6cda 100644 --- a/pkg/apis/storage/types.go +++ b/pkg/apis/storage/types.go @@ -293,8 +293,47 @@ type CSIDriverSpec struct { // "csi.storage.k8s.io/pod.uid": string(pod.UID) // +optional PodInfoOnMount *bool + + // 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 mode and + // more modes may be added in the future. + // +optional + VolumeLifecycleModes []VolumeLifecycleMode } +// VolumeLifecycleMode specifies how a CSI volume is used in Kubernetes. +// More modes may be added in the future. +type VolumeLifecycleMode string + +const ( + // VolumeLifecyclePersistent explicitly confirms that the driver implements + // the full CSI spec. It is the default when CSIDriverSpec.VolumeLifecycleModes is not + // set. Such volumes are managed in Kubernetes via the persistent volume + // claim mechanism and have a lifecycle that is independent of the pods which + // use them. + VolumeLifecyclePersistent VolumeLifecycleMode = "Persistent" + // VolumeLifecycleEphemeral indicates that the driver can be used for + // ephemeral inline volumes. Such volumes are specified inside the pod + // spec with a CSIVolumeSource and, as far as Kubernetes is concerned, have + // a lifecycle that is tied to the lifecycle of the pod. For example, such + // a volume might contain data that gets created specifically for that pod, + // like secrets. + // But how the volume actually gets created and managed is entirely up to + // the driver. It might also use reference counting to share the same volume + // instance among different pods if the CSIVolumeSource of those pods is + // identical. + VolumeLifecycleEphemeral VolumeLifecycleMode = "Ephemeral" +) + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // CSINode holds information about all CSI drivers installed on a node. diff --git a/pkg/apis/storage/v1beta1/defaults.go b/pkg/apis/storage/v1beta1/defaults.go index 30803281659..50cc4fe15c0 100644 --- a/pkg/apis/storage/v1beta1/defaults.go +++ b/pkg/apis/storage/v1beta1/defaults.go @@ -20,6 +20,8 @@ import ( "k8s.io/api/core/v1" storagev1beta1 "k8s.io/api/storage/v1beta1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kubernetes/pkg/features" ) func addDefaultingFuncs(scheme *runtime.Scheme) error { @@ -47,4 +49,7 @@ func SetDefaults_CSIDriver(obj *storagev1beta1.CSIDriver) { obj.Spec.PodInfoOnMount = new(bool) *(obj.Spec.PodInfoOnMount) = false } + if len(obj.Spec.VolumeLifecycleModes) == 0 && utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) { + obj.Spec.VolumeLifecycleModes = append(obj.Spec.VolumeLifecycleModes, storagev1beta1.VolumeLifecyclePersistent) + } } diff --git a/pkg/apis/storage/v1beta1/defaults_test.go b/pkg/apis/storage/v1beta1/defaults_test.go index d1cebcf7b90..a76efa5de24 100644 --- a/pkg/apis/storage/v1beta1/defaults_test.go +++ b/pkg/apis/storage/v1beta1/defaults_test.go @@ -22,8 +22,11 @@ import ( storagev1beta1 "k8s.io/api/storage/v1beta1" "k8s.io/apimachinery/pkg/runtime" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/api/legacyscheme" _ "k8s.io/kubernetes/pkg/apis/storage/install" + "k8s.io/kubernetes/pkg/features" ) func roundTrip(t *testing.T, obj runtime.Object) runtime.Object { @@ -81,3 +84,30 @@ func TestSetDefaultAttachRequired(t *testing.T) { t.Errorf("Expected PodInfoOnMount to be defaulted to: %+v, got: %+v", defaultPodInfo, outPodInfo) } } + +func TestSetDefaultVolumeLifecycleModesEnabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, true)() + driver := &storagev1beta1.CSIDriver{} + + // field should be defaulted + defaultMode := storagev1beta1.VolumeLifecyclePersistent + output := roundTrip(t, runtime.Object(driver)).(*storagev1beta1.CSIDriver) + outModes := output.Spec.VolumeLifecycleModes + if len(outModes) != 1 { + t.Errorf("Expected VolumeLifecycleModes to be defaulted to: %+v, got: %+v", defaultMode, outModes) + } else if outModes[0] != defaultMode { + t.Errorf("Expected VolumeLifecycleModes to be defaulted to: %+v, got: %+v", defaultMode, outModes) + } +} + +func TestSetDefaultVolumeLifecycleModesDisabled(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, false)() + driver := &storagev1beta1.CSIDriver{} + + // field should not be defaulted + output := roundTrip(t, runtime.Object(driver)).(*storagev1beta1.CSIDriver) + outModes := output.Spec.VolumeLifecycleModes + if outModes != nil { + t.Errorf("Expected VolumeLifecycleModes to remain nil, got: %+v", outModes) + } +} diff --git a/pkg/registry/storage/csidriver/strategy.go b/pkg/registry/storage/csidriver/strategy.go index 1f534b2e764..a968fcbaf17 100644 --- a/pkg/registry/storage/csidriver/strategy.go +++ b/pkg/registry/storage/csidriver/strategy.go @@ -22,9 +22,11 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/apis/storage/validation" + "k8s.io/kubernetes/pkg/features" ) // csiDriverStrategy implements behavior for CSIDriver objects @@ -41,8 +43,12 @@ func (csiDriverStrategy) NamespaceScoped() bool { return false } -// ResetBeforeCreate clears the Status field which is not allowed to be set by end users on creation. +// PrepareForCreate clears the VolumeLifecycleModes field if the corresponding feature is disabled. func (csiDriverStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { + if !utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) { + csiDriver := obj.(*storage.CSIDriver) + csiDriver.Spec.VolumeLifecycleModes = nil + } } func (csiDriverStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { @@ -62,8 +68,15 @@ func (csiDriverStrategy) AllowCreateOnUpdate() bool { return false } -// PrepareForUpdate sets the Status fields which is not allowed to be set by an end user updating a CSIDriver +// PrepareForUpdate clears the VolumeLifecycleModes field if the corresponding feature is disabled and +// existing object does not already have that field set. This allows the field to remain when +// downgrading to a version that has the feature disabled. func (csiDriverStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + if old.(*storage.CSIDriver).Spec.VolumeLifecycleModes == nil && + !utilfeature.DefaultFeatureGate.Enabled(features.CSIInlineVolume) { + newCSIDriver := obj.(*storage.CSIDriver) + newCSIDriver.Spec.VolumeLifecycleModes = nil + } } func (csiDriverStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { diff --git a/pkg/registry/storage/csidriver/strategy_test.go b/pkg/registry/storage/csidriver/strategy_test.go index e5334fc1edc..442b1005e8d 100644 --- a/pkg/registry/storage/csidriver/strategy_test.go +++ b/pkg/registry/storage/csidriver/strategy_test.go @@ -19,10 +19,14 @@ package csidriver import ( "testing" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/apis/storage" + "k8s.io/kubernetes/pkg/features" ) func getValidCSIDriver(name string) *storage.CSIDriver { @@ -74,6 +78,169 @@ func TestCSIDriverStrategy(t *testing.T) { } } +func TestCSIDriverPrepareForCreate(t *testing.T) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ + APIGroup: "storage.k8s.io", + APIVersion: "v1beta1", + Resource: "csidrivers", + }) + + attachRequired := true + podInfoOnMount := true + csiDriver := &storage.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachRequired, + PodInfoOnMount: &podInfoOnMount, + VolumeLifecycleModes: []storage.VolumeLifecycleMode{ + storage.VolumeLifecyclePersistent, + }, + }, + } + + tests := []struct { + name string + withInline bool + }{ + { + name: "inline enabled", + withInline: true, + }, + { + name: "inline disabled", + withInline: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, test.withInline)() + + Strategy.PrepareForCreate(ctx, csiDriver) + errs := Strategy.Validate(ctx, csiDriver) + if len(errs) != 0 { + t.Errorf("unexpected validating errors: %v", errs) + } + if test.withInline { + if len(csiDriver.Spec.VolumeLifecycleModes) != 1 { + t.Errorf("VolumeLifecycleModes modified: %v", csiDriver.Spec) + } + } else { + if len(csiDriver.Spec.VolumeLifecycleModes) != 0 { + t.Errorf("VolumeLifecycleModes not stripped: %v", csiDriver.Spec) + } + } + }) + } +} + +func TestCSIDriverPrepareForUpdate(t *testing.T) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ + APIGroup: "storage.k8s.io", + APIVersion: "v1beta1", + Resource: "csidrivers", + }) + + attachRequired := true + podInfoOnMount := true + driverWithoutModes := &storage.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachRequired, + PodInfoOnMount: &podInfoOnMount, + }, + } + driverWithPersistent := &storage.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachRequired, + PodInfoOnMount: &podInfoOnMount, + VolumeLifecycleModes: []storage.VolumeLifecycleMode{ + storage.VolumeLifecyclePersistent, + }, + }, + } + driverWithEphemeral := &storage.CSIDriver{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSIDriverSpec{ + AttachRequired: &attachRequired, + PodInfoOnMount: &podInfoOnMount, + VolumeLifecycleModes: []storage.VolumeLifecycleMode{ + storage.VolumeLifecycleEphemeral, + }, + }, + } + var resultEmpty []storage.VolumeLifecycleMode + resultPersistent := []storage.VolumeLifecycleMode{storage.VolumeLifecyclePersistent} + resultEphemeral := []storage.VolumeLifecycleMode{storage.VolumeLifecycleEphemeral} + + tests := []struct { + name string + old, update *storage.CSIDriver + withInline, withoutInline []storage.VolumeLifecycleMode + }{ + { + name: "before: no mode, update: no mode", + old: driverWithoutModes, + update: driverWithoutModes, + withInline: resultEmpty, + withoutInline: resultEmpty, + }, + { + name: "before: no mode, update: persistent", + old: driverWithoutModes, + update: driverWithPersistent, + withInline: resultPersistent, + withoutInline: resultEmpty, + }, + { + name: "before: persistent, update: ephemeral", + old: driverWithPersistent, + update: driverWithEphemeral, + withInline: resultEphemeral, + withoutInline: resultEphemeral, + }, + { + name: "before: persistent, update: no mode", + old: driverWithPersistent, + update: driverWithoutModes, + withInline: resultEmpty, + withoutInline: resultEmpty, + }, + } + + runAll := func(t *testing.T, withInline bool) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIInlineVolume, withInline)() + + csiDriver := test.update.DeepCopy() + Strategy.PrepareForUpdate(ctx, csiDriver, test.old) + if withInline { + require.Equal(t, csiDriver.Spec.VolumeLifecycleModes, test.withInline) + } else { + require.Equal(t, csiDriver.Spec.VolumeLifecycleModes, test.withoutInline) + } + }) + } + } + + t.Run("with inline volumes", func(t *testing.T) { + runAll(t, true) + }) + t.Run("without inline volumes", func(t *testing.T) { + runAll(t, false) + }) +} + func TestCSIDriverValidation(t *testing.T) { attachRequired := true notAttachRequired := false diff --git a/staging/src/k8s.io/api/storage/v1beta1/types.go b/staging/src/k8s.io/api/storage/v1beta1/types.go index 762fcfcd001..130f6a4c01e 100644 --- a/staging/src/k8s.io/api/storage/v1beta1/types.go +++ b/staging/src/k8s.io/api/storage/v1beta1/types.go @@ -291,10 +291,59 @@ type CSIDriverSpec struct { // "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" iff the volume is an ephemeral inline volume + // defined by a CSIVolumeSource, otherwise "false" + // + // "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. // +optional PodInfoOnMount *bool `json:"podInfoOnMount,omitempty" protobuf:"bytes,2,opt,name=podInfoOnMount"` + + // 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. + // +optional + VolumeLifecycleModes []VolumeLifecycleMode `json:"volumeLifecycleModes,omitempty" protobuf:"bytes,3,opt,name=volumeLifecycleModes"` } +// VolumeLifecycleMode is an enumeration of possible usage modes for a volume +// provided by a CSI driver. More modes may be added in the future. +type VolumeLifecycleMode string + +const ( + // VolumeLifecyclePersistent explicitly confirms that the driver implements + // the full CSI spec. It is the default when CSIDriverSpec.VolumeLifecycleModes is not + // set. Such volumes are managed in Kubernetes via the persistent volume + // claim mechanism and have a lifecycle that is independent of the pods which + // use them. + VolumeLifecyclePersistent VolumeLifecycleMode = "Persistent" + + // VolumeLifecycleEphemeral indicates that the driver can be used for + // ephemeral inline volumes. Such volumes are specified inside the pod + // spec with a CSIVolumeSource and, as far as Kubernetes is concerned, have + // a lifecycle that is tied to the lifecycle of the pod. For example, such + // a volume might contain data that gets created specifically for that pod, + // like secrets. + // But how the volume actually gets created and managed is entirely up to + // the driver. It might also use reference counting to share the same volume + // instance among different pods if the CSIVolumeSource of those pods is + // identical. + VolumeLifecycleEphemeral VolumeLifecycleMode = "Ephemeral" +) + // +genclient // +genclient:nonNamespaced // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object