diff --git a/controllers/argocd/route.go b/controllers/argocd/route.go index 90bf38661..71de0e35a 100644 --- a/controllers/argocd/route.go +++ b/controllers/argocd/route.go @@ -29,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. @@ -340,10 +346,19 @@ func (r *ReconcileArgoCD) reconcileApplicationSetControllerWebhookRoute(cr *argo route.Spec.Host = hostname - if cr.Spec.Server.Insecure { - // Disable TLS and rely on the cluster certificate. - route.Spec.Port = &routev1.RoutePort{ - TargetPort: intstr.FromString("webhook"), + route.Spec.Port = &routev1.RoutePort{ + TargetPort: intstr.FromString("webhook"), + } + + // Allow override of TLS options for the Route + if cr.Spec.ApplicationSet.WebhookServer.Route.TLS != nil { + tls := &routev1.TLSConfig{} + + // Set Termination + if cr.Spec.ApplicationSet.WebhookServer.Route.TLS.Termination != "" { + tls.Termination = cr.Spec.ApplicationSet.WebhookServer.Route.TLS.Termination + } else { + tls.Termination = routev1.TLSTerminationEdge } // Set Certificate @@ -399,18 +414,28 @@ 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 > 255, then shorten the FIRST label until the host name is < 255 +// - 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 + } + // Split the hostname into labels labels := strings.Split(hostname, ".") // Check and truncate the FIRST label if longer than 63 characters - if len(labels[0]) > 63 { - labels[0] = labels[0][:63] + 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) > 63 { + if len(label) > maxLabelLength { return "", fmt.Errorf("label length exceeds 63 characters") } } @@ -418,15 +443,18 @@ func shortenHostname(hostname string) (string, error) { // Join the labels back into a hostname resultHostname := strings.Join(labels, ".") - // Check and shorten the overall hostname if > 253 characters - if len(resultHostname) > 255 { - resultHostname = resultHostname[:253] - } + // 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 < 20 and return an error if true - if len(labels[0]) < 20 { - return "", fmt.Errorf("first label length is less than 20 characters") + // 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 } diff --git a/controllers/argocd/route_test.go b/controllers/argocd/route_test.go index fbd8f24e6..af83bc166 100644 --- a/controllers/argocd/route_test.go +++ b/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