Skip to content

Commit

Permalink
Support mixed case Kind
Browse files Browse the repository at this point in the history
The implementation
* uses the same validation logic that runs against `Kind` in
apimachinery; i.e. allows any `Kind` value that, when converted to
lowercase, is a valid DNS-1035 label.
* additionally requires that `Kind`s  start with an uppercase character,
  to ensure they map to exported types.

Co-Authored-By: Hannes Hörl <[email protected]>
Co-Authored-By: Camila Macedo <[email protected]>
  • Loading branch information
3 people committed Feb 3, 2020
1 parent c6e1ee1 commit e49d943
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 20 deletions.
14 changes: 11 additions & 3 deletions pkg/model/resource/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,17 @@ func (opts *Options) Validate() error {
"version must match %s (was %s)", versionPattern, opts.Version)
}

// Check if the Kind is PascalCase
if opts.Kind != flect.Pascalize(opts.Kind) {
return fmt.Errorf("kind must be PascalCase (expected %s was %s)", flect.Pascalize(opts.Kind), opts.Kind)
validationErrors := []string{}

// require Kind to start with an uppercase character
if string(opts.Kind[0]) == strings.ToLower(string(opts.Kind[0])) {
validationErrors = append(validationErrors, "Kind must start with an uppercase character")
}

validationErrors = append(validationErrors, isDNS1035Label(strings.ToLower(opts.Kind))...)

if len(validationErrors) != 0 {
return fmt.Errorf("Invalid Kind: %#v", validationErrors)
}

// TODO: validate plural strings if provided
Expand Down
51 changes: 34 additions & 17 deletions pkg/model/resource/options_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package resource_test

import (
"strings"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"

. "sigs.k8s.io/kubebuilder/pkg/model/resource"
Expand Down Expand Up @@ -73,24 +76,38 @@ var _ = Describe("Resource Options", func() {
Expect(options.Validate().Error()).To(ContainSubstring("kind cannot be empty"))
})

It("should fail if the Kind is not pascal cased", func() {
// Base case
options := &Options{Group: "crew", Version: "v1", Kind: "FirstMate"}
Expect(options.Validate()).To(Succeed())

// Can't detect this case :(
options = &Options{Group: "crew", Version: "v1", Kind: "Firstmate"}
Expect(options.Validate()).To(Succeed())

options = &Options{Group: "crew", Version: "v1", Kind: "firstMate"}
Expect(options.Validate()).NotTo(Succeed())
Expect(options.Validate().Error()).To(ContainSubstring(
`kind must be PascalCase (expected FirstMate was firstMate)`))
DescribeTable("valid Kind values-according to core Kubernetes",
func(kind string) {
options := &Options{Group: "crew", Kind: kind, Version: "v1"}
Expect(options.Validate()).To(Succeed())
},
Entry("should pass validation if Kind is camelcase", "FirstMate"),
Entry("should pass validation if Kind has more than one caps at the start", "FIRSTMate"),
)

It("should fail if Kind is too long", func() {
kind := strings.Repeat("a", 64)

options := &Options{Group: "crew", Kind: kind, Version: "v1"}
err := options.Validate()
Expect(err).To(MatchError(ContainSubstring("must be no more than 63 characters")))
})

options = &Options{Group: "crew", Version: "v1", Kind: "firstmate"}
Expect(options.Validate()).NotTo(Succeed())
Expect(options.Validate().Error()).To(ContainSubstring(
`kind must be PascalCase (expected Firstmate was firstmate)`))
DescribeTable("invalid Kind values-according to core Kubernetes",
func(kind string) {
options := &Options{Group: "crew", Kind: kind, Version: "v1"}
Expect(options.Validate()).To(MatchError(
ContainSubstring("a DNS-1035 label must consist of lower case alphanumeric characters")))
},
Entry("should fail validation if Kind contains whitespaces", "Something withSpaces"),
Entry("should fail validation if Kind ends in -", "KindEndingIn-"),
Entry("should fail validation if Kind starts with number", "0ValidityKind"),
)

It("should fail if Kind starts with a lowercase character", func() {
options := &Options{Group: "crew", Kind: "lOWERCASESTART", Version: "v1"}
err := options.Validate()
Expect(err).To(MatchError(ContainSubstring("Kind must start with an uppercase character")))
})
})
})
19 changes: 19 additions & 0 deletions pkg/model/resource/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,17 @@ const (

// dns1123SubdomainMaxLength is a subdomain's max length in DNS (RFC 1123)
dns1123SubdomainMaxLength int = 253

dns1035LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
dns1035LabelErrMsg string = "a DNS-1035 label must consist of lower case alphanumeric characters or '-', " +
"start with an alphabetic character, and end with an alphanumeric character"

// DNS1035LabelMaxLength is a label's max length in DNS (RFC 1035)
dns1035LabelMaxLength int = 63
)

var dns1123SubdomainRegexp = regexp.MustCompile("^" + dns1123SubdomainFmt + "$")
var dns1035LabelRegexp = regexp.MustCompile("^" + dns1035LabelFmt + "$")

// IsDNS1123Subdomain tests for a string that conforms to the definition of a
// subdomain in DNS (RFC 1123).
Expand All @@ -50,6 +58,17 @@ func IsDNS1123Subdomain(value string) []string {
return errs
}

func isDNS1035Label(value string) []string {
var errs []string
if len(value) > dns1035LabelMaxLength {
errs = append(errs, maxLenError(dns1035LabelMaxLength))
}
if !dns1035LabelRegexp.MatchString(value) {
errs = append(errs, regexError(dns1035LabelErrMsg, dns1035LabelFmt, "my-name", "abc-123"))
}
return errs
}

// MaxLenError returns a string explanation of a "string too long" validation
// failure.
func maxLenError(length int) string {
Expand Down

0 comments on commit e49d943

Please sign in to comment.