diff --git a/api/v1/updateservice_types.go b/api/v1/updateservice_types.go index 8b37d4305..df60cb2ac 100644 --- a/api/v1/updateservice_types.go +++ b/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 diff --git a/controllers/names.go b/controllers/names.go index d29837fdf..06ebb2bc6 100644 --- a/controllers/names.go +++ b/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 { diff --git a/controllers/updateservice_controller.go b/controllers/updateservice_controller.go index 1f5e2dcc9..19755f816 100644 --- a/controllers/updateservice_controller.go +++ b/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 diff --git a/controllers/updateservice_controller_test.go b/controllers/updateservice_controller_test.go index f025f3f12..b2ec74714 100644 --- a/controllers/updateservice_controller_test.go +++ b/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