Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows host process work #99576

Merged
merged 5 commits into from May 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions api/openapi-spec/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 28 additions & 0 deletions pkg/api/pod/util.go
Expand Up @@ -401,6 +401,7 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
AllowInvalidPodDeletionCost: !utilfeature.DefaultFeatureGate.Enabled(features.PodDeletionCost),
// Do not allow pod spec to use non-integer multiple of huge page unit size default
AllowIndivisibleHugePagesValues: false,
AllowWindowsHostProcessField: utilfeature.DefaultFeatureGate.Enabled(features.WindowsHostProcessContainers),
}

if oldPodSpec != nil {
Expand All @@ -415,6 +416,8 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
return !opts.AllowDownwardAPIHugePages
})
}
// if old spec has Windows Host Process fields set, we must allow it
opts.AllowWindowsHostProcessField = opts.AllowWindowsHostProcessField || setsWindowsHostProcess(oldPodSpec)

// if old spec used non-integer multiple of huge page unit size, we must allow it
opts.AllowIndivisibleHugePagesValues = usesIndivisibleHugePagesValues(oldPodSpec)
Expand Down Expand Up @@ -944,3 +947,28 @@ func SeccompFieldForAnnotation(annotation string) *api.SeccompProfile {
// length or if the annotation has an unrecognized value
return nil
}

// setsWindowsHostProcess returns true if WindowsOptions.HostProcess is set (true or false)
// anywhere in the pod spec.
func setsWindowsHostProcess(podSpec *api.PodSpec) bool {
if podSpec == nil {
return false
}

// Check Pod's WindowsOptions.HostProcess
if podSpec.SecurityContext != nil && podSpec.SecurityContext.WindowsOptions != nil && podSpec.SecurityContext.WindowsOptions.HostProcess != nil {
return true
}

// Check WindowsOptions.HostProcess for each container
inUse := false
VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
if c.SecurityContext != nil && c.SecurityContext.WindowsOptions != nil && c.SecurityContext.WindowsOptions.HostProcess != nil {
inUse = true
return false
}
return true
})

