Bug 1990826: routes without TLS are rejected for missing HSTS annotation

pkg/route/apiserver/admission/requiredrouteannotations/admission.go

@@ -108,6 +108,15 @@ func (o *requiredRouteAnnotations) Validate(ctx context.Context, a admission.Att
 		}
 	}
 
+	newRoute := a.GetObject().(*routeapi.Route)
+
+	// Cannot apply HSTS if route is not TLS.  Ignore silently to keep backward compatibility.
+	tls := newRoute.Spec.TLS
+	if tls == nil || (tls.Termination != routeapi.TLSTerminationEdge && tls.Termination != routeapi.TLSTerminationReencrypt) {
+		// TODO - will address missing annotations on routes as route status in https://issues.redhat.com/browse/NE-678
+		return nil
+	}
+
 	// Wait just once up to 20 seconds for all caches to sync
 	if !o.waitForSyncedStore(ctx) {
 		return admission.NewForbidden(a, errors.New(pluginName+": caches not synchronized"))
@@ -118,7 +127,6 @@ func (o *requiredRouteAnnotations) Validate(ctx context.Context, a admission.Att
 		return admission.NewForbidden(a, err)
 	}
 
-	newRoute := a.GetObject().(*routeapi.Route)
 	namespace, err := o.nsLister.Get(newRoute.Namespace)
 	if err != nil {
 		return admission.NewForbidden(a, err)
@@ -183,18 +191,6 @@ func (o *requiredRouteAnnotations) SetOpenShiftConfigInformers(informers configi
 
 // isRouteHSTSAllowed returns nil if the route is allowed.  Otherwise, returns details and a suggestion in the error
 func isRouteHSTSAllowed(ingress *configv1.Ingress, newRoute *routeapi.Route, namespace *corev1.Namespace) error {
-	// Invalid if a HSTS Policy is specified but this route is not TLS.  Just log a warning.
-	if tls := newRoute.Spec.TLS; tls != nil {
-		switch termination := tls.Termination; termination {
-		case routeapi.TLSTerminationEdge, routeapi.TLSTerminationReencrypt:
-		// Valid case
-		default:
-			// Non-tls routes will not get HSTS headers, but can still be valid
-			klog.Warningf("HSTS Policy not added for %s, wrong termination type: %s", newRoute.Name, termination)
-			return nil
-		}
-	}
-
 	requirements := ingress.Spec.RequiredHSTSPolicies
 	for _, requirement := range requirements {
 		// Check if the required namespaceSelector (if any) and the domainPattern match

pkg/route/apiserver/admission/requiredrouteannotations/admission_test.go

@@ -216,6 +216,27 @@ func TestValidate(t *testing.T) {
 			expectForbiddenClause: "max-age must be set in HSTS annotation",
 			expectForbidden:       true, // Not okay because the old route had HSTS
 		},
+		{
+			description: "unannotated route, non-TLS (nil), matching domain",
+			config:      nonemptyConfig,
+			namespace:   "matchingDomainNamespace",
+			spec: &routeapi.RouteSpec{
+				Host: "abc.foo.com",
+			},
+			name:            "config1.5",
+			expectForbidden: false,
+		},
+		{
+			description: "unannotated route, non-TLS (passthrough), matching domain",
+			config:      nonemptyConfig,
+			namespace:   "matchingDomainNamespace",
+			spec: &routeapi.RouteSpec{
+				Host: "abc.foo.com",
+				TLS:  &routeapi.TLSConfig{Termination: routeapi.TLSTerminationPassthrough},
+			},
+			name:            "config1.6",
+			expectForbidden: false,
+		},
 		{
 			description: "appropriately annotated route for required annotations in ingress config",
 			config:      nonemptyConfig,