diff --git a/cmd/testdata/configurations/clickhouse-network-sources/expected.yaml b/cmd/testdata/configurations/clickhouse-network-sources/expected.yaml index b0d914b1..e480c13f 100644 --- a/cmd/testdata/configurations/clickhouse-network-sources/expected.yaml +++ b/cmd/testdata/configurations/clickhouse-network-sources/expected.yaml @@ -4,7 +4,7 @@ paths: url: https://ip-ranges.amazonaws.com/ip-ranges.json tls: enable: false - verify: true + skipverify: false cafile: "" certfile: "" keyfile: "" diff --git a/cmd/testdata/configurations/orchestrator-clickhousedb/expected.yaml b/cmd/testdata/configurations/orchestrator-clickhousedb/expected.yaml index be3ab99d..6badc6f5 100644 --- a/cmd/testdata/configurations/orchestrator-clickhousedb/expected.yaml +++ b/cmd/testdata/configurations/orchestrator-clickhousedb/expected.yaml @@ -12,7 +12,7 @@ paths: dialtimeout: 5s tls: enable: true - verify: true + skipverify: false cafile: "" certfile: "" keyfile: "" diff --git a/common/clickhousedb/config.go b/common/clickhousedb/config.go index 04fa78fa..719112ee 100644 --- a/common/clickhousedb/config.go +++ b/common/clickhousedb/config.go @@ -41,10 +41,6 @@ func DefaultConfiguration() Configuration { Username: "default", MaxOpenConns: 10, DialTimeout: 5 * time.Second, - TLS: helpers.TLSConfiguration{ - Enable: false, - Verify: true, - }, } } diff --git a/common/helpers/tls.go b/common/helpers/tls.go index a0a374ae..9e1e8c06 100644 --- a/common/helpers/tls.go +++ b/common/helpers/tls.go @@ -9,17 +9,20 @@ import ( "errors" "fmt" "os" + "reflect" + + "github.com/go-viper/mapstructure/v2" ) // TLSConfiguration defines TLS configuration. type TLSConfiguration struct { // Enable says if TLS should be used to connect to remote servers. Enable bool `validate:"required_with=CAFile CertFile KeyFile"` - // Verify says if we need to check remote certificates - Verify bool + // SkipVerify removes validity checks of remote certificates + SkipVerify bool // CAFile tells the location of the CA certificate to check broker // certificate. If empty, the system CA certificates are used instead. - CAFile string // no validation as the orchestrator may not have the file + CAFile string // no file as the orchestrator may not have the file // CertFile tells the location of the user certificate if any. CertFile string `validate:"required_with=KeyFile"` // KeyFile tells the location of the user key if any. @@ -33,7 +36,7 @@ func (config TLSConfiguration) MakeTLSConfig() (*tls.Config, error) { return nil, nil } tlsConfig := &tls.Config{ - InsecureSkipVerify: !config.Verify, + InsecureSkipVerify: config.SkipVerify, } // Read CA certificate if provided if config.CAFile != "" { @@ -60,3 +63,45 @@ func (config TLSConfiguration) MakeTLSConfig() (*tls.Config, error) { } return tlsConfig, nil } + +// RenameKeyUnmarshallerHook move a configuration setting from one place to another. +func tlsUnmarshallerHook() mapstructure.DecodeHookFunc { + var zeroConfiguration TLSConfiguration + return func(from, to reflect.Value) (any, error) { + if from.Kind() != reflect.Map || from.IsNil() || to.Type() != reflect.TypeOf(zeroConfiguration) { + return from.Interface(), nil + } + + // verify → skip-verify + var verifyKey, skipVerifyKey *reflect.Value + fromMap := from.MapKeys() + for i, k := range fromMap { + k = ElemOrIdentity(k) + if k.Kind() != reflect.String { + return from.Interface(), nil + } + if MapStructureMatchName(k.String(), "Verify") { + verifyKey = &fromMap[i] + } else if MapStructureMatchName(k.String(), "SkipVerify") { + skipVerifyKey = &fromMap[i] + } + } + if verifyKey != nil && skipVerifyKey != nil { + return nil, fmt.Errorf("cannot have both %q and %q", verifyKey.String(), skipVerifyKey.String()) + } + if verifyKey != nil { + value := ElemOrIdentity(from.MapIndex(*verifyKey)) + if value.Kind() != reflect.Bool { + return from.Interface(), nil + } + from.SetMapIndex(reflect.ValueOf("skip-verify"), reflect.ValueOf(!value.Bool())) + from.SetMapIndex(*verifyKey, reflect.Value{}) + } + + return from.Interface(), nil + } +} + +func init() { + RegisterMapstructureUnmarshallerHook(tlsUnmarshallerHook()) +} diff --git a/common/helpers/tls_test.go b/common/helpers/tls_test.go new file mode 100644 index 00000000..50d02306 --- /dev/null +++ b/common/helpers/tls_test.go @@ -0,0 +1,80 @@ +// SPDX-FileCopyrightText: 2025 Free Mobile +// SPDX-License-Identifier: AGPL-3.0-only + +package helpers_test + +import ( + "testing" + + "github.com/gin-gonic/gin" + + "akvorado/common/helpers" +) + +func TestTLSConfigurationMigration(t *testing.T) { + helpers.TestConfigurationDecode(t, helpers.ConfigurationDecodeCases{ + { + Description: "new skip-verify field", + Initial: func() any { return helpers.TLSConfiguration{} }, + Configuration: func() any { + return gin.H{ + "enable": true, + "skip-verify": true, + } + }, + Expected: helpers.TLSConfiguration{ + Enable: true, + SkipVerify: true, + }, + }, { + Description: "no verify/skip-verify", + Initial: func() any { return helpers.TLSConfiguration{} }, + Configuration: func() any { + return gin.H{ + "enable": true, + } + }, + Expected: helpers.TLSConfiguration{ + Enable: true, + SkipVerify: false, + }, + }, { + Description: "old verify=true migrates to skip-verify=false", + Initial: func() any { return helpers.TLSConfiguration{} }, + Configuration: func() any { + return gin.H{ + "enable": true, + "verify": true, + } + }, + Expected: helpers.TLSConfiguration{ + Enable: true, + SkipVerify: false, + }, + }, { + Description: "old verify=false migrates to skip-verify=true", + Initial: func() any { return helpers.TLSConfiguration{} }, + Configuration: func() any { + return gin.H{ + "enable": true, + "verify": false, + } + }, + Expected: helpers.TLSConfiguration{ + Enable: true, + SkipVerify: true, + }, + }, { + Description: "both verify and skip-verify causes error", + Initial: func() any { return helpers.TLSConfiguration{} }, + Configuration: func() any { + return gin.H{ + "enable": true, + "verify": true, + "skip-verify": false, + } + }, + Error: true, + }, + }) +} diff --git a/common/kafka/config.go b/common/kafka/config.go index c2152ae9..e28856a1 100644 --- a/common/kafka/config.go +++ b/common/kafka/config.go @@ -53,10 +53,6 @@ func DefaultConfiguration() Configuration { return Configuration{ Topic: "flows", Brokers: []string{"127.0.0.1:9092"}, - TLS: helpers.TLSConfiguration{ - Enable: false, - Verify: true, - }, } } diff --git a/common/kafka/config_test.go b/common/kafka/config_test.go index 707cb5cb..8d761d5e 100644 --- a/common/kafka/config_test.go +++ b/common/kafka/config_test.go @@ -110,8 +110,8 @@ func TestTLSConfiguration(t *testing.T) { Topic: "flows", Brokers: []string{"127.0.0.1:9092"}, TLS: helpers.TLSConfiguration{ - Enable: true, - Verify: true, + Enable: true, + SkipVerify: false, }, }, }, { @@ -132,8 +132,8 @@ func TestTLSConfiguration(t *testing.T) { Topic: "flows", Brokers: []string{"127.0.0.1:9092"}, TLS: helpers.TLSConfiguration{ - Enable: true, - Verify: false, + Enable: true, + SkipVerify: true, }, SASL: SASLConfiguration{ Username: "hello", @@ -157,8 +157,8 @@ func TestTLSConfiguration(t *testing.T) { Topic: "flows", Brokers: []string{"127.0.0.1:9092"}, TLS: helpers.TLSConfiguration{ - Enable: false, - Verify: true, + Enable: false, + SkipVerify: false, }, SASL: SASLConfiguration{ Username: "hello", @@ -187,7 +187,7 @@ func TestTLSConfiguration(t *testing.T) { TLS: helpers.TLSConfiguration{ Enable: true, // Value from DefaultConfig is true - Verify: true, + SkipVerify: false, }, SASL: SASLConfiguration{ Username: "hello", @@ -218,7 +218,7 @@ func TestTLSConfiguration(t *testing.T) { TLS: helpers.TLSConfiguration{ Enable: true, // Value from DefaultConfig is true - Verify: true, + SkipVerify: false, }, SASL: SASLConfiguration{ Username: "hello", diff --git a/common/kafka/tests.go b/common/kafka/tests.go index 099419a5..b5057daa 100644 --- a/common/kafka/tests.go +++ b/common/kafka/tests.go @@ -30,10 +30,6 @@ func SetupKafkaBroker(t *testing.T) (*kgo.Client, []string) { r := reporter.NewMock(t) opts, err := NewConfig(r, Configuration{ Brokers: []string{broker}, - TLS: helpers.TLSConfiguration{ - Enable: false, - Verify: true, - }, }) if err != nil { t.Fatalf("NewConfig() error: %v", err) diff --git a/common/remotedatasource/config.go b/common/remotedatasource/config.go index c06ff240..964a8b4b 100644 --- a/common/remotedatasource/config.go +++ b/common/remotedatasource/config.go @@ -68,10 +68,6 @@ func DefaultSourceConfiguration() Source { return Source{ Method: "GET", Timeout: time.Minute, - TLS: helpers.TLSConfiguration{ - Enable: false, - Verify: true, - }, } } diff --git a/common/remotedatasource/config_test.go b/common/remotedatasource/config_test.go index 7d865154..f23347cc 100644 --- a/common/remotedatasource/config_test.go +++ b/common/remotedatasource/config_test.go @@ -29,7 +29,7 @@ func TestSourceDecode(t *testing.T) { Timeout: time.Minute, Interval: 10 * time.Minute, TLS: helpers.TLSConfiguration{ - Verify: true, + SkipVerify: false, }, }, }, { @@ -49,7 +49,7 @@ func TestSourceDecode(t *testing.T) { Interval: 10 * time.Minute, Transform: MustParseTransformQuery(".[]"), TLS: helpers.TLSConfiguration{ - Verify: true, + SkipVerify: false, }, }, }, { @@ -71,7 +71,7 @@ func TestSourceDecode(t *testing.T) { Interval: 10 * time.Minute, Transform: MustParseTransformQuery(".[]"), TLS: helpers.TLSConfiguration{ - Verify: true, + SkipVerify: false, }, }, }, { @@ -93,9 +93,9 @@ func TestSourceDecode(t *testing.T) { Timeout: time.Minute, Interval: 10 * time.Minute, TLS: helpers.TLSConfiguration{ - Enable: true, - Verify: false, // TODO this should be fixed - CAFile: "something.crt", + Enable: true, + SkipVerify: false, + CAFile: "something.crt", }, }, }, { @@ -119,7 +119,7 @@ func TestSourceDecode(t *testing.T) { .prefixes[] | {prefix: .ip_prefix, tenant: "amazon", region: .region, role: .service|ascii_downcase} `), TLS: helpers.TLSConfiguration{ - Verify: true, + SkipVerify: false, }, }, }, { diff --git a/common/remotedatasource/root_test.go b/common/remotedatasource/root_test.go index 4d16f7c0..b4e25186 100644 --- a/common/remotedatasource/root_test.go +++ b/common/remotedatasource/root_test.go @@ -314,8 +314,8 @@ func TestSourceWithTLS(t *testing.T) { Timeout: 1 * time.Second, Interval: 1 * time.Minute, TLS: helpers.TLSConfiguration{ - Enable: true, - Verify: false, + Enable: true, + SkipVerify: true, }, Transform: MustParseTransformQuery(".results[]"), }, diff --git a/console/data/docs/02-configuration.md b/console/data/docs/02-configuration.md index c87a17b5..1c6ba522 100644 --- a/console/data/docs/02-configuration.md +++ b/console/data/docs/02-configuration.md @@ -767,7 +767,7 @@ flows. It accepts the following keys: The following keys are accepted for the TLS configuration: - `enable` should be set to `true` to enable TLS. -- `verify` can be set to `false` to skip checking server certificate (not recommended). +- `skip-verify` can be set to `true` to skip checking server certificate (not recommended). - `ca-file` gives the location of the file containing the CA certificate in PEM format to check the server certificate. If not provided, the system certificates are used instead. diff --git a/console/data/docs/99-changelog.md b/console/data/docs/99-changelog.md index 00eaa277..54cb231d 100644 --- a/console/data/docs/99-changelog.md +++ b/console/data/docs/99-changelog.md @@ -12,7 +12,12 @@ identified with a specific icon: ## Unreleased -- 🌱 *common*: remote data sources accept a specific TLS configuration +- 💥 *config*: `skip-verify` is false by default in TLS configurations for + ClickHouse, Kafka and remote data sources (previously, `verify` was set to + false by default) +- 🌱 *config*: rename `verify` to `skip-verify` in TLS configurations for + ClickHouse, Kafka and remote data sources (with inverted logic) +- 🌱 *config*: remote data sources accept a specific TLS configuration ## 2.0.2 - 2025-10-29