From 73a696ddb33fb57aecacbabf025e824d35ccf682 Mon Sep 17 00:00:00 2001 From: irbekrm Date: Mon, 25 Oct 2021 07:07:22 +0300 Subject: [PATCH] Pprof addr for webhook defaults to localhost Also whether it is enabled and the address can now be configured via commandline flags Signed-off-by: irbekrm --- cmd/util/defaults.go | 3 ++ cmd/webhook/app/options/BUILD.bazel | 1 + cmd/webhook/app/options/options.go | 26 +++++++++++-- cmd/webhook/app/webhook.go | 3 +- pkg/webhook/server/server.go | 60 +++++++++++++++++++++-------- 5 files changed, 74 insertions(+), 19 deletions(-) diff --git a/cmd/util/defaults.go b/cmd/util/defaults.go index 6bb2c40dc..d3d0bf60e 100644 --- a/cmd/util/defaults.go +++ b/cmd/util/defaults.go @@ -26,4 +26,7 @@ const ( DefaultLeaderElectionLeaseDuration = 60 * time.Second DefaultLeaderElectionRenewDeadline = 40 * time.Second DefaultLeaderElectionRetryPeriod = 15 * time.Second + + DefaultEnableProfiling = false + DefaultProfilerAddr = "localhost:6060" ) diff --git a/cmd/webhook/app/options/BUILD.bazel b/cmd/webhook/app/options/BUILD.bazel index 168810fad..1d01de9a4 100644 --- a/cmd/webhook/app/options/BUILD.bazel +++ b/cmd/webhook/app/options/BUILD.bazel @@ -6,6 +6,7 @@ go_library( importpath = "github.com/jetstack/cert-manager/cmd/webhook/app/options", visibility = ["//visibility:public"], deps = [ + "//cmd/util:go_default_library", "@com_github_spf13_pflag//:go_default_library", "@io_k8s_component_base//cli/flag:go_default_library", ], diff --git a/cmd/webhook/app/options/options.go b/cmd/webhook/app/options/options.go index 9693daed0..f94efa15a 100644 --- a/cmd/webhook/app/options/options.go +++ b/cmd/webhook/app/options/options.go @@ -21,6 +21,15 @@ import ( "github.com/spf13/pflag" cliflag "k8s.io/component-base/cli/flag" + + cmdutil "github.com/jetstack/cert-manager/cmd/util" +) + +const ( + // Default port on which /validate, /mutate, /convert endpoints will be served + defaultListeningPort = 6443 + // Default health check port + defaultHealthPort = 6080 ) type WebhookOptions struct { @@ -56,12 +65,19 @@ type WebhookOptions struct { // MinTLSVersion is the minimum TLS version supported. // Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). MinTLSVersion string + + // EnablePprof determines whether pprof is enabled. + EnablePprof bool + + // Address on which /debug/pprof endpoint will be served if enabled. Default is + // localhost:6060. + PprofAddress string } func (o *WebhookOptions) AddFlags(fs *pflag.FlagSet) { // TODO: rename secure-port to listen-port - fs.IntVar(&o.ListenPort, "secure-port", 6443, "port number to listen on for secure TLS connections") - fs.IntVar(&o.HealthzPort, "healthz-port", 6080, "port number to listen on for insecure healthz connections") + fs.IntVar(&o.ListenPort, "secure-port", defaultListeningPort, "port number to listen on for secure TLS connections") + fs.IntVar(&o.HealthzPort, "healthz-port", defaultHealthPort, "port number to listen on for insecure healthz connections") fs.StringVar(&o.TLSCertFile, "tls-cert-file", "", "path to the file containing the TLS certificate to serve with") fs.StringVar(&o.TLSKeyFile, "tls-private-key-file", "", "path to the file containing the TLS private key to serve with") fs.StringVar(&o.DynamicServingCASecretNamespace, "dynamic-serving-ca-secret-namespace", "", "namespace of the secret used to store the CA that signs serving certificates") @@ -71,7 +87,10 @@ func (o *WebhookOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.APIServerHost, "api-server-host", "", ""+ "Optional apiserver host address to connect to. If not specified, autoconfiguration "+ "will be attempted.") - + fs.BoolVar(&o.EnablePprof, "enable-profiling", cmdutil.DefaultEnableProfiling, ""+ + "Enable profiling for controller.") + fs.StringVar(&o.PprofAddress, "profiler-address", cmdutil.DefaultProfilerAddr, + "Address of the Go profiler (pprof). This should never be exposed on a public interface. If this flag is not set, the profiler is not run.") tlsCipherPossibleValues := cliflag.TLSCipherPossibleValues() fs.StringSliceVar(&o.TLSCipherSuites, "tls-cipher-suites", o.TLSCipherSuites, "Comma-separated list of cipher suites for the server. "+ @@ -81,6 +100,7 @@ func (o *WebhookOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.MinTLSVersion, "tls-min-version", o.MinTLSVersion, "Minimum TLS version supported. "+ "Possible values: "+strings.Join(tlsPossibleVersions, ", ")) + } func FileTLSSourceEnabled(o WebhookOptions) bool { diff --git a/cmd/webhook/app/webhook.go b/cmd/webhook/app/webhook.go index 71936070e..bf2c56791 100644 --- a/cmd/webhook/app/webhook.go +++ b/cmd/webhook/app/webhook.go @@ -85,7 +85,8 @@ func NewServerWithOptions(log logr.Logger, opts options.WebhookOptions) (*server return &server.Server{ ListenAddr: fmt.Sprintf(":%d", opts.ListenPort), HealthzAddr: fmt.Sprintf(":%d", opts.HealthzPort), - EnablePprof: true, + PprofAddr: opts.PprofAddress, + EnablePprof: opts.EnablePprof, CertificateSource: source, CipherSuites: opts.TLSCipherSuites, MinTLSVersion: opts.MinTLSVersion, diff --git a/pkg/webhook/server/server.go b/pkg/webhook/server/server.go index 9e2b63b7c..d1eb20c77 100644 --- a/pkg/webhook/server/server.go +++ b/pkg/webhook/server/server.go @@ -84,8 +84,9 @@ type Server struct { // If not specified, the healthz endpoint will not be exposed. HealthzAddr string - // EnablePprof controls whether net/http/pprof handlers are registered with - // the HTTP listener. + // PprofAddr is the address the pprof endpoint should be served on if enabled. + PprofAddr string + // EnablePprof determines whether pprof is enabled. EnablePprof bool // Scheme is used to decode/encode request/response payloads. @@ -134,12 +135,12 @@ func (s *Server) Run(stopCh <-chan struct{}) error { return err } - mux := http.NewServeMux() - mux.HandleFunc("/healthz", s.handleHealthz) - mux.HandleFunc("/livez", s.handleLivez) + healthMux := http.NewServeMux() + healthMux.HandleFunc("/healthz", s.handleHealthz) + healthMux.HandleFunc("/livez", s.handleLivez) s.Log.V(logf.InfoLevel).Info("listening for insecure healthz connections", "address", s.HealthzAddr) server := &http.Server{ - Handler: mux, + Handler: healthMux, } g.Go(func() error { <-gctx.Done() @@ -160,6 +161,39 @@ func (s *Server) Run(stopCh <-chan struct{}) error { }) } + // if a PprofAddr is provided, start the pprof listener + if s.EnablePprof { + pprofListener, err := net.Listen("tcp", s.PprofAddr) + if err != nil { + return err + } + + profilerMux := http.NewServeMux() + // Add pprof endpoints to this mux + profiling.Install(profilerMux) + s.Log.V(logf.InfoLevel).Info("running go profiler on", "address", s.PprofAddr) + server := &http.Server{ + Handler: profilerMux, + } + g.Go(func() error { + <-gctx.Done() + // allow a timeout for graceful shutdown + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + if err := server.Shutdown(ctx); err != nil { + return err + } + return nil + }) + g.Go(func() error { + if err := server.Serve(pprofListener); err != http.ErrServerClosed { + return err + } + return nil + }) + } + // create a listener for actual webhook requests listener, err := net.Listen("tcp", s.ListenAddr) if err != nil { @@ -194,16 +228,12 @@ func (s *Server) Run(stopCh <-chan struct{}) error { } s.listener = listener - mux := http.NewServeMux() - mux.HandleFunc("/validate", s.handle(s.validate)) - mux.HandleFunc("/mutate", s.handle(s.mutate)) - mux.HandleFunc("/convert", s.handle(s.convert)) - if s.EnablePprof { - profiling.Install(mux) - s.Log.V(logf.InfoLevel).Info("registered pprof handlers") - } + serverMux := http.NewServeMux() + serverMux.HandleFunc("/validate", s.handle(s.validate)) + serverMux.HandleFunc("/mutate", s.handle(s.mutate)) + serverMux.HandleFunc("/convert", s.handle(s.convert)) server := &http.Server{ - Handler: mux, + Handler: serverMux, } g.Go(func() error { <-gctx.Done()