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

Promote pod OS to GA #111229

Merged
merged 3 commits into from Jul 19, 2022
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
2 changes: 1 addition & 1 deletion api/openapi-spec/swagger.json

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

2 changes: 1 addition & 1 deletion api/openapi-spec/v3/api__v1_openapi.json
Expand Up @@ -5083,7 +5083,7 @@
"$ref": "#/components/schemas/io.k8s.api.core.v1.PodOS"
}
],
"description": "Specifies the OS of the containers in the pod. Some pod and container fields are restricted if this is set.\n\nIf the OS field is set to linux, the following fields must be unset: -securityContext.windowsOptions\n\nIf the OS field is set to windows, following fields must be unset: - spec.hostPID - spec.hostIPC - spec.securityContext.seLinuxOptions - spec.securityContext.seccompProfile - spec.securityContext.fsGroup - spec.securityContext.fsGroupChangePolicy - spec.securityContext.sysctls - spec.shareProcessNamespace - spec.securityContext.runAsUser - spec.securityContext.runAsGroup - spec.securityContext.supplementalGroups - spec.containers[*].securityContext.seLinuxOptions - spec.containers[*].securityContext.seccompProfile - spec.containers[*].securityContext.capabilities - spec.containers[*].securityContext.readOnlyRootFilesystem - spec.containers[*].securityContext.privileged - spec.containers[*].securityContext.allowPrivilegeEscalation - spec.containers[*].securityContext.procMount - spec.containers[*].securityContext.runAsUser - spec.containers[*].securityContext.runAsGroup This is a beta field and requires the IdentifyPodOS feature"
"description": "Specifies the OS of the containers in the pod. Some pod and container fields are restricted if this is set.\n\nIf the OS field is set to linux, the following fields must be unset: -securityContext.windowsOptions\n\nIf the OS field is set to windows, following fields must be unset: - spec.hostPID - spec.hostIPC - spec.securityContext.seLinuxOptions - spec.securityContext.seccompProfile - spec.securityContext.fsGroup - spec.securityContext.fsGroupChangePolicy - spec.securityContext.sysctls - spec.shareProcessNamespace - spec.securityContext.runAsUser - spec.securityContext.runAsGroup - spec.securityContext.supplementalGroups - spec.containers[*].securityContext.seLinuxOptions - spec.containers[*].securityContext.seccompProfile - spec.containers[*].securityContext.capabilities - spec.containers[*].securityContext.readOnlyRootFilesystem - spec.containers[*].securityContext.privileged - spec.containers[*].securityContext.allowPrivilegeEscalation - spec.containers[*].securityContext.procMount - spec.containers[*].securityContext.runAsUser - spec.containers[*].securityContext.runAsGroup"
},
"overhead": {
"additionalProperties": {
Expand Down
2 changes: 1 addition & 1 deletion api/openapi-spec/v3/apis__apps__v1_openapi.json
Expand Up @@ -3506,7 +3506,7 @@
"$ref": "#/components/schemas/io.k8s.api.core.v1.PodOS"
}
],
"description": "Specifies the OS of the containers in the pod. Some pod and container fields are restricted if this is set.\n\nIf the OS field is set to linux, the following fields must be unset: -securityContext.windowsOptions\n\nIf the OS field is set to windows, following fields must be unset: - spec.hostPID - spec.hostIPC - spec.securityContext.seLinuxOptions - spec.securityContext.seccompProfile - spec.securityContext.fsGroup - spec.securityContext.fsGroupChangePolicy - spec.securityContext.sysctls - spec.shareProcessNamespace - spec.securityContext.runAsUser - spec.securityContext.runAsGroup - spec.securityContext.supplementalGroups - spec.containers[*].securityContext.seLinuxOptions - spec.containers[*].securityContext.seccompProfile - spec.containers[*].securityContext.capabilities - spec.containers[*].securityContext.readOnlyRootFilesystem - spec.containers[*].securityContext.privileged - spec.containers[*].securityContext.allowPrivilegeEscalation - spec.containers[*].securityContext.procMount - spec.containers[*].securityContext.runAsUser - spec.containers[*].securityContext.runAsGroup This is a beta field and requires the IdentifyPodOS feature"
"description": "Specifies the OS of the containers in the pod. Some pod and container fields are restricted if this is set.\n\nIf the OS field is set to linux, the following fields must be unset: -securityContext.windowsOptions\n\nIf the OS field is set to windows, following fields must be unset: - spec.hostPID - spec.hostIPC - spec.securityContext.seLinuxOptions - spec.securityContext.seccompProfile - spec.securityContext.fsGroup - spec.securityContext.fsGroupChangePolicy - spec.securityContext.sysctls - spec.shareProcessNamespace - spec.securityContext.runAsUser - spec.securityContext.runAsGroup - spec.securityContext.supplementalGroups - spec.containers[*].securityContext.seLinuxOptions - spec.containers[*].securityContext.seccompProfile - spec.containers[*].securityContext.capabilities - spec.containers[*].securityContext.readOnlyRootFilesystem - spec.containers[*].securityContext.privileged - spec.containers[*].securityContext.allowPrivilegeEscalation - spec.containers[*].securityContext.procMount - spec.containers[*].securityContext.runAsUser - spec.containers[*].securityContext.runAsGroup"
},
"overhead": {
"additionalProperties": {
Expand Down
2 changes: 1 addition & 1 deletion api/openapi-spec/v3/apis__batch__v1_openapi.json
Expand Up @@ -2585,7 +2585,7 @@
"$ref": "#/components/schemas/io.k8s.api.core.v1.PodOS"
}
],
"description": "Specifies the OS of the containers in the pod. Some pod and container fields are restricted if this is set.\n\nIf the OS field is set to linux, the following fields must be unset: -securityContext.windowsOptions\n\nIf the OS field is set to windows, following fields must be unset: - spec.hostPID - spec.hostIPC - spec.securityContext.seLinuxOptions - spec.securityContext.seccompProfile - spec.securityContext.fsGroup - spec.securityContext.fsGroupChangePolicy - spec.securityContext.sysctls - spec.shareProcessNamespace - spec.securityContext.runAsUser - spec.securityContext.runAsGroup - spec.securityContext.supplementalGroups - spec.containers[*].securityContext.seLinuxOptions - spec.containers[*].securityContext.seccompProfile - spec.containers[*].securityContext.capabilities - spec.containers[*].securityContext.readOnlyRootFilesystem - spec.containers[*].securityContext.privileged - spec.containers[*].securityContext.allowPrivilegeEscalation - spec.containers[*].securityContext.procMount - spec.containers[*].securityContext.runAsUser - spec.containers[*].securityContext.runAsGroup This is a beta field and requires the IdentifyPodOS feature"
"description": "Specifies the OS of the containers in the pod. Some pod and container fields are restricted if this is set.\n\nIf the OS field is set to linux, the following fields must be unset: -securityContext.windowsOptions\n\nIf the OS field is set to windows, following fields must be unset: - spec.hostPID - spec.hostIPC - spec.securityContext.seLinuxOptions - spec.securityContext.seccompProfile - spec.securityContext.fsGroup - spec.securityContext.fsGroupChangePolicy - spec.securityContext.sysctls - spec.shareProcessNamespace - spec.securityContext.runAsUser - spec.securityContext.runAsGroup - spec.securityContext.supplementalGroups - spec.containers[*].securityContext.seLinuxOptions - spec.containers[*].securityContext.seccompProfile - spec.containers[*].securityContext.capabilities - spec.containers[*].securityContext.readOnlyRootFilesystem - spec.containers[*].securityContext.privileged - spec.containers[*].securityContext.allowPrivilegeEscalation - spec.containers[*].securityContext.procMount - spec.containers[*].securityContext.runAsUser - spec.containers[*].securityContext.runAsGroup"
},
"overhead": {
"additionalProperties": {
Expand Down
20 changes: 0 additions & 20 deletions pkg/api/pod/util.go
Expand Up @@ -418,8 +418,6 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
AllowWindowsHostProcessField: utilfeature.DefaultFeatureGate.Enabled(features.WindowsHostProcessContainers),
// Allow pod spec with expanded DNS configuration
AllowExpandedDNSConfig: utilfeature.DefaultFeatureGate.Enabled(features.ExpandedDNSConfig) || haveSameExpandedDNSConfig(podSpec, oldPodSpec),
// Allow pod spec to use OS field
AllowOSField: utilfeature.DefaultFeatureGate.Enabled(features.IdentifyPodOS),
}

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

// if old spec has OS field set, we must allow it
opts.AllowOSField = opts.AllowOSField || oldPodSpec.OS != nil

// if old spec used non-integer multiple of huge page unit size, we must allow it
opts.AllowIndivisibleHugePagesValues = usesIndivisibleHugePagesValues(oldPodSpec)

Expand Down Expand Up @@ -556,10 +551,6 @@ func dropDisabledFields(

dropDisabledCSIVolumeSourceAlphaFields(podSpec, oldPodSpec)

if !utilfeature.DefaultFeatureGate.Enabled(features.IdentifyPodOS) && !podOSInUse(oldPodSpec) {
podSpec.OS = nil
}

dropDisabledTopologySpreadConstraintsFields(podSpec, oldPodSpec)
dropDisabledNodeInclusionPolicyFields(podSpec, oldPodSpec)
}
Expand Down Expand Up @@ -591,17 +582,6 @@ func minDomainsInUse(podSpec *api.PodSpec) bool {
return false
}

// podOSInUse returns true if the pod spec is non-nil and has OS field set
func podOSInUse(podSpec *api.PodSpec) bool {
if podSpec == nil {
return false
}
if podSpec.OS != nil {
return true
}
return false
}

// dropDisabledProcMountField removes disabled fields from PodSpec related
// to ProcMount only if it is not already used by the old spec
func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
Expand Down
82 changes: 0 additions & 82 deletions pkg/api/pod/util_test.go
Expand Up @@ -1687,88 +1687,6 @@ func TestDropDisabledTopologySpreadConstraintsFields(t *testing.T) {
}
}

