Skip to content

Commit 40bc8d2

Browse files
authored
Merge pull request #4 from leandroberetta/netobserv-584-2
Reuse ClientConfig.TLS from Prometheus for consistency
2 parents f26971f + 9c77a0a commit 40bc8d2

File tree

2 files changed

+14
-63
lines changed

2 files changed

+14
-63
lines changed

grpc/config.go

Lines changed: 12 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
package grpc
22

33
import (
4-
"crypto/tls"
5-
"crypto/x509"
64
"flag"
7-
"fmt"
8-
"os"
95
"time"
106

117
"github.com/netobserv/loki-client-go/pkg/backoff"
128
"github.com/netobserv/loki-client-go/pkg/labelutil"
9+
promConfig "github.com/prometheus/common/config"
1310
"google.golang.org/grpc"
1411
"google.golang.org/grpc/credentials"
1512
"google.golang.org/grpc/credentials/insecure"
@@ -38,10 +35,10 @@ type Config struct {
3835
BatchSize int `yaml:"batch_size"`
3936

4037
// Connection configuration
41-
Timeout time.Duration `yaml:"timeout"`
38+
Timeout time.Duration `yaml:"timeout"`
4239

43-
// TLS configuration
44-
TLS TLSConfig `yaml:"tls"`
40+
// TLS configuration (uses Prometheus common TLSConfig for consistency)
41+
TLS promConfig.TLSConfig `yaml:"tls"`
4542

4643
// Keep alive configuration
4744
KeepAlive time.Duration `yaml:"keep_alive"`
@@ -57,27 +54,6 @@ type Config struct {
5754
TenantID string `yaml:"tenant_id"`
5855
}
5956

60-
// TLSConfig contains TLS configuration for GRPC client
61-
type TLSConfig struct {
62-
// Enable TLS
63-
Enabled bool `yaml:"enabled"`
64-
65-
// Path to certificate file
66-
CertFile string `yaml:"cert_file"`
67-
68-
// Path to key file
69-
KeyFile string `yaml:"key_file"`
70-
71-
// Path to CA file
72-
CAFile string `yaml:"ca_file"`
73-
74-
// Server name for certificate verification
75-
ServerName string `yaml:"server_name"`
76-
77-
// Skip certificate verification (insecure)
78-
InsecureSkipVerify bool `yaml:"insecure_skip_verify"`
79-
}
80-
8157
// NewDefaultConfig creates a default configuration for a given GRPC server address.
8258
func NewDefaultConfig(serverAddress string) (Config, error) {
8359
var cfg Config
@@ -103,7 +79,6 @@ func (c *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
10379

10480
f.DurationVar(&c.Timeout, prefix+"grpc.timeout", DefaultTimeout, "Maximum time to wait for server to respond to a request")
10581

106-
f.BoolVar(&c.TLS.Enabled, prefix+"grpc.tls.enabled", false, "Enable TLS")
10782
f.StringVar(&c.TLS.CertFile, prefix+"grpc.tls.cert-file", "", "Path to client certificate file")
10883
f.StringVar(&c.TLS.KeyFile, prefix+"grpc.tls.key-file", "", "Path to client key file")
10984
f.StringVar(&c.TLS.CAFile, prefix+"grpc.tls.ca-file", "", "Path to CA certificate file")
@@ -125,40 +100,22 @@ func (c *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
125100
func (c *Config) BuildDialOptions() ([]grpc.DialOption, error) {
126101
var opts []grpc.DialOption
127102

128-
129103
// Keep alive settings
130104
opts = append(opts, grpc.WithKeepaliveParams(keepalive.ClientParameters{
131105
Time: c.KeepAlive,
132106
Timeout: c.KeepAliveTimeout,
133107
PermitWithoutStream: true,
134108
}))
135109

136-
// TLS configuration
137-
if c.TLS.Enabled {
138-
tlsConfig := &tls.Config{
139-
ServerName: c.TLS.ServerName,
140-
InsecureSkipVerify: c.TLS.InsecureSkipVerify,
141-
}
142-
143-
// Load CA certificate if specified
144-
if c.TLS.CAFile != "" {
145-
caCert, err := os.ReadFile(c.TLS.CAFile)
146-
if err != nil {
147-
return nil, fmt.Errorf("failed to read CA certificate file %s: %w", c.TLS.CAFile, err)
148-
}
149-
caCertPool := x509.NewCertPool()
150-
if !caCertPool.AppendCertsFromPEM(caCert) {
151-
return nil, fmt.Errorf("failed to parse CA certificate from %s", c.TLS.CAFile)
152-
}
153-
tlsConfig.RootCAs = caCertPool
154-
}
110+
// TLS configuration - check if any TLS field is set to determine if TLS should be enabled
111+
tlsEnabled := c.TLS.CAFile != "" || c.TLS.CertFile != "" || c.TLS.KeyFile != "" ||
112+
c.TLS.CA != "" || c.TLS.Cert != "" || string(c.TLS.Key) != ""
155113

156-
if c.TLS.CertFile != "" && c.TLS.KeyFile != "" {
157-
cert, err := tls.LoadX509KeyPair(c.TLS.CertFile, c.TLS.KeyFile)
158-
if err != nil {
159-
return nil, err
160-
}
161-
tlsConfig.Certificates = []tls.Certificate{cert}
114+
if tlsEnabled {
115+
// Use Prometheus common config to build TLS config
116+
tlsConfig, err := promConfig.NewTLSConfig(&c.TLS)
117+
if err != nil {
118+
return nil, err
162119
}
163120

164121
creds := credentials.NewTLS(tlsConfig)

grpc/config_test.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66
"time"
77

8+
promConfig "github.com/prometheus/common/config"
89
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
1011
)
@@ -33,7 +34,6 @@ func TestConfigRegisterFlags(t *testing.T) {
3334
"-grpc.batch-wait=5s",
3435
"-grpc.batch-size-bytes=2048",
3536
"-grpc.timeout=30s",
36-
"-grpc.tls.enabled=true",
3737
"-grpc.tls.server-name=loki.example.com",
3838
"-grpc.tenant-id=test-tenant",
3939
}
@@ -45,7 +45,6 @@ func TestConfigRegisterFlags(t *testing.T) {
4545
assert.Equal(t, 5*time.Second, cfg.BatchWait)
4646
assert.Equal(t, 2048, cfg.BatchSize)
4747
assert.Equal(t, 30*time.Second, cfg.Timeout)
48-
assert.True(t, cfg.TLS.Enabled)
4948
assert.Equal(t, "loki.example.com", cfg.TLS.ServerName)
5049
assert.Equal(t, "test-tenant", cfg.TenantID)
5150
}
@@ -71,10 +70,6 @@ func TestBuildDialOptions(t *testing.T) {
7170
cfg := Config{
7271
KeepAlive: 30 * time.Second,
7372
KeepAliveTimeout: 5 * time.Second,
74-
TLS: TLSConfig{
75-
Enabled: false,
76-
InsecureSkipVerify: true,
77-
},
7873
}
7974

8075
opts, err := cfg.BuildDialOptions()
@@ -86,8 +81,7 @@ func TestBuildDialOptionsWithTLS(t *testing.T) {
8681
cfg := Config{
8782
KeepAlive: 30 * time.Second,
8883
KeepAliveTimeout: 5 * time.Second,
89-
TLS: TLSConfig{
90-
Enabled: true,
84+
TLS: promConfig.TLSConfig{
9185
ServerName: "loki.example.com",
9286
InsecureSkipVerify: true,
9387
},

0 commit comments

Comments
 (0)