Skip to content

Commit f35213c

Browse files
piotrjanikjakobmoellerdev
piotrjanik
and
jakobmoellerdev
authored
feat: add configurable HTTP client timeouts via config file (#1887)
#### What this PR does / why we need it 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. #### Which issue(s) this PR fixes Fixes: #1731 <!-- markdownlint-disable MD041 --> #### What this PR does / why we need it #### Which issue(s) this PR is related to <!-- Usage: `Related to #<issue number>`, or `Related to (paste link of issue)`. --> --------- Signed-off-by: Piotr Janik <piotr.janik@sap.com> Co-authored-by: Jakob Möller <jakob.moeller@sap.com>
1 parent 7d7576d commit f35213c

18 files changed

Lines changed: 1008 additions & 26 deletions

File tree

‎.github/config/wordlist.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ templater
274274
templaters
275275
templating
276276
tgz
277+
tcp
277278
tls
278279
todo
279280
toi

‎api/oci/config/httpconfig.go‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
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+
if err := a.HTTPSettings.Validate(); err != nil {
35+
return err
36+
}
37+
s, err := t.GetHTTPSettings()
38+
if err != nil {
39+
return err
40+
}
41+
if a.Timeout != nil {
42+
s.Timeout = a.Timeout
43+
}
44+
if a.TCPDialTimeout != nil {
45+
s.TCPDialTimeout = a.TCPDialTimeout
46+
}
47+
if a.TCPKeepAlive != nil {
48+
s.TCPKeepAlive = a.TCPKeepAlive
49+
}
50+
if a.TLSHandshakeTimeout != nil {
51+
s.TLSHandshakeTimeout = a.TLSHandshakeTimeout
52+
}
53+
if a.ResponseHeaderTimeout != nil {
54+
s.ResponseHeaderTimeout = a.ResponseHeaderTimeout
55+
}
56+
if a.IdleConnTimeout != nil {
57+
s.IdleConnTimeout = a.IdleConnTimeout
58+
}
59+
t.SetHTTPSettings(&s)
60+
return nil
61+
}
62+
63+
const httpUsage = `
64+
The config type <code>` + HTTPConfigType + `</code> can be used to configure
65+
HTTP client settings:
66+
67+
<pre>
68+
type: generic.config.ocm.software/v1
69+
configurations:
70+
- type: ` + HTTPConfigType + `
71+
timeout: "0s"
72+
tcpDialTimeout: "30s"
73+
tcpKeepAlive: "30s"
74+
tlsHandshakeTimeout: "10s"
75+
responseHeaderTimeout: "0s"
76+
idleConnTimeout: "90s"
77+
</pre>
78+
79+
All timeout values are Go duration strings (e.g. "30s", "5m", "1h").
80+
Use "0s" to disable a specific timeout. If not set, the <code>http.DefaultTransport</code>
81+
values from the Go standard library are used.
82+
83+
The fields have the following meaning:
84+
85+
- <code>timeout</code> &mdash; specifies a time limit for requests made by the HTTP
86+
client. The timeout includes connection time, any redirects, and reading
87+
the response body. A timeout of zero means no timeout.
88+
89+
- <code>tcpDialTimeout</code> &mdash; the maximum amount of time a dial will wait
90+
for a TCP connect to complete. When dialing a host name with multiple IP
91+
addresses, the timeout may be divided between them. The operating system
92+
may impose its own earlier timeout.
93+
94+
- <code>tcpKeepAlive</code> &mdash; specifies the interval between keep-alive
95+
probes for an active network connection. If negative, keep-alive probes
96+
are disabled.
97+
98+
- <code>tlsHandshakeTimeout</code> &mdash; specifies the maximum amount of time
99+
to wait for a TLS handshake. Zero means no timeout.
100+
101+
- <code>responseHeaderTimeout</code> &mdash; specifies the amount of time to wait
102+
for a server's response headers after fully writing the request (including
103+
its body, if any). This time does not include the time to read the response
104+
body.
105+
106+
- <code>idleConnTimeout</code> &mdash; the maximum amount of time an idle
107+
(keep-alive) connection will remain idle before closing itself. Zero means
108+
no limit.
109+
`

