Merge pull request #2378 from rene-dekker/automated-cherry-pick-of-#2…

…377-origin-release-v1.28 Automated cherry pick of #2377: Fix race condition where trusted bundle is rendered twice
tigera · Jan 9, 2023 · 1d6185f · 1d6185f
2 parents 9f5c92d + 200f2eb
commit 1d6185f
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 42 deletions.
diff --git a/pkg/controller/logstorage/esgateway.go b/pkg/controller/logstorage/esgateway.go
@@ -1,4 +1,4 @@
-// Copyright (c) 2022 Tigera, Inc. All rights reserved.
+// Copyright (c) 2023 Tigera, Inc. All rights reserved.
 
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -16,11 +16,13 @@ package logstorage
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/go-logr/logr"
 	"github.com/tigera/operator/pkg/controller/certificatemanager"
 	"github.com/tigera/operator/pkg/dns"
 	rcertificatemanagement "github.com/tigera/operator/pkg/render/certificatemanagement"
+	"github.com/tigera/operator/pkg/render/monitor"
 	"github.com/tigera/operator/pkg/tls/certificatemanagement"
 	corev1 "k8s.io/api/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -43,44 +45,54 @@ func (r *ReconcileLogStorage) createEsGateway(
 	reqLogger logr.Logger,
 	ctx context.Context,
 	certificateManager certificatemanager.CertificateManager,
