common/helpers: migrate from verify to skip-verify in TLS config
Some checks failed
CI / 🤖 Check dependabot status (push) Has been cancelled
CI / 🐧 Test on Linux (${{ github.ref_type == 'tag' }}, misc) (push) Has been cancelled
CI / 🐧 Test on Linux (coverage) (push) Has been cancelled
CI / 🐧 Test on Linux (regular) (push) Has been cancelled
CI / ❄️ Build on Nix (push) Has been cancelled
CI / 🍏 Build and test on macOS (push) Has been cancelled
CI / 🧪 End-to-end testing (push) Has been cancelled
CI / 🔍 Upload code coverage (push) Has been cancelled
CI / 🔬 Test only Go (push) Has been cancelled
CI / 🔬 Test only JS (${{ needs.dependabot.outputs.package-ecosystem }}, 20) (push) Has been cancelled
CI / 🔬 Test only JS (${{ needs.dependabot.outputs.package-ecosystem }}, 22) (push) Has been cancelled
CI / 🔬 Test only JS (${{ needs.dependabot.outputs.package-ecosystem }}, 24) (push) Has been cancelled
CI / ⚖️ Check licenses (push) Has been cancelled
CI / 🐋 Build Docker images (push) Has been cancelled
CI / 🐋 Tag Docker images (push) Has been cancelled
CI / 🚀 Publish release (push) Has been cancelled

Otherwise, the default is "false" for verify. This is a breaking change.

Fix #2055.
This commit is contained in:
Vincent Bernat
2025-10-30 08:31:27 +01:00
parent a2339312ac
commit e68b2de72c
13 changed files with 155 additions and 41 deletions

View File

@@ -4,7 +4,7 @@ paths:
url: https://ip-ranges.amazonaws.com/ip-ranges.json
tls:
enable: false
verify: true
skipverify: false
cafile: ""
certfile: ""
keyfile: ""

View File

@@ -12,7 +12,7 @@ paths:
dialtimeout: 5s
tls:
enable: true
verify: true
skipverify: false
cafile: ""
certfile: ""
keyfile: ""

View File

@@ -41,10 +41,6 @@ func DefaultConfiguration() Configuration {
Username: "default",
MaxOpenConns: 10,
DialTimeout: 5 * time.Second,
TLS: helpers.TLSConfiguration{
Enable: false,
Verify: true,
},
}
}

View File

@@ -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())
}

View File

@@ -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,
},
})
}

View File

@@ -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,
},
}
}

View File

@@ -111,7 +111,7 @@ func TestTLSConfiguration(t *testing.T) {
Brokers: []string{"127.0.0.1:9092"},
TLS: helpers.TLSConfiguration{
Enable: true,
Verify: true,
SkipVerify: false,
},
},
}, {
@@ -133,7 +133,7 @@ func TestTLSConfiguration(t *testing.T) {
Brokers: []string{"127.0.0.1:9092"},
TLS: helpers.TLSConfiguration{
Enable: true,
Verify: false,
SkipVerify: true,
},
SASL: SASLConfiguration{
Username: "hello",
@@ -158,7 +158,7 @@ func TestTLSConfiguration(t *testing.T) {
Brokers: []string{"127.0.0.1:9092"},
TLS: helpers.TLSConfiguration{
Enable: false,
Verify: true,
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",

View File

@@ -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)

View File

@@ -68,10 +68,6 @@ func DefaultSourceConfiguration() Source {
return Source{
Method: "GET",
Timeout: time.Minute,
TLS: helpers.TLSConfiguration{
Enable: false,
Verify: true,
},
}
}

View File

@@ -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,
},
},
}, {
@@ -94,7 +94,7 @@ func TestSourceDecode(t *testing.T) {
Interval: 10 * time.Minute,
TLS: helpers.TLSConfiguration{
Enable: true,
Verify: false, // TODO this should be fixed
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,
},
},
}, {

View File

@@ -315,7 +315,7 @@ func TestSourceWithTLS(t *testing.T) {
Interval: 1 * time.Minute,
TLS: helpers.TLSConfiguration{
Enable: true,
Verify: false,
SkipVerify: true,
},
Transform: MustParseTransformQuery(".results[]"),
},

View File

@@ -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.

View File

@@ -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