type Duration string -> type Duration time.Duration

Signed-off-by: Piotr Janik <piotr.janik@sap.com>

Author: piotrjanik

api/oci/config/httpconfig_test.go

@@ -1,6 +1,7 @@
 package config_test
 
 import (
+	"encoding/json"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -11,7 +12,11 @@ import (
 )
 
 func dur(s string) *cpi.Duration {
-	d := cpi.Duration(s)
+	td, err := time.ParseDuration(s)
+	if err != nil {
+		panic(err)
+	}
+	d := cpi.Duration(td)
 	return &d
 }
 
@@ -29,7 +34,7 @@ var _ = Describe("http config", func() {
 
 			Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
 			g := MustGetHTTPSettings(ctx)
-			Expect(g.Timeout.TimeDuration()).To(HaveValue(Equal(5 * time.Minute)))
+			Expect(time.Duration(*g.Timeout)).To(Equal(5 * time.Minute))
 		})
 
 		It("applies via config context", func() {
@@ -38,7 +43,7 @@ var _ = Describe("http config", func() {
 
 			Expect(ctx.ConfigContext().ApplyConfig(cfg, "programmatic")).To(Succeed())
 			g := MustGetHTTPSettings(ctx)
-			Expect(g.Timeout.TimeDuration()).To(HaveValue(Equal(30 * time.Second)))
+			Expect(time.Duration(*g.Timeout)).To(Equal(30 * time.Second))
 		})
 
 		It("parses all fields from JSON config", func() {
@@ -49,12 +54,12 @@ var _ = Describe("http config", func() {
 			Expect(ctx.ConfigContext().ApplyConfig(cfg, "config file")).To(Succeed())
 
 			g := MustGetHTTPSettings(ctx)
-			Expect(g.Timeout.TimeDuration()).To(HaveValue(Equal(10 * time.Second)))
-			Expect(g.TCPDialTimeout.TimeDuration()).To(HaveValue(Equal(15 * time.Second)))
-			Expect(g.TCPKeepAlive.TimeDuration()).To(HaveValue(Equal(20 * time.Second)))
-			Expect(g.TLSHandshakeTimeout.TimeDuration()).To(HaveValue(Equal(5 * time.Second)))
-			Expect(g.ResponseHeaderTimeout.TimeDuration()).To(HaveValue(Equal(8 * time.Second)))
-			Expect(g.IdleConnTimeout.TimeDuration()).To(HaveValue(Equal(45 * time.Second)))
+			Expect(time.Duration(*g.Timeout)).To(Equal(10 * time.Second))
+			Expect(time.Duration(*g.TCPDialTimeout)).To(Equal(15 * time.Second))
+			Expect(time.Duration(*g.TCPKeepAlive)).To(Equal(20 * time.Second))
+			Expect(time.Duration(*g.TLSHandshakeTimeout)).To(Equal(5 * time.Second))
+			Expect(time.Duration(*g.ResponseHeaderTimeout)).To(Equal(8 * time.Second))
+			Expect(time.Duration(*g.IdleConnTimeout)).To(Equal(45 * time.Second))
 		})
 
 		It("successive ApplyConfig overrides only non-nil fields", func() {
@@ -71,8 +76,8 @@ var _ = Describe("http config", func() {
 			Expect(second.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
 
 			g := MustGetHTTPSettings(ctx)
-			Expect(g.Timeout.TimeDuration()).To(HaveValue(Equal(1 * time.Minute)))
-			Expect(g.TCPDialTimeout.TimeDuration()).To(HaveValue(Equal(15 * time.Second)))
+			Expect(time.Duration(*g.Timeout)).To(Equal(1 * time.Minute))
+			Expect(time.Duration(*g.TCPDialTimeout)).To(Equal(15 * time.Second))
 		})
 
 		It("applies via generic config wrapper", func() {
@@ -93,21 +98,27 @@ configurations:
 			Expect(ctx.ConfigContext().ApplyConfig(cfg, "config file")).To(Succeed())
 
 			g := MustGetHTTPSettings(ctx)
-			Expect(g.Timeout.TimeDuration()).To(HaveValue(Equal(10 * time.Second)))
-			Expect(g.TCPDialTimeout.TimeDuration()).To(HaveValue(Equal(15 * time.Second)))
-			Expect(g.TCPKeepAlive.TimeDuration()).To(HaveValue(Equal(20 * time.Second)))
-			Expect(g.TLSHandshakeTimeout.TimeDuration()).To(HaveValue(Equal(5 * time.Second)))
-			Expect(g.ResponseHeaderTimeout.TimeDuration()).To(HaveValue(Equal(8 * time.Second)))
-			Expect(g.IdleConnTimeout.TimeDuration()).To(HaveValue(Equal(45 * time.Second)))
+			Expect(time.Duration(*g.Timeout)).To(Equal(10 * time.Second))
+			Expect(time.Duration(*g.TCPDialTimeout)).To(Equal(15 * time.Second))
+			Expect(time.Duration(*g.TCPKeepAlive)).To(Equal(20 * time.Second))
+			Expect(time.Duration(*g.TLSHandshakeTimeout)).To(Equal(5 * time.Second))
+			Expect(time.Duration(*g.ResponseHeaderTimeout)).To(Equal(8 * time.Second))
+			Expect(time.Duration(*g.IdleConnTimeout)).To(Equal(45 * time.Second))
 		})
 
-		It("rejects invalid duration string", func() {
-			ctx := cpi.New()
-			raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":"notaduration"}`)
-			_, err := ctx.ConfigContext().GetConfigForData(raw, nil)
-			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("invalid duration: notaduration"))
-		})
+		DescribeTable("rejects invalid duration values on unmarshal",
+			func(jsonValue string, expectedErr string) {
+				ctx := cpi.New()
+				raw := []byte(`{"type":"http.config.ocm.software/v1alpha1","timeout":` + jsonValue + `}`)
+				_, err := ctx.ConfigContext().GetConfigForData(raw, nil)
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(ContainSubstring(expectedErr))
+			},
+			Entry("garbage string", `"notaduration"`, "expected a Go duration string"),
+			Entry("number instead of string", `42`, "expected a Go duration string"),
+			Entry("boolean instead of string", `true`, "expected a Go duration string"),
+			Entry("empty string", `""`, "expected a Go duration string"),
+		)
 
 		DescribeTable("rejects negative duration for timeout fields",
 			func(field string, cfg *config.HTTPConfig) {
@@ -116,24 +127,24 @@ configurations:
 					ContainSubstring("invalid value for " + field),
 				))
 			},
-			Entry("timeout", "timeout",
+			Entry("timeout -5m", "timeout",
 				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("-5m")}}),
-			Entry("tcpDialTimeout", "tcpDialTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-5m")}}),
-			Entry("tlsHandshakeTimeout", "tlsHandshakeTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-5m")}}),
-			Entry("responseHeaderTimeout", "responseHeaderTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-5m")}}),
-			Entry("idleConnTimeout", "idleConnTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-5m")}}),
+			Entry("tcpDialTimeout -10s", "tcpDialTimeout",
+				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-10s")}}),
+			Entry("tlsHandshakeTimeout -10h5m", "tlsHandshakeTimeout",
+				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-10h5m")}}),
+			Entry("responseHeaderTimeout -1s", "responseHeaderTimeout",
+				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-1s")}}),
+			Entry("idleConnTimeout -30s", "idleConnTimeout",
+				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-30s")}}),
 		)
 
 		It("allows negative tcpKeepAlive to disable keep-alive probes", func() {
 			ctx := cpi.New()
 			cfg := &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPKeepAlive: dur("-1s")}}
 			Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(Succeed())
 			g := MustGetHTTPSettings(ctx)
-			Expect(g.TCPKeepAlive.TimeDuration()).To(HaveValue(Equal(-1 * time.Second)))
+			Expect(time.Duration(*g.TCPKeepAlive)).To(Equal(-1 * time.Second))
 		})
 
 		DescribeTable("accepts compound duration like 1h5s",
@@ -149,44 +160,6 @@ configurations:
 			Entry("idleConnTimeout", &config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("1h5s")}}),
 		)
 
