Skip to content

Commit d76a870

Browse files
piotrjanikpiotrjanik
committed
feat: add configurable HTTP client timeouts via config file
Adds a new `http.config.ocm.software` config type that allows users to configure HTTP client timeout settings for OCI registry and Docker daemon connections. This prevents requests from hanging indefinitely on slow or broken networks. Supported timeout fields (all optional, Go duration strings like "30s", "5m"): - timeout — overall http.Client deadline - tcpDialTimeout — TCP connection establishment - tcpKeepAlive — TCP keep-alive probe interval - tlsHandshakeTimeout — TLS handshake - responseHeaderTimeout — waiting for response headers - idleConnTimeout — idle connection lifetime Design: - Follows the regular OCI config pattern (like credentials config) — HTTPSettings is stored on the OCI context via GetHTTPSettings()/SetHTTPSettings(), not as a datacontext attribute. - Duration is a validated string type with TimeDuration() returning (*time.Duration, error) — nil means "not configured" (use default), distinct from zero ("explicitly disabled"). - httpclient.NewTransport() clones http.DefaultTransport and selectively overrides only the configured timeouts. - Integration tests use Toxiproxy to verify timeout behavior against a real registry with injected latency. Fixes: #1731 Signed-off-by: Piotr Janik <piotr.janik@sap.com>
1 parent b776b68 commit d76a870

16 files changed

Lines changed: 873 additions & 13 deletions

File tree

‎api/oci/config/httpconfig_test.go‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package config_test
2+
3+
import (
4+
"time"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
9+
"ocm.software/ocm/api/oci/config"
10+
"ocm.software/ocm/api/oci/cpi"
11+
)
12+
13+
func dur(s string) *cpi.Duration {
14+
d := cpi.Duration(s)
15+
return &d
16+
}
17+
18+
func MustGetHTTPSettings(ctx cpi.Context) cpi.HTTPSettings {
19+
g, err := ctx.GetHTTPSettings()
20+
ExpectWithOffset(1, err).To(Succeed())
21+
return g
22+
}
23+
24+
var _ = Describe("http config", func() {
25+
Context("apply", func() {
26+
It("applies timeout via ApplyTo", func() {
27+
ctx := cpi.New()
28+
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("5m")}}
29+
30+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
31+
g := MustGetHTTPSettings(ctx)
32+
Expect(g.GetTimeout()).To(HaveValue(Equal(5 * time.Minute)))
33+
})
34+
35+
It("applies via config context", func() {
36+
ctx := cpi.New()
37+
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("30s")}}
38+
39+
Expect(ctx.ConfigContext().ApplyConfig(cfg, "programmatic")).To(Succeed())
40+
g := MustGetHTTPSettings(ctx)
41+
Expect(g.GetTimeout()).To(HaveValue(Equal(30 * time.Second)))
42+
})
43+
44+
It("parses all fields from JSON config", func() {
45+
ctx := cpi.New()
46+
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":"10s","tcpDialTimeout":"15s","tcpKeepAlive":"20s","tlsHandshakeTimeout":"5s","responseHeaderTimeout":"8s","idleConnTimeout":"45s"}`)
47+
cfg, err := ctx.ConfigContext().GetConfigForData(raw, nil)
48+
Expect(err).To(Succeed())
49+
Expect(ctx.ConfigContext().ApplyConfig(cfg, "config file")).To(Succeed())
50+
51+
g := MustGetHTTPSettings(ctx)
52+
Expect(g.GetTimeout()).To(HaveValue(Equal(10 * time.Second)))
53+
Expect(g.TCPDialTimeout.TimeDuration()).To(HaveValue(Equal(15 * time.Second)))
54+
Expect(g.TCPKeepAlive.TimeDuration()).To(HaveValue(Equal(20 * time.Second)))
55+
Expect(g.TLSHandshakeTimeout.TimeDuration()).To(HaveValue(Equal(5 * time.Second)))
56+
Expect(g.ResponseHeaderTimeout.TimeDuration()).To(HaveValue(Equal(8 * time.Second)))
57+
Expect(g.IdleConnTimeout.TimeDuration()).To(HaveValue(Equal(45 * time.Second)))
58+
})
59+
60+
It("successive ApplyConfig overrides only non-nil fields", func() {
61+
ctx := cpi.New()
62+
63+
first := &config.HTTPConfig{
64+
HTTPSettings: cpi.HTTPSettings{
65+
TCPDialTimeout: dur("15s"),
66+
},
67+
}
68+
Expect(first.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
69+
70+
second := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("1m")}}
71+
Expect(second.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
72+
73+
g := MustGetHTTPSettings(ctx)
74+
Expect(g.GetTimeout()).To(HaveValue(Equal(1 * time.Minute)))
75+
Expect(g.TCPDialTimeout.TimeDuration()).To(HaveValue(Equal(15 * time.Second)))
76+
})
77+
78+
It("applies via generic config wrapper", func() {
79+
ctx := cpi.New()
80+
raw := []byte(`
81+
type: generic.config.ocm.software/v1
82+
configurations:
83+
- type: http.config.ocm.software
84+
timeout: "10s"
85+
tcpDialTimeout: "15s"
86+
tcpKeepAlive: "20s"
87+
tlsHandshakeTimeout: "5s"
88+
responseHeaderTimeout: "8s"
89+
idleConnTimeout: "45s"
90+
`)
91+
cfg, err := ctx.ConfigContext().GetConfigForData(raw, nil)
92+
Expect(err).To(Succeed())
93+
Expect(ctx.ConfigContext().ApplyConfig(cfg, "config file")).To(Succeed())
94+
95+
g := MustGetHTTPSettings(ctx)
96+
Expect(g.GetTimeout()).To(HaveValue(Equal(10 * time.Second)))
97+
Expect(g.GetTCPDialTimeout()).To(HaveValue(Equal(15 * time.Second)))
98+
Expect(g.GetTCPKeepAlive()).To(HaveValue(Equal(20 * time.Second)))
99+
Expect(g.GetTLSHandshakeTimeout()).To(HaveValue(Equal(5 * time.Second)))
100+
Expect(g.GetResponseHeaderTimeout()).To(HaveValue(Equal(8 * time.Second)))
101+
Expect(g.GetIdleConnTimeout()).To(HaveValue(Equal(45 * time.Second)))
102+
})
103+
104+
It("rejects invalid duration string", func() {
105+
ctx := cpi.New()
106+
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":"notaduration"}`)
107+
_, err := ctx.ConfigContext().GetConfigForData(raw, nil)
108+
Expect(err).To(HaveOccurred())
109+
Expect(err.Error()).To(ContainSubstring("invalid duration: notaduration"))
110+
})
111+
112+
It("default settings are nil", func() {
113+
g := MustGetHTTPSettings(cpi.New())
114+
Expect(g.Timeout).To(BeNil())
115+
Expect(g.TCPDialTimeout).To(BeNil())
116+
Expect(g.TCPKeepAlive).To(BeNil())
117+
Expect(g.TLSHandshakeTimeout).To(BeNil())
118+
Expect(g.ResponseHeaderTimeout).To(BeNil())
119+
Expect(g.IdleConnTimeout).To(BeNil())
120+
})
121+
122+
It("nil timeout returns nil not zero", func() {
123+
g := MustGetHTTPSettings(cpi.New())
124+
timeout, err := g.GetTimeout()
125+
Expect(err).To(Succeed())
126+
Expect(timeout).To(BeNil())
127+
})
128+
})
129+
})

