add new validation to NginxIngressController CRD

api/v1alpha1/nginxingresscontroller_types.go

@@ -171,34 +171,21 @@ 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 {
 		return fmt.Sprintf("spec.controllerNamePrefix length must be less than or equal to %d characters", maxControllerNamePrefix)
 
 	}
 
-	// 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
+}

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)
+			}
+		})
+	}
+}

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)