diff --git a/cmd/schema-tweak/overrides.go b/cmd/schema-tweak/overrides.go index 4089a4ffc6ab..ae50384ee7ce 100644 --- a/cmd/schema-tweak/overrides.go +++ b/cmd/schema-tweak/overrides.go @@ -227,6 +227,10 @@ func revSpecOverrides(prefixPath string) []entry { "mountPath", "subPath", ), + featureFlagFields: []flagField{{ + name: "mountPropagation", + flag: config.FeaturePodSpecMountPropagation, + }}, }, { path: "volumes", allowedFields: sets.New( diff --git a/config/core/300-resources/configuration.yaml b/config/core/300-resources/configuration.yaml index ce62641a5f9f..82bb0b2c3765 100644 --- a/config/core/300-resources/configuration.yaml +++ b/config/core/300-resources/configuration.yaml @@ -926,6 +926,10 @@ spec: Path within the container at which the volume should be mounted. Must not contain ':'. type: string + mountPropagation: + description: |- + This is accessible behind a feature flag - kubernetes.podspec-mount-propagation + type: string name: description: This must match the Name of a Volume. type: string diff --git a/config/core/300-resources/revision.yaml b/config/core/300-resources/revision.yaml index d0eb983f2efe..f7044ad684c4 100644 --- a/config/core/300-resources/revision.yaml +++ b/config/core/300-resources/revision.yaml @@ -902,6 +902,10 @@ spec: Path within the container at which the volume should be mounted. Must not contain ':'. type: string + mountPropagation: + description: |- + This is accessible behind a feature flag - kubernetes.podspec-mount-propagation + type: string name: description: This must match the Name of a Volume. type: string diff --git a/config/core/300-resources/service.yaml b/config/core/300-resources/service.yaml index 61d796396281..5b0c24dbd6bf 100644 --- a/config/core/300-resources/service.yaml +++ b/config/core/300-resources/service.yaml @@ -944,6 +944,10 @@ spec: Path within the container at which the volume should be mounted. Must not contain ':'. type: string + mountPropagation: + description: |- + This is accessible behind a feature flag - kubernetes.podspec-mount-propagation + type: string name: description: This must match the Name of a Volume. type: string diff --git a/config/core/configmaps/features.yaml b/config/core/configmaps/features.yaml index 51830a89b6d5..cc58fa9ff781 100644 --- a/config/core/configmaps/features.yaml +++ b/config/core/configmaps/features.yaml @@ -22,7 +22,7 @@ metadata: app.kubernetes.io/component: controller app.kubernetes.io/version: devel annotations: - knative.dev/example-checksum: "63a13754" + knative.dev/example-checksum: "f3931300" data: _example: |- ################################ @@ -223,6 +223,11 @@ data: # 2. Disabled: disabling write access for persistent volumes kubernetes.podspec-persistent-volume-write: "disabled" + # Controls whether volume mount propagation support is enabled or not. + # 1. Enabled: enabling volume mount propagation support + # 2. Disabled: disabling volume mount propagation support + kubernetes.podspec-mount-propagation: "disabled" + # Controls if the queue proxy podInfo feature is enabled, allowed or disabled # # This feature should be enabled/allowed when using queue proxy Options (Extensions) diff --git a/pkg/apis/config/features.go b/pkg/apis/config/features.go index 56db93419c74..84a348a3db73 100644 --- a/pkg/apis/config/features.go +++ b/pkg/apis/config/features.go @@ -66,6 +66,7 @@ const ( FeaturePodSpecHostPID = "kubernetes.podspec-hostpid" FeaturePodSpecHostPath = "kubernetes.podspec-volumes-hostpath" FeaturePodSpecInitContainers = "kubernetes.podspec-init-containers" + FeaturePodSpecMountPropagation = "kubernetes.podspec-mount-propagation" FeaturePodSpecNodeSelector = "kubernetes.podspec-nodeselector" FeaturePodSpecPVClaim = "kubernetes.podspec-persistent-volume-claim" FeaturePodSpecPriorityClassName = "kubernetes.podspec-priorityclassname" @@ -99,6 +100,7 @@ func defaultFeaturesConfig() *Features { PodSpecTolerations: Disabled, PodSpecVolumesEmptyDir: Enabled, PodSpecVolumesHostPath: Disabled, + PodSpecVolumeMountPropagation: Disabled, PodSpecPersistentVolumeClaim: Disabled, PodSpecPersistentVolumeWrite: Disabled, QueueProxyMountPodInfo: Disabled, @@ -139,6 +141,7 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) { asFlag(FeaturePodSpecHostPID, &nc.PodSpecHostPID), asFlag(FeaturePodSpecHostPath, &nc.PodSpecVolumesHostPath), asFlag(FeaturePodSpecInitContainers, &nc.PodSpecInitContainers), + asFlag(FeaturePodSpecMountPropagation, &nc.PodSpecVolumeMountPropagation), asFlag(FeaturePodSpecNodeSelector, &nc.PodSpecNodeSelector), asFlag(FeaturePodSpecPVClaim, &nc.PodSpecPersistentVolumeClaim), asFlag(FeaturePodSpecPriorityClassName, &nc.PodSpecPriorityClassName), @@ -181,6 +184,7 @@ type Features struct { PodSpecTolerations Flag PodSpecVolumesEmptyDir Flag PodSpecVolumesHostPath Flag + PodSpecVolumeMountPropagation Flag PodSpecInitContainers Flag PodSpecPersistentVolumeClaim Flag PodSpecPersistentVolumeWrite Flag diff --git a/pkg/apis/config/features_test.go b/pkg/apis/config/features_test.go index 6a1762d892cf..ec6a261193c2 100644 --- a/pkg/apis/config/features_test.go +++ b/pkg/apis/config/features_test.go @@ -473,6 +473,24 @@ func TestFeaturesConfiguration(t *testing.T) { data: map[string]string{ "kubernetes.podspec-persistent-volume-claim": "Enabled", }, + }, { + name: "kubernetes.podspec-mount-propagation Disabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecVolumeMountPropagation: Disabled, + }), + data: map[string]string{ + "kubernetes.podspec-mount-propagation": "Disabled", + }, + }, { + name: "kubernetes.podspec-mount-propagation Enabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecVolumeMountPropagation: Enabled, + }), + data: map[string]string{ + "kubernetes.podspec-mount-propagation": "Enabled", + }, }, { name: "kubernetes.podspec-persistent-volume-write Disabled", wantErr: false, diff --git a/pkg/apis/serving/fieldmask.go b/pkg/apis/serving/fieldmask.go index b1761e2387d2..e7e721dc3418 100644 --- a/pkg/apis/serving/fieldmask.go +++ b/pkg/apis/serving/fieldmask.go @@ -333,11 +333,12 @@ func ContainerMask(in *corev1.Container) *corev1.Container { // VolumeMountMask performs a _shallow_ copy of the Kubernetes VolumeMount object to a new // Kubernetes VolumeMount object bringing over only the fields allowed in the Knative API. This // does not validate the contents or the bounds of the provided fields. -func VolumeMountMask(in *corev1.VolumeMount) *corev1.VolumeMount { +func VolumeMountMask(ctx context.Context, in *corev1.VolumeMount) *corev1.VolumeMount { if in == nil { return nil } + cfg := config.FromContextOrDefaults(ctx) out := new(corev1.VolumeMount) // Allowed fields @@ -345,10 +346,15 @@ func VolumeMountMask(in *corev1.VolumeMount) *corev1.VolumeMount { out.ReadOnly = in.ReadOnly out.MountPath = in.MountPath out.SubPath = in.SubPath + if cfg.Features.PodSpecVolumeMountPropagation != config.Disabled { + out.MountPropagation = in.MountPropagation + } else { + out.MountPropagation = nil + } // Disallowed fields // This list is unnecessary, but added here for clarity - out.MountPropagation = nil + out.RecursiveReadOnly = nil return out } diff --git a/pkg/apis/serving/fieldmask_test.go b/pkg/apis/serving/fieldmask_test.go index b94f18c7efbe..884b56a449c1 100644 --- a/pkg/apis/serving/fieldmask_test.go +++ b/pkg/apis/serving/fieldmask_test.go @@ -315,7 +315,8 @@ func TestContainerMask(t *testing.T) { } func TestVolumeMountMask(t *testing.T) { - mode := corev1.MountPropagationBidirectional + mode := corev1.MountPropagationHostToContainer + ctx := context.Background() want := &corev1.VolumeMount{ Name: "foo", @@ -331,7 +332,51 @@ func TestVolumeMountMask(t *testing.T) { MountPropagation: &mode, } - got := VolumeMountMask(in) + got := VolumeMountMask(ctx, in) + + if &want == &got { + t.Error("Input and output share addresses. Want different addresses") + } + + if diff, err := kmp.SafeDiff(want, got); err != nil { + t.Error("Got error comparing output, err =", err) + } else if diff != "" { + t.Error("VolumeMountMask (-want, +got):", diff) + } + + if got = VolumeMountMask(ctx, nil); got != nil { + t.Errorf("VolumeMountMask(nil) = %v, want: nil", got) + } +} + +func TestVolumeMountMask_FeatMountPropagation(t *testing.T) { + mode := corev1.MountPropagationHostToContainer + rrr := corev1.RecursiveReadOnlyEnabled + ctx := config.ToContext(context.Background(), + &config.Config{ + Features: &config.Features{ + PodSpecVolumeMountPropagation: config.Enabled, + }, + }, + ) + + want := &corev1.VolumeMount{ + Name: "foo", + ReadOnly: true, + MountPath: "/foo/bar", + SubPath: "baz", + MountPropagation: &mode, + } + in := &corev1.VolumeMount{ + Name: "foo", + ReadOnly: true, + MountPath: "/foo/bar", + SubPath: "baz", + MountPropagation: &mode, + RecursiveReadOnly: &rrr, + } + + got := VolumeMountMask(ctx, in) if &want == &got { t.Error("Input and output share addresses. Want different addresses") @@ -343,7 +388,7 @@ func TestVolumeMountMask(t *testing.T) { t.Error("VolumeMountMask (-want, +got):", diff) } - if got = VolumeMountMask(nil); got != nil { + if got = VolumeMountMask(ctx, nil); got != nil { t.Errorf("VolumeMountMask(nil) = %v, want: nil", got) } } diff --git a/pkg/apis/serving/k8s_validation.go b/pkg/apis/serving/k8s_validation.go index ece260a9689b..a4ef695007e8 100644 --- a/pkg/apis/serving/k8s_validation.go +++ b/pkg/apis/serving/k8s_validation.go @@ -616,7 +616,8 @@ func validate(ctx context.Context, container corev1.Container, volumes map[strin errs = errs.Also(apis.ErrInvalidValue(container.TerminationMessagePolicy, "terminationMessagePolicy")) } // VolumeMounts - errs = errs.Also(validateVolumeMounts(container.VolumeMounts, volumes).ViaField("volumeMounts")) + isPrivilegedContainer := container.SecurityContext != nil && container.SecurityContext.Privileged != nil && *container.SecurityContext.Privileged + errs = errs.Also(validateVolumeMounts(ctx, container.VolumeMounts, volumes, isPrivilegedContainer).ViaField("volumeMounts")) return errs } @@ -659,15 +660,16 @@ func validateSecurityContext(ctx context.Context, sc *corev1.SecurityContext) *a return errs } -func validateVolumeMounts(mounts []corev1.VolumeMount, volumes map[string]corev1.Volume) *apis.FieldError { +func validateVolumeMounts(ctx context.Context, mounts []corev1.VolumeMount, volumes map[string]corev1.Volume, isPrivilegedContainer bool) *apis.FieldError { var errs *apis.FieldError // Check that volume mounts match names in "volumes", that "volumes" has 100% // coverage, and the field restrictions. + features := config.FromContextOrDefaults(ctx).Features seenName := make(sets.Set[string], len(mounts)) seenMountPath := make(sets.Set[string], len(mounts)) for i := range mounts { vm := mounts[i] - errs = errs.Also(apis.CheckDisallowedFields(vm, *VolumeMountMask(&vm)).ViaIndex(i)) + errs = errs.Also(apis.CheckDisallowedFields(vm, *VolumeMountMask(ctx, &vm)).ViaIndex(i)) // This effectively checks that Name is non-empty because Volume name must be non-empty. if _, ok := volumes[vm.Name]; !ok { errs = errs.Also((&apis.FieldError{ @@ -699,6 +701,19 @@ func validateVolumeMounts(mounts []corev1.VolumeMount, volumes map[string]corev1 Paths: []string{"readOnly"}, }).ViaIndex(i)) } + if vm.MountPropagation != nil { + if features.PodSpecVolumeMountPropagation != config.Enabled { + errs = errs.Also((&apis.FieldError{ + Message: fmt.Sprintf("Volume Mount Propagation support is disabled, but found volume mount %s with mount propagation", vm.Name), + }).ViaIndex(i)) + } + if *vm.MountPropagation != corev1.MountPropagationNone && *vm.MountPropagation != corev1.MountPropagationHostToContainer { + errs = errs.Also((&apis.FieldError{ + Message: "mount propagation should be set to None or HostToContainer", + Paths: []string{"mountPropagation"}, + }).ViaIndex(i)) + } + } if volumes[vm.Name].PersistentVolumeClaim != nil { if volumes[vm.Name].PersistentVolumeClaim.ReadOnly && !vm.ReadOnly { diff --git a/pkg/apis/serving/k8s_validation_test.go b/pkg/apis/serving/k8s_validation_test.go index e38fae560997..73328dcc52bf 100644 --- a/pkg/apis/serving/k8s_validation_test.go +++ b/pkg/apis/serving/k8s_validation_test.go @@ -143,6 +143,13 @@ func withPodSpecPersistentVolumeWriteEnabled() configOption { } } +func WithPodSpecMountPropagationEnabled() configOption { + return func(cfg *config.Config) *config.Config { + cfg.Features.PodSpecVolumeMountPropagation = config.Enabled + return cfg + } +} + func withPodSpecPriorityClassNameEnabled() configOption { return func(cfg *config.Config) *config.Config { cfg.Features.PodSpecPriorityClassName = config.Enabled @@ -193,6 +200,8 @@ func withPodSpecDNSConfigEnabled() configOption { } func TestPodSpecValidation(t *testing.T) { + bidir := corev1.MountPropagationBidirectional + hostToContainer := corev1.MountPropagationHostToContainer tests := []struct { name string ps corev1.PodSpec @@ -647,6 +656,78 @@ func TestPodSpecValidation(t *testing.T) { }}, }, cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecPersistentVolumeWriteEnabled()}, + }, { + name: "mount uses mountPropagation, but the feature is not enabled", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + MountPropagation: &hostToContainer, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + }, + }, + }}, + }, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecPersistentVolumeWriteEnabled()}, + want: &apis.FieldError{ + Message: "Volume Mount Propagation support is disabled, but found volume mount foo with mount propagation: \nmust not set the field(s)", + Paths: []string{"containers[0].volumeMounts[0].mountPropagation"}, + }, + }, { + name: "mount uses mountPropagation", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + MountPropagation: &hostToContainer, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + }, + }, + }}, + }, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecPersistentVolumeWriteEnabled(), WithPodSpecMountPropagationEnabled()}, + want: nil, + }, { + name: "mount uses mountPropagation Bidirectional, which is disallowed due to container privilege being disabled", + ps: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "busybox", + VolumeMounts: []corev1.VolumeMount{{ + Name: "foo", + MountPath: "/data", + MountPropagation: &bidir, + }}, + }}, + Volumes: []corev1.Volume{{ + Name: "foo", + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "myclaim", + }, + }, + }}, + }, + cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecPersistentVolumeWriteEnabled(), WithPodSpecMountPropagationEnabled()}, + want: &apis.FieldError{ + Message: "mount propagation should be set to None or HostToContainer", + Paths: []string{"containers[0].volumeMounts[0].mountPropagation"}, + }, }, { name: "insecure security context default struct", ps: corev1.PodSpec{ @@ -2409,7 +2490,6 @@ func TestInitContainerValidation(t *testing.T) { } func getCommonContainerValidationTestCases() []containerValidationTestCase { - bidir := corev1.MountPropagationBidirectional return []containerValidationTestCase{{ name: "empty container", c: corev1.Container{}, @@ -2535,9 +2615,8 @@ func getCommonContainerValidationTestCases() []containerValidationTestCase { c: corev1.Container{ Image: "foo", VolumeMounts: []corev1.VolumeMount{{ - Name: "the-name", - SubPath: "oops", - MountPropagation: &bidir, + Name: "the-name", + SubPath: "oops", }}, }, want: (&apis.FieldError{ @@ -2548,8 +2627,7 @@ func getCommonContainerValidationTestCases() []containerValidationTestCase { Message: "volume mount should be readOnly for this type of volume", Paths: []string{"readOnly"}, }).ViaFieldIndex("volumeMounts", 0)).Also( - apis.ErrMissingField("mountPath").ViaFieldIndex("volumeMounts", 0)).Also( - apis.ErrDisallowedFields("mountPropagation").ViaFieldIndex("volumeMounts", 0)), + apis.ErrMissingField("mountPath").ViaFieldIndex("volumeMounts", 0)), }, { name: "has known volumeMounts", c: corev1.Container{ diff --git a/pkg/reconciler/route/resources/service_test.go b/pkg/reconciler/route/resources/service_test.go index 829dfe914254..77e85a4e45e9 100644 --- a/pkg/reconciler/route/resources/service_test.go +++ b/pkg/reconciler/route/resources/service_test.go @@ -426,21 +426,22 @@ func testConfig() *config.Config { SystemInternalTLS: netcfg.EncryptionDisabled, }, Features: &apiConfig.Features{ - MultiContainer: apiConfig.Disabled, - PodSpecAffinity: apiConfig.Disabled, - PodSpecFieldRef: apiConfig.Disabled, - PodSpecDryRun: apiConfig.Enabled, - PodSpecHostAliases: apiConfig.Disabled, - PodSpecNodeSelector: apiConfig.Disabled, - PodSpecTolerations: apiConfig.Disabled, - PodSpecVolumesEmptyDir: apiConfig.Disabled, - PodSpecVolumesHostPath: apiConfig.Disabled, - PodSpecPersistentVolumeClaim: apiConfig.Disabled, - PodSpecPersistentVolumeWrite: apiConfig.Disabled, - PodSpecInitContainers: apiConfig.Disabled, - PodSpecPriorityClassName: apiConfig.Disabled, - PodSpecSchedulerName: apiConfig.Disabled, - TagHeaderBasedRouting: apiConfig.Disabled, + MultiContainer: apiConfig.Disabled, + PodSpecAffinity: apiConfig.Disabled, + PodSpecFieldRef: apiConfig.Disabled, + PodSpecDryRun: apiConfig.Enabled, + PodSpecHostAliases: apiConfig.Disabled, + PodSpecNodeSelector: apiConfig.Disabled, + PodSpecTolerations: apiConfig.Disabled, + PodSpecVolumesEmptyDir: apiConfig.Disabled, + PodSpecVolumesHostPath: apiConfig.Disabled, + PodSpecPersistentVolumeClaim: apiConfig.Disabled, + PodSpecPersistentVolumeWrite: apiConfig.Disabled, + PodSpecVolumeMountPropagation: apiConfig.Disabled, + PodSpecInitContainers: apiConfig.Disabled, + PodSpecPriorityClassName: apiConfig.Disabled, + PodSpecSchedulerName: apiConfig.Disabled, + TagHeaderBasedRouting: apiConfig.Disabled, }, } }