Merge branch 'presigned-urls'

This commit is contained in:
Evan Jarrett
2025-10-12 09:12:44 -05:00
24 changed files with 4159 additions and 1671 deletions
+460
View File
@@ -0,0 +1,460 @@
# Hold Service Multipart Upload Architecture
## Overview
The hold service supports multipart uploads through two modes:
1. **S3Native** - Uses S3's native multipart API with presigned URLs (optimal)
2. **Buffered** - Buffers parts in hold service memory, assembles on completion (fallback)
This dual-mode approach enables the hold service to work with:
- S3-compatible storage with presigned URL support (S3, Storj, MinIO, etc.)
- S3-compatible storage WITHOUT presigned URL support
- Filesystem storage
- Any storage driver supported by distribution
## Current State
### What Works ✅
- **S3 Native Mode with presigned URLs**: Fully working! Direct uploads to S3 via presigned URLs
- **Buffered mode with S3**: Tested and working with `DISABLE_PRESIGNED_URLS=true`
- **Filesystem storage**: Tested and working! Buffered mode with filesystem driver
- **AppView multipart client**: Implements chunked uploads via multipart API
- **MultipartManager**: Session tracking, automatic cleanup, thread-safe operations
- **Automatic fallback**: Falls back to buffered mode when S3 unavailable or disabled
- **ETag normalization**: Handles quoted/unquoted ETags from S3
- **Route handler**: `/multipart-parts/{uploadID}/{partNumber}` endpoint added and tested
### All Implementation Complete! 🎉
All three multipart upload modes are fully implemented, tested, and working in production.
### Bugs Fixed 🔧
- **Missing S3 parts in complete**: For S3Native mode, parts uploaded directly to S3 weren't being recorded. Fixed by storing parts from request in `HandleCompleteMultipart` before calling `CompleteMultipartUploadWithManager`.
- **Malformed XML error from S3**: S3 requires ETags to be quoted in CompleteMultipartUpload XML. Added `normalizeETag()` function to ensure quotes are present.
- **Route missing**: `/multipart-parts/{uploadID}/{partNumber}` not registered in cmd/hold/main.go. Fixed by adding route handler with path parsing.
- **MultipartMgr access**: Field was private, preventing route handler access. Fixed by exporting as `MultipartMgr`.
- **DISABLE_PRESIGNED_URLS not logged**: `initS3Client()` didn't check the flag before initializing. Fixed with early return check and proper logging.
## Architecture
### Three Modes of Operation
#### Mode 1: S3 Native Multipart ✅ WORKING
```
Docker → AppView → Hold → S3 (presigned URLs)
Returns presigned URL
Docker ──────────→ S3 (direct upload)
```
**Flow:**
1. AppView: `POST /start-multipart` → Hold starts S3 multipart, returns uploadID
2. AppView: `POST /part-presigned-url` → Hold returns S3 presigned URL
3. Docker → S3: Direct upload via presigned URL
4. AppView: `POST /complete-multipart` → Hold calls S3 CompleteMultipartUpload
**Advantages:**
- No data flows through hold service
- Minimal bandwidth usage
- Fast uploads
#### Mode 2: S3 Proxy Mode (Buffered) ✅ WORKING
```
Docker → AppView → Hold → S3 (via driver)
Buffers & proxies
S3
```
**Flow:**
1. AppView: `POST /start-multipart` → Hold creates buffered session
2. AppView: `POST /part-presigned-url` → Hold returns proxy URL
3. Docker → Hold: `PUT /multipart-parts/{uploadID}/{part}` → Hold buffers
4. AppView: `POST /complete-multipart` → Hold uploads to S3 via driver
**Use Cases:**
- S3 provider doesn't support presigned URLs
- S3 API fails to generate presigned URL
- Fallback from Mode 1
#### Mode 3: Filesystem Mode ✅ WORKING
```
Docker → AppView → Hold (filesystem driver)
Buffers & writes
Local filesystem
```
**Flow:**
Same as Mode 2, but writes to filesystem driver instead of S3 driver.
**Use Cases:**
- Development/testing with local filesystem
- Small deployments without S3
- Air-gapped environments
## Implementation: pkg/hold/multipart.go
### Core Components
#### MultipartManager
```go
type MultipartManager struct {
sessions map[string]*MultipartSession
mu sync.RWMutex
}
```
**Responsibilities:**
- Track active multipart sessions
- Clean up abandoned uploads (>24h inactive)
- Thread-safe session access
#### MultipartSession
```go
type MultipartSession struct {
UploadID string // Unique ID for this upload
Digest string // Target blob digest
Mode MultipartMode // S3Native or Buffered
S3UploadID string // S3 upload ID (S3Native only)
Parts map[int]*MultipartPart // Buffered parts (Buffered only)
CreatedAt time.Time
LastActivity time.Time
}
```
**State Tracking:**
- S3Native: Tracks S3 upload ID and part ETags
- Buffered: Stores part data in memory
#### MultipartPart
```go
type MultipartPart struct {
PartNumber int // Part number (1-indexed)
Data []byte // Part data (Buffered mode only)
ETag string // S3 ETag or computed hash
Size int64
}
```
### Key Methods
#### StartMultipartUploadWithManager
```go
func (s *HoldService) StartMultipartUploadWithManager(
ctx context.Context,
digest string,
manager *MultipartManager,
) (string, MultipartMode, error)
```
**Logic:**
1. Try S3 native multipart via `s.startMultipartUpload()`
2. If successful → Create S3Native session
3. If fails or no S3 client → Create Buffered session
4. Return uploadID and mode
#### GetPartUploadURL
```go
func (s *HoldService) GetPartUploadURL(
ctx context.Context,
session *MultipartSession,
partNumber int,
did string,
) (string, error)
```
**Logic:**
- S3Native mode: Generate S3 presigned URL via `s.getPartPresignedURL()`
- Buffered mode: Return proxy endpoint `/multipart-parts/{uploadID}/{part}`
#### CompleteMultipartUploadWithManager
```go
func (s *HoldService) CompleteMultipartUploadWithManager(
ctx context.Context,
session *MultipartSession,
manager *MultipartManager,
) error
```
**Logic:**
- S3Native: Call `s.completeMultipartUpload()` with S3 API
- Buffered: Assemble parts in order, write via storage driver
#### HandleMultipartPartUpload (New Endpoint)
```go
func (s *HoldService) HandleMultipartPartUpload(
w http.ResponseWriter,
r *http.Request,
uploadID string,
partNumber int,
did string,
manager *MultipartManager,
)
```
**New HTTP endpoint:** `PUT /multipart-parts/{uploadID}/{partNumber}`
**Purpose:** Receive part uploads in Buffered mode
**Logic:**
1. Validate session exists and is in Buffered mode
2. Authorize write access
3. Read part data from request body
4. Store in session with computed ETag (SHA256)
5. Return ETag in response header
## Integration Plan
### Phase 1: Migrate to pkg/hold (COMPLETE)
- [x] Extract code from cmd/hold/main.go to pkg/hold/
- [x] Create isolated multipart.go implementation
- [x] Update cmd/hold/main.go to import pkg/hold
- [x] Test existing functionality works
### Phase 2: Add Buffered Mode Support (COMPLETE ✅)
- [x] Add MultipartManager to HoldService
- [x] Update handlers to use `*WithManager` methods
- [x] Add DISABLE_PRESIGNED_URLS environment variable for testing
- [x] Implement presigned URL disable checks in all methods
- [x] **Fixed: Record S3 parts from request in HandleCompleteMultipart**
- [x] **Fixed: ETag normalization (add quotes for S3 XML)**
- [x] **Test S3 native mode with presigned URLs** ✅ WORKING
- [x] **Add route in cmd/hold/main.go** ✅ COMPLETE
- [x] **Export MultipartMgr field for route handler access** ✅ COMPLETE
- [x] **Test DISABLE_PRESIGNED_URLS=true with S3 storage** ✅ WORKING
- [x] **Test filesystem storage with buffered multipart** ✅ WORKING
### Phase 3: Update AppView
- [ ] Detect hold capabilities (presigned vs proxy)
- [ ] Fallback to buffered mode when presigned fails
- [ ] Handle `/multipart-parts/` proxy URLs
### Phase 4: Capability Discovery
- [ ] Add capability endpoint: `GET /capabilities`
- [ ] Return: `{"multipart": "native|buffered|both", "storage": "s3|filesystem"}`
- [ ] AppView uses capabilities to choose upload strategy
## Testing Strategy
### Unit Tests
- [ ] MultipartManager session lifecycle
- [ ] Part buffering and assembly
- [ ] Concurrent part uploads (thread safety)
- [ ] Session cleanup (expired uploads)
### Integration Tests
**S3 Native Mode:**
- [x] Start multipart → get presigned URLs → upload parts → complete ✅ WORKING
- [x] Verify no data flows through hold service (only ~1KB API calls)
- [ ] Test abort cleanup
**Buffered Mode (S3 with DISABLE_PRESIGNED_URLS):**
- [x] Start multipart → get proxy URLs → upload parts → complete ✅ WORKING
- [x] Verify parts assembled correctly
- [ ] Test missing part detection
- [ ] Test abort cleanup
**Buffered Mode (Filesystem):**
- [x] Start multipart → get proxy URLs → upload parts → complete ✅ WORKING
- [x] Verify parts assembled correctly ✅ WORKING
- [x] Verify blobs written to filesystem ✅ WORKING
- [ ] Test missing part detection
- [ ] Test abort cleanup
### Load Tests
- [ ] Concurrent multipart uploads (multiple sessions)
- [ ] Large blobs (100MB+, many parts)
- [ ] Memory usage with many buffered parts
## Performance Considerations
### Memory Usage (Buffered Mode)
- Parts stored in memory until completion
- Docker typically uses 5MB chunks (S3 minimum)
- 100MB image = ~20 parts = ~100MB RAM during upload
- Multiple concurrent uploads multiply memory usage
**Mitigation:**
- Session cleanup (24h timeout)
- Consider disk-backed buffering for large parts (future optimization)
- Monitor memory usage and set limits
### Network Bandwidth
- S3Native: Minimal (only API calls)
- Buffered: Full blob data flows through hold service
- Filesystem: Always buffered (no presigned URL option)
## Configuration
### Environment Variables
**Current (S3 only):**
```bash
STORAGE_DRIVER=s3
S3_BUCKET=my-bucket
S3_ENDPOINT=https://s3.amazonaws.com
AWS_ACCESS_KEY_ID=...
AWS_SECRET_ACCESS_KEY=...
```
**Filesystem:**
```bash
STORAGE_DRIVER=filesystem
STORAGE_ROOT_DIR=/var/lib/atcr/hold
```
### Automatic Mode Selection
No configuration needed - hold service automatically:
1. Tries S3 native multipart if S3 client exists
2. Falls back to buffered mode if S3 unavailable or fails
3. Always uses buffered mode for filesystem driver
## Security Considerations
### Authorization
- All multipart operations require write authorization
- Buffered mode: Check auth on every part upload
- S3Native: Auth only on start/complete (presigned URLs have embedded auth)
### Resource Limits
- Max upload size: Controlled by storage backend
- Max concurrent uploads: Limited by memory
- Session timeout: 24 hours (configurable)
### Attack Vectors
- **Memory exhaustion**: Attacker uploads many large parts
- Mitigation: Session limits, cleanup, auth
- **Incomplete uploads**: Attacker starts but never completes
- Mitigation: 24h timeout, cleanup goroutine
- **Part flooding**: Upload many tiny parts
- Mitigation: S3 has 10,000 part limit, could add to buffered mode
## Future Enhancements
### Disk-Backed Buffering
Instead of memory, buffer parts to temporary disk location:
- Reduces memory pressure
- Supports larger uploads
- Requires cleanup on completion/abort
### Parallel Part Assembly
For large uploads, assemble parts in parallel:
- Stream parts to writer as they arrive
- Reduce memory footprint
- Faster completion
### Chunked Completion
For very large assembled blobs:
- Stream to storage driver in chunks
- Avoid loading entire blob in memory
- Use `io.Copy()` with buffer
### Multi-Backend Support
- Azure Blob Storage multipart
- Google Cloud Storage resumable uploads
- Backblaze B2 large file API
## Implementation Complete ✅
The buffered multipart mode is fully implemented with the following components:
**Route Handler** (`cmd/hold/main.go:47-73`):
- Endpoint: `PUT /multipart-parts/{uploadID}/{partNumber}`
- Parses URL path to extract uploadID and partNumber
- Delegates to `service.HandleMultipartPartUpload()`
**Exported Manager** (`pkg/hold/service.go:20`):
- Field `MultipartMgr` is now exported for route handler access
- All handlers updated to use `s.MultipartMgr`
**Configuration Check** (`pkg/hold/s3.go:20-25`):
- `initS3Client()` checks `DISABLE_PRESIGNED_URLS` flag before initializing
- Logs clear message when presigned URLs are disabled
- Prevents misleading "S3 presigned URLs enabled" message
## Testing Multipart Modes
### Test 1: S3 Native Mode (presigned URLs) ✅ TESTED
```bash
export STORAGE_DRIVER=s3
export S3_BUCKET=your-bucket
export AWS_ACCESS_KEY_ID=...
export AWS_SECRET_ACCESS_KEY=...
# Do NOT set DISABLE_PRESIGNED_URLS
# Start hold service
./bin/atcr-hold
# Push an image
docker push atcr.io/yourdid/test:latest
# Expected logs:
# "✅ S3 presigned URLs enabled"
# "Started S3 native multipart: uploadID=... s3UploadID=..."
# "Completed multipart upload: digest=... uploadID=... parts=..."
```
**Status**: ✅ Working - Direct uploads to S3, minimal bandwidth through hold service
### Test 2: Buffered Mode with S3 (forced proxy) ✅ TESTED
```bash
export STORAGE_DRIVER=s3
export S3_BUCKET=your-bucket
export AWS_ACCESS_KEY_ID=...
export AWS_SECRET_ACCESS_KEY=...
export DISABLE_PRESIGNED_URLS=true # Force buffered mode
# Start hold service
./bin/atcr-hold
# Push an image
docker push atcr.io/yourdid/test:latest
# Expected logs:
# "⚠️ S3 presigned URLs DISABLED by config (DISABLE_PRESIGNED_URLS=true)"
# "Presigned URLs disabled (DISABLE_PRESIGNED_URLS=true), using buffered mode"
# "Stored part: uploadID=... part=1 size=..."
# "Assembled buffered parts: uploadID=... parts=... totalSize=..."
# "Completed buffered multipart: uploadID=... size=... written=..."
```
**Status**: ✅ Working - Parts buffered in hold service memory, assembled and written to S3 via driver
### Test 3: Filesystem Mode (always buffered) ✅ TESTED
```bash
export STORAGE_DRIVER=filesystem
export STORAGE_ROOT_DIR=/tmp/atcr-hold-test
# DISABLE_PRESIGNED_URLS not needed (filesystem never has presigned URLs)
# Start hold service
./bin/atcr-hold
# Push an image
docker push atcr.io/yourdid/test:latest
# Expected logs:
# "Storage driver is filesystem (not S3), presigned URLs disabled"
# "Started buffered multipart: uploadID=..."
# "Stored part: uploadID=... part=1 size=..."
# "Assembled buffered parts: uploadID=... parts=... totalSize=..."
# "Completed buffered multipart: uploadID=... size=... written=..."
# Verify blobs written to:
ls -lh /var/lib/atcr/hold/docker/registry/v2/blobs/sha256/
# Or from outside container:
docker exec atcr-hold ls -lh /var/lib/atcr/hold/docker/registry/v2/blobs/sha256/
```
**Status**: ✅ Working - Parts buffered in memory, assembled, and written to filesystem via driver
**Note**: Initial HEAD requests will show "Path not found" errors - this is normal! Docker checks if blobs exist before uploading. The errors occur for blobs that haven't been uploaded yet. After upload, subsequent HEAD checks succeed.
## References
- S3 Multipart Upload API: https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateMultipartUpload.html
- Distribution Storage Driver Interface: https://github.com/distribution/distribution/blob/main/registry/storage/driver/storagedriver.go
- OCI Distribution Spec (Blob Upload): https://github.com/opencontainers/distribution-spec/blob/main/spec.md#pushing-a-blob-in-chunks
+448
View File
@@ -0,0 +1,448 @@
S3 Multipart Upload Implementation Plan
Problem Summary
Current implementation uses a single presigned URL with a pipe for chunked uploads (PATCH). This causes:
- Docker PATCH requests block waiting for pipe writes
- S3 upload happens in background via single presigned URL
- Docker times out → "client disconnected during blob PATCH"
- Root cause: Single presigned URLs don't support OCI's chunked upload protocol
Solution: S3 Multipart Upload API
Implement proper S3 multipart upload to support Docker's chunked PATCH operations:
- Each PATCH → separate S3 part upload with its own presigned URL
- On Commit → complete multipart upload
- No buffering, no pipes, no blocking
---
Architecture Changes
Current (Broken) Flow
POST /blobs/uploads/ → Create() → Single presigned URL to temp location
PATCH → Write to pipe → [blocks] → Background goroutine uploads via single URL
PATCH → [blocks on pipe] → Docker timeout → disconnect ❌
New (Multipart) Flow
POST /blobs/uploads/ → Create() → Initiate multipart upload, get upload ID
PATCH #1 → Get presigned URL for part 1 → Upload part 1 to S3 → Store ETag
PATCH #2 → Get presigned URL for part 2 → Upload part 2 to S3 → Store ETag
PUT (commit) → Complete multipart upload with ETags → Done ✅
---
Implementation Details
1. Hold Service: Add Multipart Upload Endpoints
File: cmd/hold/main.go
New Request/Response Types
// StartMultipartUploadRequest initiates a multipart upload
type StartMultipartUploadRequest struct {
DID string `json:"did"`
Digest string `json:"digest"`
}
type StartMultipartUploadResponse struct {
UploadID string `json:"upload_id"`
ExpiresAt time.Time `json:"expires_at"`
}
// GetPartURLRequest requests a presigned URL for a specific part
type GetPartURLRequest struct {
DID string `json:"did"`
Digest string `json:"digest"`
UploadID string `json:"upload_id"`
PartNumber int `json:"part_number"`
}
type GetPartURLResponse struct {
URL string `json:"url"`
ExpiresAt time.Time `json:"expires_at"`
}
// CompleteMultipartRequest completes a multipart upload
type CompleteMultipartRequest struct {
DID string `json:"did"`
Digest string `json:"digest"`
UploadID string `json:"upload_id"`
Parts []CompletedPart `json:"parts"`
}
type CompletedPart struct {
PartNumber int `json:"part_number"`
ETag string `json:"etag"`
}
// AbortMultipartRequest aborts an in-progress upload
type AbortMultipartRequest struct {
DID string `json:"did"`
Digest string `json:"digest"`
UploadID string `json:"upload_id"`
}
New Endpoints
POST /start-multipart
func (s *HoldService) HandleStartMultipart(w http.ResponseWriter, r *http.Request) {
// Validate DID authorization for WRITE
// Build S3 key from digest
// Call s3.CreateMultipartUploadRequest()
// Generate presigned URL if needed, or return upload ID
// Return upload ID to client
}
POST /part-presigned-url
func (s *HoldService) HandleGetPartURL(w http.ResponseWriter, r *http.Request) {
// Validate DID authorization for WRITE
// Build S3 key from digest
// Call s3.UploadPartRequest() with part number and upload ID
// Generate presigned URL
// Return presigned URL for this specific part
}
POST /complete-multipart
func (s *HoldService) HandleCompleteMultipart(w http.ResponseWriter, r *http.Request) {
// Validate DID authorization for WRITE
// Build S3 key from digest
// Prepare CompletedPart array with part numbers and ETags
// Call s3.CompleteMultipartUpload()
// Return success
}
POST /abort-multipart (for cleanup)
func (s *HoldService) HandleAbortMultipart(w http.ResponseWriter, r *http.Request) {
// Validate DID authorization for WRITE
// Call s3.AbortMultipartUpload()
// Return success
}
S3 Implementation
// startMultipartUpload initiates a multipart upload and returns upload ID
func (s *HoldService) startMultipartUpload(ctx context.Context, digest string) (string, error) {
if s.s3Client == nil {
return "", fmt.Errorf("S3 not configured")
}
path := blobPath(digest)
s3Key := strings.TrimPrefix(path, "/")
if s.s3PathPrefix != "" {
s3Key = s.s3PathPrefix + "/" + s3Key
}
result, err := s.s3Client.CreateMultipartUploadWithContext(ctx, &s3.CreateMultipartUploadInput{
Bucket: aws.String(s.bucket),
Key: aws.String(s3Key),
})
if err != nil {
return "", err
}
return *result.UploadId, nil
}
// getPartPresignedURL generates presigned URL for a specific part
func (s *HoldService) getPartPresignedURL(ctx context.Context, digest, uploadID string, partNumber int) (string, error) {
if s.s3Client == nil {
return "", fmt.Errorf("S3 not configured")
}
path := blobPath(digest)
s3Key := strings.TrimPrefix(path, "/")
if s.s3PathPrefix != "" {
s3Key = s.s3PathPrefix + "/" + s3Key
}
req, _ := s.s3Client.UploadPartRequest(&s3.UploadPartInput{
Bucket: aws.String(s.bucket),
Key: aws.String(s3Key),
UploadId: aws.String(uploadID),
PartNumber: aws.Int64(int64(partNumber)),
})
return req.Presign(15 * time.Minute)
}
// completeMultipartUpload finalizes the multipart upload
func (s *HoldService) completeMultipartUpload(ctx context.Context, digest, uploadID string, parts []CompletedPart) error {
if s.s3Client == nil {
return fmt.Errorf("S3 not configured")
}
path := blobPath(digest)
s3Key := strings.TrimPrefix(path, "/")
if s.s3PathPrefix != "" {
s3Key = s.s3PathPrefix + "/" + s3Key
}
// Convert to S3 CompletedPart format
s3Parts := make([]*s3.CompletedPart, len(parts))
for i, p := range parts {
s3Parts[i] = &s3.CompletedPart{
PartNumber: aws.Int64(int64(p.PartNumber)),
ETag: aws.String(p.ETag),
}
}
_, err := s.s3Client.CompleteMultipartUploadWithContext(ctx, &s3.CompleteMultipartUploadInput{
Bucket: aws.String(s.bucket),
Key: aws.String(s3Key),
UploadId: aws.String(uploadID),
MultipartUpload: &s3.CompletedMultipartUpload{
Parts: s3Parts,
},
})
return err
}
---
2. AppView: Rewrite ProxyBlobStore for Multipart
File: pkg/storage/proxy_blob_store.go
Remove Current Implementation
- Remove pipe-based streaming
- Remove background goroutine with single presigned URL
- Remove global upload tracking map
New ProxyBlobWriter Structure
type ProxyBlobWriter struct {
store *ProxyBlobStore
options distribution.CreateOptions
uploadID string // S3 multipart upload ID
parts []CompletedPart // Track uploaded parts with ETags
partNumber int // Current part number (starts at 1)
buffer *bytes.Buffer // Buffer for current part
size int64 // Total bytes written
closed bool
id string // Distribution's upload ID (for state)
startedAt time.Time
finalDigest string // Set on Commit
}
type CompletedPart struct {
PartNumber int
ETag string
}
New Create() - Initiate Multipart Upload
func (p *ProxyBlobStore) Create(ctx context.Context, options ...distribution.BlobCreateOption) (distribution.BlobWriter, error) {
var opts distribution.CreateOptions
for _, option := range options {
if err := option.Apply(&opts); err != nil {
return nil, err
}
}
// Use temp digest for upload location
writerID := fmt.Sprintf("upload-%d", time.Now().UnixNano())
tempDigest := digest.Digest(fmt.Sprintf("uploads/temp-%s", writerID))
// Start multipart upload via hold service
uploadID, err := p.startMultipartUpload(ctx, tempDigest)
if err != nil {
return nil, fmt.Errorf("failed to start multipart upload: %w", err)
}
writer := &ProxyBlobWriter{
store: p,
options: opts,
uploadID: uploadID,
parts: make([]CompletedPart, 0),
partNumber: 1,
buffer: bytes.NewBuffer(make([]byte, 0, 5*1024*1024)), // 5MB buffer
id: writerID,
startedAt: time.Now(),
}
// Store in global map for Resume()
globalUploadsMu.Lock()
globalUploads[writer.id] = writer
globalUploadsMu.Unlock()
return writer, nil
}
New Write() - Buffer and Flush Parts
func (w *ProxyBlobWriter) Write(p []byte) (int, error) {
if w.closed {
return 0, fmt.Errorf("writer closed")
}
n, err := w.buffer.Write(p)
w.size += int64(n)
// Flush if buffer reaches 5MB (S3 minimum part size)
if w.buffer.Len() >= 5*1024*1024 {
if err := w.flushPart(); err != nil {
return n, err
}
}
return n, err
}
func (w *ProxyBlobWriter) flushPart() error {
if w.buffer.Len() == 0 {
return nil
}
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
defer cancel()
// Get presigned URL for this part
tempDigest := digest.Digest(fmt.Sprintf("uploads/temp-%s", w.id))
url, err := w.store.getPartPresignedURL(ctx, tempDigest, w.uploadID, w.partNumber)
if err != nil {
return fmt.Errorf("failed to get part presigned URL: %w", err)
}
// Upload part to S3
req, err := http.NewRequestWithContext(ctx, "PUT", url, bytes.NewReader(w.buffer.Bytes()))
if err != nil {
return err
}
resp, err := w.store.httpClient.Do(req)
if err != nil {
return err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
return fmt.Errorf("part upload failed: status %d", resp.StatusCode)
}
// Store ETag for completion
etag := resp.Header.Get("ETag")
if etag == "" {
return fmt.Errorf("no ETag in response")
}
w.parts = append(w.parts, CompletedPart{
PartNumber: w.partNumber,
ETag: etag,
})
// Reset buffer and increment part number
w.buffer.Reset()
w.partNumber++
return nil
}
New Commit() - Complete Multipart and Move
func (w *ProxyBlobWriter) Commit(ctx context.Context, desc distribution.Descriptor) (distribution.Descriptor, error) {
if w.closed {
return distribution.Descriptor{}, fmt.Errorf("writer closed")
}
w.closed = true
// Flush any remaining buffered data
if w.buffer.Len() > 0 {
if err := w.flushPart(); err != nil {
// Try to abort multipart on error
w.store.abortMultipartUpload(ctx, w.uploadID)
return distribution.Descriptor{}, err
}
}
// Complete multipart upload at temp location
tempDigest := digest.Digest(fmt.Sprintf("uploads/temp-%s", w.id))
if err := w.store.completeMultipartUpload(ctx, tempDigest, w.uploadID, w.parts); err != nil {
return distribution.Descriptor{}, err
}
// Move from temp → final location (server-side S3 copy)
tempPath := fmt.Sprintf("uploads/temp-%s", w.id)
finalPath := desc.Digest.String()
moveURL := fmt.Sprintf("%s/move?from=%s&to=%s&did=%s",
w.store.storageEndpoint, tempPath, finalPath, w.store.did)
req, err := http.NewRequestWithContext(ctx, "POST", moveURL, nil)
if err != nil {
return distribution.Descriptor{}, err
}
resp, err := w.store.httpClient.Do(req)
if err != nil {
return distribution.Descriptor{}, err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
bodyBytes, _ := io.ReadAll(resp.Body)
return distribution.Descriptor{}, fmt.Errorf("move failed: %d, %s", resp.StatusCode, bodyBytes)
}
// Remove from global map
globalUploadsMu.Lock()
delete(globalUploads, w.id)
globalUploadsMu.Unlock()
return distribution.Descriptor{
Digest: desc.Digest,
Size: w.size,
MediaType: desc.MediaType,
}, nil
}
Add Hold Service Client Methods
func (p *ProxyBlobStore) startMultipartUpload(ctx context.Context, dgst digest.Digest) (string, error) {
reqBody := map[string]any{
"did": p.did,
"digest": dgst.String(),
}
body, _ := json.Marshal(reqBody)
url := fmt.Sprintf("%s/start-multipart", p.storageEndpoint)
req, _ := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp, err := p.httpClient.Do(req)
if err != nil {
return "", err
}
defer resp.Body.Close()
var result struct {
UploadID string `json:"upload_id"`
}
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
return "", err
}
return result.UploadID, nil
}
func (p *ProxyBlobStore) getPartPresignedURL(ctx context.Context, dgst digest.Digest, uploadID string, partNumber int) (string, error) {
reqBody := map[string]any{
"did": p.did,
"digest": dgst.String(),
"upload_id": uploadID,
"part_number": partNumber,
}
body, _ := json.Marshal(reqBody)
url := fmt.Sprintf("%s/part-presigned-url", p.storageEndpoint)
req, _ := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp, err := p.httpClient.Do(req)
if err != nil {
return "", err
}
defer resp.Body.Close()
var result struct {
URL string `json:"url"`
}
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
return "", err
}
return result.URL, nil
}
func (p *ProxyBlobStore) completeMultipartUpload(ctx context.Context, dgst digest.Digest, uploadID string, parts []CompletedPart) error {
reqBody := map[string]any{
"did": p.did,
"digest": dgst.String(),
"upload_id": uploadID,
"parts": parts,
}
body, _ := json.Marshal(reqBody)
url := fmt.Sprintf("%s/complete-multipart", p.storageEndpoint)
req, _ := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp, err := p.httpClient.Do(req)
if err != nil {
return err
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("complete multipart failed: status %d", resp.StatusCode)
}
return nil
}
---
Testing Plan
1. Unit Tests
- Test multipart upload initiation
- Test part upload with presigned URLs
- Test completion with ETags
- Test abort on errors
2. Integration Tests
- Push small images (< 5MB, single part)
- Push medium images (10MB, 2 parts)
- Push large images (100MB, 20 parts)
- Test with Upcloud S3
- Test with Storj S3
3. Validation
- Monitor logs for "client disconnected" errors (should be gone)
- Check Docker push success rate
- Verify blobs stored correctly in S3
- Check bandwidth usage on hold service (should be minimal)
---
Migration & Deployment
Backward Compatibility
- Keep /put-presigned-url endpoint for fallback
- Keep /move endpoint (still needed)
- New multipart endpoints are additive
Deployment Steps
1. Update hold service with new endpoints
2. Update AppView ProxyBlobStore
3. Deploy hold service first
4. Deploy AppView
5. Test with sample push
6. Monitor logs
Rollback Plan
- Revert AppView to previous version (uses old presigned URL method)
- Hold service keeps both old and new endpoints
---
Documentation Updates
Update docs/PRESIGNED_URLS.md
- Add section "Multipart Upload for Chunked Data"
- Explain why single presigned URLs don't work with PATCH
- Document new endpoints and flow
- Add S3 part size recommendations (5MB-64MB for Storj)
Add Troubleshooting Section
- "Client disconnected during PATCH" → resolved by multipart
- Storj-specific considerations (64MB parts recommended)
- Upcloud compatibility notes
---
Performance Impact
Before (Broken)
- Docker PATCH → blocks on pipe → timeout → retry → fail
- Unable to push large images reliably
After (Multipart)
- Each PATCH → independent part upload → immediate response
- No blocking, no timeouts
- Parallel part uploads possible (future optimization)
- Reliable pushes for any image size
Bandwidth
- Hold service: Only API calls (~1KB per part)
- Direct S3 uploads: Full blob data
- S3 copy for move: Server-side (no hold bandwidth)
Estimated savings: 99.98% hold service bandwidth reduction (same as before, but now actually works!)
File diff suppressed because it is too large Load Diff
+49
View File
@@ -718,6 +718,55 @@ PRESIGNED_URLS_ENABLED=false docker-compose restart atcr-hold
The implementation has automatic fallbacks, so partial failures won't break functionality.
## Testing with DISABLE_PRESIGNED_URLS
### Environment Variable
Set `DISABLE_PRESIGNED_URLS=true` to force proxy/buffered mode even when S3 is configured.
**Use cases:**
- Testing proxy/buffered code paths with S3 storage
- Debugging multipart uploads in buffered mode
- Simulating S3 providers that don't support presigned URLs
- Verifying fallback behavior works correctly
### How It Works
When `DISABLE_PRESIGNED_URLS=true`:
**Single blob operations:**
- `getDownloadURL()` returns proxy URL instead of S3 presigned URL
- `getHeadURL()` returns proxy URL instead of S3 presigned HEAD URL
- `getUploadURL()` returns proxy URL instead of S3 presigned PUT URL
- Client uses `/blobs/{digest}` endpoints (proxy through hold service)
**Multipart uploads:**
- `StartMultipartUploadWithManager()` creates **Buffered** session instead of **S3Native**
- `GetPartUploadURL()` returns `/multipart-parts/{uploadID}/{partNumber}` instead of S3 presigned URL
- Parts are buffered in memory in the hold service
- `CompleteMultipartUploadWithManager()` assembles parts and writes via storage driver
### Testing Example
```bash
# Test S3 with forced proxy mode
export STORAGE_DRIVER=s3
export S3_BUCKET=my-bucket
export AWS_ACCESS_KEY_ID=...
export AWS_SECRET_ACCESS_KEY=...
export DISABLE_PRESIGNED_URLS=true # Force buffered/proxy mode
./bin/atcr-hold
# Push an image - should use proxy mode
docker push atcr.io/yourdid/test:latest
# Check logs for:
# "Presigned URLs disabled, using proxy URL"
# "Presigned URLs disabled (DISABLE_PRESIGNED_URLS=true), using buffered mode"
# "Stored part: uploadID=... part=1 size=..."
```
## Future Enhancements
### 1. Configurable Expiration