Get urls and pass to CD for states and events uploading #1764
Conversation
📝 WalkthroughWalkthroughThreads StatesUrl and EventsUrl upload URLs through Deploy and CdCommand flows, adds CreateUploadURL to Fabric client, refactors postDown to remove etag/update logic, and records a DOWN deployment after successful CdCommand. URLs are retrieved (env override or Fabric) for non-Playground providers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant Provider
participant Fabric
participant DeploymentAPI
User->>CLI: trigger deploy/down
CLI->>Provider: Build DeployRequest (may call GetStatesAndEventsUploadUrls)
alt non-Playground
CLI->>Fabric: CreateUploadURL (states/events)
Fabric-->>CLI: StatesUrl, EventsUrl
else Playground
CLI-->>CLI: read env or skip URL creation
end
CLI->>Provider: Call Deploy/CdCommand with StatesUrl/EventsUrl
Provider->>Provider: run cdCommand (inject env URLs)
Provider->>DeploymentAPI: putDeployment Action:DOWN (StatesUrl/EventsUrl)
DeploymentAPI-->>Provider: ack
Provider-->>CLI: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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" 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/cli/client/byoc/do/byoc.go (1)
607-649: Mark upload URLs as secrets and avoid printing them in debug output.These URLs are pre-signed upload URLs created by the fabric service (via
CreateUploadURL) and may contain embedded credentials or signatures. Currently, they are (a) added to the environment withoutType: godo.AppVariableType_Secretand (b) printed verbatim in debug output viaDebugPulumiNodeJS. This is inconsistent with how other sensitive data (AWS credentials, Pulumi passphrase, DigitalOcean token, etc.) is handled elsewhere in theenvironment()method.The fix should mark both
DEFANG_STATES_UPLOAD_URLandDEFANG_EVENTS_UPLOAD_URLwithType: godo.AppVariableType_Secret, and mask secrets in the debug output (similar to the proposed diff). The same issue exists in the AWS and GCP implementations.
🧹 Nitpick comments (1)
src/pkg/cli/client/provider.go (1)
38-43: Consider adding documentation for the public API.The new
DeployRequesttype is part of the public API but lacks documentation explaining its purpose and the meaning ofStatesUrlandEventsUrlfields.📝 Suggested documentation
+// DeployRequest extends the protobuf DeployRequest with additional +// metadata for deployment state and event upload URLs. type DeployRequest struct { defangv1.DeployRequest + // StatesUrl is the upload URL for deployment state snapshots StatesUrl string + // EventsUrl is the upload URL for deployment events EventsUrl string }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/pkg/cli/cd.gosrc/pkg/cli/client/byoc/aws/byoc.gosrc/pkg/cli/client/byoc/aws/validation_test.gosrc/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/client/client.gosrc/pkg/cli/client/grpc.gosrc/pkg/cli/client/mock.gosrc/pkg/cli/client/playground.gosrc/pkg/cli/client/provider.gosrc/pkg/cli/common.gosrc/pkg/cli/composeUp.gosrc/pkg/cli/composeUp_test.go
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-07T00:34:13.131Z
Learnt from: lionello
Repo: DefangLabs/defang PR: 1742
File: src/pkg/cli/composeDown.go:14-18
Timestamp: 2026-01-07T00:34:13.131Z
Learning: In Defang's Defang CLI, CdCommandDown performs refresh + destroy, while CdCommandDestroy performs destroy only (no refresh). Update ComposeDown (src/pkg/cli/composeDown.go) to call CdCommandDestroy to perform destruction without refreshing. This ensures the intended semantics are preserved when tearing down compositions; avoid using CdCommandDown in ComposeDown unless a refresh is explicitly desired. Verify that ComposeDown's destroy path does not trigger a refresh side effect from CdCommandDown and that tests cover both pathways if they exist.
Applied to files:
src/pkg/cli/client/client.gosrc/pkg/cli/client/mock.gosrc/pkg/cli/composeUp_test.gosrc/pkg/cli/common.gosrc/pkg/cli/client/byoc/aws/validation_test.gosrc/pkg/cli/client/provider.gosrc/pkg/cli/client/grpc.gosrc/pkg/cli/client/byoc/aws/byoc.gosrc/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/playground.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/cd.gosrc/pkg/cli/composeUp.go
📚 Learning: 2025-12-31T13:47:12.225Z
Learnt from: lionello
Repo: DefangLabs/defang PR: 1740
File: src/pkg/cli/client/byoc/parse_test.go:18-21
Timestamp: 2025-12-31T13:47:12.225Z
Learning: In Go test files (any _test.go under the Defang codebase), it's acceptable for mocks to panic to surface issues quickly during tests. Do not add defensive error handling in mocks within tests, since panics will fail fast and highlight problems. Ensure this behavior is confined to test code and does not affect production code or non-test paths.
Applied to files:
src/pkg/cli/composeUp_test.gosrc/pkg/cli/client/byoc/aws/validation_test.go
📚 Learning: 2026-01-07T03:07:48.228Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/cli/client/byoc/gcp/byoc.go:448-450
Timestamp: 2026-01-07T03:07:48.228Z
Learning: In src/pkg/cli/client/byoc/gcp/byoc.go, the GetDeploymentStatus method intentionally does not pre-validate b.cdExecution before calling b.driver.GetBuildStatus. If b.cdExecution is empty, it represents an error state that will be surfaced by the GCP API as an "invalid operation name" error, which is the intended behavior.
Applied to files:
src/pkg/cli/client/byoc/aws/validation_test.gosrc/pkg/cli/client/byoc/aws/byoc.gosrc/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/cd.go
🧬 Code graph analysis (12)
src/pkg/cli/client/client.go (1)
src/protos/io/defang/v1/fabric.pb.go (6)
UploadURLRequest(2024-2032)UploadURLRequest(2045-2045)UploadURLRequest(2060-2062)UploadURLResponse(2092-2097)UploadURLResponse(2110-2110)UploadURLResponse(2125-2127)
src/pkg/cli/client/mock.go (1)
src/protos/io/defang/v1/fabric.pb.go (6)
UploadURLRequest(2024-2032)UploadURLRequest(2045-2045)UploadURLRequest(2060-2062)UploadURLResponse(2092-2097)UploadURLResponse(2110-2110)UploadURLResponse(2125-2127)
src/pkg/cli/composeUp_test.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (6)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)DeployResponse(1608-1614)DeployResponse(1627-1627)DeployResponse(1642-1644)
src/pkg/cli/client/byoc/aws/validation_test.go (1)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)
src/pkg/cli/client/provider.go (1)
src/protos/io/defang/v1/fabric.pb.go (6)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)DeployResponse(1608-1614)DeployResponse(1627-1627)DeployResponse(1642-1644)
src/pkg/cli/client/grpc.go (1)
src/protos/io/defang/v1/fabric.pb.go (6)
UploadURLRequest(2024-2032)UploadURLRequest(2045-2045)UploadURLRequest(2060-2062)UploadURLResponse(2092-2097)UploadURLResponse(2110-2110)UploadURLResponse(2125-2127)
src/pkg/cli/client/byoc/aws/byoc.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (6)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)DeployResponse(1608-1614)DeployResponse(1627-1627)DeployResponse(1642-1644)
src/pkg/cli/client/byoc/do/byoc.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (6)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)DeployResponse(1608-1614)DeployResponse(1627-1627)DeployResponse(1642-1644)
src/pkg/cli/client/playground.go (1)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)
src/pkg/cli/client/byoc/gcp/byoc.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (6)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)DeployResponse(1608-1614)DeployResponse(1627-1627)DeployResponse(1642-1644)
src/pkg/cli/cd.go (4)
src/pkg/cli/client/provider.go (3)
CdCommand(20-20)CdCommandRequest(31-36)Provider(63-89)src/protos/io/defang/v1/fabric.pb.go (1)
DeploymentAction_DEPLOYMENT_ACTION_DOWN(428-428)src/pkg/types/etag.go (1)
ETag(9-9)src/pkg/cli/client/client.go (1)
FabricClient(11-39)
src/pkg/cli/composeUp.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (3)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: go-test
🔇 Additional comments (26)
src/pkg/cli/client/provider.go (2)
31-36: LGTM!The addition of
StatesUrlandEventsUrlfields toCdCommandRequestis straightforward and aligns with the PR objective to pass upload URLs to the CD pipeline.
71-71: LGTM!The
Providerinterface methods now accept*DeployRequestinstead of*defangv1.DeployRequest, enabling propagation of upload URLs through the deployment flow. This is a breaking change but aligns with the PR objectives and implementing providers have been updated accordingly.Also applies to: 80-80
src/pkg/cli/common.go (2)
42-50: LGTM!The addition of
StatesUrlandEventsUrltoputDeploymentParamscorrectly extends the deployment metadata structure.
52-75: LGTM!The URL fields are properly propagated to the
PutDeploymentrequest, ensuring deployment records include the upload URLs.src/pkg/cli/client/client.go (1)
16-16: LGTM!The
CreateUploadURLmethod addition to theFabricClientinterface follows existing patterns and enables upload URL generation via the Fabric API.src/pkg/cli/client/grpc.go (1)
115-117: LGTM!The
CreateUploadURLimplementation correctly follows the existing RPC wrapper pattern used throughout the file, delegating to the gRPC client and using thegetMsghelper for response extraction.src/pkg/cli/client/byoc/aws/validation_test.go (2)
8-8: LGTM!The import addition supports the test updates to use the new
client.DeployRequestwrapper type.
135-148: LGTM!The test cases have been correctly updated to use
client.DeployRequestwrapper with the embeddeddefangv1.DeployRequest. The syntaxclient.DeployRequest{DeployRequest: defangv1.DeployRequest{...}}properly constructs the wrapper while leavingStatesUrlandEventsUrlas empty strings (appropriate for these tests).Also applies to: 158-171, 180-193
src/pkg/cli/composeUp_test.go (1)
28-52: LGTM!The mock implementations correctly adapt to the new
*client.DeployRequestwrapper type. TheDeploymethod properly delegates toPreview, andPreviewcorrectly accesses the embeddedDeployRequestfields likeCompose.src/pkg/cli/client/mock.go (1)
171-183: LGTM!The mock implementation correctly builds a URL path from the request fields using
path.Join. The path construction prioritizesFilenameoverDigestwhen present, and properly prefixes withStackandProjectwhen available.src/pkg/cli/client/playground.go (1)
29-43: LGTM!The
PlaygroundProvidercorrectly adapts to the new*DeployRequestwrapper type.Deployproperly extracts the embeddeddefangv1.DeployRequestwhen calling the fabric client, andPreviewcorrectly sets thePreviewflag before delegating. TheStatesUrl/EventsUrlfields are appropriately unused here since Playground doesn't need upload URLs.src/pkg/cli/composeUp.go (3)
88-95: LGTM!The
DeployRequestwrapper is correctly constructed with the embeddeddefangv1.DeployRequestfields properly initialized. This aligns with the new wrapper type defined insrc/pkg/cli/client/provider.go.
130-137: LGTM!The type assertion correctly identifies non-Playground providers to retrieve upload URLs. The URLs are properly assigned to the
deployRequestfor use in the deploy flow.
152-154: LGTM!The
StatesUrlandEventsUrlare correctly passed toputDeploymentParamsfor deployment metadata recording.src/pkg/cli/client/byoc/gcp/byoc.go (3)
334-338: LGTM!The
CdCommandcorrectly extractsStatesUrlandEventsUrlfrom the request and passes them to thecdCommandstruct. The struct fields are properly added to carry these URLs through the command execution flow.Also applies to: 346-354
391-397: LGTM!The environment variables
DEFANG_STATES_UPLOAD_URLandDEFANG_EVENTS_UPLOAD_URLare correctly set only when the respective URLs are non-empty, preventing empty environment variable injection.
439-445: LGTM!The
DeployandPreviewmethod signatures correctly accept*client.DeployRequest, and the internaldeploymethod properly extracts and forwardsStatesUrlandEventsUrlto the CD command construction.Also applies to: 482-482, 539-540
src/pkg/cli/client/byoc/aws/byoc.go (3)
175-183: LGTM!The
Deploy,Preview, and internaldeploymethod signatures correctly use*client.DeployRequest. TheStatesUrlandEventsUrlare properly extracted and forwarded to the CD command construction.Also applies to: 249-250
507-510: LGTM!The
cdCommandstruct correctly includes the URL fields, and the environment variables are conditionally set only when URLs are non-empty, consistent with the GCP implementation.Also applies to: 550-557
921-924: LGTM!The
CdCommandmethod correctly extracts and forwardsStatesUrlandEventsUrlfrom the request to the CD command execution.src/pkg/cli/cd.go (4)
31-40: LGTM!The URL retrieval correctly skips Playground providers and retrieves upload URLs before executing the CD command. The URLs are properly passed to
CdCommandRequest.
45-63: Verify the error handling flow for putDeployment.The
putDeploymentcall now happens beforepostDown, and failures inputDeploymentreturn early (line 54), whilepostDownfailures only log a warning and continue. This asymmetry may be intentional, but consider whether a failed deployment history update should block the operation.
67-82: LGTM!The refactored
postDownfunction now focuses solely on subdomain zone cleanup, with deployment history updates handled separately byputDeployment. The error handling correctly logs and warns on failures without blocking the operation.
160-187: LGTM!The
GetStatesAndEventsUploadUrlsfunction correctly:
- Checks environment variable overrides first (
DEFANG_STATES_UPLOAD_URL,DEFANG_EVENTS_UPLOAD_URL)- Independently retrieves missing URLs from Fabric
- Provides descriptive error wrapping for failures
This allows flexible testing and debugging via environment variables while falling back to Fabric-generated URLs in production.
src/pkg/cli/client/byoc/do/byoc.go (2)
190-197: Good propagation ofStatesUrl/EventsUrlinto the CD invocation.
Wiring looks consistent with the PR goal; main remaining concern is treating these URLs as sensitive when passed via env (seerunCdCommand).
226-238:CdCommandnow threads upload URLs too — please confirm semantics for “dummy.domain”.
If CD relies onDOMAINfor anything beyond deploy/preview,delegateDomain: "dummy.domain"may cause surprises now that the command also depends on additional env inputs.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pkg/cli/common.go (1)
42-50: Do not persist pre-signed upload URLs in deployment records.
StatesUrl/EventsUrlare created on-demand viaCreateUploadURL()and used during CD execution, but then persisted indefangv1.Deployment. Since these are pre-signed URLs with limited TTL, storing them in the deployment record extends their exposure unnecessarily—they can leak write access through deployment logs, storage, and UI views long after the CD run completes.These URLs serve only the active CD phase; omit them from the persisted deployment record:
Proposed change
Deployment: &defangv1.Deployment{ Action: req.Action, Id: req.ETag, Project: req.ProjectName, Provider: accountInfo.Provider.Value(), ProviderAccountId: accountInfo.AccountID, ProviderString: string(accountInfo.Provider), Region: accountInfo.Region, ServiceCount: int32(req.ServiceCount), Stack: provider.GetStackName(), Timestamp: timestamppb.Now(), Mode: req.Mode, - StatesUrl: req.StatesUrl, - EventsUrl: req.EventsUrl, },
🧹 Nitpick comments (1)
src/pkg/cli/client/byoc/gcp/byoc.go (1)
329-337: Consider centralizing the env var key names to avoid drift across providers.Right now the strings are duplicated (GCP/DO/etc). A small shared const (e.g., in
src/pkg/cli/clientor abyochelper) would reduce typo risk.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/pkg/cli/client/byoc/aws/byoc_integration_test.gosrc/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/common.go
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2026-01-07T00:34:13.131Z
Learnt from: lionello
Repo: DefangLabs/defang PR: 1742
File: src/pkg/cli/composeDown.go:14-18
Timestamp: 2026-01-07T00:34:13.131Z
Learning: In Defang's Defang CLI, CdCommandDown performs refresh + destroy, while CdCommandDestroy performs destroy only (no refresh). Update ComposeDown (src/pkg/cli/composeDown.go) to call CdCommandDestroy to perform destruction without refreshing. This ensures the intended semantics are preserved when tearing down compositions; avoid using CdCommandDown in ComposeDown unless a refresh is explicitly desired. Verify that ComposeDown's destroy path does not trigger a refresh side effect from CdCommandDown and that tests cover both pathways if they exist.
Applied to files:
src/pkg/cli/common.gosrc/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/client/byoc/aws/byoc_integration_test.go
📚 Learning: 2026-01-09T20:12:21.986Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/cli/client/byoc/gcp/byoc.go:30-30
Timestamp: 2026-01-09T20:12:21.986Z
Learning: In Go files, recognize and accept the import path go.yaml.in/yaml/v3 as the maintained fork of the YAML library. Do not flag this import as incorrect; this fork supersedes the archived gopkg.in/yaml.v3 path. If you encounter this or similar forked import paths, treat them as valid Go imports and do not raise review flags.
Applied to files:
src/pkg/cli/common.gosrc/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/client/byoc/aws/byoc_integration_test.go
📚 Learning: 2026-01-07T03:07:48.228Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/cli/client/byoc/gcp/byoc.go:448-450
Timestamp: 2026-01-07T03:07:48.228Z
Learning: In src/pkg/cli/client/byoc/gcp/byoc.go, the GetDeploymentStatus method intentionally does not pre-validate b.cdExecution before calling b.driver.GetBuildStatus. If b.cdExecution is empty, it represents an error state that will be surfaced by the GCP API as an "invalid operation name" error, which is the intended behavior.
Applied to files:
src/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/client/byoc/aws/byoc_integration_test.go
📚 Learning: 2026-01-09T20:31:15.468Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/cli/client/byoc/gcp/stream.go:497-512
Timestamp: 2026-01-09T20:31:15.468Z
Learning: In src/pkg/cli/client/byoc/gcp/stream.go, the getReadyServicesCompletedResps helper function intentionally uses variable shadowing. The loop variable `status` from `readyServices` map represents individual service statuses, while the function parameter (to be renamed `cdStatus`) represents only the CD service (defangCD) completion status. Each ready service should retain its original status from the map.
Applied to files:
src/pkg/cli/client/byoc/do/byoc.go
📚 Learning: 2026-01-09T20:18:59.234Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/clouds/gcp/cloudbuild.go:62-86
Timestamp: 2026-01-09T20:18:59.234Z
Learning: In src/pkg/clouds/gcp/cloudbuild.go, BuildTag.Parse should fail on unexpected tag formats (tags that don't have 3-4 underscore-separated parts or aren't DefangCDBuildTag) because build tags are strictly controlled and only created in two places: (1) running CD in cloudbuild by CLI, and (2) building images by CD. Unexpected tags indicate an error case.
Applied to files:
src/pkg/cli/client/byoc/do/byoc.go
📚 Learning: 2026-01-09T20:31:15.468Z
Learnt from: edwardrf
Repo: DefangLabs/defang PR: 1747
File: src/pkg/cli/client/byoc/gcp/stream.go:497-512
Timestamp: 2026-01-09T20:31:15.468Z
Learning: Pattern: Apply to all Go files under the byoc/gcp client code. Enriched instruction: In Go, avoid variable shadowing when iterating maps or ranges. In getReadyServicesCompletedResps, the loop variable derived from the readyServices map should not shadow the function parameter that represents the CD service completion status. Use distinct names (for example, use serviceStatus for the map value and cdStatus for the function parameter) and preserve each ready service's original status from the map. This prevents overwriting the parameter and ensures per-service statuses are kept intact during processing.
Applied to files:
src/pkg/cli/client/byoc/gcp/byoc.go
📚 Learning: 2025-12-25T04:38:40.356Z
Learnt from: lionello
Repo: DefangLabs/defang PR: 1734
File: src/cmd/cli/command/commands.go:1220-1226
Timestamp: 2025-12-25T04:38:40.356Z
Learning: In the Defang CLI codebase (src/cmd/cli/command/commands.go), empty project names (from inputs like "/stack" or when --project-name is omitted) are acceptable and intentional behavior, as they work similarly to not providing the --project-name flag at all.
Applied to files:
src/pkg/cli/client/byoc/aws/byoc_integration_test.go
📚 Learning: 2025-12-31T13:47:12.225Z
Learnt from: lionello
Repo: DefangLabs/defang PR: 1740
File: src/pkg/cli/client/byoc/parse_test.go:18-21
Timestamp: 2025-12-31T13:47:12.225Z
Learning: In Go test files (any _test.go under the Defang codebase), it's acceptable for mocks to panic to surface issues quickly during tests. Do not add defensive error handling in mocks within tests, since panics will fail fast and highlight problems. Ensure this behavior is confined to test code and does not affect production code or non-test paths.
Applied to files:
src/pkg/cli/client/byoc/aws/byoc_integration_test.go
🧬 Code graph analysis (3)
src/pkg/cli/client/byoc/do/byoc.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (6)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)DeployResponse(1608-1614)DeployResponse(1627-1627)DeployResponse(1642-1644)
src/pkg/cli/client/byoc/gcp/byoc.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (6)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)DeployResponse(1608-1614)DeployResponse(1627-1627)DeployResponse(1642-1644)
src/pkg/cli/client/byoc/aws/byoc_integration_test.go (2)
src/pkg/cli/client/provider.go (1)
DeployRequest(38-43)src/protos/io/defang/v1/fabric.pb.go (3)
DeployRequest(1512-1525)DeployRequest(1538-1538)DeployRequest(1553-1555)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (go)
- GitHub Check: go-test
🔇 Additional comments (5)
src/pkg/cli/client/byoc/gcp/byoc.go (2)
316-321: Nice propagation of StatesUrl/EventsUrl through CdCommand.Threading these through
cdCommandkeeps the plumbing localized and avoids leaking concerns into unrelated call sites.
453-459: Deploy/Preview signature update + URL threading looks consistent.The helper
deploy()now receives*client.DeployRequestand forwardsreq.StatesUrl/req.EventsUrlto the CD environment as expected.Also applies to: 465-465, 516-524
src/pkg/cli/client/byoc/aws/byoc_integration_test.go (1)
10-10: Test updated cleanly for the new DeployRequest wrapper.This aligns the integration test with the updated provider signature while keeping the payload unchanged.
Also applies to: 25-27
src/pkg/cli/client/byoc/do/byoc.go (2)
129-137: DO provider plumbing for StatesUrl/EventsUrl looks correct.The new request type is adopted consistently and URLs are passed through both Deploy and CdCommand paths.
Also applies to: 190-197, 226-237
607-614: Good: upload URLs are stored as DO “secret” env vars; watch debug output.Using
godo.AppVariableType_Secretis the right direction for capability URLs. One thing to double-check:term.DoDebug()builds aKEY=VALUElist from all env vars (including secrets), so running with debug enabled can still leak these URLs locally.Also applies to: 623-630
Description
Create states and events upload URL and pass them down to CD for non-playground deployments
Requires https://github.com/DefangLabs/defang-mvp/pull/2583 to be merged and deployed first.
Linked Issues
https://github.com/DefangLabs/defang-mvp/issues/2562
Checklist
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.