fix: improve logging - remove logrus mixing; dont fail controller for…

… errors, just report - keeps controller healthy, while secrets may be broken (#42)

* fix: improve logging - remove logrus mixing; dont fail controller for errors, just report - keeps controller healthy, while secrets may be broken

Makefile

@@ -1,6 +1,6 @@
 SHELL := /bin/bash
 GO := GO15VENDOREXPERIMENT=1 GO111MODULE=on GOPROXY=https://proxy.golang.org go
-SOPS_SEC_OPERATOR_VERSION := 0.1.7
+SOPS_SEC_OPERATOR_VERSION := 0.1.8
 
 # https://github.com/kubernetes-sigs/controller-tools/releases
 CONTROLLER_TOOLS_VERSION := "v0.3.0"

chart/helm2/sops-secrets-operator/Chart.yaml

@@ -1,6 +1,6 @@
 apiVersion: v1
-version: 0.5.2
-appVersion: 0.1.7
+version: 0.5.3
+appVersion: 0.1.8
 description: sops secrets operator
 name: sops-secrets-operator
 sources:

chart/helm2/sops-secrets-operator/README.md

@@ -83,7 +83,7 @@ The following table lists the configurable parameters of the Sops-secrets-operat
 | ------------------------ | ----------------------- | -------------- |
 | `replicaCount` | Deployment replica count  - should not be modified | `1` |
 | `image.repository` | Operator image | `"isindir/sops-secrets-operator"` |
-| `image.tag` | Operator image tag | `"0.1.7"` |
+| `image.tag` | Operator image tag | `"0.1.8"` |
 | `image.pullPolicy` | Operator image pull policy | `"Always"` |
 | `imagePullSecrets` | Secrets to pull image from private docker repository | `[]` |
 | `nameOverride` | Overrides auto-generated short resource name | `""` |

chart/helm2/sops-secrets-operator/values.yaml

@@ -6,7 +6,7 @@ replicaCount: 1  # Deployment replica count  - should not be modified
 
 image:
   repository: isindir/sops-secrets-operator  # Operator image
-  tag: 0.1.7  # Operator image tag
+  tag: 0.1.8  # Operator image tag
   pullPolicy: Always  # Operator image pull policy
 
 imagePullSecrets: []  # Secrets to pull image from private docker repository

chart/helm3/sops-secrets-operator/Chart.yaml

@@ -1,6 +1,6 @@
 apiVersion: v2
-version: 0.6.2
-appVersion: 0.1.7
+version: 0.6.3
+appVersion: 0.1.8
 type: application
 description: sops secrets operator
 name: sops-secrets-operator

chart/helm3/sops-secrets-operator/README.md

@@ -83,7 +83,7 @@ The following table lists the configurable parameters of the Sops-secrets-operat
 | ------------------------ | ----------------------- | -------------- |
 | `replicaCount` | Deployment replica count  - should not be modified | `1` |
 | `image.repository` | Operator image | `"isindir/sops-secrets-operator"` |
-| `image.tag` | Operator image tag | `"0.1.7"` |
+| `image.tag` | Operator image tag | `"0.1.8"` |
 | `image.pullPolicy` | Operator image pull policy | `"Always"` |
 | `imagePullSecrets` | Secrets to pull image from private docker repository | `[]` |
 | `nameOverride` | Overrides auto-generated short resource name | `""` |

chart/helm3/sops-secrets-operator/tests/operator_test.yaml

@@ -30,8 +30,8 @@ tests:
             app.kubernetes.io/instance: sops
             app.kubernetes.io/managed-by: Helm
             app.kubernetes.io/name: sops-secrets-operator
-            app.kubernetes.io/version: 0.1.7
-            helm.sh/chart: sops-secrets-operator-0.6.2
+            app.kubernetes.io/version: 0.1.8
+            helm.sh/chart: sops-secrets-operator-0.6.3
 
   # template metadata and spec selector
   - it: should correctly render template metadata and spec selector
@@ -140,7 +140,7 @@ tests:
     asserts:
       - equal:
           path: spec.template.spec.containers[0].image
-          value: isindir/sops-secrets-operator:0.1.7
+          value: isindir/sops-secrets-operator:0.1.8
       - equal:
           path: spec.template.spec.containers[0].imagePullPolicy
           value: Always

chart/helm3/sops-secrets-operator/values.yaml

@@ -6,7 +6,7 @@ replicaCount: 1  # Deployment replica count  - should not be modified
 
 image:
   repository: isindir/sops-secrets-operator  # Operator image
-  tag: 0.1.7  # Operator image tag
+  tag: 0.1.8  # Operator image tag
   pullPolicy: Always  # Operator image pull policy
 
 imagePullSecrets: []  # Secrets to pull image from private docker repository

controllers/sopssecret_controller.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io/ioutil"
 
 	"github.com/go-logr/logr"
 	"github.com/sirupsen/logrus"
@@ -71,20 +72,14 @@ func (r *SopsSecretReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error)
 
 	instance, err := decryptSopsSecretInstance(instanceEncrypted, r.Log)
 	if err != nil {
-		r.Log.Info(
-			"Decryption error",
-			"sopssecret",
-			req.NamespacedName,
-			"error",
-			err,
-		)
 		//instance.Status.SecretsTotal = len(instance.Spec.SecretsTemplate)
 		instanceEncrypted.Status.Message = "Decryption error"
 
 		// will not process instance error as we are already in error mode here
 		r.Status().Update(context.Background(), instanceEncrypted)
 
-		return reconcile.Result{}, err
+		// Error conditon, but don't fail controller as it will not help, the actual error is already logged
+		return reconcile.Result{}, nil
 	}
 
 	// iterating over secret templates
@@ -96,7 +91,14 @@ func (r *SopsSecretReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error)
 			instanceEncrypted.Status.Message = "New child secret creation error"
 			r.Status().Update(context.Background(), instanceEncrypted)
 
-			return reconcile.Result{}, err
+			r.Log.Info(
+				"New child secret creation error",
+				"sopssecret",
+				req.NamespacedName,
+				"error",
+				err,
+			)
+			return reconcile.Result{}, nil
 		}
 
 		// Set SopsSecret instance as the owner and controller
