Fix webhook route fail in argocd due to long route name (#1129)

* Fix webhook route failure when hostname is too long

---------

Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>

controllers/argocd/ingress.go

@@ -363,10 +363,15 @@ func (r *ReconcileArgoCD) reconcileApplicationSetControllerIngress(cr *argoproj.
 	ingress.ObjectMeta.Annotations = atns
 
 	pathType := networkingv1.PathTypeImplementationSpecific
+	httpServerHost, err := getApplicationSetHTTPServerHost(cr)
+	if err != nil {
+		return err
+	}
+
 	// Add rules
 	ingress.Spec.Rules = []networkingv1.IngressRule{
 		{
-			Host: getApplicationSetHTTPServerHost(cr),
+			Host: httpServerHost,
 			IngressRuleValue: networkingv1.IngressRuleValue{
 				HTTP: &networkingv1.HTTPIngressRuleValue{
 					Paths: []networkingv1.HTTPIngressPath{

controllers/argocd/route.go

@@ -17,6 +17,7 @@ package argocd
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	routev1 "github.com/openshift/api/route/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -28,6 +29,12 @@ import (
 	"github.com/argoproj-labs/argocd-operator/controllers/argoutil"
 )
 
+const (
+	maxLabelLength    = 63
+	maxHostnameLength = 253
+	minFirstLabelSize = 20
+)
+
 var routeAPIFound = false
 
 // IsRouteAPIAvailable returns true if the Route API is present.
@@ -207,6 +214,13 @@ func (r *ReconcileArgoCD) reconcileServerRoute(cr *argoproj.ArgoCD) error {
 		route.Spec.Host = cr.Spec.Server.Host // TODO: What additional role needed for this?
 	}
 
+	hostname, err := shortenHostname(route.Spec.Host)
+	if err != nil {
+		return err
+	}
+
+	route.Spec.Host = hostname
+
 	if cr.Spec.Server.Insecure {
 		// Disable TLS and rely on the cluster certificate.
 		route.Spec.Port = &routev1.RoutePort{
@@ -284,6 +298,13 @@ func (r *ReconcileArgoCD) reconcileApplicationSetControllerWebhookRoute(cr *argo
 		route.Spec.Host = cr.Spec.ApplicationSet.WebhookServer.Host
 	}
 
+	hostname, err := shortenHostname(route.Spec.Host)
+	if err != nil {
+		return err
+	}
+
+	route.Spec.Host = hostname
+
 	route.Spec.Port = &routev1.RoutePort{
 		TargetPort: intstr.FromString("webhook"),
 	}
@@ -351,3 +372,53 @@ func (r *ReconcileArgoCD) reconcileApplicationSetControllerWebhookRoute(cr *argo
 	}
 	return r.Client.Update(context.TODO(), route)
 }
+
+// The algorithm used by this function is:
+// - If the FIRST label ("console-openshift-console" in the above case) is longer than 63 characters, shorten (truncate the end) it to 63.
+// - If any other label is longer than 63 characters, return an error
+// - After all the labels are 63 characters or less, check the length of the overall hostname:
+//   - If the overall hostname is > 253, then shorten the FIRST label until the host name is < 253
+//   - After the FIRST label has been shortened, if it is < 20, then return an error (this is a sanity test to ensure the label is likely to be unique)
+func shortenHostname(hostname string) (string, error) {
+	if hostname == "" {
+		return "", nil
+	}
+
+	// Return the hostname as it is if hostname is already within the size limit
+	if len(hostname) <= maxHostnameLength {
+		return hostname, nil
+	}
+
+	// Split the hostname into labels
+	labels := strings.Split(hostname, ".")
+
+	// Check and truncate the FIRST label if longer than 63 characters
+	if len(labels[0]) > maxLabelLength {
+		labels[0] = labels[0][:maxLabelLength]
+	}
+
+	// Check other labels and return an error if any is longer than 63 characters
+	for _, label := range labels[1:] {
+		if len(label) > maxLabelLength {
+			return "", fmt.Errorf("label length exceeds 63 characters")
+		}
+	}
+
+	// Join the labels back into a hostname
+	resultHostname := strings.Join(labels, ".")
+
+	// Check and shorten the overall hostname
+	if len(resultHostname) > maxHostnameLength {
+		// Shorten the first label until the length is less than 253
+		for len(resultHostname) > maxHostnameLength && len(labels[0]) > 20 {
+			labels[0] = labels[0][:len(labels[0])-1]
+			resultHostname = strings.Join(labels, ".")
+		}
+
+		// Check if the first label is still less than 20 characters
+		if len(labels[0]) < minFirstLabelSize {
+			return "", fmt.Errorf("shortened first label is less than 20 characters")
+		}
+	}
+	return resultHostname, nil
+}

controllers/argocd/route_test.go

@@ -3,6 +3,7 @@ package argocd
 import (
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -405,6 +406,90 @@ func TestReconcileRouteApplicationSetTls(t *testing.T) {
 	}
 }
 
+func TestReconcileRouteForShorteningHostname(t *testing.T) {
+	routeAPIFound = true
+	ctx := context.Background()
+	logf.SetLogger(ZapLogger(true))
+
+	tests := []struct {
+		testName string
+		expected string
+		hostname string
+	}{
+		{
+			testName: "longHostname",
+			hostname: "myhostnameaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.redhat.com",
+			expected: "myhostnameaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.redhat.com",
+		},
+		{
+			testName: "twentySixLetterHostname",
+			hostname: "myhostnametwentysixletteraaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.redhat.com",
+			expected: "myhostnametwentysixletteraaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.redhat.com",
+		},
+	}
+
+	for _, v := range tests {
+		t.Run(v.testName, func(t *testing.T) {
+
+			argoCD := makeArgoCD(func(a *argoproj.ArgoCD) {
+				a.Spec.Server.Route.Enabled = true
+				a.Spec.ApplicationSet = &argoproj.ArgoCDApplicationSet{
+					WebhookServer: argoproj.WebhookServerSpec{
+						Route: argoproj.ArgoCDRouteSpec{
+							Enabled: true,
+						},
+						Host: v.hostname,
+					},
+				}
+			})
+
+			resObjs := []client.Object{argoCD}
+			subresObjs := []client.Object{argoCD}
+			runtimeObjs := []runtime.Object{}
+			sch := makeTestReconcilerScheme(argoproj.AddToScheme, configv1.Install, routev1.Install)
+			cl := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+			r := makeTestReconciler(cl, sch)
+
+			assert.NoError(t, createNamespace(r, argoCD.Namespace, ""))
+
+			req := reconcile.Request{
+				NamespacedName: types.NamespacedName{
+					Name:      testArgoCDName,
+					Namespace: testNamespace,
+				},
+			}
+
+			// Check if it returns nil when hostname is empty
+			_, err := r.Reconcile(context.TODO(), req)
+			assert.NoError(t, err)
+
+			// second reconciliation after changing the hostname.
+			err = r.Client.Get(ctx, req.NamespacedName, argoCD)
+			fatalIfError(t, err, "failed to load ArgoCD %q: %s", testArgoCDName+"-server", err)
+
+			argoCD.Spec.Server.Host = v.hostname
+			err = r.Client.Update(ctx, argoCD)
+			fatalIfError(t, err, "failed to update the ArgoCD: %s", err)
+
+			_, err = r.Reconcile(context.TODO(), req)
+			assert.NoError(t, err)
+
+			loaded := &routev1.Route{}
+			err = r.Client.Get(ctx, types.NamespacedName{Name: testArgoCDName + "-server", Namespace: testNamespace}, loaded)
+			fatalIfError(t, err, "failed to load route %q: %s", testArgoCDName+"-server", err)
+
+			if diff := cmp.Diff(v.expected, loaded.Spec.Host); diff != "" {
+				t.Fatalf("failed to reconcile route:\n%s", diff)
+			}
+
+			// Check if first label is greater than 20
+			labels := strings.Split(loaded.Spec.Host, ".")
+			assert.True(t, len(labels[0]) > 20)
+
+		})
+	}
+}
+
 func makeReconciler(t *testing.T, acd *argoproj.ArgoCD, objs ...runtime.Object) *ReconcileArgoCD {
 	t.Helper()
 	s := scheme.Scheme

controllers/argocd/util.go

@@ -1574,10 +1574,14 @@ func contains(s []string, g string) bool {
 }
 
 // getApplicationSetHTTPServerHost will return the host for the given ArgoCD.
-func getApplicationSetHTTPServerHost(cr *argoproj.ArgoCD) string {
+func getApplicationSetHTTPServerHost(cr *argoproj.ArgoCD) (string, error) {
 	host := cr.Name
 	if len(cr.Spec.ApplicationSet.WebhookServer.Host) > 0 {
-		host = cr.Spec.ApplicationSet.WebhookServer.Host
+		hostname, err := shortenHostname(cr.Spec.ApplicationSet.WebhookServer.Host)
+		if err != nil {
+			return "", err
+		}
+		host = hostname
 	}
-	return host
+	return host, nil
 }