Skip to content

Commit 26f3bf2

Browse files
piotrjanikpiotrjanik
committed
validation
Signed-off-by: Piotr Janik <piotr.janik@sap.com>
1 parent 3aec505 commit 26f3bf2

3 files changed

Lines changed: 71 additions & 40 deletions

File tree

‎api/oci/config/httpconfig_test.go‎

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,21 +128,64 @@ configurations:
128128
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-5m")}}),
129129
)
130130

131-
It("allows -1 tcpKeepAlive to disable keep-alive probes", func() {
131+
It("allows negative tcpKeepAlive to disable keep-alive probes", func() {
132132
ctx := cpi.New()
133133
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("-1s")}}
134134
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
135135
g := MustGetHTTPSettings(ctx)
136136
Expect(g.TCPKeepAlive.TimeDuration()).To(HaveValue(Equal(-1 * time.Second)))
137137
})
138138

139-
It("rejects tcpKeepAlive values more negative than -1", func() {
140-
ctx := cpi.New()
141-
cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("-2s")}}
142-
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
143-
ContainSubstring("invalid value for tcpKeepAlive"),
144-
))
145-
})
139+
DescribeTable("accepts compound duration like 1h5s",
140+
func(cfg *config.HTTPConfig) {
141+
ctx := cpi.New()
142+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
143+
},
144+
Entry("timeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("1h5s")}}),
145+
Entry("tcpDialTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("1h5s")}}),
146+
Entry("tcpKeepAlive", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("1h5s")}}),
147+
Entry("tlsHandshakeTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("1h5s")}}),
148+
Entry("responseHeaderTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("1h5s")}}),
149+
Entry("idleConnTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("1h5s")}}),
150+
)
151+
152+
DescribeTable("rejects negative compound duration -10h5m",
153+
func(field string, cfg *config.HTTPConfig) {
154+
ctx := cpi.New()
155+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
156+
ContainSubstring("invalid value for " + field),
157+
))
158+
},
159+
Entry("timeout", "timeout",
160+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("-10h5m")}}),
161+
Entry("tcpDialTimeout", "tcpDialTimeout",
162+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-10h5m")}}),
163+
Entry("tlsHandshakeTimeout", "tlsHandshakeTimeout",
164+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-10h5m")}}),
165+
Entry("responseHeaderTimeout", "responseHeaderTimeout",
166+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-10h5m")}}),
167+
Entry("idleConnTimeout", "idleConnTimeout",
168+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-10h5m")}}),
169+
)
170+
171+
DescribeTable("rejects -10s",
172+
func(field string, cfg *config.HTTPConfig) {
173+
ctx := cpi.New()
174+
Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
175+
ContainSubstring("invalid value for " + field),
176+
))
177+
},
178+
Entry("timeout", "timeout",
179+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("-10s")}}),
180+
Entry("tcpDialTimeout", "tcpDialTimeout",
181+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-10s")}}),
182+
Entry("tlsHandshakeTimeout", "tlsHandshakeTimeout",
183+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-10s")}}),
184+
Entry("responseHeaderTimeout", "responseHeaderTimeout",
185+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-10s")}}),
186+
Entry("idleConnTimeout", "idleConnTimeout",
187+
&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-10s")}}),
188+
)
146189

147190
It("default settings are nil", func() {
148191
g := MustGetHTTPSettings(cpi.New())

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ var _ = Describe("Registry timeout:", Ordered, func() {
6666
Expect(err).To(Succeed(), "failed to start toxiproxy container")
6767

6868
host, port, err := toxiContainer.ProxiedEndpoint(8666)
69-
Expect(err).To(Succeed())
69+
Expect(err).ToNot(HaveOccurred())
7070
proxyHost = fmt.Sprintf("%s:%s", host, port)
7171

7272
uri, err := toxiContainer.URI(ctx)
73-
Expect(err).To(Succeed())
73+
Expect(err).ToNot(HaveOccurred())
7474
toxiClient := toxiproxy.NewClient(uri)
7575
proxy, err = toxiClient.Proxy(proxyName)
7676
Expect(err).To(Succeed(), "failed to get toxiproxy proxy")
@@ -218,7 +218,7 @@ func addLatency(proxy *toxiproxy.Proxy, latencyMs int, stream string) {
218218
_, err := proxy.AddToxic("latency", "latency", stream, 1.0, toxiproxy.Attributes{
219219
"latency": latencyMs,
220220
})
221-
Expect(err).To(Succeed())
221+
Expect(err).ToNot(HaveOccurred())
222222
}
223223

224224
func removeToxic(proxy *toxiproxy.Proxy, name string) {

‎api/oci/internal/httpsettings.go‎

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package internal
33
import (
44
"encoding/json"
55
"fmt"
6-
"strings"
76
"time"
87
)
98

@@ -39,26 +38,16 @@ func (d *Duration) TimeDuration() (*time.Duration, error) {
3938
return &pd, nil
4039
}
4140

42-
// nonNegative requires the duration to be zero or positive.
43-
func nonNegative(d Duration) bool {
44-
return !strings.HasPrefix(string(d), "-")
45-
}
46-
47-
// nonNegativeOrMinusOne requires the duration to be zero, positive,
48-
// or -1 (to disable keep-alive probes).
49-
func nonNegativeOrMinusOne(d Duration) bool {
50-
return !strings.HasPrefix(string(d), "-") || strings.HasPrefix(string(d), "-1")
51-
}
52-
53-
func validateDuration(name string, d *Duration, valid func(Duration) bool) error {
41+
func validateNonNegative(name string, d *Duration) error {
5442
if d == nil {
5543
return nil
5644
}
57-
if _, err := d.TimeDuration(); err != nil {
45+
td, err := d.TimeDuration()
46+
if err != nil {
5847
return err
5948
}
60-
if !valid(*d) {
61-
return fmt.Errorf("invalid value for %s: %s", name, string(*d))
49+
if td != nil && *td < 0 {
50+
return fmt.Errorf("invalid value for %s: %s, must be zero or positive", name, string(*d))
6251
}
6352
return nil
6453
}
@@ -82,7 +71,8 @@ type HTTPSettings struct {
8271
TCPDialTimeout *Duration `json:"tcpDialTimeout,omitempty"`
8372

8473
// TCPKeepAlive is the interval between TCP keep-alive probes.
85-
// Use -1 to disable keep-alive probes.
74+
// If zero, probes are sent with a default value (currently 15 seconds).
75+
// If negative, keep-alive probes are disabled.
8676
TCPKeepAlive *Duration `json:"tcpKeepAlive,omitempty"`
8777

8878
// TLSHandshakeTimeout is the maximum time to wait for a TLS handshake.
@@ -96,22 +86,20 @@ type HTTPSettings struct {
9686
}
9787

9888
// Validate checks that timeout values are non-negative.
99-
// TCPKeepAlive additionally allows -1 to disable keep-alive probes
100-
// (consistent with Go's net.Dialer.KeepAlive).
89+
// TCPKeepAlive is not validated because any negative value
90+
// disables keep-alive probes (consistent with Go's net.Dialer.KeepAlive).
10191
func (s *HTTPSettings) Validate() error {
10292
for _, check := range []struct {
103-
name string
104-
val *Duration
105-
valid func(Duration) bool
93+
name string
94+
val *Duration
10695
}{
107-
{"timeout", s.Timeout, nonNegative},
108-
{"tcpDialTimeout", s.TCPDialTimeout, nonNegative},
109-
{"tcpKeepAlive", s.TCPKeepAlive, nonNegativeOrMinusOne},
110-
{"tlsHandshakeTimeout", s.TLSHandshakeTimeout, nonNegative},
111-
{"responseHeaderTimeout", s.ResponseHeaderTimeout, nonNegative},
112-
{"idleConnTimeout", s.IdleConnTimeout, nonNegative},
96+
{"timeout", s.Timeout},
97+
{"tcpDialTimeout", s.TCPDialTimeout},
98+
{"tlsHandshakeTimeout", s.TLSHandshakeTimeout},
99+
{"responseHeaderTimeout", s.ResponseHeaderTimeout},
100+
{"idleConnTimeout", s.IdleConnTimeout},
113101
} {
114-
if err := validateDuration(check.name, check.val, check.valid); err != nil {
102+
if err := validateNonNegative(check.name, check.val); err != nil {
115103
return err
116104
}
117105
}

0 commit comments

Comments
 (0)