From 341f6e42673fda908a85ec6bb9e2cda79dff203a Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Mon, 31 May 2021 22:18:45 +0200 Subject: [PATCH] Refactor logs options --- .../src/k8s.io/component-base/logs/config.go | 94 ++++++++++++++++++ .../src/k8s.io/component-base/logs/options.go | 99 +------------------ .../component-base/logs/options_test.go | 4 +- .../k8s.io/component-base/logs/registry.go | 12 --- .../k8s.io/component-base/logs/validate.go | 65 ++++++++++++ 5 files changed, 166 insertions(+), 108 deletions(-) create mode 100644 staging/src/k8s.io/component-base/logs/config.go create mode 100644 staging/src/k8s.io/component-base/logs/validate.go diff --git a/staging/src/k8s.io/component-base/logs/config.go b/staging/src/k8s.io/component-base/logs/config.go new file mode 100644 index 00000000000..ee05d823881 --- /dev/null +++ b/staging/src/k8s.io/component-base/logs/config.go @@ -0,0 +1,94 @@ +/* +Copyright 2021 The Kubernetes 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 logs + +import ( + "flag" + "fmt" + "strings" + + "github.com/spf13/pflag" + "k8s.io/klog/v2" + + json "k8s.io/component-base/logs/json" +) + +// Supported klog formats +const ( + DefaultLogFormat = "text" + JSONLogFormat = "json" +) + +// LogRegistry is new init LogFormatRegistry struct +var LogRegistry = NewLogFormatRegistry() + +func init() { + // Text format is default klog format + LogRegistry.Register(DefaultLogFormat, nil) + LogRegistry.Register(JSONLogFormat, json.JSONLogger) +} + +// List of logs (k8s.io/klog + k8s.io/component-base/logs) flags supported by all logging formats +var supportedLogsFlags = map[string]struct{}{ + "v": {}, + // TODO: support vmodule after 1.19 Alpha +} + +// BindLoggingFlags binds the Options struct fields to a flagset +func BindLoggingFlags(o *Options, fs *pflag.FlagSet) { + normalizeFunc := func(name string) string { + f := fs.GetNormalizeFunc() + return string(f(fs, name)) + } + + unsupportedFlags := fmt.Sprintf("--%s", strings.Join(UnsupportedLoggingFlags(normalizeFunc), ", --")) + formats := fmt.Sprintf(`"%s"`, strings.Join(LogRegistry.List(), `", "`)) + fs.StringVar(&o.LogFormat, "logging-format", DefaultLogFormat, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags)) + + // No new log formats should be added after generation is of flag options + LogRegistry.Freeze() + fs.BoolVar(&o.LogSanitization, "experimental-logging-sanitization", o.LogSanitization, `[Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). +Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production.`) +} + +// UnsupportedLoggingFlags lists unsupported logging flags +func UnsupportedLoggingFlags(normalizeFunc func(name string) string) []string { + allFlags := []string{} + + // k8s.io/klog flags + fs := &flag.FlagSet{} + klog.InitFlags(fs) + fs.VisitAll(func(flag *flag.Flag) { + if _, found := supportedLogsFlags[flag.Name]; !found { + name := flag.Name + if normalizeFunc != nil { + name = normalizeFunc(name) + } + allFlags = append(allFlags, name) + } + }) + + // k8s.io/component-base/logs flags + pfs := &pflag.FlagSet{} + AddFlags(pfs) + pfs.VisitAll(func(flag *pflag.Flag) { + if _, found := supportedLogsFlags[flag.Name]; !found { + allFlags = append(allFlags, flag.Name) + } + }) + return allFlags +} diff --git a/staging/src/k8s.io/component-base/logs/options.go b/staging/src/k8s.io/component-base/logs/options.go index 1b53a6b2cde..be0420ca194 100644 --- a/staging/src/k8s.io/component-base/logs/options.go +++ b/staging/src/k8s.io/component-base/logs/options.go @@ -17,28 +17,14 @@ limitations under the License. package logs import ( - "flag" - "fmt" - "strings" - "github.com/go-logr/logr" "github.com/spf13/pflag" - "k8s.io/component-base/logs/sanitization" "k8s.io/klog/v2" -) -const ( - logFormatFlagName = "logging-format" - defaultLogFormat = "text" + "k8s.io/component-base/logs/sanitization" ) -// List of logs (k8s.io/klog + k8s.io/component-base/logs) flags supported by all logging formats -var supportedLogsFlags = map[string]struct{}{ - "v": {}, - // TODO: support vmodule after 1.19 Alpha -} - // Options has klog format parameters type Options struct { LogFormat string @@ -48,67 +34,19 @@ type Options struct { // NewOptions return new klog options func NewOptions() *Options { return &Options{ - LogFormat: defaultLogFormat, + LogFormat: DefaultLogFormat, } } // Validate verifies if any unsupported flag is set // for non-default logging format func (o *Options) Validate() []error { - errs := []error{} - if o.LogFormat != defaultLogFormat { - allFlags := unsupportedLoggingFlags(hyphensToUnderscores) - for _, fname := range allFlags { - if flagIsSet(fname, hyphensToUnderscores) { - errs = append(errs, fmt.Errorf("non-default logging format doesn't honor flag: %s", fname)) - } - } - } - if _, err := o.Get(); err != nil { - errs = append(errs, fmt.Errorf("unsupported log format: %s", o.LogFormat)) - } - return errs -} - -// hyphensToUnderscores replaces hyphens with underscores -// we should always use underscores instead of hyphens when validate flags -func hyphensToUnderscores(s string) string { - return strings.Replace(s, "-", "_", -1) -} - -func flagIsSet(name string, normalizeFunc func(name string) string) bool { - f := flag.Lookup(name) - if f != nil { - return f.DefValue != f.Value.String() - } - if normalizeFunc != nil { - f = flag.Lookup(normalizeFunc(name)) - if f != nil { - return f.DefValue != f.Value.String() - } - } - pf := pflag.Lookup(name) - if pf != nil { - return pf.DefValue != pf.Value.String() - } - panic("failed to lookup unsupported log flag") + return ValidateLoggingConfiguration(o) } // AddFlags add logging-format flag func (o *Options) AddFlags(fs *pflag.FlagSet) { - normalizeFunc := func(name string) string { - f := fs.GetNormalizeFunc() - return string(f(fs, name)) - } - - unsupportedFlags := fmt.Sprintf("--%s", strings.Join(unsupportedLoggingFlags(normalizeFunc), ", --")) - formats := fmt.Sprintf(`"%s"`, strings.Join(logRegistry.List(), `", "`)) - fs.StringVar(&o.LogFormat, logFormatFlagName, defaultLogFormat, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags)) - - // No new log formats should be added after generation is of flag options - logRegistry.Freeze() - fs.BoolVar(&o.LogSanitization, "experimental-logging-sanitization", o.LogSanitization, `[Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). -Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production.`) + BindLoggingFlags(o, fs) } // Apply set klog logger from LogFormat type @@ -123,32 +61,5 @@ func (o *Options) Apply() { // Get logger with LogFormat field func (o *Options) Get() (logr.Logger, error) { - return logRegistry.Get(o.LogFormat) -} - -func unsupportedLoggingFlags(normalizeFunc func(name string) string) []string { - allFlags := []string{} - - // k8s.io/klog flags - fs := &flag.FlagSet{} - klog.InitFlags(fs) - fs.VisitAll(func(flag *flag.Flag) { - if _, found := supportedLogsFlags[flag.Name]; !found { - name := flag.Name - if normalizeFunc != nil { - name = normalizeFunc(name) - } - allFlags = append(allFlags, name) - } - }) - - // k8s.io/component-base/logs flags - pfs := &pflag.FlagSet{} - AddFlags(pfs) - pfs.VisitAll(func(flag *pflag.Flag) { - if _, found := supportedLogsFlags[flag.Name]; !found { - allFlags = append(allFlags, flag.Name) - } - }) - return allFlags + return LogRegistry.Get(o.LogFormat) } diff --git a/staging/src/k8s.io/component-base/logs/options_test.go b/staging/src/k8s.io/component-base/logs/options_test.go index 623a9ca9b8e..18a72d3dfb9 100644 --- a/staging/src/k8s.io/component-base/logs/options_test.go +++ b/staging/src/k8s.io/component-base/logs/options_test.go @@ -63,14 +63,14 @@ func TestOptions(t *testing.T) { name: "JSON log format", args: []string{"--logging-format=json"}, want: &Options{ - LogFormat: jsonLogFormat, + LogFormat: JSONLogFormat, }, }, { name: "log sanitization", args: []string{"--experimental-logging-sanitization"}, want: &Options{ - LogFormat: defaultLogFormat, + LogFormat: DefaultLogFormat, LogSanitization: true, }, }, diff --git a/staging/src/k8s.io/component-base/logs/registry.go b/staging/src/k8s.io/component-base/logs/registry.go index c71899db66d..150af394c27 100644 --- a/staging/src/k8s.io/component-base/logs/registry.go +++ b/staging/src/k8s.io/component-base/logs/registry.go @@ -21,15 +21,8 @@ import ( "sort" "github.com/go-logr/logr" - json "k8s.io/component-base/logs/json" ) -const ( - jsonLogFormat = "json" -) - -var logRegistry = NewLogFormatRegistry() - // LogFormatRegistry store klog format registry type LogFormatRegistry struct { registry map[string]logr.Logger @@ -99,8 +92,3 @@ func (lfr *LogFormatRegistry) List() []string { func (lfr *LogFormatRegistry) Freeze() { lfr.frozen = true } -func init() { - // Text format is default klog format - logRegistry.Register(defaultLogFormat, nil) - logRegistry.Register(jsonLogFormat, json.JSONLogger) -} diff --git a/staging/src/k8s.io/component-base/logs/validate.go b/staging/src/k8s.io/component-base/logs/validate.go new file mode 100644 index 00000000000..0fca18a0b40 --- /dev/null +++ b/staging/src/k8s.io/component-base/logs/validate.go @@ -0,0 +1,65 @@ +/* +Copyright 2021 The Kubernetes 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 logs + +import ( + "flag" + "fmt" + "strings" + + "github.com/spf13/pflag" +) + +func ValidateLoggingConfiguration(o *Options) []error { + errs := []error{} + if o.LogFormat != DefaultLogFormat { + allFlags := UnsupportedLoggingFlags(hyphensToUnderscores) + for _, fname := range allFlags { + if flagIsSet(fname, hyphensToUnderscores) { + errs = append(errs, fmt.Errorf("non-default logging format doesn't honor flag: %s", fname)) + } + } + } + if _, err := o.Get(); err != nil { + errs = append(errs, fmt.Errorf("unsupported log format: %s", o.LogFormat)) + } + return errs +} + +// hyphensToUnderscores replaces hyphens with underscores +// we should always use underscores instead of hyphens when validate flags +func hyphensToUnderscores(s string) string { + return strings.Replace(s, "-", "_", -1) +} + +func flagIsSet(name string, normalizeFunc func(name string) string) bool { + f := flag.Lookup(name) + if f != nil { + return f.DefValue != f.Value.String() + } + if normalizeFunc != nil { + f = flag.Lookup(normalizeFunc(name)) + if f != nil { + return f.DefValue != f.Value.String() + } + } + pf := pflag.Lookup(name) + if pf != nil { + return pf.DefValue != pf.Value.String() + } + panic("failed to lookup unsupported log flag") +}