Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module sigs.k8s.io/kubebuilder
go 1.13

require (
github.com/blang/semver v3.5.1+incompatible
github.com/gobuffalo/flect v0.1.5
github.com/golang/protobuf v1.3.1 // indirect
github.com/kr/pretty v0.1.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ=
github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
Expand Down
38 changes: 38 additions & 0 deletions pkg/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/spf13/cobra"

internalconfig "sigs.k8s.io/kubebuilder/internal/config"
"sigs.k8s.io/kubebuilder/pkg/internal/validation"
"sigs.k8s.io/kubebuilder/pkg/model/config"
"sigs.k8s.io/kubebuilder/pkg/plugin"
)
Expand Down Expand Up @@ -146,6 +147,12 @@ func (c *cli) initialize() error {
return fmt.Errorf("failed to read config: %v", err)
}

// Validate after setting projectVersion but before buildRootCmd so we error
// out before an error resulting from an incorrect cli is returned downstream.
if err = c.validate(); err != nil {
return err
}

c.cmd = c.buildRootCmd()

// Add extra commands injected by options.
Expand Down Expand Up @@ -175,6 +182,37 @@ func (c *cli) initialize() error {
return nil
}

// validate validates fields in a cli.
func (c cli) validate() error {
// Validate project versions.
if err := validation.ValidateProjectVersion(c.defaultProjectVersion); err != nil {
return fmt.Errorf("failed to validate default project version %q: %v", c.defaultProjectVersion, err)
}
if err := validation.ValidateProjectVersion(c.projectVersion); err != nil {
return fmt.Errorf("failed to validate project version %q: %v", c.projectVersion, err)
}

// Validate plugin versions and name.
for _, versionedPlugins := range c.plugins {
for _, versionedPlugin := range versionedPlugins {
pluginName := versionedPlugin.Name()
pluginVersion := versionedPlugin.Version()
if err := plugin.ValidateVersion(pluginVersion); err != nil {
return fmt.Errorf("failed to validate plugin %q version %q: %v",
pluginName, pluginVersion, err)
}
for _, projectVersion := range versionedPlugin.SupportedProjectVersions() {
if err := validation.ValidateProjectVersion(projectVersion); err != nil {
return fmt.Errorf("failed to validate plugin %q supported project version %q: %v",
pluginName, projectVersion, err)
}
}
}
}

return nil
}

// buildRootCmd returns a root command with a subcommand tree reflecting the
// current project's state.
func (c cli) buildRootCmd() *cobra.Command {
Expand Down
114 changes: 114 additions & 0 deletions pkg/internal/validation/dns.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package validation

import (
"fmt"
"regexp"
)

// This file's code was modified from "k8s.io/apimachinery/pkg/util/validation"
// to avoid package dependencies. In case of additional functionality from
// "k8s.io/apimachinery" is needed, re-consider whether to add the dependency.

const (
dns1123LabelFmt string = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
dns1123SubdomainFmt string = dns1123LabelFmt + "(\\." + dns1123LabelFmt + ")*"
dns1035LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?"
)

type dnsValidationConfig struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Adirio @camilamacedo86 refactored these for convenience after rebasing onto the feature branch, PTAL.

Copy link
Contributor

@Adirio Adirio Feb 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do like the refactor you did, avoiding code duplications through the use of the dnsValidationConfig struct and its check method. A couple of considerations:

  • When I read IsDNS1XXXLabel I instantly imagine a func (string) bool signature. As the different errors need to be differentiate, we can not change the function signature to that one, so what about changing the name? Maybe around MustBe... or ConformsTo.... I guess we could check how the standard library
  • Returning a []string containing the errors doesn't seem very idiomatic in Go. Maybe we should just return an error and have the user solve the problem in two steps in case he encounters two errors, as the second one will not be reported until the first one is solved.
  • Would it make sense to group functions for the same specification? Something in the line of validation.DNS1XXX.ConformsToLabel(myString). You would have either to export the struct instance or an interface to it:
type DNSValidator interface {
	ConformsToLabel(string) error
	ConformsToSubdomain(string) error
}

var (
	DNS1023 DNSValidator = dns1023Config
	DNS1135 DNSValidator = dns1135Config
)

They can slo be left for further PRs if you prefer, they are just ideas.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming and return values could use some work. Doing this in a follow-up sounds good to me.

format string
maxLen int
re *regexp.Regexp
errMsg string
examples []string
}

