github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-31 15:25:57 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #66843 from verult/validate-allowedtopologies

Browse Source 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Added set and map structural validation for AllowedTopologies

**What this PR does / why we need it**: Adding structural validation to AllowedTopologies field in StorageClass.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #66184

**Release note**:

```release-note
AllowedTopologies field inside StorageClass is now validated against set and map semantics. Specifically, there cannot be duplicate TopologySelectorTerms, MatchLabelExpressions keys, and TopologySelectorLabelRequirement Values.
```
This commit is contained in:
Kubernetes Submit Queue 2018-08-27 15:09:52 -07:00 committed by GitHub
commit 344b915484
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 276 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
pkg/apis/core/validation/validation.go
View File

@ -3105,30 +3105,55 @@ func ValidateNodeSelector(nodeSelector *core.NodeSelector, fldPath *field.Path)
return allErrs
}
// validateTopologySelectorLabelRequirement tests that the specified TopologySelectorLabelRequirement fields has valid data
func validateTopologySelectorLabelRequirement(rq core.TopologySelectorLabelRequirement, fldPath *field.Path) field.ErrorList {
// validateTopologySelectorLabelRequirement tests that the specified TopologySelectorLabelRequirement fields has valid data,
// and constructs a set containing all of its Values.
func validateTopologySelectorLabelRequirement(rq core.TopologySelectorLabelRequirement, fldPath *field.Path) (sets.String, field.ErrorList) {
allErrs := field.ErrorList{}
valueSet := make(sets.String)
valuesPath := fldPath.Child("values")
if len(rq.Values) == 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("values"), "must specify as least one value"))
allErrs = append(allErrs, field.Required(valuesPath, ""))
}
// Validate set property of Values field
for i, value := range rq.Values {
if valueSet.Has(value) {
allErrs = append(allErrs, field.Duplicate(valuesPath.Index(i), value))
}
valueSet.Insert(value)
}
allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(rq.Key, fldPath.Child("key"))...)
return allErrs
return valueSet, allErrs
}
// ValidateTopologySelectorTerm tests that the specified topology selector term has valid data
func ValidateTopologySelectorTerm(term core.TopologySelectorTerm, fldPath *field.Path) field.ErrorList {
// ValidateTopologySelectorTerm tests that the specified topology selector term has valid data,
// and constructs a map representing the term in raw form.
func ValidateTopologySelectorTerm(term core.TopologySelectorTerm, fldPath *field.Path) (map[string]sets.String, field.ErrorList) {
allErrs := field.ErrorList{}
exprMap := make(map[string]sets.String)
exprPath := fldPath.Child("matchLabelExpressions")
if utilfeature.DefaultFeatureGate.Enabled(features.DynamicProvisioningScheduling) {
// Allow empty MatchLabelExpressions, in case this field becomes optional in the future.
for i, req := range term.MatchLabelExpressions {
allErrs = append(allErrs, validateTopologySelectorLabelRequirement(req, fldPath.Child("matchLabelExpressions").Index(i))...)
idxPath := exprPath.Index(i)
valueSet, exprErrs := validateTopologySelectorLabelRequirement(req, idxPath)
allErrs = append(allErrs, exprErrs...)
// Validate no duplicate keys exist.
if _, exists := exprMap[req.Key]; exists {
allErrs = append(allErrs, field.Duplicate(idxPath.Child("key"), req.Key))
}
exprMap[req.Key] = valueSet
}
} else if len(term.MatchLabelExpressions) != 0 {
allErrs = append(allErrs, field.Forbidden(fldPath, "field is disabled by feature-gate DynamicProvisioningScheduling"))
}
return allErrs
return exprMap, allErrs
}
// ValidateAvoidPodsInNodeAnnotations tests that the serialized AvoidPods in Node.Annotations has valid data

1
pkg/apis/storage/validation/BUILD
View File

@ -12,6 +12,7 @@ go_library(
importpath = "k8s.io/kubernetes/pkg/apis/storage/validation",
deps = [
"//pkg/apis/core:go_default_library",
"//pkg/apis/core/helper:go_default_library",
"//pkg/apis/core/validation:go_default_library",
"//pkg/apis/storage:go_default_library",
"//pkg/features:go_default_library",

15
pkg/apis/storage/validation/validation.go
View File

@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/core/helper"
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/apis/storage"
"k8s.io/kubernetes/pkg/features"
@ -253,8 +254,20 @@ func validateAllowedTopologies(topologies []api.TopologySelectorTerm, fldPath *f
allErrs = append(allErrs, field.Forbidden(fldPath, "field is disabled by feature-gate DynamicProvisioningScheduling"))
}
rawTopologies := make([]map[string]sets.String, len(topologies))
for i, term := range topologies {
allErrs = append(allErrs, apivalidation.ValidateTopologySelectorTerm(term, fldPath.Index(i))...)
idxPath := fldPath.Index(i)
exprMap, termErrs := apivalidation.ValidateTopologySelectorTerm(term, fldPath.Index(i))
allErrs = append(allErrs, termErrs...)
// TODO (verult) consider improving runtime
for _, t := range rawTopologies {
if helper.Semantic.DeepEqual(exprMap, t) {
allErrs = append(allErrs, field.Duplicate(idxPath.Child("matchLabelExpressions"), ""))
}
}
rawTopologies = append(rawTopologies, exprMap)
}
return allErrs

229
pkg/apis/storage/validation/validation_test.go
View File

@ -644,6 +644,187 @@ func TestValidateAllowedTopologies(t *testing.T) {
},
}
topologyDupValues := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node1", "node1"},
},
},
},
}
topologyMultiValues := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node1", "node2"},
},
},
},
}
topologyEmptyMatchLabelExpressions := []api.TopologySelectorTerm{
{
MatchLabelExpressions: nil,
},
}
topologyDupKeys := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node1"},
},
{
Key: "kubernetes.io/hostname",
Values: []string{"node2"},
},
},
},
}
topologyMultiTerm := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node1"},
},
},
},
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node2"},
},
},
},
}
topologyDupTermsIdentical := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone1"},
},
{
Key: "kubernetes.io/hostname",
Values: []string{"node1"},
},
},
},
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone1"},
},
{
Key: "kubernetes.io/hostname",
Values: []string{"node1"},
},
},
},
}
topologyExprsOneSameOneDiff := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone1"},
},
{
Key: "kubernetes.io/hostname",
Values: []string{"node1"},
},
},
},
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone1"},
},
{
Key: "kubernetes.io/hostname",
Values: []string{"node2"},
},
},
},
}
topologyValuesOneSameOneDiff := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node1", "node2"},
},
},
},
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node1", "node3"},
},
},
},
}
topologyDupTermsDiffExprOrder := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "kubernetes.io/hostname",
Values: []string{"node1"},
},
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone1"},
},
},
},
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone1"},
},
{
Key: "kubernetes.io/hostname",
Values: []string{"node1"},
},
},
},
}
topologyDupTermsDiffValueOrder := []api.TopologySelectorTerm{
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone1", "zone2"},
},
},
},
{
MatchLabelExpressions: []api.TopologySelectorLabelRequirement{
{
Key: "failure-domain.beta.kubernetes.io/zone",
Values: []string{"zone2", "zone1"},
},
},
},
}
cases := map[string]bindingTest{
"no topology": {
class: makeClass(nil, nil),
@ -661,10 +842,56 @@ func TestValidateAllowedTopologies(t *testing.T) {
class: makeClass(nil, topologyLackOfValues),
shouldSucceed: false,
},
"duplicate TopologySelectorRequirement values": {
class: makeClass(nil, topologyDupValues),
shouldSucceed: false,
},
"multiple TopologySelectorRequirement values": {
class: makeClass(nil, topologyMultiValues),
shouldSucceed: true,
},
"empty MatchLabelExpressions": {
class: makeClass(nil, topologyEmptyMatchLabelExpressions),
shouldSucceed: false,
},
"duplicate MatchLabelExpression keys": {
class: makeClass(nil, topologyDupKeys),
shouldSucceed: false,
},
"duplicate MatchLabelExpression keys but across separate terms": {
class: makeClass(nil, topologyMultiTerm),
shouldSucceed: true,
},
"duplicate AllowedTopologies terms - identical": {
class: makeClass(nil, topologyDupTermsIdentical),
shouldSucceed: false,
},
"two AllowedTopologies terms, with a pair of the same MatchLabelExpressions and a pair of different ones": {
class: makeClass(nil, topologyExprsOneSameOneDiff),
shouldSucceed: true,
},
"two AllowedTopologies terms, with a pair of the same Values and a pair of different ones": {
class: makeClass(nil, topologyValuesOneSameOneDiff),
shouldSucceed: true,
},
"duplicate AllowedTopologies terms - different MatchLabelExpressions order": {
class: makeClass(nil, topologyDupTermsDiffExprOrder),
shouldSucceed: false,
},
"duplicate AllowedTopologies terms - different TopologySelectorRequirement values order": {
class: makeClass(nil, topologyDupTermsDiffValueOrder),
shouldSucceed: false,
},
}
// Disable VolumeScheduling so nil VolumeBindingMode doesn't fail to validate.
err := utilfeature.DefaultFeatureGate.Set("VolumeScheduling=false")
if err != nil {
t.Fatalf("Failed to disable feature gate for VolumeScheduling: %v", err)
}
// TODO: remove when feature gate not required
err := utilfeature.DefaultFeatureGate.Set("DynamicProvisioningScheduling=true")
err = utilfeature.DefaultFeatureGate.Set("DynamicProvisioningScheduling=true")
if err != nil {
t.Fatalf("Failed to enable feature gate for DynamicProvisioningScheduling: %v", err)
}