fix: race condition when the TLS secret was created before setting th…

…e default policy (#1469)

Signed-off-by: Chetan Banavikalmutt <[email protected]>

controllers/argocd/route.go

@@ -236,10 +236,13 @@ func (r *ReconcileArgoCD) reconcileServerRoute(cr *argoproj.ArgoCD) error {
 			TargetPort: intstr.FromString("https"),
 		}
 
-		isTLSSecretFound := argoutil.IsObjectFound(r.Client, cr.Namespace, common.ArgoCDServerTLSSecretName, &corev1.Secret{})
+		tlsSecret := &corev1.Secret{}
+		isTLSSecretFound := argoutil.IsObjectFound(r.Client, cr.Namespace, common.ArgoCDServerTLSSecretName, tlsSecret)
 		// Since Passthrough was the default policy in the previous versions of the operator, we don't want to
 		// break users who have already configured a TLS secret for Passthrough.
-		if cr.Spec.Server.Route.TLS == nil && isTLSSecretFound && route.Spec.TLS != nil && route.Spec.TLS.Termination == routev1.TLSTerminationPassthrough {
+		// We continue with Passthrough if we find a TLS secret that was manually configured
+		// by the user and not by the OpenShift Service CA.
+		if cr.Spec.Server.Route.TLS == nil && isTLSSecretFound && !isCreatedByServiceCA(cr.Name, *tlsSecret) {
 			route.Spec.TLS = &routev1.TLSConfig{
 				InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyRedirect,
 				Termination:                   routev1.TLSTerminationPassthrough,
@@ -257,6 +260,8 @@ func (r *ReconcileArgoCD) reconcileServerRoute(cr *argoproj.ArgoCD) error {
 		route.Spec.TLS = cr.Spec.Server.Route.TLS
 	}
 
+	log.Info(fmt.Sprintf("Using %s termination policy for the Server Route", string(route.Spec.TLS.Termination)))
+
 	route.Spec.To.Kind = "Service"
 	route.Spec.To.Name = nameWithSuffix("server", cr)
 
@@ -274,6 +279,30 @@ func (r *ReconcileArgoCD) reconcileServerRoute(cr *argoproj.ArgoCD) error {
 	return r.Client.Update(context.TODO(), route)
 }
 
+// isCreatedByServiceCA checks if the secret was created by the OpenShift Service CA
+func isCreatedByServiceCA(crName string, secret corev1.Secret) bool {
+	serviceName := fmt.Sprintf("%s-%s", crName, "server")
+	serviceAnnFound := false
+	if secret.Annotations != nil {
+		value, ok := secret.Annotations["service.beta.openshift.io/originating-service-name"]
+		if ok && value == serviceName {
+			serviceAnnFound = true
+		}
+	}
+
+	if !serviceAnnFound {
+		return false
+	}
+
+	for _, ref := range secret.OwnerReferences {
+		if ref.Kind == "Service" && ref.Name == serviceName {
+			return true
+		}
+	}
+
+	return false
+}
+
 // reconcileApplicationSetControllerWebhookRoute will ensure that the ArgoCD Server Route is present.
 func (r *ReconcileArgoCD) reconcileApplicationSetControllerWebhookRoute(cr *argoproj.ArgoCD) error {
 	name := fmt.Sprintf("%s-%s", common.ApplicationSetServiceNameSuffix, "webhook")

controllers/argocd/route_test.go

@@ -548,6 +548,33 @@ func TestReconcileRouteTLSConfig(t *testing.T) {
 				assert.NoError(t, err)
 			},
 		},
+		{
+			name: "should overwrite if the TLS secret is created by the OpenShift Service CA",
+			want: routev1.TLSTerminationReencrypt,
+			updateArgoCD: func(cr *argoproj.ArgoCD) {
+				cr.Spec.Server.Route.Enabled = true
+			},
+			createResources: func(k8sClient client.Client, cr *argoproj.ArgoCD) {
+				serviceName := fmt.Sprintf("%s-%s", cr.Name, "server")
+				secret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      common.ArgoCDServerTLSSecretName,
+						Namespace: cr.Namespace,
+						Annotations: map[string]string{
+							"service.beta.openshift.io/originating-service-name": serviceName,
+						},
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								Name: serviceName,
+								Kind: "Service",
+							},
+						},
+					},
+				}
+				err := k8sClient.Create(context.Background(), secret)
+				assert.NoError(t, err)
+			},
+		},
 	}
 
 	for _, test := range tt {
@@ -578,6 +605,68 @@ func TestReconcileRouteTLSConfig(t *testing.T) {
 	}
 }
 
+func TestIsCreatedByServiceCA(t *testing.T) {
+	cr := makeArgoCD()
+	serviceName := fmt.Sprintf("%s-%s", cr.Name, "server")
+	secret := corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      common.ArgoCDServerTLSSecretName,
+			Namespace: cr.Namespace,
+			Annotations: map[string]string{
+				"service.beta.openshift.io/originating-service-name": serviceName,
+			},
+			OwnerReferences: []metav1.OwnerReference{
+				{
+					Name: serviceName,
+					Kind: "Service",
+				},
+			},
+		},
+	}
+
+	tests := []struct {
+		name         string
+		want         bool
+		updateSecret func(s *corev1.Secret)
+	}{
+		{
+			"secret is created by OpenShift Service CA",
+			true,
+			func(s *corev1.Secret) {},
+		},
+		{
+			"secret is not created by OpenShift Service CA",
+			false,
+			func(s *corev1.Secret) {
+				s.Annotations = nil
+				s.OwnerReferences = nil
+			},
+		},
+		{
+			"secret doesn't have the OpenShift Service CA annotation",
+			false,
+			func(s *corev1.Secret) {
+				s.Annotations = nil
+			},
+		},
+		{
+			"secret is not owned by the correct CR",
+			false,
+			func(s *corev1.Secret) {
+				s.OwnerReferences = nil
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			testSecret := secret.DeepCopy()
+			test.updateSecret(testSecret)
+			assert.Equal(t, test.want, isCreatedByServiceCA(cr.Name, *testSecret))
+		})
+	}
+}
+
 func makeReconciler(t *testing.T, acd *argoproj.ArgoCD, objs ...runtime.Object) *ReconcileArgoCD {
 	t.Helper()
 	s := scheme.Scheme