Refactor: Use shared UploadPrefix constant across cloud drivers

[yuta519]

Description

This PR introduces a shared UploadPrefix constant in the byoc package and updates all relevant cloud drivers (AWS, DigitalOcean, GCP) to use it instead of defining provider-specific upload prefixes.

Added UploadPrefix constant in pkg/cli/client/byoc/common.go and referenced it in:

• AWS BYOC client (byoc/aws/byoc.go)
• DigitalOcean BYOC client (byoc/do/byoc.go)
• GCP BYOC client (byoc/gcp/byoc.go)

Avoids duplicate hardcoded prefixes and ensures uniform upload path structure.

Linked Issues

#1904

Checklist

• I have performed a self-review of my code
• I have added appropriate tests
• I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

• Refactor
  • Restructured upload URL generation to support configurable path prefixes across all cloud providers (AWS, DigitalOcean, Docker, Local), consolidating prefix definitions for improved consistency and maintainability.

[coderabbitai]

📝 Walkthrough

Walkthrough

Centralizes BYOC upload prefix into byoc.UploadPrefix, updates call sites to pass that prefix, and changes the CreateUploadURL driver/API to accept a prefix and filename (instead of a single name) with corresponding updates to AWS, DO, Docker, Local, and other implementations.

Changes

Cohort / File(s)	Summary
BYOC package & clients src/pkg/cli/client/byoc/common.go, src/pkg/cli/client/byoc/aws/byoc.go, src/pkg/cli/client/byoc/do/byoc.go, src/pkg/cli/client/byoc/gcp/byoc.go	Added exported byoc.UploadPrefix and updated BYOC client call sites to pass byoc.UploadPrefix when requesting upload URLs; removed local UploadPrefix from GCP file.
Driver API change src/pkg/clouds/driver.go	Driver interface CreateUploadURL signature changed from (ctx, name) to (ctx, prefix, name). All callers/implementations must pass the prefix argument.
AWS ECS upload implementation src/pkg/clouds/aws/ecs/upload.go	Signature changed to accept (prefix, filename); filename is generated when empty, validated, sanitized, and S3 key built as prefix + filename (removed hardcoded prefix constant).
DigitalOcean AppPlatform upload implementation src/pkg/clouds/do/appPlatform/setup.go	Signature changed to (prefix, filename) with analogous UUID fallback, validation, sanitization, and key construction using provided prefix.
Other driver implementations src/pkg/crun/docker/common.go, src/pkg/crun/local/local.go	Updated CreateUploadURL signatures to accept an extra prefix parameter; Local still returns not-implemented error.
BYOC client deploy flow src/pkg/cli/client/byoc/aws/byoc.go, src/pkg/cli/client/byoc/do/byoc.go	Deploy path for large payloads now calls CreateUploadURL(ctx, byoc.UploadPrefix, etag) and uses the returned presigned URL for PUT. CreateUploadURL handler updated to pass byoc.UploadPrefix to driver.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• lionello
• jordanstephens

Poem

🐇 I hopped through code with nimble paws,

Moved prefixes into tidy laws,
Filenames trimmed and UUIDs spun,
Uploads march under one bright sun,
Hooray — the burrow's neat and done! 🥕

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: refactoring to use a shared UploadPrefix constant across cloud drivers, which aligns with the core objective of consolidating hardcoded prefixes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter"
level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/pkg/clouds/do/appPlatform/setup.go (1)

236-266: ⚠️ Potential issue | 🔴 Critical

Same sanitization bug as AWS ECS: / in objectKeyName will be replaced with _.

The s3InvalidCharsRegexp on Line 234 doesn't allow /, so when callers pass "uploads/<digest>", the sanitization on Line 249 produces "uploads_<digest>". See the detailed comment on src/pkg/clouds/aws/ecs/upload.go.

🐛 Proposed fix: allow `/` in the regex

-var s3InvalidCharsRegexp = regexp.MustCompile(`[^a-zA-Z0-9!_.*'()-]`)
+var s3InvalidCharsRegexp = regexp.MustCompile(`[^a-zA-Z0-9!_.*'()/-]`)

🤖 Fix all issues with AI agents

In `@src/pkg/cli/client/byoc/do/byoc.go`:
- Line 166: The call to b.driver.CreateUploadURL currently uses
path.Join(byoc.UploadPrefix, etag) which removes the slash and later gets
sanitized by s3InvalidCharsRegexp, producing "uploads_<etag>" instead of
"uploads/<etag>"; change the argument to construct the upload path with an
explicit slash (e.g., byoc.UploadPrefix + "/" + etag) so the resulting string
retains the separator when passed to b.driver.CreateUploadURL (update the call
site in byoc.go where b.driver.CreateUploadURL is invoked).

In `@src/pkg/clouds/aws/ecs/upload.go`:
- Around line 22-31: The sanitization currently replaces `/` in objectKeyName
which flattens S3 keys and the 64-char check is applied to the full prefixed
path; update the logic so directory separators are preserved and the length
check applies to the actual basename: either modify s3InvalidCharsRegexp to
permit `/` (so objectKeyName like "uploads/abc123" keeps its separators) or
split objectKeyName into dir and base (use path.Dir/path.Base), run
s3InvalidCharsRegexp.ReplaceAllString only on the basename, then rejoin; also
perform the 64-character validation on the sanitized basename (or adjust the
limit to exclude any upload prefix) to restore the original effective name
length constraint.

[coderabbitai]

🧹 Nitpick comments (2)

src/pkg/clouds/aws/ecs/upload.go (1)

16-37: Prefix is concatenated without ensuring a path separator.

The key is built as prefix + filename (Line 37). This works correctly when prefix ends with / (as byoc.UploadPrefix = "uploads/" does), but if a caller passes a prefix without a trailing slash, the key will be malformed (e.g. "uploadsfoo").

Consider using path.Join or documenting/enforcing the trailing-slash convention.

♻️ Suggested defensive fix

+	"path"
...
-		Key:    ptr.String(prefix + filename),
+		Key:    ptr.String(path.Join(prefix, filename)),

src/pkg/clouds/do/appPlatform/setup.go (1)

234-258: Duplicated upload-URL logic between DO and AWS ECS implementations.

CreateUploadURL in this file is nearly identical to src/pkg/clouds/aws/ecs/upload.go — same regex, same UUID fallback, same 64-char limit, same sanitization, same presign flow. This is a good candidate for extracting a shared helper (e.g., a sanitizeFilename function) to keep the validation/sanitization logic in one place.

Same trailing-slash note as the AWS implementation applies here for prefix + filename on Line 258.