Skip to content

Commit bce0335

Browse files
carlorycarlory
committed
DRA: enhance validation for the ResourceClaimParametersReference and ResourceClassParametersReference with the following rules:
1. `apiGroup`: If set, it must be a valid DNS subdomain (e.g. 'example.com'). 2. `kind` and `name`: It must be valid path segment name. It may not be '.' or '..' and it may not contain '/' and '%' characters.
1 parent bce55b9 commit bce0335

File tree

4 files changed

+125
-14
lines changed

4 files changed

+125
-14
lines changed

‎pkg/apis/resource/validation/validation.go‎

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ package validation
1919
import (
2020
apiequality "k8s.io/apimachinery/pkg/api/equality"
2121
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
22+
pathvalidation "k8s.io/apimachinery/pkg/api/validation/path"
2223
"k8s.io/apimachinery/pkg/types"
2324
"k8s.io/apimachinery/pkg/util/sets"
25+
utilvalidation "k8s.io/apimachinery/pkg/util/validation"
2426
"k8s.io/apimachinery/pkg/util/validation/field"
2527
corevalidation "k8s.io/kubernetes/pkg/apis/core/validation"
2628
"k8s.io/kubernetes/pkg/apis/resource"
@@ -58,30 +60,70 @@ var supportedAllocationModes = sets.NewString(string(resource.AllocationModeImme
5860

5961
func validateResourceClaimParametersRef(ref *resource.ResourceClaimParametersReference, fldPath *field.Path) field.ErrorList {
6062
var allErrs field.ErrorList
61-
if ref != nil {
62-
if ref.Kind == "" {
63-
allErrs = append(allErrs, field.Required(fldPath.Child("kind"), ""))
63+
64+
if ref == nil {
65+
return allErrs
66+
}
67+
68+
// group is required but the Core group is the empty value, so it can not be enforced.
69+
if ref.APIGroup != "" {
70+
for _, msg := range utilvalidation.IsDNS1123Subdomain(ref.APIGroup) {
71+
allErrs = append(allErrs, field.Invalid(fldPath.Child("apiGroup"), ref.APIGroup, msg))
72+
}
73+
}
74+
75+
if ref.Kind == "" {
76+
allErrs = append(allErrs, field.Required(fldPath.Child("kind"), ""))
77+
} else {
78+
for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Kind) {
79+
allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg))
6480
}
65-
if ref.Name == "" {
66-
allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
81+
}
82+
83+
if ref.Name == "" {
84+
allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
85+
} else {
86+
for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Name) {
87+
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg))
6788
}
6889
}
6990
return allErrs
7091
}
7192

7293
func validateClassParameters(ref *resource.ResourceClassParametersReference, fldPath *field.Path) field.ErrorList {
7394
var allErrs field.ErrorList
74-
if ref != nil {
75-
if ref.Kind == "" {
76-
allErrs = append(allErrs, field.Required(fldPath.Child("kind"), ""))
95+
96+
if ref == nil {
97+
return allErrs
98+
}
99+
100+
// group is required but the Core group is the empty value, so it can not be enforced.
101+
if ref.APIGroup != "" {
102+
for _, msg := range utilvalidation.IsDNS1123Subdomain(ref.APIGroup) {
103+
allErrs = append(allErrs, field.Invalid(fldPath.Child("apiGroup"), ref.APIGroup, msg))
77104
}
78-
if ref.Name == "" {
79-
allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
105+
}
106+
107+
if ref.Kind == "" {
108+
allErrs = append(allErrs, field.Required(fldPath.Child("kind"), ""))
109+
} else {
110+
for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Kind) {
111+
allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg))
80112
}
81-
if ref.Namespace != "" {
82-
for _, msg := range apimachineryvalidation.ValidateNamespaceName(ref.Namespace, false) {
83-
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), ref.Namespace, msg))
84-
}
113+
}
114+
115+
if ref.Name == "" {
116+
allErrs = append(allErrs, field.Required(fldPath.Child("name"), ""))
117+
} else {
118+
for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Name) {
119+
allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg))
120+
}
121+
}
122+
123+
// namespace is optional.
124+
if ref.Namespace != "" {
125+
for _, msg := range apimachineryvalidation.ValidateNamespaceName(ref.Namespace, false) {
126+
allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), ref.Namespace, msg))
85127
}
86128
}
87129
return allErrs

‎pkg/apis/resource/validation/validation_resourceclaim_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ func TestValidateClaim(t *testing.T) {
5353
}
5454
now := metav1.Now()
5555
badValue := "spaces not allowed"
56+
badAPIGroup := "example.com/v1"
57+
goodAPIGroup := "example.com"
5658

