use same logging flags for every cli and simplify flag logic

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

cmd/acmesolver/app/app.go

@@ -18,9 +18,11 @@ package app
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/spf13/cobra"
+	"k8s.io/component-base/logs"
 
 	"github.com/cert-manager/cert-manager/internal/cmd/util"
 	"github.com/cert-manager/cert-manager/pkg/issuer/acme/http/solver"
@@ -29,12 +31,18 @@ import (
 
 func NewACMESolverCommand(stopCh <-chan struct{}) *cobra.Command {
 	s := new(solver.HTTP01Solver)
+	logOptions := logs.NewOptions()
 
 	cmd := &cobra.Command{
 		Use:   "acmesolver",
 		Short: "HTTP server used to solve ACME challenges.",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			rootCtx := util.ContextWithStopCh(context.Background(), stopCh)
+
+			if err := logf.ValidateAndApply(logOptions); err != nil {
+				return fmt.Errorf("error validating options: %s", err)
+			}
+
 			rootCtx = logf.NewContext(rootCtx, logf.Log, "acmesolver")
 			log := logf.FromContext(rootCtx)
 
@@ -66,5 +74,7 @@ func NewACMESolverCommand(stopCh <-chan struct{}) *cobra.Command {
 	cmd.Flags().StringVar(&s.Token, "token", "", "the challenge token to verify against")
 	cmd.Flags().StringVar(&s.Key, "key", "", "the challenge key to respond with")
 
+	logf.AddFlags(logOptions, cmd.Flags())
+
 	return cmd
 }

cmd/acmesolver/main.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/cert-manager/cert-manager/acmesolver-binary/app"
 	"github.com/cert-manager/cert-manager/internal/cmd/util"
+	logf "github.com/cert-manager/cert-manager/pkg/logs"
 )
 
 // acmesolver solves ACME http-01 challenges. This is intended to run as a pod
@@ -32,6 +33,9 @@ func main() {
 	stopCh, exit := util.SetupExitHandler(util.GracefulShutdown)
 	defer exit() // This function might call os.Exit, so defer last
 
+	logf.InitLogs()
+	defer logf.FlushLogs()
+
 	cmd := app.NewACMESolverCommand(stopCh)
 
 	if err := cmd.Execute(); err != nil {

cmd/cainjector/app/start.go

@@ -18,6 +18,7 @@ package app
 
 import (
 	"context"
+	"flag"
 	"fmt"
 	"io"
 	"net"
@@ -35,10 +36,9 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"k8s.io/client-go/tools/leaderelection/resourcelock"
 	"k8s.io/component-base/logs"
-	logsapi "k8s.io/component-base/logs/api/v1"
-	_ "k8s.io/component-base/logs/json/register"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
 	cmdutil "github.com/cert-manager/cert-manager/internal/cmd/util"
 	"github.com/cert-manager/cert-manager/pkg/api"
@@ -132,16 +132,16 @@ func (o *InjectorControllerOptions) AddFlags(fs *pflag.FlagSet) {
 
 	utilfeature.DefaultMutableFeatureGate.AddFlag(fs)
 
-	logsapi.AddFlags(o.Logging, fs)
-}
+	logf.AddFlags(o.Logging, fs)
 
-func (o *InjectorControllerOptions) Validate() error {
-	err := logsapi.ValidateAndApply(o.Logging, nil)
-	if err != nil {
-		return err
+	{
+		var controllerRuntimeFlags flag.FlagSet
+		config.RegisterFlags(&controllerRuntimeFlags)
+
+		controllerRuntimeFlags.VisitAll(func(f *flag.Flag) {
+			fs.AddGoFlag(f)
+		})
 	}
-
-	return nil
 }
 
 // NewInjectorControllerOptions returns a new InjectorControllerOptions
@@ -174,7 +174,7 @@ servers and webhook servers.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			o.log = logf.Log.WithName("cainjector")
 
-			if err := o.Validate(); err != nil {
+			if err := logf.ValidateAndApply(o.Logging); err != nil {
 				return fmt.Errorf("error validating options: %s", err)
 			}
 

cmd/cainjector/main.go

@@ -18,7 +18,6 @@ package main
 
 import (
 	"context"
-	"flag"
 
 	"os"
 
@@ -35,16 +34,14 @@ func main() {
 	stopCh, exit := util.SetupExitHandler(util.GracefulShutdown)
 	defer exit() // This function might call os.Exit, so defer last
 
-	logf.InitLogs(flag.CommandLine)
+	logf.InitLogs()
 	defer logf.FlushLogs()
 	ctrl.SetLogger(logf.Log)
 
 	ctx := util.ContextWithStopCh(context.Background(), stopCh)
 
 	cmd := app.NewCommandStartInjectorController(ctx, os.Stdout, os.Stderr)
-	cmd.Flags().AddGoFlagSet(flag.CommandLine)
 
-	flag.CommandLine.Parse([]string{})
 	if err := cmd.Execute(); err != nil {
 		cmd.PrintErrln(err)
 		util.SetExitCode(err)

cmd/controller/app/options/options.go

@@ -26,9 +26,6 @@ import (
 	"github.com/spf13/pflag"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/component-base/logs"
-	logsapi "k8s.io/component-base/logs/api/v1"
-
-	_ "k8s.io/component-base/logs/json/register"
 
 	cmdutil "github.com/cert-manager/cert-manager/internal/cmd/util"
 	"github.com/cert-manager/cert-manager/internal/controller/feature"
@@ -417,7 +414,7 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) {
 		"Leader election healthz checks within this timeout period after the lease expires will still return healthy")
 	fs.MarkHidden("internal-healthz-leader-election-timeout")
 
-	logsapi.AddFlags(s.Logging, fs)
+	logf.AddFlags(s.Logging, fs)
 }
 
 func (o *ControllerOptions) Validate() error {
@@ -458,7 +455,7 @@ func (o *ControllerOptions) Validate() error {
 		}
 	}
 
-	err := logsapi.ValidateAndApply(o.Logging, nil)
+	err := logf.ValidateAndApply(o.Logging)
 	if err != nil {
 		errs = append(errs, err)
 	}

cmd/controller/main.go

@@ -17,8 +17,6 @@ limitations under the License.
 package main
 
 import (
-	"flag"
-
 	"github.com/cert-manager/cert-manager/controller-binary/app"
 	"github.com/cert-manager/cert-manager/internal/cmd/util"
 	logf "github.com/cert-manager/cert-manager/pkg/logs"
@@ -28,13 +26,11 @@ func main() {
 	stopCh, exit := util.SetupExitHandler(util.GracefulShutdown)
 	defer exit() // This function might call os.Exit, so defer last
 
-	logf.InitLogs(flag.CommandLine)
+	logf.InitLogs()
 	defer logf.FlushLogs()
 
 	cmd := app.NewCommandStartCertManagerController(stopCh)
-	cmd.Flags().AddGoFlagSet(flag.CommandLine)
 
-	flag.CommandLine.Parse([]string{})
 	if err := cmd.Execute(); err != nil {
 		logf.Log.Error(err, "error while executing")
 		util.SetExitCode(err)

cmd/webhook/app/options/globalflags.go

@@ -1,36 +0,0 @@
-/*
-Copyright 2021 The cert-manager Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package options
-
-import (
-	"flag"
-	"os"
-
-	"github.com/spf13/pflag"
-
-	"github.com/cert-manager/cert-manager/pkg/logs"
-)
-
-func AddGlobalFlags(fs *pflag.FlagSet) {
-	addKlogFlags(fs)
-}
-
-func addKlogFlags(fs *pflag.FlagSet) {
-	local := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
-	logs.InitLogs(local)
-	fs.AddGoFlagSet(local)
-}

cmd/webhook/app/options/options.go

@@ -22,12 +22,11 @@ import (
 	"github.com/spf13/pflag"
 	cliflag "k8s.io/component-base/cli/flag"
 	"k8s.io/component-base/logs"
-	logsapi "k8s.io/component-base/logs/api/v1"
-	_ "k8s.io/component-base/logs/json/register"
 
 	config "github.com/cert-manager/cert-manager/internal/apis/config/webhook"
 	configscheme "github.com/cert-manager/cert-manager/internal/apis/config/webhook/scheme"
 	configv1alpha1 "github.com/cert-manager/cert-manager/pkg/apis/config/webhook/v1alpha1"
+	logf "github.com/cert-manager/cert-manager/pkg/logs"
 	utilfeature "github.com/cert-manager/cert-manager/pkg/util/feature"
 )
 
@@ -47,16 +46,7 @@ func NewWebhookFlags() *WebhookFlags {
 
 func (f *WebhookFlags) AddFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&f.Config, "config", "", "Path to a file containing a WebhookConfiguration object used to configure the webhook")
-	logsapi.AddFlags(f.Logging, fs)
-}
-
-func ValidateWebhookFlags(f *WebhookFlags) error {
-	err := logsapi.ValidateAndApply(f.Logging, nil)
-	if err != nil {
-		return err
-	}
-
-	return nil
+	logf.AddFlags(f.Logging, fs)
 }
 
 func NewWebhookConfiguration() (*config.WebhookConfiguration, error) {

cmd/webhook/app/webhook.go

@@ -94,7 +94,7 @@ func NewServerCommand(stopCh <-chan struct{}) *cobra.Command {
 				os.Exit(1)
 			}
 
-			if err := options.ValidateWebhookFlags(webhookFlags); err != nil {
+			if err := logf.ValidateAndApply(webhookFlags.Logging); err != nil {
 				log.Error(err, "Failed to validate webhook flags")
 				os.Exit(1)
 			}
@@ -110,6 +110,7 @@ func NewServerCommand(stopCh <-chan struct{}) *cobra.Command {
 					log.Error(err, "Failed to merge flags with config file values")
 					os.Exit(1)
 				}
+
 				// update feature gates based on new config
 				if err := utilfeature.DefaultMutableFeatureGate.SetFromMap(webhookConfig.FeatureGates); err != nil {
 					log.Error(err, "Failed to set feature gates from config file")
@@ -132,7 +133,6 @@ func NewServerCommand(stopCh <-chan struct{}) *cobra.Command {
 
 	webhookFlags.AddFlags(cleanFlagSet)
 	options.AddConfigFlags(cleanFlagSet, webhookConfig)
-	options.AddGlobalFlags(cleanFlagSet)
 
 	cleanFlagSet.BoolP("help", "h", false, fmt.Sprintf("help for %s", cmd.Name()))
 
@@ -149,26 +149,15 @@ func NewServerCommand(stopCh <-chan struct{}) *cobra.Command {
 	return cmd
 }
 
-// newFlagSetWithGlobals constructs a new pflag.FlagSet with global flags registered
-// on it.
-func newFlagSetWithGlobals() *pflag.FlagSet {
-	fs := pflag.NewFlagSet("", pflag.ExitOnError)
-	// set the normalize func, similar to k8s.io/component-base/cli//flags.go:InitFlags
-	fs.SetNormalizeFunc(cliflag.WordSepNormalizeFunc)
-	// explicitly add flags from libs that register global flags
-	options.AddGlobalFlags(fs)
-	return fs
-}
-
 // newFakeFlagSet constructs a pflag.FlagSet with the same flags as fs, but where
 // all values have noop Set implementations
-func newFakeFlagSet(fs *pflag.FlagSet) *pflag.FlagSet {
-	ret := pflag.NewFlagSet("", pflag.ExitOnError)
-	ret.SetNormalizeFunc(fs.GetNormalizeFunc())
-	fs.VisitAll(func(f *pflag.Flag) {
-		ret.VarP(cliflag.NoOp{}, f.Name, f.Shorthand, f.Usage)
-	})
-	return ret
+func newFakeFlagSet() *pflag.FlagSet {
+	fs := pflag.NewFlagSet("", pflag.ExitOnError)
+
+	// set the normalize func, similar to k8s.io/component-base/cli//flags.go:InitFlags
+	fs.SetNormalizeFunc(cliflag.WordSepNormalizeFunc)
+
+	return fs
 }
 
 // webhookConfigFlagPrecedence re-parses flags over the WebhookConfiguration object.
@@ -176,17 +165,21 @@ func newFakeFlagSet(fs *pflag.FlagSet) *pflag.FlagSet {
 // This is necessary to preserve backwards-compatibility across binary upgrades.
 // See issue #56171 for more details.
 func webhookConfigFlagPrecedence(cfg *config.WebhookConfiguration, args []string) error {
-	// We use a throwaway webhookFlags and a fake global flagset to avoid double-parses,
+	// We use a throwaway webhookFlags and a fake flagset to avoid double-parses,
 	// as some Set implementations accumulate values from multiple flag invocations.
-	fs := newFakeFlagSet(newFlagSetWithGlobals())
+	fs := newFakeFlagSet()
+
 	// register throwaway KubeletFlags
 	options.NewWebhookFlags().AddFlags(fs)
+
 	// register new WebhookConfiguration
 	options.AddConfigFlags(fs, cfg)
+
 	// re-parse flags
 	if err := fs.Parse(args); err != nil {
 		return err
 	}
+
 	return nil
 }
 

cmd/webhook/main.go

@@ -17,8 +17,6 @@ limitations under the License.
 package main
 
 import (
-	"flag"
-
 	"github.com/cert-manager/cert-manager/internal/cmd/util"
 	logf "github.com/cert-manager/cert-manager/pkg/logs"
 	"github.com/cert-manager/cert-manager/webhook-binary/app"
@@ -28,13 +26,11 @@ func main() {
 	stopCh, exit := util.SetupExitHandler(util.GracefulShutdown)
 	defer exit() // This function might call os.Exit, so defer last
 
-	logf.InitLogs(flag.CommandLine)
+	logf.InitLogs()
 	defer logf.FlushLogs()
 
 	cmd := app.NewServerCommand(stopCh)
-	cmd.Flags().AddGoFlagSet(flag.CommandLine)
 
-	flag.CommandLine.Parse([]string{})
 	if err := cmd.Execute(); err != nil {
 		logf.Log.Error(err, "error executing command")
 		util.SetExitCode(err)

pkg/controller/test/context_builder.go

@@ -53,7 +53,7 @@ import (
 )
 
 func init() {
-	logs.InitLogs(nil)
+	logs.InitLogs()
 	_ = flag.Set("alsologtostderr", fmt.Sprintf("%t", true))
 	ktesting.DefaultConfig = ktesting.NewConfig(ktesting.Verbosity(4))
 }

pkg/logs/logs.go

@@ -23,10 +23,13 @@ import (
 	"log"
 
 	"github.com/go-logr/logr"
+	"github.com/spf13/pflag"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/component-base/logs"
+	logsapi "k8s.io/component-base/logs/api/v1"
+	_ "k8s.io/component-base/logs/json/register"
 	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/klogr"
 
@@ -58,19 +61,46 @@ func (writer GlogWriter) Write(data []byte) (n int, err error) {
 }
 
 // InitLogs initializes logs the way we want for kubernetes.
-func InitLogs(fs *flag.FlagSet) {
+func InitLogs() {
 	logs.InitLogs()
 
-	if fs == nil {
-		fs = flag.CommandLine
-	}
-	initDeprecatedFlags(fs)
-	_ = fs.Set("logtostderr", "true")
-
 	log.SetOutput(GlogWriter{})
 	log.SetFlags(0)
 }
 
+func AddFlags(opts *logs.Options, fs *pflag.FlagSet) {
+	// init deprecated flags
+	{
+		var allFlags flag.FlagSet
+		klog.InitFlags(&allFlags)
+
+		allFlags.VisitAll(func(f *flag.Flag) {
+			switch f.Name {
+			case "add_dir_header", "alsologtostderr", "log_backtrace_at", "log_dir", "log_file", "log_file_max_size",
+				"logtostderr", "one_output", "skip_headers", "skip_log_headers", "stderrthreshold":
+				fs.AddGoFlag(f)
+			}
+		})
+	}
+	_ = fs.Set("logtostderr", "true")
+
+	{
+		var allFlags pflag.FlagSet
+		logsapi.AddFlags(opts, &allFlags)
+
+		allFlags.VisitAll(func(f *pflag.Flag) {
+			switch f.Name {
+			case "logging-format", "log-flush-frequency", "v", "vmodule":
+				fs.AddFlag(f)
+			}
+		})
+	}
+}
+
+func ValidateAndApply(opts *logs.Options) error {
+	return logsapi.ValidateAndApply(opts, nil)
+}
+
 // FlushLogs flushes logs immediately.
 func FlushLogs() {
 	logs.FlushLogs()
@@ -132,8 +162,6 @@ func WithRelatedResourceName(l logr.Logger, name, namespace, kind string) logr.L
 	)
 }
 
-var contextKey = &struct{}{}
-
 func FromContext(ctx context.Context, names ...string) logr.Logger {
 	l, err := logr.FromContext(ctx)
 	if err != nil {
@@ -172,15 +200,3 @@ func WithInfof(l logr.Logger) *LogWithFormat {
 func (l *LogWithFormat) Infof(format string, a ...interface{}) {
 	l.Info(fmt.Sprintf(format, a...))
 }
-
-func initDeprecatedFlags(fs *flag.FlagSet) {
-	var allFlags flag.FlagSet
-	klog.InitFlags(&allFlags)
-	allFlags.VisitAll(func(f *flag.Flag) {
-		switch f.Name {
-		case "add_dir_header", "alsologtostderr", "log_backtrace_at", "log_dir", "log_file", "log_file_max_size",
-			"logtostderr", "one_output", "skip_headers", "skip_log_headers", "stderrthreshold":
-			fs.Var(f.Value, f.Name, f.Usage)
-		}
-	})
-}

test/e2e/e2e_test.go

@@ -32,7 +32,7 @@ import (
 )
 
 func init() {
-	logs.InitLogs(flag.CommandLine)
+	logs.InitLogs()
 	cfg.AddFlags(flag.CommandLine)
 
 	wait.ForeverTestTimeout = time.Second * 60