Skip to content

Commit 60e3711

Browse files
piotrjanikpiotrjanik
authored
feat: add registry client timeout attribute (#1823)
<!-- markdownlint-disable MD041 --> #### What this PR does / why we need it Adds a configurable timeout attribute preventing the CLI from terminating connections too early when interacting with slower OCI or Docker registries. - Introduces a new `http.config.ocm.software/v1alpha1` config type and `timeout` attribute for configuring client timeouts (default: 30s). - Adds global `--timeout` flag to the OCM CLI that applies the timeout via config context. **Integration test**: - Uses `toxiproxy` + `registry:2.8.3` testcontainers on a shared Docker network to simulate latency - Verifies transfer fails with a timeout shorter than proxy latency (2s timeout, 30s latency) - Verifies transfer succeeds with a timeout exceeding proxy latency (30s timeout, 1s latency) #### Which issue(s) this PR fixes <!-- Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`. --> Fixes: #1731 Signed-off-by: Piotr Janik <[email protected]>
1 parent ac1c9a6 commit 60e3711

File tree

19 files changed

+722
-6
lines changed

19 files changed

+722
-6
lines changed

‎api/datacontext/attrs/httptimeoutattr/attr.go‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package httptimeoutattr
2+
3+
import (
4+
"fmt"
5+
"time"
6+
7+
"ocm.software/ocm/api/datacontext"
8+
"ocm.software/ocm/api/utils/runtime"
9+
)
10+
11+
const (
12+
ATTR_KEY = "ocm.software/ocm/api/datacontext/attrs/httptimeout"
13+
ATTR_SHORT = "timeout"
14+
15+
// DefaultTimeout is the default HTTP client timeout used when no
16+
// configuration is provided.
17+
DefaultTimeout = 30 * time.Second
18+
)
19+
20+
func init() {
21+
datacontext.RegisterAttributeType(ATTR_KEY, AttributeType{}, ATTR_SHORT)
22+
}
23+
24+
// AttributeType implements the datacontext.AttributeType interface for
25+
// the httptimeout attribute.
26+
type AttributeType struct{}
27+
28+
func (a AttributeType) Name() string {
29+
return ATTR_KEY
30+
}
31+
32+
func (a AttributeType) Description() string {
33+
return `
34+
*string*
35+
Configures the timeout duration for HTTP client requests used to access
36+
OCI registries and other remote endpoints. The value is specified as a
37+
Go duration string (e.g. "30s", "5m", "1h").
38+
39+
If not set, the default timeout of 30s is used.
40+
`
41+
}
42+
43+
func (a AttributeType) Encode(v interface{}, marshaller runtime.Marshaler) ([]byte, error) {
44+
switch val := v.(type) {
45+
case time.Duration:
46+
return marshaller.Marshal(val.String())
47+
case string:
48+
if _, err := time.ParseDuration(val); err != nil {
49+
return nil, fmt.Errorf("invalid duration string for %s: %q", ATTR_SHORT, val)
50+
}
51+
return marshaller.Marshal(val)
52+
default:
53+
return nil, fmt.Errorf("duration or duration string required for %s, got %T", ATTR_SHORT, v)
54+
}
55+
}
56+
57+
func (a AttributeType) Decode(data []byte, unmarshaller runtime.Unmarshaler) (interface{}, error) {
58+
var v interface{}
59+
if err := unmarshaller.Unmarshal(data, &v); err != nil {
60+
return nil, fmt.Errorf("failed to decode %s: %w", ATTR_SHORT, err)
61+
}
62+
63+
switch value := v.(type) {
64+
case float64:
65+
return time.Duration(value), nil
66+
case string:
67+
d, err := time.ParseDuration(value)
68+
if err != nil {
69+
return nil, fmt.Errorf("invalid timeout value %q for %s: must be a duration like 30s, 5m, or nanoseconds number: %w", value, ATTR_SHORT, err)
70+
}
71+
return d, nil
72+
default:
73+
return nil, fmt.Errorf("timeout for %s must be a duration string or nanoseconds number, got %T", ATTR_SHORT, v)
74+
}
75+
}
76+
77+
////////////////////////////////////////////////////////////////////////////////
78+
79+
// Get returns the configured HTTP client timeout from the context.
80+
// If not set, DefaultTimeout is returned.
81+
func Get(ctx datacontext.Context) time.Duration {
82+
a := ctx.GetAttributes().GetAttribute(ATTR_KEY)
83+
if a == nil {
84+
return DefaultTimeout
85+
}
86+
return a.(time.Duration)
87+
}
88+
89+
// Set stores the HTTP client timeout attribute in the context.
90+
func Set(ctx datacontext.Context, d time.Duration) error {
91+
return ctx.GetAttributes().SetAttribute(ATTR_KEY, d)
92+
}

‎api/datacontext/attrs/httptimeoutattr/attr_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package httptimeoutattr_test
2+
3+
import (
4+
"time"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
9+
"ocm.software/ocm/api/datacontext"
10+
"ocm.software/ocm/api/datacontext/attrs/httptimeoutattr"
11+
"ocm.software/ocm/api/utils/runtime"
12+
)
13+
14+
var _ = Describe("httptimeout attribute", func() {
15+
var ctx datacontext.Context
16+
attr := httptimeoutattr.AttributeType{}
17+
enc := runtime.DefaultJSONEncoding
18+
19+
BeforeEach(func() {
20+
ctx = datacontext.New(nil)
21+
})
22+
23+
Context("get and set", func() {
24+
It("defaults to DefaultTimeout", func() {
25+
Expect(httptimeoutattr.Get(ctx)).To(Equal(httptimeoutattr.DefaultTimeout))
26+
})
27+
28+
It("sets and retrieves duration", func() {
29+
Expect(httptimeoutattr.Set(ctx, 5*time.Minute)).To(Succeed())
30+
Expect(httptimeoutattr.Get(ctx)).To(Equal(5 * time.Minute))
31+
32+
Expect(httptimeoutattr.Set(ctx, 2*time.Minute)).To(Succeed())
33+
Expect(httptimeoutattr.Get(ctx)).To(Equal(2 * time.Minute))
34+
})
35+
})
36+
37+
Context("encoding values to JSON", func() {
38+
DescribeTable("encodes valid input",
39+
func(input interface{}, expected string) {
40+
data, err := attr.Encode(input, enc)
41+
Expect(err).To(Succeed())
42+
Expect(string(data)).To(Equal(expected))
43+
},
44+
Entry("time.Duration 30s to JSON string", 30*time.Second, `"30s"`),
45+
Entry("duration string 5m to JSON string", "5m", `"5m"`),
46+
)
47+
48+
DescribeTable("rejects invalid input",
49+
func(input interface{}, errSubstring string) {
50+
_, err := attr.Encode(input, enc)
51+
Expect(err).To(HaveOccurred())
52+
Expect(err.Error()).To(ContainSubstring(errSubstring))
53+
},
54+
Entry("non-parseable string like notaduration", "notaduration", "invalid duration string"),
55+
Entry("string with unknown unit like 1Gb", "1Gb", "invalid duration string"),
56+
Entry("unsupported type like bool", true, "duration or duration string required"),
57+
)
58+
})
59+
60+
Context("decoding values from JSON", func() {
61+
DescribeTable("decodes valid JSON input",
62+
func(input string, expected time.Duration) {
63+
val, err := attr.Decode([]byte(input), enc)
64+
Expect(err).To(Succeed())
65+
Expect(val).To(Equal(expected))
66+
},
67+
Entry("duration string 30s", `"30s"`, 30*time.Second),
68+
Entry("duration string 5m", `"5m"`, 5*time.Minute),
69+
Entry("duration string 1h", `"1h"`, 1*time.Hour),
70+
Entry("nanoseconds number 300000000000 as 5m", `300000000000`, 5*time.Minute),
71+
)
72+
73+
DescribeTable("rejects invalid JSON input",
74+
func(input string, errSubstring string) {
75+
_, err := attr.Decode([]byte(input), enc)
76+
Expect(err).To(HaveOccurred())
77+
Expect(err.Error()).To(ContainSubstring(errSubstring))
78+
},
79+
Entry("non-parseable string like notaduration", `"notaduration"`, "invalid timeout value"),
80+
Entry("string with unknown unit like 1Gb", `"1Gb"`, "invalid timeout value"),
81+
Entry("digit-only string like 300000000000", `"300000000000"`, "invalid timeout value"),
82+
Entry("JSON boolean true", `true`, "must be a duration string or nanoseconds number"),
83+
)
84+
})
85+
})

‎api/datacontext/attrs/httptimeoutattr/config.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package httptimeoutattr
2+
3+
import (
4+
"encoding/json"
5+
"fmt"
6+
"time"
7+
8+
"github.com/mandelsoft/goutils/errors"
9+
10+
cfgcpi "ocm.software/ocm/api/config/cpi"
11+
"ocm.software/ocm/api/utils/runtime"
12+
)
13+
14+
const (
15+
ConfigType = "http" + cfgcpi.OCM_CONFIG_TYPE_SUFFIX
16+
ConfigTypeV1Alpha1 = ConfigType + runtime.VersionSeparator + "v1alpha1"
17+
)
18+
19+
func init() {
20+
cfgcpi.RegisterConfigType(cfgcpi.NewConfigType[*Config](ConfigType, configUsage))
21+
cfgcpi.RegisterConfigType(cfgcpi.NewConfigType[*Config](ConfigTypeV1Alpha1, configUsage))
22+
}
23+
24+
// Timeout wraps time.Duration to support JSON/YAML marshaling
25+
// of both human-readable duration strings (e.g. "30s", "5m", "1h")
26+
// and nanosecond numbers.
27+
type Timeout time.Duration
28+
29+
func (d Timeout) MarshalJSON() ([]byte, error) {
30+
return json.Marshal(time.Duration(d).String())
31+
}
32+
33+
func (d *Timeout) UnmarshalJSON(b []byte) error {
34+
var v interface{}
35+
if err := json.Unmarshal(b, &v); err != nil {
36+
return fmt.Errorf("failed to parse HTTP client timeout: %w", err)
37+
}
38+
39+
switch value := v.(type) {
40+
case float64:
41+
*d = Timeout(time.Duration(value))
42+
return nil
43+
case string:
44+
tmp, err := time.ParseDuration(value)
45+
if err != nil {
46+
return fmt.Errorf("invalid timeout value %q: must be a duration like 30s, 5m, or nanoseconds number: %w", value, err)
47+
}
48+
*d = Timeout(tmp)
49+
return nil
50+
default:
51+
return fmt.Errorf("timeout must be a duration string or nanoseconds number, got %T", v)
52+
}
53+
}
54+
55+
// Config describes the configuration for HTTP client settings.
56+
type Config struct {
57+
runtime.ObjectVersionedType `json:",inline"`
58+
Timeout Timeout `json:"timeout,omitempty"`
59+
}
60+
61+
// NewConfig creates a new HTTP config with the given timeout.
62+
func NewConfig(timeout time.Duration) *Config {
63+
return &Config{
64+
ObjectVersionedType: runtime.NewVersionedTypedObject(ConfigType),
65+
Timeout: Timeout(timeout),
66+
}
67+
}
68+
69+
func (a *Config) GetType() string {
70+
return ConfigType
71+
}
72+
73+
func (a *Config) ApplyTo(ctx cfgcpi.Context, target interface{}) error {
74+
t, ok := target.(cfgcpi.Context)
75+
if !ok {
76+
return cfgcpi.ErrNoContext(ConfigType)
77+
}
78+
if a.Timeout != 0 {
79+
return errors.Wrapf(t.GetAttributes().SetAttribute(ATTR_KEY, time.Duration(a.Timeout)), "applying config failed")
80+
}
81+
return nil
82+
}
83+
84+
const configUsage = `
85+
The config type <code>` + ConfigType + `</code> can be used to configure
86+
HTTP client settings:
87+
88+
<pre>
89+
type: ` + ConfigType + `
90+
timeout: 30s
91+
</pre>
92+
93+
The <code>timeout</code> field specifies the HTTP client timeout as a
94+
Go duration string (e.g. "30s", "5m", "1h"). If not set, the default
95+
timeout of 30s is used.
96+
`

‎api/datacontext/attrs/httptimeoutattr/config_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
package httptimeoutattr_test
2+
3+
import (
4+
"time"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
9+
"ocm.software/ocm/api/config"
10+
"ocm.software/ocm/api/credentials"
11+
"ocm.software/ocm/api/datacontext"
12+
"ocm.software/ocm/api/datacontext/attrs/httptimeoutattr"
13+
)
14+
15+
var _ = Describe("http config type", func() {
16+
var ctx config.Context
17+
18+
BeforeEach(func() {
19+
ctx = config.WithSharedAttributes(datacontext.New(nil)).New()
20+
})
21+
22+
It("applies timeout via config", func() {
23+
cfg := httptimeoutattr.NewConfig(5 * time.Minute)
24+
Expect(ctx.ApplyConfig(cfg, "test")).To(Succeed())
25+
26+
ocmCtx := credentials.WithConfigs(ctx).New()
27+
Expect(httptimeoutattr.Get(ocmCtx)).To(Equal(5 * time.Minute))
28+
})
29+
30+
It("applies timeout to existing context", func() {
31+
ocmCtx := credentials.WithConfigs(ctx).New()
32+
cfg := httptimeoutattr.NewConfig(2 * time.Minute)
33+
Expect(ctx.ApplyConfig(cfg, "test")).To(Succeed())
34+
35+
Expect(httptimeoutattr.Get(ocmCtx)).To(Equal(2 * time.Minute))
36+
})
37+
38+
It("skips zero timeout", func() {
39+
cfg := httptimeoutattr.NewConfig(0)
40+
Expect(ctx.ApplyConfig(cfg, "test")).To(Succeed())
41+
42+
ocmCtx := credentials.WithConfigs(ctx).New()
43+
Expect(httptimeoutattr.Get(ocmCtx)).To(Equal(httptimeoutattr.DefaultTimeout))
44+
})
45+
46+
It("applies timeout from config file with timeout field", func() {
47+
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":"10s"}`)
48+
cfg, err := ctx.GetConfigForData(raw, nil)
49+
Expect(err).To(Succeed())
50+
Expect(ctx.ApplyConfig(cfg, "config file")).To(Succeed())
51+
52+
ocmCtx := credentials.WithConfigs(ctx).New()
53+
Expect(httptimeoutattr.Get(ocmCtx)).To(Equal(10 * time.Second))
54+
})
55+
56+
It("flag overrides config file timeout", func() {
57+
// Config file sets timeout to 10s.
58+
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":"10s"}`)
59+
cfg, err := ctx.GetConfigForData(raw, nil)
60+
Expect(err).To(Succeed())
61+
Expect(ctx.ApplyConfig(cfg, "config file")).To(Succeed())
62+
63+
// CLI flag sets timeout to 2m (applied after config file, same as in Evaluate).
64+
flagCfg := httptimeoutattr.NewConfig(2 * time.Minute)
65+
Expect(ctx.ApplyConfig(flagCfg, "cli timeout flag")).To(Succeed())
66+
67+
ocmCtx := credentials.WithConfigs(ctx).New()
68+
Expect(httptimeoutattr.Get(ocmCtx)).To(Equal(2 * time.Minute))
69+
})
70+
71+
It("keeps default timeout when config file omits timeout field", func() {
72+
raw := []byte(`{"type":"http.config.ocm.software/v1alpha1"}`)
73+
cfg, err := ctx.GetConfigForData(raw, nil)
74+
Expect(err).To(Succeed())
75+
Expect(ctx.ApplyConfig(cfg, "config file")).To(Succeed())
76+
77+
ocmCtx := credentials.WithConfigs(ctx).New()
78+
Expect(httptimeoutattr.Get(ocmCtx)).To(Equal(httptimeoutattr.DefaultTimeout))
79+
})
80+
})

‎api/datacontext/attrs/httptimeoutattr/suite_test.go‎

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

‎api/datacontext/attrs/init.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package attrs
22

33
import (
4+
_ "ocm.software/ocm/api/datacontext/attrs/httptimeoutattr"
45
_ "ocm.software/ocm/api/datacontext/attrs/logforward"
56
_ "ocm.software/ocm/api/datacontext/attrs/rootcertsattr"
67
_ "ocm.software/ocm/api/datacontext/attrs/tmpcache"

0 commit comments

Comments
 (0)