Merge pull request openshift#449 from ewolinetz/43_address_lf_cert_regen

[release-4.3] Bug 1813381: addressing lf causing cert reconciliation

pkg/controller/clusterlogging/clusterlogging_controller.go

@@ -5,14 +5,12 @@ import (
 	"time"
 
 	loggingv1 "github.com/openshift/cluster-logging-operator/pkg/apis/logging/v1"
-	logforwarding "github.com/openshift/cluster-logging-operator/pkg/apis/logging/v1alpha1"
 	"github.com/openshift/cluster-logging-operator/pkg/constants"
 	"github.com/openshift/cluster-logging-operator/pkg/k8shandler"
 	"github.com/sirupsen/logrus"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -99,16 +97,6 @@ func (r *ReconcileClusterLogging) Reconcile(request reconcile.Request) (reconcil
 		return reconcile.Result{}, nil
 	}
 
-	forwardinginstance := &logforwarding.LogForwarding{}
-	fiName := types.NamespacedName{Name: constants.SingletonName, Namespace: constants.OpenshiftNS}
-	err = r.client.Get(context.TODO(), fiName, forwardinginstance)
-	if err != nil && !errors.IsNotFound(err) {
-		// Request object not found, could have been deleted after reconcile request.
-		// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
-		// Return and don't requeue
-		return reconcile.Result{}, err
-	}
-
-	err = k8shandler.Reconcile(instance, forwardinginstance, r.client)
+	err = k8shandler.Reconcile(instance, r.client)
 	return reconcileResult, err
 }

pkg/controller/forwarding/forwarding_controller.go

