diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 7d00c3c4b0..e7818b0386 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -28,12 +28,11 @@ import ( "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" - "k8s.io/utils/pointer" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" - logf "sigs.k8s.io/controller-runtime/pkg/internal/log" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + logf "sigs.k8s.io/controller-runtime/pkg/internal/log" intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder" ) @@ -95,17 +94,9 @@ type Options struct { // value only if you know what you are doing. Defaults to 10 hours if unset. // there will a 10 percent jitter between the SyncPeriod of all controllers // so that all controllers will not send list requests simultaneously. - SyncPeriod *time.Duration - - // Namespace if specified restricts the manager's cache to watch objects in - // the desired namespace Defaults to all namespaces // - // Note: If a namespace is specified, controllers can still Watch for a - // cluster-scoped resource (e.g Node). For namespaced resources the cache - // will only hold objects from the desired namespace. - // - // Deprecated: Use Cache.Namespaces instead. - Namespace string + // Deprecated: Use Cache.SyncPeriod instead. + SyncPeriod *time.Duration // HTTPClient is the http client that will be used to create the default // Cache and Client. If not set the rest.HTTPClientFor function will be used @@ -141,18 +132,6 @@ type Options struct { // Only use a custom NewClient if you know what you are doing. NewClient client.NewClientFunc - // ClientDisableCacheFor tells the client that, if any cache is used, to bypass it - // for the given objects. - // - // Deprecated: Use Client.Cache.DisableFor instead. - ClientDisableCacheFor []client.Object - - // DryRunClient specifies whether the client should be configured to enforce - // dryRun mode. - // - // Deprecated: Use Client.DryRun instead. - DryRunClient bool - // EventBroadcaster records Events emitted by the manager and sends them to the Kubernetes API // Use this to customize the event correlator and spam filter // @@ -211,9 +190,6 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) { if cacheOpts.SyncPeriod == nil { cacheOpts.SyncPeriod = options.SyncPeriod } - if len(cacheOpts.Namespaces) == 0 && options.Namespace != "" { - cacheOpts.Namespaces = []string{options.Namespace} - } } cache, err := options.NewCache(config, cacheOpts) if err != nil { @@ -240,16 +216,6 @@ func New(config *rest.Config, opts ...Option) (Cluster, error) { if clientOpts.Cache.Reader == nil { clientOpts.Cache.Reader = cache } - - // For backward compatibility, the ClientDisableCacheFor option should - // be appended to the DisableFor option in the client. - clientOpts.Cache.DisableFor = append(clientOpts.Cache.DisableFor, options.ClientDisableCacheFor...) - - if clientOpts.DryRun == nil && options.DryRunClient { - // For backward compatibility, the DryRunClient (if set) option should override - // the DryRun option in the client (if unset). - clientOpts.DryRun = pointer.Bool(true) - } } clientWriter, err := options.NewClient(config, clientOpts) if err != nil { diff --git a/pkg/envtest/webhook_test.go b/pkg/envtest/webhook_test.go index 3a87052580..79a5227088 100644 --- a/pkg/envtest/webhook_test.go +++ b/pkg/envtest/webhook_test.go @@ -29,6 +29,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -40,12 +41,12 @@ var _ = Describe("Test", func() { Describe("Webhook", func() { It("should reject create request for webhook that rejects all requests", func() { m, err := manager.New(env.Config, manager.Options{ - Port: env.WebhookInstallOptions.LocalServingPort, - Host: env.WebhookInstallOptions.LocalServingHost, - CertDir: env.WebhookInstallOptions.LocalServingCertDir, - TLSOpts: []func(*tls.Config){ - func(config *tls.Config) {}, - }, + WebhookServer: webhook.NewServer(webhook.Options{ + Port: env.WebhookInstallOptions.LocalServingPort, + Host: env.WebhookInstallOptions.LocalServingHost, + CertDir: env.WebhookInstallOptions.LocalServingCertDir, + TLSOpts: []func(*tls.Config){func(config *tls.Config) {}}, + }), }) // we need manager here just to leverage manager.SetFields Expect(err).NotTo(HaveOccurred()) server := m.GetWebhookServer() diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 04e6d94cb2..360c9a2a44 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -18,7 +18,6 @@ package manager import ( "context" - "crypto/tls" "fmt" "net" "net/http" @@ -140,35 +139,6 @@ type Options struct { // Only use a custom NewClient if you know what you are doing. NewClient client.NewClientFunc - // SyncPeriod determines the minimum frequency at which watched resources are - // reconciled. A lower period will correct entropy more quickly, but reduce - // responsiveness to change if there are many watched resources. Change this - // value only if you know what you are doing. Defaults to 10 hours if unset. - // there will a 10 percent jitter between the SyncPeriod of all controllers - // so that all controllers will not send list requests simultaneously. - // - // This applies to all controllers. - // - // A period sync happens for two reasons: - // 1. To insure against a bug in the controller that causes an object to not - // be requeued, when it otherwise should be requeued. - // 2. To insure against an unknown bug in controller-runtime, or its dependencies, - // that causes an object to not be requeued, when it otherwise should be - // requeued, or to be removed from the queue, when it otherwise should not - // be removed. - // - // If you want - // 1. to insure against missed watch events, or - // 2. to poll services that cannot be watched, - // then we recommend that, instead of changing the default period, the - // controller requeue, with a constant duration `t`, whenever the controller - // is "done" with an object, and would otherwise not requeue it, i.e., we - // recommend the `Reconcile` function return `reconcile.Result{RequeueAfter: t}`, - // instead of `reconcile.Result{}`. - // - // Deprecated: Use Cache.SyncPeriod instead. - SyncPeriod *time.Duration - // Logger is the logger that should be used by this manager. // If none is set, it defaults to log.Log global logger. Logger logr.Logger @@ -239,23 +209,15 @@ type Options struct { // wait to force acquire leadership. This is measured against time of // last observed ack. Default is 15 seconds. LeaseDuration *time.Duration + // RenewDeadline is the duration that the acting controlplane will retry // refreshing leadership before giving up. Default is 10 seconds. RenewDeadline *time.Duration + // RetryPeriod is the duration the LeaderElector clients should wait // between tries of actions. Default is 2 seconds. RetryPeriod *time.Duration - // Namespace, if specified, restricts the manager's cache to watch objects in - // the desired namespace. Defaults to all namespaces. - // - // Note: If a namespace is specified, controllers can still Watch for a - // cluster-scoped resource (e.g Node). For namespaced resources, the cache - // will only hold objects from the desired namespace. - // - // Deprecated: Use Cache.Namespaces instead. - Namespace string - // MetricsBindAddress is the TCP address that the controller should bind to // for serving prometheus metrics. // It can be set to "0" to disable the metrics serving. @@ -279,31 +241,6 @@ type Options struct { // before exposing it to public. PprofBindAddress string - // Port is the port that the webhook server serves at. - // It is used to set webhook.Server.Port if WebhookServer is not set. - // - // Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer. - Port int - // Host is the hostname that the webhook server binds to. - // It is used to set webhook.Server.Host if WebhookServer is not set. - // - // Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer. - Host string - - // CertDir is the directory that contains the server key and certificate. - // If not set, webhook server would look up the server key and certificate in - // {TempDir}/k8s-webhook-server/serving-certs. The server key and certificate - // must be named tls.key and tls.crt, respectively. - // It is used to set webhook.Server.CertDir if WebhookServer is not set. - // - // Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer. - CertDir string - - // TLSOpts is used to allow configuring the TLS config used for the webhook server. - // - // Deprecated: Use WebhookServer instead. A WebhookServer can be created via webhook.NewServer. - TLSOpts []func(*tls.Config) - // WebhookServer is an externally configured webhook.Server. By default, // a Manager will create a default server using Port, Host, and CertDir; // if this is set, the Manager will use this server instead. @@ -314,18 +251,6 @@ type Options struct { // will receive a new Background Context instead. BaseContext BaseContextFunc - // ClientDisableCacheFor tells the client that, if any cache is used, to bypass it - // for the given objects. - // - // Deprecated: Use Client.Cache.DisableCacheFor instead. - ClientDisableCacheFor []client.Object - - // DryRunClient specifies whether the client should be configured to enforce - // dryRun mode. - // - // Deprecated: Use Client.DryRun instead. - DryRunClient bool - // EventBroadcaster records Events emitted by the manager and sends them to the Kubernetes API // Use this to customize the event correlator and spam filter // @@ -401,15 +326,11 @@ func New(config *rest.Config, options Options) (Manager, error) { clusterOptions.Scheme = options.Scheme clusterOptions.MapperProvider = options.MapperProvider clusterOptions.Logger = options.Logger - clusterOptions.SyncPeriod = options.SyncPeriod clusterOptions.NewCache = options.NewCache clusterOptions.NewClient = options.NewClient clusterOptions.Cache = options.Cache clusterOptions.Client = options.Client - clusterOptions.Namespace = options.Namespace //nolint:staticcheck - clusterOptions.ClientDisableCacheFor = options.ClientDisableCacheFor //nolint:staticcheck - clusterOptions.DryRunClient = options.DryRunClient //nolint:staticcheck - clusterOptions.EventBroadcaster = options.EventBroadcaster //nolint:staticcheck + clusterOptions.EventBroadcaster = options.EventBroadcaster //nolint:staticcheck }) if err != nil { return nil, err @@ -526,12 +447,12 @@ func (o Options) AndFrom(loader config.ControllerManagerConfiguration) (Options, o = o.setLeaderElectionConfig(newObj) - if o.SyncPeriod == nil && newObj.SyncPeriod != nil { - o.SyncPeriod = &newObj.SyncPeriod.Duration + if o.Cache.SyncPeriod == nil && newObj.SyncPeriod != nil { + o.Cache.SyncPeriod = &newObj.SyncPeriod.Duration } - if o.Namespace == "" && newObj.CacheNamespace != "" { - o.Namespace = newObj.CacheNamespace + if len(o.Cache.Namespaces) == 0 && newObj.CacheNamespace != "" { + o.Cache.Namespaces = []string{newObj.CacheNamespace} } if o.MetricsBindAddress == "" && newObj.Metrics.BindAddress != "" { @@ -550,20 +471,15 @@ func (o Options) AndFrom(loader config.ControllerManagerConfiguration) (Options, o.LivenessEndpointName = newObj.Health.LivenessEndpointName } - if o.Port == 0 && newObj.Webhook.Port != nil { - o.Port = *newObj.Webhook.Port - } - if o.Host == "" && newObj.Webhook.Host != "" { - o.Host = newObj.Webhook.Host - } - if o.CertDir == "" && newObj.Webhook.CertDir != "" { - o.CertDir = newObj.Webhook.CertDir - } if o.WebhookServer == nil { + port := 0 + if newObj.Webhook.Port != nil { + port = *newObj.Webhook.Port + } o.WebhookServer = webhook.NewServer(webhook.Options{ - Port: o.Port, - Host: o.Host, - CertDir: o.CertDir, + Port: port, + Host: newObj.Webhook.Host, + CertDir: newObj.Webhook.CertDir, }) } @@ -737,12 +653,7 @@ func setOptionsDefaults(options Options) Options { } if options.WebhookServer == nil { - options.WebhookServer = webhook.NewServer(webhook.Options{ - Host: options.Host, - Port: options.Port, - CertDir: options.CertDir, - TLSOpts: options.TLSOpts, - }) + options.WebhookServer = webhook.NewServer(webhook.Options{}) } return options diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 0186f46f8b..dd6d3b2470 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -156,7 +156,7 @@ var _ = Describe("manger.Manager", func() { m, err := Options{}.AndFrom(&fakeDeferredLoader{ccfg}) Expect(err).ToNot(HaveOccurred()) - Expect(*m.SyncPeriod).To(Equal(duration.Duration)) + Expect(*m.Cache.SyncPeriod).To(Equal(duration.Duration)) Expect(m.LeaderElection).To(Equal(leaderElect)) Expect(m.LeaderElectionResourceLock).To(Equal("leases")) Expect(m.LeaderElectionNamespace).To(Equal("default")) @@ -164,14 +164,14 @@ var _ = Describe("manger.Manager", func() { Expect(m.LeaseDuration.String()).To(Equal(duration.Duration.String())) Expect(m.RenewDeadline.String()).To(Equal(duration.Duration.String())) Expect(m.RetryPeriod.String()).To(Equal(duration.Duration.String())) - Expect(m.Namespace).To(Equal("default")) + Expect(m.Cache.Namespaces).To(Equal([]string{"default"})) Expect(m.MetricsBindAddress).To(Equal(":6000")) Expect(m.HealthProbeBindAddress).To(Equal("6060")) Expect(m.ReadinessEndpointName).To(Equal("/readyz")) Expect(m.LivenessEndpointName).To(Equal("/livez")) - Expect(m.Port).To(Equal(port)) - Expect(m.Host).To(Equal("localhost")) - Expect(m.CertDir).To(Equal("/certs")) + Expect(m.WebhookServer.(*webhook.DefaultServer).Options.Port).To(Equal(port)) + Expect(m.WebhookServer.(*webhook.DefaultServer).Options.Host).To(Equal("localhost")) + Expect(m.WebhookServer.(*webhook.DefaultServer).Options.CertDir).To(Equal("/certs")) }) It("should be able to keep Options when cfg.ControllerManagerConfiguration set", func() { @@ -213,7 +213,10 @@ var _ = Describe("manger.Manager", func() { func(config *tls.Config) {}, } m, err := Options{ - SyncPeriod: &optDuration, + Cache: cache.Options{ + SyncPeriod: &optDuration, + Namespaces: []string{"ctrl"}, + }, LeaderElection: true, LeaderElectionResourceLock: "configmaps", LeaderElectionNamespace: "ctrl", @@ -221,7 +224,6 @@ var _ = Describe("manger.Manager", func() { LeaseDuration: &optDuration, RenewDeadline: &optDuration, RetryPeriod: &optDuration, - Namespace: "ctrl", MetricsBindAddress: ":7000", HealthProbeBindAddress: "5000", ReadinessEndpointName: "/readiness", @@ -235,7 +237,7 @@ var _ = Describe("manger.Manager", func() { }.AndFrom(&fakeDeferredLoader{ccfg}) Expect(err).ToNot(HaveOccurred()) - Expect(m.SyncPeriod.String()).To(Equal(optDuration.String())) + Expect(m.Cache.SyncPeriod.String()).To(Equal(optDuration.String())) Expect(m.LeaderElection).To(BeTrue()) Expect(m.LeaderElectionResourceLock).To(Equal("configmaps")) Expect(m.LeaderElectionNamespace).To(Equal("ctrl")) @@ -243,7 +245,7 @@ var _ = Describe("manger.Manager", func() { Expect(m.LeaseDuration.String()).To(Equal(optDuration.String())) Expect(m.RenewDeadline.String()).To(Equal(optDuration.String())) Expect(m.RetryPeriod.String()).To(Equal(optDuration.String())) - Expect(m.Namespace).To(Equal("ctrl")) + Expect(m.Cache.Namespaces).To(Equal([]string{"ctrl"})) Expect(m.MetricsBindAddress).To(Equal(":7000")) Expect(m.HealthProbeBindAddress).To(Equal("5000")) Expect(m.ReadinessEndpointName).To(Equal("/readiness")) @@ -267,23 +269,10 @@ var _ = Describe("manger.Manager", func() { Expect(svr.(*webhook.DefaultServer).Options.Host).To(Equal("foo.com")) }) - It("should lazily initialize a webhook server if needed (deprecated)", func() { - By("creating a manager with options") - m, err := New(cfg, Options{Port: 9440, Host: "foo.com"}) - Expect(err).NotTo(HaveOccurred()) - Expect(m).NotTo(BeNil()) - - By("checking options are passed to the webhook server") - svr := m.GetWebhookServer() - Expect(svr).NotTo(BeNil()) - Expect(svr.(*webhook.DefaultServer).Options.Port).To(Equal(9440)) - Expect(svr.(*webhook.DefaultServer).Options.Host).To(Equal("foo.com")) - }) - It("should not initialize a webhook server if Options.WebhookServer is set", func() { By("creating a manager with options") srv := webhook.NewServer(webhook.Options{Port: 9440}) - m, err := New(cfg, Options{Port: 9441, WebhookServer: srv}) + m, err := New(cfg, Options{WebhookServer: srv}) Expect(err).NotTo(HaveOccurred()) Expect(m).NotTo(BeNil()) diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index 23d5bf4350..8613737798 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -95,11 +95,6 @@ type Options struct { // Defaults to "", which means server does not verify client's certificate. ClientCAName string - // TLSVersion is the minimum version of TLS supported. Accepts - // "", "1.0", "1.1", "1.2" and "1.3" only ("" is equivalent to "1.0" for backwards compatibility) - // Deprecated: Use TLSOpts instead. - TLSMinVersion string - // TLSOpts is used to allow configuring the TLS config used for the server TLSOpts []func(*tls.Config) @@ -187,26 +182,6 @@ func (s *DefaultServer) Register(path string, hook http.Handler) { regLog.Info("Registering webhook") } -// tlsVersion converts from human-readable TLS version (for example "1.1") -// to the values accepted by tls.Config (for example 0x301). -func tlsVersion(version string) (uint16, error) { - switch version { - // default is previous behaviour - case "": - return tls.VersionTLS10, nil - case "1.0": - return tls.VersionTLS10, nil - case "1.1": - return tls.VersionTLS11, nil - case "1.2": - return tls.VersionTLS12, nil - case "1.3": - return tls.VersionTLS13, nil - default: - return 0, fmt.Errorf("invalid TLSMinVersion %v: expects 1.0, 1.1, 1.2, 1.3 or empty", version) - } -} - // Start runs the server. // It will install the webhook related resources depend on the server configuration. func (s *DefaultServer) Start(ctx context.Context) error { @@ -215,14 +190,8 @@ func (s *DefaultServer) Start(ctx context.Context) error { baseHookLog := log.WithName("webhooks") baseHookLog.Info("Starting webhook server") - tlsMinVersion, err := tlsVersion(s.Options.TLSMinVersion) - if err != nil { - return err - } - cfg := &tls.Config{ //nolint:gosec NextProtos: []string{"h2"}, - MinVersion: tlsMinVersion, } // fallback TLS config ready, will now mutate if passer wants full control over it for _, op := range s.Options.TLSOpts { diff --git a/pkg/webhook/server_test.go b/pkg/webhook/server_test.go index 9702b0fd6e..04d4ac7f86 100644 --- a/pkg/webhook/server_test.go +++ b/pkg/webhook/server_test.go @@ -29,6 +29,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/webhook" ) @@ -169,14 +170,14 @@ var _ = Describe("Webhook Server", func() { tls.TLS_AES_128_GCM_SHA256, tls.TLS_AES_256_GCM_SHA384, } + cfg.MinVersion = tls.VersionTLS12 // save cfg after changes to test against finalCfg = cfg } server = webhook.NewServer(webhook.Options{ - Host: servingOpts.LocalServingHost, - Port: servingOpts.LocalServingPort, - CertDir: servingOpts.LocalServingCertDir, - TLSMinVersion: "1.2", + Host: servingOpts.LocalServingHost, + Port: servingOpts.LocalServingPort, + CertDir: servingOpts.LocalServingCertDir, TLSOpts: []func(*tls.Config){ tlsCfgFunc, }, @@ -213,13 +214,14 @@ var _ = Describe("Webhook Server", func() { return &finalCert, nil } server = &webhook.DefaultServer{Options: webhook.Options{ - Host: servingOpts.LocalServingHost, - Port: servingOpts.LocalServingPort, - CertDir: servingOpts.LocalServingCertDir, - TLSMinVersion: "1.2", + Host: servingOpts.LocalServingHost, + Port: servingOpts.LocalServingPort, + CertDir: servingOpts.LocalServingCertDir, + TLSOpts: []func(*tls.Config){ func(cfg *tls.Config) { cfg.GetCertificate = finalGetCertificate + cfg.MinVersion = tls.VersionTLS12 // save cfg after changes to test against finalCfg = cfg }, diff --git a/pkg/webhook/webhook_integration_test.go b/pkg/webhook/webhook_integration_test.go index 716692124b..6bce47621e 100644 --- a/pkg/webhook/webhook_integration_test.go +++ b/pkg/webhook/webhook_integration_test.go @@ -28,6 +28,7 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -73,10 +74,12 @@ var _ = Describe("Webhook", func() { Context("when running a webhook server with a manager", func() { It("should reject create request for webhook that rejects all requests", func() { m, err := manager.New(cfg, manager.Options{ - Port: testenv.WebhookInstallOptions.LocalServingPort, - Host: testenv.WebhookInstallOptions.LocalServingHost, - CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, - TLSOpts: []func(*tls.Config){func(config *tls.Config) {}}, + WebhookServer: webhook.NewServer(webhook.Options{ + Port: testenv.WebhookInstallOptions.LocalServingPort, + Host: testenv.WebhookInstallOptions.LocalServingHost, + CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, + TLSOpts: []func(*tls.Config){func(config *tls.Config) {}}, + }), }) // we need manager here just to leverage manager.SetFields Expect(err).NotTo(HaveOccurred()) server := m.GetWebhookServer() @@ -98,10 +101,12 @@ var _ = Describe("Webhook", func() { It("should reject create request for multi-webhook that rejects all requests", func() { m, err := manager.New(cfg, manager.Options{ MetricsBindAddress: "0", - Port: testenv.WebhookInstallOptions.LocalServingPort, - Host: testenv.WebhookInstallOptions.LocalServingHost, - CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, - TLSOpts: []func(*tls.Config){func(config *tls.Config) {}}, + WebhookServer: webhook.NewServer(webhook.Options{ + Port: testenv.WebhookInstallOptions.LocalServingPort, + Host: testenv.WebhookInstallOptions.LocalServingHost, + CertDir: testenv.WebhookInstallOptions.LocalServingCertDir, + TLSOpts: []func(*tls.Config){func(config *tls.Config) {}}, + }), }) // we need manager here just to leverage manager.SetFields Expect(err).NotTo(HaveOccurred()) server := m.GetWebhookServer()