Use klog and logr logger instead of log

Signed-off-by: Richard Wall <[email protected]>

Author: wallrj

cmd/root.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"strings"
@@ -45,10 +46,13 @@ func init() {
 // will be logged and the process will exit with status 1.
 func Execute() {
 	logs.AddFlags(rootCmd.PersistentFlags())
+	log := klog.Background()
+	ctx := klog.NewContext(context.Background(), log)
+
 	var exitCode int
-	if err := rootCmd.Execute(); err != nil {
+	if err := rootCmd.ExecuteContext(ctx); err != nil {
 		exitCode = 1
-		klog.ErrorS(err, "Exiting due to error", "exit-code", exitCode)
+		log.Error(err, "Exiting due to error", "exit-code", exitCode)
 	}
 	klog.FlushAndExit(klog.ExitFlushTimeout, exitCode)
 }

pkg/agent/config.go

@@ -3,11 +3,11 @@ package agent
 import (
 	"fmt"
 	"io"
-	"log"
 	"net/url"
 	"os"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -355,7 +355,8 @@ type CombinedConfig struct {
 // The error returned may be a multierror.Error. Use multierror.Prefix(err,
 // "context:") rather than fmt.Errorf("context: %w", err) when wrapping the
 // error.
-func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags) (CombinedConfig, client.Client, error) {
+func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) (CombinedConfig, client.Client, error) {
+
 	res := CombinedConfig{}
 	var errs error
 
@@ -364,23 +365,23 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 		switch {
 		case flags.VenafiCloudMode && flags.CredentialsPath != "":
 			mode = VenafiCloudKeypair
-			log.Printf("Using the %s auth mode since --venafi-cloud and --credentials-path were specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --venafi-cloud and --credentials-path were specified.", mode))
 		case flags.ClientID != "" && flags.PrivateKeyPath != "":
 			mode = VenafiCloudKeypair
-			log.Printf("Using the %s auth mode since --client-id and --private-key-path were specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --client-id and --private-key-path were specified.", mode))
 		case flags.ClientID != "":
 			return CombinedConfig{}, nil, fmt.Errorf("if --client-id is specified, --private-key-path must also be specified")
 		case flags.PrivateKeyPath != "":
 			return CombinedConfig{}, nil, fmt.Errorf("--private-key-path is specified, --client-id must also be specified")
 		case flags.VenConnName != "":
 			mode = VenafiCloudVenafiConnection
-			log.Printf("Using the %s auth mode since --venafi-connection was specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --venafi-connection was specified.", mode))
 		case flags.APIToken != "":
 			mode = JetstackSecureAPIToken
-			log.Printf("Using the %s auth mode since --api-token was specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --api-token was specified.", mode))
 		case !flags.VenafiCloudMode && flags.CredentialsPath != "":
 			mode = JetstackSecureOAuth
-			log.Printf("Using the %s auth mode since --credentials-file was specified without --venafi-cloud.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --credentials-file was specified without --venafi-cloud.", mode))
 		default:
 			return CombinedConfig{}, nil, fmt.Errorf("no auth mode specified. You can use one of four auth modes:\n" +
 				" - Use (--venafi-cloud with --credentials-file) or (--client-id with --private-key-path) to use the " + string(VenafiCloudKeypair) + " mode.\n" +
@@ -403,10 +404,10 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 		case hasServerField && hasEndpointField:
 			// The `server` field takes precedence over the deprecated
 			// `endpoint` field.
-			log.Printf("The `server` and `endpoint` fields are both set in the config; using the `server` field.")
+			log.Info("The `server` and `endpoint` fields are both set in the config; using the `server` field.")
 			server = cfg.Server
 		case !hasServerField && hasEndpointField:
-			log.Printf("Using deprecated Endpoint configuration. User Server instead.")
+			log.Info("Using deprecated Endpoint configuration. User Server instead.")
 			if cfg.Endpoint.Protocol == "" && cfg.Server == "" {
 				cfg.Endpoint.Protocol = "http"
 			}
@@ -424,7 +425,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 			errs = multierror.Append(errs, fmt.Errorf("server %q is not a valid URL", server))
 		}
 		if res.AuthMode == VenafiCloudVenafiConnection && server != "" {
-			log.Printf("ignoring the server field specified in the config file. In %s mode, this field is not needed.", VenafiCloudVenafiConnection)
+			log.Info(fmt.Sprintf("Ignoring the server field specified in the config file. In %s mode, this field is not needed.", VenafiCloudVenafiConnection))
 			server = ""
 		}
 		res.Server = server
@@ -454,7 +455,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 			// change this value with the new --venafi-connection flag, and this
 			// field is simply ignored.
 			if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploadPath != "" {
-				log.Printf(`ignoring the venafi-cloud.upload_path field in the config file. In %s mode, this field is not needed.`, res.AuthMode)
+				log.Info(fmt.Sprintf(`Ignoring the venafi-cloud.upload_path field in the config file. In %s mode, this field is not needed.`, res.AuthMode))
 			}
 			uploadPath = ""
 		}
@@ -472,7 +473,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 	// https://venafi.atlassian.net/browse/VC-35385 is done.
 	{
 		if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploaderID != "" {
-			log.Printf(`ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in %s mode.`, res.AuthMode)
+			log.Info(fmt.Sprintf(`Ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in %s mode.`, res.AuthMode))
 		}
 	}
 
@@ -524,13 +525,13 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 		case flags.Period == 0 && cfg.Period == 0:
 			errs = multierror.Append(errs, fmt.Errorf("period must be set using --period or -p, or using the 'period' field in the config file"))
 		case flags.Period == 0 && cfg.Period > 0:
