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.

Sign up for GitHub

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

Jump to bottom

Add extra value validation for matchExpression field in LabelSelector #113699

Merged
merged 2 commits into from Nov 8, 2022
Merged

Add extra value validation for matchExpression field in LabelSelector #113699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0843c4d
Add extra value validation for matchExpression field in LabelSelector
Zheaoli Jan 13, 2022
fc69084
Update workload selector validation
liggitt Nov 7, 2022
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
39 changes: 38 additions & 1 deletion pkg/api/pod/util.go
View file
Open in desktop
Expand Up @@ -412,7 +412,8 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
// Do not allow pod spec to use non-integer multiple of huge page unit size default
AllowIndivisibleHugePagesValues: false,
// Allow pod spec with expanded DNS configuration
AllowExpandedDNSConfig: utilfeature.DefaultFeatureGate.Enabled(features.ExpandedDNSConfig) || haveSameExpandedDNSConfig(podSpec, oldPodSpec),
AllowExpandedDNSConfig: utilfeature.DefaultFeatureGate.Enabled(features.ExpandedDNSConfig) || haveSameExpandedDNSConfig(podSpec, oldPodSpec),
AllowInvalidLabelValueInSelector: false,
}

if oldPodSpec != nil {
Expand All @@ -429,6 +430,8 @@ func GetValidationOptionsFromPodSpecAndMeta(podSpec, oldPodSpec *api.PodSpec, po
// if old spec used non-integer multiple of huge page unit size, we must allow it
opts.AllowIndivisibleHugePagesValues = usesIndivisibleHugePagesValues(oldPodSpec)

opts.AllowInvalidLabelValueInSelector = hasInvalidLabelValueInAffinitySelector(oldPodSpec)

}
if oldPodMeta != nil && !opts.AllowInvalidPodDeletionCost {
// This is an update, so validate only if the existing object was valid.
Expand Down Expand Up @@ -756,3 +759,37 @@ func SeccompAnnotationForField(field *api.SeccompProfile) string {
// type is specified
return ""
}

func hasInvalidLabelValueInAffinitySelector(spec *api.PodSpec) bool {
if spec.Affinity != nil {
if spec.Affinity.PodAffinity != nil {
for _, term := range spec.Affinity.PodAffinity.RequiredDuringSchedulingIgnoredDuringExecution {
allErrs := apivalidation.ValidatePodAffinityTermSelector(term, false, nil)
if len(allErrs) != 0 {
return true
}
}
for _, term := range spec.Affinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution {
allErrs := apivalidation.ValidatePodAffinityTermSelector(term.PodAffinityTerm, false, nil)
if len(allErrs) != 0 {
return true
}
}
}
if spec.Affinity.PodAntiAffinity != nil {
for _, term := range spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution {
allErrs := apivalidation.ValidatePodAffinityTermSelector(term, false, nil)
if len(allErrs) != 0 {
return true
}
}
for _, term := range spec.Affinity.PodAntiAffinity.PreferredDuringSchedulingIgnoredDuringExecution {
allErrs := apivalidation.ValidatePodAffinityTermSelector(term.PodAffinityTerm, false, nil)
if len(allErrs) != 0 {
return true
}
}
}
}
return false
}
68 changes: 62 additions & 6 deletions pkg/apis/admissionregistration/validation/validation.go
View file
Open in desktop
Expand Up @@ -210,6 +210,7 @@ func ValidateValidatingWebhookConfiguration(e *admissionregistration.ValidatingW
requireNoSideEffects: true,
requireRecognizedAdmissionReviewVersion: true,
requireUniqueWebhookNames: true,
allowInvalidLabelValueInSelector: false,
})
}

Expand All @@ -236,17 +237,20 @@ func ValidateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebho
requireNoSideEffects: true,
requireRecognizedAdmissionReviewVersion: true,
requireUniqueWebhookNames: true,
allowInvalidLabelValueInSelector: false,
})
}

type validationOptions struct {
requireNoSideEffects bool
requireRecognizedAdmissionReviewVersion bool
requireUniqueWebhookNames bool
allowInvalidLabelValueInSelector bool
}

func validateMutatingWebhookConfiguration(e *admissionregistration.MutatingWebhookConfiguration, opts validationOptions) field.ErrorList {
allErrors := genericvalidation.ValidateObjectMeta(&e.ObjectMeta, false, genericvalidation.NameIsDNSSubdomain, field.NewPath("metadata"))

hookNames := sets.NewString()
for i, hook := range e.Webhooks {
allErrors = append(allErrors, validateMutatingWebhook(&hook, opts, field.NewPath("webhooks").Index(i))...)
Expand All @@ -266,6 +270,9 @@ func validateValidatingWebhook(hook *admissionregistration.ValidatingWebhook, op
var allErrors field.ErrorList
// hook.Name must be fully qualified
allErrors = append(allErrors, utilvalidation.IsFullyQualifiedName(fldPath.Child("name"), hook.Name)...)
labelSelectorValidationOpts := metav1validation.LabelSelectorValidationOptions{
AllowInvalidLabelValueInSelector: opts.allowInvalidLabelValueInSelector,
}

for i, rule := range hook.Rules {
allErrors = append(allErrors, validateRuleWithOperations(&rule, fldPath.Child("rules").Index(i))...)
Expand All @@ -291,11 +298,11 @@ func validateValidatingWebhook(hook *admissionregistration.ValidatingWebhook, op
}

if hook.NamespaceSelector != nil {
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, fldPath.Child("namespaceSelector"))...)
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, labelSelectorValidationOpts, fldPath.Child("namespaceSelector"))...)
}

