From 9fd7fc035b952de8fe81a5ec21f801c3d3dc943f Mon Sep 17 00:00:00 2001 From: Krzysztof Ostrowski Date: Tue, 7 May 2024 14:03:41 +0200 Subject: [PATCH 1/2] c/k/a/opts: fix bool,deprecated flags regression --- cmd/kube-rbac-proxy/app/options/deprecated.go | 77 +++++++++++++++++++ cmd/kube-rbac-proxy/app/options/options.go | 34 +------- test/e2e/flags/deployment-logtostderr.yaml | 2 +- 3 files changed, 81 insertions(+), 32 deletions(-) create mode 100644 cmd/kube-rbac-proxy/app/options/deprecated.go diff --git a/cmd/kube-rbac-proxy/app/options/deprecated.go b/cmd/kube-rbac-proxy/app/options/deprecated.go new file mode 100644 index 000000000..987ebbfc1 --- /dev/null +++ b/cmd/kube-rbac-proxy/app/options/deprecated.go @@ -0,0 +1,77 @@ +/* +Copyright 2024 the kube-rbac-proxy maintainers. All rights reserved. + +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 ( + "fmt" + + "github.com/spf13/pflag" + "k8s.io/klog/v2" +) + +var disabledFlagsType = map[string]string{ + "logtostderr": "bool", + "add-dir-header": "bool", + "alsologtostderr": "bool", + "log-backtrace-at": "string", + "log-dir": "string", + "log-file": "string", + "log-file-max-size": "uint64", + "one-output": "bool", + "skip-headers": "bool", + "skip-log-headers": "bool", + "stderrthreshold": "string", +} + +func (o *ProxyRunOptions) addDisabledFlags(flagset *pflag.FlagSet) { + // disabled flags + o.flagSet = flagset // reference used for validation + + for name, typeStr := range disabledFlagsType { + switch typeStr { + case "bool": + _ = flagset.Bool(name, false, "[DISABLED]") + case "string": + _ = flagset.String(name, "", "[DISABLED]") + case "uint64": + _ = flagset.Uint64(name, 0, "[DISABLED]") + default: + panic(fmt.Sprintf("unknown type %q", typeStr)) + } + + if err := flagset.MarkHidden(name); err != nil { + panic(err) + } + } +} + +func (o *ProxyRunOptions) validateDisabledFlags() error { + // Removed upstream flags shouldn't be use + for disabledOpt := range disabledFlagsType { + if flag := o.flagSet.Lookup(disabledOpt); flag.Changed { + klog.Warningf(` +==== Removed Flag Warning ====================== + +%s is removed in the k8s upstream and has no effect any more. + +=============================================== + `, disabledOpt) + } + } + + return nil +} diff --git a/cmd/kube-rbac-proxy/app/options/options.go b/cmd/kube-rbac-proxy/app/options/options.go index 3ef6ed739..6624792c6 100644 --- a/cmd/kube-rbac-proxy/app/options/options.go +++ b/cmd/kube-rbac-proxy/app/options/options.go @@ -54,20 +54,6 @@ type ProxyRunOptions struct { flagSet *pflag.FlagSet } -var disabledFlags = []string{ - "logtostderr", - "add-dir-header", - "alsologtostderr", - "log-backtrace-at", - "log-dir", - "log-file", - "log-file-max-size", - "one-output", - "skip-headers", - "skip-log-headers", - "stderrthreshold", -} - type TLSConfig struct { CertFile string KeyFile string @@ -144,13 +130,7 @@ func (o *ProxyRunOptions) Flags() k8sapiflag.NamedFlagSets { flagset.Uint32Var(&o.HTTP2MaxSize, "http2-max-size", 256*1024, "The maximum number of bytes that the server will accept for frame size and buffer per stream in a HTTP/2 request.") // disabled flags - o.flagSet = flagset // reference used for validation - for _, disabledOpt := range disabledFlags { - _ = flagset.String(disabledOpt, "", "[DISABLED]") - if err := flagset.MarkHidden(disabledOpt); err != nil { - panic(err) - } - } + o.addDisabledFlags(flagset) return namedFlagSets } @@ -210,16 +190,8 @@ For more information, please go to https://github.com/brancz/kube-rbac-proxy/iss } // Removed upstream flags shouldn't be use - for _, disabledOpt := range disabledFlags { - if flag := o.flagSet.Lookup(disabledOpt); flag.Changed { - klog.Warningf(` -==== Removed Flag Warning ====================== - -%s is removed in the k8s upstream and has no effect any more. - -=============================================== - `, disabledOpt) - } + if err := o.validateDisabledFlags(); err != nil { + errs = append(errs, err) } return utilerrors.NewAggregate(errs) diff --git a/test/e2e/flags/deployment-logtostderr.yaml b/test/e2e/flags/deployment-logtostderr.yaml index 086622c1c..489ec7bea 100644 --- a/test/e2e/flags/deployment-logtostderr.yaml +++ b/test/e2e/flags/deployment-logtostderr.yaml @@ -19,8 +19,8 @@ spec: image: quay.io/brancz/kube-rbac-proxy:local args: - "--secure-listen-address=0.0.0.0:8443" + - "--logtostderr" - "--upstream=http://127.0.0.1:8081/" - - "--logtostderr=true" - "--v=10" ports: - containerPort: 8443 From ee50f38b55571f6360891b537e6ad186214018f8 Mon Sep 17 00:00:00 2001 From: Krzysztof Ostrowski Date: Tue, 7 May 2024 14:04:17 +0200 Subject: [PATCH 2/2] c/k/app: add non-empty listen address check --- cmd/kube-rbac-proxy/app/kube-rbac-proxy.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go b/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go index e0ffb794e..380fc74dd 100644 --- a/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go +++ b/cmd/kube-rbac-proxy/app/kube-rbac-proxy.go @@ -485,6 +485,10 @@ func Run(cfg *completedProxyRunOptions) error { }) } + if len(cfg.secureListenAddress) == 0 && len(cfg.insecureListenAddress) == 0 { + return fmt.Errorf("no listen address provided") + } + if err := gr.Run(); err != nil { return fmt.Errorf("failed to run groups: %w", err) }