Configurable http timeout and http response header timeout for azurebs

azurebs/client/storage_client.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"net/http"
 	"os"
 	"strconv"
 	"strings"
@@ -107,6 +108,7 @@ type DefaultStorageClient struct {
 	credential    *azblob.SharedKeyCredential
 	serviceURL    string
 	storageConfig config.AZStorageConfig
+	clientOptions *azcore.ClientOptions
 }
 
 func NewStorageClient(storageConfig config.AZStorageConfig) (StorageClient, error) {
@@ -115,9 +117,64 @@ func NewStorageClient(storageConfig config.AZStorageConfig) (StorageClient, erro
 		return nil, err
 	}
 
+	clientOptions, err := buildClientOptions(storageConfig)
+	if err != nil {
+		return nil, err
+	}
+
 	serviceURL := fmt.Sprintf("https://%s.%s/%s", storageConfig.AccountName, storageConfig.StorageEndpoint(), storageConfig.ContainerName)
 
-	return DefaultStorageClient{credential: credential, serviceURL: serviceURL, storageConfig: storageConfig}, nil
+	return DefaultStorageClient{credential: credential, serviceURL: serviceURL, storageConfig: storageConfig, clientOptions: clientOptions}, nil
+}
+
+// buildClientOptions builds the shared azcore client options carrying a custom
+// *http.Client whenever an HTTP request timeout and/or response header timeout is
+// configured. It returns nil when neither is set, preserving the SDK defaults.
+func buildClientOptions(storageConfig config.AZStorageConfig) (*azcore.ClientOptions, error) {
+	httpRequestTimeout, err := storageConfig.HTTPRequestTimeoutValue()
+	if err != nil {
+		return nil, err
+	}
+
+	responseHeaderTimeout, err := storageConfig.HTTPResponseHeaderTimeoutValue()
+	if err != nil {
+		return nil, err
+	}
+
+	if httpRequestTimeout == 0 && responseHeaderTimeout == 0 {
+		return nil, nil
+	}
+
+	// preserve the default transport settings from the azure-sdk-for-go runtime package
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	if responseHeaderTimeout > 0 {
+		transport.ResponseHeaderTimeout = responseHeaderTimeout
+	}
+
+	httpClient := &http.Client{Timeout: httpRequestTimeout, Transport: transport}
+
+	return &azcore.ClientOptions{Transport: httpClient}, nil
+}
+
+func (dsc DefaultStorageClient) blockblobOptions() *blockblob.ClientOptions {
+	if dsc.clientOptions == nil {
+		return nil
+	}
+	return &blockblob.ClientOptions{ClientOptions: *dsc.clientOptions}
+}
+
+func (dsc DefaultStorageClient) blobOptions() *azBlob.ClientOptions {
+	if dsc.clientOptions == nil {
+		return nil
+	}
+	return &azBlob.ClientOptions{ClientOptions: *dsc.clientOptions}
+}
+
+func (dsc DefaultStorageClient) containerOptions() *azContainer.ClientOptions {
+	if dsc.clientOptions == nil {
+		return nil
+	}
+	return &azContainer.ClientOptions{ClientOptions: *dsc.clientOptions}
 }
 
 func (dsc DefaultStorageClient) Upload(
@@ -138,7 +195,7 @@ func (dsc DefaultStorageClient) Upload(
 	}
 	defer cancel()
 
-	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blockblobOptions())
 	if err != nil {
 		return nil, err
 	}
@@ -173,7 +230,7 @@ func (dsc DefaultStorageClient) UploadStream(
 	}
 	defer cancel()
 
-	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blockblobOptions())
 	if err != nil {
 		return err
 	}
@@ -196,7 +253,7 @@ func (dsc DefaultStorageClient) Download(
 ) error {
 	blobURL := fmt.Sprintf("%s/%s", dsc.serviceURL, source)
 	slog.Info("Downloading blob from container", "container", dsc.storageConfig.ContainerName, "blob", source, "local_file", dest.Name())
-	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blockblobOptions())
 	if err != nil {
 		return err
 	}
