Skip to content

Commit fd8d0de

Browse files
wallrjwallrj
authored
Use klog and logr logger instead of log (#609)
Signed-off-by: Richard Wall <[email protected]>
1 parent 39e6c1f commit fd8d0de

File tree

4 files changed

+89
-70
lines changed

4 files changed

+89
-70
lines changed

‎cmd/root.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"context"
45
"fmt"
56
"os"
67
"strings"
@@ -45,8 +46,9 @@ func init() {
4546
// will be logged and the process will exit with status 1.
4647
func Execute() {
4748
logs.AddFlags(rootCmd.PersistentFlags())
49+
ctx := klog.NewContext(context.Background(), klog.Background())
4850
var exitCode int
49-
if err := rootCmd.Execute(); err != nil {
51+
if err := rootCmd.ExecuteContext(ctx); err != nil {
5052
exitCode = 1
5153
klog.ErrorS(err, "Exiting due to error", "exit-code", exitCode)
5254
}

‎pkg/agent/config.go‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ package agent
33
import (
44
"fmt"
55
"io"
6-
"log"
76
"net/url"
87
"os"
98
"time"
109

10+
"github.com/go-logr/logr"
1111
"github.com/hashicorp/go-multierror"
1212
"github.com/pkg/errors"
1313
"github.com/spf13/cobra"
@@ -355,32 +355,35 @@ type CombinedConfig struct {
355355
// The error returned may be a multierror.Error. Use multierror.Prefix(err,
356356
// "context:") rather than fmt.Errorf("context: %w", err) when wrapping the
357357
// error.
358-
func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags) (CombinedConfig, client.Client, error) {
358+
func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) (CombinedConfig, client.Client, error) {
359359
res := CombinedConfig{}
360360
var errs error
361361

362362
{
363-
var mode AuthMode
363+
var (
364+
mode AuthMode
365+
reason string
366+
)
364367
switch {
365368
case flags.VenafiCloudMode && flags.CredentialsPath != "":
366369
mode = VenafiCloudKeypair
367-
log.Printf("Using the %s auth mode since --venafi-cloud and --credentials-path were specified.", mode)
370+
reason = fmt.Sprintf("Using the %s auth mode since --venafi-cloud and --credentials-path were specified.", mode)
368371
case flags.ClientID != "" && flags.PrivateKeyPath != "":
369372
mode = VenafiCloudKeypair
370-
log.Printf("Using the %s auth mode since --client-id and --private-key-path were specified.", mode)
373+
reason = fmt.Sprintf("Using the %s auth mode since --client-id and --private-key-path were specified.", mode)
371374
case flags.ClientID != "":
372375
return CombinedConfig{}, nil, fmt.Errorf("if --client-id is specified, --private-key-path must also be specified")
373376
case flags.PrivateKeyPath != "":
374377
return CombinedConfig{}, nil, fmt.Errorf("--private-key-path is specified, --client-id must also be specified")
375378
case flags.VenConnName != "":
376379
mode = VenafiCloudVenafiConnection
377-
log.Printf("Using the %s auth mode since --venafi-connection was specified.", mode)
380+
reason = fmt.Sprintf("Using the %s auth mode since --venafi-connection was specified.", mode)
378381
case flags.APIToken != "":
379382
mode = JetstackSecureAPIToken
380-
log.Printf("Using the %s auth mode since --api-token was specified.", mode)
383+
reason = fmt.Sprintf("Using the %s auth mode since --api-token was specified.", mode)
381384
case !flags.VenafiCloudMode && flags.CredentialsPath != "":
382385
mode = JetstackSecureOAuth
383-
log.Printf("Using the %s auth mode since --credentials-file was specified without --venafi-cloud.", mode)
386+
reason = fmt.Sprintf("Using the %s auth mode since --credentials-file was specified without --venafi-cloud.", mode)
384387
default:
385388
return CombinedConfig{}, nil, fmt.Errorf("no auth mode specified. You can use one of four auth modes:\n" +
386389
" - Use (--venafi-cloud with --credentials-file) or (--client-id with --private-key-path) to use the " + string(VenafiCloudKeypair) + " mode.\n" +
@@ -389,6 +392,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
389392
" - Use --api-token if you want to use the " + string(JetstackSecureAPIToken) + " mode.\n")
390393
}
391394
res.AuthMode = mode
395+
log.Info(reason)
392396
}
393397

394398
// Validation and defaulting of `server` and the deprecated `endpoint.path`.
@@ -403,10 +407,10 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
403407
case hasServerField && hasEndpointField:
404408
// The `server` field takes precedence over the deprecated
405409
// `endpoint` field.
406-
log.Printf("The `server` and `endpoint` fields are both set in the config; using the `server` field.")
410+
log.Info("The `server` and `endpoint` fields are both set in the config; using the `server` field.")
407411
server = cfg.Server
408412
case !hasServerField && hasEndpointField:
409-
log.Printf("Using deprecated Endpoint configuration. User Server instead.")
413+
log.Info("Using deprecated Endpoint configuration. User Server instead.")
410414
if cfg.Endpoint.Protocol == "" && cfg.Server == "" {
411415
cfg.Endpoint.Protocol = "http"
412416
}
@@ -424,7 +428,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
424428
errs = multierror.Append(errs, fmt.Errorf("server %q is not a valid URL", server))
425429
}
426430
if res.AuthMode == VenafiCloudVenafiConnection && server != "" {
427-
log.Printf("ignoring the server field specified in the config file. In %s mode, this field is not needed.", VenafiCloudVenafiConnection)
431+
log.Info(fmt.Sprintf("ignoring the server field specified in the config file. In %s mode, this field is not needed.", VenafiCloudVenafiConnection))
428432
server = ""
429433
}
430434
res.Server = server
@@ -454,7 +458,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
454458
// change this value with the new --venafi-connection flag, and this
455459
// field is simply ignored.
456460
if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploadPath != "" {
457-
log.Printf(`ignoring the venafi-cloud.upload_path field in the config file. In %s mode, this field is not needed.`, res.AuthMode)
461+
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))
458462
}
459463
uploadPath = ""
460464
}
@@ -472,7 +476,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
472476
// https://venafi.atlassian.net/browse/VC-35385 is done.
473477
{
474478
if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploaderID != "" {
475-
log.Printf(`ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in %s mode.`, res.AuthMode)
479+
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))
476480
}
477481
}
478482

