Refactor DAV client, add missing operations by kathap · Pull Request #70 · cloudfoundry/storage-cli

kathap · 2026-03-13T15:10:32Z

Added Missing Operations:

COPY - Server-side blob copying via WebDAV COPY method
PROPERTIES - Retrieve blob metadata (ContentLength, ETag, LastModified)
ENSURE-STORAGE-EXISTS - Initialize WebDAV directory structure
SIGN - Generate pre-signed URLs with HMAC-SHA256
DELETE-RECURSIVE - Delete all blobs matching a prefix

Structural Changes:

Split into two-layer architecture like other providers (S3, Azure, etc.)
- client.go: High-level DavBlobstore implementing storage.Storager interface
- storage_client.go: Low-level StorageClient handling HTTP/WebDAV operations

**Added Missing Operations:** - COPY - Server-side blob copying via WebDAV COPY method - PROPERTIES - Retrieve blob metadata (ContentLength, ETag, LastModified) - ENSURE-STORAGE-EXISTS - Initialize WebDAV directory structure - SIGN - Generate pre-signed URLs with HMAC-SHA256 - DELETE-RECURSIVE - Delete all blobs matching a prefix **Structural Changes:** - Split into two-layer architecture like other providers (S3, Azure, etc.) - client.go: High-level DavBlobstore implementing storage.Storager interface - storage_client.go: Low-level StorageClient handling HTTP/WebDAV operations

dav/client/client.go

dav/client/storage_client.go

dav/TESTING.md

.github/workflows/dav-integration.yml

dav/client/storage_client.go

…ect names, not basenames.Change low-level Exists to (bool, error), matching Azure/GCS/S3.

dav/client/storage_client.go

Extend WebDAV signer to support both BOSH-compatible SHA256 HMAC signing and CAPI-compatible MD5 signing via optional signing_method config field. - Add SigningMethod field to config (default: "sha256") - Implement generateSHA256SignedURL() for BOSH (/signed/ paths) - Implement generateMD5SignedURL() for CAPI (/read/, /write/ paths) - Add HEAD verb support - Improve WebDAV compatibility with PROPFIND - Simplify blob path handling for CCNG pre-partitioned IDs - Add comprehensive tests for both signing methods

…ling

1. Simple blob IDs (like my-blob) get stored with hash-prefix directory structure (8c/my-blob) 2. Pre-partitioned blob IDs (like ru/by/ruby-buildpack from CCNG) are stored as-is 3. Both PUT and COPY operations correctly create parent directories by using the built path (with hash prefix) when calling ensureObjectParentsExist

Preserve endpoint base paths when constructing signed URLs to ensure directory keys (cc-droplets, cc-packages, etc.) are included in the final URL path. The signer now combines: - Path prefix (/signed/, /read/, or /write/) - Endpoint base path (directory key from config) - Blob ID with hash prefix This ensures storage-cli and fog/webdav clients store blobs at identical physical locations for backward compatibility. Also fixed DAV integration tests to validate signed URL generation without requiring nginx infrastructure in the test environment.

dav/README.md

storage/factory.go

stephanme · 2026-03-30T12:07:58Z

dav/client/storage_client.go

+
+	// Build the full blob path (with CCNG partitioning if using signed URLs)
+	useSignedURLs := c.config.Secret != ""
+	blobPath := buildBlobPath(path, useSignedURLs)


Not sure that storage-cli should invent a CC specific blob path. I would expect the CC to provide an already correct path, i.e. call storage-cli -s dav -c dav-config.json put blob.file /12/34/1234-...
The other storage types don't manipulate the blob path either.

Good point — I removed the CC-specific blob path rewriting from the DAV client, it no longer applies any partitioning or prefixing on its own.

Make DAV client consistent with S3/Azure/GCS/AliOSS by removing path transformation logic. Storage-cli now uses blob paths exactly as provided by the caller. Cloud Controller handles partitioning via BlobKeyGenerator before calling storage-cli. - Remove buildBlobPath() and path partitioning - Simplify List() to generic WebDAV traversal - Remove "webdav" → "dav" alias (CC handles this) - Fix Sign() and Copy() consistency issues - Update README to reflect caller-defined paths

stephanme · 2026-04-13T06:24:04Z

dav/signer/signer_test.go

+		// timestamp: 1566817860 (2019-08-26 11:11:00 UTC)
+		// expires: 1566818760 (timestamp + 900 seconds = 15 minutes later)
+		// Signature matches BOSH secure_link_hmac format: $request_method$object_id$arg_ts$arg_e
+		expected := "https://api.example.com/signed/fake-object-id?e=1566818760&st=YUBIL21YRsFY_w-NrYiAPUnIhlenFuLEa6WsQUhpGLI&ts=1566817860"


Why does the expected signature link change? SHA256 signatures (Bosh format) should not change.

stephanme · 2026-04-13T06:27:59Z

dav/signer/signer.go

-	expiresAfterSeconds := int(expiresAfter.Seconds())
-	signature := s.generateSignature(prefixedBlobID, verb, timeStamp, expiresAfterSeconds)
+	timestamp := timeStamp.Unix()
+	expires := timestamp + int64(expiresAfter.Seconds())


If I got it right: old implementation uses expiresAfterSeconds for the signature, the new implementation an absolute timestamp. This might be more correct for an expiration, but is it compatible with Bosh?

