Skip to content

Commit

Permalink
cmd/krp/app: validate disabled flags
Browse files Browse the repository at this point in the history
  • Loading branch information
@ibihim
ibihim committed Jan 23, 2024
1 parent e466704 commit f4aa76e
Show file tree
Hide file tree
Showing 6 changed files with 296 additions and 102 deletions.
26 changes: 13 additions & 13 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ Usage:
Kube-rbac-proxy flags:
--add-dir-header [DISABLED]
--allow-paths strings Comma-separated list of paths against which kube-rbac-proxy pattern-matches the incoming request. If the request doesn't match, kube-rbac-proxy responds with a 404 status code. If omitted, the incoming request path isn't checked. Cannot be used with --ignore-paths.
--alsologtostderr [DISABLED]
--auth-header-fields-enabled When set to true, kube-rbac-proxy adds auth-related fields to the headers of http requests sent to the upstream
--auth-header-groups-field-name string The name of the field inside a http(2) request header to tell the upstream server about the user's groups (default "x-remote-groups")
--auth-header-groups-field-separator string The separator string used for concatenating multiple group names in a groups header field's value (default "|")
Expand All @@ -68,15 +70,24 @@ Kube-rbac-proxy flags:
--ignore-paths strings Comma-separated list of paths against which kube-rbac-proxy pattern-matches the incoming request. If the requst matches, it will proxy the request without performing an authentication or authorization check. Cannot be used with --allow-paths.
--insecure-listen-address string [DEPRECATED] The address the kube-rbac-proxy HTTP server should listen on.
--kubeconfig string Path to a kubeconfig file, specifying how to connect to the API server. If unset, in-cluster configuration will be used
--log-backtrace-at int [DISABLED]
--log-dir string [DISABLED]
--log-file string [DISABLED]
--log-file-max-size uint32 [DISABLED] (default 1800)
--logtostderr [DISABLED]
--oidc-ca-file string If set, the OpenID server's certificate will be verified by one of the authorities in the oidc-ca-file, otherwise the host's root CA set will be used.
--oidc-clientID string The client ID for the OpenID Connect client, must be set if oidc-issuer-url is set.
--oidc-groups-claim string Identifier of groups in JWT claim, by default set to 'groups' (default "groups")
--oidc-groups-prefix string If provided, all groups will be prefixed with this value to prevent conflicts with other authentication strategies.
--oidc-issuer string The URL of the OpenID issuer, only HTTPS scheme will be accepted. If set, it will be used to verify the OIDC JSON Web Token (JWT).
--oidc-sign-alg stringArray Supported signing algorithms, default RS256 (default [RS256])
--oidc-username-claim string Identifier of the user in JWT claim, by default set to 'email' (default "email")
--one-output [DISABLED]
--proxy-endpoints-port int The port to securely serve proxy-specific endpoints (such as '/healthz'). Uses the host from the '--secure-listen-address'.
--secure-listen-address string The address the kube-rbac-proxy HTTPs server should listen on.
--skip-headers [DISABLED]
--skip-log-headers [DISABLED]
--stderrthreshold int [DISABLED] (default 2)
--tls-cert-file string File containing the default x509 Certificate for HTTPS. (CA cert, if any, concatenated after server cert)
--tls-cipher-suites strings Comma-separated list of cipher suites for the server. Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). If omitted, the default Go cipher suites will be used
--tls-min-version string Minimum TLS version supported. Value must match version names from https://golang.org/pkg/crypto/tls/#pkg-constants. (default "VersionTLS12")
Expand All @@ -90,19 +101,8 @@ Kube-rbac-proxy flags:
Global flags:
--add-dir-header If true, adds the file directory to the header of the log messages (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--alsologtostderr log to standard error as well as files (no effect when -logtostderr=true) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
-h, --help help for kube-rbac-proxy
--log-backtrace-at traceLocation when logging hits line file:N, emit a stack trace (default :0) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--log-dir string If non-empty, write log files in this directory (no effect when -logtostderr=true) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--log-file string If non-empty, use this log file (no effect when -logtostderr=true) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--log-file-max-size uint Defines the maximum size a log file can grow to (no effect when -logtostderr=true). Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--logtostderr log to standard error instead of files (default true) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--one-output If true, only write logs to their native severity level (vs also writing to each lower severity level; no effect when -logtostderr=true) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--skip-headers If true, avoid header prefixes in the log messages (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--skip-log-headers If true, avoid headers when opening log files (no effect when -logtostderr=true) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--stderrthreshold severity logs at or above this threshold go to stderr when writing to files and stderr (no effect when -logtostderr=true or -alsologtostderr=false) (default 2) (DEPRECATED: will be removed in a future release, see https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/2845-deprecate-klog-specific-flags-in-k8s-components)
--version version[=true] Print version information and quit
-h, --help help for kube-rbac-proxy
--version version[=true] --version, --version=raw prints version information and quits; --version=vX.Y.Z... sets the reported version
```


Expand Down
93 changes: 17 additions & 76 deletions cmd/kube-rbac-proxy/app/kube-rbac-proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
"golang.org/x/net/http2"
"golang.org/x/net/http2/h2c"

utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apiserver/pkg/authentication/authenticator"
"k8s.io/apiserver/pkg/authorization/union"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -84,17 +83,16 @@ that can perform RBAC authorization against the Kubernetes API using SubjectAcce

k8sapiflag.PrintFlags(fs)

if err := o.Validate(); err != nil {
return err
}

// set default options
completedOptions, err := Complete(o)
if err != nil {
return err
}

// validate options
if errs := completedOptions.Validate(); len(errs) != 0 {
return utilerrors.NewAggregate(errs)
}

return Run(completedOptions)
},
Args: func(cmd *cobra.Command, args []string) error {
Expand Down Expand Up @@ -125,7 +123,7 @@ type configfile struct {
AuthorizationConfig *authz.Config `json:"authorization,omitempty"`
}

type completedProxyRunOptions struct {
type proxyConfig struct {
insecureListenAddress string // DEPRECATED
secureListenAddress string
proxyEndpointsPort int
Expand All @@ -146,66 +144,9 @@ type completedProxyRunOptions struct {
ignorePaths []string
}

func (o *completedProxyRunOptions) Validate() []error {
var errs []error

hasCerts := !(o.tls.CertFile == "") && !(o.tls.KeyFile == "")
hasInsecureListenAddress := o.insecureListenAddress != ""
if !hasCerts || hasInsecureListenAddress {
klog.Warning(`
==== Deprecation Warning ======================
Insecure listen address will be removed.
Using --insecure-listen-address won't be possible!
The ability to run kube-rbac-proxy without TLS certificates will be removed.
Not using --tls-cert-file and --tls-private-key-file won't be possible!
For more information, please go to https://github.com/brancz/kube-rbac-proxy/issues/187
===============================================
`)
}

if o.tls.ReloadInterval != time.Minute {
klog.Warning(`
==== Deprecation Warning ======================
tls-reload-interval will be removed.
Using --tls-reload-interval won't be possible!
For more information, please go to https://github.com/brancz/kube-rbac-proxy/issues/196
===============================================
`)

}

if len(o.allowPaths) > 0 && len(o.ignorePaths) > 0 {
errs = append(errs, fmt.Errorf("cannot use --allow-paths and --ignore-paths together"))
}

for _, pathAllowed := range o.allowPaths {
_, err := path.Match(pathAllowed, "")
if err != nil {
errs = append(errs, fmt.Errorf("failed to verify allow path: %s", pathAllowed))
}
}

for _, pathIgnored := range o.ignorePaths {
_, err := path.Match(pathIgnored, "")
if err != nil {
errs = append(errs, fmt.Errorf("failed to verify ignored path: %s", pathIgnored))
}
}

return errs
}

func Complete(o *options.ProxyRunOptions) (*completedProxyRunOptions, error) {
func Complete(o *options.ProxyRunOptions) (*proxyConfig, error) {
var err error
completed := &completedProxyRunOptions{
config := &proxyConfig{
insecureListenAddress: o.InsecureListenAddress,
secureListenAddress: o.SecureListenAddress,
proxyEndpointsPort: o.ProxyEndpointsPort,
Expand All @@ -215,7 +156,7 @@ func Complete(o *options.ProxyRunOptions) (*completedProxyRunOptions, error) {
ignorePaths: o.IgnorePaths,
}

completed.upstreamURL, err = url.Parse(o.Upstream)
config.upstreamURL, err = url.Parse(o.Upstream)
if err != nil {
return nil, fmt.Errorf("failed to parse upstream URL: %w", err)
}
Expand All @@ -230,14 +171,14 @@ func Complete(o *options.ProxyRunOptions) (*completedProxyRunOptions, error) {
if ok := upstreamCACertPool.AppendCertsFromPEM(upstreamCAPEM); !ok {
return nil, errors.New("error parsing upstream CA certificate")
}
completed.upstreamCABundle = upstreamCACertPool
config.upstreamCABundle = upstreamCACertPool
}

completed.auth = o.Auth
completed.tls = o.TLS
config.auth = o.Auth
config.tls = o.TLS

if configFileName := o.ConfigFileName; len(configFileName) > 0 {
completed.auth.Authorization, err = parseAuthorizationConfigFile(configFileName)
config.auth.Authorization, err = parseAuthorizationConfigFile(configFileName)
if err != nil {
return nil, fmt.Errorf("failed to read the config file: %w", err)
}
Expand All @@ -248,24 +189,24 @@ func Complete(o *options.ProxyRunOptions) (*completedProxyRunOptions, error) {
return nil, fmt.Errorf("failed to load kubeconfig: %w", err)
}

completed.kubeClient, err = kubernetes.NewForConfig(kubeconfig)
config.kubeClient, err = kubernetes.NewForConfig(kubeconfig)
if err != nil {
return nil, fmt.Errorf("failed to instantiate Kubernetes client: %w", err)
}

completed.http2Disable = o.HTTP2Disable
completed.http2Options = &http2.Server{
config.http2Disable = o.HTTP2Disable
config.http2Options = &http2.Server{
IdleTimeout: 90 * time.Second,
MaxConcurrentStreams: o.HTTP2MaxConcurrentStreams,
MaxReadFrameSize: o.HTTP2MaxSize,
MaxUploadBufferPerStream: int32(o.HTTP2MaxSize),
MaxUploadBufferPerConnection: int32(o.HTTP2MaxSize) * int32(o.HTTP2MaxConcurrentStreams),
}

return completed, nil
return config, nil
}

func Run(cfg *completedProxyRunOptions) error {
func Run(cfg *proxyConfig) error {
var authenticator authenticator.Request
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand Down
118 changes: 105 additions & 13 deletions cmd/kube-rbac-proxy/app/options/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,18 @@ limitations under the License.
package options

import (
"fmt"
"path"
"time"

utilerrors "k8s.io/apimachinery/pkg/util/errors"
k8sapiflag "k8s.io/component-base/cli/flag"
"k8s.io/klog/v2"

"github.com/brancz/kube-rbac-proxy/pkg/authn"
"github.com/brancz/kube-rbac-proxy/pkg/authz"
"github.com/brancz/kube-rbac-proxy/pkg/proxy"
"github.com/spf13/pflag"
)

type ProxyRunOptions struct {
Expand All @@ -47,11 +52,11 @@ type ProxyRunOptions struct {
HTTP2MaxSize uint32

disabled disabledOptions
flagSet *pflag.FlagSet
}

type disabledOptions struct {
logtostderr bool

logtostderr bool
adddirheader bool
alsologtostderr bool
logbacktrace int
Expand Down Expand Up @@ -87,6 +92,9 @@ func NewProxyRunOptions() *ProxyRunOptions {
Authorization: &authz.Config{},
},
TLS: &TLSConfig{},

// Removed flags
disabled: disabledOptions{},
}
}

Expand Down Expand Up @@ -140,17 +148,101 @@ 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
flagset.BoolVar(&o.disabled.logtostderr, "logtostderr", false, "[DISABLED] Log to standard error instead of files")
flagset.BoolVar(&o.disabled.adddirheader, "add-dir-header", false, "[DISABLED] If true, adds the file directory to the header of the log messages")
flagset.BoolVar(&o.disabled.alsologtostderr, "alsologtostderr", false, "[DISABLED] Log to standard error as well as files")
flagset.IntVar(&o.disabled.logbacktrace, "log-backtrace-at", 0, "[DISABLED] when logging hits line file:N, emit a stack trace")
flagset.StringVar(&o.disabled.logdir, "log-dir", "", "[DISABLED] If non-empty, write log files in this directory")
flagset.StringVar(&o.disabled.logfile, "log-file", "", "[DISABLED] If non-empty, use this log file")
flagset.Uint32Var(&o.disabled.logfilemaxsize, "log-file-max-size", 1800, "[DISABLED] Defines the maximum size a log file can grow to. Unit is megabytes. If the value is 0, the maximum file size is unlimited.")
flagset.BoolVar(&o.disabled.oneoutput, "one-output", false, "[DISABLED] If true, only write logs to their native severity level (vs also writing to each lower severity level)")
flagset.BoolVar(&o.disabled.skipheaders, "skip-headers", false, "[DISABLED] If true, avoid header prefixes in the log messages")
flagset.BoolVar(&o.disabled.skiplogheaders, "skip-log-headers", false, "[DISABLED] If true, avoid headers when opening log files")
flagset.IntVar(&o.disabled.stderrthreshold, "stderrthreshold", 2, "[DISABLED] logs at or above this threshold go to stderr")
o.flagSet = flagset // reference used for validation
flagset.BoolVar(&o.disabled.logtostderr, "logtostderr", false, "[DISABLED]")
flagset.BoolVar(&o.disabled.adddirheader, "add-dir-header", false, "[DISABLED]")
flagset.BoolVar(&o.disabled.alsologtostderr, "alsologtostderr", false, "[DISABLED]")
flagset.IntVar(&o.disabled.logbacktrace, "log-backtrace-at", 0, "[DISABLED]")
flagset.StringVar(&o.disabled.logdir, "log-dir", "", "[DISABLED]")
flagset.StringVar(&o.disabled.logfile, "log-file", "", "[DISABLED]")
flagset.Uint32Var(&o.disabled.logfilemaxsize, "log-file-max-size", 1800, "[DISABLED]")
flagset.BoolVar(&o.disabled.oneoutput, "one-output", false, "[DISABLED]")
flagset.BoolVar(&o.disabled.skipheaders, "skip-headers", false, "[DISABLED]")
flagset.BoolVar(&o.disabled.skiplogheaders, "skip-log-headers", false, "[DISABLED]")
flagset.IntVar(&o.disabled.stderrthreshold, "stderrthreshold", 2, "[DISABLED]")

return namedFlagSets
}

func (o *ProxyRunOptions) Validate() error {
var errs []error

hasCerts := !(o.TLS.CertFile == "") && !(o.TLS.KeyFile == "")
hasInsecureListenAddress := o.InsecureListenAddress != ""
if !hasCerts || hasInsecureListenAddress {
klog.Warning(`
==== Deprecation Warning ======================
Insecure listen address will be removed.
Using --insecure-listen-address won't be possible!
The ability to run kube-rbac-proxy without TLS certificates will be removed.
Not using --tls-cert-file and --tls-private-key-file won't be possible!
For more information, please go to https://github.com/brancz/kube-rbac-proxy/issues/187
===============================================
`)
}

if o.TLS.ReloadInterval != time.Minute {
klog.Warning(`
==== Deprecation Warning ======================
tls-reload-interval will be removed.
Using --tls-reload-interval won't be possible!
For more information, please go to https://github.com/brancz/kube-rbac-proxy/issues/196
===============================================
`)

}

if len(o.AllowPaths) > 0 && len(o.IgnorePaths) > 0 {
errs = append(errs, fmt.Errorf("cannot use --allow-paths and --ignore-paths together"))
}

for _, pathAllowed := range o.AllowPaths {
_, err := path.Match(pathAllowed, "")
if err != nil {
errs = append(errs, fmt.Errorf("failed to verify allow path: %s", pathAllowed))
}
}

for _, pathIgnored := range o.IgnorePaths {
_, err := path.Match(pathIgnored, "")
if err != nil {
errs = append(errs, fmt.Errorf("failed to verify ignored path: %s", pathIgnored))
}
}

// Removed upstream flags shouldn't be use
for _, disabledOpt := range []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",
} {
if flag := o.flagSet.Lookup(disabledOpt); flag.Changed {
klog.Warningf("flag %s is no longer supported by upstream and disabled", disabledOpt)
klog.Warningf(`
==== Removed Flag Warning ======================
%s is removed in the k8s upstream and has no effect any more.
===============================================
`, disabledOpt)
}
}

return utilerrors.NewAggregate(errs)
}
Loading

0 comments on commit f4aa76e

Please sign in to comment.