feat: Update the default TLS termination policy to reencrypt (#1363)

* Update the default TLS termination policy to reencrypt Signed-off-by: Chetan Banavikalmutt <[email protected]> * Revert the edge termination policy back to Redirect Signed-off-by: Chetan Banavikalmutt <[email protected]> * Add a test to verify the TLS config Signed-off-by: Chetan Banavikalmutt <[email protected]> * Remove redundant nil check while verifying AutoTLS Signed-off-by: Chetan Banavikalmutt <[email protected]> --------- Signed-off-by: Chetan Banavikalmutt <[email protected]>
argoproj-labs · May 22, 2024 · 1ff45e5 · 1ff45e5
1 parent 83f97da
commit 1ff45e5
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 12 deletions.
diff --git a/api/v1alpha1/argocd_types.go b/api/v1alpha1/argocd_types.go
@@ -960,10 +960,11 @@ func (argocd *ArgoCD) IsDeletionFinalizerPresent() bool {
 	return false
 }
 
-// WantsAutoTLS returns true if user configured a route with reencryption
-// termination policy.
+// WantsAutoTLS returns true if:
+// 1. user has configured a route with reencrypt.
+// 2. user has not configured TLS and we default to reencrypt.
 func (s *ArgoCDServerSpec) WantsAutoTLS() bool {
-	return s.Route.TLS != nil && s.Route.TLS.Termination == routev1.TLSTerminationReencrypt
+	return s.Route.TLS == nil || s.Route.TLS.Termination == routev1.TLSTerminationReencrypt
 }
 
 // WantsAutoTLS returns true if the repository server configuration has set

diff --git a/api/v1beta1/argocd_types.go b/api/v1beta1/argocd_types.go
@@ -991,10 +991,11 @@ func (argocd *ArgoCD) IsDeletionFinalizerPresent() bool {
 	return false
 }
 
-// WantsAutoTLS returns true if user configured a route with reencryption
-// termination policy.
+// WantsAutoTLS returns true if:
+// 1. user has configured a route with reencrypt.
+// 2. user has not configured TLS and we default to reencrypt.
 func (s *ArgoCDServerSpec) WantsAutoTLS() bool {
-	return s.Route.TLS != nil && s.Route.TLS.Termination == routev1.TLSTerminationReencrypt
+	return s.Route.TLS == nil || s.Route.TLS.Termination == routev1.TLSTerminationReencrypt
 }
 
 // WantsAutoTLS returns true if the repository server configuration has set

diff --git a/controllers/argocd/route.go b/controllers/argocd/route.go
@@ -231,13 +231,13 @@ func (r *ReconcileArgoCD) reconcileServerRoute(cr *argoproj.ArgoCD) error {
 			Termination:                   routev1.TLSTerminationEdge,
 		}
 	} else {
-		// Server is using TLS configure passthrough.
+		// Server is using TLS configure reencrypt.
 		route.Spec.Port = &routev1.RoutePort{
 			TargetPort: intstr.FromString("https"),
 		}
 		route.Spec.TLS = &routev1.TLSConfig{
 			InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
-			Termination:                   routev1.TLSTerminationPassthrough,
+			Termination:                   routev1.TLSTerminationReencrypt,
 		}
 	}
 