func TestDropOSField(t *testing.T) {
podWithOSField := func() *api.Pod {
osField := api.PodOS{Name: "linux"}
return &api.Pod{
Spec: api.PodSpec{
OS: &osField,
},
}
}
podWithoutOSField := func() *api.Pod { return &api.Pod{} }
podInfo := []struct {
description string
hasPodOSField bool
pod func() *api.Pod
}{
{
description: "has PodOS field",
hasPodOSField: true,
pod: podWithOSField,
},
{
description: "does not have PodOS field",
hasPodOSField: false,
pod: podWithoutOSField,
},
{
description: "is nil",
hasPodOSField: false,
pod: func() *api.Pod { return nil },
},
}

for _, enabled := range []bool{true, false} {
for _, oldPodInfo := range podInfo {
for _, newPodInfo := range podInfo {
oldPodHasOsField, oldPod := oldPodInfo.hasPodOSField, oldPodInfo.pod()
newPodHasOSField, newPod := newPodInfo.hasPodOSField, newPodInfo.pod()
if newPod == nil {
continue
}

t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.IdentifyPodOS, enabled)()

var oldPodSpec *api.PodSpec
if oldPod != nil {
oldPodSpec = &oldPod.Spec
}
dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)

// old pod should never be changed
if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) {
t.Errorf("old pod changed: %v", cmp.Diff(oldPod, oldPodInfo.pod()))
}

switch {
case enabled || oldPodHasOsField:
// new pod should not be changed if the feature is enabled, or if the old pod had subpaths
if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
}
case newPodHasOSField:
// new pod should be changed
if reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod was not changed")
}
// new pod should not have OSfield
if !reflect.DeepEqual(newPod, podWithoutOSField()) {
t.Errorf("new pod has OS field: %v", cmp.Diff(newPod, podWithoutOSField()))
}
default:
// new pod should not need to be changed
if !reflect.DeepEqual(newPod, newPodInfo.pod()) {
t.Errorf("new pod changed: %v", cmp.Diff(newPod, newPodInfo.pod()))
}
}
})
}
}
}
}

