diff --git a/cmd/otel-allocator/config/config.go b/cmd/otel-allocator/config/config.go index 7d13658e32..5ecf75c4a5 100644 --- a/cmd/otel-allocator/config/config.go +++ b/cmd/otel-allocator/config/config.go @@ -36,14 +36,14 @@ import ( ) const ( - DefaultResyncTime = 5 * time.Minute - DefaultConfigFilePath string = "/conf/targetallocator.yaml" - DefaultCRScrapeInterval model.Duration = model.Duration(time.Second * 30) - DefaultAllocationStrategy = "consistent-hashing" - DefaultFilterStrategy = "relabel-config" - DefaultServerCAFilePath = "/conf/tls/ca.crt" - DefaultServerTLSCertFilePath = "/conf/tls/tls.crt" - DefaultServerTLSKeyFilePath = "/conf/tls/tls.key" + DefaultResyncTime = 5 * time.Minute + DefaultConfigFilePath string = "/conf/targetallocator.yaml" + DefaultCRScrapeInterval model.Duration = model.Duration(time.Second * 30) + DefaultAllocationStrategy = "consistent-hashing" + DefaultFilterStrategy = "relabel-config" + DefaultHttpsCAFilePath = "/conf/tls/ca.crt" + DefaultHttpsTLSCertFilePath = "/conf/tls/tls.crt" + DefaultHttpsTLSKeyFilePath = "/conf/tls/tls.key" ) type Config struct { @@ -69,11 +69,11 @@ type PrometheusCRConfig struct { } type HTTPSServerConfig struct { - Enabled bool `yaml:"enabled,omitempty"` - ListenAddr string `yaml:"listen_addr,omitempty"` - ServerCAFilePath string `yaml:"ca_file_path,omitempty"` - ServerTLSCertFilePath string `yaml:"tls_cert_file_path,omitempty"` - ServerTLSKeyFilePath string `yaml:"tls_key_file_path,omitempty"` + Enabled bool `yaml:"enabled,omitempty"` + ListenAddr string `yaml:"listen_addr,omitempty"` + CAFilePath string `yaml:"ca_file_path,omitempty"` + TLSCertFilePath string `yaml:"tls_cert_file_path,omitempty"` + TLSKeyFilePath string `yaml:"tls_key_file_path,omitempty"` } func LoadFromFile(file string, target *Config) error { @@ -125,6 +125,21 @@ func LoadFromCLI(target *Config, flagSet *pflag.FlagSet) error { return err } + target.HTTPS.CAFilePath, err = getHttpsCAFilePath(flagSet) + if err != nil { + return err + } + + target.HTTPS.TLSCertFilePath, err = getHttpsTLSCertFilePath(flagSet) + if err != nil { + return err + } + + target.HTTPS.TLSKeyFilePath, err = getHttpsTLSKeyFilePath(flagSet) + if err != nil { + return err + } + return nil } diff --git a/cmd/otel-allocator/config/flags.go b/cmd/otel-allocator/config/flags.go index d4bf994998..88d10fde1a 100644 --- a/cmd/otel-allocator/config/flags.go +++ b/cmd/otel-allocator/config/flags.go @@ -25,13 +25,16 @@ import ( // Flag names. const ( - targetAllocatorName = "target-allocator" - configFilePathFlagName = "config-file" - listenAddrFlagName = "listen-addr" - prometheusCREnabledFlagName = "enable-prometheus-cr-watcher" - kubeConfigPathFlagName = "kubeconfig-path" - listenAddrHttpsFlagName = "listen-addr-https" - httpsEnabledFlagName = "enable-https-server" + targetAllocatorName = "target-allocator" + configFilePathFlagName = "config-file" + listenAddrFlagName = "listen-addr" + prometheusCREnabledFlagName = "enable-prometheus-cr-watcher" + kubeConfigPathFlagName = "kubeconfig-path" + httpsEnabledFlagName = "enable-https-server" + listenAddrHttpsFlagName = "listen-addr-https" + httpsCAFilePathFlagName = "https-ca-file" + httpsTLSCertFilePathFlagName = "https-tls-cert-file" + httpsTLSKeyFilePathFlagName = "https-tls-key-file" ) // We can't bind this flag to our FlagSet, so we need to handle it separately. @@ -42,9 +45,12 @@ func getFlagSet(errorHandling pflag.ErrorHandling) *pflag.FlagSet { flagSet.String(configFilePathFlagName, DefaultConfigFilePath, "The path to the config file.") flagSet.String(listenAddrFlagName, ":8080", "The address where this service serves.") flagSet.Bool(prometheusCREnabledFlagName, false, "Enable Prometheus CRs as target sources") + flagSet.String(kubeConfigPathFlagName, filepath.Join(homedir.HomeDir(), ".kube", "config"), "absolute path to the KubeconfigPath file") flagSet.Bool(httpsEnabledFlagName, false, "Enable HTTPS additional server") flagSet.String(listenAddrHttpsFlagName, ":8443", "The address where this service serves over HTTPS.") - flagSet.String(kubeConfigPathFlagName, filepath.Join(homedir.HomeDir(), ".kube", "config"), "absolute path to the KubeconfigPath file") + flagSet.String(httpsCAFilePathFlagName, DefaultHttpsCAFilePath, "The path to the HTTPS server TLS CA file.") + flagSet.String(httpsTLSCertFilePathFlagName, DefaultHttpsTLSCertFilePath, "The path to the HTTPS server TLS certificate file.") + flagSet.String(httpsTLSKeyFilePathFlagName, DefaultHttpsTLSKeyFilePath, "The path to the HTTPS server TLS key file.") zapFlagSet := flag.NewFlagSet("", flag.ErrorHandling(errorHandling)) zapCmdLineOpts.BindFlags(zapFlagSet) flagSet.AddGoFlagSet(zapFlagSet) @@ -74,3 +80,15 @@ func getHttpsListenAddr(flagSet *pflag.FlagSet) (string, error) { func getHttpsEnabled(flagSet *pflag.FlagSet) (bool, error) { return flagSet.GetBool(httpsEnabledFlagName) } + +func getHttpsCAFilePath(flagSet *pflag.FlagSet) (string, error) { + return flagSet.GetString(httpsCAFilePathFlagName) +} + +func getHttpsTLSCertFilePath(flagSet *pflag.FlagSet) (string, error) { + return flagSet.GetString(httpsTLSCertFilePathFlagName) +} + +func getHttpsTLSKeyFilePath(flagSet *pflag.FlagSet) (string, error) { + return flagSet.GetString(httpsTLSKeyFilePathFlagName) +} diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index cf56f09ed6..78dcc6f647 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -89,7 +89,7 @@ func main() { srv := server.NewServer(log, allocator, cfg.ListenAddr) if cfg.HTTPS.Enabled { - srv = server.NewServer(log, allocator, cfg.ListenAddr, server.WithTLSServer("/certs/ca.crt", "/certs/server.crt", "/certs/server.key", cfg.HTTPS.ListenAddr)) + srv = server.NewServer(log, allocator, cfg.ListenAddr, server.WithHTTPSServer(cfg.HTTPS.CAFilePath, cfg.HTTPS.TLSCertFilePath, cfg.HTTPS.TLSKeyFilePath, cfg.HTTPS.ListenAddr)) } discoveryCtx, discoveryCancel := context.WithCancel(ctx) @@ -184,14 +184,14 @@ func main() { }) runGroup.Add( func() error { - err := srv.StartTls() - setupLog.Info("TLS Server failed to start") + err := srv.StartHTTPS() + setupLog.Info("HTTPS Server failed to start") return err }, func(_ error) { - setupLog.Info("Closing TLS server") - if shutdownErr := srv.ShutdownTls(ctx); shutdownErr != nil { - setupLog.Error(shutdownErr, "Error on TLS server shutdown") + setupLog.Info("Closing HTTPS server") + if shutdownErr := srv.ShutdownHTTPS(ctx); shutdownErr != nil { + setupLog.Error(shutdownErr, "Error on HTTPS server shutdown") } }) runGroup.Add( diff --git a/cmd/otel-allocator/server/bench_test.go b/cmd/otel-allocator/server/bench_test.go index 8fcea90b0e..acb6b2aea3 100644 --- a/cmd/otel-allocator/server/bench_test.go +++ b/cmd/otel-allocator/server/bench_test.go @@ -87,7 +87,7 @@ func BenchmarkScrapeConfigsHandler(b *testing.B) { gin.SetMode(gin.ReleaseMode) c.Request = httptest.NewRequest("GET", "/scrape_configs", nil) - s.ScrapeConfigsHandler(c) + s.ScrapeConfigsHandler(c, false) } }) } diff --git a/cmd/otel-allocator/server/server.go b/cmd/otel-allocator/server/server.go index e9d94bbace..9b7d7e3a1d 100644 --- a/cmd/otel-allocator/server/server.go +++ b/cmd/otel-allocator/server/server.go @@ -67,22 +67,22 @@ type Server struct { logger logr.Logger allocator allocation.Allocator server *http.Server - tlsServer *http.Server + httpsServer *http.Server jsonMarshaller jsoniter.API // Use RWMutex to protect scrapeConfigResponse, since it // will be predominantly read and only written when config // is applied. - mtx sync.RWMutex - scrapeConfigResponse []byte - tlsScrapeConfigResponse []byte + mtx sync.RWMutex + scrapeConfigResponse []byte + ScrapeConfigMarshalledSecretResponse []byte } -type ServerOption func(*Server) +type Option func(*Server) -// ServerOption to create an additional server with mTLS configuration. +// ServerOption to create an additional https server with mTLS configuration. // Used for getting the scrape config with actual secret values. -func WithTLSServer(caFile, certFile, keyFile, tlsListenAddr string) ServerOption { +func WithHTTPSServer(caFile, certFile, keyFile, httpsListenAddr string) Option { return func(s *Server) { cert, err := tls.LoadX509KeyPair(certFile, keyFile) if err != nil { @@ -103,22 +103,21 @@ func WithTLSServer(caFile, certFile, keyFile, tlsListenAddr string) ServerOption ClientCAs: caCertPool, } - tlsRouter := gin.New() - s.setRouter(tlsRouter, true) + httpsRouter := gin.New() + s.setRouter(httpsRouter, true) - s.tlsServer = &http.Server{Addr: tlsListenAddr, Handler: tlsRouter, ReadHeaderTimeout: 90 * time.Second} - s.tlsServer.TLSConfig = tlsConfig + s.httpsServer = &http.Server{Addr: httpsListenAddr, Handler: httpsRouter, ReadHeaderTimeout: 90 * time.Second, TLSConfig: tlsConfig} } } -func (s *Server) setRouter(router *gin.Engine, tlsRouter bool) { +func (s *Server) setRouter(router *gin.Engine, httpsRouter bool) { router.Use(gin.Recovery()) router.UseRawPath = true router.UnescapePathValues = false router.Use(s.PrometheusMiddleware) router.GET("/scrape_configs", func(c *gin.Context) { - s.ScrapeConfigsHandler(c, tlsRouter) + s.ScrapeConfigsHandler(c, httpsRouter) }) router.GET("/jobs", s.JobHandler) router.GET("/jobs/:job_id/targets", s.TargetsHandler) @@ -128,7 +127,7 @@ func (s *Server) setRouter(router *gin.Engine, tlsRouter bool) { registerPprof(router.Group("/debug/pprof/")) } -func NewServer(log logr.Logger, allocator allocation.Allocator, listenAddr string, options ...ServerOption) *Server { +func NewServer(log logr.Logger, allocator allocation.Allocator, listenAddr string, options ...Option) *Server { s := &Server{ logger: log, allocator: allocator, @@ -158,14 +157,14 @@ func (s *Server) Shutdown(ctx context.Context) error { return s.server.Shutdown(ctx) } -func (s *Server) StartTls() error { - s.logger.Info("Starting TLS server...") - return s.tlsServer.ListenAndServeTLS("", "") +func (s *Server) StartHTTPS() error { + s.logger.Info("Starting HTTPS server...") + return s.httpsServer.ListenAndServeTLS("", "") } -func (s *Server) ShutdownTls(ctx context.Context) error { - s.logger.Info("Shutting down TLS server...") - return s.tlsServer.Shutdown(ctx) +func (s *Server) ShutdownHTTPS(ctx context.Context) error { + s.logger.Info("Shutting down HTTPS server...") + return s.httpsServer.Shutdown(ctx) } // RemoveRegexFromRelabelAction is needed specifically for keepequal/dropequal actions because even though the user doesn't specify the @@ -217,7 +216,7 @@ func RemoveRegexFromRelabelAction(jsonConfig []byte) ([]byte, error) { // in to a JSON format for consumers to use. func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.ScrapeConfig) error { marshalSecretValues := []bool{false} - if s.tlsServer != nil { + if s.httpsServer != nil { marshalSecretValues = append(marshalSecretValues, true) } @@ -241,7 +240,7 @@ func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.Scrap s.mtx.Lock() if marshalSecretValue { - s.tlsScrapeConfigResponse = jsonConfigNew + s.ScrapeConfigMarshalledSecretResponse = jsonConfigNew } else { s.scrapeConfigResponse = jsonConfigNew } @@ -252,11 +251,11 @@ func (s *Server) UpdateScrapeConfigResponse(configs map[string]*promconfig.Scrap } // ScrapeConfigsHandler returns the available scrape configuration discovered by the target allocator. -func (s *Server) ScrapeConfigsHandler(c *gin.Context, tlsRouter bool) { +func (s *Server) ScrapeConfigsHandler(c *gin.Context, httpsRouter bool) { s.mtx.RLock() result := s.scrapeConfigResponse - if tlsRouter { - result = s.tlsScrapeConfigResponse + if httpsRouter { + result = s.ScrapeConfigMarshalledSecretResponse } s.mtx.RUnlock() diff --git a/cmd/otel-allocator/server/server_test.go b/cmd/otel-allocator/server/server_test.go index 1a83875cf5..a1c6016c8f 100644 --- a/cmd/otel-allocator/server/server_test.go +++ b/cmd/otel-allocator/server/server_test.go @@ -199,6 +199,7 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) { scrapeConfigs map[string]*promconfig.ScrapeConfig expectedCode int expectedBody []byte + serverOptions []Option }{ { description: "nil scrape config", @@ -455,17 +456,66 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) { }, expectedCode: http.StatusOK, }, + { + description: "https secret handling", + scrapeConfigs: map[string]*promconfig.ScrapeConfig{ + "serviceMonitor/testapp/testapp3/0": { + JobName: "serviceMonitor/testapp/testapp3/0", + HonorTimestamps: true, + ScrapeInterval: model.Duration(30 * time.Second), + ScrapeTimeout: model.Duration(30 * time.Second), + MetricsPath: "/metrics", + Scheme: "http", + HTTPClientConfig: config.HTTPClientConfig{ + FollowRedirects: true, + BasicAuth: &config.BasicAuth{ + Username: "test", + Password: "P@$$w0rd1!?", + }, + }, + }, + }, + expectedCode: http.StatusOK, + serverOptions: []Option{ + WithHTTPSServer("", "", "", ""), + }, + }, + { + description: "https secret handling", + scrapeConfigs: map[string]*promconfig.ScrapeConfig{ + "serviceMonitor/testapp/testapp3/0": { + JobName: "serviceMonitor/testapp/testapp3/0", + HonorTimestamps: true, + ScrapeInterval: model.Duration(30 * time.Second), + ScrapeTimeout: model.Duration(30 * time.Second), + MetricsPath: "/metrics", + Scheme: "http", + HTTPClientConfig: config.HTTPClientConfig{ + FollowRedirects: true, + BasicAuth: &config.BasicAuth{ + Username: "test", + Password: "P@$$w0rd1!?", + }, + }, + }, + }, + expectedCode: http.StatusOK, + }, } for _, tc := range tests { t.Run(tc.description, func(t *testing.T) { listenAddr := ":8080" - s := NewServer(logger, nil, listenAddr) + s := NewServer(logger, nil, listenAddr, tc.serverOptions...) assert.NoError(t, s.UpdateScrapeConfigResponse(tc.scrapeConfigs)) request := httptest.NewRequest("GET", "/scrape_configs", nil) w := httptest.NewRecorder() - s.server.Handler.ServeHTTP(w, request) + if s.httpsServer != nil { + s.httpsServer.Handler.ServeHTTP(w, request) + } else { + s.server.Handler.ServeHTTP(w, request) + } result := w.Result() assert.Equal(t, tc.expectedCode, result.StatusCode) @@ -478,6 +528,19 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) { scrapeConfigs := map[string]*promconfig.ScrapeConfig{} err = yaml.Unmarshal(bodyBytes, scrapeConfigs) require.NoError(t, err) + + for _, c := range scrapeConfigs { + if s.httpsServer == nil && c.HTTPClientConfig.BasicAuth != nil { + assert.Equal(t, c.HTTPClientConfig.BasicAuth.Password, config.Secret("")) + } + } + + for _, c := range tc.scrapeConfigs { + if s.httpsServer == nil && c.HTTPClientConfig.BasicAuth != nil { + c.HTTPClientConfig.BasicAuth.Password = "" + } + } + assert.Equal(t, tc.scrapeConfigs, scrapeConfigs) }) } diff --git a/cmd/otel-allocator/server/testdata/certs/client.crt b/cmd/otel-allocator/server/testdata/certs/client.crt new file mode 100644 index 0000000000..b3d7d0a51c --- /dev/null +++ b/cmd/otel-allocator/server/testdata/certs/client.crt @@ -0,0 +1,7 @@ +-----BEGIN CERTIFICATE----- +MIHYMIGLAgEBMAUGAytlcDAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMjQwNDMw +MTgxMjM0WhcNMjQwNTMwMTgxMjM0WjAdMRswGQYDVQQDDBI8c29tZSBjbGllbnQg +VVVJRD4wKjAFBgMrZXADIQCC5UY+cEq5sPaP6YfTRF3NeEvDWdSNc2yeEuEYeOyx +dzAFBgMrZXADQQBSzzYIhGcULClkRnzj70lvGLKCp9adRjsKFNOLO9+qrgVDURBA +lCT6hFkhpsrbtYJRlLQAJWwFF1FNBBoh/54D +-----END CERTIFICATE----- diff --git a/cmd/otel-allocator/server/testdata/certs/client.csr b/cmd/otel-allocator/server/testdata/certs/client.csr new file mode 100644 index 0000000000..f552185d89 --- /dev/null +++ b/cmd/otel-allocator/server/testdata/certs/client.csr @@ -0,0 +1,6 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIGcMFACAQAwHTEbMBkGA1UEAwwSPHNvbWUgY2xpZW50IFVVSUQ+MCowBQYDK2Vw +AyEAguVGPnBKubD2j+mH00RdzXhLw1nUjXNsnhLhGHjssXegADAFBgMrZXADQQBG +5ySR27+Ql2AY2y6qGwA4buNOKAp757cUzA1TKjF2Y71Zom65umFNVQ/kjEn7RI/4 +Pgm/FuG+1T/f1kwqxxYM +-----END CERTIFICATE REQUEST----- diff --git a/cmd/otel-allocator/server/testdata/certs/client.key b/cmd/otel-allocator/server/testdata/certs/client.key new file mode 100644 index 0000000000..977684f531 --- /dev/null +++ b/cmd/otel-allocator/server/testdata/certs/client.key @@ -0,0 +1,3 @@ +-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEIBfJ284rTnnaI2iGqx5JRFePMRpkfCeu2vg9wBht824T +-----END PRIVATE KEY----- diff --git a/cmd/otel-allocator/server/testdata/certs/file.srl b/cmd/otel-allocator/server/testdata/certs/file.srl new file mode 100644 index 0000000000..8a0f05e166 --- /dev/null +++ b/cmd/otel-allocator/server/testdata/certs/file.srl @@ -0,0 +1 @@ +01 diff --git a/cmd/otel-allocator/server/testdata/certs/server.crt b/cmd/otel-allocator/server/testdata/certs/server.crt new file mode 100644 index 0000000000..c3e99d4841 --- /dev/null +++ b/cmd/otel-allocator/server/testdata/certs/server.crt @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBUzCCAQWgAwIBAgIUEHn6oYd++A6sAWx8ezGS+kt+EaYwBQYDK2VwMBQxEjAQ +BgNVBAMMCWxvY2FsaG9zdDAeFw0yNDA0MzAxODEyMzRaFw0yNTA0MzAxODEyMzRa +MBQxEjAQBgNVBAMMCWxvY2FsaG9zdDAqMAUGAytlcAMhAGwPPAlzRK2zoWVrny5d +DdC+g1XLmaZjnzYzwFFjRfn8o2kwZzAdBgNVHQ4EFgQUCTGaddBsjToe96AKry+Z +LaXS84IwHwYDVR0jBBgwFoAUCTGaddBsjToe96AKry+ZLaXS84IwDwYDVR0TAQH/ +BAUwAwEB/zAUBgNVHREEDTALgglsb2NhbGhvc3QwBQYDK2VwA0EA1IrMqggtEjrt +Tp9os5Sf1Womi0Pli6wIBtfW13uidupAWnRDjLqMSbCBqACf767E1Mqt5B4+wg4i +nNaAn364CA== +-----END CERTIFICATE----- diff --git a/cmd/otel-allocator/server/testdata/certs/server.key b/cmd/otel-allocator/server/testdata/certs/server.key new file mode 100644 index 0000000000..fe41f1d36a --- /dev/null +++ b/cmd/otel-allocator/server/testdata/certs/server.key @@ -0,0 +1,3 @@ +-----BEGIN PRIVATE KEY----- +MC4CAQAwBQYDK2VwBCIEIBYCdtMTl6Zb1WThEW4uvx6t8addD2RVDP8kS7Pj6Auk +-----END PRIVATE KEY----- diff --git a/cmd/otel-allocator/server/testdata/generate_certs.sh b/cmd/otel-allocator/server/testdata/generate_certs.sh new file mode 100644 index 0000000000..b38136eee4 --- /dev/null +++ b/cmd/otel-allocator/server/testdata/generate_certs.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +set -euo pipefail + +# This script generates both client and server certificates and keys + +# get the dir where the script is exec'd from +# so we can move to the 'certs' subdir regardless of where this script is called from +SCRIPT_DIR=$(cd -P -- $(dirname -- $0) && pwd -P) +pushd $SCRIPT_DIR/certs > /dev/null + +# Server Key and Cert +openssl genpkey -algorithm Ed25519 -out server.key +openssl req -new -x509 -sha256 -key server.key -out server.crt -days 365 -subj '/CN=localhost' -addext "subjectAltName = DNS:localhost" + +echo "Server cert and key created" +echo "===========================" +openssl x509 -noout -text -in server.crt +echo "===========================" + +# Client Key and Cert +openssl genpkey -algorithm Ed25519 -out client.key +openssl req -new -key client.key -out client.csr -subj '/CN=' + +# Sign it with the server cert +# IRL you wouldn't do this, the leaf cert for a server would not have the same key as the CA authority +# See https://github.com/joekir/YUBIHSM_mTLS_PKI as an example of that done more thoroughly +echo "00" > file.srl +openssl x509 -req -in client.csr -CA server.crt -CAkey server.key -CAserial file.srl -out client.crt + +echo "Client cert and key created" +echo "===========================" +openssl x509 -noout -text -in client.crt +echo "===========================" + +popd > /dev/null \ No newline at end of file