‎api/oci/config/httpconfig_test.go‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
package config_test
2+
3+
import (
4+
"encoding/json"
5+
"time"
6+
7+
. "github.com/onsi/ginkgo/v2"
8+
. "github.com/onsi/gomega"
9+
10+
"ocm.software/ocm/api/oci/config"
11+
"ocm.software/ocm/api/oci/cpi"
12+
)
13+
14+
func dur(s string) *cpi.Duration {
15+
td, err := time.ParseDuration(s)
16+
if err != nil {
17+
panic(err)
18+
}
19+
d := cpi.Duration(td)
20+
return &d
21+
}
22+
23+
func MustGetHTTPSettings(ctx cpi.Context) cpi.HTTPSettings {
24+
g, err := ctx.GetHTTPSettings()
25+
ExpectWithOffset(1, err).To(Succeed())
26+
return g
27+
}
28+
29+
var _ = Describe("http config", func() {
30+
Context("apply", func() {
31+
It("applies timeout via ApplyTo", func() {
32+
ctx := cpi.New()
33+
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("5m")}}
34+
35+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
36+
g := MustGetHTTPSettings(ctx)
37+
Expect(time.Duration(*g.Timeout)).To(Equal(5 * time.Minute))
38+
})
39+
40+
It("applies via config context", func() {
41+
ctx := cpi.New()
42+
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("30s")}}
43+
44+
Expect(ctx.ConfigContext().ApplyConfig(cfg, "programmatic")).To(Succeed())
45+
g := MustGetHTTPSettings(ctx)
46+
Expect(time.Duration(*g.Timeout)).To(Equal(30 * time.Second))
47+
})
48+
49+
It("parses all fields from JSON config", func() {
50+
ctx := cpi.New()
51+
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":"10s","tcpDialTimeout":"15s","tcpKeepAlive":"20s","tlsHandshakeTimeout":"5s","responseHeaderTimeout":"8s","idleConnTimeout":"45s"}`)
52+
cfg, err := ctx.ConfigContext().GetConfigForData(raw, nil)
53+
Expect(err).To(Succeed())
54+
Expect(ctx.ConfigContext().ApplyConfig(cfg, "config file")).To(Succeed())
55+
56+
g := MustGetHTTPSettings(ctx)
57+
Expect(time.Duration(*g.Timeout)).To(Equal(10 * time.Second))
58+
Expect(time.Duration(*g.TCPDialTimeout)).To(Equal(15 * time.Second))
59+
Expect(time.Duration(*g.TCPKeepAlive)).To(Equal(20 * time.Second))
60+
Expect(time.Duration(*g.TLSHandshakeTimeout)).To(Equal(5 * time.Second))
61+
Expect(time.Duration(*g.ResponseHeaderTimeout)).To(Equal(8 * time.Second))
62+
Expect(time.Duration(*g.IdleConnTimeout)).To(Equal(45 * time.Second))
63+
})
64+
65+
It("successive ApplyConfig overrides only non-nil fields", func() {
66+
ctx := cpi.New()
67+
68+
first := &config.HTTPConfig{
69+
HTTPSettings: cpi.HTTPSettings{
70+
TCPDialTimeout: dur("15s"),
71+
},
72+
}
73+
Expect(first.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
74+
75+
second := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("1m")}}
76+
Expect(second.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
77+
78+
g := MustGetHTTPSettings(ctx)
79+
Expect(time.Duration(*g.Timeout)).To(Equal(1 * time.Minute))
80+
Expect(time.Duration(*g.TCPDialTimeout)).To(Equal(15 * time.Second))
81+
})
82+
83+
It("applies via generic config wrapper", func() {
84+
ctx := cpi.New()
85+
raw := []byte(`
86+
type: generic.config.ocm.software/v1
87+
configurations:
88+
- type: http.config.ocm.software
89+
timeout: "10s"
90+
tcpDialTimeout: "15s"
91+
tcpKeepAlive: "20s"
92+
tlsHandshakeTimeout: "5s"
93+
responseHeaderTimeout: "8s"
94+
idleConnTimeout: "45s"
95+
`)
96+
cfg, err := ctx.ConfigContext().GetConfigForData(raw, nil)
97+
Expect(err).To(Succeed())
98+
Expect(ctx.ConfigContext().ApplyConfig(cfg, "config file")).To(Succeed())
99+
100+
g := MustGetHTTPSettings(ctx)
101+
Expect(time.Duration(*g.Timeout)).To(Equal(10 * time.Second))
102+
Expect(time.Duration(*g.TCPDialTimeout)).To(Equal(15 * time.Second))
103+
Expect(time.Duration(*g.TCPKeepAlive)).To(Equal(20 * time.Second))
104+
Expect(time.Duration(*g.TLSHandshakeTimeout)).To(Equal(5 * time.Second))
105+
Expect(time.Duration(*g.ResponseHeaderTimeout)).To(Equal(8 * time.Second))
106+
Expect(time.Duration(*g.IdleConnTimeout)).To(Equal(45 * time.Second))
107+
})
108+
109+
DescribeTable("rejects invalid duration values on unmarshal",
110+
func(jsonValue string, expectedErr string) {
111+
ctx := cpi.New()
112+
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":` + jsonValue + `}`)
113+
_, err := ctx.ConfigContext().GetConfigForData(raw, nil)
114+
Expect(err).To(HaveOccurred())
115+
Expect(err.Error()).To(ContainSubstring(expectedErr))
116+
},
117+
Entry("garbage string", `"notaduration"`, "expected a Go duration string"),
118+
Entry("number instead of string", `42`, "expected a Go duration string"),
119+
Entry("boolean instead of string", `true`, "expected a Go duration string"),
120+
Entry("empty string", `""`, "expected a Go duration string"),
121+
)
122+
123+
DescribeTable("rejects negative duration for timeout fields",
124+
func(field string, cfg *config.HTTPConfig) {
125+
ctx := cpi.New()
126+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
127+
ContainSubstring("invalid value for " + field),
128+
))
129+
},
130+
Entry("timeout -5m", "timeout",
131+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("-5m")}}),
132+
Entry("tcpDialTimeout -10s", "tcpDialTimeout",
133+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-10s")}}),
134+
Entry("tlsHandshakeTimeout -10h5m", "tlsHandshakeTimeout",
135+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-10h5m")}}),
136+
Entry("responseHeaderTimeout -1s", "responseHeaderTimeout",
137+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-1s")}}),
138+
Entry("idleConnTimeout -30s", "idleConnTimeout",
139+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-30s")}}),
140+
)
141+
142+
It("allows negative tcpKeepAlive to disable keep-alive probes", func() {
143+
ctx := cpi.New()
144+
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("-1s")}}
145+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
146+
g := MustGetHTTPSettings(ctx)
147+
Expect(time.Duration(*g.TCPKeepAlive)).To(Equal(-1 * time.Second))
148+
})
149+
150+
DescribeTable("accepts compound duration like 1h5s",
151+
func(cfg *config.HTTPConfig) {
152+
ctx := cpi.New()
153+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
154+
},
155+
Entry("timeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("1h5s")}}),
156+
Entry("tcpDialTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("1h5s")}}),
157+
Entry("tcpKeepAlive", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("1h5s")}}),
158+
Entry("tlsHandshakeTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("1h5s")}}),
159+
Entry("responseHeaderTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("1h5s")}}),
160+
Entry("idleConnTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("1h5s")}}),
161+
)
162+
163+
It("default settings are nil", func() {
164+
g := MustGetHTTPSettings(cpi.New())
165+
Expect(g.Timeout).To(BeNil())
166+
Expect(g.TCPDialTimeout).To(BeNil())
167+
Expect(g.TCPKeepAlive).To(BeNil())
168+
Expect(g.TLSHandshakeTimeout).To(BeNil())
169+
Expect(g.ResponseHeaderTimeout).To(BeNil())
170+
Expect(g.IdleConnTimeout).To(BeNil())
171+
})
172+
173+
It("nil timeout returns nil not zero", func() {
174+
g := MustGetHTTPSettings(cpi.New())
175+
Expect(g.Timeout).To(BeNil())
176+
})
177+
178+
It("round-trips Duration through MarshalJSON and UnmarshalJSON", func() {
179+
original := cpi.HTTPSettings{
180+
Timeout: dur("5m30s"),
181+
TCPDialTimeout: dur("15s"),
182+
}
183+
data, err := json.Marshal(original)
184+
Expect(err).To(Succeed())
185+
186+
var restored cpi.HTTPSettings
187+
Expect(json.Unmarshal(data, &restored)).To(Succeed())
188+
Expect(time.Duration(*restored.Timeout)).To(Equal(5*time.Minute + 30*time.Second))
189+
Expect(time.Duration(*restored.TCPDialTimeout)).To(Equal(15 * time.Second))
190+
Expect(restored.TCPKeepAlive).To(BeNil())
191+
})
192+
})
193+
})

‎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: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package docker
55
import (
66
"net/http"
77
"os"
8+
"time"
89

910
"github.com/docker/cli/cli/command"
1011
"github.com/docker/cli/cli/config"
@@ -13,11 +14,18 @@ import (
1314
dockerclient "github.com/moby/moby/client"
1415
"github.com/spf13/pflag"
1516

17+
"ocm.software/ocm/api/oci/cpi"
18+
"ocm.software/ocm/api/utils/httpclient"
1619
"ocm.software/ocm/api/utils/logging"
1720
)
1821

19-
func newDockerClient(dockerhost string, logger mlog.UnboundLogger) (*dockerclient.Client, error) {
22+
func newDockerClient(dockerhost string, logger mlog.UnboundLogger, httpCfg *cpi.HTTPSettings) (*dockerclient.Client, error) {
2023
if dockerhost == "" {
24+
// Use Docker CLI context resolution (DOCKER_CONTEXT env,
25+
// currentContext in ~/.docker/config.json, DOCKER_HOST env,
26+
// default socket). NewAPIClientFromFlags builds its own HTTP
27+
// transport internally, so OCM HTTP timeout settings do not
28+
// apply to this path.
2129
opts := cliflags.NewClientOptions()
2230
// set defaults
2331
opts.SetDefaultOptions(pflag.NewFlagSet("", pflag.ContinueOnError))
@@ -35,8 +43,15 @@ func newDockerClient(dockerhost string, logger mlog.UnboundLogger) (*dockerclien
3543
if err == nil && url.Scheme == "unix" {
3644
opts = append(opts, dockerclient.WithScheme(url.Scheme))
3745
}
38-
clnt := http.Client{}
39-
clnt.Transport = logging.NewRoundTripper(clnt.Transport, logger)
46+
47+
transport := httpclient.NewTransport(httpCfg)
48+
49+
clnt := http.Client{
50+
Transport: logging.NewRoundTripper(transport, logger),
51+
}
52+
if httpCfg.Timeout != nil {
53+
clnt.Timeout = time.Duration(*httpCfg.Timeout)
54+
}
4055
opts = append(opts, dockerclient.WithHTTPClient(&clnt))
4156
c, err := dockerclient.New(opts...)
4257
if err != nil {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@ 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+
client, err := newDockerClient(spec.DockerHost, logger, &httpSettings)
2731
if err != nil {
2832
return nil, err
2933
}

‎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
}

0 commit comments

Comments
 (0)