Skip to content

Commit 3323822

Browse files
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 3323822

16 files changed

Lines changed: 848 additions & 13 deletions

File tree

api/oci/config/httpconfig_test.go

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

api/oci/config/httptype.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
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+
t.SetHTTPSettings(&a.HTTPSettings)
35+
return nil
36+
}
37+
38+
const httpUsage = `
39+
The config type <code>` + HTTPConfigType + `</code> can be used to configure
40+
HTTP client settings:
41+
42+
<pre>
43+
type: generic.config.ocm.software/v1
44+
configurations:
45+
- type: ` + HTTPConfigType + `
46+
timeout: "0s"
47+
tcpDialTimeout: "30s"
48+
tcpKeepAlive: "30s"
49+
tlsHandshakeTimeout: "10s"
50+
responseHeaderTimeout: "0s"
51+
idleConnTimeout: "90s"
52+
</pre>
53+
54+
All timeout values are Go duration strings (e.g. "30s", "5m", "1h").
55+
Use "0s" to disable a specific timeout. If not set, the <code>http.DefaultTransport</code>
56+
values from the Go standard library are used.
57+
58+
Note: <code>timeout</code> controls the overall <code>http.Client</code> request deadline and is
59+
independent of the transport-level settings. Setting only <code>timeout</code> does not
60+
affect <code>tcpDialTimeout</code>, <code>tlsHandshakeTimeout</code>, or other transport timeouts.
61+
`

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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ 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+
client, err := newDockerClient(spec.DockerHost, logger, ctx.GetHTTPSettings())
2727
if err != nil {
2828
return nil, err
2929
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ 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+
client, err := newDockerClient(a.DockerHost, logger, ctx.GetHTTPSettings())
6060
if err != nil {
6161
return err
6262
}

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

Lines changed: 21 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,19 @@ 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+
httpCfg := r.GetContext().GetHTTPSettings()
158+
baseTransport, err := httpclient.NewTransport(httpCfg)
159+
if err != nil {
160+
return nil, err
161+
}
162+
timeout, err := httpCfg.GetTimeout()
163+
if err != nil {
164+
return nil, err
165+
}
166+
158167
if r.info.Scheme == "https" {
159-
// set up TLS
160168
//nolint:gosec // used like the default, there are OCI servers (quay.io) not working with min version.
161-
conf := &tls.Config{
169+
baseTransport.TLSClientConfig = &tls.Config{
162170
// MinVersion: tls.VersionTLS13,
163171
RootCAs: func() *x509.CertPool {
164172
rootCAs := rootcertsattr.Get(r.GetContext()).GetRootCertPool(true)
@@ -168,13 +176,18 @@ func (r *RepositoryImpl) getResolver(comp string) (oras.Resolver, error) {
168176
rootCAs.AppendCertsFromPEM([]byte(c))
169177
}
170178
}
171-
172179
return rootCAs
173180
}(),
174181
}
175-
client.Transport = ocmlog.NewRoundTripper(retry.NewTransport(&http.Transport{
176-
TLSClientConfig: conf,
177-
}), logger)
182+
}
183+
184+
retryTransport := retry.NewTransport(baseTransport)
185+
186+
client := &http.Client{
187+
Transport: ocmlog.NewRoundTripper(retryTransport, logger),
188+
}
189+
if timeout != nil {
190+
client.Timeout = *timeout
178191
}
179192

180193
authClient := &auth.Client{
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)