From 5c909a0e0c784a76d1af9c88ca9f78ec3c1419dd Mon Sep 17 00:00:00 2001 From: Danil-Grigorev Date: Thu, 11 Jun 2020 10:43:07 +0200 Subject: [PATCH] Add metrics TLS serving option --- .../internal/certwatcher/certwatcher.go | 0 pkg/manager/manager.go | 33 +++++-- pkg/manager/manager_test.go | 90 +++++++++++++++++++ pkg/manager/testdata/certs/tls.crt | 18 ++++ pkg/manager/testdata/certs/tls.key | 27 ++++++ pkg/metrics/listener.go | 46 +++++++++- pkg/webhook/server.go | 3 +- 7 files changed, 207 insertions(+), 10 deletions(-) rename pkg/{webhook => }/internal/certwatcher/certwatcher.go (100%) create mode 100644 pkg/manager/testdata/certs/tls.crt create mode 100644 pkg/manager/testdata/certs/tls.key diff --git a/pkg/webhook/internal/certwatcher/certwatcher.go b/pkg/internal/certwatcher/certwatcher.go similarity index 100% rename from pkg/webhook/internal/certwatcher/certwatcher.go rename to pkg/internal/certwatcher/certwatcher.go diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 1526cef476..f18636ea99 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -164,6 +164,12 @@ type Options struct { // It can be set to "0" to disable the metrics serving. MetricsBindAddress string + // MetricsCertDir is the directory that contains the secure metrics server key and certificate. + // When is set, server would be served over TLS and look up the server key and certificate in + // {MetricsCertDir}/tls.key and {MetricsCertDir}/tls.crt. The server key and certificate + // must be named tls.key and tls.crt, respectively. + MetricsCertDir string + // HealthProbeBindAddress is the TCP address that the controller should bind to // for serving health probes HealthProbeBindAddress string @@ -206,9 +212,11 @@ type Options struct { EventBroadcaster record.EventBroadcaster // Dependency injection for testing - newRecorderProvider func(config *rest.Config, scheme *runtime.Scheme, logger logr.Logger, broadcaster record.EventBroadcaster) (recorder.Provider, error) - newResourceLock func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) - newMetricsListener func(addr string) (net.Listener, error) + newRecorderProvider func(config *rest.Config, scheme *runtime.Scheme, logger logr.Logger, broadcaster record.EventBroadcaster) (recorder.Provider, error) + newResourceLock func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error) + newMetricsListener func(addr string) (net.Listener, error) + newSecureMetricsListener func(addr, certDir string) (net.Listener, error) + newHealthProbeListener func(addr string) (net.Listener, error) } @@ -299,9 +307,17 @@ func New(config *rest.Config, options Options) (Manager, error) { // Create the metrics listener. This will throw an error if the metrics bind // address is invalid or already in use. - metricsListener, err := options.newMetricsListener(options.MetricsBindAddress) - if err != nil { - return nil, err + var metricsListener net.Listener + if options.MetricsCertDir == "" { + metricsListener, err = options.newMetricsListener(options.MetricsBindAddress) + if err != nil { + return nil, err + } + } else { + metricsListener, err = options.newSecureMetricsListener(options.MetricsBindAddress, options.MetricsCertDir) + if err != nil { + return nil, err + } } // By default we have no extra endpoints to expose on metrics http server. @@ -410,6 +426,11 @@ func setOptionsDefaults(options Options) Options { if options.newMetricsListener == nil { options.newMetricsListener = metrics.NewListener } + + if options.newSecureMetricsListener == nil { + options.newSecureMetricsListener = metrics.NewSecureListener + } + leaseDuration, renewDeadline, retryPeriod := defaultLeaseDuration, defaultRenewDeadline, defaultRetryPeriod if options.LeaseDuration == nil { options.LeaseDuration = &leaseDuration diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 0788c5a972..d7c161dbe9 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -17,10 +17,12 @@ limitations under the License. package manager import ( + "crypto/tls" "fmt" "io/ioutil" "net" "net/http" + "path/filepath" rt "runtime" "github.com/go-logr/logr" @@ -233,6 +235,24 @@ var _ = Describe("manger.Manager", func() { Expect(listener.Close()).ToNot(HaveOccurred()) }) + It("should create a TLS listener for the metrics if a certificates are provided and the address is correct", func() { + var listener net.Listener + certDir := filepath.Join(".", "testdata", "certs") + m, err := New(cfg, Options{ + MetricsBindAddress: ":0", + MetricsCertDir: certDir, + newSecureMetricsListener: func(addr, certDir string) (net.Listener, error) { + var err error + listener, err = metrics.NewSecureListener(addr, certDir) + return listener, err + }, + }) + Expect(m).ToNot(BeNil()) + Expect(err).ToNot(HaveOccurred()) + Expect(listener).ToNot(BeNil()) + Expect(listener.Close()).ToNot(HaveOccurred()) + }) + It("should return an error if the metrics bind address is already in use", func() { ln, err := metrics.NewListener(":0") Expect(err).ShouldNot(HaveOccurred()) @@ -559,6 +579,76 @@ var _ = Describe("manger.Manager", func() { }) }) + Context("should start serving metrics over TLS", func() { + var listener net.Listener + var opts Options + + BeforeEach(func() { + listener = nil + opts = Options{ + MetricsCertDir: filepath.Join(".", "testdata", "certs"), + newSecureMetricsListener: func(addr, certDir string) (net.Listener, error) { + var err error + listener, err = metrics.NewSecureListener(addr, certDir) + return listener, err + }, + } + }) + + AfterEach(func() { + if listener != nil { + listener.Close() + } + }) + + It("should serve secure metrics endpoint", func(done Done) { + opts.MetricsBindAddress = ":0" + m, err := New(cfg, opts) + Expect(err).NotTo(HaveOccurred()) + + s := make(chan struct{}) + defer close(s) + go func() { + defer GinkgoRecover() + Expect(m.Start(s)).NotTo(HaveOccurred()) + close(done) + }() + + metricsEndpoint := fmt.Sprintf("https://%s/metrics", listener.Addr().String()) + tr := &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, + }, + } + client := &http.Client{Transport: tr} + resp, err := client.Get(metricsEndpoint) + Expect(err).NotTo(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(200)) + }) + + It("should not serve both endpoints", func(done Done) { + var insecureListener net.Listener + opts.MetricsBindAddress = ":0" + opts.newMetricsListener = func(addr string) (net.Listener, error) { + var err error + insecureListener, err = metrics.NewListener(addr) + return listener, err + } + + m, err := New(cfg, opts) + Expect(err).NotTo(HaveOccurred()) + + s := make(chan struct{}) + defer close(s) + go func() { + defer GinkgoRecover() + Expect(m.Start(s)).NotTo(HaveOccurred()) + close(done) + }() + Expect(insecureListener).To(BeNil()) + }) + }) + Context("should start serving health probes", func() { var listener net.Listener var opts Options diff --git a/pkg/manager/testdata/certs/tls.crt b/pkg/manager/testdata/certs/tls.crt new file mode 100644 index 0000000000..c154409a42 --- /dev/null +++ b/pkg/manager/testdata/certs/tls.crt @@ -0,0 +1,18 @@ +-----BEGIN CERTIFICATE----- +MIIC8zCCAdugAwIBAgIJAOBRXEjnBd8DMA0GCSqGSIb3DQEBBQUAMCQxIjAgBgNV +BAMTGWh0dHBzbG9jYWxob3N0NDQ0M21ldHJpY3MwHhcNMjAwNjExMTEzMDI3WhcN +MzAwNjA5MTEzMDI3WjAkMSIwIAYDVQQDExlodHRwc2xvY2FsaG9zdDQ0NDNtZXRy +aWNzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxY6bVSLkfdHlI9kl +4nAYTXCEdqEjVyrBQpwNeOkQ+E2hX2T28CBUPgWcHfD2wD85rVRh26Cs1ACifpIS +SJI5nSswBqkA+whzV1uIofcjbxGFlVBfeOEQnbuXpHb60ZzgCVH5/9XIlw5NiR7V +wfDzm9ZlNZzhCI8dGYFObop3fUfunmublUUyrmkxxdyFAC09w2NYijha+kz0FkGF +Ao/4Ppqa4lFBYTR41wh5XM3cZFPUTrAw4W3gc64KQIuwXNeUxYYgBA/Xpoz5EQey +jH/OVKFNKM6IKZNxyx5f9LQ9Lgzl0Vt7jjvjaGsQQ+4c9Ea6eBA9NlaKyzxyILlV +vAnw1QIDAQABoygwJjAkBgNVHREEHTAbghlodHRwc2xvY2FsaG9zdDQ0NDNtZXRy +aWNzMA0GCSqGSIb3DQEBBQUAA4IBAQBJBGV7xTrxFGyPNWautrTGo65/fCv7ddra ++1Kkh0Hlf/m9ZXRBkDW7SVlNYV+Uag/jVyT/OfYTe6TO1rxI/0L8gQhXPZ9n59cH +O8mfT+/bk1CzaUV0pKpf4fO87twLXgp0VSRPkCE3PG3xzhGrbaB36dS8Ploz9nWE +mG3NAsVc4jXYoVKZBmFCwew5h9IobqXACdWX9ik2eMS5oAOp+U7L1RZQjlXFWxEO +46TgvNxQP6IPQZlR4ed4j7viY/KSm2FXKKNVjtXLBlGKfdPuKEtCL1axrFWumFi2 +CcOjeGbwwUn602Pd3lNj/GwaKdzC40n9/r7JalgVs3Jc+oiGfZJ9 +-----END CERTIFICATE----- diff --git a/pkg/manager/testdata/certs/tls.key b/pkg/manager/testdata/certs/tls.key new file mode 100644 index 0000000000..d2f06ad868 --- /dev/null +++ b/pkg/manager/testdata/certs/tls.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpQIBAAKCAQEAxY6bVSLkfdHlI9kl4nAYTXCEdqEjVyrBQpwNeOkQ+E2hX2T2 +8CBUPgWcHfD2wD85rVRh26Cs1ACifpISSJI5nSswBqkA+whzV1uIofcjbxGFlVBf +eOEQnbuXpHb60ZzgCVH5/9XIlw5NiR7VwfDzm9ZlNZzhCI8dGYFObop3fUfunmub +lUUyrmkxxdyFAC09w2NYijha+kz0FkGFAo/4Ppqa4lFBYTR41wh5XM3cZFPUTrAw +4W3gc64KQIuwXNeUxYYgBA/Xpoz5EQeyjH/OVKFNKM6IKZNxyx5f9LQ9Lgzl0Vt7 +jjvjaGsQQ+4c9Ea6eBA9NlaKyzxyILlVvAnw1QIDAQABAoIBADFaMsvN767O5KNT +9/bdcfTGixDnqGB6OdVeDq+J6cdd/VZLbrUGHoVv+VQxgjL8mHgIgHnRZduAXRep +fg/LF8F/rHu9dJVBwy6rmzJ6/sscYXavoWodL314A6X+YyJCQmWRqRaUXYv+8rey +kEvm2bSwlpASJNVyix54AxPyW29cPHt/8kWgyrVyr2U+hQwPHntTYrmioRMlSX5v +qFGt4s4/oo+aXA689Q14Qu1RWldxLog4581VAIjg47FwGRJ1PYyC0ywkgI+b7Kl5 +7YXrBAp8f6xrtpUd4SULbWQ/CKoSembhhZ8eRxfqMXH+Qz4xCgSw5DAaXQoiehkn +itrsuoECgYEA6XRtZ+tY8dGvypbB+BUdz58QgM9sFKXf5W2DpLbKGFfyaqFTouAs +mI4YIWrS53CWm1PiG8v3O7duAgtzpr584QjAkw7t3zOqVdtnoE4orxvM6hRhCOV5 +9Wi9axE5iG5Pp66SNlbOvypX8OJUbdi8cFZos4xr54VyXG0gSy3PK30CgYEA2KKz +Hn+oErsdhpVGC+iOS8QZVL4nfJeAL7AKo6sO66hSvP5tdNTTh06MI5HXz7mUOWVE +SyampkUD+qk+ptGbD8cF+k3D4/6a5KxL9nnAGxIK9KwMYQsc8/0brieFwy4FWcKH +huEtX3Bnab7MU6iTmwnnciGG2/5uo5+uvpYd6jkCgYEAh/J4049FmGxXRk5MXj9N +wN4MKjaf5dZCb8Q6aOzY+xwb2uRfY/XPgnccrjka4BO8YG+UuEMqkefbc+1fR7ad +2h3SptCGzPe1NZIy4jMhlfdGePmtGBUp1DNOOs8pBb3XPPp3wpUCiGgMFgZ2zBDu +iyyGhCg9nfEkC5awu5bNkbECgYEAyzcMOW7clf2Ku+WpWKBlYzNn47OgzOI9L/6+ +bDuZenxiaMFuoerHJqULFo7H2CcooRKalriCGXSiP++lQs1a3NkAhYWPXX9Hg30Q +oPwitgId3tjJn/rRxRrIbXzLoIS6JjIx+defPWjuySZe+5cmJ4iJ4OkMXa/1z22K +eWPOWhkCgYEA19jUW9NweDMWlkYm92SNzvZIv8brPrWGNthTxUUflaA9NQRe++fZ +RR1wT84WmV94lDOC4AKHtCQXx4o9m4GVwFcNIzdtexxbdBYPY9FPpZlR/S/VC9Ef +73rEV46bie8TMDd0CmHBSpiDHDqgPyNwaYpttuoVFGNxw59/8OdeJQM= +-----END RSA PRIVATE KEY----- diff --git a/pkg/metrics/listener.go b/pkg/metrics/listener.go index d32ae58186..bbccdd3f96 100644 --- a/pkg/metrics/listener.go +++ b/pkg/metrics/listener.go @@ -17,9 +17,13 @@ limitations under the License. package metrics import ( + "context" + "crypto/tls" "fmt" "net" + "path/filepath" + "sigs.k8s.io/controller-runtime/pkg/internal/certwatcher" logf "sigs.k8s.io/controller-runtime/pkg/internal/log" ) @@ -29,6 +33,12 @@ var log = logf.RuntimeLog.WithName("metrics") // The metrics is on by default. var DefaultBindAddress = ":8080" +// DefaultKeyPath is prefix path for TLS key +var DefaultKeyPath = "tls.key" + +// DefualtCertPath is prefix path for TLS certificate +var DefualtCertPath = "tls.crt" + // NewListener creates a new TCP listener bound to the given address. func NewListener(addr string) (net.Listener, error) { if addr == "" { @@ -44,9 +54,39 @@ func NewListener(addr string) (net.Listener, error) { log.Info("metrics server is starting to listen", "addr", addr) ln, err := net.Listen("tcp", addr) if err != nil { - er := fmt.Errorf("error listening on %s: %w", addr, err) - log.Error(er, "metrics server failed to listen. You may want to disable the metrics server or use another port if it is due to conflicts") - return nil, er + err = fmt.Errorf("error listening on %s: %w", addr, err) + log.Error(err, "metrics server failed to listen. You may want to disable the metrics server or use another port if it is due to conflicts") + return nil, err } + return ln, nil } + +// NewSecureListener creates a new TCP listener over TLS bound to the given address. +func NewSecureListener(addr, certDir string) (net.Listener, error) { + ln, err := NewListener(addr) + if err != nil { + return nil, err + } + + certPath := filepath.Join(certDir, DefualtCertPath) + certKey := filepath.Join(certDir, DefaultKeyPath) + certWatcher, err := certwatcher.New(certPath, certKey) + if err != nil { + return nil, err + } + + go func() { + if err := certWatcher.Start(context.Background().Done()); err != nil { + log.Error(err, "certificate watcher error") + return + } + }() + + cfg := &tls.Config{ + NextProtos: []string{"h2"}, + GetCertificate: certWatcher.GetCertificate, + } + + return tls.NewListener(ln, cfg), nil +} diff --git a/pkg/webhook/server.go b/pkg/webhook/server.go index e771a936cb..588319654a 100644 --- a/pkg/webhook/server.go +++ b/pkg/webhook/server.go @@ -31,8 +31,9 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" + + "sigs.k8s.io/controller-runtime/pkg/internal/certwatcher" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" - "sigs.k8s.io/controller-runtime/pkg/webhook/internal/certwatcher" "sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics" )