Oh, you're right - I accidentally changed the BOSH signature format. Will revert it!

dav/client/storage_client.go

stephanme · 2026-04-13T06:38:12Z

dav/client/storage_client.go

+
+	if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
+		err := fmt.Errorf("invalid status: %d", resp.StatusCode)
+		return fmt.Errorf("deleting blob %q: %w", path, err)


Could be merged into one fmt.Errorf().

stephanme · 2026-04-13T06:52:33Z

dav/client/storage_client.go

+}
+
+// newPropfindBody creates a properly formatted PROPFIND request body
+func newPropfindBody() (*strings.Reader, error) {


Looks like this function returns a static string (no parameters).
newPropfindBody() is called e.g. in listRecursive() -> there is no need to create the xml again and again.

True. I changed it to generate the PROPFIND XML once at initialization time and now just create a new reader from the static string for each request.

dav/client/storage_client.go

stephanme · 2026-04-13T07:07:35Z

dav/client/client_test.go

-			err := client.Exists("/somefile")
-			Expect(err).NotTo(HaveOccurred())
+			// We can't easily test this without refactoring to inject storageClient
+			// This is a structural example


Is this test case really testing anything? The FakeStorageClient is not used by DavBlobstore.

You're right — the previous test was not actually using the fake storage client through DavBlobstore.

I changed the tests to inject the fake via NewWithStorageClient, so they now verify that DavBlobstore correctly delegates to the underlying StorageClient.

…ction

kathap marked this pull request as draft March 13, 2026 15:10

cf-foundation-community-automation bot added this to Foundational Infrastructure Working Group Mar 13, 2026

cf-foundation-community-automation bot moved this to Inbox in Foundational Infrastructure Working Group Mar 13, 2026

add integration tests

3c726fa

kathap force-pushed the add-missing-operations-for-webdav branch 2 times, most recently from 19fa8b5 to a69e251 Compare March 13, 2026 15:54

exclude integration tests run

93b80d1

kathap force-pushed the add-missing-operations-for-webdav branch from a69e251 to 93b80d1 Compare March 13, 2026 15:57

serdarozerr suggested changes Mar 13, 2026

View reviewed changes

dav/client/client.go Outdated Show resolved Hide resolved

dav/client/client.go Outdated Show resolved Hide resolved

dav/client/storage_client.go Outdated Show resolved Hide resolved

dav/TESTING.md Outdated Show resolved Hide resolved

github-project-automation bot moved this from Inbox to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group Mar 13, 2026

kathap force-pushed the add-missing-operations-for-webdav branch 3 times, most recently from e6899c3 to 230cd5a Compare March 13, 2026 16:14

run integration tests separately

cc66878

kathap force-pushed the add-missing-operations-for-webdav branch from 230cd5a to cc66878 Compare March 13, 2026 16:17

serdarozerr reviewed Mar 16, 2026

View reviewed changes

.github/workflows/dav-integration.yml Show resolved Hide resolved

Adapt error handling, debug level and dav integration test setup

9a45943

kathap force-pushed the add-missing-operations-for-webdav branch from 4e7c568 to 9a45943 Compare March 16, 2026 15:13

delete obsolete files

2580f2c

serdarozerr reviewed Mar 17, 2026

View reviewed changes

.github/workflows/dav-integration.yml Outdated Show resolved Hide resolved

serdarozerr suggested changes Mar 17, 2026

View reviewed changes

dav/client/storage_client.go Outdated Show resolved Hide resolved

cleaner string empty check

0fae218

serdarozerr reviewed Mar 17, 2026

View reviewed changes

dav/client/storage_client.go Outdated Show resolved Hide resolved

kathap added 6 commits March 17, 2026 14:04

critical List and Exists fixes, Fix List to return full canonical obj…

ade8b48

…ect names, not basenames.Change low-level Exists to (bool, error), matching Azure/GCS/S3.

Centralize path building and validate signing actions

da47086

use native webdav COPY method

66f87d5

improvements

dffc75e

clean up storage_client and outsource helper funcs

fb5cd74

fix linting

f759fa9

use go struct to parse xml

842b398

serdarozerr reviewed Mar 19, 2026

View reviewed changes

dav/client/storage_client.go Outdated Show resolved Hide resolved

kathap mentioned this pull request Mar 25, 2026

Enhance storage-cli webdav config cloudfoundry/capi-release#634

Open

3 tasks

kathap added 5 commits March 25, 2026 10:47

refactored inline XML strings to use proper Go structs and XML marsha…

3da9d1a

…ling

fix linting

98bda4e

kathap mentioned this pull request Mar 26, 2026

Add WebDAV support to storage-cli client and improve test quality cloudfoundry/cloud_controller_ng#4974

Open

5 tasks

kathap marked this pull request as ready for review March 27, 2026 11:54

stephanme reviewed Mar 30, 2026

View reviewed changes

dav/README.md Outdated Show resolved Hide resolved

stephanme requested changes Mar 30, 2026

View reviewed changes

kathap added 2 commits April 10, 2026 12:06

fix linting

599c2b3

stephanme requested changes Apr 13, 2026

View reviewed changes

Fix PR review feedback: optimize propfind, merge loops, add test inje…

0e1e113

…ction

Conversation

kathap commented Mar 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants