OTA-423: Validate route host before creating app

Author: jottofar

api/v1/updateservice_types.go

@@ -50,6 +50,8 @@ const (
 
 	// ConditionRegistryCACertFound reports whether the updateservice registry CA cert had been found
 	ConditionRegistryCACertFound conditionsv1.ConditionType = "RegistryCACertFound"
+
+	ConditionReconcileError conditionsv1.ConditionType = "ReconcileError"
 )
 
 // +kubebuilder:object:root=true

controllers/names.go

@@ -47,7 +47,7 @@ func nameGraphBuilderService(instance *cv1.UpdateService) string {
 }
 
 func namePolicyEngineRoute(instance *cv1.UpdateService) string {
-	return namePolicyEngineService(instance) + "-route"
+	return instance.Name + "-route"
 }
 
 func nameAdditionalTrustedCA(instance *cv1.UpdateService) string {

controllers/updateservice_controller.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"regexp"
 
 	"github.com/go-logr/logr"
 	conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1"
@@ -28,6 +29,13 @@ import (
 	"github.com/openshift/library-go/pkg/route/routeapihelpers"
 )
 
+const dns1123LabelFmt string = "^[a-z]([-a-z0-9]*[a-z0-9])?$"
+
+// DNS1123LabelMaxLength is a label's max length in DNS (RFC 1123)
+const DNS1123LabelMaxLength int = 63
+
+var dns1123LabelRegexp = regexp.MustCompile(dns1123LabelFmt)
+
 var log = logf.Log.WithName("controller_updateservice")
 
 // blank assignment to verify that ReconcileUpdateService implements reconcile.Reconciler
@@ -78,6 +86,20 @@ func (r *UpdateServiceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	instanceCopy := instance.DeepCopy()
 	instanceCopy.Status = cv1.UpdateServiceStatus{}
 
+	if err := validateRouteName(instanceCopy, req.Name, req.Namespace); err != nil {
+		conditionsv1.SetStatusCondition(&instanceCopy.Status.Conditions, conditionsv1.Condition{
+			Type:    cv1.ConditionReconcileError,
+			Status:  corev1.ConditionTrue,
+			Reason:  "Unable to create UpdateService route",
+			Message: err.Error(),
+		})
+		if err := r.Client.Status().Update(ctx, instanceCopy); err != nil {
+			reqLogger.Error(err, "Failed to update Status")
+		}
+		reqLogger.Error(err, "Unable to create UpdateService route")
+		return ctrl.Result{}, nil
+	}
+
 	// 1. Gather conditions
 	//    Look at the existing cluster resources and communicate to kubeResources
 	//    how it should create resources.
@@ -464,6 +486,28 @@ func (r *UpdateServiceReconciler) ensurePolicyEngineService(ctx context.Context,
 	return nil
 }
 
+func validateRouteName(instance *cv1.UpdateService, name string, namespace string) error {
+	var errReasons []string
+	routeName := namePolicyEngineRoute(instance) + "-" + namespace
+
+	if len(routeName) > DNS1123LabelMaxLength {
+		errReasons = append(errReasons,
+			fmt.Sprintf("cannot exceed RFC 1123 maximum length of %d. Shorten the application name and/or namespace.",
+				DNS1123LabelMaxLength))
+	}
+	if !dns1123LabelRegexp.MatchString(routeName) {
+		errReasons = append(errReasons,
+			fmt.Sprintf("has invalid format; must comply with %q.", dns1123LabelFmt))
+	}
+	numErrors := len(errReasons)
+	if numErrors == 0 {
+		return nil
+	} else if numErrors == 1 {
+		return fmt.Errorf(fmt.Sprintf("UpdateService route name %q %s", routeName, errReasons[0]))
+	}
+	return fmt.Errorf(fmt.Sprintf("UpdateService route name %q %s Route name %s", routeName, errReasons[0], errReasons[1]))
+}
+
 func (r *UpdateServiceReconciler) ensurePolicyEngineRoute(ctx context.Context, reqLogger logr.Logger, instance *cv1.UpdateService, resources *kubeResources) error {
 	route := resources.policyEngineRoute
 	// Set UpdateService instance as the owner and controller

controllers/updateservice_controller_test.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"testing"
@@ -16,7 +17,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	policyv1beta1 "k8s.io/api/policy/v1beta1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -190,7 +191,7 @@ func TestEnsurePullSecret(t *testing.T) {
 
 			resources, err := newKubeResources(updateservice, testOperandImage, ps, cm)
 
-			if !errors.IsNotFound(err) {
+			if !apierrors.IsNotFound(err) {
 				err = r.ensurePullSecret(context.TODO(), log, updateservice, resources)
 			}
 			found := &corev1.Secret{}
@@ -480,7 +481,7 @@ func TestEnsurePodDisruptionBudget(t *testing.T) {
 			updateservice := &cv1.UpdateService{}
 			err := r.Client.Get(context.TODO(), types.NamespacedName{Name: testName, Namespace: testNamespace}, updateservice)
 			if err != nil {
-				if errors.IsNotFound(err) {
+				if apierrors.IsNotFound(err) {
 					assert.Error(t, err)
 				}
 				assert.Error(t, err)
@@ -517,6 +518,55 @@ func TestEnsurePodDisruptionBudget(t *testing.T) {
 	}
 }
 
+func TestValidateRouteName(t *testing.T) {
+	tests := []struct {
+		name      string
+		appName   string
+		namespace string
+		err       error
+	}{
+		{
+			name:      "RouteNameValid",
+			appName:   "foo",
+			namespace: "openshift-update-service",
+			err:       nil,
+		},
+		{
+			name:      "RouteNameMaxLen",
+			appName:   "foo",
+			namespace: "openshift-update-service-0123456789012345678901234567",
+			err:       nil,
+		},
+		{
+			name:      "RouteNameTooLong",
+			appName:   "foo",
+			namespace: "openshift-update-service-01234567890123456789012345678",
+			err:       errors.New("UpdateService route name \"foo-route-openshift-update-service-01234567890123456789012345678\" cannot exceed RFC 1123 maximum length of 63. Shorten the application name and/or namespace."),
+		},
+		{
+			name:      "RouteNameInvalidFormat",
+			appName:   "foo",
+			namespace: "openshift-update-service-012345678901234567890123456.",
+			err:       errors.New(fmt.Sprintf("UpdateService route name \"foo-route-openshift-update-service-012345678901234567890123456.\" has invalid format; must comply with %q.", dns1123LabelFmt)),
+		},
+		{
+			name:      "RouteNameMultipleErrors",
+			appName:   "foo",
+			namespace: "openshift-update-service-0123456789012345678901234567.",
+			err: errors.New("UpdateService route name \"foo-route-openshift-update-service-0123456789012345678901234567.\" cannot exceed RFC 1123 maximum length of 63. Shorten the application name and/or namespace. " +
+				fmt.Sprintf("Route name has invalid format; must comply with %q.", dns1123LabelFmt)),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			updateservice := newDefaultUpdateService()
+			err := validateRouteName(updateservice, test.appName, test.namespace)
+			assert.Equal(t, test.err, err)
+		})
+	}
+}
+
 func TestEnsurePolicyEngineRoute(t *testing.T) {
 	tests := []struct {
 		name         string