func TestDropNodeInclusionPolicyFields(t *testing.T) {
ignore := api.NodeInclusionPolicyIgnore
honor := api.NodeInclusionPolicyHonor
Expand Down
1 change: 0 additions & 1 deletion pkg/apis/core/types.go
Expand Up @@ -2992,7 +2992,6 @@ type PodSpec struct {
// - spec.containers[*].securityContext.runAsUser
// - spec.containers[*].securityContext.runAsGroup
// +optional
// This is a beta field and requires the IdentifyPodOS feature
OS *PodOS
}

Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/core/validation/validation.go
Expand Up @@ -3396,8 +3396,6 @@ type PodValidationOptions struct {
AllowWindowsHostProcessField bool
// Allow more DNSSearchPaths and longer DNSSearchListChars
AllowExpandedDNSConfig bool
// Allow OSField to be set in the pod spec
AllowOSField bool
}

// validatePodMetadataAndSpec tests if required fields in the pod.metadata and pod.spec are set,
Expand Down Expand Up @@ -6345,9 +6343,6 @@ func validateOS(podSpec *core.PodSpec, fldPath *field.Path, opts PodValidationOp
if os == nil {
return allErrs
}
if !opts.AllowOSField {
return append(allErrs, field.Forbidden(fldPath, "cannot be set when IdentifyPodOS feature is not enabled"))
}
if len(os.Name) == 0 {
return append(allErrs, field.Required(fldPath.Child("name"), "cannot be empty"))
}
Expand Down