From f81c48cd0ae86da76720f4746802e5708823cdf9 Mon Sep 17 00:00:00 2001 From: Lee Verberne Date: Sat, 16 Oct 2021 07:02:10 -0700 Subject: [PATCH] Disallow subpath for ephemeral container mounts --- pkg/apis/core/types.go | 1 + pkg/apis/core/validation/validation.go | 12 +++++++ pkg/apis/core/validation/validation_test.go | 36 +++++++++++++++++++++ staging/src/k8s.io/api/core/v1/types.go | 2 +- 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index d0a0ce57125..7a690f64b19 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -3137,6 +3137,7 @@ type EphemeralContainerCommon struct { // already allocated to the pod. // +optional Resources ResourceRequirements + // Pod volumes to mount into the container's filesystem. Subpath mounts are not allowed for ephemeral containers. // +optional VolumeMounts []VolumeMount // volumeDevices is the list of block devices to be used by the container. diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 62bd05a68e1..11fcb32908b 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -2873,6 +2873,18 @@ func validateEphemeralContainers(ephemeralContainers []core.EphemeralContainer, // Lifecycle, probes, resources and ports should be disallowed. This is implemented as a list // of allowed fields so that new fields will be given consideration prior to inclusion in Ephemeral Containers. allErrs = append(allErrs, validateFieldAllowList(ec.EphemeralContainerCommon, allowedEphemeralContainerFields, "cannot be set for an Ephemeral Container", idxPath)...) + + // VolumeMount subpaths have the potential to leak resources since they're implemented with bind mounts + // that aren't cleaned up until the pod exits. Since they also imply that the container is being used + // as part of the workload, they're disallowed entirely. + for i, vm := range ec.VolumeMounts { + if vm.SubPath != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPath"), "cannot be set for an Ephemeral Container")) + } + if vm.SubPathExpr != "" { + allErrs = append(allErrs, field.Forbidden(idxPath.Child("volumeMounts").Index(i).Child("subPathExpr"), "cannot be set for an Ephemeral Container")) + } + } } return allErrs diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 0da7ec7896a..91c4e05acb4 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -6360,6 +6360,42 @@ func TestValidateEphemeralContainers(t *testing.T) { }, field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].resources"}, }, + { + "Container uses disallowed field: VolumeMount.SubPath", + []core.EphemeralContainer{ + { + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + VolumeMounts: []core.VolumeMount{ + {Name: "vol", MountPath: "/vol"}, + {Name: "vol", MountPath: "/volsub", SubPath: "foo"}, + }, + }, + }, + }, + field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPath"}, + }, + { + "Container uses disallowed field: VolumeMount.SubPathExpr", + []core.EphemeralContainer{ + { + EphemeralContainerCommon: core.EphemeralContainerCommon{ + Name: "debug", + Image: "image", + ImagePullPolicy: "IfNotPresent", + TerminationMessagePolicy: "File", + VolumeMounts: []core.VolumeMount{ + {Name: "vol", MountPath: "/vol"}, + {Name: "vol", MountPath: "/volsub", SubPathExpr: "$(POD_NAME)"}, + }, + }, + }, + }, + field.Error{Type: field.ErrorTypeForbidden, Field: "ephemeralContainers[0].volumeMounts[1].subPathExpr"}, + }, } for _, tc := range tcs { diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index c5325d71b21..b7fba95f446 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -3500,7 +3500,7 @@ type EphemeralContainerCommon struct { // already allocated to the pod. // +optional Resources ResourceRequirements `json:"resources,omitempty" protobuf:"bytes,8,opt,name=resources"` - // Pod volumes to mount into the container's filesystem. + // Pod volumes to mount into the container's filesystem. Subpath mounts are not allowed for ephemeral containers. // Cannot be updated. // +optional // +patchMergeKey=mountPath