From 3834508fa6a8f4ac24865e578b0ec0ebf32c976b Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Wed, 15 Nov 2023 18:17:37 +0000 Subject: [PATCH 1/9] added certificate reload logic changes --- cmd/extensions/main.go | 29 +++++++++++++++++++++++++++++ pkg/util/https/server.go | 15 +++++++++++++-- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/cmd/extensions/main.go b/cmd/extensions/main.go index 33c0411004..5e5f253d99 100644 --- a/cmd/extensions/main.go +++ b/cmd/extensions/main.go @@ -17,6 +17,7 @@ package main import ( "context" + "crypto/tls" "io" "net/http" "os" @@ -35,6 +36,7 @@ import ( "agones.dev/agones/pkg/gameserversets" "agones.dev/agones/pkg/metrics" "agones.dev/agones/pkg/util/apiserver" + "agones.dev/agones/pkg/util/fswatch" "agones.dev/agones/pkg/util/https" "agones.dev/agones/pkg/util/runtime" "agones.dev/agones/pkg/util/signals" @@ -51,6 +53,10 @@ import ( "k8s.io/client-go/tools/clientcmd" ) +const ( + tlsDir = "/home/agones/certs/" +) + const ( enableStackdriverMetricsFlag = "stackdriver-exporter" stackdriverLabels = "stackdriver-labels" @@ -139,6 +145,21 @@ func main() { } // https server and the items that share the Mux for routing httpsServer := https.NewServer(ctlConf.CertFile, ctlConf.KeyFile) + + cancelTLS, err := fswatch.Watch(logger, tlsDir, time.Second, func() { + tlsCert, err := readTLSCert() + if err != nil { + logger.WithError(err).Error("could not load TLS certs; keeping old one") + return + } + httpsServer.SetCertificate(tlsCert) + logger.Info("TLS certs updated") + }) + if err != nil { + logger.WithError(err).Fatal("could not create watcher for TLS certs") + } + defer cancelTLS() + wh := webhooks.NewWebHook(httpsServer.Mux) api := apiserver.NewAPIServer(httpsServer.Mux) @@ -221,6 +242,14 @@ func main() { logger.Info("Shut down agones extensions") } +func readTLSCert() (*tls.Certificate, error) { + tlsCert, err := tls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") + if err != nil { + return nil, err + } + return &tlsCert, nil +} + func parseEnvFlags() config { exec, err := os.Executable() if err != nil { diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index b1b6311571..0e8824d3fa 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -16,7 +16,9 @@ package https import ( "context" + "crypto/tls" "net/http" + "sync" "agones.dev/agones/pkg/util/runtime" "github.com/pkg/errors" @@ -24,7 +26,7 @@ import ( ) // tls is a http server interface to enable easier testing -type tls interface { +type testTLS interface { Close() error ListenAndServeTLS(certFile, keyFile string) error } @@ -35,7 +37,9 @@ type tls interface { type Server struct { logger *logrus.Entry Mux *http.ServeMux - tls tls + tls testTLS + certMu sync.RWMutex + cert *tls.Certificate certFile string keyFile string } @@ -53,6 +57,7 @@ func NewServer(certFile, keyFile string) *Server { tls: tls, certFile: certFile, keyFile: keyFile, + cert: nil, } wh.Mux.HandleFunc("/", wh.defaultHandler) wh.logger = runtime.NewLoggerWithType(wh) @@ -60,6 +65,12 @@ func NewServer(certFile, keyFile string) *Server { return wh } +func (s *Server) SetCertificate(cert *tls.Certificate) { + s.certMu.Lock() + defer s.certMu.Unlock() + s.cert = cert +} + // Run runs the webhook server, starting a https listener. // Will close the http server on stop channel close. func (s *Server) Run(ctx context.Context, _ int) error { From 95bd60bca1392e79582aa83bf53ca2048e1570f8 Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Tue, 21 Nov 2023 15:24:55 +0000 Subject: [PATCH 2/9] refresh certificate --- cmd/extensions/main.go | 31 +------------------- pkg/util/https/server.go | 54 +++++++++++++++++++++++++++-------- pkg/util/https/server_test.go | 2 +- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/cmd/extensions/main.go b/cmd/extensions/main.go index 5e5f253d99..a7ed0e9e47 100644 --- a/cmd/extensions/main.go +++ b/cmd/extensions/main.go @@ -17,7 +17,6 @@ package main import ( "context" - "crypto/tls" "io" "net/http" "os" @@ -36,7 +35,6 @@ import ( "agones.dev/agones/pkg/gameserversets" "agones.dev/agones/pkg/metrics" "agones.dev/agones/pkg/util/apiserver" - "agones.dev/agones/pkg/util/fswatch" "agones.dev/agones/pkg/util/https" "agones.dev/agones/pkg/util/runtime" "agones.dev/agones/pkg/util/signals" @@ -53,10 +51,6 @@ import ( "k8s.io/client-go/tools/clientcmd" ) -const ( - tlsDir = "/home/agones/certs/" -) - const ( enableStackdriverMetricsFlag = "stackdriver-exporter" stackdriverLabels = "stackdriver-labels" @@ -144,22 +138,7 @@ func main() { logger.WithError(err).Fatal("Could not initialize cloud product") } // https server and the items that share the Mux for routing - httpsServer := https.NewServer(ctlConf.CertFile, ctlConf.KeyFile) - - cancelTLS, err := fswatch.Watch(logger, tlsDir, time.Second, func() { - tlsCert, err := readTLSCert() - if err != nil { - logger.WithError(err).Error("could not load TLS certs; keeping old one") - return - } - httpsServer.SetCertificate(tlsCert) - logger.Info("TLS certs updated") - }) - if err != nil { - logger.WithError(err).Fatal("could not create watcher for TLS certs") - } - defer cancelTLS() - + httpsServer := https.NewServer(ctlConf.CertFile, ctlConf.KeyFile, logger) wh := webhooks.NewWebHook(httpsServer.Mux) api := apiserver.NewAPIServer(httpsServer.Mux) @@ -242,14 +221,6 @@ func main() { logger.Info("Shut down agones extensions") } -func readTLSCert() (*tls.Certificate, error) { - tlsCert, err := tls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") - if err != nil { - return nil, err - } - return &tlsCert, nil -} - func parseEnvFlags() config { exec, err := os.Executable() if err != nil { diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index 0e8824d3fa..2e0645e5cf 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -19,12 +19,20 @@ import ( "crypto/tls" "net/http" "sync" + "time" + "agones.dev/agones/pkg/util/fswatch" "agones.dev/agones/pkg/util/runtime" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) +const ( + tlsDir = "/certs/" +) + +var tlsMutex sync.Mutex + // tls is a http server interface to enable easier testing type testTLS interface { Close() error @@ -38,26 +46,46 @@ type Server struct { logger *logrus.Entry Mux *http.ServeMux tls testTLS - certMu sync.RWMutex - cert *tls.Certificate certFile string keyFile string } // NewServer returns a Server instance. -func NewServer(certFile, keyFile string) *Server { +func NewServer(certFile, keyFile string, logger *logrus.Entry) *Server { mux := http.NewServeMux() - tls := &http.Server{ + tls_server := &http.Server{ Addr: ":8081", Handler: mux, } + go func() { + cancelTLS, err := fswatch.Watch(logger, tlsDir, time.Second, func() { + tlsCert, err := readTLSCert() + if err != nil { + logger.WithError(err).Error("could not load TLS certs; keeping old one") + return + } + tlsMutex.Lock() + defer tlsMutex.Unlock() + tls_server.TLSConfig = &tls.Config{ + GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { + return tlsCert, nil + }, + } + logger.Info("TLS certs updated") + }) + if err != nil { + logger.WithError(err).Fatal("could not create watcher for TLS certs") + } + defer cancelTLS() + + }() + wh := &Server{ Mux: mux, - tls: tls, + tls: tls_server, certFile: certFile, keyFile: keyFile, - cert: nil, } wh.Mux.HandleFunc("/", wh.defaultHandler) wh.logger = runtime.NewLoggerWithType(wh) @@ -65,12 +93,6 @@ func NewServer(certFile, keyFile string) *Server { return wh } -func (s *Server) SetCertificate(cert *tls.Certificate) { - s.certMu.Lock() - defer s.certMu.Unlock() - s.cert = cert -} - // Run runs the webhook server, starting a https listener. // Will close the http server on stop channel close. func (s *Server) Run(ctx context.Context, _ int) error { @@ -101,3 +123,11 @@ func (s *Server) defaultHandler(w http.ResponseWriter, r *http.Request) { FourZeroFour(s.logger, w, r) } + +func readTLSCert() (*tls.Certificate, error) { + tlsCert, err := tls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") + if err != nil { + return nil, err + } + return &tlsCert, nil +} diff --git a/pkg/util/https/server_test.go b/pkg/util/https/server_test.go index 95348e6308..4a9d97b0f2 100644 --- a/pkg/util/https/server_test.go +++ b/pkg/util/https/server_test.go @@ -41,7 +41,7 @@ func (ts *testServer) ListenAndServeTLS(certFile, keyFile string) error { func TestServerRun(t *testing.T) { t.Parallel() - s := NewServer("", "") + s := NewServer("", "",nil) ts := &testServer{server: httptest.NewUnstartedServer(s.Mux)} s.tls = ts From d90d5a2b29151d58437b70b4005c920555ddd88b Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Thu, 28 Dec 2023 13:43:11 +0000 Subject: [PATCH 3/9] made changes in certificate handling --- cmd/extensions/main.go | 2 +- pkg/util/https/server.go | 71 +++++++++++++---------------------- pkg/util/https/server_test.go | 2 +- 3 files changed, 28 insertions(+), 47 deletions(-) diff --git a/cmd/extensions/main.go b/cmd/extensions/main.go index a7ed0e9e47..33c0411004 100644 --- a/cmd/extensions/main.go +++ b/cmd/extensions/main.go @@ -138,7 +138,7 @@ func main() { logger.WithError(err).Fatal("Could not initialize cloud product") } // https server and the items that share the Mux for routing - httpsServer := https.NewServer(ctlConf.CertFile, ctlConf.KeyFile, logger) + httpsServer := https.NewServer(ctlConf.CertFile, ctlConf.KeyFile) wh := webhooks.NewWebHook(httpsServer.Mux) api := apiserver.NewAPIServer(httpsServer.Mux) diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index 2e0645e5cf..94bbfbabd9 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -16,12 +16,9 @@ package https import ( "context" - "crypto/tls" + cryptotls "crypto/tls" "net/http" - "sync" - "time" - "agones.dev/agones/pkg/util/fswatch" "agones.dev/agones/pkg/util/runtime" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -31,10 +28,8 @@ const ( tlsDir = "/certs/" ) -var tlsMutex sync.Mutex - // tls is a http server interface to enable easier testing -type testTLS interface { +type tls interface { Close() error ListenAndServeTLS(certFile, keyFile string) error } @@ -45,54 +40,48 @@ type testTLS interface { type Server struct { logger *logrus.Entry Mux *http.ServeMux - tls testTLS + tls tls certFile string keyFile string } // NewServer returns a Server instance. -func NewServer(certFile, keyFile string, logger *logrus.Entry) *Server { +func NewServer(certFile, keyFile string) *Server { mux := http.NewServeMux() - tls_server := &http.Server{ - Addr: ":8081", - Handler: mux, - } - - go func() { - cancelTLS, err := fswatch.Watch(logger, tlsDir, time.Second, func() { - tlsCert, err := readTLSCert() - if err != nil { - logger.WithError(err).Error("could not load TLS certs; keeping old one") - return - } - tlsMutex.Lock() - defer tlsMutex.Unlock() - tls_server.TLSConfig = &tls.Config{ - GetCertificate: func(*tls.ClientHelloInfo) (*tls.Certificate, error) { - return tlsCert, nil - }, - } - logger.Info("TLS certs updated") - }) - if err != nil { - logger.WithError(err).Fatal("could not create watcher for TLS certs") - } - defer cancelTLS() - - }() wh := &Server{ Mux: mux, - tls: tls_server, certFile: certFile, keyFile: keyFile, } + wh.setupServer() + wh.Mux.HandleFunc("/", wh.defaultHandler) wh.logger = runtime.NewLoggerWithType(wh) return wh } +func (s *Server) setupServer() { + s.tls = &http.Server{ + Addr: ":8081", + Handler: s.Mux, + TLSConfig: &cryptotls.Config{ + GetCertificate: func(hello *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { + return s.loadTLSCert() + }, + }, + } +} + +func (s *Server) loadTLSCert() (*cryptotls.Certificate, error) { + tlsCert, err := cryptotls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") + if err != nil { + return nil, err + } + return &tlsCert, nil +} + // Run runs the webhook server, starting a https listener. // Will close the http server on stop channel close. func (s *Server) Run(ctx context.Context, _ int) error { @@ -123,11 +112,3 @@ func (s *Server) defaultHandler(w http.ResponseWriter, r *http.Request) { FourZeroFour(s.logger, w, r) } - -func readTLSCert() (*tls.Certificate, error) { - tlsCert, err := tls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") - if err != nil { - return nil, err - } - return &tlsCert, nil -} diff --git a/pkg/util/https/server_test.go b/pkg/util/https/server_test.go index 4a9d97b0f2..95348e6308 100644 --- a/pkg/util/https/server_test.go +++ b/pkg/util/https/server_test.go @@ -41,7 +41,7 @@ func (ts *testServer) ListenAndServeTLS(certFile, keyFile string) error { func TestServerRun(t *testing.T) { t.Parallel() - s := NewServer("", "",nil) + s := NewServer("", "") ts := &testServer{server: httptest.NewUnstartedServer(s.Mux)} s.tls = ts From be333fbf8a5cbf9bd95ca894a721e36f00c15d74 Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Wed, 3 Jan 2024 10:09:54 +0000 Subject: [PATCH 4/9] added suggested change --- pkg/util/https/server.go | 42 +++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index 94bbfbabd9..37d04eef1b 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -18,7 +18,10 @@ import ( "context" cryptotls "crypto/tls" "net/http" + "sync" + "time" + "agones.dev/agones/pkg/util/fswatch" "agones.dev/agones/pkg/util/runtime" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -43,6 +46,8 @@ type Server struct { tls tls certFile string keyFile string + CertMu sync.Mutex + Certs *cryptotls.Certificate } // NewServer returns a Server instance. @@ -67,19 +72,42 @@ func (s *Server) setupServer() { Addr: ":8081", Handler: s.Mux, TLSConfig: &cryptotls.Config{ - GetCertificate: func(hello *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { - return s.loadTLSCert() - }, + GetCertificate: s.getCertificate, }, } + + // Start a goroutine to watch for certificate changes + go s.watchForCertificateChanges() +} + +// getCertificate returns the current TLS certificate +func (s *Server) getCertificate(hello *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { + s.CertMu.Lock() + defer s.CertMu.Unlock() + return s.Certs, nil } -func (s *Server) loadTLSCert() (*cryptotls.Certificate, error) { - tlsCert, err := cryptotls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") +// watchForCertificateChanges watches for changes in the certificate files +func (s *Server) watchForCertificateChanges() { + // Watch for changes in the tlsDir + cancelTLS, err := fswatch.Watch(s.logger, tlsDir, time.Second, func() { + // Load the new TLS certificate + tlsCert, err := cryptotls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") + if err != nil { + s.logger.WithError(err).Error("could not load TLS certs; keeping old one") + return + } + s.CertMu.Lock() + defer s.CertMu.Unlock() + // Update the Certs structure with the new certificate + s.Certs = &tlsCert + s.logger.Info("TLS certs updated") + }) if err != nil { - return nil, err + s.logger.WithError(err).Fatal("could not create watcher for TLS certs") } - return &tlsCert, nil + + defer cancelTLS() } // Run runs the webhook server, starting a https listener. From ac0837254532084223ead8ceae546eaafcb1e8b5 Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Wed, 3 Jan 2024 10:40:16 +0000 Subject: [PATCH 5/9] fixing alignment issue of govet check --- pkg/util/https/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index 37d04eef1b..af1a61bd8f 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -43,11 +43,11 @@ type tls interface { type Server struct { logger *logrus.Entry Mux *http.ServeMux + Certs *cryptotls.Certificate + CertMu sync.Mutex tls tls certFile string keyFile string - CertMu sync.Mutex - Certs *cryptotls.Certificate } // NewServer returns a Server instance. From 1c5c50318508a17c7cb62724375b06220b1b5c94 Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Thu, 11 Jan 2024 16:11:07 +0000 Subject: [PATCH 6/9] watcher should keep watching the certificate --- pkg/util/https/server.go | 49 ++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index af1a61bd8f..8fea8825ac 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -31,6 +31,8 @@ const ( tlsDir = "/certs/" ) +var closeCh = make(chan struct{}) + // tls is a http server interface to enable easier testing type tls interface { Close() error @@ -59,36 +61,40 @@ func NewServer(certFile, keyFile string) *Server { certFile: certFile, keyFile: keyFile, } - wh.setupServer() - - wh.Mux.HandleFunc("/", wh.defaultHandler) - wh.logger = runtime.NewLoggerWithType(wh) - return wh -} + tlsCert, err := readTLSCert() + if err != nil { + logrus.WithError(err).Fatal("could not load TLS certs.") + } + wh.CertMu.Lock() + wh.Certs = tlsCert + wh.CertMu.Unlock() -func (s *Server) setupServer() { - s.tls = &http.Server{ + wh.tls = &http.Server{ Addr: ":8081", - Handler: s.Mux, - TLSConfig: &cryptotls.Config{ - GetCertificate: s.getCertificate, - }, + Handler: wh.Mux, } // Start a goroutine to watch for certificate changes - go s.watchForCertificateChanges() + go watchForCertificateChanges(wh) + + wh.Mux.HandleFunc("/", wh.defaultHandler) + wh.logger = runtime.NewLoggerWithType(wh) + + return wh } -// getCertificate returns the current TLS certificate -func (s *Server) getCertificate(hello *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { - s.CertMu.Lock() - defer s.CertMu.Unlock() - return s.Certs, nil +// It will load the key pair certificate +func readTLSCert() (*cryptotls.Certificate, error) { + tlsCert, err := cryptotls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") + if err != nil { + return nil, err + } + return &tlsCert, nil } // watchForCertificateChanges watches for changes in the certificate files -func (s *Server) watchForCertificateChanges() { +func watchForCertificateChanges(s *Server) { // Watch for changes in the tlsDir cancelTLS, err := fswatch.Watch(s.logger, tlsDir, time.Second, func() { // Load the new TLS certificate @@ -107,7 +113,9 @@ func (s *Server) watchForCertificateChanges() { s.logger.WithError(err).Fatal("could not create watcher for TLS certs") } - defer cancelTLS() + // Wait for the signal to close + <-closeCh + cancelTLS() } // Run runs the webhook server, starting a https listener. @@ -116,6 +124,7 @@ func (s *Server) Run(ctx context.Context, _ int) error { go func() { <-ctx.Done() s.tls.Close() // nolint: errcheck,gosec + close(closeCh) }() s.logger.WithField("server", s).Infof("https server started") From fd4eb4fbd698748d2b6ba6083099b7d38203edfe Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Tue, 16 Jan 2024 10:03:05 +0000 Subject: [PATCH 7/9] fixed certificate reload issue --- cmd/extensions/main.go | 5 ++++ pkg/util/https/server.go | 65 +++++++++++++++++++--------------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/cmd/extensions/main.go b/cmd/extensions/main.go index 33c0411004..02fcbb8847 100644 --- a/cmd/extensions/main.go +++ b/cmd/extensions/main.go @@ -139,6 +139,11 @@ func main() { } // https server and the items that share the Mux for routing httpsServer := https.NewServer(ctlConf.CertFile, ctlConf.KeyFile) + cancelTLS, err := httpsServer.WatchForCertificateChanges() + if err != nil { + logger.WithError(err).Fatal("Got an error while watching certificate changes") + } + defer cancelTLS() wh := webhooks.NewWebHook(httpsServer.Mux) api := apiserver.NewAPIServer(httpsServer.Mux) diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index 8fea8825ac..5b0cd3a22e 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -31,8 +31,6 @@ const ( tlsDir = "/certs/" ) -var closeCh = make(chan struct{}) - // tls is a http server interface to enable easier testing type tls interface { Close() error @@ -45,9 +43,9 @@ type tls interface { type Server struct { logger *logrus.Entry Mux *http.ServeMux - Certs *cryptotls.Certificate CertMu sync.Mutex tls tls + Certs *cryptotls.Certificate certFile string keyFile string } @@ -61,43 +59,46 @@ func NewServer(certFile, keyFile string) *Server { certFile: certFile, keyFile: keyFile, } - - tlsCert, err := readTLSCert() - if err != nil { - logrus.WithError(err).Fatal("could not load TLS certs.") - } - wh.CertMu.Lock() - wh.Certs = tlsCert - wh.CertMu.Unlock() - - wh.tls = &http.Server{ - Addr: ":8081", - Handler: wh.Mux, - } - - // Start a goroutine to watch for certificate changes - go watchForCertificateChanges(wh) - - wh.Mux.HandleFunc("/", wh.defaultHandler) wh.logger = runtime.NewLoggerWithType(wh) + wh.setupServer() + wh.Mux.HandleFunc("/", wh.defaultHandler) return wh } -// It will load the key pair certificate -func readTLSCert() (*cryptotls.Certificate, error) { +func (s *Server) setupServer() { + s.tls = &http.Server{ + Addr: ":8081", + Handler: s.Mux, + TLSConfig: &cryptotls.Config{ + GetCertificate: s.getCertificate, + }, + } + tlsCert, err := cryptotls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") if err != nil { - return nil, err + s.logger.WithError(err).Error("could not load Initial TLS certs; keeping old one") + return } - return &tlsCert, nil + + s.CertMu.Lock() + defer s.CertMu.Unlock() + s.Certs = &tlsCert +} + +// getCertificate returns the current TLS certificate +func (s *Server) getCertificate(hello *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { + s.CertMu.Lock() + defer s.CertMu.Unlock() + return s.Certs, nil } -// watchForCertificateChanges watches for changes in the certificate files -func watchForCertificateChanges(s *Server) { - // Watch for changes in the tlsDir +// WatchForCertificateChanges watches for changes in the certificate files +func (s *Server) WatchForCertificateChanges() (func(), error) { + cancelTLS, err := fswatch.Watch(s.logger, tlsDir, time.Second, func() { // Load the new TLS certificate + s.logger.Info("TLS certs changed, reloading") tlsCert, err := cryptotls.LoadX509KeyPair(tlsDir+"server.crt", tlsDir+"server.key") if err != nil { s.logger.WithError(err).Error("could not load TLS certs; keeping old one") @@ -105,17 +106,14 @@ func watchForCertificateChanges(s *Server) { } s.CertMu.Lock() defer s.CertMu.Unlock() - // Update the Certs structure with the new certificate s.Certs = &tlsCert s.logger.Info("TLS certs updated") }) if err != nil { s.logger.WithError(err).Fatal("could not create watcher for TLS certs") + return nil, err } - - // Wait for the signal to close - <-closeCh - cancelTLS() + return cancelTLS, nil } // Run runs the webhook server, starting a https listener. @@ -124,7 +122,6 @@ func (s *Server) Run(ctx context.Context, _ int) error { go func() { <-ctx.Done() s.tls.Close() // nolint: errcheck,gosec - close(closeCh) }() s.logger.WithField("server", s).Infof("https server started") From 1c998452030fbc87867967b8aba9b6f60f347ec7 Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Tue, 16 Jan 2024 14:01:05 +0000 Subject: [PATCH 8/9] fixed lint check --- pkg/util/https/server.go | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index 5b0cd3a22e..209fc45236 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -37,17 +37,22 @@ type tls interface { ListenAndServeTLS(certFile, keyFile string) error } +// CertServer holds the Server certificate +type CertServer struct { + Certs *cryptotls.Certificate + CertMu sync.Mutex +} + // Server is a HTTPs server that conforms to the runner interface // we use in /cmd/controller, and has a public Mux that can be updated // has a default 404 handler, to make discovery of k8s services a bit easier. type Server struct { - logger *logrus.Entry - Mux *http.ServeMux - CertMu sync.Mutex - tls tls - Certs *cryptotls.Certificate - certFile string - keyFile string + CertServer CertServer + logger *logrus.Entry + Mux *http.ServeMux + tls tls + certFile string + keyFile string } // NewServer returns a Server instance. @@ -81,16 +86,16 @@ func (s *Server) setupServer() { return } - s.CertMu.Lock() - defer s.CertMu.Unlock() - s.Certs = &tlsCert + s.CertServer.CertMu.Lock() + defer s.CertServer.CertMu.Unlock() + s.CertServer.Certs = &tlsCert } // getCertificate returns the current TLS certificate func (s *Server) getCertificate(hello *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { - s.CertMu.Lock() - defer s.CertMu.Unlock() - return s.Certs, nil + s.CertServer.CertMu.Lock() + defer s.CertServer.CertMu.Unlock() + return s.CertServer.Certs, nil } // WatchForCertificateChanges watches for changes in the certificate files @@ -104,9 +109,9 @@ func (s *Server) WatchForCertificateChanges() (func(), error) { s.logger.WithError(err).Error("could not load TLS certs; keeping old one") return } - s.CertMu.Lock() - defer s.CertMu.Unlock() - s.Certs = &tlsCert + s.CertServer.CertMu.Lock() + defer s.CertServer.CertMu.Unlock() + s.CertServer.Certs = &tlsCert s.logger.Info("TLS certs updated") }) if err != nil { From c4f2ab7e9d81008bcac579c961546e6404fbff65 Mon Sep 17 00:00:00 2001 From: Ashutosh Singh Date: Wed, 17 Jan 2024 05:58:11 +0000 Subject: [PATCH 9/9] changed exported type to unexported --- pkg/util/https/server.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/util/https/server.go b/pkg/util/https/server.go index 209fc45236..34a2a471b8 100644 --- a/pkg/util/https/server.go +++ b/pkg/util/https/server.go @@ -37,17 +37,17 @@ type tls interface { ListenAndServeTLS(certFile, keyFile string) error } -// CertServer holds the Server certificate -type CertServer struct { - Certs *cryptotls.Certificate - CertMu sync.Mutex +// certServer holds the Server certificate +type certServer struct { + certs *cryptotls.Certificate + certMu sync.Mutex } // Server is a HTTPs server that conforms to the runner interface // we use in /cmd/controller, and has a public Mux that can be updated // has a default 404 handler, to make discovery of k8s services a bit easier. type Server struct { - CertServer CertServer + certServer certServer logger *logrus.Entry Mux *http.ServeMux tls tls @@ -86,16 +86,16 @@ func (s *Server) setupServer() { return } - s.CertServer.CertMu.Lock() - defer s.CertServer.CertMu.Unlock() - s.CertServer.Certs = &tlsCert + s.certServer.certMu.Lock() + defer s.certServer.certMu.Unlock() + s.certServer.certs = &tlsCert } // getCertificate returns the current TLS certificate func (s *Server) getCertificate(hello *cryptotls.ClientHelloInfo) (*cryptotls.Certificate, error) { - s.CertServer.CertMu.Lock() - defer s.CertServer.CertMu.Unlock() - return s.CertServer.Certs, nil + s.certServer.certMu.Lock() + defer s.certServer.certMu.Unlock() + return s.certServer.certs, nil } // WatchForCertificateChanges watches for changes in the certificate files @@ -109,9 +109,9 @@ func (s *Server) WatchForCertificateChanges() (func(), error) { s.logger.WithError(err).Error("could not load TLS certs; keeping old one") return } - s.CertServer.CertMu.Lock() - defer s.CertServer.CertMu.Unlock() - s.CertServer.Certs = &tlsCert + s.certServer.certMu.Lock() + defer s.certServer.certMu.Unlock() + s.certServer.certs = &tlsCert s.logger.Info("TLS certs updated") }) if err != nil {