Merge pull request #10434 from ramr/route-gen-check

Merged by openshift-bot

Author: OpenShift Bot

pkg/route/api/validation/validation.go

@@ -26,6 +26,12 @@ func ValidateRoute(route *routeapi.Route) field.ErrorList {
 
 	//host is not required but if it is set ensure it meets DNS requirements
 	if len(route.Spec.Host) > 0 {
+		// TODO: Add a better check that the host name matches up to
+		//       DNS requirements. Change to use:
+		//         ValidateHostName(route)
+		//       Need to check the implications of doing it here in
+		//       ValidateRoute - probably needs to be done only on
+		//       creation time for new routes.
 		if len(kvalidation.IsDNS1123Subdomain(route.Spec.Host)) != 0 {
 			result = append(result, field.Invalid(specPath.Child("host"), route.Spec.Host, "host must conform to DNS 952 subdomain conventions"))
 		}
@@ -160,6 +166,31 @@ func ExtendedValidateRoute(route *routeapi.Route) field.ErrorList {
 	return result
 }
 
+// ValidateHostName checks that a route's host name satisfies DNS requirements.
+func ValidateHostName(route *routeapi.Route) field.ErrorList {
+	result := field.ErrorList{}
+	if len(route.Spec.Host) < 1 {
+		return result
+	}
+
+	specPath := field.NewPath("spec")
+	hostPath := specPath.Child("host")
+
+	if len(kvalidation.IsDNS1123Subdomain(route.Spec.Host)) != 0 {
+		result = append(result, field.Invalid(hostPath, route.Spec.Host, "host must conform to DNS 952 subdomain conventions"))
+	}
+
+	segments := strings.Split(route.Spec.Host, ".")
+	for _, s := range segments {
+		errs := kvalidation.IsDNS1123Label(s)
+		for _, e := range errs {
+			result = append(result, field.Invalid(hostPath, route.Spec.Host, e))
+		}
+	}
+
+	return result
+}
+
 // validateTLS tests fields for different types of TLS combinations are set.  Called
 // by ValidateRoute.
 func validateTLS(route *routeapi.Route, fldPath *field.Path) field.ErrorList {

pkg/route/api/validation/validation_test.go

@@ -997,3 +997,81 @@ func TestExtendedValidateRoute(t *testing.T) {
 		}
 	}
 }
+
+// TestValidateHostName checks that a route's host name matches DNS requirements.
+func TestValidateHostName(t *testing.T) {
+	tests := []struct {
+		name           string
+		route          *api.Route
+		expectedErrors bool
+	}{
+		{
+			name: "valid-host-name",
+			route: &api.Route{
+				Spec: api.RouteSpec{
+					Host: "www.example.test",
+				},
+			},
+			expectedErrors: false,
+		},
+		{
+			name: "invalid-host-name",
+			route: &api.Route{
+				Spec: api.RouteSpec{
+					Host: "name-namespace-1234567890-1234567890-1234567890-1234567890-1234567890-1234567890-1234567890.example.test",
+				},
+			},
+			expectedErrors: true,
+		},
+		{
+			name: "valid-host-63-chars-label",
+			route: &api.Route{
+				Spec: api.RouteSpec{
+					Host: "name-namespace-1234567890-1234567890-1234567890-1234567890-1234.example.test",
+				},
+			},
+			expectedErrors: false,
+		},
+		{
+			name: "invalid-host-64-chars-label",
+			route: &api.Route{
+				Spec: api.RouteSpec{
+					Host: "name-namespace-1234567890-1234567890-1234567890-1234567890-12345.example.test",
+				},
+			},
+			expectedErrors: true,
+		},
+		{
+			name: "valid-name-253-chars",
+			route: &api.Route{
+				Spec: api.RouteSpec{
+					Host: "name-namespace.a1234567890.b1234567890.c1234567890.d1234567890.e1234567890.f1234567890.g1234567890.h1234567890.i1234567890.j1234567890.k1234567890.l1234567890.m1234567890.n1234567890.o1234567890.p1234567890.q1234567890.r1234567890.s12345678.example.test",
+				},
+			},
+			expectedErrors: false,
+		},
+		{
+			name: "invalid-name-279-chars",
+			route: &api.Route{
+				Spec: api.RouteSpec{
+					Host: "name-namespace.a1234567890.b1234567890.c1234567890.d1234567890.e1234567890.f1234567890.g1234567890.h1234567890.i1234567890.j1234567890.k1234567890.l1234567890.m1234567890.n1234567890.o1234567890.p1234567890.q1234567890.r1234567890.s1234567890.t1234567890.u1234567890.example.test",
+				},
+			},
+			expectedErrors: true,
+		},
+	}
+
+	for _, tc := range tests {
+		errs := ValidateHostName(tc.route)
+
+		if tc.expectedErrors {
+			if len(errs) < 1 {
+				t.Errorf("Test case %s expected errors, got none.", tc.name)
+			}
+		} else {
+			if len(errs) > 0 {
+				t.Errorf("Test case %s expected no errors, got %d. %v", tc.name, len(errs), errs)
+			}
+		}
+	}
+}

pkg/router/controller/unique_host.go

@@ -2,13 +2,15 @@ package controller
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/golang/glog"
 	kapi "k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/watch"
 
 	routeapi "github.com/openshift/origin/pkg/route/api"
+	"github.com/openshift/origin/pkg/route/api/validation"
 	"github.com/openshift/origin/pkg/router"
 )
 
@@ -108,6 +110,20 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routeapi.Rout
 	}
 	route.Spec.Host = host
 
+	// Run time check to defend against older routes. Validate that the
+	// route host name conforms to DNS requirements.
+	if errs := validation.ValidateHostName(route); len(errs) > 0 {
+		glog.V(4).Infof("Route %s - invalid host name %s", routeName, host)
+		errMessages := make([]string, len(errs))
+		for i := 0; i < len(errs); i++ {
+			errMessages[i] = errs[i].Error()
+		}
+
+		err := fmt.Errorf("host name validation errors: %s", strings.Join(errMessages, ", "))
+		p.recorder.RecordRouteRejection(route, "InvalidHost", err.Error())
+		return err
+	}
+
 	// ensure hosts can only be claimed by one namespace at a time
 	// TODO: this could be abstracted above this layer?
 	if old, ok := p.hostToRoute[host]; ok {