Skip to content

Reuse ClientConfig.TLS from Prometheus for consistency #4

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reuse ClientConfig.TLS from Prometheus for consistency #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
67 changes: 12 additions & 55 deletions grpc/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
package grpc

import (
"crypto/tls"
"crypto/x509"
"flag"
"fmt"
"os"
"time"

"github.com/netobserv/loki-client-go/pkg/backoff"
"github.com/netobserv/loki-client-go/pkg/labelutil"
promConfig "github.com/prometheus/common/config"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"
Expand Down Expand Up @@ -38,10 +35,10 @@ type Config struct {
BatchSize int `yaml:"batch_size"`

// Connection configuration
Timeout time.Duration `yaml:"timeout"`
Timeout time.Duration `yaml:"timeout"`

// TLS configuration
TLS TLSConfig `yaml:"tls"`
// TLS configuration (uses Prometheus common TLSConfig for consistency)
TLS promConfig.TLSConfig `yaml:"tls"`

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

// TLSConfig contains TLS configuration for GRPC client
type TLSConfig struct {
// Enable TLS
Enabled bool `yaml:"enabled"`

// Path to certificate file
CertFile string `yaml:"cert_file"`

// Path to key file
KeyFile string `yaml:"key_file"`

// Path to CA file
CAFile string `yaml:"ca_file"`

// Server name for certificate verification
ServerName string `yaml:"server_name"`

// Skip certificate verification (insecure)
InsecureSkipVerify bool `yaml:"insecure_skip_verify"`
}

// NewDefaultConfig creates a default configuration for a given GRPC server address.
func NewDefaultConfig(serverAddress string) (Config, error) {
var cfg Config
Expand All @@ -103,7 +79,6 @@ func (c *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {

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

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


// Keep alive settings
opts = append(opts, grpc.WithKeepaliveParams(keepalive.ClientParameters{
Time: c.KeepAlive,
Timeout: c.KeepAliveTimeout,
PermitWithoutStream: true,
}))

// TLS configuration
if c.TLS.Enabled {
tlsConfig := &tls.Config{
ServerName: c.TLS.ServerName,
InsecureSkipVerify: c.TLS.InsecureSkipVerify,
}

// Load CA certificate if specified
if c.TLS.CAFile != "" {
caCert, err := os.ReadFile(c.TLS.CAFile)
if err != nil {
return nil, fmt.Errorf("failed to read CA certificate file %s: %w", c.TLS.CAFile, err)
}
caCertPool := x509.NewCertPool()
if !caCertPool.AppendCertsFromPEM(caCert) {
return nil, fmt.Errorf("failed to parse CA certificate from %s", c.TLS.CAFile)
}
tlsConfig.RootCAs = caCertPool
}
// TLS configuration - check if any TLS field is set to determine if TLS should be enabled
tlsEnabled := c.TLS.CAFile != "" || c.TLS.CertFile != "" || c.TLS.KeyFile != "" ||
c.TLS.CA != "" || c.TLS.Cert != "" || string(c.TLS.Key) != ""

if c.TLS.CertFile != "" && c.TLS.KeyFile != "" {
cert, err := tls.LoadX509KeyPair(c.TLS.CertFile, c.TLS.KeyFile)
if err != nil {
return nil, err
}
tlsConfig.Certificates = []tls.Certificate{cert}
if tlsEnabled {
// Use Prometheus common config to build TLS config
tlsConfig, err := promConfig.NewTLSConfig(&c.TLS)
if err != nil {
return nil, err
}

creds := credentials.NewTLS(tlsConfig)
Expand Down
11 changes: 3 additions & 8 deletions grpc/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"
"time"

promConfig "github.com/prometheus/common/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -33,7 +34,6 @@ func TestConfigRegisterFlags(t *testing.T) {
"-grpc.batch-wait=5s",
"-grpc.batch-size-bytes=2048",
"-grpc.timeout=30s",
"-grpc.tls.enabled=true",
"-grpc.tls.server-name=loki.example.com",
"-grpc.tenant-id=test-tenant",
}
Expand All @@ -45,7 +45,6 @@ func TestConfigRegisterFlags(t *testing.T) {
assert.Equal(t, 5*time.Second, cfg.BatchWait)
assert.Equal(t, 2048, cfg.BatchSize)
assert.Equal(t, 30*time.Second, cfg.Timeout)
assert.True(t, cfg.TLS.Enabled)
assert.Equal(t, "loki.example.com", cfg.TLS.ServerName)
assert.Equal(t, "test-tenant", cfg.TenantID)
}
Expand All @@ -71,10 +70,7 @@ func TestBuildDialOptions(t *testing.T) {
cfg := Config{
KeepAlive: 30 * time.Second,
KeepAliveTimeout: 5 * time.Second,
TLS: TLSConfig{
Enabled: false,
InsecureSkipVerify: true,
},
// No TLS configured - should use insecure credentials
}

opts, err := cfg.BuildDialOptions()
Expand All @@ -86,8 +82,7 @@ func TestBuildDialOptionsWithTLS(t *testing.T) {
cfg := Config{
KeepAlive: 30 * time.Second,
KeepAliveTimeout: 5 * time.Second,
TLS: TLSConfig{
Enabled: true,
TLS: promConfig.TLSConfig{
ServerName: "loki.example.com",
InsecureSkipVerify: true,
},
Expand Down