Added some tests

open-telemetry · Apr 30, 2024 · 0551f39 · 0551f39
1 parent 72e0174
commit 0551f39
Show file tree

Hide file tree

Showing 13 changed files with 216 additions and 55 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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("<secret>"))
+				}
+			}
+
+			for _, c := range tc.scrapeConfigs {
+				if s.httpsServer == nil && c.HTTPClientConfig.BasicAuth != nil {
+					c.HTTPClientConfig.BasicAuth.Password = "<secret>"
+				}
+			}
+
 			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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1 @@
+01