var dns1123LabelConfig = dnsValidationConfig{
format: dns1123LabelFmt,
maxLen: 56, // = 63 - len("-system")
Copy link
Contributor

@Adirio Adirio Feb 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: I think the validation package should not know about the "-system" prefix, the method should be called with that already prefixed and the maxLen field should be left to 63.

Copy link
Member

@camilamacedo86 camilamacedo86 Feb 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarifies: The project name will be used as part of the namespace added in the files.
So, it will be <projectname-system>. In this way, when we scaffold the project we cannot allow a projectName with more than 56 characters. However, I agree that how it will be implemented as a hall for improvements like you described.

However, it may not part of the context of this PR as well.

re: regexp.MustCompile("^" + dns1123LabelFmt + "$"),
errMsg: "a DNS-1123 label must consist of lower case alphanumeric characters or '-'",
examples: []string{"example.com"},
}

var dns1123SubdomainConfig = dnsValidationConfig{
format: dns1123SubdomainFmt,
maxLen: 253, // a subdomain's max length in DNS (RFC 1123).
re: regexp.MustCompile("^" + dns1123SubdomainFmt + "$"),
errMsg: "a DNS-1123 subdomain must consist of lower case alphanumeric characters, " +
"'-' or '.', and must start and end with an alphanumeric character",
examples: []string{"my-name", "abc-123"},
}

var dns1035LabelConfig = dnsValidationConfig{
format: dns1035LabelFmt,
maxLen: 63, // a label's max length in DNS (RFC 1035).
re: regexp.MustCompile("^" + dns1035LabelFmt + "$"),
errMsg: "a DNS-1035 label must consist of lower case alphanumeric characters or '-', " +
"start with an alphabetic character, and end with an alphanumeric character",
examples: []string{"my-name", "123-abc"},
}

func (c dnsValidationConfig) check(value string) (errs []string) {
if len(value) > c.maxLen {
errs = append(errs, maxLenError(c.maxLen))
}
if !c.re.MatchString(value) {
errs = append(errs, regexError(c.errMsg, c.format, c.examples...))
}
return errs
}

// IsDNS1123Subdomain tests for a string that conforms to the definition of a
// subdomain in DNS (RFC 1123).
func IsDNS1123Subdomain(value string) []string {
return dns1123SubdomainConfig.check(value)
}

// IsDNS1123Label tests for a string that conforms to the definition of a label in DNS (RFC 1123).
func IsDNS1123Label(value string) []string {
return dns1123LabelConfig.check(value)
}

// IsDNS1035Label tests for a string that conforms to the definition of a label in DNS (RFC 1035).
func IsDNS1035Label(value string) []string {
return dns1035LabelConfig.check(value)
}

// maxLenError returns a string explanation of a "string too long" validation
// failure.
func maxLenError(length int) string {
return fmt.Sprintf("must be no more than %d characters", length)
}

// regexError returns a string explanation of a regex validation failure.
func regexError(msg string, fmt string, examples ...string) string {
if len(examples) == 0 {
return msg + " (regex used for validation is '" + fmt + "')"
}
msg += " (e.g. "
for i := range examples {
if i > 0 {
msg += " or "
}
msg += "'" + examples[i] + "', "
}
msg += "regex used for validation is '" + fmt + "')"
return msg
}
38 changes: 38 additions & 0 deletions pkg/internal/validation/project.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
Copyright 2020 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package validation

import (
"errors"
"regexp"
)

// projectVersionFmt defines the project version format from a project config.
const projectVersionFmt string = "[1-9][0-9]*(-(alpha|beta))?"

var projectVersionRe = regexp.MustCompile("^" + projectVersionFmt + "$")

// ValidateProjectVersion ensures version adheres to the project version format.
func ValidateProjectVersion(version string) error {
if version == "" {
return errors.New("project version is empty")
}
if !projectVersionRe.MatchString(version) {
return errors.New(regexError("invalid value for project version", projectVersionFmt))
}
return nil
}
5 changes: 3 additions & 2 deletions pkg/model/resource/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"github.com/gobuffalo/flect"

"sigs.k8s.io/kubebuilder/pkg/internal/validation"
"sigs.k8s.io/kubebuilder/pkg/model/config"
)

Expand Down Expand Up @@ -116,7 +117,7 @@ func (opts *Options) Validate() error {
}

// Check if the Group has a valid DNS1123 subdomain value
if err := IsDNS1123Subdomain(opts.Group); err != nil {
if err := validation.IsDNS1123Subdomain(opts.Group); err != nil {
return fmt.Errorf("group name is invalid: (%v)", err)
}

Expand All @@ -133,7 +134,7 @@ func (opts *Options) Validate() error {
validationErrors = append(validationErrors, "Kind must start with an uppercase character")
}

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

if len(validationErrors) != 0 {
return fmt.Errorf("Invalid Kind: %#v", validationErrors)
Expand Down
92 changes: 0 additions & 92 deletions pkg/model/resource/validation.go

This file was deleted.

Loading