diff --git a/controllers/argocd/route_test.go b/controllers/argocd/route_test.go
@@ -97,7 +97,7 @@ func TestReconcileRouteSetsInsecure(t *testing.T) {
 	fatalIfError(t, err, "failed to load route %q: %s", testArgoCDName+"-server", err)
 
 	wantTLSConfig := &routev1.TLSConfig{
-		Termination:                   routev1.TLSTerminationPassthrough,
+		Termination:                   routev1.TLSTerminationReencrypt,
 		InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
 	}
 	if diff := cmp.Diff(wantTLSConfig, loaded.Spec.TLS); diff != "" {
@@ -202,7 +202,7 @@ func TestReconcileRouteUnsetsInsecure(t *testing.T) {
 	fatalIfError(t, err, "failed to load route %q: %s", testArgoCDName+"-server", err)
 
 	wantTLSConfig = &routev1.TLSConfig{
-		Termination:                   routev1.TLSTerminationPassthrough,
+		Termination:                   routev1.TLSTerminationReencrypt,
 		InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
 	}
 	if diff := cmp.Diff(wantTLSConfig, loaded.Spec.TLS); diff != "" {
@@ -280,7 +280,8 @@ func TestReconcileRouteApplicationSetTlsTermination(t *testing.T) {
 				Route: argoproj.ArgoCDRouteSpec{
 					Enabled: true,
 					TLS: &routev1.TLSConfig{
-						Termination: "passthrough",
+						Termination:                   routev1.TLSTerminationPassthrough,
+						InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
 					},
 				},
 			},
@@ -490,6 +491,62 @@ func TestReconcileRouteForShorteningHostname(t *testing.T) {
 	}
 }
 
+func TestReconcileRouteTLSConfig(t *testing.T) {
+	routeAPIFound = true
+	ctx := context.Background()
+	logf.SetLogger(ZapLogger(true))
+
+	tt := []struct {
+		name         string
+		want         routev1.TLSTerminationType
+		updateArgoCD func(cr *argoproj.ArgoCD)
+	}{
+		{
+			name: "should set the default termination policy to renencrypt",
+			want: routev1.TLSTerminationReencrypt,
+			updateArgoCD: func(cr *argoproj.ArgoCD) {
+				cr.Spec.Server.Route.Enabled = true
+			},
+		},
+		{
+			name: "shouldn't overwrite the TLS config if it's already configured",
+			want: routev1.TLSTerminationEdge,
+			updateArgoCD: func(cr *argoproj.ArgoCD) {
+				cr.Spec.Server.Route.Enabled = true
+				cr.Spec.Server.Route.TLS = &routev1.TLSConfig{
+					Termination: routev1.TLSTerminationEdge,
+				}
+			},
+		},
+	}
+
+	for _, test := range tt {
+		t.Run(test.name, func(t *testing.T) {
+			argoCD := makeArgoCD(test.updateArgoCD)
+
+			resObjs := []client.Object{argoCD}
+			subresObjs := []client.Object{argoCD}
+			runtimeObjs := []runtime.Object{}
+			sch := makeTestReconcilerScheme(argoproj.AddToScheme, configv1.Install, routev1.Install)
+			fakeClient := makeTestReconcilerClient(sch, resObjs, subresObjs, runtimeObjs)
+			reconciler := makeTestReconciler(fakeClient, sch)
+
+			req := reconcile.Request{
+				NamespacedName: testNamespacedName(testArgoCDName),
+			}
+
+			_, err := reconciler.Reconcile(ctx, req)
+			assert.Nil(t, err)
+
+			route := &routev1.Route{}
+			err = reconciler.Client.Get(ctx, types.NamespacedName{Name: argoCD.Name + "-server", Namespace: argoCD.Namespace}, route)
+			assert.Nil(t, err)
+			assert.Equal(t, test.want, route.Spec.TLS.Termination)
+
+		})
+	}
+}
+
 func makeReconciler(t *testing.T, acd *argoproj.ArgoCD, objs ...runtime.Object) *ReconcileArgoCD {
 	t.Helper()
 	s := scheme.Scheme

diff --git a/docs/usage/routes.md b/docs/usage/routes.md
@@ -48,7 +48,7 @@ $ kubectl get secret argocd-cluster -n argocd -ojsonpath='{.data.admin\.password
 
 ## Setting TLS modes for routes
 
-You can parameterize the route's TLS configuration by setting appropriate values in the `.spec.server.route.tls` field of the `ArgoCD` CR.
+By default, the operator creates the Argo CD server route with `reencrypt` termination policy. You can parameterize the route's TLS configuration by setting appropriate values in the `.spec.server.route.tls` field of the `ArgoCD` CR.
 
 ### TLS edge termination mode