fix: missing digest in default mode

[lionello]

Description

Fixes a regression from #1801 : missing fallthrough caused an empty Digest for all services in the project, causing them to overwrite each other's contexts.

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

• Bug Fixes

  • Safer upload URL construction with error propagation and normalization (including gs:// form).
  • More reproducible archives by clearing variable timestamps and ownership metadata.

• Enhancements

  • Uploads now use content digests for stable, predictable URLs; preview/estimate modes return preview-style URLs.
  • Improved upload handling and response validation; correct content types applied.

• Tests

  • Expanded end-to-end tests covering multiple upload modes and server-side validation.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Replaces manual URL path joins with url.JoinPath, centralizes digest computation and digest-based placeholder URLs, updates uploadArchive to use ArchiveType (MIME) and validate/normalize responses, tightens tar reproducibility fields, and expands tests for remote build context and upload flows.

Changes

Cohort / File(s)	Summary
Client URL construction src/pkg/cli/client/mock.go	Replace manual string concatenation with url.JoinPath(m.UploadUrl, ...) in CreateUploadURL; propagate join errors; add net/url import.
Compose: digest, archive upload & tar reproducibility src/pkg/cli/compose/context.go	Add calcDigest([]byte); clear tar header AccessTime/ChangeTime and Gname/Uname; merge UploadModeDefault/UploadModeDigest behavior; preview/estimate return digest-based s3:// placeholders; change uploadArchive to accept projectName and archiveType, use archiveType.MimeType, validate HTTP 200, close response body, strip query params, and convert GCS HTTPS URLs to gs://.
Tests: remote build context and uploads src/pkg/cli/compose/context_test.go, src/pkg/cli/compose/*	Add Test_getRemoteBuildContext covering multiple UploadModes with an HTTP test server that captures uploads, validates digests and normalized URLs; update TestUploadArchive to include project scoping and use t.Cleanup; adjust expected paths to include project segment.
Tests: DNS debug state handling src/pkg/dns/check_test.go	Preserve and restore global resolver and debug state in TestCheckDomainDNSReady; enable debug during test body and restore previous state in cleanup.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CLI as CLI (compose)
  participant Server as Remote Upload Endpoint
  participant Storage as Cloud Storage
  CLI->>CLI: create archive stream, calcDigest(data)
  CLI->>Server: HTTP PUT /upload/<project>/<stack>/<digest><ext> (body, Content-Type=archiveType.MimeType)
  Server->>Storage: persist object, respond with HTTPS URL / Location
  Server-->>CLI: 200 OK (Location header / URL)
  CLI->>CLI: close response body, normalize URL (strip query, convert GCS https -> gs://)
  CLI-->>User: return normalized uploaded URL (s3://... or gs://...)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fix: missing digest in default mode #1808 — Modifies same functions and code paths (CreateUploadURL, getRemoteBuildContext, uploadArchive and tests).
• Get urls and pass to CD for states and events uploading #1764 — Also updates upload URL construction in src/pkg/cli/client/mock.go to use proper path joining.

Suggested reviewers

• jordanstephens
• edwardrf

Poem

🐰 I joined the paths where strings once bled,
I hashed the bytes and found the thread,
I PUT the parcel, checked the score,
I trimmed the query, changed the door,
A hop, a test — uploads align ahead. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: missing digest in default mode' clearly and accurately describes the main change - fixing a missing digest regression in default upload mode.

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

✨ Finishing touches

• 📝 Generate docstrings

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"

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/pkg/cli/compose/context_test.go`:
- Around line 177-213: The Ignore-mode expectation is hardcoded to a macOS path;
update Test_getRemoteBuildContext so the UploadModeIgnore test computes the
expected path dynamically by calling filepath.Abs on the local test project path
(e.g. filepath.Join("testdata","testproj")), then normalize separators (e.g.
filepath.ToSlash) before comparing; modify the test case for uploadMode ==
UploadModeIgnore to use that computed, normalized path instead of the hardcoded
"/Users/$USER/..." string so it is OS-agnostic.

🧹 Nitpick comments (1)

src/pkg/cli/client/mock.go (1)

24-26: Avoid shadowing the url package name.
Renaming the local variable improves readability.

♻️ Proposed refactor

-	url, err := url.JoinPath(m.UploadUrl, req.Digest)
-	return &defangv1.UploadURLResponse{Url: url}, err
+	uploadURL, err := url.JoinPath(m.UploadUrl, req.Digest)
+	return &defangv1.UploadURLResponse{Url: uploadURL}, err

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/pkg/cli/compose/context_test.go`:
- Around line 90-93: The assertion's error message is inconsistent with the
condition: the code computes expectedPath (path + testproj + "/" +
ArchiveTypeZip.Extension) and compares url against server.URL+expectedPath, but
the t.Errorf message prints server.URL+path+ArchiveTypeZip.Extension which can
confuse failures; update the t.Errorf to use the same expected value
(server.URL+expectedPath) or directly print expectedPath so the message matches
the comparison involving expectedPath, referencing variables expectedPath,
server.URL, path, testproj, ArchiveTypeZip.Extension and url in the test.

🧹 Nitpick comments (1)

src/pkg/cli/compose/context_test.go (1)

245-245: Consider restoring debug state after test.

term.SetDebug(true) modifies global state which could affect other tests running in parallel. Consider saving and restoring the original value.

♻️ Suggested fix

 	t.Cleanup(server.Close)
-	term.SetDebug(true)
+	oldDebug := term.DoDebug()
+	term.SetDebug(true)
+	t.Cleanup(func() { term.SetDebug(oldDebug) })