@@ -524,13 +528,13 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
524528
case flags.Period == 0 && cfg.Period == 0:
525529
errs = multierror.Append(errs, fmt.Errorf("period must be set using --period or -p, or using the 'period' field in the config file"))
526530
case flags.Period == 0 && cfg.Period > 0:
527-
log.Printf("Using period from config %s", cfg.Period)
531+
log.Info("Using period from config", "period", cfg.Period)
528532
period = cfg.Period
529533
case flags.Period > 0 && cfg.Period == 0:
530534
period = flags.Period
531535
case flags.Period > 0 && cfg.Period > 0:
532536
// The flag takes precedence.
533-
log.Printf("Both the 'period' field and --period are set. Using the value provided with --period.")
537+
log.Info("Both the 'period' field and --period are set. Using the value provided with --period.")
534538
period = flags.Period
535539
}
536540
res.Period = period
@@ -599,7 +603,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
599603
// The error returned may be a multierror.Error. Use multierror.Prefix(err,
600604
// "context:") rather than fmt.Errorf("context: %w", err) when wrapping the
601605
// error.
602-
func validateCredsAndCreateClient(log *log.Logger, flagCredentialsPath, flagClientID, flagPrivateKeyPath, flagAPIToken string, cfg CombinedConfig) (client.Client, error) {
606+
func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClientID, flagPrivateKeyPath, flagAPIToken string, cfg CombinedConfig) (client.Client, error) {
603607
var errs error
604608

605609
var preflightClient client.Client
@@ -719,7 +723,7 @@ func ValidateDataGatherers(dataGatherers []DataGatherer) error {
719723

720724
// The error returned may be a multierror.Error. Instead of adding context to
721725
// the error with fmt.Errorf("%w", err), use multierror.Prefix(err, "context").
722-
func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg CombinedConfig, agentMetadata *api.AgentMetadata) (client.Client, error) {
726+
func createCredentialClient(log logr.Logger, credentials client.Credentials, cfg CombinedConfig, agentMetadata *api.AgentMetadata) (client.Client, error) {
723727
switch creds := credentials.(type) {
724728
case *client.VenafiSvcAccountCredentials:
725729
// The uploader ID isn't actually used in the backend, let's use an
@@ -730,7 +734,7 @@ func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg
730734
if cfg.AuthMode == VenafiCloudKeypair {
731735
// We don't do this for the VenafiCloudVenafiConnection mode because
732736
// the upload_path field is ignored in that mode.
733-
log.Println("Loading upload_path from \"venafi-cloud\" configuration.")
737+
log.Info("Loading upload_path from \"venafi-cloud\" configuration.")
734738
uploadPath = cfg.UploadPath
735739
}
736740
return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression)

‎pkg/agent/config_test.go‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@ import (
66
"context"
77
"fmt"
88
"io"
9-
"log"
109
"net/http"
1110
"os"
1211
"testing"
1312
"time"
1413

14+
"github.com/go-logr/logr"
1515
"github.com/spf13/cobra"
1616
"github.com/stretchr/testify/assert"
1717
"github.com/stretchr/testify/require"
18+
"k8s.io/klog/v2/ktesting"
1819

1920
"github.com/jetstack/preflight/pkg/client"
2021
"github.com/jetstack/preflight/pkg/testutil"
@@ -86,7 +87,7 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
8687

8788
t.Run("--period flag takes precedence over period field in config, shows warning", func(t *testing.T) {
8889
t.Setenv("POD_NAMESPACE", "venafi")
89-
log, gotLogs := recordLogs()
90+
log, gotLogs := recordLogs(t)
9091
got, _, err := ValidateAndCombineConfig(log,
9192
withConfig(testutil.Undent(`
9293
server: https://api.venafi.eu
@@ -97,8 +98,8 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
9798
withCmdLineFlags("--period", "99m", "--credentials-file", fakeCredsPath))
9899
require.NoError(t, err)
99100
assert.Equal(t, testutil.Undent(`
100-
Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
101-
Both the 'period' field and --period are set. Using the value provided with --period.
101+
INFO Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
102+
INFO Both the 'period' field and --period are set. Using the value provided with --period.
102103
`), gotLogs.String())
103104
assert.Equal(t, 99*time.Minute, got.Period)
104105
})
@@ -573,7 +574,7 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
573574
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) {
574575
t.Setenv("POD_NAMESPACE", "venafi")
575576
t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
576-
log, gotLogs := recordLogs()
577+
log, gotLogs := recordLogs(t)
577578
got, gotCl, err := ValidateAndCombineConfig(log,
578579
withConfig(testutil.Undent(`
579580
server: https://api.venafi.eu
@@ -587,11 +588,11 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
587588
)
588589
require.NoError(t, err)
589590
assert.Equal(t, testutil.Undent(`
590-
Using the Venafi Cloud VenafiConnection auth mode since --venafi-connection was specified.
591-
ignoring the server field specified in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
592-
ignoring the venafi-cloud.upload_path field in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
593-
ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in Venafi Cloud VenafiConnection mode.
594-
Using period from config 1h0m0s
591+
INFO Using the Venafi Cloud VenafiConnection auth mode since --venafi-connection was specified.
592+
INFO ignoring the server field specified in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
593+
INFO ignoring the venafi-cloud.upload_path field in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
594+
INFO ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in Venafi Cloud VenafiConnection mode.
595+
INFO Using period from config period="1h0m0s"
595596
`), gotLogs.String())
596597
assert.Equal(t, VenafiCloudVenafiConnection, got.AuthMode)
597598
assert.IsType(t, &client.VenConnClient{}, gotCl)
@@ -994,13 +995,15 @@ func withFile(t testing.TB, content string) string {
994995
return f.Name()
995996
}
996997

997-
func recordLogs() (*log.Logger, *bytes.Buffer) {
998-
b := bytes.Buffer{}
999-
return log.New(&b, "", 0), &b
998+
func recordLogs(t *testing.T) (logr.Logger, ktesting.Buffer) {
999+
log := ktesting.NewLogger(t, ktesting.NewConfig(ktesting.BufferLogs(true)))
1000+
testingLogger, ok := log.GetSink().(ktesting.Underlier)
1001+
require.True(t, ok)
1002+
return log, testingLogger.GetBuffer()
10001003
}
10011004

1002-
func discardLogs() *log.Logger {
1003-
return log.New(io.Discard, "", 0)
1005+
func discardLogs() logr.Logger {
1006+
return logr.Discard()
10041007
}
10051008

10061009
// Shortcut for ParseConfig.

0 commit comments

Comments
 (0)