@@ -108,7 +110,14 @@ func (r *SopsSecretReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error)
 			instanceEncrypted.Status.Message = "Setting controller ownership of the child secret error"
 			r.Status().Update(context.Background(), instanceEncrypted)
 
-			return reconcile.Result{}, err
+			r.Log.Info(
+				"Setting controller ownership of the child secret error",
+				"sopssecret",
+				req.NamespacedName,
+				"error",
+				err,
+			)
+			return reconcile.Result{}, nil
 		}
 
 		// Check if this Secret already exists
@@ -136,18 +145,28 @@ func (r *SopsSecretReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error)
 			instanceEncrypted.Status.Message = "Unknown Error"
 			r.Status().Update(context.Background(), instanceEncrypted)
 
-			return reconcile.Result{}, err
+			r.Log.Info(
+				"Unknown Error",
+				"sopssecret",
+				req.NamespacedName,
+				"error",
+				err,
+			)
+			return reconcile.Result{}, nil
 		}
 
 		if !metav1.IsControlledBy(foundSecret, instance) {
 			instanceEncrypted.Status.Message = "Child secret is not owned by controller error"
 			r.Status().Update(context.Background(), instanceEncrypted)
 
-			return reconcile.Result{}, fmt.Errorf(
-				"secret/%s in %s has a conflict with reconciling request sops secret, potential reasons: target k8s secret already existed or managed secret duplicated in multiple sops secrets",
-				foundSecret.Name,
-				foundSecret.Namespace,
+			r.Log.Info(
+				"Child secret is not owned by controller or sopssecret Error",
+				"sopssecret",
+				req.NamespacedName,
+				"error",
+				fmt.Errorf("sopssecret has a conflict with existing kubernetes secret resource, potential reasons: target secret already pre-existed or is managed by multiple sops secrets"),
 			)
+			return reconcile.Result{}, nil
 		}
 
 		origSecret := foundSecret
@@ -171,7 +190,14 @@ func (r *SopsSecretReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error)
 				instanceEncrypted.Status.Message = "Child secret update error"
 				r.Status().Update(context.Background(), instanceEncrypted)
 
-				return reconcile.Result{}, err
+				r.Log.Info(
+					"Child secret update error",
+					"sopssecret",
+					req.NamespacedName,
+					"error",
+					err,
+				)
+				return reconcile.Result{}, nil
 			}
 			r.Log.Info(
 				"Secret successfully refreshed",
@@ -186,14 +212,25 @@ func (r *SopsSecretReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error)
 	instanceEncrypted.Status.Message = "Healthy"
 	r.Status().Update(context.Background(), instanceEncrypted)
 
+	r.Log.Info(
+		"SopsSecret is Healthy",
+		"sopssecret",
+		req.NamespacedName,
+	)
 	return ctrl.Result{}, nil
 }
 
 // SetupWithManager - setup with manager
 func (r *SopsSecretReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
+	// Set logging level
 	sopslogging.SetLevel(logrus.InfoLevel)
 
+	// Set logrus logs to be discarded
+	for k := range sopslogging.Loggers {
+		sopslogging.Loggers[k].Out = ioutil.Discard
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&isindirv1alpha2.SopsSecret{}).
 		Owns(&corev1.Secret{}).
@@ -294,6 +331,8 @@ func decryptSopsSecretInstance(
 	if err != nil {
 		reqLogger.Info(
 			"Failed to convert encrypted sops secret to bytes[]",
+			"sopssecret",
+			fmt.Sprintf("%s/%s", instanceEncrypted.Namespace, instanceEncrypted.Name),
 			"error",
 			err,
 		)
@@ -304,6 +343,8 @@ func decryptSopsSecretInstance(
 	if err != nil {
 		reqLogger.Info(
 			"Failed to Decrypt encrypted sops secret instance",
+			"sopssecret",
+			fmt.Sprintf("%s/%s", instanceEncrypted.Namespace, instanceEncrypted.Name),
 			"error",
 			err,
 		)
@@ -315,6 +356,8 @@ func decryptSopsSecretInstance(
 	if err != nil {
 		reqLogger.Info(
 			"Failed to Unmarshal decrypted sops secret instance",
+			"sopssecret",
+			fmt.Sprintf("%s/%s", instanceEncrypted.Namespace, instanceEncrypted.Name),
 			"error",
 			err,
 		)