if hook.ObjectSelector != nil {
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.ObjectSelector, fldPath.Child("objectSelector"))...)
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.ObjectSelector, labelSelectorValidationOpts, fldPath.Child("objectSelector"))...)
}

cc := hook.ClientConfig
Expand All @@ -314,6 +321,9 @@ func validateMutatingWebhook(hook *admissionregistration.MutatingWebhook, opts v
var allErrors field.ErrorList
// hook.Name must be fully qualified
allErrors = append(allErrors, utilvalidation.IsFullyQualifiedName(fldPath.Child("name"), hook.Name)...)
labelSelectorValidationOpts := metav1validation.LabelSelectorValidationOptions{
AllowInvalidLabelValueInSelector: opts.allowInvalidLabelValueInSelector,
}

for i, rule := range hook.Rules {
allErrors = append(allErrors, validateRuleWithOperations(&rule, fldPath.Child("rules").Index(i))...)
Expand All @@ -339,10 +349,10 @@ func validateMutatingWebhook(hook *admissionregistration.MutatingWebhook, opts v
}

if hook.NamespaceSelector != nil {
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, fldPath.Child("namespaceSelector"))...)
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, labelSelectorValidationOpts, fldPath.Child("namespaceSelector"))...)
}
if hook.ObjectSelector != nil {
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.ObjectSelector, fldPath.Child("objectSelector"))...)
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.ObjectSelector, labelSelectorValidationOpts, fldPath.Child("objectSelector"))...)
}
if hook.ReinvocationPolicy != nil && !supportedReinvocationPolicies.Has(string(*hook.ReinvocationPolicy)) {
allErrors = append(allErrors, field.NotSupported(fldPath.Child("reinvocationPolicy"), *hook.ReinvocationPolicy, supportedReinvocationPolicies.List()))
Expand Down Expand Up @@ -508,12 +518,55 @@ func validatingHasNoSideEffects(webhooks []admissionregistration.ValidatingWebho
return true
}

// validatingWebhookAllowInvalidLabelValueInSelector returns true if all webhooksallow invalid label value in selector
func validatingWebhookHasInvalidLabelValueInSelector(webhooks []admissionregistration.ValidatingWebhook) bool {
labelSelectorValidationOpts := metav1validation.LabelSelectorValidationOptions{
AllowInvalidLabelValueInSelector: false,
}

for _, hook := range webhooks {
if hook.NamespaceSelector != nil {
if len(metav1validation.ValidateLabelSelector(hook.NamespaceSelector, labelSelectorValidationOpts, nil)) > 0 {
return true
}
}
if hook.ObjectSelector != nil {
if len(metav1validation.ValidateLabelSelector(hook.ObjectSelector, labelSelectorValidationOpts, nil)) > 0 {
return true
}
}
}
return false
}

// mutatingWebhookAllowInvalidLabelValueInSelector returns true if all webhooks allow invalid label value in selector
func mutatingWebhookHasInvalidLabelValueInSelector(webhooks []admissionregistration.MutatingWebhook) bool {
labelSelectorValidationOpts := metav1validation.LabelSelectorValidationOptions{
AllowInvalidLabelValueInSelector: false,
}

for _, hook := range webhooks {
if hook.NamespaceSelector != nil {
if len(metav1validation.ValidateLabelSelector(hook.NamespaceSelector, labelSelectorValidationOpts, nil)) > 0 {
return true
}
}
if hook.ObjectSelector != nil {
if len(metav1validation.ValidateLabelSelector(hook.ObjectSelector, labelSelectorValidationOpts, nil)) > 0 {
return true
}
}
}
return false
}

// ValidateValidatingWebhookConfigurationUpdate validates update of validating webhook configuration
func ValidateValidatingWebhookConfigurationUpdate(newC, oldC *admissionregistration.ValidatingWebhookConfiguration) field.ErrorList {
return validateValidatingWebhookConfiguration(newC, validationOptions{
requireNoSideEffects: validatingHasNoSideEffects(oldC.Webhooks),
requireRecognizedAdmissionReviewVersion: validatingHasAcceptedAdmissionReviewVersions(oldC.Webhooks),
requireUniqueWebhookNames: validatingHasUniqueWebhookNames(oldC.Webhooks),
allowInvalidLabelValueInSelector: validatingWebhookHasInvalidLabelValueInSelector(oldC.Webhooks),
})
}

Expand All @@ -523,6 +576,7 @@ func ValidateMutatingWebhookConfigurationUpdate(newC, oldC *admissionregistratio
requireNoSideEffects: mutatingHasNoSideEffects(oldC.Webhooks),
requireRecognizedAdmissionReviewVersion: mutatingHasAcceptedAdmissionReviewVersions(oldC.Webhooks),
requireUniqueWebhookNames: mutatingHasUniqueWebhookNames(oldC.Webhooks),
allowInvalidLabelValueInSelector: mutatingWebhookHasInvalidLabelValueInSelector(oldC.Webhooks),
})
}