5759
scenarios := map[string]struct {
5860
claim *resource.ResourceClaim
@@ -216,6 +218,29 @@ func TestValidateClaim(t *testing.T) {
216218
return claim
217219
}(),
218220
},
221+
"good-parameters-apigroup": {
222+
claim: func() *resource.ResourceClaim {
223+
claim := testClaim(goodName, goodNS, goodClaimSpec)
224+
claim.Spec.ParametersRef = &resource.ResourceClaimParametersReference{
225+
APIGroup: goodAPIGroup,
226+
Kind: "foo",
227+
Name: "bar",
228+
}
229+
return claim
230+
}(),
231+
},
232+
"bad-parameters-apigroup": {
233+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "parametersRef", "apiGroup"), badAPIGroup, "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')")},
234+
claim: func() *resource.ResourceClaim {
235+
claim := testClaim(goodName, goodNS, goodClaimSpec)
236+
claim.Spec.ParametersRef = &resource.ResourceClaimParametersReference{
237+
APIGroup: badAPIGroup,
238+
Kind: "foo",
239+
Name: "bar",
240+
}
241+
return claim
242+
}(),
243+
},
219244
"missing-parameters-kind": {
220245
wantFailures: field.ErrorList{field.Required(field.NewPath("spec", "parametersRef", "kind"), "")},
221246
claim: func() *resource.ResourceClaim {

‎pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ func TestValidateClaimTemplate(t *testing.T) {
5151
}
5252
now := metav1.Now()
5353
badValue := "spaces not allowed"
54+
badAPIGroup := "example.com/v1"
55+
goodAPIGroup := "example.com"
5456

5557
scenarios := map[string]struct {
5658
template *resource.ResourceClaimTemplate
@@ -214,6 +216,29 @@ func TestValidateClaimTemplate(t *testing.T) {
214216
return template
215217
}(),
216218
},
219+
"good-parameters-apigroup": {
220+
template: func() *resource.ResourceClaimTemplate {
221+
template := testClaimTemplate(goodName, goodNS, goodClaimSpec)
222+
template.Spec.Spec.ParametersRef = &resource.ResourceClaimParametersReference{
223+
APIGroup: goodAPIGroup,
224+
Kind: "foo",
225+
Name: "bar",
226+
}
227+
return template
228+
}(),
229+
},
230+
"bad-parameters-apigroup": {
231+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("spec", "spec", "parametersRef", "apiGroup"), badAPIGroup, "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')")},
232+
template: func() *resource.ResourceClaimTemplate {
233+
template := testClaimTemplate(goodName, goodNS, goodClaimSpec)
234+
template.Spec.Spec.ParametersRef = &resource.ResourceClaimParametersReference{
235+
APIGroup: badAPIGroup,
236+
Kind: "foo",
237+
Name: "bar",
238+
}
239+
return template
240+
}(),
241+
},
217242
"missing-parameters-kind": {
218243
wantFailures: field.ErrorList{field.Required(field.NewPath("spec", "spec", "parametersRef", "kind"), "")},
219244
template: func() *resource.ResourceClaimTemplate {

‎pkg/apis/resource/validation/validation_resourceclass_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ func TestValidateClass(t *testing.T) {
4747
}
4848
badName := "!@#$%^"
4949
badValue := "spaces not allowed"
50+
badAPIGroup := "example.com/v1"
51+
goodAPIGroup := "example.com"
5052

5153
scenarios := map[string]struct {
5254
class *resource.ResourceClass
@@ -204,6 +206,23 @@ func TestValidateClass(t *testing.T) {
204206
return class
205207
}(),
206208
},
209+
"good-parameters-apigroup": {
210+
class: func() *resource.ResourceClass {
211+
class := testClass(goodName, goodName)
212+
class.ParametersRef = goodParameters.DeepCopy()
213+
class.ParametersRef.APIGroup = goodAPIGroup
214+
return class
215+
}(),
216+
},
217+
"bad-parameters-apigroup": {
218+
wantFailures: field.ErrorList{field.Invalid(field.NewPath("parametersRef", "apiGroup"), badAPIGroup, "a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')")},
219+
class: func() *resource.ResourceClass {
220+
class := testClass(goodName, goodName)
221+
class.ParametersRef = goodParameters.DeepCopy()
222+
class.ParametersRef.APIGroup = badAPIGroup
223+
return class
224+
}(),
225+
},
207226
"missing-parameters-name": {
208227
wantFailures: field.ErrorList{field.Required(field.NewPath("parametersRef", "name"), "")},
209228
class: func() *resource.ResourceClass {

0 commit comments

Comments
 (0)