-) (reconcile.Result, bool, error) {
+) (reconcile.Result, certificatemanagement.TrustedBundle, bool, error) {
 	svcDNSNames := dns.GetServiceDNSNames(render.ElasticsearchServiceName, render.ElasticsearchNamespace, r.clusterDomain)
 	svcDNSNames = append(svcDNSNames, dns.GetServiceDNSNames(esgateway.ServiceName, render.ElasticsearchNamespace, r.clusterDomain)...)
 	gatewayKeyPair, err := certificateManager.GetOrCreateKeyPair(r.client, render.TigeraElasticsearchGatewaySecret, common.OperatorNamespace(), svcDNSNames)
 	if err != nil {
 		log.Error(err, "Error creating TLS certificate")
 		r.status.SetDegraded("Error creating TLS certificate", err.Error())
-		return reconcile.Result{}, false, err
+		return reconcile.Result{}, nil, false, err
 	}
 	var kibanaCertificate certificatemanagement.CertificateInterface
 	if !operatorv1.IsFIPSModeEnabled(install.FIPSMode) {
 		kibanaCertificate, err = certificateManager.GetCertificate(r.client, render.TigeraKibanaCertSecret, common.OperatorNamespace())
 		if err != nil {
 			reqLogger.Error(err, "failed to get Kibana tls certificate secret")
 			r.status.SetDegraded("Failed to get Kibana tls certificate secret", err.Error())
-			return reconcile.Result{}, false, err
+			return reconcile.Result{}, nil, false, err
 		} else if kibanaCertificate == nil {
 			reqLogger.Info("Waiting for internal Kibana tls certificate secret to be available")
 			r.status.SetDegraded("Waiting for internal Kibana tls certificate secret to be available", "")
-			return reconcile.Result{}, false, nil
+			return reconcile.Result{}, nil, false, nil
 		}
 	}
 	esInternalCertificate, err := certificateManager.GetCertificate(r.client, render.TigeraElasticsearchInternalCertSecret, common.OperatorNamespace())
 	if err != nil {
 		reqLogger.Error(err, "failed to get Elasticsearch tls certificate secret")
 		r.status.SetDegraded("Failed to get Elasticsearch tls certificate secret", err.Error())
-		return reconcile.Result{}, false, err
+		return reconcile.Result{}, nil, false, err
 	} else if esInternalCertificate == nil {
 		reqLogger.Info("Waiting for internal Elasticsearch tls certificate secret to be available")
 		r.status.SetDegraded("Waiting for internal Elasticsearch tls certificate secret to be available", "")
-		return reconcile.Result{}, false, nil
+		return reconcile.Result{}, nil, false, nil
 	}
-	trustedBundle := certificateManager.CreateTrustedBundle(esInternalCertificate, kibanaCertificate)
+
+	// Esgateway.go will render the trusted bundle for the whole namespace, so we want to include also the Prometheus
+	// TLS certificate, which the elasticsearch-metrics server depends on.
+	prometheusCertificate, err := certificateManager.GetCertificate(r.client, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace())
+	if err != nil {
+		return reconcile.Result{}, nil, false, err
+	} else if prometheusCertificate == nil {
+		r.status.SetDegraded("Prometheus secrets are not available yet, waiting until they become available", "")
+		return reconcile.Result{}, nil, false, nil
+	}
+	trustedBundle := certificateManager.CreateTrustedBundle(esInternalCertificate, kibanaCertificate, prometheusCertificate, gatewayKeyPair)
 
 	// This secret should only ever contain one key.
 	if len(esAdminUserSecret.Data) != 1 {
-		r.status.SetDegraded("Elasticsearch admin user secret contains too many entries", "")
-		return reconcile.Result{}, false, nil
+		r.status.SetDegraded("Elasticsearch admin user secret contains has too many entries", "")
+		return reconcile.Result{}, nil, false, fmt.Errorf("elasticsearch admin user secret contains has too many entries")
 	}
 
 	var esAdminUserName string
@@ -93,7 +105,7 @@ func (r *ReconcileLogStorage) createEsGateway(
 	if err != nil {
 		reqLogger.Error(err, err.Error())
 		r.status.SetDegraded("Failed to create kube-controllers secrets for Elasticsearch gateway", "")
-		return reconcile.Result{}, false, err
+		return reconcile.Result{}, nil, false, err
 	}
 
 	cfg := &esgateway.Config{
@@ -111,7 +123,7 @@ func (r *ReconcileLogStorage) createEsGateway(
 	if err = imageset.ApplyImageSet(ctx, r.client, variant, esGatewayComponent); err != nil {
 		reqLogger.Error(err, "Error with images from ImageSet")
 		r.status.SetDegraded("Error with images from ImageSet", err.Error())
-		return reconcile.Result{}, false, err
+		return reconcile.Result{}, nil, false, err
 	}
 
 	certificateComponent := rcertificatemanagement.CertificateManagement(&rcertificatemanagement.Config{
@@ -126,9 +138,9 @@ func (r *ReconcileLogStorage) createEsGateway(
 	for _, comp := range []render.Component{esGatewayComponent, certificateComponent} {
 		if err := hdler.CreateOrUpdateOrDelete(ctx, comp, r.status); err != nil {
 			r.status.SetDegraded("Error creating / updating / deleting resource", err.Error())
-			return reconcile.Result{}, false, err
+			return reconcile.Result{}, nil, false, err
 		}
 	}
 
-	return reconcile.Result{}, true, nil
+	return reconcile.Result{}, trustedBundle, true, nil
 }
diff --git a/pkg/controller/logstorage/esmetrics.go b/pkg/controller/logstorage/esmetrics.go
@@ -1,4 +1,4 @@
-// Copyright (c) 2022 Tigera, Inc. All rights reserved.
+// Copyright (c) 2023 Tigera, Inc. All rights reserved.
 
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@ package logstorage
 
 import (
 	"context"
-	"fmt"
 
 	"github.com/go-logr/logr"
 
@@ -33,7 +32,7 @@ import (
 	rcertificatemanagement "github.com/tigera/operator/pkg/render/certificatemanagement"
 	relasticsearch "github.com/tigera/operator/pkg/render/common/elasticsearch"
 	"github.com/tigera/operator/pkg/render/logstorage/esmetrics"
-	"github.com/tigera/operator/pkg/render/monitor"
+	"github.com/tigera/operator/pkg/tls/certificatemanagement"
 )
 
 func (r *ReconcileLogStorage) createEsMetrics(
@@ -45,6 +44,7 @@ func (r *ReconcileLogStorage) createEsMetrics(
 	ctx context.Context,
 	hdler utils.ComponentHandler,
 	clusterDomain string,
+	trustedBundle certificatemanagement.TrustedBundle,
 ) (reconcile.Result, bool, error) {
 	esMetricsSecret, err := utils.GetSecret(context.Background(), r.client, esmetrics.ElasticsearchMetricsSecret, common.OperatorNamespace())
 	if err != nil {
@@ -63,27 +63,6 @@ func (r *ReconcileLogStorage) createEsMetrics(
 		r.status.SetDegraded("Unable to create the Tigera CA", err.Error())
 		return reconcile.Result{}, false, err
 	}
-	prometheusCertificate, err := certificateManager.GetCertificate(r.client, monitor.PrometheusClientTLSSecretName, common.OperatorNamespace())
-	if err != nil {
-		reqLogger.Error(err, "Failed to get certificate")
-		r.status.SetDegraded("Failed to get certificate", err.Error())
-		return reconcile.Result{}, false, err
-	} else if prometheusCertificate == nil {
-		reqLogger.Info("Prometheus secrets are not available yet, waiting until they become available")
-		r.status.SetDegraded("Prometheus secrets are not available yet, waiting until they become available", "")
-		return reconcile.Result{}, false, nil
-	}
-	esgwCertificate, err := certificateManager.GetCertificate(r.client, relasticsearch.PublicCertSecret, common.OperatorNamespace())
-	if err != nil {
-		log.Error(err, fmt.Sprintf("failed to retrieve / validate %s", relasticsearch.PublicCertSecret))
-		r.status.SetDegraded(fmt.Sprintf("Failed to retrieve / validate  %s", relasticsearch.PublicCertSecret), err.Error())
-		return reconcile.Result{}, false, err
-	} else if esgwCertificate == nil {
-		log.Info("Elasticsearch gateway certificate is not available yet, waiting until they become available")
-		r.status.SetDegraded("Elasticsearch gateway certificate are not available yet, waiting until they become available", "")
-		return reconcile.Result{}, false, nil
-	}
-	trustedBundle := certificateManager.CreateTrustedBundle(prometheusCertificate, esgwCertificate)
 
 	serverTLS, err := certificateManager.GetOrCreateKeyPair(
 		r.client,
@@ -113,7 +92,9 @@ func (r *ReconcileLogStorage) createEsMetrics(
 			KeyPairOptions: []rcertificatemanagement.KeyPairOption{
 				rcertificatemanagement.NewKeyPairOption(serverTLS, true, true),
 			},
-			TrustedBundle: trustedBundle,
+			// The trusted bundle should have already been rendered by the logstorage_controller.go for es-gateway.
+			// It already includes the certificates for es gateway and prometheus server, which es-metrics relies upon.
+			TrustedBundle: nil,
 		}),
 	}
 

diff --git a/pkg/controller/logstorage/logstorage_controller.go b/pkg/controller/logstorage/logstorage_controller.go
@@ -1,4 +1,4 @@
-// Copyright (c) 2020-2022 Tigera, Inc. All rights reserved.
+// Copyright (c) 2020-2023 Tigera, Inc. All rights reserved.
 
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -21,6 +21,7 @@ import (
 
 	"github.com/tigera/operator/pkg/render/common/networkpolicy"
 	"github.com/tigera/operator/pkg/render/kubecontrollers"
+	"github.com/tigera/operator/pkg/tls/certificatemanagement"
 	"k8s.io/client-go/kubernetes"
 
 	v3 "github.com/tigera/api/pkg/apis/projectcalico/v3"
@@ -594,8 +595,8 @@ func (r *ReconcileLogStorage) Reconcile(ctx context.Context, request reconcile.R
 		if err != nil || !proceed {
 			return result, err
 		}
-
-		result, proceed, err = r.createEsGateway(
+		var trustedBundle certificatemanagement.TrustedBundle
+		result, trustedBundle, proceed, err = r.createEsGateway(
 			install,
 			variant,
 			pullSecrets,
@@ -628,6 +629,7 @@ func (r *ReconcileLogStorage) Reconcile(ctx context.Context, request reconcile.R
 			ctx,
 			hdler,
 			r.clusterDomain,
+			trustedBundle,
 		)
 		if err != nil || !proceed {
 			return result, err