Reduce CLI boilerplate

flags.go

@@ -0,0 +1,46 @@
+package main
+
+import (
+	"fmt"
+
+	"github.com/urfave/cli/v2"
+)
+
+func newLogLevelFlag() *cli.IntFlag {
+	return &cli.IntFlag{
+		Name: "log-level", Aliases: []string{"v"}, EnvVars: []string{"LOG_LEVEL"},
+		Usage: "number of the log level verbosity",
+		Value: 0,
+	}
+}
+
+func newLogFormatFlag() *cli.StringFlag {
+	return &cli.StringFlag{
+		Name: "log-format", EnvVars: []string{"LOG_FORMAT"},
+		Usage: "sets the log format (values: [json, console])",
+		Value: "console",
+		Action: func(context *cli.Context, format string) error {
+			if format == "console" || format == "json" {
+				return nil
+			}
+			_ = cli.ShowAppHelp(context)
+			return fmt.Errorf("unknown log format: %s", format)
+		},
+	}
+}
+
+func newLeaderElectionEnabledFlag(dest *bool) *cli.BoolFlag {
+	return &cli.BoolFlag{
+		Name: "leader-election-enabled", Value: false, EnvVars: []string{"LEADER_ELECTION_ENABLED"},
+		Usage:       "Use leader election for the controller manager.",
+		Destination: dest,
+	}
+}
+
+func newWebhookTLSCertDirFlag(dest *string) *cli.StringFlag {
+	return &cli.StringFlag{
+		Name: "webhook-tls-cert-dir", EnvVars: []string{"WEBHOOK_TLS_CERT_DIR"}, // Env var is set by Crossplane
+		Usage:       "Directory containing the certificates for the webhook server. If empty, the webhook server is not started.",
+		Destination: dest,
+	}
+}

logger.go