@@ -4,15 +4,13 @@ import (
 	"context"
 	"time"
 
-	loggingv1 "github.com/openshift/cluster-logging-operator/pkg/apis/logging/v1"
 	logforwarding "github.com/openshift/cluster-logging-operator/pkg/apis/logging/v1alpha1"
 	"github.com/openshift/cluster-logging-operator/pkg/constants"
 	"github.com/openshift/cluster-logging-operator/pkg/k8shandler"
 	"github.com/openshift/cluster-logging-operator/pkg/logger"
 
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -83,46 +81,48 @@ func (r *ReconcileForwarding) Reconcile(request reconcile.Request) (reconcile.Re
 	err := r.client.Get(context.TODO(), request.NamespacedName, instance)
 	if err != nil {
 		logger.Debugf("logforwarding-controller Error getting instance. It will be retried if other then 'NotFound': %v", err)
-		if errors.IsNotFound(err) {
-			// Request object not found, could have been deleted after reconcile request.
-			// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
-			// Return and don't requeue
-			return reconcile.Result{}, nil
+		if !errors.IsNotFound(err) {
+			// Error reading the - requeue the request.
+			return reconcileResult, err
 		}
-		// Error reading the object - requeue the request.
-		return reconcile.Result{}, err
+
+		// else the object is not found -- meaning it was removed so do clean up manually
+		reconcileErr := k8shandler.ReconcileForLogForwarding(instance, r.client)
+
+		if reconcileErr != nil {
+			logger.Debugf("logforwarding-controller returning, error: %v", reconcileErr)
+		}
+		return reconcile.Result{}, reconcileErr
 	}
+
 	logger.Debugf("logforwarding-controller fetched LF instance: %v", instance)
 
 	//check for instancename and then update status
-	var reconcileErr error = nil
 	if instance.Name != constants.SingletonName {
 		instance.Status = logforwarding.NewForwardingStatus(logforwarding.LogForwardingStateRejected, logforwarding.LogForwardingReasonName, singletonMessage)
-	} else {
-		instance.Status = logforwarding.NewForwardingStatus(logforwarding.LogForwardingStateAccepted, logforwarding.LogForwardingReasonName, "")
-
-		logger.Debug("logforwarding-controller fetching ClusterLogging instance...")
-		clInstance := &loggingv1.ClusterLogging{}
-		clName := types.NamespacedName{Name: constants.SingletonName, Namespace: constants.OpenshiftNS}
-		err = r.client.Get(context.TODO(), clName, clInstance)
-		if err != nil && !errors.IsNotFound(err) {
-			// Request object not found, could have been deleted after reconcile request.
-			// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
-			// Return and don't requeue
-			logger.Debugf("logforwarding-controller error fetching ClusterLogging instance: %v", err)
-			return reconcile.Result{}, err
+
+		logger.Debugf("logforwarding-controller updating status of instance: %v", instance)
+		if err = r.client.Status().Update(context.TODO(), instance); err != nil {
+			logger.Debugf("logforwarding-controller error updating status: %v", err)
+			return reconcileResult, err
 		}
 
-		logger.Debug("logforwarding-controller calling ClusterLogging reconciler...")
-		reconcileErr = k8shandler.Reconcile(clInstance, instance, r.client)
+		return reconcile.Result{}, nil
 	}
 
+	instance.Status = logforwarding.NewForwardingStatus(logforwarding.LogForwardingStateAccepted, logforwarding.LogForwardingReasonName, "")
+
 	logger.Debugf("logforwarding-controller updating status of instance: %v", instance)
 	if err = r.client.Status().Update(context.TODO(), instance); err != nil {
 		logger.Debugf("logforwarding-controller error updating status: %v", err)
-		return reconcile.Result{}, err
+		return reconcileResult, err
 	}
 
-	logger.Debugf("logforwarding-controller returning %v, error: %v", reconcileResult, reconcileErr)
-	return reconcileResult, reconcileErr
+	logger.Debug("logforwarding-controller calling ClusterLogging reconciler...")
+	reconcileErr := k8shandler.ReconcileForLogForwarding(instance, r.client)
+
+	if reconcileErr != nil {
+		logger.Debugf("logforwarding-controller returning, error: %v", reconcileErr)
+	}
+	return reconcile.Result{}, reconcileErr
 }

pkg/controller/proxyconfig/proxyconfig_controller.go

@@ -5,8 +5,6 @@ import (
 	"time"
 
 	configv1 "github.com/openshift/api/config/v1"
-	loggingv1 "github.com/openshift/cluster-logging-operator/pkg/apis/logging/v1"
-	logforwarding "github.com/openshift/cluster-logging-operator/pkg/apis/logging/v1alpha1"
 	"github.com/openshift/cluster-logging-operator/pkg/constants"
 	"github.com/openshift/cluster-logging-operator/pkg/k8shandler"
 	"github.com/openshift/cluster-logging-operator/pkg/utils"
@@ -89,7 +87,6 @@ type ReconcileProxyConfig struct {
 // When the user configured and/or system certs are updated, the change is propagated to the
 // configmap objects and this reconciler triggers to restart those pods.
 func (r *ReconcileProxyConfig) Reconcile(request reconcile.Request) (reconcile.Result, error) {
-	loggingNamespacedName := types.NamespacedName{Name: constants.SingletonName, Namespace: constants.OpenshiftNS}
 	proxyNamespacedName := types.NamespacedName{Name: constants.ProxyName}
 	proxyConfig := &configv1.Proxy{}
 	if request.NamespacedName == proxyNamespacedName || utils.ContainsString(constants.ReconcileForGlobalProxyList, request.Name) {
@@ -106,31 +103,7 @@ func (r *ReconcileProxyConfig) Reconcile(request reconcile.Request) (reconcile.R
 		return reconcile.Result{}, nil
 	}
 
-	// Fetch the ClusterLogging instance
-	instance := &loggingv1.ClusterLogging{}
-	if err := r.client.Get(context.TODO(), loggingNamespacedName, instance); err != nil {
-		if apierrors.IsNotFound(err) {
-			// Request object not found, could have been deleted after reconcile request.
-			// Owned objects are automatically garbage collected. For additional cleanup logic use finalizers.
-			// Return and don't requeue
-			return reconcile.Result{}, nil
-		}
-		// Error reading the object - just return without requeuing.
-		return reconcile.Result{}, err
-	}
-
-	if instance.Spec.ManagementState == loggingv1.ManagementStateUnmanaged {
-		return reconcile.Result{}, nil
-	}
-
-	forwardinginstance := &logforwarding.LogForwarding{}
-	err := r.client.Get(context.TODO(), loggingNamespacedName, forwardinginstance)
-	if err != nil && !apierrors.IsNotFound(err) {
-		// Error reading the object - just return without requeuing.
-		return reconcile.Result{}, err
-	}
-
-	if err := k8shandler.ReconcileForGlobalProxy(instance, forwardinginstance, proxyConfig, r.client); err != nil {
+	if err := k8shandler.ReconcileForGlobalProxy(proxyConfig, r.client); err != nil {
 		// Failed to reconcile - requeuing.
 		return reconcileResult, err
 	}

pkg/k8shandler/certificates.go

@@ -34,8 +34,9 @@ var secretCertificates = map[string]map[string]string{
 		"cert": "system.logging.kibana.crt",
 	},
 	"kibana-proxy": map[string]string{
-		"server-key":  "kibana-internal.key",
-		"server-cert": "kibana-internal.crt",
+		"server-key":     "kibana-internal.key",
+		"server-cert":    "kibana-internal.crt",
+		"session-secret": "kibana-session-secret",
 	},
 	"curator": map[string]string{
 		"ca":       "ca.crt",
@@ -49,14 +50,6 @@ var secretCertificates = map[string]map[string]string{
 		"ca-bundle.crt": "ca.crt",
 		"tls.key":       "system.logging.fluentd.key",
 		"tls.crt":       "system.logging.fluentd.crt",
-		/*legacy to be removed in future
-		"app-ca":     "ca.crt",
-		"app-key":    "system.logging.fluentd.key",
-		"app-cert":   "system.logging.fluentd.crt",
-		"infra-ca":   "ca.crt",
-		"infra-key":  "system.logging.fluentd.key",
-		"infra-cert": "system.logging.fluentd.crt",
-		*/
 	},
 }
 

pkg/k8shandler/reconciler.go

@@ -15,26 +15,19 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 )
 
-func Reconcile(requestCluster *logging.ClusterLogging, forwarding *logforwarding.LogForwarding, requestClient client.Client) (err error) {
-	logger.Debugf("Reconciling cl: %v, forwarding: %v", requestCluster, forwarding)
+func Reconcile(requestCluster *logging.ClusterLogging, requestClient client.Client) (err error) {
 	clusterLoggingRequest := ClusterLoggingRequest{
-		client:            requestClient,
-		cluster:           requestCluster,
-		ForwardingRequest: forwarding,
+		client:  requestClient,
+		cluster: requestCluster,
 	}
+
+	forwarding := clusterLoggingRequest.getLogForwarding()
 	if forwarding != nil {
+		clusterLoggingRequest.ForwardingRequest = forwarding
 		clusterLoggingRequest.ForwardingSpec = forwarding.Spec
 	}
 
-	// we need to see if we have the proxy available so we
-	// don't blank out any proxy configured changes...
-	proxyNamespacedName := types.NamespacedName{Name: constants.ProxyName}
-	proxyConfig := &configv1.Proxy{}
-	if err := clusterLoggingRequest.client.Get(context.TODO(), proxyNamespacedName, proxyConfig); err != nil {
-		if !apierrors.IsNotFound(err) {
-			fmt.Errorf("Encountered unexpected error getting %v", proxyNamespacedName)
-		}
-	}
+	proxyConfig := clusterLoggingRequest.getProxyConfig()
 
 	// Reconcile certs
 	if err = clusterLoggingRequest.CreateOrUpdateCertificates(); err != nil {
@@ -64,14 +57,49 @@ func Reconcile(requestCluster *logging.ClusterLogging, forwarding *logforwarding
 	return nil
 }
 
-func ReconcileForGlobalProxy(requestCluster *logging.ClusterLogging, forwarding *logforwarding.LogForwarding, proxyConfig *configv1.Proxy, requestClient client.Client) (err error) {
+func ReconcileForLogForwarding(forwarding *logforwarding.LogForwarding, requestClient client.Client) (err error) {
 
 	clusterLoggingRequest := ClusterLoggingRequest{
-		client:            requestClient,
-		cluster:           requestCluster,
-		ForwardingRequest: forwarding,
+		client: requestClient,
 	}
 	if forwarding != nil {
+		clusterLoggingRequest.ForwardingRequest = forwarding
+		clusterLoggingRequest.ForwardingSpec = forwarding.Spec
+	}
+
+	clusterLogging := clusterLoggingRequest.getClusterLogging()
+	clusterLoggingRequest.cluster = clusterLogging
+
+	if clusterLogging.Spec.ManagementState == logging.ManagementStateUnmanaged {
+		return nil
+	}
+
+	proxyConfig := clusterLoggingRequest.getProxyConfig()
+
+	// Reconcile Collection
+	if err = clusterLoggingRequest.CreateOrUpdateCollection(proxyConfig); err != nil {
+		return fmt.Errorf("Unable to create or update collection for %q: %v", clusterLoggingRequest.cluster.Name, err)
+	}
+
+	return nil
+}
+
+func ReconcileForGlobalProxy(proxyConfig *configv1.Proxy, requestClient client.Client) (err error) {
+
+	clusterLoggingRequest := ClusterLoggingRequest{
+		client: requestClient,
+	}
+
+	clusterLogging := clusterLoggingRequest.getClusterLogging()
+	clusterLoggingRequest.cluster = clusterLogging
+
+	if clusterLogging.Spec.ManagementState == logging.ManagementStateUnmanaged {
+		return nil
+	}
+
+	forwarding := clusterLoggingRequest.getLogForwarding()
+	if forwarding != nil {
+		clusterLoggingRequest.ForwardingRequest = forwarding
 		clusterLoggingRequest.ForwardingSpec = forwarding.Spec
 	}
 

pkg/k8shandler/visualization.go

@@ -522,11 +522,18 @@ func (clusterRequest *ClusterLoggingRequest) createOrUpdateKibanaSecret() error
 		return err
 	}
 
+	sessionSecret := []byte{}
+
+	sessionSecret = utils.GetWorkingDirFileContents("kibana-session-secret")
+	if sessionSecret == nil {
+		sessionSecret = utils.GetRandomWord(32)
+	}
+
 	proxySecret := NewSecret(
 		"kibana-proxy",
 		clusterRequest.cluster.Namespace,
 		map[string][]byte{
-			"session-secret": utils.GetRandomWord(32),
+			"session-secret": sessionSecret,
 			"server-key":     utils.GetWorkingDirFileContents("kibana-internal.key"),
 			"server-cert":    utils.GetWorkingDirFileContents("kibana-internal.crt"),
 		})