Expand Down Expand Up @@ -634,13 +688,15 @@ func validateMatchResources(mc *admissionregistration.MatchResources, fldPath *f
if mc.NamespaceSelector == nil {
allErrors = append(allErrors, field.Required(fldPath.Child("namespaceSelector"), ""))
} else {
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(mc.NamespaceSelector, fldPath.Child("namespaceSelector"))...)
// validate selector strictly, this type was released after issue #99139 was resolved
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(mc.NamespaceSelector, metav1validation.LabelSelectorValidationOptions{}, fldPath.Child("namespaceSelector"))...)
}

if mc.ObjectSelector == nil {
allErrors = append(allErrors, field.Required(fldPath.Child("labelSelector"), ""))
} else {
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(mc.ObjectSelector, fldPath.Child("labelSelector"))...)
// validate selector strictly, this type was released after issue #99139 was resolved
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(mc.ObjectSelector, metav1validation.LabelSelectorValidationOptions{}, fldPath.Child("labelSelector"))...)
}

for i, namedRuleWithOperations := range mc.ResourceRules {
Expand Down
12 changes: 8 additions & 4 deletions pkg/apis/apps/validation/validation.go
View file
Open in desktop
Expand Up @@ -132,7 +132,8 @@ func ValidateStatefulSetSpec(spec *apps.StatefulSetSpec, fldPath *field.Path, op
if spec.Selector == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
} else {
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
// validate selector strictly, spec.selector was always required to pass LabelSelectorAsSelector below
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, unversionedvalidation.LabelSelectorValidationOptions{AllowInvalidLabelValueInSelector: false}, fldPath.Child("selector"))...)
if len(spec.Selector.MatchLabels)+len(spec.Selector.MatchExpressions) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty selector is invalid for statefulset"))
}
Expand Down Expand Up @@ -336,8 +337,9 @@ func ValidateDaemonSetStatusUpdate(ds, oldDS *apps.DaemonSet) field.ErrorList {
// ValidateDaemonSetSpec tests if required fields in the DaemonSetSpec are set.
func ValidateDaemonSetSpec(spec *apps.DaemonSetSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
allErrs := field.ErrorList{}
labelSelectorValidationOpts := unversionedvalidation.LabelSelectorValidationOptions{AllowInvalidLabelValueInSelector: opts.AllowInvalidLabelValueInSelector}

allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, labelSelectorValidationOpts, fldPath.Child("selector"))...)

selector, err := metav1.LabelSelectorAsSelector(spec.Selector)
if err == nil && !selector.Matches(labels.Set(spec.Template.Labels)) {
Expand Down Expand Up @@ -540,7 +542,8 @@ func ValidateDeploymentSpec(spec *apps.DeploymentSpec, fldPath *field.Path, opts
if spec.Selector == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
} else {
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
// validate selector strictly, spec.selector was always required to pass LabelSelectorAsSelector below
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, unversionedvalidation.LabelSelectorValidationOptions{AllowInvalidLabelValueInSelector: false}, fldPath.Child("selector"))...)
if len(spec.Selector.MatchLabels)+len(spec.Selector.MatchExpressions) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty selector is invalid for deployment"))
}
Expand Down Expand Up @@ -702,7 +705,8 @@ func ValidateReplicaSetSpec(spec *apps.ReplicaSetSpec, fldPath *field.Path, opts
if spec.Selector == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
} else {
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
// validate selector strictly, spec.selector was always required to pass LabelSelectorAsSelector below
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, unversionedvalidation.LabelSelectorValidationOptions{AllowInvalidLabelValueInSelector: false}, fldPath.Child("selector"))...)
if len(spec.Selector.MatchLabels)+len(spec.Selector.MatchExpressions) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("selector"), spec.Selector, "empty selector is invalid for deployment"))
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/apis/batch/validation/validation.go
View file
Open in desktop
Expand Up @@ -146,11 +146,13 @@ func hasJobTrackingAnnotation(job *batch.Job) bool {
// ValidateJobSpec validates a JobSpec and returns an ErrorList with any errors.
func ValidateJobSpec(spec *batch.JobSpec, fldPath *field.Path, opts apivalidation.PodValidationOptions) field.ErrorList {
allErrs := validateJobSpec(spec, fldPath, opts)

if spec.Selector == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("selector"), ""))
} else {
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, fldPath.Child("selector"))...)
labelSelectorValidationOpts := unversionedvalidation.LabelSelectorValidationOptions{
AllowInvalidLabelValueInSelector: opts.AllowInvalidLabelValueInSelector,
}
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(spec.Selector, labelSelectorValidationOpts, fldPath.Child("selector"))...)
}

// Whether manually or automatically generated, the selector of the job must match the pods it will produce.
Expand Down