-			log.Printf("Using period from config %s", cfg.Period)
+			log.Info("Using period from config", "period", cfg.Period)
 			period = cfg.Period
 		case flags.Period > 0 && cfg.Period == 0:
 			period = flags.Period
 		case flags.Period > 0 && cfg.Period > 0:
 			// The flag takes precedence.
-			log.Printf("Both the 'period' field and --period are set. Using the value provided with --period.")
+			log.Info("Both the 'period' field and --period are set. Using the value provided with --period.")
 			period = flags.Period
 		}
 		res.Period = period
@@ -599,7 +600,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 // The error returned may be a multierror.Error. Use multierror.Prefix(err,
 // "context:") rather than fmt.Errorf("context: %w", err) when wrapping the
 // error.
-func validateCredsAndCreateClient(log *log.Logger, flagCredentialsPath, flagClientID, flagPrivateKeyPath, flagAPIToken string, cfg CombinedConfig) (client.Client, error) {
+func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClientID, flagPrivateKeyPath, flagAPIToken string, cfg CombinedConfig) (client.Client, error) {
 	var errs error
 
 	var preflightClient client.Client
@@ -719,7 +720,7 @@ func ValidateDataGatherers(dataGatherers []DataGatherer) error {
 
 // The error returned may be a multierror.Error. Instead of adding context to
 // the error with fmt.Errorf("%w", err), use multierror.Prefix(err, "context").
-func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg CombinedConfig, agentMetadata *api.AgentMetadata) (client.Client, error) {
+func createCredentialClient(log logr.Logger, credentials client.Credentials, cfg CombinedConfig, agentMetadata *api.AgentMetadata) (client.Client, error) {
 	switch creds := credentials.(type) {
 	case *client.VenafiSvcAccountCredentials:
 		// The uploader ID isn't actually used in the backend, let's use an
@@ -730,7 +731,7 @@ func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg
 		if cfg.AuthMode == VenafiCloudKeypair {
 			// We don't do this for the VenafiCloudVenafiConnection mode because
 			// the upload_path field is ignored in that mode.
-			log.Println("Loading upload_path from \"venafi-cloud\" configuration.")
+			log.Info("Loading upload_path from \"venafi-cloud\" configuration.")
 			uploadPath = cfg.UploadPath
 		}
 		return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression)

pkg/agent/config_test.go

@@ -6,15 +6,16 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"log"
 	"net/http"
 	"os"
 	"testing"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"k8s.io/klog/v2/ktesting"
 
 	"github.com/jetstack/preflight/pkg/client"
 	"github.com/jetstack/preflight/pkg/testutil"
@@ -86,7 +87,7 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 
 	t.Run("--period flag takes precedence over period field in config, shows warning", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
-		log, gotLogs := recordLogs()
+		log, gotLogs := recordLogs(t)
 		got, _, err := ValidateAndCombineConfig(log,
 			withConfig(testutil.Undent(`
 				server: https://api.venafi.eu
@@ -97,8 +98,8 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 			withCmdLineFlags("--period", "99m", "--credentials-file", fakeCredsPath))
 		require.NoError(t, err)
 		assert.Equal(t, testutil.Undent(`
-			Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
-			Both the 'period' field and --period are set. Using the value provided with --period.
+			INFO Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
+			INFO Both the 'period' field and --period are set. Using the value provided with --period.
 		`), gotLogs.String())
 		assert.Equal(t, 99*time.Minute, got.Period)
 	})
@@ -573,7 +574,7 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 	t.Run("venafi-cloud-workload-identity-auth: warning about server, venafi-cloud.uploader_id, and venafi-cloud.upload_path being skipped", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
 		t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
-		log, gotLogs := recordLogs()
+		log, gotLogs := recordLogs(t)
 		got, gotCl, err := ValidateAndCombineConfig(log,
 			withConfig(testutil.Undent(`
 				server: https://api.venafi.eu
@@ -587,11 +588,11 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 		)
 		require.NoError(t, err)
 		assert.Equal(t, testutil.Undent(`
-			Using the Venafi Cloud VenafiConnection auth mode since --venafi-connection was specified.
-			ignoring the server field specified in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
-			ignoring the venafi-cloud.upload_path field in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
-			ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in Venafi Cloud VenafiConnection mode.
-			Using period from config 1h0m0s
+			INFO Using the Venafi Cloud VenafiConnection auth mode since --venafi-connection was specified.
+			INFO Ignoring the server field specified in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
+			INFO Ignoring the venafi-cloud.upload_path field in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
+			INFO Ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in Venafi Cloud VenafiConnection mode.
+			INFO Using period from config period="1h0m0s"
 		`), gotLogs.String())
 		assert.Equal(t, VenafiCloudVenafiConnection, got.AuthMode)
 		assert.IsType(t, &client.VenConnClient{}, gotCl)
@@ -994,13 +995,15 @@ func withFile(t testing.TB, content string) string {
 	return f.Name()
 }
 
-func recordLogs() (*log.Logger, *bytes.Buffer) {
-	b := bytes.Buffer{}
-	return log.New(&b, "", 0), &b
+func recordLogs(t *testing.T) (logr.Logger, ktesting.Buffer) {
+	log := ktesting.NewLogger(t, ktesting.NewConfig(ktesting.BufferLogs(true)))
+	testingLogger, ok := log.GetSink().(ktesting.Underlier)
+	require.True(t, ok)
+	return log, testingLogger.GetBuffer()
 }
 
-func discardLogs() *log.Logger {
-	return log.New(io.Discard, "", 0)
+func discardLogs() logr.Logger {
+	return logr.Discard()
 }
 
 // Shortcut for ParseConfig.