return inUse
}
10 changes: 10 additions & 0 deletions pkg/apis/core/types.go
Expand Up @@ -5351,6 +5351,16 @@ type WindowsSecurityContextOptions struct {
// PodSecurityContext, the value specified in SecurityContext takes precedence.
// +optional
RunAsUserName *string

// HostProcess determines if a container should be run as a 'Host Process' container.
// This field is alpha-level and will only be honored by components that enable the
marosset marked this conversation as resolved.
Show resolved Hide resolved
// WindowsHostProcessContainers feature flag. Setting this field without the feature
// flag will result in errors when validating the Pod. All of a Pod's containers must
// have the same effective HostProcess value (it is not allowed to have a mix of HostProcess
// containers and non-HostProcess containers). In addition, if HostProcess is true
// then HostNetwork must also be set to true.
// +optional
HostProcess *bool
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/core/v1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

88 changes: 88 additions & 0 deletions pkg/apis/core/validation/validation.go
Expand Up @@ -3204,6 +3204,8 @@ type PodValidationOptions struct {
AllowInvalidPodDeletionCost bool
// Allow pod spec to use non-integer multiple of huge page unit size
AllowIndivisibleHugePagesValues bool
// Allow hostProcess field to be set in windows security context
AllowWindowsHostProcessField bool
}

// ValidatePodSingleHugePageResources checks if there are multiple huge
Expand Down Expand Up @@ -3327,6 +3329,7 @@ func ValidatePodSpec(spec *core.PodSpec, podMeta *metav1.ObjectMeta, fldPath *fi
allErrs = append(allErrs, validatePodDNSConfig(spec.DNSConfig, &spec.DNSPolicy, fldPath.Child("dnsConfig"))...)
allErrs = append(allErrs, validateReadinessGates(spec.ReadinessGates, fldPath.Child("readinessGates"))...)
allErrs = append(allErrs, validateTopologySpreadConstraints(spec.TopologySpreadConstraints, fldPath.Child("topologySpreadConstraints"))...)
allErrs = append(allErrs, validateWindowsHostProcessPod(spec, fldPath, opts)...)
if len(spec.ServiceAccountName) > 0 {
for _, msg := range ValidateServiceAccountName(spec.ServiceAccountName, false) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceAccountName"), spec.ServiceAccountName, msg))
Expand Down Expand Up @@ -5974,6 +5977,91 @@ func validateWindowsSecurityContextOptions(windowsOptions *core.WindowsSecurityC
return allErrs
}

func validateWindowsHostProcessPod(podSpec *core.PodSpec, fieldPath *field.Path, opts PodValidationOptions) field.ErrorList {
liggitt marked this conversation as resolved.
Show resolved Hide resolved
allErrs := field.ErrorList{}

// Keep track of container and hostProcess container count for validate
containerCount := 0
hostProcessContainerCount := 0

var podHostProcess *bool
liggitt marked this conversation as resolved.
Show resolved Hide resolved
if podSpec.SecurityContext != nil && podSpec.SecurityContext.WindowsOptions != nil {
podHostProcess = podSpec.SecurityContext.WindowsOptions.HostProcess
}

if !opts.AllowWindowsHostProcessField && podHostProcess != nil {
// Do not allow pods to persist data that sets hostProcess (true or false)
errMsg := "not allowed when feature gate 'WindowsHostProcessContainers' is not enabled"
allErrs = append(allErrs, field.Forbidden(fieldPath.Child("securityContext", "windowsOptions", "hostProcess"), errMsg))
return allErrs
}

hostNetwork := false
if podSpec.SecurityContext != nil {
hostNetwork = podSpec.SecurityContext.HostNetwork
}

podshelper.VisitContainersWithPath(podSpec, fieldPath, func(c *core.Container, cFieldPath *field.Path) bool {
containerCount++
liggitt marked this conversation as resolved.
Show resolved Hide resolved

var containerHostProcess *bool = nil
if c.SecurityContext != nil && c.SecurityContext.WindowsOptions != nil {
containerHostProcess = c.SecurityContext.WindowsOptions.HostProcess
}

if !opts.AllowWindowsHostProcessField && containerHostProcess != nil {
// Do not allow pods to persist data that sets hostProcess (true or false)
errMsg := "not allowed when feature gate 'WindowsHostProcessContainers' is not enabled"
allErrs = append(allErrs, field.Forbidden(cFieldPath.Child("securityContext", "windowsOptions", "hostProcess"), errMsg))
}

if podHostProcess != nil && containerHostProcess != nil && *podHostProcess != *containerHostProcess {
errMsg := fmt.Sprintf("pod hostProcess value must be identical if both are specified, was %v", *podHostProcess)
allErrs = append(allErrs, field.Invalid(cFieldPath.Child("securityContext", "windowsOptions", "hostProcess"), *containerHostProcess, errMsg))
}

switch {
case containerHostProcess != nil && *containerHostProcess:
// Container explitly sets hostProcess=true
hostProcessContainerCount++
case containerHostProcess == nil && podHostProcess != nil && *podHostProcess:
// Container inherits hostProcess=true from pod settings
hostProcessContainerCount++
}

return true
})

if hostProcessContainerCount > 0 {
liggitt marked this conversation as resolved.
Show resolved Hide resolved
// Fail Pod validation if feature is not enabled (unless podspec already exists and contains HostProcess fields) instead of dropping fields based on PRR reivew.
if !opts.AllowWindowsHostProcessField {
errMsg := "pod must not contain Windows hostProcess containers when feature gate 'WindowsHostProcessContainers' is not enabled"
allErrs = append(allErrs, field.Forbidden(fieldPath, errMsg))
liggitt marked this conversation as resolved.
Show resolved Hide resolved
return allErrs
}

// At present, if a Windows Pods contains any HostProcess containers than all containers must be
// HostProcess containers (explicitly set or inherited).
if hostProcessContainerCount != containerCount {
errMsg := "If pod contains any hostProcess containers then all containers must be HostProcess containers"
allErrs = append(allErrs, field.Invalid(fieldPath, "", errMsg))
}

// At present Windows Pods which contain HostProcess containers must also set HostNetwork.
if hostNetwork != true {
errMsg := "hostNetwork must be true if pod contains any hostProcess containers"
allErrs = append(allErrs, field.Invalid(fieldPath.Child("hostNetwork"), hostNetwork, errMsg))
}

if !capabilities.Get().AllowPrivileged {
errMsg := "hostProcess containers are disallowed by cluster policy"
allErrs = append(allErrs, field.Forbidden(fieldPath, errMsg))
}
}

return allErrs
}

func ValidatePodLogOptions(opts *core.PodLogOptions) field.ErrorList {
allErrs := field.ErrorList{}
if opts.TailLines != nil && *opts.TailLines < 0 {
Expand Down