From 822f760eb4b864672320ca5d1aa000d266d2d2e5 Mon Sep 17 00:00:00 2001 From: jannfis Date: Thu, 9 Sep 2021 05:49:34 +0200 Subject: [PATCH] feat: Enable service CA for reencrypt routes to argocd-server on OpenShift (#326) * feat: Enable service CA for reencrypt routes to argocd-server on OpenShift Signed-off-by: jannfis --- api/v1alpha1/argocd_types.go | 12 +++++ common/annotations.go | 4 ++ common/values.go | 3 ++ controllers/argocd/service.go | 49 ++++++++++++++----- controllers/argocd/service_test.go | 44 +++++++++++++++++ docs/usage/routes.md | 76 ++++++++++++++++++++++++++++++ 6 files changed, 176 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/argocd_types.go b/api/v1alpha1/argocd_types.go index 04577ac9f..5becd986f 100644 --- a/api/v1alpha1/argocd_types.go +++ b/api/v1alpha1/argocd_types.go @@ -699,6 +699,18 @@ func (argocd *ArgoCD) IsDeletionFinalizerPresent() bool { return false } +// WantsAutoTLS returns true if user configured a route with reencryption +// termination policy. +func (s *ArgoCDServerSpec) WantsAutoTLS() bool { + return s.Route.TLS != nil && s.Route.TLS.Termination == routev1.TLSTerminationReencrypt +} + +// WantsAutoTLS returns true if the repository server configuration has set +// the autoTLS toggle to a supported provider. +func (r *ArgoCDRepoSpec) WantsAutoTLS() bool { + return r.AutoTLS == "openshift" +} + // ApplicationInstanceLabelKey returns either the custom application instance // label key if set, or the default value. func (a *ArgoCD) ApplicationInstanceLabelKey() string { diff --git a/common/annotations.go b/common/annotations.go index bbed514bf..831983e20 100644 --- a/common/annotations.go +++ b/common/annotations.go @@ -8,4 +8,8 @@ const ( // AnnotationNamespace is the annotation on child resources that specifies which ArgoCD instance // namespace a specific object is associated with AnnotationNamespace = "argocds.argoproj.io/namespace" + + // AnnotationOpenShiftServiceCA is the annotation on services used to + // request a TLS certificate from OpenShift's Service CA for AutoTLS + AnnotationOpenShiftServiceCA = "service.beta.openshift.io/serving-cert-secret-name" ) diff --git a/common/values.go b/common/values.go index 50d9e6091..d4adda402 100644 --- a/common/values.go +++ b/common/values.go @@ -76,4 +76,7 @@ const ( // ArgoCDRepoServerTLSSecretName is the name of the TLS secret for the repo-server ArgoCDRepoServerTLSSecretName = "argocd-repo-server-tls" + + // ArgoCDServerTLSSecretName is the name of the TLS secret for the argocd-server + ArgoCDServerTLSSecretName = "argocd-server-tls" ) diff --git a/controllers/argocd/service.go b/controllers/argocd/service.go index 9e90c4234..9eb0715d1 100644 --- a/controllers/argocd/service.go +++ b/controllers/argocd/service.go @@ -308,20 +308,40 @@ func (r *ReconcileArgoCD) reconcileRedisService(cr *argoprojv1a1.ArgoCD) error { return r.Client.Create(context.TODO(), svc) } -func ensureAutoTLSAnnotation(cr *argoprojv1a1.ArgoCD, svc *corev1.Service) bool { - autoTLSAnnotationName := "" - if cr.Spec.Repo.AutoTLS == "openshift" { - autoTLSAnnotationName = "service.beta.openshift.io/serving-cert-secret-name" - } - if autoTLSAnnotationName != "" { +// ensureAutoTLSAnnotation ensures that the service svc has the desired state +// of the auto TLS annotation set, which is either set (when enabled is true) +// or unset (when enabled is false). +// +// Returns true when annotations have been updated, otherwise returns false. +// +// When this method returns true, the svc resource will need to be updated on +// the cluster. +func ensureAutoTLSAnnotation(svc *corev1.Service, secretName string, enabled bool) bool { + var autoTLSAnnotationName, autoTLSAnnotationValue string + + // We currently only support OpenShift for automatic TLS + if IsRouteAPIAvailable() { + autoTLSAnnotationName = common.AnnotationOpenShiftServiceCA if svc.Annotations == nil { svc.Annotations = make(map[string]string) } + autoTLSAnnotationValue = secretName + } + + if autoTLSAnnotationName != "" { val, ok := svc.Annotations[autoTLSAnnotationName] - if !ok || val != common.ArgoCDRepoServerTLSSecretName { - log.Info(fmt.Sprintf("requesting AutoTLS on service %s", svc.ObjectMeta.Name)) - svc.Annotations[autoTLSAnnotationName] = common.ArgoCDRepoServerTLSSecretName - return true + if enabled { + if !ok || val != secretName { + log.Info(fmt.Sprintf("requesting AutoTLS on service %s", svc.ObjectMeta.Name)) + svc.Annotations[autoTLSAnnotationName] = autoTLSAnnotationValue + return true + } + } else { + if ok { + log.Info(fmt.Sprintf("removing AutoTLS from service %s", svc.ObjectMeta.Name)) + delete(svc.Annotations, autoTLSAnnotationName) + return true + } } } @@ -333,13 +353,13 @@ func (r *ReconcileArgoCD) reconcileRepoService(cr *argoprojv1a1.ArgoCD) error { svc := newServiceWithSuffix("repo-server", "repo-server", cr) if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) { - if ensureAutoTLSAnnotation(cr, svc) { + if ensureAutoTLSAnnotation(svc, common.ArgoCDRepoServerTLSSecretName, cr.Spec.Repo.WantsAutoTLS()) { return r.Client.Update(context.TODO(), svc) } return nil // Service found, do nothing } - ensureAutoTLSAnnotation(cr, svc) + ensureAutoTLSAnnotation(svc, common.ArgoCDRepoServerTLSSecretName, cr.Spec.Repo.WantsAutoTLS()) svc.Spec.Selector = map[string]string{ common.ArgoCDKeyName: nameWithSuffix("repo-server", cr), @@ -395,9 +415,14 @@ func (r *ReconcileArgoCD) reconcileServerMetricsService(cr *argoprojv1a1.ArgoCD) func (r *ReconcileArgoCD) reconcileServerService(cr *argoprojv1a1.ArgoCD) error { svc := newServiceWithSuffix("server", "server", cr) if argoutil.IsObjectFound(r.Client, cr.Namespace, svc.Name, svc) { + if ensureAutoTLSAnnotation(svc, common.ArgoCDServerTLSSecretName, cr.Spec.Server.WantsAutoTLS()) { + return r.Client.Update(context.TODO(), svc) + } return nil // Service found, do nothing } + ensureAutoTLSAnnotation(svc, common.ArgoCDServerTLSSecretName, cr.Spec.Server.WantsAutoTLS()) + svc.Spec.Ports = []corev1.ServicePort{ { Name: "http", diff --git a/controllers/argocd/service_test.go b/controllers/argocd/service_test.go index d38baddf4..32d11d5a7 100644 --- a/controllers/argocd/service_test.go +++ b/controllers/argocd/service_test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/argoproj-labs/argocd-operator/common" "gotest.tools/assert" "k8s.io/apimachinery/pkg/types" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -45,3 +46,46 @@ func TestReconcileArgoCD_reconcileDexService_Dex_Disabled(t *testing.T) { assert.NilError(t, r.reconcileDexService(a)) assert.ErrorContains(t, r.Client.Get(context.TODO(), types.NamespacedName{Namespace: s.Namespace, Name: s.Name}, s), "not found") } + +func TestEnsureAutoTLSAnnotation(t *testing.T) { + a := makeTestArgoCD() + t.Run("Ensure annotation will be set for OpenShift", func(t *testing.T) { + routeAPIFound = true + svc := newService(a) + + // Annotation is inserted, update is required + needUpdate := ensureAutoTLSAnnotation(svc, "some-secret", true) + assert.Equal(t, needUpdate, true) + atls, ok := svc.Annotations[common.AnnotationOpenShiftServiceCA] + assert.Equal(t, ok, true) + assert.Equal(t, atls, "some-secret") + + // Annotation already set, doesn't need update + needUpdate = ensureAutoTLSAnnotation(svc, "some-secret", true) + assert.Equal(t, needUpdate, false) + }) + t.Run("Ensure annotation will be unset for OpenShift", func(t *testing.T) { + routeAPIFound = true + svc := newService(a) + svc.Annotations = make(map[string]string) + svc.Annotations[common.AnnotationOpenShiftServiceCA] = "some-secret" + + // Annotation getting removed, update required + needUpdate := ensureAutoTLSAnnotation(svc, "some-secret", false) + assert.Equal(t, needUpdate, true) + _, ok := svc.Annotations[common.AnnotationOpenShiftServiceCA] + assert.Equal(t, ok, false) + + // Annotation does not exist, no update required + needUpdate = ensureAutoTLSAnnotation(svc, "some-secret", false) + assert.Equal(t, needUpdate, false) + }) + t.Run("Ensure annotation will not be set for non-OpenShift", func(t *testing.T) { + routeAPIFound = false + svc := newService(a) + needUpdate := ensureAutoTLSAnnotation(svc, "some-secret", true) + assert.Equal(t, needUpdate, false) + _, ok := svc.Annotations[common.AnnotationOpenShiftServiceCA] + assert.Equal(t, ok, false) + }) +} diff --git a/docs/usage/routes.md b/docs/usage/routes.md index d4e23ee88..baee40c4c 100644 --- a/docs/usage/routes.md +++ b/docs/usage/routes.md @@ -45,3 +45,79 @@ To get the password for the admin user: ```shell $ kubectl get secret argocd-cluster -n argocd -ojsonpath='{.data.admin\.password}' | base64 --decode ``` + +## 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. + +### TLS edge termination mode + +In `edge` termination mode, the route controller terminates the TLS connection and proxies the requests +to Argo CD in plain text throughout the cluster. + +The `edge` termination mode requires the Argo CD server to run in `insecure` mode, so it will accept +HTTP requests instead of TLS requests. + +To set a route to `edge` mode, you can use the following configuration: + +```yaml +spec: + server: + insecure: true + route: + enabled: true + tls: + termination: edge + insecureEdgeTerminationPolicy: Redirect +``` + +Keep in mind that the connection will be unencrypted within your cluster. + +### TLS passthrough mode + +Passthrough will terminate TLS not on the route controller, but at the `argocd-server` service. This means, +that Argo CD will need to be configured with a valid TLS certificate, otherwise clients will issue +a warning upon trying to connect. + +To set a route to `passthrough` mode, you can use the following configuration: + +```yaml +spec: + server: + route: + enabled: true + tls: + termination: passthrough +``` + +### TLS reencrypt mode + +The `reencrypt` mode works a bit like the `edge` mode, in that TLS termination of the client +will happen at the route controller. However, unlike `edge` mode, the communication between +the route controller and the Argo CD server will be encrypted as well, so the Argo CD server +does not need to be set in `insecure` mode. + +For this to work, the route controller needs to be able to validate the Argo CD server's TLS +certificate, otherwise the request will fail. + +If you enable `reencrypt` mode in an OCP cluster, the Operator will request a valid TLS +certificate for the `argocd-server` service from OpenShift's Service CA, which is sufficient +for satisfying the validation constraints of the route controller. The Service CA will issue +this certificate to a secret named `argocd-server-tls` in the operand's namespace if it does +not yet exist. + +When you later chose to switch back to another TLS termination policy, you should manually +delete the `argocd-server-tls` secret from the namespace after changing the mode. + +To enable `reencrypt` mode, you can use the following configuration: + +```yaml +spec: + server: + route: + enabled: true + tls: + termination: reencrypt + insecureEdgeTerminationPolicy: Redirect +``` +