From f4db3458c9bbb02a868051a49a2890e2994faee0 Mon Sep 17 00:00:00 2001 From: Alex Price Date: Wed, 17 Jul 2024 15:28:18 +1000 Subject: [PATCH] Also use tls options for metrics/diagnostics server Signed-off-by: Alex Price --- bootstrap/kubeadm/main.go | 16 +- controlplane/kubeadm/main.go | 16 +- .../providers/migrations/v1.7-to-v1.8.md | 1 + main.go | 16 +- test/extension/main.go | 16 +- test/infrastructure/docker/main.go | 16 +- test/infrastructure/inmemory/main.go | 16 +- util/flags/diagnostics.go | 94 ---------- util/flags/manager.go | 154 ++++++++++++++++ util/flags/manager_test.go | 166 ++++++++++++++++++ util/flags/tls.go | 75 -------- 11 files changed, 357 insertions(+), 229 deletions(-) delete mode 100644 util/flags/diagnostics.go create mode 100644 util/flags/manager.go create mode 100644 util/flags/manager_test.go delete mode 100644 util/flags/tls.go diff --git a/bootstrap/kubeadm/main.go b/bootstrap/kubeadm/main.go index 34b9075904e6..e6a210478b31 100644 --- a/bootstrap/kubeadm/main.go +++ b/bootstrap/kubeadm/main.go @@ -80,8 +80,7 @@ var ( webhookCertName string webhookKeyName string healthAddr string - tlsOptions = flags.TLSOptions{} - diagnosticsOptions = flags.DiagnosticsOptions{} + managerOptions = flags.ManagerOptions{} logOptions = logs.NewOptions() // CABPK specific flags. clusterConcurrency int @@ -170,8 +169,7 @@ func InitFlags(fs *pflag.FlagSet) { fs.StringVar(&healthAddr, "health-addr", ":9440", "The address the health endpoint binds to.") - flags.AddDiagnosticsOptions(fs, &diagnosticsOptions) - flags.AddTLSOptions(fs, &tlsOptions) + flags.AddManagerOptions(fs, &managerOptions) feature.MutableGates.AddFlag(fs) } @@ -204,14 +202,12 @@ func main() { restConfig.Burst = restConfigBurst restConfig.UserAgent = remote.DefaultClusterAPIUserAgent(controllerName) - tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) + tlsOptions, metricsOptions, err := flags.GetManagerOptions(managerOptions) if err != nil { - setupLog.Error(err, "unable to add TLS settings to the webhook server") + setupLog.Error(err, "Unable to start manager: invalid flags") os.Exit(1) } - diagnosticsOpts := flags.GetDiagnosticsOptions(diagnosticsOptions) - var watchNamespaces map[string]cache.Config if watchNamespace != "" { watchNamespaces = map[string]cache.Config{ @@ -236,7 +232,7 @@ func main() { LeaderElectionResourceLock: resourcelock.LeasesResourceLock, HealthProbeBindAddress: healthAddr, PprofBindAddress: profilerAddress, - Metrics: diagnosticsOpts, + Metrics: *metricsOptions, Cache: cache.Options{ DefaultNamespaces: watchNamespaces, SyncPeriod: &syncPeriod, @@ -265,7 +261,7 @@ func main() { CertDir: webhookCertDir, CertName: webhookCertName, KeyName: webhookKeyName, - TLSOpts: tlsOptionOverrides, + TLSOpts: tlsOptions, }, ), } diff --git a/controlplane/kubeadm/main.go b/controlplane/kubeadm/main.go index 6aae5d116805..ea023f91a50a 100644 --- a/controlplane/kubeadm/main.go +++ b/controlplane/kubeadm/main.go @@ -84,8 +84,7 @@ var ( webhookCertName string webhookKeyName string healthAddr string - tlsOptions = flags.TLSOptions{} - diagnosticsOptions = flags.DiagnosticsOptions{} + managerOptions = flags.ManagerOptions{} logOptions = logs.NewOptions() // KCP specific flags. kubeadmControlPlaneConcurrency int @@ -180,8 +179,7 @@ func InitFlags(fs *pflag.FlagSet) { "Use the deprecated naming convention for infra machines where they are named after the InfraMachineTemplate.") _ = fs.MarkDeprecated("use-deprecated-infra-machine-naming", "This flag will be removed in v1.9.") - flags.AddDiagnosticsOptions(fs, &diagnosticsOptions) - flags.AddTLSOptions(fs, &tlsOptions) + flags.AddManagerOptions(fs, &managerOptions) feature.MutableGates.AddFlag(fs) } @@ -214,14 +212,12 @@ func main() { restConfig.Burst = restConfigBurst restConfig.UserAgent = remote.DefaultClusterAPIUserAgent(controllerName) - tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) + tlsOptions, metricsOptions, err := flags.GetManagerOptions(managerOptions) if err != nil { - setupLog.Error(err, "unable to add TLS settings to the webhook server") + setupLog.Error(err, "Unable to start manager: invalid flags") os.Exit(1) } - diagnosticsOpts := flags.GetDiagnosticsOptions(diagnosticsOptions) - var watchNamespaces map[string]cache.Config if watchNamespace != "" { watchNamespaces = map[string]cache.Config{ @@ -246,7 +242,7 @@ func main() { LeaderElectionResourceLock: resourcelock.LeasesResourceLock, HealthProbeBindAddress: healthAddr, PprofBindAddress: profilerAddress, - Metrics: diagnosticsOpts, + Metrics: *metricsOptions, Cache: cache.Options{ DefaultNamespaces: watchNamespaces, SyncPeriod: &syncPeriod, @@ -278,7 +274,7 @@ func main() { CertDir: webhookCertDir, CertName: webhookCertName, KeyName: webhookKeyName, - TLSOpts: tlsOptionOverrides, + TLSOpts: tlsOptions, }, ), } diff --git a/docs/book/src/developer/providers/migrations/v1.7-to-v1.8.md b/docs/book/src/developer/providers/migrations/v1.7-to-v1.8.md index f5ccea55795c..d4cfd96d9c26 100644 --- a/docs/book/src/developer/providers/migrations/v1.7-to-v1.8.md +++ b/docs/book/src/developer/providers/migrations/v1.7-to-v1.8.md @@ -29,3 +29,4 @@ maintainers of providers and consumers of our Go API. and [kubernetes-sigs/controller-runtime#2811](https://github.com/kubernetes-sigs/controller-runtime/pull/2811). - `remote.NewClusterCacheTracker` now has options to configure QPS & Burst. It's highly recommended to implement corresponding flags the same way as core Cluster API (see PR: https://github.com/kubernetes-sigs/cluster-api/pull/10880). +- There were changes made to flags in core CAPI (https://github.com/kubernetes-sigs/cluster-api/pull/10883, https://github.com/kubernetes-sigs/cluster-api/pull/10880). It's recommended to adopt these changes in providers as well. diff --git a/main.go b/main.go index 5f39f03c66dc..36ed3aea29ef 100644 --- a/main.go +++ b/main.go @@ -102,8 +102,7 @@ var ( webhookCertName string webhookKeyName string healthAddr string - tlsOptions = flags.TLSOptions{} - diagnosticsOptions = flags.DiagnosticsOptions{} + managerOptions = flags.ManagerOptions{} logOptions = logs.NewOptions() // core Cluster API specific flags. clusterTopologyConcurrency int @@ -243,8 +242,7 @@ func InitFlags(fs *pflag.FlagSet) { "Use deprecated infrastructure machine naming") _ = fs.MarkDeprecated("use-deprecated-infra-machine-naming", "This flag will be removed in v1.9.") - flags.AddDiagnosticsOptions(fs, &diagnosticsOptions) - flags.AddTLSOptions(fs, &tlsOptions) + flags.AddManagerOptions(fs, &managerOptions) feature.MutableGates.AddFlag(fs) } @@ -292,14 +290,12 @@ func main() { os.Exit(1) } - tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) + tlsOptions, metricsOptions, err := flags.GetManagerOptions(managerOptions) if err != nil { - setupLog.Error(err, "Unable to add TLS settings to the webhook server") + setupLog.Error(err, "Unable to start manager: invalid flags") os.Exit(1) } - diagnosticsOpts := flags.GetDiagnosticsOptions(diagnosticsOptions) - var watchNamespaces map[string]cache.Config if watchNamespace != "" { watchNamespaces = map[string]cache.Config{ @@ -324,7 +320,7 @@ func main() { LeaderElectionResourceLock: resourcelock.LeasesResourceLock, HealthProbeBindAddress: healthAddr, PprofBindAddress: profilerAddress, - Metrics: diagnosticsOpts, + Metrics: *metricsOptions, Cache: cache.Options{ DefaultNamespaces: watchNamespaces, SyncPeriod: &syncPeriod, @@ -353,7 +349,7 @@ func main() { CertDir: webhookCertDir, CertName: webhookCertName, KeyName: webhookKeyName, - TLSOpts: tlsOptionOverrides, + TLSOpts: tlsOptions, }, ), } diff --git a/test/extension/main.go b/test/extension/main.go index 0cd1f263a607..b2c77def5b89 100644 --- a/test/extension/main.go +++ b/test/extension/main.go @@ -85,8 +85,7 @@ var ( webhookCertName string webhookKeyName string healthAddr string - tlsOptions = flags.TLSOptions{} - diagnosticsOptions = flags.DiagnosticsOptions{} + managerOptions = flags.ManagerOptions{} logOptions = logs.NewOptions() ) @@ -153,8 +152,7 @@ func InitFlags(fs *pflag.FlagSet) { fs.StringVar(&healthAddr, "health-addr", ":9440", "The address the health endpoint binds to.") - flags.AddDiagnosticsOptions(fs, &diagnosticsOptions) - flags.AddTLSOptions(fs, &tlsOptions) + flags.AddManagerOptions(fs, &managerOptions) // Add test-extension specific flags // NOTE: it is not mandatory to use the same flag names in all RuntimeExtension, but it is recommended when @@ -197,14 +195,12 @@ func main() { restConfig.Burst = restConfigBurst restConfig.UserAgent = remote.DefaultClusterAPIUserAgent(controllerName) - tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) + tlsOptions, metricsOptions, err := flags.GetManagerOptions(managerOptions) if err != nil { - setupLog.Error(err, "Unable to add TLS settings to the webhook server") + setupLog.Error(err, "Unable to start manager: invalid flags") os.Exit(1) } - diagnosticsOpts := flags.GetDiagnosticsOptions(diagnosticsOptions) - if enableContentionProfiling { goruntime.SetBlockProfileRate(1) } @@ -215,7 +211,7 @@ func main() { CertDir: webhookCertDir, CertName: webhookCertName, KeyName: webhookKeyName, - TLSOpts: tlsOptionOverrides, + TLSOpts: tlsOptions, Catalog: catalog, }) if err != nil { @@ -233,7 +229,7 @@ func main() { LeaderElectionResourceLock: resourcelock.LeasesResourceLock, HealthProbeBindAddress: healthAddr, PprofBindAddress: profilerAddress, - Metrics: diagnosticsOpts, + Metrics: *metricsOptions, Cache: cache.Options{ SyncPeriod: &syncPeriod, }, diff --git a/test/infrastructure/docker/main.go b/test/infrastructure/docker/main.go index 63a4ee72ccfc..e1e774952c5c 100644 --- a/test/infrastructure/docker/main.go +++ b/test/infrastructure/docker/main.go @@ -87,8 +87,7 @@ var ( webhookCertName string webhookKeyName string healthAddr string - tlsOptions = flags.TLSOptions{} - diagnosticsOptions = flags.DiagnosticsOptions{} + managerOptions = flags.ManagerOptions{} logOptions = logs.NewOptions() // CAPD specific flags. concurrency int @@ -171,8 +170,7 @@ func InitFlags(fs *pflag.FlagSet) { fs.StringVar(&healthAddr, "health-addr", ":9440", "The address the health endpoint binds to.") - flags.AddDiagnosticsOptions(fs, &diagnosticsOptions) - flags.AddTLSOptions(fs, &tlsOptions) + flags.AddManagerOptions(fs, &managerOptions) feature.MutableGates.AddFlag(fs) } @@ -210,14 +208,12 @@ func main() { restConfig.Burst = restConfigBurst restConfig.UserAgent = remote.DefaultClusterAPIUserAgent(controllerName) - tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) + tlsOptions, metricsOptions, err := flags.GetManagerOptions(managerOptions) if err != nil { - setupLog.Error(err, "Unable to add TLS settings to the webhook server") + setupLog.Error(err, "Unable to start manager: invalid flags") os.Exit(1) } - diagnosticsOpts := flags.GetDiagnosticsOptions(diagnosticsOptions) - var watchNamespaces map[string]cache.Config if watchNamespace != "" { watchNamespaces = map[string]cache.Config{ @@ -242,7 +238,7 @@ func main() { LeaderElectionResourceLock: resourcelock.LeasesResourceLock, HealthProbeBindAddress: healthAddr, PprofBindAddress: profilerAddress, - Metrics: diagnosticsOpts, + Metrics: *metricsOptions, Cache: cache.Options{ DefaultNamespaces: watchNamespaces, SyncPeriod: &syncPeriod, @@ -271,7 +267,7 @@ func main() { CertDir: webhookCertDir, CertName: webhookCertName, KeyName: webhookKeyName, - TLSOpts: tlsOptionOverrides, + TLSOpts: tlsOptions, }, ), } diff --git a/test/infrastructure/inmemory/main.go b/test/infrastructure/inmemory/main.go index a1d08368677c..b3b712b8f6ce 100644 --- a/test/infrastructure/inmemory/main.go +++ b/test/infrastructure/inmemory/main.go @@ -79,8 +79,7 @@ var ( webhookCertName string webhookKeyName string healthAddr string - tlsOptions = flags.TLSOptions{} - diagnosticsOptions = flags.DiagnosticsOptions{} + managerOptions = flags.ManagerOptions{} logOptions = logs.NewOptions() // CAPIM specific flags. clusterConcurrency int @@ -158,8 +157,7 @@ func InitFlags(fs *pflag.FlagSet) { fs.StringVar(&healthAddr, "health-addr", ":9440", "The address the health endpoint binds to.") - flags.AddDiagnosticsOptions(fs, &diagnosticsOptions) - flags.AddTLSOptions(fs, &tlsOptions) + flags.AddManagerOptions(fs, &managerOptions) feature.MutableGates.AddFlag(fs) } @@ -192,14 +190,12 @@ func main() { restConfig.Burst = restConfigBurst restConfig.UserAgent = remote.DefaultClusterAPIUserAgent(controllerName) - tlsOptionOverrides, err := flags.GetTLSOptionOverrideFuncs(tlsOptions) + tlsOptions, metricsOptions, err := flags.GetManagerOptions(managerOptions) if err != nil { - setupLog.Error(err, "Unable to add TLS settings to the webhook server") + setupLog.Error(err, "Unable to start manager: invalid flags") os.Exit(1) } - diagnosticsOpts := flags.GetDiagnosticsOptions(diagnosticsOptions) - var watchNamespaces map[string]cache.Config if watchNamespace != "" { watchNamespaces = map[string]cache.Config{ @@ -221,7 +217,7 @@ func main() { LeaderElectionResourceLock: resourcelock.LeasesResourceLock, HealthProbeBindAddress: healthAddr, PprofBindAddress: profilerAddress, - Metrics: diagnosticsOpts, + Metrics: *metricsOptions, Cache: cache.Options{ DefaultNamespaces: watchNamespaces, SyncPeriod: &syncPeriod, @@ -242,7 +238,7 @@ func main() { CertDir: webhookCertDir, CertName: webhookCertName, KeyName: webhookKeyName, - TLSOpts: tlsOptionOverrides, + TLSOpts: tlsOptions, }, ), } diff --git a/util/flags/diagnostics.go b/util/flags/diagnostics.go deleted file mode 100644 index f88dfdc20b6f..000000000000 --- a/util/flags/diagnostics.go +++ /dev/null @@ -1,94 +0,0 @@ -/* -Copyright 2022 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 flags implements the webhook server TLS options utilities. -package flags - -import ( - "net/http" - "net/http/pprof" - - "github.com/spf13/pflag" - "k8s.io/apiserver/pkg/server/routes" - "k8s.io/component-base/logs" - "sigs.k8s.io/controller-runtime/pkg/metrics/filters" - metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" -) - -// DiagnosticsOptions has the options to configure diagnostics. -type DiagnosticsOptions struct { - // MetricsBindAddr - // - // Deprecated: This field will be removed in an upcoming release. - MetricsBindAddr string - DiagnosticsAddress string - InsecureDiagnostics bool -} - -// AddDiagnosticsOptions adds the diagnostics flags to the flag set. -func AddDiagnosticsOptions(fs *pflag.FlagSet, options *DiagnosticsOptions) { - fs.StringVar(&options.MetricsBindAddr, "metrics-bind-addr", "", - "The address the metrics endpoint binds to.") - _ = fs.MarkDeprecated("metrics-bind-addr", "Please use --diagnostics-address instead. To continue to serve"+ - "metrics via http and without authentication/authorization set --insecure-diagnostics as well.") - - fs.StringVar(&options.DiagnosticsAddress, "diagnostics-address", ":8443", - "The address the diagnostics endpoint binds to. Per default metrics are served via https and with"+ - "authentication/authorization. To serve via http and without authentication/authorization set --insecure-diagnostics."+ - "If --insecure-diagnostics is not set the diagnostics endpoint also serves pprof endpoints and an endpoint to change the log level.") - - fs.BoolVar(&options.InsecureDiagnostics, "insecure-diagnostics", false, - "Enable insecure diagnostics serving. For more details see the description of --diagnostics-address.") -} - -// GetDiagnosticsOptions returns metrics options which can be used to configure a Manager. -func GetDiagnosticsOptions(options DiagnosticsOptions) metricsserver.Options { - // If the deprecated "--metrics-bind-addr" flag is set, continue to serve metrics via http - // and without authentication/authorization. - if options.MetricsBindAddr != "" { - return metricsserver.Options{ - BindAddress: options.MetricsBindAddr, - } - } - - // If "--insecure-diagnostics" is set, serve metrics via http - // and without authentication/authorization. - if options.InsecureDiagnostics { - return metricsserver.Options{ - BindAddress: options.DiagnosticsAddress, - SecureServing: false, - } - } - - // If "--insecure-diagnostics" is not set, serve metrics via https - // and with authentication/authorization. As the endpoint is protected, - // we also serve pprof endpoints and an endpoint to change the log level. - return metricsserver.Options{ - BindAddress: options.DiagnosticsAddress, - SecureServing: true, - FilterProvider: filters.WithAuthenticationAndAuthorization, - ExtraHandlers: map[string]http.Handler{ - // Add handler to dynamically change log level. - "/debug/flags/v": routes.StringFlagPutHandler(logs.GlogSetter), - // Add pprof handler. - "/debug/pprof/": http.HandlerFunc(pprof.Index), - "/debug/pprof/cmdline": http.HandlerFunc(pprof.Cmdline), - "/debug/pprof/profile": http.HandlerFunc(pprof.Profile), - "/debug/pprof/symbol": http.HandlerFunc(pprof.Symbol), - "/debug/pprof/trace": http.HandlerFunc(pprof.Trace), - }, - } -} diff --git a/util/flags/manager.go b/util/flags/manager.go new file mode 100644 index 000000000000..c3ccb88c68c4 --- /dev/null +++ b/util/flags/manager.go @@ -0,0 +1,154 @@ +/* +Copyright 2024 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 flags implements the manager options utilities. +package flags + +import ( + "crypto/tls" + "fmt" + "net/http" + "net/http/pprof" + "strings" + + "github.com/spf13/pflag" + "k8s.io/apiserver/pkg/server/routes" + cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" + "sigs.k8s.io/controller-runtime/pkg/metrics/filters" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" +) + +// ManagerOptions provides command line flags for manager options. +type ManagerOptions struct { + // TLS Options + // These are used for the webhook and for the metrics server (the latter only if secure serving is used) + + // TLSMinVersion is the field that stores the value of the --tls-min-version flag. + // For further details, please see the description of the flag. + TLSMinVersion string + // TLSCipherSuites is the field that stores the value of the --tls-cipher-suites flag. + // For further details, please see the description of the flag. + TLSCipherSuites []string + + // Metrics Options + // These are used to configure the metrics server + + // MetricsBindAddr is the field that stores the value of the --metrics-bind-addr flag. + // For further details, please see the description of the flag. + // + // Deprecated: This field will be removed in an upcoming release. + MetricsBindAddr string + // DiagnosticsAddress is the field that stores the value of the --diagnostics-address flag. + // For further details, please see the description of the flag. + DiagnosticsAddress string + // InsecureDiagnostics is the field that stores the value of the --insecure-diagnostics flag. + // For further details, please see the description of the flag. + InsecureDiagnostics bool +} + +// AddManagerOptions adds the manager options flags to the flag set. +func AddManagerOptions(fs *pflag.FlagSet, options *ManagerOptions) { + fs.StringVar(&options.TLSMinVersion, "tls-min-version", "VersionTLS12", + "The minimum TLS version in use by the webhook server and metrics server (the latter only if --insecure-diagnostics is not set to true).\n"+ + fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSPossibleVersions(), ", ")), + ) + + tlsCipherPreferredValues := cliflag.PreferredTLSCipherNames() + tlsCipherInsecureValues := cliflag.InsecureTLSCipherNames() + fs.StringSliceVar(&options.TLSCipherSuites, "tls-cipher-suites", []string{}, + "Comma-separated list of cipher suites for the webhook server and metrics server (the latter only if --insecure-diagnostics is not set to true). "+ + "If omitted, the default Go cipher suites will be used. \n"+ + "Preferred values: "+strings.Join(tlsCipherPreferredValues, ", ")+". \n"+ + "Insecure values: "+strings.Join(tlsCipherInsecureValues, ", ")+".") + + fs.StringVar(&options.MetricsBindAddr, "metrics-bind-addr", "", + "The address the metrics endpoint binds to.") + _ = fs.MarkDeprecated("metrics-bind-addr", "Please use --diagnostics-address instead. To continue to serve "+ + "metrics via http and without authentication/authorization set --insecure-diagnostics as well.") + + fs.StringVar(&options.DiagnosticsAddress, "diagnostics-address", ":8443", + "The address the diagnostics endpoint binds to. Per default metrics are served via https and with"+ + "authentication/authorization. To serve via http and without authentication/authorization set --insecure-diagnostics. "+ + "If --insecure-diagnostics is not set the diagnostics endpoint also serves pprof endpoints and an endpoint to change the log level.") + + fs.BoolVar(&options.InsecureDiagnostics, "insecure-diagnostics", false, + "Enable insecure diagnostics serving. For more details see the description of --diagnostics-address.") +} + +// GetManagerOptions returns options which can be used to configure a Manager. +// This function should be used with the corresponding AddManagerOptions func. +func GetManagerOptions(options ManagerOptions) ([]func(config *tls.Config), *metricsserver.Options, error) { + var tlsOptions []func(config *tls.Config) + var metricsOptions *metricsserver.Options + + tlsVersion, err := cliflag.TLSVersion(options.TLSMinVersion) + if err != nil { + return nil, nil, err + } + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { + cfg.MinVersion = tlsVersion + }) + + if len(options.TLSCipherSuites) != 0 { + suites, err := cliflag.TLSCipherSuites(options.TLSCipherSuites) + if err != nil { + return nil, nil, err + } + tlsOptions = append(tlsOptions, func(cfg *tls.Config) { + cfg.CipherSuites = suites + }) + } + + // If the deprecated "--metrics-bind-addr" flag is set, continue to serve metrics via http + // and without authentication/authorization. + if options.MetricsBindAddr != "" { + metricsOptions = &metricsserver.Options{ + BindAddress: options.MetricsBindAddr, + } + } else { + // If "--insecure-diagnostics" is set, serve metrics via http + // and without authentication/authorization. + if options.InsecureDiagnostics { + metricsOptions = &metricsserver.Options{ + BindAddress: options.DiagnosticsAddress, + SecureServing: false, + } + } else { + // If "--insecure-diagnostics" is not set, serve metrics via https + // and with authentication/authorization. As the endpoint is protected, + // we also serve pprof endpoints and an endpoint to change the log level. + metricsOptions = &metricsserver.Options{ + BindAddress: options.DiagnosticsAddress, + SecureServing: true, + FilterProvider: filters.WithAuthenticationAndAuthorization, + ExtraHandlers: map[string]http.Handler{ + // Add handler to dynamically change log level. + "/debug/flags/v": routes.StringFlagPutHandler(logs.GlogSetter), + // Add pprof handler. + "/debug/pprof/": http.HandlerFunc(pprof.Index), + "/debug/pprof/cmdline": http.HandlerFunc(pprof.Cmdline), + "/debug/pprof/profile": http.HandlerFunc(pprof.Profile), + "/debug/pprof/symbol": http.HandlerFunc(pprof.Symbol), + "/debug/pprof/trace": http.HandlerFunc(pprof.Trace), + }, + TLSOpts: tlsOptions, + } + } + } + + return tlsOptions, metricsOptions, nil +} diff --git a/util/flags/manager_test.go b/util/flags/manager_test.go new file mode 100644 index 000000000000..9183d1ec663a --- /dev/null +++ b/util/flags/manager_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2024 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 flags + +import ( + "crypto/tls" + "net/http" + "net/http/pprof" + "testing" + + . "github.com/onsi/gomega" + "k8s.io/apiserver/pkg/server/routes" + "k8s.io/component-base/logs" + "sigs.k8s.io/controller-runtime/pkg/metrics/filters" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" +) + +func TestGetManagerOptions(t *testing.T) { + tests := []struct { + name string + managerOptions ManagerOptions + wantTLSConfig *tls.Config + wantMetricsOptions *metricsserver.Options + wantMetricsOptionsTLSConfig *tls.Config + wantErr bool + }{ + { + name: "invalid tls min version", + managerOptions: ManagerOptions{ + TLSMinVersion: "VersionTLS_DOES_NOT_EXIST", + }, + wantErr: true, + }, + { + name: "invalid tls cipher suites", + managerOptions: ManagerOptions{ + TLSCipherSuites: []string{"TLS_DOES_NOT_EXIST"}, + }, + wantErr: true, + }, + { + name: "valid tls + metrics bind addr", + managerOptions: ManagerOptions{ + TLSMinVersion: "VersionTLS13", + TLSCipherSuites: []string{"TLS_AES_256_GCM_SHA384"}, + MetricsBindAddr: ":8080", + }, + wantTLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS13, + CipherSuites: []uint16{tls.TLS_AES_256_GCM_SHA384}, + }, + wantMetricsOptions: &metricsserver.Options{ + BindAddress: ":8080", + }, + wantErr: false, + }, + { + name: "valid tls + insecure diagnostics + diagnostics address", + managerOptions: ManagerOptions{ + TLSMinVersion: "VersionTLS13", + TLSCipherSuites: []string{"TLS_AES_256_GCM_SHA384"}, + InsecureDiagnostics: true, + DiagnosticsAddress: ":8443", + }, + wantTLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS13, + CipherSuites: []uint16{tls.TLS_AES_256_GCM_SHA384}, + }, + wantMetricsOptions: &metricsserver.Options{ + BindAddress: ":8443", + }, + wantErr: false, + }, + { + name: "valid tls + diagnostics address", + managerOptions: ManagerOptions{ + TLSMinVersion: "VersionTLS13", + TLSCipherSuites: []string{"TLS_AES_256_GCM_SHA384"}, + DiagnosticsAddress: ":8443", + }, + wantTLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS13, + CipherSuites: []uint16{tls.TLS_AES_256_GCM_SHA384}, + }, + wantMetricsOptions: &metricsserver.Options{ + BindAddress: ":8443", + SecureServing: true, + FilterProvider: filters.WithAuthenticationAndAuthorization, + ExtraHandlers: map[string]http.Handler{ + // Add handler to dynamically change log level. + "/debug/flags/v": routes.StringFlagPutHandler(logs.GlogSetter), + // Add pprof handler. + "/debug/pprof/": http.HandlerFunc(pprof.Index), + "/debug/pprof/cmdline": http.HandlerFunc(pprof.Cmdline), + "/debug/pprof/profile": http.HandlerFunc(pprof.Profile), + "/debug/pprof/symbol": http.HandlerFunc(pprof.Symbol), + "/debug/pprof/trace": http.HandlerFunc(pprof.Trace), + }, + }, + wantMetricsOptionsTLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS13, + CipherSuites: []uint16{tls.TLS_AES_256_GCM_SHA384}, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + tlsOpts, metricsOptions, err := GetManagerOptions(tt.managerOptions) + + tlsConfig := &tls.Config{} //nolint:gosec // it's fine to not set a TLS min version here. + for _, opt := range tlsOpts { + opt(tlsConfig) + } + var metricsOptionsTLSConfig *tls.Config + if metricsOptions != nil && metricsOptions.TLSOpts != nil { + metricsOptionsTLSConfig = &tls.Config{} //nolint:gosec // it's fine to not set a TLS min version here. + for _, opt := range metricsOptions.TLSOpts { + opt(metricsOptionsTLSConfig) + } + } + + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + return + } + + g.Expect(err).ToNot(HaveOccurred()) + // When we don't get an error we expect that metricsOptions is never nil. + g.Expect(metricsOptions).ToNot(BeNil()) + + g.Expect(tlsConfig).To(Equal(tt.wantTLSConfig)) + // There is no way we can compare the tlsOpts functions with equal + metricsOptions.TLSOpts = nil + // There is no way we can compare the FilterProvider functions with equal + g.Expect(metricsOptions.FilterProvider == nil).To(Equal(tt.wantMetricsOptions.FilterProvider == nil)) + metricsOptions.FilterProvider = nil + tt.wantMetricsOptions.FilterProvider = nil + // There is no way we can compare the handler funcs with equal + for k := range metricsOptions.ExtraHandlers { + metricsOptions.ExtraHandlers[k] = nil + } + for k := range tt.wantMetricsOptions.ExtraHandlers { + tt.wantMetricsOptions.ExtraHandlers[k] = nil + } + g.Expect(metricsOptions).To(Equal(tt.wantMetricsOptions)) + g.Expect(metricsOptionsTLSConfig).To(Equal(tt.wantMetricsOptionsTLSConfig)) + }) + } +} diff --git a/util/flags/tls.go b/util/flags/tls.go deleted file mode 100644 index 631b23b7af36..000000000000 --- a/util/flags/tls.go +++ /dev/null @@ -1,75 +0,0 @@ -/* -Copyright 2022 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 flags implements the webhook server TLS options utilities. -package flags - -import ( - "crypto/tls" - "fmt" - "strings" - - "github.com/spf13/pflag" - cliflag "k8s.io/component-base/cli/flag" -) - -// TLSOptions has the options to configure the TLS settings -// for a webhook server. -type TLSOptions struct { - TLSMinVersion string - TLSCipherSuites []string -} - -// AddTLSOptions adds the webhook server TLS configuration flags -// to the flag set. -func AddTLSOptions(fs *pflag.FlagSet, options *TLSOptions) { - fs.StringVar(&options.TLSMinVersion, "tls-min-version", "VersionTLS12", - "The minimum TLS version in use by the webhook server.\n"+ - fmt.Sprintf("Possible values are %s.", strings.Join(cliflag.TLSPossibleVersions(), ", ")), - ) - - tlsCipherPreferredValues := cliflag.PreferredTLSCipherNames() - tlsCipherInsecureValues := cliflag.InsecureTLSCipherNames() - fs.StringSliceVar(&options.TLSCipherSuites, "tls-cipher-suites", []string{}, - "Comma-separated list of cipher suites for the webhook server. "+ - "If omitted, the default Go cipher suites will be used. \n"+ - "Preferred values: "+strings.Join(tlsCipherPreferredValues, ", ")+". \n"+ - "Insecure values: "+strings.Join(tlsCipherInsecureValues, ", ")+".") -} - -// GetTLSOptionOverrideFuncs returns a list of TLS configuration overrides to be used -// by the webhook server. -func GetTLSOptionOverrideFuncs(options TLSOptions) ([]func(*tls.Config), error) { - var tlsOptions []func(config *tls.Config) - tlsVersion, err := cliflag.TLSVersion(options.TLSMinVersion) - if err != nil { - return nil, err - } - tlsOptions = append(tlsOptions, func(cfg *tls.Config) { - cfg.MinVersion = tlsVersion - }) - - if len(options.TLSCipherSuites) != 0 { - suites, err := cliflag.TLSCipherSuites(options.TLSCipherSuites) - if err != nil { - return nil, err - } - tlsOptions = append(tlsOptions, func(cfg *tls.Config) { - cfg.CipherSuites = suites - }) - } - return tlsOptions, nil -}