Browse code

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

Merged by openshift-bot

OpenShift Bot authored on 2016/10/31 22:57:45
Showing 3 changed files
... ...
@@ -26,6 +26,12 @@ func ValidateRoute(route *routeapi.Route) field.ErrorList {
26 26
 
27 27
 	//host is not required but if it is set ensure it meets DNS requirements
28 28
 	if len(route.Spec.Host) > 0 {
29
+		// TODO: Add a better check that the host name matches up to
30
+		//       DNS requirements. Change to use:
31
+		//         ValidateHostName(route)
32
+		//       Need to check the implications of doing it here in
33
+		//       ValidateRoute - probably needs to be done only on
34
+		//       creation time for new routes.
29 35
 		if len(kvalidation.IsDNS1123Subdomain(route.Spec.Host)) != 0 {
30 36
 			result = append(result, field.Invalid(specPath.Child("host"), route.Spec.Host, "host must conform to DNS 952 subdomain conventions"))
31 37
 		}
... ...
@@ -160,6 +166,31 @@ func ExtendedValidateRoute(route *routeapi.Route) field.ErrorList {
160 160
 	return result
161 161
 }
162 162
 
163
+// ValidateHostName checks that a route's host name satisfies DNS requirements.
164
+func ValidateHostName(route *routeapi.Route) field.ErrorList {
165
+	result := field.ErrorList{}
166
+	if len(route.Spec.Host) < 1 {
167
+		return result
168
+	}
169
+
170
+	specPath := field.NewPath("spec")
171
+	hostPath := specPath.Child("host")
172
+
173
+	if len(kvalidation.IsDNS1123Subdomain(route.Spec.Host)) != 0 {
174
+		result = append(result, field.Invalid(hostPath, route.Spec.Host, "host must conform to DNS 952 subdomain conventions"))
175
+	}
176
+
177
+	segments := strings.Split(route.Spec.Host, ".")
178
+	for _, s := range segments {
179
+		errs := kvalidation.IsDNS1123Label(s)
180
+		for _, e := range errs {
181
+			result = append(result, field.Invalid(hostPath, route.Spec.Host, e))
182
+		}
183
+	}
184
+
185
+	return result
186
+}
187
+
163 188
 // validateTLS tests fields for different types of TLS combinations are set.  Called
164 189
 // by ValidateRoute.
165 190
 func validateTLS(route *routeapi.Route, fldPath *field.Path) field.ErrorList {
... ...
@@ -997,3 +997,81 @@ func TestExtendedValidateRoute(t *testing.T) {
997 997
 		}
998 998
 	}
999 999
 }
1000
+
1001
+// TestValidateHostName checks that a route's host name matches DNS requirements.
1002
+func TestValidateHostName(t *testing.T) {
1003
+	tests := []struct {
1004
+		name           string
1005
+		route          *api.Route
1006
+		expectedErrors bool
1007
+	}{
1008
+		{
1009
+			name: "valid-host-name",
1010
+			route: &api.Route{
1011
+				Spec: api.RouteSpec{
1012
+					Host: "www.example.test",
1013
+				},
1014
+			},
1015
+			expectedErrors: false,
1016
+		},
1017
+		{
1018
+			name: "invalid-host-name",
1019
+			route: &api.Route{
1020
+				Spec: api.RouteSpec{
1021
+					Host: "name-namespace-1234567890-1234567890-1234567890-1234567890-1234567890-1234567890-1234567890.example.test",
1022
+				},
1023
+			},
1024
+			expectedErrors: true,
1025
+		},
1026
+		{
1027
+			name: "valid-host-63-chars-label",
1028
+			route: &api.Route{
1029
+				Spec: api.RouteSpec{
1030
+					Host: "name-namespace-1234567890-1234567890-1234567890-1234567890-1234.example.test",
1031
+				},
1032
+			},
1033
+			expectedErrors: false,
1034
+		},
1035
+		{
1036
+			name: "invalid-host-64-chars-label",
1037
+			route: &api.Route{
1038
+				Spec: api.RouteSpec{
1039
+					Host: "name-namespace-1234567890-1234567890-1234567890-1234567890-12345.example.test",
1040
+				},
1041
+			},
1042
+			expectedErrors: true,
1043
+		},
1044
+		{
1045
+			name: "valid-name-253-chars",
1046
+			route: &api.Route{
1047
+				Spec: api.RouteSpec{
1048
+					Host: "name-namespace.a1234567890.b1234567890.c1234567890.d1234567890.e1234567890.f1234567890.g1234567890.h1234567890.i1234567890.j1234567890.k1234567890.l1234567890.m1234567890.n1234567890.o1234567890.p1234567890.q1234567890.r1234567890.s12345678.example.test",
1049
+				},
1050
+			},
1051
+			expectedErrors: false,
1052
+		},
1053
+		{
1054
+			name: "invalid-name-279-chars",
1055
+			route: &api.Route{
1056
+				Spec: api.RouteSpec{
1057
+					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",
1058
+				},
1059
+			},
1060
+			expectedErrors: true,
1061
+		},
1062
+	}
1063
+
1064
+	for _, tc := range tests {
1065
+		errs := ValidateHostName(tc.route)
1066
+
1067
+		if tc.expectedErrors {
1068
+			if len(errs) < 1 {
1069
+				t.Errorf("Test case %s expected errors, got none.", tc.name)
1070
+			}
1071
+		} else {
1072
+			if len(errs) > 0 {
1073
+				t.Errorf("Test case %s expected no errors, got %d. %v", tc.name, len(errs), errs)
1074
+			}
1075
+		}
1076
+	}
1077
+}
... ...
@@ -2,6 +2,7 @@ package controller
2 2
 
3 3
 import (
4 4
 	"fmt"
5
+	"strings"
5 6
 
6 7
 	"github.com/golang/glog"
7 8
 	kapi "k8s.io/kubernetes/pkg/api"
... ...
@@ -9,6 +10,7 @@ import (
9 9
 	"k8s.io/kubernetes/pkg/watch"
10 10
 
11 11
 	routeapi "github.com/openshift/origin/pkg/route/api"
12
+	"github.com/openshift/origin/pkg/route/api/validation"
12 13
 	"github.com/openshift/origin/pkg/router"
13 14
 )
14 15
 
... ...
@@ -108,6 +110,20 @@ func (p *UniqueHost) HandleRoute(eventType watch.EventType, route *routeapi.Rout
108 108
 	}
109 109
 	route.Spec.Host = host
110 110
 
111
+	// Run time check to defend against older routes. Validate that the
112
+	// route host name conforms to DNS requirements.
113
+	if errs := validation.ValidateHostName(route); len(errs) > 0 {
114
+		glog.V(4).Infof("Route %s - invalid host name %s", routeName, host)
115
+		errMessages := make([]string, len(errs))
116
+		for i := 0; i < len(errs); i++ {
117
+			errMessages[i] = errs[i].Error()
118
+		}
119
+
120
+		err := fmt.Errorf("host name validation errors: %s", strings.Join(errMessages, ", "))
121
+		p.recorder.RecordRouteRejection(route, "InvalidHost", err.Error())
122
+		return err
123
+	}
124
+
111 125
 	// ensure hosts can only be claimed by one namespace at a time
112 126
 	// TODO: this could be abstracted above this layer?
113 127
 	if old, ok := p.hostToRoute[host]; ok {