Address feedbacks

This commit is contained in:
Lalit Chauhan
2025-10-27 17:18:52 +00:00
parent 94101ba6e0
commit 37ba7bb454
14 changed files with 81 additions and 89 deletions

View File

@@ -33,6 +33,7 @@ import (
"k8s.io/apimachinery/pkg/api/operation"
apiresource "k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/api/validate"
"k8s.io/apimachinery/pkg/api/validate/content"
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation"
"k8s.io/apimachinery/pkg/runtime"
@@ -1087,11 +1088,6 @@ func validateDeviceCounter(counter resource.Counter, fldPath *field.Path) field.
func validateQualifiedName(name resource.QualifiedName, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
if name == "" {
allErrs = append(allErrs, field.Required(fldPath, "name required"))
return allErrs
}
parts := strings.Split(string(name), "/")
switch len(parts) {
case 1:
@@ -1125,15 +1121,11 @@ func validateQualifiedName(name resource.QualifiedName, fldPath *field.Path) fie
func validateFullyQualifiedName(name resource.FullyQualifiedName, fldPath *field.Path) field.ErrorList {
var allErrs field.ErrorList
if name == "" {
allErrs = append(allErrs, field.Required(fldPath, "name required"))
return allErrs
}
allErrs = append(allErrs, validateQualifiedName(resource.QualifiedName(name), fldPath)...)
// validateQualifiedName checks that the name isn't empty and both parts are valid.
// validateQualifiedName checks that both parts are valid.
// What we need to enforce here is that there really is a domain.
if name != "" && !strings.Contains(string(name), "/") {
allErrs = append(allErrs, field.Invalid(fldPath, name, "must include a domain"))
if !strings.Contains(string(name), "/") {
allErrs = append(allErrs, field.Invalid(fldPath, name, "a fully qualified name must be a domain and a name separated by a slash"))
}
return allErrs.WithOrigin("format=k8s-resource-fully-qualified-name")
}
@@ -1143,7 +1135,7 @@ func validateCIdentifier(id string, fldPath *field.Path) field.ErrorList {
if len(id) > resource.DeviceMaxIDLength {
allErrs = append(allErrs, field.TooLong(fldPath, "" /*unused*/, resource.DeviceMaxIDLength))
}
for _, msg := range validation.IsCIdentifier(id) {
for _, msg := range content.IsCIdentifier(id) {
allErrs = append(allErrs, field.Invalid(fldPath, id, msg))
}
return allErrs

View File

@@ -263,8 +263,9 @@ func TestValidateClaim(t *testing.T) {
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "missing-domain", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), resource.FullyQualifiedName("missing-domain"), "must include a domain").MarkCoveredByDeclarative(),
field.Required(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), "name required").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), resource.FullyQualifiedName("missing-domain"), "a fully qualified name must be a domain and a name separated by a slash").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), "", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), resource.FullyQualifiedName(""), "a fully qualified name must be a domain and a name separated by a slash").MarkCoveredByDeclarative(),
field.Required(field.NewPath("spec", "devices", "constraints").Index(2).Child("matchAttribute"), ""),
field.TooMany(field.NewPath("spec", "devices", "constraints"), resource.DeviceConstraintsMaxSize+1, resource.DeviceConstraintsMaxSize).MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"),
@@ -322,8 +323,9 @@ func TestValidateClaim(t *testing.T) {
"invalid-distinct-constraint": {
wantFailures: field.ErrorList{
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("distinctAttribute"), "missing-domain", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("distinctAttribute"), resource.FullyQualifiedName("missing-domain"), "must include a domain"),
field.Required(field.NewPath("spec", "devices", "constraints").Index(1).Child("distinctAttribute"), "name required"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("distinctAttribute"), resource.FullyQualifiedName("missing-domain"), "a fully qualified name must be a domain and a name separated by a slash"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(1).Child("distinctAttribute"), "", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(1).Child("distinctAttribute"), resource.FullyQualifiedName(""), "a fully qualified name must be a domain and a name separated by a slash"),
field.Required(field.NewPath("spec", "devices", "constraints").Index(2), `exactly one of "matchAttribute" or "distinctAttribute" is required, but multiple fields are set`)},
claim: func() *resource.ResourceClaim {
claim := testClaim(goodName, goodNS, validClaimSpec)
@@ -375,8 +377,9 @@ func TestValidateClaim(t *testing.T) {
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "missing-domain", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), resource.FullyQualifiedName("missing-domain"), "must include a domain").MarkCoveredByDeclarative(),
field.Required(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), "name required").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), resource.FullyQualifiedName("missing-domain"), "a fully qualified name must be a domain and a name separated by a slash").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), "", "a valid C identifier must start with alphabetic character or '_', followed by a string of alphanumeric characters or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', regex used for validation is '[A-Za-z_][A-Za-z0-9_]*')").MarkCoveredByDeclarative(),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(1).Child("matchAttribute"), resource.FullyQualifiedName(""), "a fully qualified name must be a domain and a name separated by a slash").MarkCoveredByDeclarative(),
field.Required(field.NewPath("spec", "devices", "constraints").Index(2).Child("matchAttribute"), ""),
field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "a lowercase RFC 1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name', or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?')"),
field.Invalid(field.NewPath("spec", "devices", "config").Index(0).Child("requests").Index(1), badName, "must be the name of a request in the claim or the name of a request and a subrequest separated by '/'"),

View File

@@ -406,7 +406,7 @@ func testDeclarativeValidate(t *testing.T, apiVersion string) {
tweakMatchAttribute("nodomain"),
),
expectedErrs: field.ErrorList{
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "nodomain", "must include a domain").WithOrigin("format=k8s-resource-fully-qualified-name"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "nodomain", "a fully qualified name must be a domain and a name separated by a slash").WithOrigin("format=k8s-resource-fully-qualified-name"),
},
},
"match attribute empty": {
@@ -414,7 +414,7 @@ func testDeclarativeValidate(t *testing.T, apiVersion string) {
tweakMatchAttribute(""),
),
expectedErrs: field.ErrorList{
field.Required(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "name required"),
field.Invalid(field.NewPath("spec", "devices", "constraints").Index(0).Child("matchAttribute"), "", "").WithOrigin("format=k8s-resource-fully-qualified-name"),
},
},
"match attribute with empty domain": {