Skip to content

Commit

Permalink
Use klog and logr logger instead of log
Browse files Browse the repository at this point in the history 
Signed-off-by: Richard Wall <richard.wall@venafi.com>
  • Loading branch information
@wallrj
wallrj committed Nov 7, 2024
1 parent 3d4876b commit 6b0e405
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 70 deletions.
4 changes: 3 additions & 1 deletion cmd/root.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"context"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -45,8 +46,9 @@ func init() {
// will be logged and the process will exit with status 1.
func Execute() {
logs.AddFlags(rootCmd.PersistentFlags())
ctx := klog.NewContext(context.Background(), klog.Background())
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)
}
Expand Down
40 changes: 22 additions & 18 deletions pkg/agent/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -355,32 +355,35 @@ 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

{
var mode AuthMode
var (
mode AuthMode
reason string
)
switch {
case flags.VenafiCloudMode && flags.CredentialsPath != "":
mode = VenafiCloudKeypair
log.Printf("Using the %s auth mode since --venafi-cloud and --credentials-path were specified.", mode)
reason = 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)
reason = 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)
reason = 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)
reason = 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)
reason = 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" +
Expand All @@ -389,6 +392,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
" - Use --api-token if you want to use the " + string(JetstackSecureAPIToken) + " mode.\n")
}
res.AuthMode = mode
log.Info(reason)
}

// Validation and defaulting of `server` and the deprecated `endpoint.path`.
Expand All @@ -403,10 +407,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"
}
Expand All @@ -424,7 +428,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
Expand Down Expand Up @@ -454,7 +458,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 = ""
}
Expand All @@ -472,7 +476,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))
}
}

Expand Down Expand Up @@ -524,13 +528,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
Expand Down Expand Up @@ -599,7 +603,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
Expand Down Expand Up @@ -719,7 +723,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
Expand All @@ -730,7 +734,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)
Expand Down
33 changes: 18 additions & 15 deletions pkg/agent/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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)
})
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 6b0e405

Please sign in to comment.