From 745274eefff7f422867f3d6c50f35c7e356d6aac Mon Sep 17 00:00:00 2001 From: Till Wulfram Date: Sun, 17 May 2026 23:33:02 +0200 Subject: [PATCH] feat(target-allocator): add allowInsecureAuthSecrets option --- cmd/otel-allocator/README.md | 19 +++ cmd/otel-allocator/internal/config/config.go | 10 ++ .../internal/config/config_test.go | 113 ++++++++++++++++++ cmd/otel-allocator/internal/config/flags.go | 26 ++-- .../internal/config/flags_test.go | 10 ++ cmd/otel-allocator/internal/server/server.go | 15 ++- .../internal/server/server_test.go | 29 ++++- cmd/otel-allocator/main.go | 3 + 8 files changed, 212 insertions(+), 13 deletions(-) diff --git a/cmd/otel-allocator/README.md b/cmd/otel-allocator/README.md index d1dcea3091..0a4ef36623 100644 --- a/cmd/otel-allocator/README.md +++ b/cmd/otel-allocator/README.md @@ -27,6 +27,7 @@ The Target Allocator uses a configuration file (by default under `/conf/targetal | `filter_strategy` | Filter strategy to apply to metrics | `relabel-config` | | | `prometheus_cr` | Whether to watch Prometheus Custom Resources | | | | `https` | Whether to expose the target allocator endpoint over https | | | +| `allow_insecure_auth_secrets` | Serve auth secret values over plain HTTP without mTLS | `false` | `ALLOW_INSECURE_AUTH_SECRETS` | | `collector_not_ready_grace_period` | Wait time before assigning jobs to a new collector. | 30s | | Additional configuration options are present under [./internal/config/config.go](./internal/config/config.go). @@ -423,6 +424,24 @@ Prerequisites: - Enable the `operator.targetallocator.mtls` feature gate in the operator's deployment. +#### Alternative: allow insecure auth secrets + +If the connection between the target allocator and collectors is already secured by a service mesh (e.g. Istio, Linkerd) or equivalent transport-level security, you can skip mTLS setup and serve auth secret values over plain HTTP instead. + +Set `allow_insecure_auth_secrets: true` in the target allocator config file, pass `--allow-insecure-auth-secrets` as a CLI flag, or set the `ALLOW_INSECURE_AUTH_SECRETS=true` environment variable. + +When using the `OpenTelemetryCollector` CR with an embedded target allocator, set it via the `env` field: + +```yaml +targetAllocator: + enabled: true + env: + - name: ALLOW_INSECURE_AUTH_SECRETS + value: "true" +``` + +> **Warning:** Only enable this when transport-level security is guaranteed by other means. Without mTLS or a service mesh, auth secrets will be transmitted in plaintext. + # Design diff --git a/cmd/otel-allocator/internal/config/config.go b/cmd/otel-allocator/internal/config/config.go index d3abbeb8b3..9cd8862472 100644 --- a/cmd/otel-allocator/internal/config/config.go +++ b/cmd/otel-allocator/internal/config/config.go @@ -72,6 +72,7 @@ type Config struct { PrometheusCR PrometheusCRConfig `yaml:"prometheus_cr,omitempty"` HTTPS HTTPSServerConfig `yaml:"https,omitempty"` CollectorNotReadyGracePeriod time.Duration `yaml:"collector_not_ready_grace_period,omitempty"` + AllowInsecureAuthSecrets bool `yaml:"allow_insecure_auth_secrets,omitempty"` } type PrometheusCRConfig struct { @@ -268,6 +269,12 @@ func LoadFromCLI(target *Config, flagSet *pflag.FlagSet) error { target.HTTPS.TLSKeyFilePath = tlsKeyFilePath } + if allowInsecureAuthSecrets, changed, err := getAllowInsecureAuthSecrets(flagSet); err != nil { + return err + } else if changed { + target.AllowInsecureAuthSecrets = allowInsecureAuthSecrets + } + return nil } @@ -276,6 +283,9 @@ func LoadFromEnv(target *Config) error { if ns, ok := os.LookupEnv("OTELCOL_NAMESPACE"); ok { target.CollectorNamespace = ns } + if val, ok := os.LookupEnv("ALLOW_INSECURE_AUTH_SECRETS"); ok && val == "true" { + target.AllowInsecureAuthSecrets = true + } return nil } diff --git a/cmd/otel-allocator/internal/config/config_test.go b/cmd/otel-allocator/internal/config/config_test.go index 84ce42c1e5..1cc31c66d6 100644 --- a/cmd/otel-allocator/internal/config/config_test.go +++ b/cmd/otel-allocator/internal/config/config_test.go @@ -691,6 +691,31 @@ func TestLoadFromEnv(t *testing.T) { assert.Equal(t, namespace, cfg.CollectorNamespace) } +func TestLoadFromEnvAllowInsecureAuthSecrets(t *testing.T) { + t.Run("not set defaults to false", func(t *testing.T) { + cfg := &Config{} + err := LoadFromEnv(cfg) + require.NoError(t, err) + assert.False(t, cfg.AllowInsecureAuthSecrets) + }) + + t.Run("set to true", func(t *testing.T) { + t.Setenv("ALLOW_INSECURE_AUTH_SECRETS", "true") + cfg := &Config{} + err := LoadFromEnv(cfg) + require.NoError(t, err) + assert.True(t, cfg.AllowInsecureAuthSecrets) + }) + + t.Run("set to false", func(t *testing.T) { + t.Setenv("ALLOW_INSECURE_AUTH_SECRETS", "false") + cfg := &Config{} + err := LoadFromEnv(cfg) + require.NoError(t, err) + assert.False(t, cfg.AllowInsecureAuthSecrets) + }) +} + func TestValidateConfig(t *testing.T) { testCases := []struct { name string @@ -1086,3 +1111,91 @@ kube_config_file_path: "` + kubeConfigPath + `" assert.True(t, config.PrometheusCR.Enabled, "CLI should override config file for prometheus CR enabled") }) } + +func TestAllowInsecureAuthSecrets(t *testing.T) { + createDummyKubeConfig := func(t *testing.T, dir string) string { + kubeConfigPath := filepath.Join(dir, "kube.config") + kubeConfigContent := ` +apiVersion: v1 +kind: Config +clusters: +- cluster: + server: https://example.com + name: test-cluster +contexts: +- context: + cluster: test-cluster + user: test-user + name: test-context +current-context: test-context +users: +- name: test-user + user: + token: dummy-token +` + err := os.WriteFile(kubeConfigPath, []byte(kubeConfigContent), 0o600) + require.NoError(t, err) + return kubeConfigPath + } + + t.Run("defaults to false", func(t *testing.T) { + tempDir := t.TempDir() + emptyConfigPath := filepath.Join(tempDir, "empty.yaml") + err := os.WriteFile(emptyConfigPath, []byte("{}"), 0o600) + require.NoError(t, err) + + kubeConfigPath := createDummyKubeConfig(t, tempDir) + + args := []string{ + "--" + configFilePathFlagName + "=" + emptyConfigPath, + "--" + kubeConfigPathFlagName + "=" + kubeConfigPath, + } + + config, err := Load(args) + require.NoError(t, err) + assert.False(t, config.AllowInsecureAuthSecrets) + }) + + t.Run("can be enabled via config file", func(t *testing.T) { + tempDir := t.TempDir() + kubeConfigPath := createDummyKubeConfig(t, tempDir) + + configContent := fmt.Sprintf(` +collector_namespace: default +allow_insecure_auth_secrets: true +kube_config_file_path: %q +`, kubeConfigPath) + configPath := filepath.Join(tempDir, "config.yaml") + err := os.WriteFile(configPath, []byte(configContent), 0o600) + require.NoError(t, err) + + args := []string{"--" + configFilePathFlagName + "=" + configPath} + + config, err := Load(args) + require.NoError(t, err) + assert.True(t, config.AllowInsecureAuthSecrets) + }) + + t.Run("CLI flag overrides config file", func(t *testing.T) { + tempDir := t.TempDir() + kubeConfigPath := createDummyKubeConfig(t, tempDir) + + configContent := fmt.Sprintf(` +collector_namespace: default +allow_insecure_auth_secrets: false +kube_config_file_path: %q +`, kubeConfigPath) + configPath := filepath.Join(tempDir, "config.yaml") + err := os.WriteFile(configPath, []byte(configContent), 0o600) + require.NoError(t, err) + + args := []string{ + "--" + configFilePathFlagName + "=" + configPath, + "--" + allowInsecureAuthSecretsFlagName + "=true", + } + + config, err := Load(args) + require.NoError(t, err) + assert.True(t, config.AllowInsecureAuthSecrets, "CLI flag should override config file") + }) +} diff --git a/cmd/otel-allocator/internal/config/flags.go b/cmd/otel-allocator/internal/config/flags.go index bc9f7ab99c..e71332d407 100644 --- a/cmd/otel-allocator/internal/config/flags.go +++ b/cmd/otel-allocator/internal/config/flags.go @@ -12,16 +12,17 @@ import ( // Flag names. const ( - 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" + 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" + allowInsecureAuthSecretsFlagName = "allow-insecure-auth-secrets" ) // We can't bind this flag to our FlagSet, so we need to handle it separately. @@ -38,6 +39,7 @@ func getFlagSet(errorHandling pflag.ErrorHandling) *pflag.FlagSet { flagSet.String(httpsCAFilePathFlagName, "", "The path to the HTTPS server TLS CA file.") flagSet.String(httpsTLSCertFilePathFlagName, "", "The path to the HTTPS server TLS certificate file.") flagSet.String(httpsTLSKeyFilePathFlagName, "", "The path to the HTTPS server TLS key file.") + flagSet.Bool(allowInsecureAuthSecretsFlagName, false, "Serve scrape configs with secret values over plain HTTP without mTLS. Only enable when transport is secured by a service mesh or equivalent.") zapFlagSet := flag.NewFlagSet("", flag.ErrorHandling(errorHandling)) zapCmdLineOpts.BindFlags(zapFlagSet) flagSet.AddGoFlagSet(zapFlagSet) @@ -80,6 +82,10 @@ func getHttpsTLSKeyFilePath(flagSet *pflag.FlagSet) (value string, changed bool, return getFlagValueAndChangedString(flagSet, httpsTLSKeyFilePathFlagName) } +func getAllowInsecureAuthSecrets(flagSet *pflag.FlagSet) (value, changed bool, err error) { + return getFlagValueAndChangedBool(flagSet, allowInsecureAuthSecretsFlagName) +} + // getFlagValueAndChanged returns the given flag's string value and whether it was changed. func getFlagValueAndChangedString(flagSet *pflag.FlagSet, flagName string) (value string, changed bool, err error) { if changed = flagSet.Changed(flagName); !changed { diff --git a/cmd/otel-allocator/internal/config/flags_test.go b/cmd/otel-allocator/internal/config/flags_test.go index d1802b04f9..2740097cad 100644 --- a/cmd/otel-allocator/internal/config/flags_test.go +++ b/cmd/otel-allocator/internal/config/flags_test.go @@ -19,6 +19,7 @@ func TestGetFlagSet(t *testing.T) { assert.NotNil(t, fs.Lookup(listenAddrFlagName), "Flag %s not found", listenAddrFlagName) assert.NotNil(t, fs.Lookup(prometheusCREnabledFlagName), "Flag %s not found", prometheusCREnabledFlagName) assert.NotNil(t, fs.Lookup(kubeConfigPathFlagName), "Flag %s not found", kubeConfigPathFlagName) + assert.NotNil(t, fs.Lookup(allowInsecureAuthSecretsFlagName), "Flag %s not found", allowInsecureAuthSecretsFlagName) } func TestFlagGetters(t *testing.T) { @@ -86,6 +87,15 @@ func TestFlagGetters(t *testing.T) { return value, err }, }, + { + name: "AllowInsecureAuthSecrets", + flagArgs: []string{"--" + allowInsecureAuthSecretsFlagName, "true"}, + expectedValue: true, + getterFunc: func(fs *pflag.FlagSet) (any, error) { + value, _, err := getAllowInsecureAuthSecrets(fs) + return value, err + }, + }, } for _, tt := range tests { diff --git a/cmd/otel-allocator/internal/server/server.go b/cmd/otel-allocator/internal/server/server.go index f8eefea5ed..24330adc80 100644 --- a/cmd/otel-allocator/internal/server/server.go +++ b/cmd/otel-allocator/internal/server/server.go @@ -8,6 +8,7 @@ import ( "context" "crypto/tls" "fmt" + "log/slog" "net/http" "net/http/pprof" "net/url" @@ -61,6 +62,7 @@ type Server struct { scrapeConfigResponse []byte ScrapeConfigMarshalledSecretResponse []byte httpDuration metric.Float64Histogram + allowInsecureAuthSecrets bool } type Option func(*Server) @@ -76,6 +78,12 @@ func WithTLSConfig(tlsConfig *tls.Config, httpsListenAddr string) Option { } } +func WithInsecureAuthSecrets() Option { + return func(s *Server) { + s.allowInsecureAuthSecrets = true + } +} + func (s *Server) setRouter(router *gin.Engine) { router.Use(gin.Recovery()) router.UseRawPath = true @@ -121,6 +129,11 @@ func NewServer(log logr.Logger, allocator allocation.Allocator, listenAddr strin opt(s) } + if s.allowInsecureAuthSecrets { + slog.New(logr.ToSlogHandler(s.logger)).Warn("allowInsecureAuthSecrets is enabled - auth secret values will be served over plain HTTP. " + + "Only use this when the target allocator endpoint is secured by a service mesh or equivalent transport-level security.") + } + return s, nil } @@ -241,7 +254,7 @@ func (s *Server) ScrapeConfigsHandler(c *gin.Context) { } s.mtx.RLock() result := s.scrapeConfigResponse - if c.Request.TLS != nil { + if c.Request.TLS != nil || s.allowInsecureAuthSecrets { result = s.ScrapeConfigMarshalledSecretResponse } s.mtx.RUnlock() diff --git a/cmd/otel-allocator/internal/server/server_test.go b/cmd/otel-allocator/internal/server/server_test.go index 6b7de119b6..26bd3c4751 100644 --- a/cmd/otel-allocator/internal/server/server_test.go +++ b/cmd/otel-allocator/internal/server/server_test.go @@ -485,6 +485,30 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) { }, expectedCode: http.StatusOK, }, + { + description: "http with allow-insecure-auth-secrets serves real secret values", + 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{ + WithInsecureAuthSecrets(), + }, + }, } for _, tc := range tests { t.Run(tc.description, func(t *testing.T) { @@ -514,14 +538,15 @@ func TestServer_ScrapeConfigsHandler(t *testing.T) { err = yaml.Unmarshal(bodyBytes, scrapeConfigs) require.NoError(t, err) + serveSecrets := s.httpsServer != nil || s.allowInsecureAuthSecrets for _, c := range scrapeConfigs { - if s.httpsServer == nil && c.HTTPClientConfig.BasicAuth != nil { + if !serveSecrets && 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 { + if !serveSecrets && c.HTTPClientConfig.BasicAuth != nil { c.HTTPClientConfig.BasicAuth.Password = "" } } diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index 4fd948cfed..b2178e671e 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -106,6 +106,9 @@ func main() { } httpOptions = append(httpOptions, server.WithTLSConfig(tlsConfig, cfg.HTTPS.ListenAddr)) } + if cfg.AllowInsecureAuthSecrets { + httpOptions = append(httpOptions, server.WithInsecureAuthSecrets()) + } srv, serverErr := server.NewServer(log, allocator, cfg.ListenAddr, httpOptions...) if serverErr != nil { panic(serverErr)