Skip to content

Ta https server #2921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .chloggen/ta-add-https.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: target allocator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Added option for creating an mTLS-configured HTTPS server to fetch scrape config with real secret values.

# One or more tracking issues related to the change
issues: [1669]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
The change introduces an option to create an additional HTTPS server with mTLS configuration.
This server is specifically utilized for obtaining the scrape configuration with actual secret values.
59 changes: 59 additions & 0 deletions cmd/otel-allocator/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package config

import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"io/fs"
Expand Down Expand Up @@ -53,6 +55,7 @@ type Config struct {
AllocationStrategy string `yaml:"allocation_strategy,omitempty"`
FilterStrategy string `yaml:"filter_strategy,omitempty"`
PrometheusCR PrometheusCRConfig `yaml:"prometheus_cr,omitempty"`
HTTPS HTTPSServerConfig `yaml:"https,omitempty"`
}

type PrometheusCRConfig struct {
Expand All @@ -64,6 +67,14 @@ type PrometheusCRConfig struct {
ScrapeInterval model.Duration `yaml:"scrape_interval,omitempty"`
}

type HTTPSServerConfig struct {
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 {
return unmarshal(target, file)
}
Expand Down Expand Up @@ -103,6 +114,31 @@ func LoadFromCLI(target *Config, flagSet *pflag.FlagSet) error {
return err
}

target.HTTPS.Enabled, err = getHttpsEnabled(flagSet)
if err != nil {
return err
}

target.HTTPS.ListenAddr, err = getHttpsListenAddr(flagSet)
if err != nil {
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
}

Expand Down Expand Up @@ -164,3 +200,26 @@ func ValidateConfig(config *Config) error {
}
return nil
}

func (c HTTPSServerConfig) NewTLSConfig() (*tls.Config, error) {
cert, err := tls.LoadX509KeyPair(c.TLSCertFilePath, c.TLSKeyFilePath)
if err != nil {
return nil, err
}

caCert, err := os.ReadFile(c.CAFilePath)
if err != nil {
return nil, err
}

caCertPool := x509.NewCertPool()
caCertPool.AppendCertsFromPEM(caCert)

tlsConfig := &tls.Config{
Certificates: []tls.Certificate{cert},
ClientAuth: tls.RequireAndVerifyClientCert,
ClientCAs: caCertPool,
MinVersion: tls.VersionTLS12,
}
return tlsConfig, nil
}
6 changes: 6 additions & 0 deletions cmd/otel-allocator/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ func TestLoad(t *testing.T) {
PrometheusCR: PrometheusCRConfig{
ScrapeInterval: model.Duration(time.Second * 60),
},
HTTPS: HTTPSServerConfig{
Enabled: true,
CAFilePath: "/path/to/ca.pem",
TLSCertFilePath: "/path/to/cert.pem",
TLSKeyFilePath: "/path/to/key.pem",
},
PromConfig: &promconfig.Config{
GlobalConfig: promconfig.GlobalConfig{
ScrapeInterval: model.Duration(60 * time.Second),
Expand Down
40 changes: 35 additions & 5 deletions cmd/otel-allocator/config/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,16 @@ import (

// Flag names.
const (
targetAllocatorName = "target-allocator"
configFilePathFlagName = "config-file"
listenAddrFlagName = "listen-addr"
prometheusCREnabledFlagName = "enable-prometheus-cr-watcher"
kubeConfigPathFlagName = "kubeconfig-path"
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.
Expand All @@ -41,6 +46,11 @@ func getFlagSet(errorHandling pflag.ErrorHandling) *pflag.FlagSet {
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(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.")
zapFlagSet := flag.NewFlagSet("", flag.ErrorHandling(errorHandling))
zapCmdLineOpts.BindFlags(zapFlagSet)
flagSet.AddGoFlagSet(zapFlagSet)
Expand All @@ -62,3 +72,23 @@ func getListenAddr(flagSet *pflag.FlagSet) (string, error) {
func getPrometheusCREnabled(flagSet *pflag.FlagSet) (bool, error) {
return flagSet.GetBool(prometheusCREnabledFlagName)
}

func getHttpsListenAddr(flagSet *pflag.FlagSet) (string, error) {
return flagSet.GetString(listenAddrHttpsFlagName)
}

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)
}
12 changes: 12 additions & 0 deletions cmd/otel-allocator/config/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ func TestFlagGetters(t *testing.T) {
expectedErr: true,
getterFunc: func(fs *pflag.FlagSet) (interface{}, error) { return getConfigFilePath(fs) },
},
{
name: "HttpsServer",
flagArgs: []string{"--" + httpsEnabledFlagName, "true"},
expectedValue: true,
getterFunc: func(fs *pflag.FlagSet) (interface{}, error) { return getHttpsEnabled(fs) },
},
{
name: "HttpsServerKey",
flagArgs: []string{"--" + httpsTLSKeyFilePathFlagName, "/path/to/tls.key"},
expectedValue: "/path/to/tls.key",
getterFunc: func(fs *pflag.FlagSet) (interface{}, error) { return getHttpsTLSKeyFilePath(fs) },
},
}

for _, tt := range tests {
Expand Down
5 changes: 5 additions & 0 deletions cmd/otel-allocator/config/testdata/config_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ collector_selector:
app.kubernetes.io/managed-by: opentelemetry-operator
prometheus_cr:
scrape_interval: 60s
https:
enabled: true
ca_file_path: /path/to/ca.pem
tls_cert_file_path: /path/to/cert.pem
tls_key_file_path: /path/to/key.pem
config:
scrape_configs:
- job_name: prometheus
Expand Down
26 changes: 25 additions & 1 deletion cmd/otel-allocator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,17 @@ func main() {
setupLog.Error(err, "Unable to initialize allocation strategy")
os.Exit(1)
}
srv := server.NewServer(log, allocator, cfg.ListenAddr)

httpOptions := []server.Option{}
if cfg.HTTPS.Enabled {
tlsConfig, confErr := cfg.HTTPS.NewTLSConfig()
if confErr != nil {
setupLog.Error(confErr, "Unable to initialize TLS configuration")
os.Exit(1)
}
httpOptions = append(httpOptions, server.WithTLSConfig(tlsConfig, cfg.HTTPS.ListenAddr))
}
srv := server.NewServer(log, allocator, cfg.ListenAddr, httpOptions...)

discoveryCtx, discoveryCancel := context.WithCancel(ctx)
sdMetrics, err := discovery.CreateAndRegisterSDMetrics(prometheus.DefaultRegisterer)
Expand Down Expand Up @@ -189,6 +199,20 @@ func main() {
setupLog.Error(shutdownErr, "Error on server shutdown")
}
})
if cfg.HTTPS.Enabled {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to put the above srv.Start() in an else block or are we okay to potentially run both a secure and insecure simultaneously? I guess doing it this way would make for a better migration from http to https.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on previous conversation, it was suggested to have both HTTP and HTTPS running at the same time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the idea is to keep both and only serve secrets over HTTPS. The HTTP service is useful for debugging, for HTTPS you need to do some work to set up mTLS locally.

runGroup.Add(
func() error {
err := srv.StartHTTPS()
setupLog.Info("HTTPS Server failed to start")
return err
},
func(_ error) {
setupLog.Info("Closing HTTPS server")
if shutdownErr := srv.ShutdownHTTPS(ctx); shutdownErr != nil {
setupLog.Error(shutdownErr, "Error on HTTPS server shutdown")
}
})
}
runGroup.Add(
func() error {
for {
Expand Down
Loading
Loading