‎api/oci/config/httptype.go‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
package config
2+
3+
import (
4+
cfgcpi "ocm.software/ocm/api/config/cpi"
5+
"ocm.software/ocm/api/oci/cpi"
6+
"ocm.software/ocm/api/utils/runtime"
7+
)
8+
9+
const (
10+
HTTPConfigType = "http" + cfgcpi.OCM_CONFIG_TYPE_SUFFIX
11+
HTTPConfigTypeV1Alpha1 = HTTPConfigType + runtime.VersionSeparator + "v1alpha1"
12+
)
13+
14+
func init() {
15+
cfgcpi.RegisterConfigType(cfgcpi.NewConfigType[*HTTPConfig](HTTPConfigType, httpUsage))
16+
cfgcpi.RegisterConfigType(cfgcpi.NewConfigType[*HTTPConfig](HTTPConfigTypeV1Alpha1, httpUsage))
17+
}
18+
19+
// HTTPConfig describes the configuration for HTTP client settings.
20+
type HTTPConfig struct {
21+
runtime.ObjectVersionedType `json:",inline"`
22+
cpi.HTTPSettings `json:",inline"`
23+
}
24+
25+
func (a *HTTPConfig) GetType() string {
26+
return HTTPConfigType
27+
}
28+
29+
func (a *HTTPConfig) ApplyTo(_ cfgcpi.Context, target interface{}) error {
30+
t, ok := target.(cpi.Context)
31+
if !ok {
32+
return cfgcpi.ErrNoContext(HTTPConfigType)
33+
}
34+
s, err := t.GetHTTPSettings()
35+
if err != nil {
36+
return err
37+
}
38+
if a.Timeout != nil {
39+
s.Timeout = a.Timeout
40+
}
41+
if a.TCPDialTimeout != nil {
42+
s.TCPDialTimeout = a.TCPDialTimeout
43+
}
44+
if a.TCPKeepAlive != nil {
45+
s.TCPKeepAlive = a.TCPKeepAlive
46+
}
47+
if a.TLSHandshakeTimeout != nil {
48+
s.TLSHandshakeTimeout = a.TLSHandshakeTimeout
49+
}
50+
if a.ResponseHeaderTimeout != nil {
51+
s.ResponseHeaderTimeout = a.ResponseHeaderTimeout
52+
}
53+
if a.IdleConnTimeout != nil {
54+
s.IdleConnTimeout = a.IdleConnTimeout
55+
}
56+
t.SetHTTPSettings(&s)
57+
return nil
58+
}
59+
60+
const httpUsage = `
61+
The config type <code>` + HTTPConfigType + `</code> can be used to configure
62+
HTTP client settings:
63+
64+
<pre>
65+
type: generic.config.ocm.software/v1
66+
configurations:
67+
- type: ` + HTTPConfigType + `
68+
timeout: "0s"
69+
tcpDialTimeout: "30s"
70+
tcpKeepAlive: "30s"
71+
tlsHandshakeTimeout: "10s"
72+
responseHeaderTimeout: "0s"
73+
idleConnTimeout: "90s"
74+
</pre>
75+
76+
All timeout values are Go duration strings (e.g. "30s", "5m", "1h").
77+
Use "0s" to disable a specific timeout. If not set, the <code>http.DefaultTransport</code>
78+
values from the Go standard library are used.
79+
80+
Note: <code>timeout</code> controls the overall <code>http.Client</code> request deadline and is
81+
independent of the transport-level settings. Setting only <code>timeout</code> does not
82+
affect <code>tcpDialTimeout</code>, <code>tlsHandshakeTimeout</code>, or other transport timeouts.
83+
`

