Skip to content

promquery: update connection logic to match new http.DefaultTransport defaults #309

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

Merged
majewsky merged 1 commit into from
Apr 1, 2026
Merged

promquery: update connection logic to match new http.DefaultTransport defaults #309

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
81 changes: 81 additions & 0 deletions internal/httputil/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company
// SPDX-License-Identifier: Apache-2.0

package httputil

import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net"
"net/http"
"os"
"time"
)

// TransportOpts contains options for building a *http.Transport object.
type TransportOpts struct {
ServerCACertificatePath string
ClientCertificatePath string
ClientCertificateKeyPath string
}

// NewTransport builds an *http.Transport based on the provided options.
// If no special options are set, this will return an instance
// that is functionally equivalent to the default settings of http.Defaultt.
func NewTransport(opts TransportOpts) (*http.Transport, error) {
if opts.ClientCertificatePath == "" && opts.ClientCertificateKeyPath != "" {
return nil, errors.New("private key given, but no client certificate given")
}
if opts.ClientCertificatePath != "" && opts.ClientCertificateKeyPath == "" {
return nil, errors.New("client certificate given, but no private key given")
}

// This is intended to construct `result` in the same way as net/http.DefaultTransport.
// If TestDefaultTransport fails, update this paragraph to match the initialization of that variable in std.
//
// NOTE: We are not just using http.DefaultTransport.Clone() because:
// 1) http.DefaultTransport is an http.RoundTripper and may contain a type other than *http.Transport
// 2) http.Transport.Clone() has known bugs, see <https://github.com/golang/go/issues/39302>
result := &http.Transport{
Proxy: http.ProxyFromEnvironment,
DialContext: (&net.Dialer{
Timeout: 30 * time.Second,
KeepAlive: 30 * time.Second,
}).DialContext,
ForceAttemptHTTP2: true,
MaxIdleConns: 100,
IdleConnTimeout: 90 * time.Second,
TLSHandshakeTimeout: 10 * time.Second,
ExpectContinueTimeout: 1 * time.Second,
}

if opts.ClientCertificatePath != "" || opts.ServerCACertificatePath != "" {
// only instantiate TLSClientConfig when actually necessary; its presence may disable
// useful behaviors like HTTP/2-by-default, so it should only be present when necessary
result.TLSClientConfig = &tls.Config{}
}

if opts.ClientCertificatePath != "" {
clientCert, err := tls.LoadX509KeyPair(opts.ClientCertificatePath, opts.ClientCertificateKeyPath)
if err != nil {
return nil, fmt.Errorf("cannot load client certificate from %s and %s: %w",
opts.ClientCertificatePath, opts.ClientCertificateKeyPath, err)
}
result.TLSClientConfig.Certificates = []tls.Certificate{clientCert}
}

if opts.ServerCACertificatePath != "" {
serverCACert, err := os.ReadFile(opts.ServerCACertificatePath)
if err != nil {
return nil, fmt.Errorf("cannot load CA certificate from %s: %w",
opts.ServerCACertificatePath, err)
}
certPool := x509.NewCertPool()
certPool.AppendCertsFromPEM(serverCACert)
result.TLSClientConfig.RootCAs = certPool
}

return result, nil
}
32 changes: 32 additions & 0 deletions internal/httputil/transport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// SPDX-FileCopyrightText: 2026 SAP SE or an SAP affiliate company
// SPDX-License-Identifier: Apache-2.0

package httputil_test

import (
"net/http"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"

"github.com/sapcc/go-bits/internal/httputil"
"github.com/sapcc/go-bits/must"
)

func TestDefaultTransport(t *testing.T) {
actual := must.ReturnT(httputil.NewTransport(httputil.TransportOpts{}))(t)
expected := http.DefaultTransport.(*http.Transport)

// we cannot use reflect.DeepEqual() to compare both sides because there are
// private fields with pointers that will always be different, and that is
// intentional (we don't want to carbon-copy the default transport object and
// thus share its internal mutexes etc.)
diff := cmp.Diff(expected, actual,
cmpopts.IgnoreUnexported(http.Transport{}),
cmpopts.IgnoreFields(http.Transport{}, "Proxy", "DialContext"), // cannot deep-compare function pointers
)
if diff != "" {
t.Errorf("mismatch in default transport (-httputil.NewTransport +http.DefaultTransport):\n%s", diff)
}
}
48 changes: 8 additions & 40 deletions promquery/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,14 @@
package promquery

import (
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net"
"net/http"
"os"
"time"

prom_api "github.com/prometheus/client_golang/api"
prom_v1 "github.com/prometheus/client_golang/api/prometheus/v1"

"github.com/sapcc/go-bits/internal/httputil"
"github.com/sapcc/go-bits/osext"
)

Expand Down Expand Up @@ -64,42 +60,14 @@ func (cfg Config) Connect() (Client, error) {
if cfg.ServerURL == "" {
return Client{}, errors.New("cannot connect to Prometheus: missing server URL")
}
if cfg.ClientCertificatePath == "" && cfg.ClientCertificateKeyPath != "" {
return Client{}, fmt.Errorf("cannot connect to Prometheus at %s: private key given, but no client certificate given", cfg.ServerURL)
}
if cfg.ClientCertificatePath != "" && cfg.ClientCertificateKeyPath == "" {
return Client{}, fmt.Errorf("cannot connect to Prometheus at %s: client certificate given, but no private key given", cfg.ServerURL)
}

// same configuration as prom_api.DefaultRoundTripper (but we cannot just clone it because it contains a Mutex)
transport := &http.Transport{
Proxy: http.ProxyFromEnvironment,
DialContext: (&net.Dialer{
Timeout: 30 * time.Second,
KeepAlive: 30 * time.Second,
}).DialContext,
TLSHandshakeTimeout: 10 * time.Second,
}
transport.TLSClientConfig = &tls.Config{}

if cfg.ClientCertificatePath != "" {
clientCert, err := tls.LoadX509KeyPair(cfg.ClientCertificatePath, cfg.ClientCertificateKeyPath)
if err != nil {
return Client{}, fmt.Errorf("cannot load client certificate from %s and %s: %w",
cfg.ClientCertificatePath, cfg.ClientCertificateKeyPath, err)
}
transport.TLSClientConfig.Certificates = []tls.Certificate{clientCert}
}

if cfg.ServerCACertificatePath != "" {
serverCACert, err := os.ReadFile(cfg.ServerCACertificatePath)
if err != nil {
return Client{}, fmt.Errorf("cannot load CA certificate from %s: %w",
cfg.ServerCACertificatePath, err)
}
certPool := x509.NewCertPool()
certPool.AppendCertsFromPEM(serverCACert)
transport.TLSClientConfig.RootCAs = certPool
transport, err := httputil.NewTransport(httputil.TransportOpts{
ServerCACertificatePath: cfg.ServerCACertificatePath,
ClientCertificatePath: cfg.ClientCertificatePath,
ClientCertificateKeyPath: cfg.ClientCertificateKeyPath,
})
if err != nil {
return Client{}, fmt.Errorf("cannot connect to Prometheus at %s: %w", cfg.ServerURL, err)
}

promCfg := prom_api.Config{
Expand Down
Loading