From 14b09c414acf12ba2383fc088f0226633bce9a1d Mon Sep 17 00:00:00 2001 From: Shiming Zhang Date: Sat, 27 May 2023 23:44:23 +0800 Subject: [PATCH] Add DownwardAPI validation for status.hostIPs --- pkg/api/pod/util.go | 54 +++++++++++++++++++++ pkg/apis/core/validation/validation.go | 14 ++++++ pkg/apis/core/validation/validation_test.go | 47 ++++++++++++++++++ 3 files changed, 115 insertions(+) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 61e50860f49..3d33c50d034 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -358,6 +358,8 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po // default pod validation options based on feature gate opts := apivalidation.PodValidationOptions{ AllowInvalidPodDeletionCost: !utilfeature.DefaultFeatureGate.Enabled(features.PodDeletionCost), + // Allow pod spec to use status.hostIPs in downward API if feature is enabled + AllowHostIPsField: utilfeature.DefaultFeatureGate.Enabled(features.PodHostIPs), // Do not allow pod spec to use non-integer multiple of huge page unit size default AllowIndivisibleHugePagesValues: false, AllowInvalidLabelValueInSelector: false, @@ -366,6 +368,9 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po } if oldPodSpec != nil { + // if old spec has status.hostIPs downwardAPI set, we must allow it + opts.AllowHostIPsField = opts.AllowHostIPsField || hasUsedDownwardAPIFieldPathWithPodSpec(oldPodSpec, "status.hostIPs") + // if old spec used non-integer multiple of huge page unit size, we must allow it opts.AllowIndivisibleHugePagesValues = usesIndivisibleHugePagesValues(oldPodSpec) @@ -382,6 +387,55 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po return opts } +func hasUsedDownwardAPIFieldPathWithPodSpec(podSpec *api.PodSpec, fieldPath string) bool { + if podSpec == nil { + return false + } + for _, vol := range podSpec.Volumes { + if hasUsedDownwardAPIFieldPathWithVolume(&vol, fieldPath) { + return true + } + } + for _, c := range podSpec.InitContainers { + if hasUsedDownwardAPIFieldPathWithContainer(&c, fieldPath) { + return true + } + } + for _, c := range podSpec.Containers { + if hasUsedDownwardAPIFieldPathWithContainer(&c, fieldPath) { + return true + } + } + return false +} + +func hasUsedDownwardAPIFieldPathWithVolume(volume *api.Volume, fieldPath string) bool { + if volume == nil || volume.DownwardAPI == nil { + return false + } + for _, file := range volume.DownwardAPI.Items { + if file.FieldRef != nil && + file.FieldRef.FieldPath == fieldPath { + return true + } + } + return false +} + +func hasUsedDownwardAPIFieldPathWithContainer(container *api.Container, fieldPath string) bool { + if container == nil { + return false + } + for _, env := range container.Env { + if env.ValueFrom != nil && + env.ValueFrom.FieldRef != nil && + env.ValueFrom.FieldRef.FieldPath == fieldPath { + return true + } + } + return false +} + // GetValidationOptionsFromPodTemplate will return pod validation options for specified template. func GetValidationOptionsFromPodTemplate(podTemplate, oldPodTemplate *api.PodTemplateSpec) apivalidation.PodValidationOptions { var newPodSpec, oldPodSpec *api.PodSpec diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 859fa3fa988..3ee8cd2f8de 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1055,6 +1055,7 @@ func validateDownwardAPIVolumeFile(file *core.DownwardAPIVolumeFile, fldPath *fi if file.ResourceFieldRef != nil { allErrs = append(allErrs, field.Invalid(fldPath, "resource", "fieldRef and resourceFieldRef can not be specified simultaneously")) } + allErrs = append(allErrs, validateDownwardAPIHostIPs(file.FieldRef, fldPath.Child("fieldRef"), opts)...) } else if file.ResourceFieldRef != nil { localValidContainerResourceFieldPathPrefixes := validContainerResourceFieldPathPrefixesWithDownwardAPIHugePages allErrs = append(allErrs, validateContainerResourceFieldSelector(file.ResourceFieldRef, &validContainerResourceFieldPathExpressions, &localValidContainerResourceFieldPathPrefixes, fldPath.Child("resourceFieldRef"), true)...) @@ -2427,6 +2428,7 @@ func validateEnvVarValueFrom(ev core.EnvVar, fldPath *field.Path, opts PodValida if ev.ValueFrom.FieldRef != nil { numSources++ allErrs = append(allErrs, validateObjectFieldSelector(ev.ValueFrom.FieldRef, &validEnvDownwardAPIFieldPathExpressions, fldPath.Child("fieldRef"))...) + allErrs = append(allErrs, validateDownwardAPIHostIPs(ev.ValueFrom.FieldRef, fldPath.Child("fieldRef"), opts)...) } if ev.ValueFrom.ResourceFieldRef != nil { numSources++ @@ -2490,6 +2492,16 @@ func validateObjectFieldSelector(fs *core.ObjectFieldSelector, expressions *sets return allErrs } +func validateDownwardAPIHostIPs(fieldSel *core.ObjectFieldSelector, fldPath *field.Path, opts PodValidationOptions) field.ErrorList { + allErrs := field.ErrorList{} + if !opts.AllowHostIPsField { + if fieldSel.FieldPath == "status.hostIPs" { + allErrs = append(allErrs, field.Forbidden(fldPath, "may not be set when feature gate 'PodHostIPs' is not enabled")) + } + } + return allErrs +} + func validateContainerResourceFieldSelector(fs *core.ResourceFieldSelector, expressions *sets.String, prefixes *sets.String, fldPath *field.Path, volume bool) field.ErrorList { allErrs := field.ErrorList{} @@ -3729,6 +3741,8 @@ type PodValidationOptions struct { AllowInvalidLabelValueInSelector bool // Allow pod spec to use non-integer multiple of huge page unit size AllowIndivisibleHugePagesValues bool + // Allow pod spec to use status.hostIPs in downward API if feature is enabled + AllowHostIPsField bool // Allow invalid topologySpreadConstraint labelSelector for backward compatibility AllowInvalidTopologySpreadConstraintLabelSelector bool // Allow node selector additions for gated pods. diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 7b87ba7722b..24df46aa32d 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -22669,6 +22669,53 @@ func TestValidateAppArmorProfileFormat(t *testing.T) { } } +func TestValidateDownwardAPIHostIPs(t *testing.T) { + testCases := []struct { + name string + expectError bool + featureEnabled bool + fieldSel *core.ObjectFieldSelector + }{ + { + name: "has no hostIPs field, featuregate enabled", + expectError: false, + featureEnabled: true, + fieldSel: &core.ObjectFieldSelector{FieldPath: "status.hostIP"}, + }, + { + name: "has hostIPs field, featuregate enabled", + expectError: false, + featureEnabled: true, + fieldSel: &core.ObjectFieldSelector{FieldPath: "status.hostIPs"}, + }, + { + name: "has no hostIPs field, featuregate disabled", + expectError: false, + featureEnabled: false, + fieldSel: &core.ObjectFieldSelector{FieldPath: "status.hostIP"}, + }, + { + name: "has hostIPs field, featuregate disabled", + expectError: true, + featureEnabled: false, + fieldSel: &core.ObjectFieldSelector{FieldPath: "status.hostIPs"}, + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.PodHostIPs, testCase.featureEnabled)() + + errs := validateDownwardAPIHostIPs(testCase.fieldSel, field.NewPath("fieldSel"), PodValidationOptions{AllowHostIPsField: testCase.featureEnabled}) + if testCase.expectError && len(errs) == 0 { + t.Errorf("Unexpected success") + } + if !testCase.expectError && len(errs) != 0 { + t.Errorf("Unexpected error(s): %v", errs) + } + }) + } +} + func TestValidatePVSecretReference(t *testing.T) { rootFld := field.NewPath("name") type args struct {