From 60a474efc4ef7d87490b6e8f6c1ee71585e6fdc9 Mon Sep 17 00:00:00 2001 From: Mahendra Paipuri <44365948+mahendrapaipuri@users.noreply.github.com> Date: Tue, 28 Jan 2025 10:07:05 +0100 Subject: [PATCH 1/3] fix: MarshalYAML receivers on `TLSVersion` and `Curve` * Currently the `MarshalYAML` receivers are defined on pointers of `TLSVersion` and `Curve` but `TLSConfig` is defined with values of `TLSVersion` and `Curve` in its fields. So, `MarshalYAML` receiver is never called upon. This commit fixes it by changing receivers on values which should work on both values and pointers. Signed-off-by: Mahendra Paipuri <44365948+mahendrapaipuri@users.noreply.github.com> --- web/tls_config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/tls_config.go b/web/tls_config.go index 0730a938..df2fd7c7 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -467,7 +467,7 @@ func (c *Curve) UnmarshalYAML(unmarshal func(interface{}) error) error { return errors.New("unknown curve: " + s) } -func (c *Curve) MarshalYAML() (interface{}, error) { +func (c Curve) MarshalYAML() (interface{}, error) { for s, curveid := range curves { if *c == curveid { return s, nil @@ -498,7 +498,7 @@ func (tv *TLSVersion) UnmarshalYAML(unmarshal func(interface{}) error) error { return errors.New("unknown TLS version: " + s) } -func (tv *TLSVersion) MarshalYAML() (interface{}, error) { +func (tv TLSVersion) MarshalYAML() (interface{}, error) { for s, v := range tlsVersions { if *tv == v { return s, nil From c4d055f187092d811fa7c1c27be6310647ba3342 Mon Sep 17 00:00:00 2001 From: Mahendra Paipuri <44365948+mahendrapaipuri@users.noreply.github.com> Date: Sat, 1 Feb 2025 08:45:59 +0100 Subject: [PATCH 2/3] Use values inside non-pointer MarshalYAML receivers Signed-off-by: Mahendra Paipuri <44365948+mahendrapaipuri@users.noreply.github.com> --- web/tls_config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/tls_config.go b/web/tls_config.go index df2fd7c7..3c1a4f7b 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -469,7 +469,7 @@ func (c *Curve) UnmarshalYAML(unmarshal func(interface{}) error) error { func (c Curve) MarshalYAML() (interface{}, error) { for s, curveid := range curves { - if *c == curveid { + if c == curveid { return s, nil } } @@ -500,7 +500,7 @@ func (tv *TLSVersion) UnmarshalYAML(unmarshal func(interface{}) error) error { func (tv TLSVersion) MarshalYAML() (interface{}, error) { for s, v := range tlsVersions { - if *tv == v { + if tv == v { return s, nil } } From 323787d82a76267fd4f4e8be6e31627f17fd4435 Mon Sep 17 00:00:00 2001 From: Mahendra Paipuri Date: Thu, 6 Feb 2025 12:21:42 +0100 Subject: [PATCH 3/3] tests: Add unit tests to verify config generation from structs * Add omitempty tags to all fields in the config to avoid rendering zero values in generated config Signed-off-by: Mahendra Paipuri --- web/tls_config.go | 32 +++++------ web/tls_config_test.go | 127 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 16 deletions(-) diff --git a/web/tls_config.go b/web/tls_config.go index 3c1a4f7b..7fd812a4 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -40,25 +40,25 @@ var ( ) type Config struct { - TLSConfig TLSConfig `yaml:"tls_server_config"` - HTTPConfig HTTPConfig `yaml:"http_server_config"` - Users map[string]config_util.Secret `yaml:"basic_auth_users"` + TLSConfig TLSConfig `yaml:"tls_server_config,omitempty"` + HTTPConfig HTTPConfig `yaml:"http_server_config,omitempty"` + Users map[string]config_util.Secret `yaml:"basic_auth_users,omitempty"` } type TLSConfig struct { - TLSCert string `yaml:"cert"` - TLSKey config_util.Secret `yaml:"key"` - ClientCAsText string `yaml:"client_ca"` - TLSCertPath string `yaml:"cert_file"` - TLSKeyPath string `yaml:"key_file"` - ClientAuth string `yaml:"client_auth_type"` - ClientCAs string `yaml:"client_ca_file"` - CipherSuites []Cipher `yaml:"cipher_suites"` - CurvePreferences []Curve `yaml:"curve_preferences"` - MinVersion TLSVersion `yaml:"min_version"` - MaxVersion TLSVersion `yaml:"max_version"` - PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites"` - ClientAllowedSans []string `yaml:"client_allowed_sans"` + TLSCert string `yaml:"cert,omitempty"` + TLSKey config_util.Secret `yaml:"key,omitempty"` + ClientCAsText string `yaml:"client_ca,omitempty"` + TLSCertPath string `yaml:"cert_file,omitempty"` + TLSKeyPath string `yaml:"key_file,omitempty"` + ClientAuth string `yaml:"client_auth_type,omitempty"` + ClientCAs string `yaml:"client_ca_file,omitempty"` + CipherSuites []Cipher `yaml:"cipher_suites,omitempty"` + CurvePreferences []Curve `yaml:"curve_preferences,omitempty"` + MinVersion TLSVersion `yaml:"min_version,omitempty"` + MaxVersion TLSVersion `yaml:"max_version,omitempty"` + PreferServerCipherSuites bool `yaml:"prefer_server_cipher_suites,omitempty"` + ClientAllowedSans []string `yaml:"client_allowed_sans,omitempty"` } type FlagConfig struct { diff --git a/web/tls_config_test.go b/web/tls_config_test.go index b28c6671..cd0884cb 100644 --- a/web/tls_config_test.go +++ b/web/tls_config_test.go @@ -27,9 +27,13 @@ import ( "net/http" "os" "regexp" + "strings" "sync" "testing" "time" + + "github.com/prometheus/common/config" + "gopkg.in/yaml.v2" ) // Helpers for literal FlagConfig @@ -693,3 +697,126 @@ func TestUsers(t *testing.T) { t.Run(testInputs.Name, testInputs.Test) } } + +func TestConfigGeneration(t *testing.T) { + // Secrets to be rendered without any masking + config.MarshalSecretValue = true + + testTables := []struct { + Name string + Config Config + Expected string + }{ + { + Name: "Only basic auth", + Config: Config{ + Users: map[string]config.Secret{ + "admin": config.Secret("$2y$10$X0h1gDsPszWURQaxFh.zoubFi6DXncSjhoQNJgRrnGs7EsimhC7zG"), + }, + }, + Expected: ` +basic_auth_users: + admin: $2y$10$X0h1gDsPszWURQaxFh.zoubFi6DXncSjhoQNJgRrnGs7EsimhC7zG`, + }, + { + Name: "Only TLS", + Config: Config{ + TLSConfig: TLSConfig{ + TLSCertPath: "cert.pem", + TLSKeyPath: "key.pem", + MinVersion: TLSVersion(tls.VersionTLS12), + CurvePreferences: []Curve{ + Curve(tls.CurveP256), + Curve(tls.CurveP521), + }, + CipherSuites: []Cipher{ + Cipher(tls.TLS_AES_128_GCM_SHA256), + }, + ClientAllowedSans: []string{ + "example.com", + "example.org", + }, + }, + }, + Expected: ` +tls_server_config: + cert_file: cert.pem + key_file: key.pem + cipher_suites: + - TLS_AES_128_GCM_SHA256 + curve_preferences: + - CurveP256 + - CurveP521 + min_version: TLS12 + client_allowed_sans: + - example.com + - example.org`, + }, + { + Name: "Only HTTP config", + Config: Config{ + HTTPConfig: HTTPConfig{ + HTTP2: true, + Header: map[string]string{ + "X-Custom-Header": "value", + }, + }, + }, + Expected: ` +http_server_config: + http2: true + headers: + X-Custom-Header: value`, + }, + { + Name: "Basic auth and TLS", + Config: Config{ + Users: map[string]config.Secret{ + "admin": config.Secret("$2y$10$X0h1gDsPszWURQaxFh.zoubFi6DXncSjhoQNJgRrnGs7EsimhC7zG"), + }, + TLSConfig: TLSConfig{ + TLSCertPath: "cert.pem", + TLSKeyPath: "key.pem", + MinVersion: TLSVersion(tls.VersionTLS12), + CurvePreferences: []Curve{ + Curve(tls.CurveP256), + Curve(tls.CurveP521), + }, + CipherSuites: []Cipher{ + Cipher(tls.TLS_AES_128_GCM_SHA256), + }, + ClientAllowedSans: []string{ + "example.com", + "example.org", + }, + }, + }, + Expected: ` +tls_server_config: + cert_file: cert.pem + key_file: key.pem + cipher_suites: + - TLS_AES_128_GCM_SHA256 + curve_preferences: + - CurveP256 + - CurveP521 + min_version: TLS12 + client_allowed_sans: + - example.com + - example.org +basic_auth_users: + admin: $2y$10$X0h1gDsPszWURQaxFh.zoubFi6DXncSjhoQNJgRrnGs7EsimhC7zG`, + }, + } + + for _, test := range testTables { + yamlConfig, err := yaml.Marshal(&test.Config) + if err != nil { + t.Error(err) + } + + if strings.TrimSpace(test.Expected) != strings.TrimSpace(string(yamlConfig)) { + t.Fatalf("Expected config: %s, got config: %s", test.Expected, string(yamlConfig)) + } + } +}