‎api/oci/cpi/interface.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ type (
4242
DataAccess = internal.DataAccess
4343
RepositorySource = internal.RepositorySource
4444
ConsumerIdentityProvider = internal.ConsumerIdentityProvider
45+
Duration = internal.Duration
46+
HTTPSettings = internal.HTTPSettings
4547
)
4648

4749
type Descriptor = ociv1.Descriptor

‎api/oci/extensions/repositories/docker/client.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,18 @@ import (
1313
dockerclient "github.com/moby/moby/client"
1414
"github.com/spf13/pflag"
1515

16+
"ocm.software/ocm/api/oci/cpi"
17+
"ocm.software/ocm/api/utils/httpclient"
1618
"ocm.software/ocm/api/utils/logging"
1719
)
1820

19-
func newDockerClient(dockerhost string, logger mlog.UnboundLogger) (*dockerclient.Client, error) {
21+
func newDockerClient(dockerhost string, logger mlog.UnboundLogger, httpCfg *cpi.HTTPSettings) (*dockerclient.Client, error) {
2022
if dockerhost == "" {
23+
// Use Docker CLI context resolution (DOCKER_CONTEXT env,
24+
// currentContext in ~/.docker/config.json, DOCKER_HOST env,
25+
// default socket). NewAPIClientFromFlags builds its own HTTP
26+
// transport internally, so OCM HTTP timeout settings do not
27+
// apply to this path.
2128
opts := cliflags.NewClientOptions()
2229
// set defaults
2330
opts.SetDefaultOptions(pflag.NewFlagSet("", pflag.ContinueOnError))
@@ -35,8 +42,22 @@ func newDockerClient(dockerhost string, logger mlog.UnboundLogger) (*dockerclien
3542
if err == nil && url.Scheme == "unix" {
3643
opts = append(opts, dockerclient.WithScheme(url.Scheme))
3744
}
38-
clnt := http.Client{}
39-
clnt.Transport = logging.NewRoundTripper(clnt.Transport, logger)
45+
46+
transport, err := httpclient.NewTransport(httpCfg)
47+
if err != nil {
48+
return nil, err
49+
}
50+
timeout, err := httpCfg.GetTimeout()
51+
if err != nil {
52+
return nil, err
53+
}
54+
55+
clnt := http.Client{
56+
Transport: logging.NewRoundTripper(transport, logger),
57+
}
58+
if timeout != nil {
59+
clnt.Timeout = *timeout
60+
}
4061
opts = append(opts, dockerclient.WithHTTPClient(&clnt))
4162
c, err := dockerclient.New(opts...)
4263
if err != nil {

‎api/oci/extensions/repositories/docker/repository.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,12 @@ var _ cpi.RepositoryImpl = (*RepositoryImpl)(nil)
2323
func NewRepository(ctx cpi.Context, spec *RepositorySpec) (cpi.Repository, error) {
2424
urs := spec.UniformRepositorySpec()
2525
logger := logging.DynamicLogger(ctx, REALM, logging.NewAttribute(ocmlog.ATTR_HOST, urs.Host))
26-
client, err := newDockerClient(spec.DockerHost, logger)
26+
httpSettings, err := ctx.GetHTTPSettings()
27+
if err != nil {
28+
return nil, err
29+
}
30+
httpCfg := &httpSettings
31+
client, err := newDockerClient(spec.DockerHost, logger, httpCfg)
2732
if err != nil {
2833
return nil, err
2934
}

‎api/oci/extensions/repositories/docker/type.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ func (a *RepositorySpec) Repository(ctx cpi.Context, creds credentials.Credentia
5656
func (a *RepositorySpec) Validate(ctx cpi.Context, creds credentials.Credentials, usageContext ...credentials.UsageContext) error {
5757
urs := a.UniformRepositorySpec()
5858
logger := logging.DynamicLogger(ctx, REALM, logging.NewAttribute(ocmlog.ATTR_HOST, urs.Host))
59-
client, err := newDockerClient(a.DockerHost, logger)
59+
httpSettings, err := ctx.GetHTTPSettings()
60+
if err != nil {
61+
return err
62+
}
63+
client, err := newDockerClient(a.DockerHost, logger, &httpSettings)
6064
if err != nil {
6165
return err
6266
}

‎api/oci/extensions/repositories/ocireg/repository.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"ocm.software/ocm/api/tech/oci/identity"
2323
"ocm.software/ocm/api/tech/oras"
2424
"ocm.software/ocm/api/utils"
25+
"ocm.software/ocm/api/utils/httpclient"
2526
ocmlog "ocm.software/ocm/api/utils/logging"
2627
"ocm.software/ocm/api/utils/refmgmt"
2728
)
@@ -153,12 +154,23 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) {
153154
}
154155
}
155156

156-
client := retry.DefaultClient
157-
client.Transport = ocmlog.NewRoundTripper(retry.DefaultClient.Transport, logger)
157+
httpSettings, err := r.GetContext().GetHTTPSettings()
158+
if err != nil {
159+
return nil, err
160+
}
161+
httpCfg := &httpSettings
162+
baseTransport, err := httpclient.NewTransport(httpCfg)
163+
if err != nil {
164+
return nil, err
165+
}
166+
timeout, err := httpCfg.GetTimeout()
167+
if err != nil {
168+
return nil, err
169+
}
170+
158171
if r.info.Scheme == "https" {
159-
// set up TLS
160172
//nolint:gosec // used like the default, there are OCI servers (quay.io) not working with min version.
161-
conf := &tls.Config{
173+
baseTransport.TLSClientConfig = &tls.Config{
162174
// MinVersion: tls.VersionTLS13,
163175
RootCAs: func() *x509.CertPool {
164176
rootCAs := rootcertsattr.Get(r.GetContext()).GetRootCertPool(true)
@@ -168,13 +180,18 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) {
168180
rootCAs.AppendCertsFromPEM([]byte(c))
169181
}
170182
}
171-
172183
return rootCAs
173184
}(),
174185
}
175-
client.Transport = ocmlog.NewRoundTripper(retry.NewTransport(&http.Transport{
176-
TLSClientConfig: conf,
177-
}), logger)
186+
}
187+
188+
retryTransport := retry.NewTransport(baseTransport)
189+
190+
client := &http.Client{
191+
Transport: ocmlog.NewRoundTripper(retryTransport, logger),
192+
}
193+
if timeout != nil {
194+
client.Timeout = *timeout
178195
}
179196

180197
authClient := &auth.Client{

‎api/oci/extensions/repositories/ocireg/suite_test.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package ocireg_test
2+
3+
import (
4+
"testing"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
)
9+
10+
func TestOCIReg(t *testing.T) {
11+
RegisterFailHandler(Fail)
12+
RunSpecs(t, "OCIReg")
13+
}

0 commit comments

Comments
 (0)