-		DescribeTable("rejects negative compound duration -10h5m",
-			func(field string, cfg *config.HTTPConfig) {
-				ctx := cpi.New()
-				Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
-					ContainSubstring("invalid value for " + field),
-				))
-			},
-			Entry("timeout", "timeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("-10h5m")}}),
-			Entry("tcpDialTimeout", "tcpDialTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-10h5m")}}),
-			Entry("tlsHandshakeTimeout", "tlsHandshakeTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-10h5m")}}),
-			Entry("responseHeaderTimeout", "responseHeaderTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-10h5m")}}),
-			Entry("idleConnTimeout", "idleConnTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-10h5m")}}),
-		)
-
-		DescribeTable("rejects -10s",
-			func(field string, cfg *config.HTTPConfig) {
-				ctx := cpi.New()
-				Expect(cfg.ApplyTo(ctx.ConfigContext(), ctx)).To(MatchError(
-					ContainSubstring("invalid value for " + field),
-				))
-			},
-			Entry("timeout", "timeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{Timeout: dur("-10s")}}),
-			Entry("tcpDialTimeout", "tcpDialTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TCPDialTimeout: dur("-10s")}}),
-			Entry("tlsHandshakeTimeout", "tlsHandshakeTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{TLSHandshakeTimeout: dur("-10s")}}),
-			Entry("responseHeaderTimeout", "responseHeaderTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{ResponseHeaderTimeout: dur("-10s")}}),
-			Entry("idleConnTimeout", "idleConnTimeout",
-				&config.HTTPConfig{HTTPSettings: cpi.HTTPSettings{IdleConnTimeout: dur("-10s")}}),
-		)
-
 		It("default settings are nil", func() {
 			g := MustGetHTTPSettings(cpi.New())
 			Expect(g.Timeout).To(BeNil())
@@ -199,9 +172,22 @@ configurations:
 
 		It("nil timeout returns nil not zero", func() {
 			g := MustGetHTTPSettings(cpi.New())
-			timeout, err := g.Timeout.TimeDuration()
+			Expect(g.Timeout).To(BeNil())
+		})
+
+		It("round-trips Duration through MarshalJSON and UnmarshalJSON", func() {
+			original := cpi.HTTPSettings{
+				Timeout:        dur("5m30s"),
+				TCPDialTimeout: dur("15s"),
+			}
+			data, err := json.Marshal(original)
 			Expect(err).To(Succeed())
-			Expect(timeout).To(BeNil())
+
+			var restored cpi.HTTPSettings
+			Expect(json.Unmarshal(data, &restored)).To(Succeed())
+			Expect(time.Duration(*restored.Timeout)).To(Equal(5*time.Minute + 30*time.Second))
+			Expect(time.Duration(*restored.TCPDialTimeout)).To(Equal(15 * time.Second))
+			Expect(restored.TCPKeepAlive).To(BeNil())
 		})
 	})
 })

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

@@ -5,6 +5,7 @@ package docker
 import (
 	"net/http"
 	"os"
+	"time"
 
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/config"
@@ -43,20 +44,13 @@ func newDockerClient(dockerhost string, logger mlog.UnboundLogger, httpCfg *cpi.
 		opts = append(opts, dockerclient.WithScheme(url.Scheme))
 	}
 
-	transport, err := httpclient.NewTransport(httpCfg)
-	if err != nil {
-		return nil, err
-	}
-	timeout, err := httpCfg.Timeout.TimeDuration()
-	if err != nil {
-		return nil, err
-	}
+	transport := httpclient.NewTransport(httpCfg)
 
 	clnt := http.Client{
 		Transport: logging.NewRoundTripper(transport, logger),
 	}
-	if timeout != nil {
-		clnt.Timeout = *timeout
+	if httpCfg.Timeout != nil {
+		clnt.Timeout = time.Duration(*httpCfg.Timeout)
 	}
 	opts = append(opts, dockerclient.WithHTTPClient(&clnt))
 	c, err := dockerclient.New(opts...)

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

@@ -27,8 +27,7 @@ func NewRepository(ctx cpi.Context, spec *RepositorySpec) (cpi.Repository, error
 	if err != nil {
 		return nil, err
 	}
-	httpCfg := &httpSettings
-	client, err := newDockerClient(spec.DockerHost, logger, httpCfg)
+	client, err := newDockerClient(spec.DockerHost, logger, &httpSettings)
 	if err != nil {
 		return nil, err
 	}

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

@@ -194,19 +194,16 @@ func configureTransport(ctx cpi.Context, scheme string, creds credentials.Creden
 	if err != nil {
 		return nil, nil, err
 	}
-	httpCfg := &httpSettings
-	baseTransport, err := httpclient.NewTransport(httpCfg)
-	if err != nil {
-		return nil, nil, err
-	}
-	timeout, err := httpCfg.Timeout.TimeDuration()
-	if err != nil {
-		return nil, nil, err
+	baseTransport := httpclient.NewTransport(&httpSettings)
+	var timeout *time.Duration
+	if httpSettings.Timeout != nil {
+		timeout = new(time.Duration(*httpSettings.Timeout))
 	}
 
 	if scheme == "https" {
-		//nolint:gosec // there are OCI servers (quay.io) not working with min version.
+		//nolint:gosec // used like the default, there are OCI servers (quay.io) not working with min version.
 		baseTransport.TLSClientConfig = &tls.Config{
+			// MinVersion: tls.VersionTLS13,
 			RootCAs: func() *x509.CertPool {
 				rootCAs := rootcertsattr.Get(ctx).GetRootCertPool(true)
 				if creds != nil {

api/oci/internal/httpsettings.go

@@ -6,50 +6,29 @@ import (
 	"time"
 )
 
-// Duration is a string type representing a Go duration (e.g. "30s", "5m").
-// It is validated on JSON unmarshaling.
-type Duration string
+// Duration is a time.Duration that marshals to/from a human-readable
+// Go duration string (e.g. "30s", "5m") in JSON/YAML.
+type Duration time.Duration
 
 // UnmarshalJSON implements the json.Unmarshaller interface.
+// It parses a quoted Go duration string (e.g. "30s", "1h5m") into a Duration.
 func (d *Duration) UnmarshalJSON(b []byte) error {
 	var str string
 	if err := json.Unmarshal(b, &str); err != nil {
-		return err
+		return fmt.Errorf("invalid duration %s: expected a Go duration string (e.g. \"30s\", \"5m\", \"1h30m\")", string(b))
 	}
-	if _, err := time.ParseDuration(str); err != nil {
-		return fmt.Errorf("invalid duration: %s", str)
-	}
-	*d = Duration(str)
-	return nil
-}
-
-// TimeDuration parses the Duration string and returns a *time.Duration.
-// Returns (nil, nil) if d is nil or empty — callers must distinguish
-// nil (not configured) from zero (explicitly disabled).
-// Returns an error if the string is malformed.
-func (d *Duration) TimeDuration() (*time.Duration, error) {
-	if d == nil || *d == "" {
-		return nil, nil
-	}
-	pd, err := time.ParseDuration(string(*d))
+	pd, err := time.ParseDuration(str)
 	if err != nil {
-		return nil, fmt.Errorf("invalid duration %q: %w", string(*d), err)
+		return fmt.Errorf("invalid duration %q: expected a Go duration string (e.g. \"30s\", \"5m\", \"1h30m\")", str)
 	}
-	return &pd, nil
+	*d = Duration(pd)
+	return nil
 }
 
-func validateNonNegative(name string, d *Duration) error {
-	if d == nil {
-		return nil
-	}
-	td, err := d.TimeDuration()
-	if err != nil {
-		return err
-	}
-	if td != nil && *td < 0 {
-		return fmt.Errorf("invalid value for %s: %s, must be zero or positive", name, string(*d))
-	}
-	return nil
+// MarshalJSON implements the json.Marshaler interface.
+// It encodes the Duration as a quoted Go duration string (e.g. "30s", "1h5m").
+func (d Duration) MarshalJSON() ([]byte, error) {
+	return json.Marshal(time.Duration(d).String())
 }
 
 // HTTPSettings contains the timeout settings for HTTP clients.
@@ -99,8 +78,8 @@ func (s *HTTPSettings) Validate() error {
 		{"responseHeaderTimeout", s.ResponseHeaderTimeout},
 		{"idleConnTimeout", s.IdleConnTimeout},
 	} {
-		if err := validateNonNegative(check.name, check.val); err != nil {
-			return err
+		if check.val != nil && time.Duration(*check.val) < 0 {
+			return fmt.Errorf("invalid value for %s: %s, must be zero or positive", check.name, time.Duration(*check.val))
 		}
 	}
 	return nil