diff --git a/api/v1alpha1/nginxingresscontroller_types.go b/api/v1alpha1/nginxingresscontroller_types.go index c20a47c6..a21a8393 100644 --- a/api/v1alpha1/nginxingresscontroller_types.go +++ b/api/v1alpha1/nginxingresscontroller_types.go @@ -171,12 +171,8 @@ func (n *NginxIngressController) Valid() string { return "spec.controllerNamePrefix must be specified" } - if !startsWithAlphaNum(n.Spec.ControllerNamePrefix) { - return "spec.controllerNamePrefix must start with alphanumeric character" - } - - if !onlyAlphaNumDashPeriod(n.Spec.ControllerNamePrefix) { - return "spec.controllerNamePrefix must contain only alphanumeric characters, dashes, and periods" + if !isLowercaseRfc1123Subdomain(n.Spec.ControllerNamePrefix) { + return "spec.controllerNamePrefix " + lowercaseRfc1123SubdomainValidationFailReason } if len(n.Spec.ControllerNamePrefix) > maxControllerNamePrefix { @@ -184,21 +180,12 @@ func (n *NginxIngressController) Valid() string { } - // ingress class name must follow https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names if n.Spec.IngressClassName == "" { return "spec.ingressClassName must be specified" } - if !startsWithAlphaNum(n.Spec.IngressClassName) { - return "spec.ingressClassName must start with alphanumeric character" - } - - if !onlyAlphaNumDashPeriod(n.Spec.IngressClassName) { - return "spec.ingressClassName must contain only alphanumeric characters, dashes, and periods" - } - - if !endsWithAlphaNum(n.Spec.IngressClassName) { - return "spec.ingressClassName must end with alphanumeric character" + if !isLowercaseRfc1123Subdomain(n.Spec.IngressClassName) { + return "spec.ingressClassName " + lowercaseRfc1123SubdomainValidationFailReason } if len(n.Name) > maxNameLength { @@ -228,6 +215,28 @@ type NginxIngressControllerList struct { Items []NginxIngressController `json:"items"` } +var lowercaseRfc1123SubdomainValidationFailReason = "must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character" + +func isLowercaseRfc1123Subdomain(s string) bool { + if !startsWithAlphaNum(s) { + return false + } + + if !endsWithAlphaNum(s) { + return false + } + + if !onlyAlphaNumDashPeriod(s) { + return false + } + + if !isLower(s) { + return false + } + + return true +} + func startsWithAlphaNum(s string) bool { if len(s) == 0 { return false @@ -253,3 +262,13 @@ func onlyAlphaNumDashPeriod(s string) bool { return true } + +func isLower(s string) bool { + for _, c := range s { + if unicode.IsUpper(c) && unicode.IsLetter(c) { + return false + } + } + + return true +} diff --git a/api/v1alpha1/nginxingresscontroller_types_test.go b/api/v1alpha1/nginxingresscontroller_types_test.go index 861f460f..47b5cfc0 100644 --- a/api/v1alpha1/nginxingresscontroller_types_test.go +++ b/api/v1alpha1/nginxingresscontroller_types_test.go @@ -15,8 +15,8 @@ func validNginxIngressController() NginxIngressController { Name: "name", }, Spec: NginxIngressControllerSpec{ - IngressClassName: "ingressClassName", - ControllerNamePrefix: "controllerNamePrefix", + IngressClassName: "ingressclassname.com", + ControllerNamePrefix: "controller-name-prefix", }, } } @@ -45,19 +45,19 @@ func TestNginxIngressControllerValid(t *testing.T) { name: "controller name prefix starts with non alphanum", nic: func() NginxIngressController { nic := validNginxIngressController() - nic.Spec.ControllerNamePrefix = "-controllerNamePrefix" + nic.Spec.ControllerNamePrefix = "-controllernameprefix" return nic }(), - want: "spec.controllerNamePrefix must start with alphanumeric character", + want: "spec.controllerNamePrefix must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", }, { name: "controller name prefix contains invalid characters", nic: func() NginxIngressController { nic := validNginxIngressController() - nic.Spec.ControllerNamePrefix = "controllerNamePrefix!" + nic.Spec.ControllerNamePrefix = "controllernameprefix!" return nic }(), - want: "spec.controllerNamePrefix must contain only alphanumeric characters, dashes, and periods", + want: "spec.controllerNamePrefix must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", }, { name: "controller name prefix too long", @@ -68,6 +68,15 @@ func TestNginxIngressControllerValid(t *testing.T) { }(), want: fmt.Sprintf("spec.controllerNamePrefix length must be less than or equal to %d characters", maxControllerNamePrefix), }, + { + name: "controller name prefix capitalized", + nic: func() NginxIngressController { + nic := validNginxIngressController() + nic.Spec.ControllerNamePrefix = "ControllerNamePrefix" + return nic + }(), + want: "spec.controllerNamePrefix must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", + }, { name: "missing ingress class name", nic: func() NginxIngressController { @@ -77,32 +86,59 @@ func TestNginxIngressControllerValid(t *testing.T) { }(), want: "spec.ingressClassName must be specified", }, + { + name: "ingress class name capitalized", + nic: func() NginxIngressController { + nic := validNginxIngressController() + nic.Spec.IngressClassName = "IngressClassName" + return nic + }(), + want: "spec.ingressClassName must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", + }, + { + name: "ingress class name capitalized with special characters", + nic: func() NginxIngressController { + nic := validNginxIngressController() + nic.Spec.IngressClassName = "ingress-Class.Name" + return nic + }(), + want: "spec.ingressClassName must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", + }, + { + name: "ingress class name with special characters", + nic: func() NginxIngressController { + nic := validNginxIngressController() + nic.Spec.IngressClassName = "ingress-class.name" + return nic + }(), + want: "", + }, { name: "ingress class name starts with non alphanum", nic: func() NginxIngressController { nic := validNginxIngressController() - nic.Spec.IngressClassName = "-ingressClassName" + nic.Spec.IngressClassName = "-ingressclassname" return nic }(), - want: "spec.ingressClassName must start with alphanumeric character", + want: "spec.ingressClassName must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", }, { name: "ingress class name contains invalid characters", nic: func() NginxIngressController { nic := validNginxIngressController() - nic.Spec.IngressClassName = "ingressClassName!" + nic.Spec.IngressClassName = "ingressclassname!" return nic }(), - want: "spec.ingressClassName must contain only alphanumeric characters, dashes, and periods", + want: "spec.ingressClassName must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", }, { name: "ingress class name ends with non alphanum", nic: func() NginxIngressController { nic := validNginxIngressController() - nic.Spec.IngressClassName = "ingressClassName-" + nic.Spec.IngressClassName = "ingressclassname-" return nic }(), - want: "spec.ingressClassName must end with alphanumeric character", + want: "spec.ingressClassName must be a lowercase RFC 1123 subdomain consisting of lowercase alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character", }, { name: "long name", @@ -443,3 +479,144 @@ func TestOnlyAlphaNumDashPeriod(t *testing.T) { }) } } + +func TestIsLower(t *testing.T) { + cases := []struct { + name string + s string + want bool + }{ + { + name: "lower", + s: "abc", + want: true, + }, + { + name: "upper", + s: "ABC", + want: false, + }, + { + name: "mixed", + s: "AbC", + want: false, + }, + { + name: "empty", + s: "", + want: true, + }, + { + name: "lower with space", + s: "abc ", + want: true, + }, + { + name: "lower with underscore", + s: "abc_", + want: true, + }, + { + name: "lower with dash", + s: "abc-", + want: true, + }, + { + name: "lower with period", + s: "abc.", + want: true, + }, + { + name: "upper with space", + s: "ABC ", + want: false, + }, + { + name: "upper with underscore", + s: "ABC_", + want: false, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := isLower(c.s) + if got != c.want { + t.Errorf("isLower(%v) = %v, want %v", c.s, got, c.want) + } + }) + } +} + +func TestIsLowercaseRfc1123Subdomain(t *testing.T) { + cases := []struct { + name string + s string + want bool + }{ + { + name: "lower", + s: "abc", + want: true, + }, + { + name: "upper", + s: "ABC", + want: false, + }, + { + name: "mixed", + s: "AbC", + want: false, + }, + { + name: "empty", + s: "", + want: false, + }, + { + name: "lower with space", + s: "abc ", + want: false, + }, + { + name: "lower with ending underscore", + s: "abc_", + want: false, + }, + { + name: "lower with ending dash", + s: "abc-", + want: false, + }, + { + name: "lower with ending period", + s: "abc.", + want: false, + }, + { + name: "lower with middle underscore", + s: "ab_c", + want: false, + }, + { + name: "lower with middle dash", + s: "ab-c", + want: true, + }, + { + name: "lower with middle period", + s: "ab.c", + want: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := isLowercaseRfc1123Subdomain(c.s) + if got != c.want { + t.Errorf("isLowercaseRfc1123Subdomain(%v) = %v, want %v", c.s, got, c.want) + } + }) + } +} diff --git a/testing/e2e/suites/nginxIngressController.go b/testing/e2e/suites/nginxIngressController.go index 4ecdba14..45f88d1f 100644 --- a/testing/e2e/suites/nginxIngressController.go +++ b/testing/e2e/suites/nginxIngressController.go @@ -60,7 +60,7 @@ func nicWebhookTests(in infra.Provisioned) []test { return fmt.Errorf("creating client: %w") } - testNIC := manifests.NewNginxIngressController("nginx-ingress-controller", "testNICIngressClass") + testNIC := manifests.NewNginxIngressController("nginx-ingress-controller", "test-nic-ingress-class") lgr.Info("creating basic NginxIngressController") if err := upsert(ctx, c, testNIC); err != nil { return fmt.Errorf("creating NginxIngressController: %w", err)