@@ -226,7 +283,7 @@ func (dsc DefaultStorageClient) Copy(
 	srcURL := fmt.Sprintf("%s/%s", dsc.serviceURL, srcBlob)
 	destURL := fmt.Sprintf("%s/%s", dsc.serviceURL, destBlob)
 
-	destClient, err := blockblob.NewClientWithSharedKeyCredential(destURL, dsc.credential, nil)
+	destClient, err := blockblob.NewClientWithSharedKeyCredential(destURL, dsc.credential, dsc.blockblobOptions())
 	if err != nil {
 		return fmt.Errorf("failed to create destination client: %w", err)
 	}
@@ -268,7 +325,7 @@ func (dsc DefaultStorageClient) Delete(
 	blobURL := fmt.Sprintf("%s/%s", dsc.serviceURL, dest)
 
 	slog.Info("Deleting blob from container", "container", dsc.storageConfig.ContainerName, "blob", dest, "url", blobURL)
-	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blockblobOptions())
 	if err != nil {
 		return err
 	}
@@ -295,7 +352,7 @@ func (dsc DefaultStorageClient) DeleteRecursive(
 		slog.Info("Deleting all blobs in container", "container", dsc.storageConfig.ContainerName)
 	}
 
-	containerClient, err := azContainer.NewClientWithSharedKeyCredential(dsc.serviceURL, dsc.credential, nil)
+	containerClient, err := azContainer.NewClientWithSharedKeyCredential(dsc.serviceURL, dsc.credential, dsc.containerOptions())
 	if err != nil {
 		return fmt.Errorf("failed to create container client: %w", err)
 	}
@@ -315,7 +372,7 @@ func (dsc DefaultStorageClient) DeleteRecursive(
 
 		for _, blob := range resp.Segment.BlobItems {
 			blobURL := fmt.Sprintf("%s/%s", dsc.serviceURL, *blob.Name)
-			blobClient, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+			blobClient, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blockblobOptions())
 			if err != nil {
 				slog.Error("Failed to create blob client", "blob", *blob.Name, "error", err)
 				continue
@@ -338,7 +395,7 @@ func (dsc DefaultStorageClient) Exists(
 	blobURL := fmt.Sprintf("%s/%s", dsc.serviceURL, dest)
 
 	slog.Info("Checking if blob exists", "container", dsc.storageConfig.ContainerName, "blob", dest, "url", blobURL)
-	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blockblobOptions())
 	if err != nil {
 		return false, err
 	}
@@ -365,7 +422,7 @@ func (dsc DefaultStorageClient) SignedUrl(
 	blobURL := fmt.Sprintf("%s/%s", dsc.serviceURL, dest)
 
 	slog.Info("Generating SAS URL for blob", "container", dsc.storageConfig.ContainerName, "blob", dest, "request_type", requestType, "expiration", expiration)
-	client, err := azBlob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+	client, err := azBlob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blobOptions())
 	if err != nil {
 		return "", err
 	}
@@ -398,7 +455,7 @@ func (dsc DefaultStorageClient) List(
 		slog.Info("Listing blobs in container", "container", dsc.storageConfig.ContainerName)
 	}
 
-	client, err := azContainer.NewClientWithSharedKeyCredential(dsc.serviceURL, dsc.credential, nil)
+	client, err := azContainer.NewClientWithSharedKeyCredential(dsc.serviceURL, dsc.credential, dsc.containerOptions())
 	if err != nil {
 		return nil, fmt.Errorf("failed to create container client: %w", err)
 	}
@@ -437,7 +494,7 @@ func (dsc DefaultStorageClient) Properties(
 	blobURL := fmt.Sprintf("%s/%s", dsc.serviceURL, dest)
 
 	slog.Info("Getting properties for blob", "container", dsc.storageConfig.ContainerName, "blob", dest, "url", blobURL)
-	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, nil)
+	client, err := blockblob.NewClientWithSharedKeyCredential(blobURL, dsc.credential, dsc.blockblobOptions())
 	if err != nil {
 		return err
 	}
@@ -469,7 +526,7 @@ func (dsc DefaultStorageClient) Properties(
 func (dsc DefaultStorageClient) EnsureContainerExists() error {
 	slog.Info("Ensuring container exists", "container", dsc.storageConfig.ContainerName)
 
-	containerClient, err := azContainer.NewClientWithSharedKeyCredential(dsc.serviceURL, dsc.credential, nil)
+	containerClient, err := azContainer.NewClientWithSharedKeyCredential(dsc.serviceURL, dsc.credential, dsc.containerOptions())
 	if err != nil {
 		return fmt.Errorf("failed to create container client: %w", err)
 	}

azurebs/client/storage_client_test.go

@@ -0,0 +1,51 @@
+package client_test
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/cloudfoundry/storage-cli/azurebs/client"
+	"github.com/cloudfoundry/storage-cli/azurebs/config"
+)
+
+var _ = Describe("NewStorageClient", func() {
+	baseConfig := func() config.AZStorageConfig {
+		return config.AZStorageConfig{
+			AccountName:   "account",
+			AccountKey:    "Zm9vYmFy", // base64("foobar")
+			ContainerName: "container",
+		}
+	}
+
+	It("succeeds without any http timeout configured", func() {
+		c, err := client.NewStorageClient(baseConfig())
+		Expect(err).ToNot(HaveOccurred())
+		Expect(c).ToNot(BeNil())
+	})
+
+	It("succeeds with http request and response header timeouts configured", func() {
+		cfg := baseConfig()
+		cfg.HTTPRequestTimeout = "30s"
+		cfg.HTTPResponseHeaderTimeout = "10s"
+
+		c, err := client.NewStorageClient(cfg)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(c).ToNot(BeNil())
+	})
+
+	It("returns an error for an invalid http_request_timeout", func() {
+		cfg := baseConfig()
+		cfg.HTTPRequestTimeout = "30" // missing unit
+
+		_, err := client.NewStorageClient(cfg)
+		Expect(err).To(MatchError(ContainSubstring("missing duration unit")))
+	})
+
+	It("returns an error for an invalid http_response_header_timeout", func() {
+		cfg := baseConfig()
+		cfg.HTTPResponseHeaderTimeout = "-5s"
+
+		_, err := client.NewStorageClient(cfg)
+		Expect(err).To(MatchError(ContainSubstring("must be greater than 0")))
+	})
+})

azurebs/config/config.go

@@ -3,11 +3,17 @@ package config
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
+	"strconv"
+	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud"
 )
 
+var errorNonPositiveHTTPRequestTimeout = errors.New("http_request_timeout must be greater than 0")
+var errorNonPositiveHTTPResponseHeaderTimeout = errors.New("http_response_header_timeout must be greater than 0")
+
 const storage cloud.ServiceName = "storage"
 
 var cloudConfig cloud.Configuration
@@ -27,11 +33,13 @@ func init() {
 }
 
 type AZStorageConfig struct {
-	AccountName   string `json:"account_name"`
-	AccountKey    string `json:"account_key"`
-	ContainerName string `json:"container_name"`
-	Environment   string `json:"environment"`
-	Timeout       string `json:"put_timeout_in_seconds"`
+	AccountName               string `json:"account_name"`
+	AccountKey                string `json:"account_key"`
+	ContainerName             string `json:"container_name"`
+	Environment               string `json:"environment"`
+	Timeout                   string `json:"put_timeout_in_seconds"`
+	HTTPRequestTimeout        string `json:"http_request_timeout"`
+	HTTPResponseHeaderTimeout string `json:"http_response_header_timeout"`
 }
 
 // NewFromReader returns a new azure-storage-cli configuration struct from the contents of reader.
@@ -53,6 +61,14 @@ func NewFromReader(reader io.Reader) (AZStorageConfig, error) {
 		return AZStorageConfig{}, err
 	}
 
+	if _, err := config.HTTPRequestTimeoutValue(); err != nil {
+		return AZStorageConfig{}, err
+	}
+
+	if _, err := config.HTTPResponseHeaderTimeoutValue(); err != nil {
+		return AZStorageConfig{}, err
+	}
+
 	return config, nil
 }
 
@@ -74,3 +90,35 @@ func (c *AZStorageConfig) configureCloud() error {
 	}
 	return nil
 }
+
+// parseOptionalPositiveDuration parses a Go duration string (e.g. "30s", "2m").
+// An empty value means "unset" and returns a zero duration with no error.
+// A bare number without a unit is rejected, as is a non-positive duration.
+func parseOptionalPositiveDuration(fieldName, value string, nonPositiveErr error) (time.Duration, error) {
+	if value == "" {
+		return 0, nil
+	}
+
+	if _, err := strconv.ParseFloat(value, 64); err == nil {
+		return 0, fmt.Errorf("invalid %s: missing duration unit", fieldName)
+	}
+
+	d, err := time.ParseDuration(value)
+	if err != nil {
+		return 0, fmt.Errorf("invalid %s: %w", fieldName, err)
+	}
+
+	if d <= 0 {
+		return 0, nonPositiveErr
+	}
+
+	return d, nil
+}
+
+func (c AZStorageConfig) HTTPRequestTimeoutValue() (time.Duration, error) {
+	return parseOptionalPositiveDuration("http_request_timeout", c.HTTPRequestTimeout, errorNonPositiveHTTPRequestTimeout)
+}
+
+func (c AZStorageConfig) HTTPResponseHeaderTimeoutValue() (time.Duration, error) {
+	return parseOptionalPositiveDuration("http_response_header_timeout", c.HTTPResponseHeaderTimeout, errorNonPositiveHTTPResponseHeaderTimeout)
+}