From f89b33f56cfd4f1ed99ea46d2b4021f7ebf62fc9 Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Wed, 26 Nov 2025 20:25:10 +0100 Subject: [PATCH 01/10] fix: refactor the http client --- .../testConfigs/correctBasicAuthConfig.yaml | 27 ++-- .../testConfigs/correctMTLSConfig.yaml | 39 +++--- .../testConfigs/incorrectBasicAuthConfig.yaml | 27 ++-- .../incorrectBasicAuthConfigCreds.yaml | 27 ++-- .../testConfigs/incorrectMTLSConfig.yaml | 39 +++--- pkg/commoncfg/config.go | 79 ++++++++++-- pkg/commonhttp/client.go | 113 ++++++++++++----- pkg/commonhttp/client_api-token.go | 91 ++++++++++++++ pkg/commonhttp/client_basic.go | 95 ++++++++++++++ pkg/commonhttp/client_test.go | 67 +++++----- pkg/otlp/audit/comm.go | 49 +++++--- pkg/otlp/audit/errors.go | 7 +- pkg/otlp/audit/logger.go | 63 ++-------- pkg/otlp/audit/logger_test.go | 23 +++- pkg/otlp/opentelemetry.go | 119 ++++++++++++++++-- pkg/utils/auth.go | 13 ++ 16 files changed, 623 insertions(+), 255 deletions(-) create mode 100644 pkg/commonhttp/client_api-token.go create mode 100644 pkg/commonhttp/client_basic.go create mode 100644 pkg/utils/auth.go diff --git a/internal/otlp/audit/testdata/testConfigs/correctBasicAuthConfig.yaml b/internal/otlp/audit/testdata/testConfigs/correctBasicAuthConfig.yaml index f7689b6..9d504f2 100644 --- a/internal/otlp/audit/testdata/testConfigs/correctBasicAuthConfig.yaml +++ b/internal/otlp/audit/testdata/testConfigs/correctBasicAuthConfig.yaml @@ -4,16 +4,17 @@ application: audit: endpoint: "http://localhost:4043/logs" - basicAuth: - username: - source: file - file: - path: ../../../internal/otlp/audit/testdata/correctBasicAuth.json - format: json - jsonPath: "$.username" - password: - source: file - file: - path: ../../../internal/otlp/audit/testdata/correctBasicAuth.json - format: json - jsonPath: "$.password" + httpClient: + basicAuth: + username: + source: file + file: + path: ../../../internal/otlp/audit/testdata/correctBasicAuth.json + format: json + jsonPath: "$.username" + password: + source: file + file: + path: ../../../internal/otlp/audit/testdata/correctBasicAuth.json + format: json + jsonPath: "$.password" diff --git a/internal/otlp/audit/testdata/testConfigs/correctMTLSConfig.yaml b/internal/otlp/audit/testdata/testConfigs/correctMTLSConfig.yaml index 6f46264..f003828 100644 --- a/internal/otlp/audit/testdata/testConfigs/correctMTLSConfig.yaml +++ b/internal/otlp/audit/testdata/testConfigs/correctMTLSConfig.yaml @@ -4,22 +4,23 @@ application: audit: endpoint: "http://localhost:4043/logs" - mtls: - cert: - source: file - file: - path: ../../../internal/otlp/audit/testdata/correctMTLS.json - format: json - jsonPath: "$.otlp-cert" - certKey: - source: file - file: - path: ../../../internal/otlp/audit/testdata/correctMTLS.json - format: json - jsonPath: "$.otlp-key" - serverCa: - source: file - file: - path: ../../../internal/otlp/audit/testdata/correctMTLS.json - format: json - jsonPath: "$.otlp-server-ca" + httpClient: + mtls: + cert: + source: file + file: + path: ../../../internal/otlp/audit/testdata/correctMTLS.json + format: json + jsonPath: "$.otlp-cert" + certKey: + source: file + file: + path: ../../../internal/otlp/audit/testdata/correctMTLS.json + format: json + jsonPath: "$.otlp-key" + serverCa: + source: file + file: + path: ../../../internal/otlp/audit/testdata/correctMTLS.json + format: json + jsonPath: "$.otlp-server-ca" diff --git a/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfig.yaml b/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfig.yaml index 92b8d72..72ce2e5 100644 --- a/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfig.yaml +++ b/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfig.yaml @@ -4,16 +4,17 @@ application: audit: endpoint: "http://localhost:4043/logs" - basicAuth: - username: - source: file - file: - path: ../../../internal/otlp/audit/testdata/incorrectBasicAuth.json - format: json - jsonPath: "$.username" - password: - source: file - file: - path: ../../../internal/otlp/audit/testdata/incorrectBasicAuth.json - format: json - jsonPath: "$.password" + httpClient: + basicAuth: + username: + source: file + file: + path: ../../../internal/otlp/audit/testdata/incorrectBasicAuth.json + format: json + jsonPath: "$.username" + password: + source: file + file: + path: ../../../internal/otlp/audit/testdata/incorrectBasicAuth.json + format: json + jsonPath: "$.password" diff --git a/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfigCreds.yaml b/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfigCreds.yaml index 6a03a77..5932441 100644 --- a/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfigCreds.yaml +++ b/internal/otlp/audit/testdata/testConfigs/incorrectBasicAuthConfigCreds.yaml @@ -4,16 +4,17 @@ application: audit: endpoint: "http://localhost:4043/logs" - basicAuth: - username: - source: file - file: - path: ../../../internal/otlp/audit/testdata/incorrectBasicAuthCreds.json - format: json - jsonPath: "$.username" - password: - source: file - file: - path: ../../../internal/otlp/audit/testdata/incorrectBasicAuthCreds.json - format: json - jsonPath: "$.password" + httpClient: + basicAuth: + username: + source: file + file: + path: ../../../internal/otlp/audit/testdata/incorrectBasicAuthCreds.json + format: json + jsonPath: "$.username" + password: + source: file + file: + path: ../../../internal/otlp/audit/testdata/incorrectBasicAuthCreds.json + format: json + jsonPath: "$.password" diff --git a/internal/otlp/audit/testdata/testConfigs/incorrectMTLSConfig.yaml b/internal/otlp/audit/testdata/testConfigs/incorrectMTLSConfig.yaml index 92bb541..c97084b 100644 --- a/internal/otlp/audit/testdata/testConfigs/incorrectMTLSConfig.yaml +++ b/internal/otlp/audit/testdata/testConfigs/incorrectMTLSConfig.yaml @@ -4,22 +4,23 @@ application: audit: endpoint: "http://localhost:4043/logs" - mtls: - cert: - source: file - file: - path: ../../../internal/otlp/audit/testdata/incorrectMTLS.json - format: json - jsonPath: "$.otlp-cert" - certKey: - source: file - file: - path: ../../../internal/otlp/audit/testdata/incorrectMTLS.json - format: json - jsonPath: "$.otlp-key" - serverCa: - source: file - file: - path: ../../../internal/otlp/audit/testdata/incorrectMTLS.json - format: json - jsonPath: "$.otlp-server-ca" + httpClient: + mtls: + cert: + source: file + file: + path: ../../../internal/otlp/audit/testdata/incorrectMTLS.json + format: json + jsonPath: "$.otlp-cert" + certKey: + source: file + file: + path: ../../../internal/otlp/audit/testdata/incorrectMTLS.json + format: json + jsonPath: "$.otlp-key" + serverCa: + source: file + file: + path: ../../../internal/otlp/audit/testdata/incorrectMTLS.json + format: json + jsonPath: "$.otlp-server-ca" diff --git a/pkg/commoncfg/config.go b/pkg/commoncfg/config.go index e0ca844..c5c2b6a 100644 --- a/pkg/commoncfg/config.go +++ b/pkg/commoncfg/config.go @@ -204,10 +204,9 @@ type MTLS struct { // Audit holds the audit log library configuration. type Audit struct { Endpoint string `yaml:"endpoint" json:"endpoint"` - // Potential mTLS for the endpoint. - MTLS *MTLS `yaml:"mtls" json:"mtls"` - // Potential BasicAuth for the endpoint. - BasicAuth *BasicAuth `yaml:"basicAuth" json:"basicAuth"` + + HTTPClient HTTPClient `yaml:"httpClient" json:"httpClient"` + // Optional set of additional properties to be added to OTLP log object. Must be added as a literal string to maintain casing. AdditionalProperties string `yaml:"additionalProperties" json:"additionalProperties"` } @@ -337,12 +336,72 @@ type GRPCClientAttributes struct { } type HTTPClient struct { - Timeout time.Duration `yaml:"timeout" json:"timeout" default:"10s"` - RootCAs *SourceRef `yaml:"rootCAs" json:"rootCAs"` - InsecureSkipVerify bool `yaml:"insecureSkipVerify" json:"insecureSkipVerify"` - MinVersion uint16 `yaml:"minVersion" json:"minVersion"` - Cert *SourceRef `yaml:"cert" json:"cert"` - CertKey *SourceRef `yaml:"certKey" json:"certKey"` + Timeout time.Duration `yaml:"timeout" json:"timeout" default:"30s"` + APIToken *SourceRef `yaml:"apiToken" json:"apiToken"` + BasicAuth *BasicAuth `yaml:"basicAuth" json:"basicAuth"` + OAuth2Auth *OAuth2 `yaml:"oauth2Auth" json:"oauth2Auth"` + MTLS *MTLS `yaml:"mtls" json:"mtls"` + TransportAttributes HTTPTransportAttributes `yaml:"transportAttributes" json:"transportAttributes"` +} + +type HTTPTransportAttributes struct { + // TLSHandshakeTimeout specifies the maximum amount of time to + // wait for a TLS handshake. Zero means no timeout. + TLSHandshakeTimeout time.Duration `yaml:"tlsHandshakeTimeout" json:"tlsHandshakeTimeout"` + + // DisableKeepAlives, if true, disables HTTP keep-alives and + // will only use the connection to the server for a single + // HTTP request. + // + // This is unrelated to the similarly named TCP keep-alives. + DisableKeepAlives bool `yaml:"disableKeepAlives" json:"disableKeepAlives"` + + // DisableCompression, if true, prevents the Transport from + // requesting compression with an "Accept-Encoding: gzip" + // request header when the Request contains no existing + // Accept-Encoding value. If the Transport requests gzip on + // its own and gets a gzipped response, it's transparently + // decoded in the Response.Body. However, if the user + // explicitly requested gzip it is not automatically + // uncompressed. + DisableCompression bool `yaml:"disableCompression" json:"disableCompression"` + + // MaxIdleConns controls the maximum number of idle (keep-alive) + // connections across all hosts. Zero means no limit. + MaxIdleConns int `yaml:"maxIdleConns" json:"maxIdleConns"` + + // MaxIdleConnsPerHost, if non-zero, controls the maximum idle + // (keep-alive) connections to keep per-host. If zero, + // DefaultMaxIdleConnsPerHost is used. + MaxIdleConnsPerHost int `yaml:"maxIdleConnsPerHost" json:"maxIdleConnsPerHost"` + + // MaxConnsPerHost optionally limits the total number of + // connections per host, including connections in the dialing, + // active, and idle states. On limit violation, dials will block. + // + // Zero means no limit. + MaxConnsPerHost int `yaml:"maxConnsPerHost" json:"maxConnsPerHost"` + + // IdleConnTimeout is the maximum amount of time an idle + // (keep-alive) connection will remain idle before closing + // itself. + // Zero means no limit. + IdleConnTimeout time.Duration `yaml:"idleConnTimeout" json:"idleConnTimeout"` + + // ResponseHeaderTimeout, if non-zero, specifies the amount of + // time to wait for a server's response headers after fully + // writing the request (including its body, if any). This + // time does not include the time to read the response body. + ResponseHeaderTimeout time.Duration `yaml:"responseHeaderTimeout" json:"responseHeaderTimeout"` + + // ExpectContinueTimeout, if non-zero, specifies the amount of + // time to wait for a server's first response headers after fully + // writing the request headers if the request has an + // "Expect: 100-continue" header. Zero means no timeout and + // causes the body to be sent immediately, without + // waiting for the server to approve. + // This time does not include the time to send the request header. + ExpectContinueTimeout time.Duration `yaml:"expectContinueTimeout" json:"expectContinueTimeout"` } // BuildInfo holds metadata about the build diff --git a/pkg/commonhttp/client.go b/pkg/commonhttp/client.go index 8bedf32..f4cc5c0 100644 --- a/pkg/commonhttp/client.go +++ b/pkg/commonhttp/client.go @@ -9,59 +9,104 @@ import ( "github.com/openkcm/common-sdk/pkg/commoncfg" ) -// NewClient creates an *http.Client configured with optional TLS/mTLS and custom settings. +// NewClient creates an *http.Client using the full HTTPClient configuration. // -// Supports: -// - Timeout -// - TLS minimum version (default TLS1.2) -// - InsecureSkipVerify -// - Custom root CAs -// - Optional client certificates (mTLS) +// It supports the following authentication methods: +// - Basic Auth +// - OAuth2 (all supported grant types & auth methods) +// - API Token authentication +// +// It also configures: +// - TLS configuration (optional mTLS) +// - Transport attributes (timeouts, connection pooling) +// - Global client timeout +// +// Important behaviour: +// - If an authentication method is used, the factory returns a client +// whose Transport is a wrapped RoundTripper (e.g., OAuth2, BasicAuth). +// - This function **preserves** that RoundTripper and wraps it with +// a proper `http.Transport` when TLS or transport attributes must be applied. +// - This avoids overwriting authentication transport logic. func NewClient(cfg *commoncfg.HTTPClient) (*http.Client, error) { if cfg == nil { return nil, errors.New("HTTPClient config is nil") } - // Base HTTP client with timeout - client := &http.Client{ - Timeout: cfg.Timeout, - } + var ( + client *http.Client + err error + ) - // Prepare TLS configuration - tlsConfig := &tls.Config{ - InsecureSkipVerify: cfg.InsecureSkipVerify, - MinVersion: tls.VersionTLS12, + // Select authentication mechanism (if any) + switch { + case cfg.BasicAuth != nil: + client, err = NewClientFromBasic(cfg.BasicAuth) + case cfg.OAuth2Auth != nil: + client, err = NewClientFromOAuth2(cfg.OAuth2Auth) + case cfg.APIToken != nil: + client, err = NewClientFromAPIToken(cfg.APIToken) + default: + // No authentication → start with a default client + client = &http.Client{Transport: http.DefaultTransport} } - // Override minimum TLS version if provided - if cfg.MinVersion >= tlsConfig.MinVersion { - tlsConfig.MinVersion = cfg.MinVersion + if err != nil { + return nil, err } - // Load custom root CAs if provided and not skipping verification - if !cfg.InsecureSkipVerify && cfg.RootCAs != nil { - certPool, err := commoncfg.LoadCACertPool(cfg.RootCAs) + // Start building TLS configuration + var tlsConfig *tls.Config + if cfg.MTLS != nil { + tlsConfig, err = commoncfg.LoadMTLSConfig(cfg.MTLS) if err != nil { - return nil, fmt.Errorf("failed to load root CAs: %w", err) + return nil, fmt.Errorf("failed to load tls config: %w", err) } + } else { + // keep default system roots if no mTLS is used + tlsConfig = &tls.Config{} + } - tlsConfig.RootCAs = certPool + // Build a proper base transport + baseTransport := &http.Transport{ + TLSClientConfig: tlsConfig, + TLSHandshakeTimeout: cfg.TransportAttributes.TLSHandshakeTimeout, + DisableKeepAlives: cfg.TransportAttributes.DisableKeepAlives, + DisableCompression: cfg.TransportAttributes.DisableCompression, + MaxIdleConns: cfg.TransportAttributes.MaxIdleConns, + MaxIdleConnsPerHost: cfg.TransportAttributes.MaxIdleConnsPerHost, + MaxConnsPerHost: cfg.TransportAttributes.MaxConnsPerHost, + IdleConnTimeout: cfg.TransportAttributes.IdleConnTimeout, + ResponseHeaderTimeout: cfg.TransportAttributes.ResponseHeaderTimeout, + ExpectContinueTimeout: cfg.TransportAttributes.ExpectContinueTimeout, } - // Load client certificate for mTLS if both Cert and CertKey are provided - if cfg.Cert != nil && cfg.CertKey != nil { - cert, err := commoncfg.LoadClientCertificate(cfg.Cert, cfg.CertKey) - if err != nil { - return nil, fmt.Errorf("failed to load client certificate: %w", err) - } + // Authentication-aware clients already set their own custom RoundTrippers. + // We must wrap the existing one with our transport (do NOT overwrite it). + switch t := client.Transport.(type) { + // OAuth2 wrapper: set its Next transport + case *clientOAuth2RoundTripper: + t.Next = baseTransport - tlsConfig.Certificates = []tls.Certificate{*cert} - } + // API Token wrapper + case *clientAPITokenRoundTripper: + t.Next = baseTransport + + // Basic Auth wrapper + case *clientBasicRoundTripper: + t.Next = baseTransport - // Assign custom transport with TLS configuration - client.Transport = &http.Transport{ - TLSClientConfig: tlsConfig, + // Custom transports: do a safe replacement + case *http.Transport: + // No custom wrapper → replace directly + client.Transport = baseTransport + + default: + // Fallback: wrap unknown transport type + client.Transport = baseTransport } + // Set global timeout + client.Timeout = cfg.Timeout + return client, nil } diff --git a/pkg/commonhttp/client_api-token.go b/pkg/commonhttp/client_api-token.go new file mode 100644 index 0000000..d500f70 --- /dev/null +++ b/pkg/commonhttp/client_api-token.go @@ -0,0 +1,91 @@ +package commonhttp + +import ( + "errors" + "fmt" + "net/http" + + "github.com/openkcm/common-sdk/pkg/commoncfg" +) + +// NewClientFromAPIToken creates a new *http.Client that automatically injects +// an API token into the Authorization header of every request. +// +// The function expects a *commoncfg.SourceRef containing the API token. +// A SourceRef may reference a literal value, environment variable, file, or +// any other supported configuration source. +// +// On success, the returned client wraps the default HTTP transport with a +// custom RoundTripper (clientAPITokenRoundTripper) which adds: +// +// Authorization: Api-Token +// +// Parameters: +// - value: pointer to a SourceRef pointing to the API token. +// +// Returns: +// - *http.Client: configured HTTP client +// - error: if the token reference is nil, unreadable, or empty. +func NewClientFromAPIToken(value *commoncfg.SourceRef) (*http.Client, error) { + if value == nil { + return nil, errors.New("api token auth config is nil") + } + + tokenBytes, err := commoncfg.ExtractValueFromSourceRef(value) + if err != nil { + return nil, fmt.Errorf("api token could not be loaded: %w", err) + } + + if len(tokenBytes) == 0 { + return nil, errors.New("api token is empty") + } + + rt := &clientAPITokenRoundTripper{ + token: string(tokenBytes), + Next: http.DefaultTransport, + } + + return &http.Client{Transport: rt}, nil +} + +// clientAPITokenRoundTripper is a custom HTTP RoundTripper that automatically +// adds an API-token–based Authorization header to all outgoing requests. +// +// Behavior: +// - Adds: `Authorization: Api-Token ` +// - Forwards the modified request to the underlying transport (Next) +// +// The token is injected into headers for *every* HTTP request sent by the +// client returned by NewClientFromAPIToken. +type clientAPITokenRoundTripper struct { + // token is the API token string used to authenticate requests. + token string + + // Next is the underlying HTTP RoundTripper to which the modified request + // is forwarded. Defaults to http.DefaultTransport. + Next http.RoundTripper +} + +// RoundTrip implements the http.RoundTripper interface. +// +// This method: +// 1. Copies the incoming request to avoid mutation. +// 2. Injects the Authorization header using the API token. +// 3. Forwards the request to the underlying transport. +// +// Parameters: +// - req: the outgoing HTTP request. +// +// Returns: +// - *http.Response: the HTTP response returned by the underlying transport. +// - error: if the underlying RoundTripper returns an error. +func (t *clientAPITokenRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + // Create a shallow copy to avoid mutating user-provided request. + newReq := req.Clone(req.Context()) + + // Inject API token header. + newReq.Header.Set("Authorization", "Api-Token "+t.token) + + // Forward the request. + return t.Next.RoundTrip(newReq) +} diff --git a/pkg/commonhttp/client_basic.go b/pkg/commonhttp/client_basic.go new file mode 100644 index 0000000..787ea4d --- /dev/null +++ b/pkg/commonhttp/client_basic.go @@ -0,0 +1,95 @@ +package commonhttp + +import ( + "errors" + "fmt" + "net/http" + + "github.com/openkcm/common-sdk/pkg/commoncfg" +) + +// NewClientFromBasic creates an *http.Client that automatically injects +// HTTP Basic Authentication credentials into every outgoing request. +// +// The BasicAuth struct contains two SourceRef fields (Username and Password): +// each can come from literals, environment variables, files, etc. +// +// Each request sent by the returned client is modified to include: +// +// Authorization: Basic +// +// Parameters: +// - clientAuth: pointer to BasicAuth config containing username & password. +// +// Returns: +// - *http.Client configured with a custom RoundTripper +// - error if configuration is invalid or credentials cannot be loaded +func NewClientFromBasic(clientAuth *commoncfg.BasicAuth) (*http.Client, error) { + if clientAuth == nil { + return nil, errors.New("basic auth config is nil") + } + + usernameBytes, err := commoncfg.ExtractValueFromSourceRef(&clientAuth.Username) + if err != nil { + return nil, fmt.Errorf("basic credentials missing username: %w", err) + } + + passwordBytes, err := commoncfg.ExtractValueFromSourceRef(&clientAuth.Password) + if err != nil { + return nil, fmt.Errorf("basic credentials missing password: %w", err) + } + + if len(usernameBytes) == 0 { + return nil, errors.New("basic auth username is empty") + } + + if len(passwordBytes) == 0 { + return nil, errors.New("basic auth password is empty") + } + + rt := &clientBasicRoundTripper{ + Username: string(usernameBytes), + Password: string(passwordBytes), + Next: http.DefaultTransport, + } + + return &http.Client{Transport: rt}, nil +} + +// clientBasicRoundTripper injects Basic Auth credentials into all outgoing +// HTTP requests when used as the Transport of an http.Client. +// +// It wraps an underlying RoundTripper (Next) and forwards modified requests. +// To avoid side effects, requests are cloned before modification. +type clientBasicRoundTripper struct { + // Username is the Basic Auth username. + Username string + + // Password is the Basic Auth password. + Password string + + // Next is the underlying transport (defaults to http.DefaultTransport). + Next http.RoundTripper +} + +// RoundTrip implements http.RoundTripper by: +// 1. Cloning the request +// 2. Injecting HTTP Basic Authentication headers +// 3. Forwarding the request to the underlying transport +// +// Parameters: +// - req: original request +// +// Returns: +// - *http.Response from the underlying transport +// - error if the underlying transport fails +func (t *clientBasicRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + // Clone the request including its context, headers, and URL. + newReq := req.Clone(req.Context()) + + // Inject Basic Auth header. + newReq.SetBasicAuth(t.Username, t.Password) + + // Forward request. + return t.Next.RoundTrip(newReq) +} diff --git a/pkg/commonhttp/client_test.go b/pkg/commonhttp/client_test.go index b361ff7..8fdce93 100644 --- a/pkg/commonhttp/client_test.go +++ b/pkg/commonhttp/client_test.go @@ -3,7 +3,6 @@ package commonhttp_test import ( "crypto/rand" "crypto/rsa" - "crypto/tls" "crypto/x509" "encoding/pem" "net/http" @@ -19,12 +18,7 @@ func TestNewClient(t *testing.T) { // Arrange mutator := testutils.NewMutator(func() commoncfg.HTTPClient { return commoncfg.HTTPClient{ - Timeout: 10 * time.Second, - RootCAs: nil, - InsecureSkipVerify: false, - MinVersion: tls.VersionTLS12, - Cert: nil, - CertKey: nil, + Timeout: 10 * time.Second, } }) @@ -68,35 +62,40 @@ func TestNewClient(t *testing.T) { cfg: mutator(func(c *commoncfg.HTTPClient) { c.Timeout = 5 * time.Second }), - }, { - name: "custom TLS min version", - cfg: mutator(func(c *commoncfg.HTTPClient) { - c.MinVersion = tls.VersionTLS13 - }), - }, { - name: "insecure skip verify", - cfg: mutator(func(c *commoncfg.HTTPClient) { - c.InsecureSkipVerify = true - }), }, { name: "custom root CAs", cfg: mutator(func(c *commoncfg.HTTPClient) { - c.RootCAs = &commoncfg.SourceRef{ - Source: commoncfg.EmbeddedSourceValue, - Value: x509CertPEM} + c.MTLS = &commoncfg.MTLS{ + Cert: commoncfg.SourceRef{ + Source: commoncfg.EmbeddedSourceValue, + Value: x509CertPEM, + }, + CertKey: commoncfg.SourceRef{ + Source: commoncfg.EmbeddedSourceValue, + Value: string(pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(rsaPrivateKey), + }))}, + ServerCA: &commoncfg.SourceRef{ + Source: commoncfg.EmbeddedSourceValue, + Value: x509CertPEM}, + } }), }, { - name: "custom mTLS certificate", + name: "custom mTLS certificate no Root CA", cfg: mutator(func(c *commoncfg.HTTPClient) { - c.Cert = &commoncfg.SourceRef{ - Source: commoncfg.EmbeddedSourceValue, - Value: x509CertPEM} - c.CertKey = &commoncfg.SourceRef{ - Source: commoncfg.EmbeddedSourceValue, - Value: string(pem.EncodeToMemory(&pem.Block{ - Type: "RSA PRIVATE KEY", - Bytes: x509.MarshalPKCS1PrivateKey(rsaPrivateKey), - }))} + c.MTLS = &commoncfg.MTLS{ + Cert: commoncfg.SourceRef{ + Source: commoncfg.EmbeddedSourceValue, + Value: x509CertPEM, + }, + CertKey: commoncfg.SourceRef{ + Source: commoncfg.EmbeddedSourceValue, + Value: string(pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(rsaPrivateKey), + }))}, + } }), }, } @@ -137,12 +136,4 @@ func checkClient(t *testing.T, client *http.Client, cfg commoncfg.HTTPClient) { if transport.TLSClientConfig == nil { t.Fatal("expected TLSClientConfig to be non-nil") } - - if transport.TLSClientConfig.InsecureSkipVerify != cfg.InsecureSkipVerify { - t.Errorf("expected InsecureSkipVerify to be %v, got %v", cfg.InsecureSkipVerify, transport.TLSClientConfig.InsecureSkipVerify) - } - - if transport.TLSClientConfig.MinVersion != cfg.MinVersion { - t.Errorf("expected MinVersion to be %v, got %v", cfg.MinVersion, transport.TLSClientConfig.MinVersion) - } } diff --git a/pkg/otlp/audit/comm.go b/pkg/otlp/audit/comm.go index 202bbc1..b23a31c 100644 --- a/pkg/otlp/audit/comm.go +++ b/pkg/otlp/audit/comm.go @@ -3,29 +3,28 @@ package otlpaudit import ( "bytes" "context" - "errors" - "fmt" "net/http" + "github.com/samber/oops" "go.opentelemetry.io/collector/pdata/plog" ) func (o *otlpClient) send(ctx context.Context, payload string) error { req, err := http.NewRequestWithContext(ctx, http.MethodPost, o.Endpoint, bytes.NewBufferString(payload)) if err != nil { - return errors.Join(errCreateReqFailed, err) + return oops.In(domain). + Hint("request failed"). + Wrap(err) } req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") - if o.BasicAuth != nil { - req.SetBasicAuth(o.BasicAuth.username, o.BasicAuth.password) - } - resp, err := o.Client.Do(req) if err != nil { - return errors.Join(errReqFailed, err) + return oops.In(domain). + Hint("request failed"). + Wrap(err) } defer func() { @@ -33,7 +32,8 @@ func (o *otlpClient) send(ctx context.Context, payload string) error { }() if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - return fmt.Errorf("%w %d", errStatusNotOK, resp.StatusCode) + return oops.In(domain). + With("status_code", resp.StatusCode).New("response status not OK") } return err @@ -42,19 +42,25 @@ func (o *otlpClient) send(ctx context.Context, payload string) error { func (auditLogger *AuditLogger) SendEvent(ctx context.Context, logs plog.Logs) error { err := auditLogger.enrichLogs(&logs) if err != nil { - return err + return oops.In(domain). + Hint("enrich failed"). + Wrap(err) } marshaller := plog.JSONMarshaler{} marshaledLogs, err := marshaller.MarshalLogs(logs) if err != nil { - return errors.Join(errMarshalingFailed, err) + return oops.In(domain). + Hint("failed to marshal audit logs"). + Wrap(err) } err = auditLogger.client.send(ctx, string(marshaledLogs)) if err != nil { - return err + return oops.In(domain). + Hint("failed to send audit logs"). + Wrap(err) } return nil @@ -63,7 +69,9 @@ func (auditLogger *AuditLogger) SendEvent(ctx context.Context, logs plog.Logs) e func (auditLogger *AuditLogger) enrichLogs(logs *plog.Logs) error { logRecord, err := getFirstLogRecord(*logs) if err != nil { - return err + return oops.In(domain). + Hint("failed to fetch audit logs"). + Wrap(err) } for k, v := range auditLogger.additionalProps { @@ -73,10 +81,15 @@ func (auditLogger *AuditLogger) enrichLogs(logs *plog.Logs) error { return nil } -func getFirstLogRecord(ld plog.Logs) (plog.LogRecord, error) { - if ld.ResourceLogs().Len() > 0 && ld.ResourceLogs().At(0).ScopeLogs().Len() > 0 && ld.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().Len() > 0 { - return ld.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().At(0), nil - } else { - return plog.LogRecord{}, errNoLogRecord +func getFirstLogRecord(ld plog.Logs) (*plog.LogRecord, error) { + exist := ld.ResourceLogs().Len() > 0 && + ld.ResourceLogs().At(0).ScopeLogs().Len() > 0 && + ld.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().Len() > 0 + + if exist { + record := ld.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().At(0) + return &record, nil } + + return nil, errNoLogRecord } diff --git a/pkg/otlp/audit/errors.go b/pkg/otlp/audit/errors.go index 6f06e8a..05113ad 100644 --- a/pkg/otlp/audit/errors.go +++ b/pkg/otlp/audit/errors.go @@ -4,11 +4,6 @@ import ( "errors" ) -var errCreateReqFailed = errors.New("request creation failed") -var errReqFailed = errors.New("request failed") -var errStatusNotOK = errors.New("response status not OK, got: ") -var errLoadValue = errors.New("failed to load value from ref: ") -var errMarshalingFailed = errors.New("marshaling failed: ") +var domain = "audit-logger:otlp" var errEventCreation = errors.New("event creation failed") -var errLoadMTLSConfigFailed = errors.New("tls: failed to find any PEM data in certificate input") var errNoLogRecord = errors.New("no log record present in the plog.Logs struct") diff --git a/pkg/otlp/audit/logger.go b/pkg/otlp/audit/logger.go index 752c488..cf48c34 100644 --- a/pkg/otlp/audit/logger.go +++ b/pkg/otlp/audit/logger.go @@ -1,13 +1,12 @@ package otlpaudit import ( - "errors" "net/http" - "time" "gopkg.in/yaml.v3" "github.com/openkcm/common-sdk/pkg/commoncfg" + "github.com/openkcm/common-sdk/pkg/commonhttp" ) type AuditLogger struct { @@ -16,42 +15,19 @@ type AuditLogger struct { } type otlpClient struct { - Endpoint string - Client *http.Client - BasicAuth *basicAuth -} - -type basicAuth struct { - username, password string + Endpoint string + Client *http.Client } func NewLogger(config *commoncfg.Audit) (*AuditLogger, error) { - var b basicAuth - - tr := &http.Transport{ - MaxIdleConns: 10, - IdleConnTimeout: 30 * time.Second, - } - - if config.MTLS != nil { - tlsConfig, err := commoncfg.LoadMTLSConfig(config.MTLS) - if err != nil { - return nil, err - } - - tr.TLSClientConfig = tlsConfig - } else if config.BasicAuth != nil { - var err error - - b, err = loadBasicAuth(config, b) - if err != nil { - return nil, err - } + client, err := commonhttp.NewClient(&config.HTTPClient) + if err != nil { + return nil, err } var m map[string]string - err := yaml.Unmarshal([]byte(config.AdditionalProperties), &m) + err = yaml.Unmarshal([]byte(config.AdditionalProperties), &m) if err != nil { return nil, err } @@ -59,31 +35,8 @@ func NewLogger(config *commoncfg.Audit) (*AuditLogger, error) { return &AuditLogger{ client: otlpClient{ Endpoint: config.Endpoint, - Client: &http.Client{ - Transport: tr, - Timeout: 30 * time.Second, - }, - BasicAuth: &b, + Client: client, }, additionalProps: m, }, nil } - -func loadBasicAuth(config *commoncfg.Audit, b basicAuth) (basicAuth, error) { - u, err := commoncfg.ExtractValueFromSourceRef(&config.BasicAuth.Username) - if err != nil { - return basicAuth{}, errors.Join(errLoadValue, err) - } - - p, err := commoncfg.ExtractValueFromSourceRef(&config.BasicAuth.Password) - if err != nil { - return basicAuth{}, errors.Join(errLoadValue, err) - } - - b = basicAuth{ - username: string(u), - password: string(p), - } - - return b, nil -} diff --git a/pkg/otlp/audit/logger_test.go b/pkg/otlp/audit/logger_test.go index cda20e0..9ddc9a2 100644 --- a/pkg/otlp/audit/logger_test.go +++ b/pkg/otlp/audit/logger_test.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" "testing" + "time" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/plog" @@ -16,6 +17,10 @@ import ( "github.com/openkcm/common-sdk/pkg/commoncfg" ) +var errLoadValue = errors.New("basic credentials missing password: field password not found") +var errLoadMTLSConfigFailed = errors.New("tls: failed to find any PEM data in certificate input") +var errStatusNotOK = errors.New("response status not OK") + const ( testFilesDir = "../../../internal/otlp/audit/testdata" testConfigDir = "testConfigs" @@ -90,8 +95,14 @@ func TestSend(t *testing.T) { auditLogger, _ := NewLogger(&cfg.Audit) err = auditLogger.SendEvent(t.Context(), logs) - if (tt.expectError != nil && !errors.Is(err, tt.expectError)) || (err == nil && tt.expectError != nil) { - t.Errorf("Expected error '%v', got '%v'", tt.expectError, err) + if tt.expectError != nil { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.expectError) + } + + if !strings.Contains(err.Error(), tt.expectError.Error()) { + t.Fatalf("expected error containing %q, got %q", tt.expectError, err.Error()) + } } if err != nil && tt.expectError == nil { @@ -166,9 +177,11 @@ func Test_NewLogger(t *testing.T) { func Test_EnrichLogs(t *testing.T) { auditCfg := commoncfg.Audit{ - Endpoint: "http://localhost:1234/logs", - MTLS: nil, - BasicAuth: nil, + Endpoint: "http://localhost:1234/logs", + HTTPClient: commoncfg.HTTPClient{ + Timeout: 10 * time.Second, + }, + AdditionalProperties: "Prop1: Val1\nProp2: Val2", } logs := plog.NewLogs() diff --git a/pkg/otlp/opentelemetry.go b/pkg/otlp/opentelemetry.go index 8c57b74..402721c 100644 --- a/pkg/otlp/opentelemetry.go +++ b/pkg/otlp/opentelemetry.go @@ -2,6 +2,7 @@ package otlp import ( "context" + "fmt" "log/slog" "os" "sync" @@ -35,7 +36,9 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.37.0" "github.com/openkcm/common-sdk/pkg/commoncfg" + "github.com/openkcm/common-sdk/pkg/commonhttp" "github.com/openkcm/common-sdk/pkg/logger" + "github.com/openkcm/common-sdk/pkg/utils" ) const ( @@ -307,12 +310,19 @@ func initTraceGrpcExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otlp switch cfg.Traces.SecretRef.Type { case commoncfg.ApiTokenSecretType: - credential, err := commoncfg.ExtractValueFromSourceRef(&cfg.Traces.SecretRef.APIToken) + token, err := computeAPITokenAuthorizationHeader(&cfg.Traces.SecretRef.APIToken) if err != nil { return nil, err } - sec = otlptracegrpc.WithHeaders(map[string]string{"Authorization": "Api-Token " + string(credential)}) + sec = otlptracegrpc.WithHeaders(map[string]string{"Authorization": token}) + case commoncfg.BasicSecretType: + value, err := computeBasicAuthorizationHeader(&cfg.Traces.SecretRef.Basic) + if err != nil { + return nil, err + } + + sec = otlptracegrpc.WithHeaders(map[string]string{"Authorization": value}) case commoncfg.MTLSSecretType: tlsConfig, err := commoncfg.LoadMTLSConfig(&cfg.Traces.SecretRef.MTLS) if err != nil { @@ -322,6 +332,8 @@ func initTraceGrpcExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otlp sec = otlptracegrpc.WithTLSCredentials(credentials.NewTLS(tlsConfig)) case commoncfg.InsecureSecretType: sec = otlptracegrpc.WithInsecure() + default: + return nil, fmt.Errorf("trace grpc doesn't unsupport secret type: %s", cfg.Traces.SecretRef.Type) } host, err := commoncfg.ExtractValueFromSourceRef(&cfg.Traces.Host) @@ -343,12 +355,12 @@ func initTraceHTTPExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otlp switch cfg.Traces.SecretRef.Type { case commoncfg.ApiTokenSecretType: - credential, err := commoncfg.ExtractValueFromSourceRef(&cfg.Traces.SecretRef.APIToken) + client, err := commonhttp.NewClientFromAPIToken(&cfg.Traces.SecretRef.APIToken) if err != nil { return nil, err } - sec = otlptracehttp.WithHeaders(map[string]string{"Authorization": "Api-Token " + string(credential)}) + sec = otlptracehttp.WithHTTPClient(client) case commoncfg.MTLSSecretType: tlsConfig, err := commoncfg.LoadMTLSConfig(&cfg.Traces.SecretRef.MTLS) if err != nil { @@ -356,6 +368,20 @@ func initTraceHTTPExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otlp } sec = otlptracehttp.WithTLSClientConfig(tlsConfig) + case commoncfg.BasicSecretType: + httpClient, err := commonhttp.NewClientFromBasic(&cfg.Traces.SecretRef.Basic) + if err != nil { + return nil, err + } + + otlptracehttp.WithHTTPClient(httpClient) + case commoncfg.OAuth2SecretType: + httpClient, err := commonhttp.NewClientFromOAuth2(&cfg.Traces.SecretRef.OAuth2) + if err != nil { + return nil, err + } + + otlptracehttp.WithHTTPClient(httpClient) case commoncfg.InsecureSecretType: sec = otlptracehttp.WithInsecure() } @@ -443,12 +469,19 @@ func initMetricGrpcExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl switch cfg.Metrics.SecretRef.Type { case commoncfg.ApiTokenSecretType: - credential, err := commoncfg.ExtractValueFromSourceRef(&cfg.Metrics.SecretRef.APIToken) + token, err := computeAPITokenAuthorizationHeader(&cfg.Metrics.SecretRef.APIToken) if err != nil { return nil, err } - sec = otlpmetricgrpc.WithHeaders(map[string]string{"Authorization": "Api-Token " + string(credential)}) + sec = otlpmetricgrpc.WithHeaders(map[string]string{"Authorization": token}) + case commoncfg.BasicSecretType: + value, err := computeBasicAuthorizationHeader(&cfg.Metrics.SecretRef.Basic) + if err != nil { + return nil, err + } + + sec = otlpmetricgrpc.WithHeaders(map[string]string{"Authorization": value}) case commoncfg.MTLSSecretType: tlsConfig, err := commoncfg.LoadMTLSConfig(&cfg.Metrics.SecretRef.MTLS) if err != nil { @@ -458,6 +491,8 @@ func initMetricGrpcExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl sec = otlpmetricgrpc.WithTLSCredentials(credentials.NewTLS(tlsConfig)) case commoncfg.InsecureSecretType: sec = otlpmetricgrpc.WithInsecure() + default: + return nil, fmt.Errorf("metric grpc doesn't unsupport secret type: %s", cfg.Metrics.SecretRef.Type) } host, err := commoncfg.ExtractValueFromSourceRef(&cfg.Metrics.Host) @@ -480,12 +515,12 @@ func initMetricHTTPExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl switch cfg.Metrics.SecretRef.Type { case commoncfg.ApiTokenSecretType: - credential, err := commoncfg.ExtractValueFromSourceRef(&cfg.Metrics.SecretRef.APIToken) + client, err := commonhttp.NewClientFromAPIToken(&cfg.Metrics.SecretRef.APIToken) if err != nil { return nil, err } - sec = otlpmetrichttp.WithHeaders(map[string]string{"Authorization": "Api-Token " + string(credential)}) + sec = otlpmetrichttp.WithHTTPClient(client) case commoncfg.MTLSSecretType: tlsConfig, err := commoncfg.LoadMTLSConfig(&cfg.Metrics.SecretRef.MTLS) if err != nil { @@ -493,6 +528,20 @@ func initMetricHTTPExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl } sec = otlpmetrichttp.WithTLSClientConfig(tlsConfig) + case commoncfg.BasicSecretType: + httpClient, err := commonhttp.NewClientFromBasic(&cfg.Metrics.SecretRef.Basic) + if err != nil { + return nil, err + } + + otlpmetrichttp.WithHTTPClient(httpClient) + case commoncfg.OAuth2SecretType: + httpClient, err := commonhttp.NewClientFromOAuth2(&cfg.Metrics.SecretRef.OAuth2) + if err != nil { + return nil, err + } + + otlpmetrichttp.WithHTTPClient(httpClient) case commoncfg.InsecureSecretType: sec = otlpmetrichttp.WithInsecure() } @@ -578,12 +627,19 @@ func initLoggerGrpcExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl switch cfg.Logs.SecretRef.Type { case commoncfg.ApiTokenSecretType: - credential, err := commoncfg.ExtractValueFromSourceRef(&cfg.Logs.SecretRef.APIToken) + token, err := computeAPITokenAuthorizationHeader(&cfg.Logs.SecretRef.APIToken) if err != nil { return nil, err } - sec = otlploggrpc.WithHeaders(map[string]string{"Authorization": "Api-Token " + string(credential)}) + sec = otlploggrpc.WithHeaders(map[string]string{"Authorization": token}) + case commoncfg.BasicSecretType: + value, err := computeBasicAuthorizationHeader(&cfg.Logs.SecretRef.Basic) + if err != nil { + return nil, err + } + + sec = otlploggrpc.WithHeaders(map[string]string{"Authorization": value}) case commoncfg.MTLSSecretType: tlsConfig, err := commoncfg.LoadMTLSConfig(&cfg.Logs.SecretRef.MTLS) if err != nil { @@ -593,6 +649,8 @@ func initLoggerGrpcExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl sec = otlploggrpc.WithTLSCredentials(credentials.NewTLS(tlsConfig)) case commoncfg.InsecureSecretType: sec = otlploggrpc.WithInsecure() + default: + return nil, fmt.Errorf("logger grpc doesn't unsupport secret type: %s", cfg.Logs.SecretRef.Type) } host, err := commoncfg.ExtractValueFromSourceRef(&cfg.Logs.Host) @@ -612,12 +670,12 @@ func initLoggerHTTPExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl switch cfg.Logs.SecretRef.Type { case commoncfg.ApiTokenSecretType: - credential, err := commoncfg.ExtractValueFromSourceRef(&cfg.Logs.SecretRef.APIToken) + client, err := commonhttp.NewClientFromAPIToken(&cfg.Logs.SecretRef.APIToken) if err != nil { return nil, err } - sec = otlploghttp.WithHeaders(map[string]string{"Authorization": "Api-Token " + string(credential)}) + sec = otlploghttp.WithHTTPClient(client) case commoncfg.MTLSSecretType: tlsConfig, err := commoncfg.LoadMTLSConfig(&cfg.Logs.SecretRef.MTLS) if err != nil { @@ -625,6 +683,20 @@ func initLoggerHTTPExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl } sec = otlploghttp.WithTLSClientConfig(tlsConfig) + case commoncfg.BasicSecretType: + httpClient, err := commonhttp.NewClientFromBasic(&cfg.Logs.SecretRef.Basic) + if err != nil { + return nil, err + } + + otlploghttp.WithHTTPClient(httpClient) + case commoncfg.OAuth2SecretType: + httpClient, err := commonhttp.NewClientFromOAuth2(&cfg.Logs.SecretRef.OAuth2) + if err != nil { + return nil, err + } + + otlploghttp.WithHTTPClient(httpClient) case commoncfg.InsecureSecretType: sec = otlploghttp.WithInsecure() } @@ -641,3 +713,26 @@ func initLoggerHTTPExporter(ctx context.Context, cfg *commoncfg.Telemetry) (*otl sec, ) } + +func computeBasicAuthorizationHeader(basicAuth *commoncfg.BasicAuth) (string, error) { + username, err := commoncfg.ExtractValueFromSourceRef(&basicAuth.Username) + if err != nil { + return "", fmt.Errorf("failed to extract basic auth username: %w", err) + } + + password, err := commoncfg.ExtractValueFromSourceRef(&basicAuth.Password) + if err != nil { + return "", fmt.Errorf("failed to extract basic auth password: %w", err) + } + + return "Basic " + utils.BasicAuth(string(username), string(password)), nil +} + +func computeAPITokenAuthorizationHeader(token *commoncfg.SourceRef) (string, error) { + value, err := commoncfg.ExtractValueFromSourceRef(token) + if err != nil { + return "", fmt.Errorf("failed to extract api token value: %w", err) + } + + return "Api-Token " + string(value), nil +} diff --git a/pkg/utils/auth.go b/pkg/utils/auth.go new file mode 100644 index 0000000..d26ab05 --- /dev/null +++ b/pkg/utils/auth.go @@ -0,0 +1,13 @@ +package utils + +import "encoding/base64" + +// See 2 (end of page 4) https://www.ietf.org/rfc/rfc2617.txt +// "To receive authorization, the client sends the userid and password, +// separated by a single colon (":") character, within a base64 +// encoded string in the credentials." +// It is not meant to be urlencoded. +func BasicAuth(username, password string) string { + auth := username + ":" + password + return base64.StdEncoding.EncodeToString([]byte(auth)) +} From a2d16a943e82ac43d2e87ad6b62547ae34cc21ca Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Wed, 26 Nov 2025 20:39:28 +0100 Subject: [PATCH 02/10] rename a function --- pkg/otlp/audit/comm.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/otlp/audit/comm.go b/pkg/otlp/audit/comm.go index b23a31c..762a7a8 100644 --- a/pkg/otlp/audit/comm.go +++ b/pkg/otlp/audit/comm.go @@ -67,7 +67,7 @@ func (auditLogger *AuditLogger) SendEvent(ctx context.Context, logs plog.Logs) e } func (auditLogger *AuditLogger) enrichLogs(logs *plog.Logs) error { - logRecord, err := getFirstLogRecord(*logs) + logRecord, err := firstLogRecord(*logs) if err != nil { return oops.In(domain). Hint("failed to fetch audit logs"). @@ -81,7 +81,7 @@ func (auditLogger *AuditLogger) enrichLogs(logs *plog.Logs) error { return nil } -func getFirstLogRecord(ld plog.Logs) (*plog.LogRecord, error) { +func firstLogRecord(ld plog.Logs) (*plog.LogRecord, error) { exist := ld.ResourceLogs().Len() > 0 && ld.ResourceLogs().At(0).ScopeLogs().Len() > 0 && ld.ResourceLogs().At(0).ScopeLogs().At(0).LogRecords().Len() > 0 From ca489ebae02cd1eea90db651b6bc933fb6e522e0 Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Wed, 26 Nov 2025 20:41:25 +0100 Subject: [PATCH 03/10] renamed the error message --- pkg/otlp/audit/comm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/otlp/audit/comm.go b/pkg/otlp/audit/comm.go index 762a7a8..d2740c4 100644 --- a/pkg/otlp/audit/comm.go +++ b/pkg/otlp/audit/comm.go @@ -70,7 +70,7 @@ func (auditLogger *AuditLogger) enrichLogs(logs *plog.Logs) error { logRecord, err := firstLogRecord(*logs) if err != nil { return oops.In(domain). - Hint("failed to fetch audit logs"). + Hint("failed to find audit record log"). Wrap(err) } From 3b2c2ee0214d1b2bd45d95c522ddce31555b3534 Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Sun, 7 Dec 2025 16:23:39 +0100 Subject: [PATCH 04/10] add RootCA variable on the MTLS structure as naming alternative --- pkg/commoncfg/config.go | 35 ++++++++++++++++++++++++++++++-- pkg/commoncfg/loader.go | 22 ++++++++++++++++++-- pkg/commongrpc/dynamic_client.go | 5 +++++ 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/pkg/commoncfg/config.go b/pkg/commoncfg/config.go index 7e8b24e..42f8ad2 100644 --- a/pkg/commoncfg/config.go +++ b/pkg/commoncfg/config.go @@ -198,9 +198,40 @@ type SecretRef struct { // MTLS holds mTLS configuration for audit library. type MTLS struct { - Cert SourceRef `yaml:"cert" json:"cert"` - CertKey SourceRef `yaml:"certKey" json:"certKey"` + Cert SourceRef `yaml:"cert" json:"cert"` + CertKey SourceRef `yaml:"certKey" json:"certKey"` + + // ServerCA or RootCA not both, as the ServerCA has precedence, rootCA was added to remove the name confusion ServerCA *SourceRef `yaml:"serverCa" json:"serverCa"` + RootCA *SourceRef `yaml:"rootCa" json:"rootCa"` + + Attributes *TLSAttributes `yaml:"attributes" json:"attributes"` +} + +type TLSAttributes struct { + // InsecureSkipVerify controls whether a client verifies the server's + // certificate chain and host name. If InsecureSkipVerify is true, crypto/tls + // accepts any certificate presented by the server and any host name in that + // certificate. In this mode, TLS is susceptible to machine-in-the-middle + // attacks unless custom verification is used. This should be used only for + // testing or in combination with VerifyConnection or VerifyPeerCertificate. + InsecureSkipVerify bool `yaml:"insecureSkipVerify" json:"insecureSkipVerify"` + // ServerName is used to verify the hostname on the returned + // certificates unless InsecureSkipVerify is given. It is also included + // in the client's handshake to support virtual hosting unless it is + // an IP address. + ServerName string `yaml:"serverName" json:"serverName"` + + // SessionTicketsDisabled may be set to true to disable session ticket and + // PSK (resumption) support. Note that on clients, session ticket support is + // also disabled if ClientSessionCache is nil. + SessionTicketsDisabled bool `yaml:"sessionTicketsDisabled" json:"sessionTicketsDisabled"` + + // DynamicRecordSizingDisabled disables adaptive sizing of TLS records. + // When true, the largest possible TLS record size is always used. When + // false, the size of TLS records may be adjusted in an attempt to + // improve latency. + DynamicRecordSizingDisabled bool `yaml:"dynamicRecordSizingDisabled" json:"dynamicRecordSizingDisabled"` } // Audit holds the audit log library configuration. diff --git a/pkg/commoncfg/loader.go b/pkg/commoncfg/loader.go index 8d65c35..d96b3a1 100644 --- a/pkg/commoncfg/loader.go +++ b/pkg/commoncfg/loader.go @@ -214,7 +214,18 @@ func LoadMTLSCACertPool(cfg *MTLS) (*x509.CertPool, error) { return nil, ErrMTLSIsNil } - return LoadCACertPool(cfg.ServerCA) + certPool, err := LoadCACertPool(cfg.ServerCA) + if err != nil { + return nil, err + } + if certPool == nil { + certPool, err = LoadCACertPool(cfg.RootCA) + if err != nil { + return nil, err + } + } + + return certPool, err } func LoadCACertPool(certRef *SourceRef) (*x509.CertPool, error) { @@ -277,7 +288,14 @@ func LoadMTLSConfig(cfg *MTLS) (*tls.Config, error) { MinVersion: tls.VersionTLS12, } - caCertPool, err := LoadCACertPool(cfg.ServerCA) + if cfg.Attributes != nil { + tlsConfig.InsecureSkipVerify = cfg.Attributes.InsecureSkipVerify + tlsConfig.ServerName = cfg.Attributes.ServerName + tlsConfig.SessionTicketsDisabled = cfg.Attributes.SessionTicketsDisabled + tlsConfig.SessionTicketsDisabled = cfg.Attributes.SessionTicketsDisabled + } + + caCertPool, err := LoadMTLSCACertPool(cfg) if err != nil { return nil, err } diff --git a/pkg/commongrpc/dynamic_client.go b/pkg/commongrpc/dynamic_client.go index 8a0f9ec..05e72cf 100644 --- a/pkg/commongrpc/dynamic_client.go +++ b/pkg/commongrpc/dynamic_client.go @@ -143,6 +143,11 @@ func (dcc *DynamicClientConn) initAndStartNotifierOnMTLSCredentials(throttleInte if caPath != "" { paths = append(paths, filepath.Dir(caPath)) } + } else if mtls.RootCA != nil { + caPath := strings.TrimSpace(mtls.RootCA.File.Path) + if caPath != "" { + paths = append(paths, filepath.Dir(caPath)) + } } nt, err := notifier.Create( From abfeb7e9855c659e9b68059d576cfde60045432b Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Sun, 7 Dec 2025 16:55:37 +0100 Subject: [PATCH 05/10] add the possibility to have multiple root ca --- pkg/commoncfg/config.go | 5 ++--- pkg/commoncfg/loader.go | 37 +++++++++++++++++++++++++------ pkg/commonfs/notifier/notifier.go | 3 +-- pkg/commongrpc/dynamic_client.go | 21 ++++++++++++------ 4 files changed, 47 insertions(+), 19 deletions(-) diff --git a/pkg/commoncfg/config.go b/pkg/commoncfg/config.go index 42f8ad2..bebeecc 100644 --- a/pkg/commoncfg/config.go +++ b/pkg/commoncfg/config.go @@ -201,9 +201,8 @@ type MTLS struct { Cert SourceRef `yaml:"cert" json:"cert"` CertKey SourceRef `yaml:"certKey" json:"certKey"` - // ServerCA or RootCA not both, as the ServerCA has precedence, rootCA was added to remove the name confusion - ServerCA *SourceRef `yaml:"serverCa" json:"serverCa"` - RootCA *SourceRef `yaml:"rootCa" json:"rootCa"` + ServerCA *SourceRef `yaml:"serverCa" json:"serverCa"` + RootCAs []SourceRef `yaml:"rootCAs,omitempty" json:"rootCAs,omitempty"` Attributes *TLSAttributes `yaml:"attributes" json:"attributes"` } diff --git a/pkg/commoncfg/loader.go b/pkg/commoncfg/loader.go index d96b3a1..dc90b90 100644 --- a/pkg/commoncfg/loader.go +++ b/pkg/commoncfg/loader.go @@ -214,16 +214,19 @@ func LoadMTLSCACertPool(cfg *MTLS) (*x509.CertPool, error) { return nil, ErrMTLSIsNil } - certPool, err := LoadCACertPool(cfg.ServerCA) + cas := make([]SourceRef, 0) + if cfg.ServerCA != nil { + cas = append(cas, *cfg.ServerCA) + } + + if len(cfg.RootCAs) > 0 { + cas = append(cas, cfg.RootCAs...) + } + + certPool, err := LoadCAsCertPool(cas) if err != nil { return nil, err } - if certPool == nil { - certPool, err = LoadCACertPool(cfg.RootCA) - if err != nil { - return nil, err - } - } return certPool, err } @@ -246,6 +249,26 @@ func LoadCACertPool(certRef *SourceRef) (*x509.CertPool, error) { return caCertPool, nil } +func LoadCAsCertPool(certRefs []SourceRef) (*x509.CertPool, error) { + if len(certRefs) == 0 { + // Returns nil instead of NewCertPool if no CA is provided, which means using the system CA pool + return nil, nil //nolint:nilnil + } + + caCertPool := x509.NewCertPool() + + for _, cert := range certRefs { + caCert, err := ExtractValueFromSourceRef(&cert) + if err != nil { + return nil, err + } + + caCertPool.AppendCertsFromPEM(caCert) + } + + return caCertPool, nil +} + func LoadClientCertificate(certRef, keyRef *SourceRef) (*tls.Certificate, error) { if certRef == nil { return nil, ErrCertificateIsNil diff --git a/pkg/commonfs/notifier/notifier.go b/pkg/commonfs/notifier/notifier.go index 87e55a5..97dfb3b 100644 --- a/pkg/commonfs/notifier/notifier.go +++ b/pkg/commonfs/notifier/notifier.go @@ -265,10 +265,9 @@ func (n *Notifier) AddPath(path string) error { return fmt.Errorf("path does not exist: %s", absPath) } - n.paths = append(n.paths, absPath) - _, ok := n.cacheEvents[absPath] if !ok { + n.paths = append(n.paths, absPath) n.cacheEvents[absPath] = make([]fsnotify.Event, 0) } diff --git a/pkg/commongrpc/dynamic_client.go b/pkg/commongrpc/dynamic_client.go index 05e72cf..81ec88e 100644 --- a/pkg/commongrpc/dynamic_client.go +++ b/pkg/commongrpc/dynamic_client.go @@ -126,30 +126,37 @@ func NewDynamicClientConn(cfg *commoncfg.GRPCClient, throttleInterval time.Durat // 3. Starts the notifier and stores it in the DynamicClientConn instance. // 4. Returns any errors encountered during creation or startup of the notifier. func (dcc *DynamicClientConn) initAndStartNotifierOnMTLSCredentials(throttleInterval time.Duration, mtls *commoncfg.MTLS) error { - var paths []string + pathMap := map[string]struct{}{} certPath := strings.TrimSpace(mtls.Cert.File.Path) if certPath != "" { - paths = append(paths, filepath.Dir(certPath)) + pathMap[filepath.Dir(certPath)] = struct{}{} } keyPath := strings.TrimSpace(mtls.CertKey.File.Path) if keyPath != "" { - paths = append(paths, filepath.Dir(keyPath)) + pathMap[filepath.Dir(keyPath)] = struct{}{} } if mtls.ServerCA != nil { caPath := strings.TrimSpace(mtls.ServerCA.File.Path) if caPath != "" { - paths = append(paths, filepath.Dir(caPath)) + pathMap[filepath.Dir(caPath)] = struct{}{} } - } else if mtls.RootCA != nil { - caPath := strings.TrimSpace(mtls.RootCA.File.Path) + } + + for _, ca := range mtls.RootCAs { + caPath := strings.TrimSpace(ca.File.Path) if caPath != "" { - paths = append(paths, filepath.Dir(caPath)) + pathMap[filepath.Dir(caPath)] = struct{}{} } } + paths := make([]string, 0, len(pathMap)) + for path := range pathMap { + paths = append(paths, path) + } + nt, err := notifier.Create( notifier.OnPaths(paths...), notifier.WithEventHandler(dcc.eventHandler), From fbe6e6cf6e5c99f4572f88dc64e842d73fe76f27 Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Wed, 18 Feb 2026 16:30:41 +0100 Subject: [PATCH 06/10] add sonar-project.properties file --- sonar-project.properties | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 sonar-project.properties diff --git a/sonar-project.properties b/sonar-project.properties new file mode 100644 index 0000000..725765e --- /dev/null +++ b/sonar-project.properties @@ -0,0 +1,2 @@ +sonar.test.inclusions=**/*_test.go +sonar.coverage.exclusions=**/*_test.go \ No newline at end of file From d368c46a375005a12d3a01599bdc43a3b315584b Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Wed, 18 Feb 2026 16:31:37 +0100 Subject: [PATCH 07/10] add pull-requests: read pemission to ci workflow --- .github/workflows/ci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 2ca3e72..855b55f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,6 +15,7 @@ on: permissions: contents: read + pull-requests: read jobs: From cf84c9e1f6c6388bf2bb4a2374b7baccd1e5fc4c Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Thu, 19 Feb 2026 16:51:49 +0100 Subject: [PATCH 08/10] fix the lints --- pkg/commoncfg/config.go | 12 ++--- pkg/commonhttp/client.go | 94 ----------------------------------- pkg/commonhttp/client_test.go | 4 +- 3 files changed, 8 insertions(+), 102 deletions(-) diff --git a/pkg/commoncfg/config.go b/pkg/commoncfg/config.go index 5ccc703..af91f49 100644 --- a/pkg/commoncfg/config.go +++ b/pkg/commoncfg/config.go @@ -368,12 +368,12 @@ type GRPCClientAttributes struct { } type HTTPClient struct { - Timeout time.Duration `yaml:"timeout" json:"timeout" default:"30s"` - APIToken *SourceRef `yaml:"apiToken" json:"apiToken"` - BasicAuth *BasicAuth `yaml:"basicAuth" json:"basicAuth"` - OAuth2Auth *OAuth2 `yaml:"oauth2Auth" json:"oauth2Auth"` - MTLS *MTLS `yaml:"mtls" json:"mtls"` - TransportAttributes HTTPTransportAttributes `yaml:"transportAttributes" json:"transportAttributes"` + Timeout time.Duration `yaml:"timeout" json:"timeout" default:"30s"` + APIToken *SourceRef `yaml:"apiToken" json:"apiToken"` + BasicAuth *BasicAuth `yaml:"basicAuth" json:"basicAuth"` + OAuth2Auth *OAuth2 `yaml:"oauth2Auth" json:"oauth2Auth"` + MTLS *MTLS `yaml:"mtls" json:"mtls"` + TransportAttributes *HTTPTransportAttributes `yaml:"transportAttributes" json:"transportAttributes"` } type HTTPTransportAttributes struct { diff --git a/pkg/commonhttp/client.go b/pkg/commonhttp/client.go index e08881b..aed36ad 100644 --- a/pkg/commonhttp/client.go +++ b/pkg/commonhttp/client.go @@ -9,100 +9,6 @@ import ( "github.com/openkcm/common-sdk/pkg/commoncfg" ) -// NewClient creates an *http.Client using the full HTTPClient configuration. -// -// Supports: -// - Timeout -// - TLS minimum version (default TLS1.2) -// - InsecureSkipVerify -// - Custom root CAs -// - Optional client certificates (mTLS) -// -// Deprecated [to be replaced with NewHTTPClient] -func NewClient(cfg *commoncfg.HTTPClient) (*http.Client, error) { - if cfg == nil { - return nil, errors.New("HTTPClient config is nil") - } - - var ( - client *http.Client - err error - ) - - // Select authentication mechanism (if any) - switch { - case cfg.BasicAuth != nil: - client, err = NewClientFromBasic(cfg.BasicAuth) - case cfg.OAuth2Auth != nil: - client, err = NewClientFromOAuth2(cfg.OAuth2Auth) - case cfg.APIToken != nil: - client, err = NewClientFromAPIToken(cfg.APIToken) - default: - // No authentication → start with a default client - client = &http.Client{Transport: http.DefaultTransport} - } - - if err != nil { - return nil, err - } - - // Start building TLS configuration - var tlsConfig *tls.Config - if cfg.MTLS != nil { - tlsConfig, err = commoncfg.LoadMTLSConfig(cfg.MTLS) - if err != nil { - return nil, fmt.Errorf("failed to load tls config: %w", err) - } - } else { - // keep default system roots if no mTLS is used - tlsConfig = &tls.Config{} - } - - // Build a proper base transport - baseTransport := &http.Transport{ - TLSClientConfig: tlsConfig, - TLSHandshakeTimeout: cfg.TransportAttributes.TLSHandshakeTimeout, - DisableKeepAlives: cfg.TransportAttributes.DisableKeepAlives, - DisableCompression: cfg.TransportAttributes.DisableCompression, - MaxIdleConns: cfg.TransportAttributes.MaxIdleConns, - MaxIdleConnsPerHost: cfg.TransportAttributes.MaxIdleConnsPerHost, - MaxConnsPerHost: cfg.TransportAttributes.MaxConnsPerHost, - IdleConnTimeout: cfg.TransportAttributes.IdleConnTimeout, - ResponseHeaderTimeout: cfg.TransportAttributes.ResponseHeaderTimeout, - ExpectContinueTimeout: cfg.TransportAttributes.ExpectContinueTimeout, - } - - // Authentication-aware clients already set their own custom RoundTrippers. - // We must wrap the existing one with our transport (do NOT overwrite it). - switch t := client.Transport.(type) { - // OAuth2 wrapper: set its Next transport - case *clientOAuth2RoundTripper: - t.Next = baseTransport - - // API Token wrapper - case *clientAPITokenRoundTripper: - t.Next = baseTransport - - // Basic Auth wrapper - case *clientBasicRoundTripper: - t.Next = baseTransport - - // Custom transports: do a safe replacement - case *http.Transport: - // No custom wrapper → replace directly - client.Transport = baseTransport - - default: - // Fallback: wrap unknown transport type - client.Transport = baseTransport - } - - // Set global timeout - client.Timeout = cfg.Timeout - - return client, nil -} - // NewHTTPClient creates an *http.Client using the full HTTPClient configuration. // // It supports the following authentication methods: diff --git a/pkg/commonhttp/client_test.go b/pkg/commonhttp/client_test.go index af62b16..271c152 100644 --- a/pkg/commonhttp/client_test.go +++ b/pkg/commonhttp/client_test.go @@ -42,7 +42,7 @@ func TestNewClient(t *testing.T) { // test nil config t.Run("nil config", func(t *testing.T) { - client, err := commonhttp.NewClient(nil) + client, err := commonhttp.NewHTTPClient(nil) if err == nil { t.Errorf("expected error for nil config, got client: %v", client) } @@ -104,7 +104,7 @@ func TestNewClient(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // Act - client, err := commonhttp.NewClient(&tc.cfg) + client, err := commonhttp.NewHTTPClient(&tc.cfg) // Assert if err != nil { From a0ff47c5c8b2e54eac06a9ed2567338d3ca2ec71 Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Thu, 19 Feb 2026 17:11:55 +0100 Subject: [PATCH 09/10] fix the lints --- pkg/commoncfg/config.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/commoncfg/config.go b/pkg/commoncfg/config.go index af91f49..cf4feeb 100644 --- a/pkg/commoncfg/config.go +++ b/pkg/commoncfg/config.go @@ -368,7 +368,8 @@ type GRPCClientAttributes struct { } type HTTPClient struct { - Timeout time.Duration `yaml:"timeout" json:"timeout" default:"30s"` + Timeout time.Duration `yaml:"timeout" json:"timeout" default:"30s"` + APIToken *SourceRef `yaml:"apiToken" json:"apiToken"` BasicAuth *BasicAuth `yaml:"basicAuth" json:"basicAuth"` OAuth2Auth *OAuth2 `yaml:"oauth2Auth" json:"oauth2Auth"` From 4bedffc4f1910c77fec952898f7f9461c4eddcc6 Mon Sep 17 00:00:00 2001 From: Nicolae Nicora Date: Wed, 15 Apr 2026 11:27:33 +0200 Subject: [PATCH 10/10] small change --- pkg/commoncfg/config.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/pkg/commoncfg/config.go b/pkg/commoncfg/config.go index de653e4..3e92076 100644 --- a/pkg/commoncfg/config.go +++ b/pkg/commoncfg/config.go @@ -370,17 +370,6 @@ type GRPCClientAttributes struct { type HTTPClient struct { Timeout time.Duration `yaml:"timeout" json:"timeout" default:"10s" mapstructure:"timeout"` - //Deprecated [to be replaced by using MTLS] - RootCAs *SourceRef `yaml:"rootCAs" json:"rootCAs" mapstructure:"rootCAs"` - //Deprecated [to be replaced by using MTLS] - InsecureSkipVerify bool `yaml:"insecureSkipVerify" json:"insecureSkipVerify" mapstructure:"insecureSkipVerify"` - //Deprecated [to be replaced by using MTLS] - MinVersion uint16 `yaml:"minVersion" json:"minVersion" mapstructure:"minVersion"` - //Deprecated [to be replaced by using MTLS] - Cert *SourceRef `yaml:"cert" json:"cert" mapstructure:"cert"` - //Deprecated [to be replaced by using MTLS] - CertKey *SourceRef `yaml:"certKey" json:"certKey" mapstructure:"certKey"` - APIToken *SourceRef `yaml:"apiToken" json:"apiToken" mapstructure:"apiToken"` BasicAuth *BasicAuth `yaml:"basicAuth" json:"basicAuth" mapstructure:"basicAuth"` OAuth2Auth *OAuth2 `yaml:"oauth2Auth" json:"oauth2Auth" mapstructure:"oauth2Auth"`