@@ -1,11 +1,10 @@
 package main
 
 import (
-	"log"
+	"fmt"
 	"os"
 	"runtime"
 	"strings"
-	"sync/atomic"
 
 	"github.com/go-logr/logr"
 	"github.com/go-logr/zapr"
@@ -14,21 +13,17 @@ import (
 	"go.uber.org/zap/zapcore"
 )
 
-type loggerContextKey struct{}
-
-// AppLogger retrieves the application-wide logger instance from the cli.Context.
-func AppLogger(c *cli.Context) logr.Logger {
-	return c.Context.Value(loggerContextKey{}).(*atomic.Value).Load().(logr.Logger)
+func init() {
+	// Remove `-v` short option from --version flag
+	cli.VersionFlag.(*cli.BoolFlag).Aliases = nil
 }
 
 // LogMetadata prints various metadata to the root logger.
 // It prints version, architecture and current user ID and returns nil.
 func LogMetadata(c *cli.Context) error {
-	logger := AppLogger(c)
-	if !usesProductionLoggingConfig(c) {
-		logger = logger.WithValues("version", version)
-	}
-	logger.WithValues(
+	log := logr.FromContextOrDiscard(c.Context)
+	log.WithValues(
+		"version", version,
 		"date", date,
 		"commit", commit,
 		"go_os", runtime.GOOS,
@@ -41,37 +36,33 @@ func LogMetadata(c *cli.Context) error {
 }
 
 func setupLogging(c *cli.Context) error {
-	logger := newZapLogger(appName, c.Int("log-level"), usesProductionLoggingConfig(c))
-	c.Context.Value(loggerContextKey{}).(*atomic.Value).Store(logger)
-	return nil
+	log, err := newZapLogger(appName, c.Int(newLogLevelFlag().Name), usesProductionLoggingConfig(c))
+	c.Context = logr.NewContext(c.Context, log)
+	return err
 }
 
 func usesProductionLoggingConfig(c *cli.Context) bool {
-	return strings.EqualFold("JSON", c.String("log-format"))
+	return strings.EqualFold("JSON", c.String(newLogFormatFlag().Name))
 }
 
-func newZapLogger(name string, verbosityLevel int, useProductionConfig bool) logr.Logger {
+func newZapLogger(name string, verbosityLevel int, useProductionConfig bool) (logr.Logger, error) {
 	cfg := zap.NewDevelopmentConfig()
 	cfg.EncoderConfig.ConsoleSeparator = " | "
 	if useProductionConfig {
 		cfg = zap.NewProductionConfig()
 	}
-	if verbosityLevel > 0 {
-		// Zap's levels get more verbose as the number gets smaller,
-		// bug logr's level increases with greater numbers.
-		cfg.Level = zap.NewAtomicLevelAt(zapcore.Level(verbosityLevel * -1))
-	} else {
-		cfg.Level = zap.NewAtomicLevelAt(zapcore.InfoLevel)
-	}
+	// Zap's levels get more verbose as the number gets smaller,
+	// bug logr's level increases with greater numbers.
+	cfg.Level = zap.NewAtomicLevelAt(zapcore.Level(verbosityLevel * -1))
 	z, err := cfg.Build()
-	zap.ReplaceGlobals(z)
 	if err != nil {
-		log.Fatalf("error configuring the logging stack")
+		return logr.Discard(), fmt.Errorf("error configuring the logging stack: %w", err)
 	}
-	logger := zapr.NewLogger(z).WithName(name)
+	zap.ReplaceGlobals(z)
+	zlog := zapr.NewLogger(z).WithName(name)
 	if useProductionConfig {
 		// Append the version to each log so that logging stacks like EFK/Loki can correlate errors with specific versions.
-		return logger.WithValues("version", version)
+		return zlog.WithValues("version", version), nil
 	}
-	return logger
+	return zlog, nil
 }

main.go

@@ -1,12 +1,9 @@
 package main
 
 import (
-	"context"
 	"fmt"
 	"os"
-	"os/signal"
 	"sync/atomic"
-	"syscall"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -21,88 +18,34 @@ var (
 
 	appName     = "provider-exoscale"
 	appLongName = "Crossplane provider that deploys resources on exoscale.com"
-
-	envPrefix = ""
 )
 
-func init() {
-	// Remove `-v` short option from --version flag
-	cli.VersionFlag.(*cli.BoolFlag).Aliases = nil
-}
-
 func main() {
-	ctx, stop, app := newApp()
-	defer stop()
-	err := app.RunContext(ctx, os.Args)
+	app := newApp()
+	err := app.Run(os.Args)
 	// If required flags aren't set, it will return with error before we could set up logging
 	if err != nil {
 		_, _ = fmt.Fprintf(os.Stderr, "%v\n", err)
 		os.Exit(1)
 	}
 }
 
-func newApp() (context.Context, context.CancelFunc, *cli.App) {
+func newApp() *cli.App {
 	logInstance := &atomic.Value{}
 	logInstance.Store(logr.Discard())
 	app := &cli.App{
 		Name:    appName,
 		Usage:   appLongName,
 		Version: fmt.Sprintf("%s, revision=%s, date=%s", version, commit, date),
 
-		EnableBashCompletion: true,
-
 		Before: setupLogging,
 		Flags: []cli.Flag{
-			&cli.IntFlag{
-				Name: "log-level", Aliases: []string{"v"}, EnvVars: envVars("LOG_LEVEL"),
-				Usage: "number of the log level verbosity",
-				Value: 0,
-			},
-			&cli.StringFlag{
-				Name: "log-format", EnvVars: envVars("LOG_FORMAT"),
-				Usage:       "sets the log format (values: [json, console])",
-				DefaultText: "console",
-			},
+			newLogLevelFlag(),
+			newLogFormatFlag(),
 		},
 		Commands: []*cli.Command{
 			newOperatorCommand(),
 		},
-		ExitErrHandler: func(ctx *cli.Context, err error) {
-			if err != nil {
-				AppLogger(ctx).Error(err, "fatal error")
-				cli.HandleExitCoder(cli.Exit("", 1))
-			}
-		},
-	}
-	hasSubcommands := len(app.Commands) > 0
-	app.Action = rootAction(hasSubcommands)
-	// There is logr.NewContext(...) which returns a context that carries the logger instance.
-	// However, since we are configuring and replacing this logger after starting up and parsing the flags,
-	// we'll store a thread-safe atomic reference.
-	parentCtx := context.WithValue(context.Background(), loggerContextKey{}, logInstance)
-	ctx, stop := signal.NotifyContext(parentCtx, syscall.SIGINT, syscall.SIGTERM)
-	return ctx, stop, app
-}
-
-func rootAction(hasSubcommands bool) func(context *cli.Context) error {
-	return func(ctx *cli.Context) error {
-		if hasSubcommands {
-			return cli.ShowAppHelp(ctx)
-		}
-		return LogMetadata(ctx)
-	}
-}
-
-// env combines envPrefix with given suffix delimited by underscore.
-func env(suffix string) string {
-	return envPrefix + suffix
-}
-
-// envVars combines envPrefix with each given suffix delimited by underscore.
-func envVars(suffixes ...string) []string {
-	arr := make([]string, len(suffixes))
-	for i := range suffixes {
-		arr[i] = env(suffixes[i])
 	}
-	return arr
+	return app
 }

operator/bucketcontroller/webhook_test.go

@@ -1,9 +1,11 @@
 package bucketcontroller
 
 import (
-	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
+	"context"
 	"testing"
 
+	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
+
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -36,7 +38,7 @@ func TestBucketValidator_ValidateCreate_RequireProviderConfig(t *testing.T) {
 				},
 			}
 			v := &BucketValidator{log: logr.Discard()}
-			err := v.ValidateCreate(nil, bucket)
+			err := v.ValidateCreate(context.TODO(), bucket)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -92,7 +94,7 @@ func TestBucketValidator_ValidateUpdate_PreventBucketNameChange(t *testing.T) {
 				},
 			}
 			v := &BucketValidator{log: logr.Discard()}
-			err := v.ValidateUpdate(nil, oldBucket, newBucket)
+			err := v.ValidateUpdate(context.TODO(), oldBucket, newBucket)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -145,7 +147,7 @@ func TestBucketValidator_ValidateUpdate_RequireProviderConfig(t *testing.T) {
 				},
 			}
 			v := &BucketValidator{log: logr.Discard()}
-			err := v.ValidateUpdate(nil, oldBucket, newBucket)
+			err := v.ValidateUpdate(context.TODO(), oldBucket, newBucket)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -190,7 +192,7 @@ func TestBucketValidator_ValidateUpdate_PreventZoneChange(t *testing.T) {
 				Status: exoscalev1.BucketStatus{AtProvider: exoscalev1.BucketObservation{BucketName: "bucket"}},
 			}
 			v := &BucketValidator{log: logr.Discard()}
-			err := v.ValidateUpdate(nil, oldBucket, newBucket)
+			err := v.ValidateUpdate(context.TODO(), oldBucket, newBucket)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {

operator/iamkeycontroller/webhook_test.go

@@ -1,9 +1,11 @@
 package iamkeycontroller
 
 import (
+	"context"
+	"testing"
+
 	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
 	exoscalev1 "github.com/vshn/provider-exoscale/apis/exoscale/v1"
-	"testing"
 
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
@@ -46,7 +48,7 @@ func TestIAMKeyValidator_ValidateCreate_RequireBuckets(t *testing.T) {
 				},
 			}
 			validator := &IAMKeyValidator{log: logr.Discard()}
-			err := validator.ValidateCreate(nil, &iamKey)
+			err := validator.ValidateCreate(context.TODO(), &iamKey)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -106,7 +108,7 @@ func TestIAMKeyValidator_ValidateCreate_RequireWriteConnectionSecretToRef(t *tes
 				},
 			}
 			validator := &IAMKeyValidator{log: logr.Discard()}
-			err := validator.ValidateCreate(nil, &iamKey)
+			err := validator.ValidateCreate(context.TODO(), &iamKey)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -162,7 +164,7 @@ func TestIAMKeyValidator_ValidateCreate_RequireProviderConfigToRef(t *testing.T)
 				},
 			}
 			validator := &IAMKeyValidator{log: logr.Discard()}
-			err := validator.ValidateCreate(nil, &iamKey)
+			err := validator.ValidateCreate(context.TODO(), &iamKey)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -222,7 +224,7 @@ func TestIAMKeyValidator_ValidateUpdate_RequireProviderConfigToRef(t *testing.T)
 				Status: exoscalev1.IAMKeyStatus{ResourceStatus: xpv1.ResourceStatus{}, AtProvider: exoscalev1.IAMKeyObservation{KeyID: "key-id"}},
 			}
 			validator := &IAMKeyValidator{log: logr.Discard()}
-			err := validator.ValidateUpdate(nil, &oldIAMKey, &newIAMKey)
+			err := validator.ValidateUpdate(context.TODO(), &oldIAMKey, &newIAMKey)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -293,7 +295,7 @@ func TestIAMKeyValidator_ValidateUpdate_RequireForProviderImmutable(t *testing.T
 				Status: exoscalev1.IAMKeyStatus{AtProvider: exoscalev1.IAMKeyObservation{KeyID: "key-id"}},
 			}
 			validator := &IAMKeyValidator{log: logr.Discard()}
-			err := validator.ValidateUpdate(nil, &oldIAMKey, &newIAMKey)
+			err := validator.ValidateUpdate(context.TODO(), &oldIAMKey, &newIAMKey)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {
@@ -358,7 +360,7 @@ func TestIAMKeyValidator_ValidateUpdate_RequireConnectionSecretToRefImmutable(t
 				Status: exoscalev1.IAMKeyStatus{AtProvider: exoscalev1.IAMKeyObservation{KeyID: "key-id"}},
 			}
 			validator := &IAMKeyValidator{log: logr.Discard()}
-			err := validator.ValidateUpdate(nil, &oldIAMKey, &newIAMKey)
+			err := validator.ValidateUpdate(context.TODO(), &oldIAMKey, &newIAMKey)
 			if tc.expectedError != "" {
 				assert.EqualError(t, err, tc.expectedError)
 			} else {