diff --git a/pkg/apis/resource/validation/validation.go b/pkg/apis/resource/validation/validation.go index a04c56c6544..7b93aae7db4 100644 --- a/pkg/apis/resource/validation/validation.go +++ b/pkg/apis/resource/validation/validation.go @@ -19,8 +19,10 @@ package validation import ( apiequality "k8s.io/apimachinery/pkg/api/equality" apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" + pathvalidation "k8s.io/apimachinery/pkg/api/validation/path" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + utilvalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" corevalidation "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/apis/resource" @@ -58,12 +60,31 @@ var supportedAllocationModes = sets.NewString(string(resource.AllocationModeImme func validateResourceClaimParametersRef(ref *resource.ResourceClaimParametersReference, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList - if ref != nil { - if ref.Kind == "" { - allErrs = append(allErrs, field.Required(fldPath.Child("kind"), "")) + + if ref == nil { + return allErrs + } + + // group is required but the Core group is the empty value, so it can not be enforced. + if ref.APIGroup != "" { + for _, msg := range utilvalidation.IsDNS1123Subdomain(ref.APIGroup) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("apiGroup"), ref.APIGroup, msg)) } - if ref.Name == "" { - allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) + } + + if ref.Kind == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("kind"), "")) + } else { + for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Kind) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg)) + } + } + + if ref.Name == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) + } else { + for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Name) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg)) } } return allErrs @@ -71,17 +92,38 @@ func validateResourceClaimParametersRef(ref *resource.ResourceClaimParametersRef func validateClassParameters(ref *resource.ResourceClassParametersReference, fldPath *field.Path) field.ErrorList { var allErrs field.ErrorList - if ref != nil { - if ref.Kind == "" { - allErrs = append(allErrs, field.Required(fldPath.Child("kind"), "")) + + if ref == nil { + return allErrs + } + + // group is required but the Core group is the empty value, so it can not be enforced. + if ref.APIGroup != "" { + for _, msg := range utilvalidation.IsDNS1123Subdomain(ref.APIGroup) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("apiGroup"), ref.APIGroup, msg)) } - if ref.Name == "" { - allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) + } + + if ref.Kind == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("kind"), "")) + } else { + for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Kind) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("kind"), ref.Kind, msg)) } - if ref.Namespace != "" { - for _, msg := range apimachineryvalidation.ValidateNamespaceName(ref.Namespace, false) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), ref.Namespace, msg)) - } + } + + if ref.Name == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) + } else { + for _, msg := range pathvalidation.IsValidPathSegmentName(ref.Name) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), ref.Name, msg)) + } + } + + // namespace is optional. + if ref.Namespace != "" { + for _, msg := range apimachineryvalidation.ValidateNamespaceName(ref.Namespace, false) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("namespace"), ref.Namespace, msg)) } } return allErrs diff --git a/pkg/apis/resource/validation/validation_resourceclaim_test.go b/pkg/apis/resource/validation/validation_resourceclaim_test.go index e5b59dba9f8..5c735baef53 100644 --- a/pkg/apis/resource/validation/validation_resourceclaim_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaim_test.go @@ -53,6 +53,8 @@ func TestValidateClaim(t *testing.T) { } now := metav1.Now() badValue := "spaces not allowed" + badAPIGroup := "example.com/v1" + goodAPIGroup := "example.com" scenarios := map[string]struct { claim *resource.ResourceClaim @@ -216,6 +218,29 @@ func TestValidateClaim(t *testing.T) { return claim }(), }, + "good-parameters-apigroup": { + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, goodClaimSpec) + claim.Spec.ParametersRef = &resource.ResourceClaimParametersReference{ + APIGroup: goodAPIGroup, + Kind: "foo", + Name: "bar", + } + return claim + }(), + }, + "bad-parameters-apigroup": { + 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])?)*')")}, + claim: func() *resource.ResourceClaim { + claim := testClaim(goodName, goodNS, goodClaimSpec) + claim.Spec.ParametersRef = &resource.ResourceClaimParametersReference{ + APIGroup: badAPIGroup, + Kind: "foo", + Name: "bar", + } + return claim + }(), + }, "missing-parameters-kind": { wantFailures: field.ErrorList{field.Required(field.NewPath("spec", "parametersRef", "kind"), "")}, claim: func() *resource.ResourceClaim { diff --git a/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go b/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go index 23263f36114..3f6c5504147 100644 --- a/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go +++ b/pkg/apis/resource/validation/validation_resourceclaimtemplate_test.go @@ -51,6 +51,8 @@ func TestValidateClaimTemplate(t *testing.T) { } now := metav1.Now() badValue := "spaces not allowed" + badAPIGroup := "example.com/v1" + goodAPIGroup := "example.com" scenarios := map[string]struct { template *resource.ResourceClaimTemplate @@ -214,6 +216,29 @@ func TestValidateClaimTemplate(t *testing.T) { return template }(), }, + "good-parameters-apigroup": { + template: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, goodClaimSpec) + template.Spec.Spec.ParametersRef = &resource.ResourceClaimParametersReference{ + APIGroup: goodAPIGroup, + Kind: "foo", + Name: "bar", + } + return template + }(), + }, + "bad-parameters-apigroup": { + 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])?)*')")}, + template: func() *resource.ResourceClaimTemplate { + template := testClaimTemplate(goodName, goodNS, goodClaimSpec) + template.Spec.Spec.ParametersRef = &resource.ResourceClaimParametersReference{ + APIGroup: badAPIGroup, + Kind: "foo", + Name: "bar", + } + return template + }(), + }, "missing-parameters-kind": { wantFailures: field.ErrorList{field.Required(field.NewPath("spec", "spec", "parametersRef", "kind"), "")}, template: func() *resource.ResourceClaimTemplate { diff --git a/pkg/apis/resource/validation/validation_resourceclass_test.go b/pkg/apis/resource/validation/validation_resourceclass_test.go index c84429f9406..e4de81d04e5 100644 --- a/pkg/apis/resource/validation/validation_resourceclass_test.go +++ b/pkg/apis/resource/validation/validation_resourceclass_test.go @@ -47,6 +47,8 @@ func TestValidateClass(t *testing.T) { } badName := "!@#$%^" badValue := "spaces not allowed" + badAPIGroup := "example.com/v1" + goodAPIGroup := "example.com" scenarios := map[string]struct { class *resource.ResourceClass @@ -204,6 +206,23 @@ func TestValidateClass(t *testing.T) { return class }(), }, + "good-parameters-apigroup": { + class: func() *resource.ResourceClass { + class := testClass(goodName, goodName) + class.ParametersRef = goodParameters.DeepCopy() + class.ParametersRef.APIGroup = goodAPIGroup + return class + }(), + }, + "bad-parameters-apigroup": { + 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])?)*')")}, + class: func() *resource.ResourceClass { + class := testClass(goodName, goodName) + class.ParametersRef = goodParameters.DeepCopy() + class.ParametersRef.APIGroup = badAPIGroup + return class + }(), + }, "missing-parameters-name": { wantFailures: field.ErrorList{field.Required(field.NewPath("parametersRef", "name"), "")}, class: func() *resource.ResourceClass {