Merge pull request #522 from cyberark/log-level

CNJR-2306: Make log level configurable

CHANGELOG.md

@@ -6,7 +6,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
-## [0.25.2] - 2023-06-26
+## [0.26.0] - 2023-07-18
+
+### Added
+- Log level is now configurable using the `LOG_LEVEL` environment variable or `conjur.org/log-level` annotation.
+  The existing `DEBUG` environment variable and `conjur.org/debug-logging` annotation is deprecated and will be removed in a future update.
+  [cyberark/conjur-authn-k8s-client#522](https://github.com/cyberark/conjur-authn-k8s-client/pull/522)
+
 ### Fixed
 - Update RH base image to `ubi9/ubi` to match the libc version of the authenticator-client-builder image.
   [cyberark/conjur-authn-k8s-client#520](https://github.com/cyberark/conjur-authn-k8s-client/pull/520)

bin/dev

@@ -9,4 +9,5 @@ docker build \
 docker run --rm \
            -it \
            -v $(cd ..; pwd):/opt/conjur-authn-k8s-client \
-           conjur-authn-k8s-client-go:builder bash
+           --entrypoint bash \
+           conjur-authn-k8s-client-go:builder

cmd/authenticator/main.go

@@ -16,6 +16,7 @@ import (
 )
 
 func main() {
+	// Note: This will log even if the log level is set to "warn" or "error" since that's loaded after this
 	log.Info(log.CAKC048, authenticator.FullVersionName)
 
 	var err error

helm/conjur-app-deploy/charts/app-secrets-provider-p2f-injected/templates/test_app_secrets_provider_p2f_injected.yaml

@@ -40,7 +40,7 @@ spec:
         conjur.org/container-image: "cyberark/secrets-provider-for-k8s:edge"
         conjur.org/conjur-inject-volumes: "test-app"
         conjur.org/container-mode: "init"
-        conjur.org/debug-logging: "true"
+        conjur.org/log-level: "debug"
         conjur.org/authn-identity: {{ quote .Values.conjur.authnLogin }}
         conjur.org/secrets-destination: "file"
         conjur.org/conjur-secrets.p2f-app: |

helm/conjur-app-deploy/charts/app-secrets-provider-p2f-jwt/templates/test_app_secrets_provider_p2f.yaml

@@ -35,7 +35,7 @@ spec:
         app: test-app-secrets-provider-p2f
       annotations:
         conjur.org/container-mode: "init"
-        conjur.org/debug-logging: "true"
+        conjur.org/log-level: "debug"
         conjur.org/secrets-destination: "file"
         conjur.org/jwt-token-path: /var/run/secrets/tokens/{{ .Values.secretsProvider.jwt.tokenFile }}
         conjur.org/conjur-secrets.p2f-app: |

helm/conjur-app-deploy/charts/app-secrets-provider-p2f/templates/test_app_secrets_provider_p2f.yaml

@@ -35,7 +35,7 @@ spec:
         app: test-app-secrets-provider-p2f
       annotations:
         conjur.org/container-mode: "init"
-        conjur.org/debug-logging: "true"
+        conjur.org/log-level: "debug"
         conjur.org/authn-identity: {{ quote .Values.conjur.authnLogin }}
         conjur.org/secrets-destination: "file"
         conjur.org/conjur-secrets.p2f-app: |

helm/conjur-app-deploy/charts/app-secrets-provider-rotation/templates/test_app_secrets_provider_rotation.yaml

@@ -37,7 +37,7 @@ spec:
         conjur.org/container-mode: "sidecar"
         conjur.org/secrets-refresh-enabled: "true"
         conjur.org/secrets-refresh-interval: "10s"
-        conjur.org/debug-logging: "true"
+        conjur.org/log-level: "debug"
         conjur.org/authn-identity: {{ quote .Values.conjur.authnLogin }}
         conjur.org/secrets-destination: "file"
         conjur.org/conjur-secrets.rotation-app: |

helm/conjur-config-cluster-prep/README.md

@@ -449,4 +449,4 @@ The following table lists the configurable parameters of the Conjur Open Source
 |`test.colorize`|Determines whether Helm test output should include color escape sequences|Defaults to `true`||
 |`test.authentication.enable`|Indicates whether the Helm test should attempt to authenticate with the Conjur instance|`false`||
 |`test.authentication.validatorID`|Indicates the Conjur Host ID that should be used to authenticate with the Conjur instance|`validator`||
-|`test.authentication.debug`|Enables Helm test authenticator init/sidecar container debug logging when set to `true`|`true`||
+|`test.authentication.logLevel`|Sets log level in authenticator init/sidecar container|`debug`||

helm/conjur-config-cluster-prep/templates/tests/_authn_container.yaml.txt

@@ -21,9 +21,9 @@
       fieldRef:
         apiVersion: v1
         fieldPath: status.podIP
-  {{- if eq .Values.test.authentication.debug true }}
-  - name: DEBUG
-    value: "true"
+  {{- if .Values.test.authentication.logLevel }}
+  - name: LOG_LEVEL
+    value: {{ .Values.test.authentication.logLevel }}
   {{- end }}
   - name: CONJUR_AUTHN_URL
     value: {{ .Values.conjur.applianceUrl }}/authn-k8s/{{ .Values.authnK8s.authenticatorID }}

helm/conjur-config-cluster-prep/test-helm

@@ -57,7 +57,7 @@ function main() {
       --reuse-values \
       --set test.authentication.enable="$test_authentication" \
       --set test.authentication.validatorID="$validator_id" \
-      --set test.authentication.debug=true \
+      --set test.authentication.logLevel=debug \
       --timeout "$UPGRADE_TIMEOUT" \
       --wait
 

helm/conjur-config-cluster-prep/values.yaml

@@ -61,5 +61,6 @@ test:
     # The authn-k8s sidecar will use a CONJUR_AUTHN_LOGIN value of:
     #    host/conjur/authn-k8s/{{authenticatorID}}/{{validatorID}}
     validatorID: "apps/validator"
-    # 'debug' enables authenticator sidecar debug logs during testing
-    debug: true
+    # 'logLevel' sets authenticator sidecar log level. Valid values are:
+    #    "debug", "info", "warn", "error". Defaults to "debug".
+    logLevel: "debug"

helm/conjur-config-namespace-prep/templates/tests/_authn_container.yaml.txt

@@ -21,9 +21,9 @@
       fieldRef:
         apiVersion: v1
         fieldPath: status.podIP
-  {{- if eq .Values.test.authentication.debug true }}
+  {{- if .Values.test.authentication.logLevel }}
   - name: DEBUG
-    value: "true"
+    value: {{ .Values.test.authentication.logLevel }}
   {{- end }}
   - name: CONJUR_AUTHN_LOGIN
     value: {{ required "A valid .Values.test.authentication.authnLogin required!" .Values.test.authentication.authnLogin }}

helm/conjur-config-namespace-prep/test-helm

@@ -70,7 +70,7 @@ function main() {
       --reuse-values \
       --set test.authentication.enable="$test_authentication" \
       --set test.authentication.authnLogin="$host_id" \
-      --set test.authentication.debug=true \
+      --set test.authentication.logLevel=debug \
       --timeout "$UPGRADE_TIMEOUT" \
       --wait
 

helm/conjur-config-namespace-prep/values.yaml

@@ -40,5 +40,5 @@ MIIC/ThisIsAMockThisIsOnlyAMock==\n
     # is required) in Conjur security policy. Example:
     #    authnLogin: host/conjur/authn-k8s/my-authenticator-id/apps/validator
     authnLogin:
-    # 'debug' enables authenticator sidecar debug logs during testing
-    debug: true
+    # 'log-level: debug' enables authenticator sidecar debug logs during testing
+    log-level: debug

pkg/authenticator/config/configuration_factory.go

@@ -3,7 +3,6 @@ package config
 import (
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"strings"
 
@@ -15,13 +14,14 @@ import (
 )
 
 const authnURLVarName string = "CONJUR_AUTHN_URL"
+const defaultLogLevel string = "info"
 
 // AuthnSettings represents a group of authenticator client configuration settings.
 type AuthnSettings map[string]string
 
 // NewConfigFromEnv returns a config ConfigFromEnv using the standard file reader for reading certs
 func NewConfigFromEnv() (Configuration, error) {
-	return ConfigFromEnv(ioutil.ReadFile)
+	return ConfigFromEnv(os.ReadFile)
 }
 
 // ConfigFromEnv returns a new authenticator configuration object
@@ -31,7 +31,8 @@ func ConfigFromEnv(readFileFunc common.ReadFileFunc) (Configuration, error) {
 
 func NewConfigFromCustomEnv(readFileFunc common.ReadFileFunc, customEnv func(key string) string) (Configuration, error) {
 	log.Debug(log.CAKC068)
-	configureDebugIfNeeded(customEnv)
+	logLevel := getConfiguredLogLevel(customEnv)
+	log.SetLogLevel(logLevel)
 	authnUrl := customEnv(authnURLVarName)
 	conf, err := getConfiguration(authnUrl)
 	if err != nil {
@@ -140,18 +141,31 @@ func getConfigVariable(getters ...func(key string) string) func(string) string {
 	}
 }
 
-func configureDebugIfNeeded(getConfigFunc func(key string) string) {
-	validVal := "true"
-	debugValue := getConfigFunc("DEBUG")
+func getConfiguredLogLevel(getConfigFunc func(key string) string) string {
+	validLogLevels := []string{"debug", "info", "warn", "error"}
+	logLevel := getConfigFunc("LOG_LEVEL")
 
-	switch debugValue {
-	case validVal:
-		log.EnableDebugMode()
-	case "":
-		// Log level not configured
-		break
-	default:
-		// Log level is configured but it's invalid
-		log.Warn(log.CAKC034, debugValue, validVal)
+	if logLevel != "" {
+		// If log level is configured, check if it's valid
+		for _, validLevel := range validLogLevels {
+			if logLevel == validLevel {
+				return logLevel
+			}
+		}
+
+		// If log level is configured but it's invalid, log a warning and return default
+		log.Warn(log.CAKC034, logLevel, validLogLevels)
+		return defaultLogLevel
 	}
+
+	// If log level is not configured, check if debug is configured.
+	// This is for backwards compatibility with the old debug env var.
+	debugValue := getConfigFunc("DEBUG")
+	if debugValue == "true" {
+		log.Warn(log.CAKC081)
+		return "debug"
+	}
+
+	// If neither log level nor debug are configured, return default
+	return defaultLogLevel
 }

pkg/authenticator/config/validate_test.go → pkg/authenticator/config/configuration_factory_test.go

@@ -1,15 +1,19 @@
 package config
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
+	"io"
+	"os"
 	"testing"
 
 	logger "github.com/cyberark/conjur-authn-k8s-client/pkg/log"
 	"github.com/stretchr/testify/assert"
 )
 
 type errorAssertFunc func(*testing.T, []error)
+type configAssertFunc func(*testing.T, error, Configuration, string)
 
 func TestValidate(t *testing.T) {
 	TestCases := []struct {
@@ -140,6 +144,119 @@ func TestValidate(t *testing.T) {
 	}
 }
 
+func TestNewConfigFromEnv(t *testing.T) {
+	TestCases := []struct {
+		description string
+		envVars     map[string]string
+		assert      configAssertFunc
+	}{
+		{
+			description: "log level set to debug",
+			envVars: mergeRequiredVars(
+				map[string]string{
+					"LOG_LEVEL": "debug",
+				}),
+			assert: func(t *testing.T, err error, config Configuration, logOutput string) {
+				assert.NoError(t, err)
+				assert.NotNil(t, config)
+				assert.Contains(t, logOutput, "CAKC052 Debug mode is enabled")
+				assert.Contains(t, logOutput, "CAKC074 Successfully validated input configuration")
+				assert.Contains(t, logOutput, "CAKC070 Chosen \"authn-jwt\" configuration")
+			},
+		},
+		{
+			description: "log level set to info",
+			envVars: mergeRequiredVars(
+				map[string]string{
+					"LOG_LEVEL": "info",
+				}),
+			assert: func(t *testing.T, err error, config Configuration, logOutput string) {
+				assert.NoError(t, err)
+				assert.NotNil(t, config)
+				assert.NotContains(t, logOutput, "CAKC052 Debug mode is enabled")
+				assert.NotContains(t, logOutput, "CAKC074 Successfully validated input configuration")
+				assert.Contains(t, logOutput, "CAKC070 Chosen \"authn-jwt\" configuration")
+			},
+		},
+		{
+			description: "log level set to warn",
+			envVars: mergeRequiredVars(
+				map[string]string{
+					"LOG_LEVEL": "warn",
+				}),
+			assert: func(t *testing.T, err error, config Configuration, logOutput string) {
+				assert.NoError(t, err)
+				assert.NotNil(t, config)
+				assert.NotContains(t, logOutput, "CAKC052 Debug mode is enabled")
+				assert.NotContains(t, logOutput, "CAKC074 Successfully validated input configuration")
+				assert.NotContains(t, logOutput, "CAKC070 Chosen \"authn-jwt\" configuration")
+			},
+		},
+		{
+			description: "invalid log level",
+			envVars: mergeRequiredVars(
+				map[string]string{
+					"LOG_LEVEL": "invalid",
+				}),
+			assert: func(t *testing.T, err error, config Configuration, logOutput string) {
+				assert.NoError(t, err)
+				assert.NotNil(t, config)
+				assert.NotContains(t, logOutput, "CAKC052 Debug mode is enabled")
+				// Should default to "info"
+				assert.NotContains(t, logOutput, "CAKC074 Successfully validated input configuration")
+				assert.Contains(t, logOutput, "CAKC070 Chosen \"authn-jwt\" configuration")
+				assert.Contains(t, logOutput, "CAKC034 Invalid value 'invalid' provided for log level")
+			},
+		},
+		{
+			description: "DEBUG set to true",
+			envVars: mergeRequiredVars(
+				map[string]string{
+					"DEBUG": "true",
+				}),
+			assert: func(t *testing.T, err error, config Configuration, logOutput string) {
+				assert.NoError(t, err)
+				assert.NotNil(t, config)
+				// Should print deprecation warning but still work
+				assert.Contains(t, logOutput, "CAKC081 'DEBUG'/'conjur.org/debug-logging' is deprecated. Use 'LOG_LEVEL'/'conjur.org/log-level'='debug' instead.")
+				assert.Contains(t, logOutput, "CAKC052 Debug mode is enabled")
+				assert.Contains(t, logOutput, "CAKC074 Successfully validated input configuration")
+				assert.Contains(t, logOutput, "CAKC070 Chosen \"authn-jwt\" configuration")
+			},
+		},
+	}
+
+	for _, tc := range TestCases {
+		t.Run(tc.description, func(t *testing.T) {
+			// SETUP & EXERCISE
+
+			// Set environment variables
+			for key, value := range tc.envVars {
+				os.Setenv(key, value)
+			}
+			// Reset environment variables after test
+			defer func() {
+				for key := range tc.envVars {
+					os.Unsetenv(key)
+				}
+			}()
+
+			// Intercept logger output
+			logOutput := io.ReadWriter(&bytes.Buffer{})
+			logger.ErrorLogger.SetOutput(logOutput)
+			logger.InfoLogger.SetOutput(logOutput)
+
+			configObj, err := NewConfigFromEnv()
+
+			// Split log output into individual messages
+			logText, _ := io.ReadAll(logOutput)
+
+			// ASSERT
+			tc.assert(t, err, configObj, string(logText))
+		})
+	}
+}
+
 func assertErrorInList(err error) errorAssertFunc {
 	return func(t *testing.T, errorList []error) {
 		assert.Contains(t, errorList, err)
@@ -161,3 +278,17 @@ func assertErrorNotInList(err error) errorAssertFunc {
 		assert.NotContains(t, errorList, err)
 	}
 }
+
+func mergeRequiredVars(newVars map[string]string) map[string]string {
+	requiredVars := map[string]string{
+		"CONJUR_AUTHN_URL":       "authn-jwt",
+		"CONJUR_ACCOUNT":         "testAccount",
+		"JWT_TOKEN_PATH":         "/tmp/token",
+		"CONTAINER_MODE":         "init",
+		"CONJUR_SSL_CERTIFICATE": "samplecertificate",
+	}
+	for key, value := range newVars {
+		requiredVars[key] = value
+	}
+	return requiredVars
+}

pkg/authenticator/jwt/config.go

@@ -44,6 +44,7 @@ var envVariables = []string{
 	"CONJUR_TOKEN_TIMEOUT",
 	"CONTAINER_MODE",
 	"DEBUG",
+	"LOG_LEVEL",
 	"JWT_TOKEN_PATH",
 	"CONJUR_AUTHN_LOGIN",
 }