Skip to content

Commit

Permalink
enable mount prop
Browse files Browse the repository at this point in the history
run update schema

add units
  • Loading branch information
elijah-rou committed Feb 12, 2025
1 parent 6265a8e commit e943d8a
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 20 deletions.
4 changes: 4 additions & 0 deletions cmd/schema-tweak/overrides.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@ func revSpecOverrides(prefixPath string) []entry {
"mountPath",
"subPath",
),
featureFlagFields: []flagField{{
name: "mountPropagation",
flag: config.FeaturePodSpecMountPropagation,
}},
}, {
path: "volumes",
allowedFields: sets.New(
Expand Down
4 changes: 4 additions & 0 deletions config/core/300-resources/configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions config/core/300-resources/revision.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions config/core/300-resources/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions config/core/configmaps/features.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/config/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -99,6 +100,7 @@ func defaultFeaturesConfig() *Features {
PodSpecTolerations: Disabled,
PodSpecVolumesEmptyDir: Enabled,
PodSpecVolumesHostPath: Disabled,
PodSpecVolumeMountPropagation: Disabled,
PodSpecPersistentVolumeClaim: Disabled,
PodSpecPersistentVolumeWrite: Disabled,
QueueProxyMountPodInfo: Disabled,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -181,6 +184,7 @@ type Features struct {
PodSpecTolerations Flag
PodSpecVolumesEmptyDir Flag
PodSpecVolumesHostPath Flag
PodSpecVolumeMountPropagation Flag
PodSpecInitContainers Flag
PodSpecPersistentVolumeClaim Flag
PodSpecPersistentVolumeWrite Flag
Expand Down
18 changes: 18 additions & 0 deletions pkg/apis/config/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 6 additions & 2 deletions pkg/apis/serving/fieldmask.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,22 +333,26 @@ 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
out.Name = in.Name
out.ReadOnly = in.ReadOnly
out.MountPath = in.MountPath
out.SubPath = in.SubPath
if cfg.Features.PodSpecVolumeMountPropagation != config.Disabled {
out.MountPropagation = in.MountPropagation
}

// Disallowed fields
// This list is unnecessary, but added here for clarity
out.MountPropagation = nil
out.RecursiveReadOnly = nil

return out
}
Expand Down
16 changes: 13 additions & 3 deletions pkg/apis/serving/k8s_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ 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"))
errs = errs.Also(validateVolumeMounts(ctx, container.VolumeMounts, volumes).ViaField("volumeMounts"))

return errs
}
Expand Down Expand Up @@ -659,15 +659,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) *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{
Expand Down Expand Up @@ -700,6 +701,15 @@ func validateVolumeMounts(mounts []corev1.VolumeMount, volumes map[string]corev1
}).ViaIndex(i))
}

if vm.MountPropagation != nil {
if *vm.MountPropagation != corev1.MountPropagationNone || *vm.MountPropagation != corev1.MountPropagationHostToContainer || *vm.MountPropagation != corev1.MountPropagationBidirectional {
errs = errs.Also((&apis.FieldError{
Message: "mount propagation should be set to None, HostToContainer or Bidirectional",
Paths: []string{"mountPropagation"},
}).ViaIndex(i))
}
}

if volumes[vm.Name].PersistentVolumeClaim != nil {
if volumes[vm.Name].PersistentVolumeClaim.ReadOnly && !vm.ReadOnly {
errs = errs.Also((&apis.FieldError{
Expand Down
55 changes: 55 additions & 0 deletions pkg/apis/serving/k8s_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -647,6 +654,54 @@ func TestPodSpecValidation(t *testing.T) {
}},
},
cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecPersistentVolumeWriteEnabled()},
}, {
name: "mount prop disabled",
ps: corev1.PodSpec{
Containers: []corev1.Container{{
Image: "busybox",
VolumeMounts: []corev1.VolumeMount{{
Name: "foo",
MountPath: "/data",
ReadOnly: true,
MountPropagation: &corev1.MountPropagationHostToContainer,
}},
}},
Volumes: []corev1.Volume{{
Name: "foo",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "myclaim",
},
},
}},
},
cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled()},
want: (&apis.FieldError{
Message: `duplicate volume name "the-name"`,
Paths: []string{"name"},
}).ViaFieldIndex("volumes", 1),
}, {
name: "mount prop enabled",
ps: corev1.PodSpec{
Containers: []corev1.Container{{
Image: "busybox",
VolumeMounts: []corev1.VolumeMount{{
Name: "foo",
MountPath: "/data",
ReadOnly: true,
MountPropagation: &corev1.MountPropagationHostToContainer,
}},
}},
Volumes: []corev1.Volume{{
Name: "foo",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "myclaim",
},
},
}},
},
cfgOpts: []configOption{withPodSpecPersistentVolumeClaimEnabled(), withPodSpecMountPropagationEnabled()},
}, {
name: "insecure security context default struct",
ps: corev1.PodSpec{
Expand Down
31 changes: 16 additions & 15 deletions pkg/reconciler/route/resources/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
}

0 comments on commit e943d8a

Please sign in to comment.