feat: add --force to 'cd setup' to allow CD downgrades#1989
feat: add --force to 'cd setup' to allow CD downgrades#1989jordanstephens merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughThis PR introduces a "force" parameter throughout the CD installation flow to bypass CloudFormation template version mismatch errors, while simultaneously removing the entire crun package (local and Docker-based cloud runners) and its command-line entrypoint. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cmd/cli/command/cd.go (1)
198-203:⚠️ Potential issue | 🟠 MajorPass the
--forceflag tocanIUseProviderto enable downgrade path.Line 198 hardcodes
allowUpgrade=false, which pins the version to previously deployed builds viaresolveVersion()(caniuse.go:80). The--forceflag (read at line 202) is meant to "allow downgrades" but is only passed toInstallCD()—not to the pre-check. Readforcebefore the guard and pass it asallowUpgradetocanIUseProvider():Suggested fix:
force, _ := cmd.Flags().GetBool("force") if err := canIUseProvider(ctx, session.Provider, "", 0, force); err != nil { return err } return cli.InstallCD(ctx, session.Provider, force)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/cli/command/cd.go` around lines 198 - 203, Read the --force flag before the provider pre-check and pass it into canIUseProvider so downgrades are allowed during the guard; specifically, call cmd.Flags().GetBool("force") before invoking canIUseProvider(ctx, session.Provider, "", 0, ...) and pass that boolean as the allowUpgrade argument, then continue to call cli.InstallCD(ctx, session.Provider, force) as before; this touches canIUseProvider, InstallCD and the force variable used instead of the hardcoded false that interacts with resolveVersion().
🧹 Nitpick comments (1)
src/pkg/clouds/aws/ecs/cfn/setup_test.go (1)
37-37: Test updated correctly for the new signature.Consider adding a test case with
force=trueto verify the forced downgrade behavior. The PR description mentions recovering from accidental upgrades as a use case, which would benefit from explicit test coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pkg/clouds/aws/ecs/cfn/setup_test.go` at line 37, Add a new test case invoking aws.SetUp(ctx, testContainers, true) alongside the existing call to aws.SetUp(..., false) to validate forced-downgrade behavior; in that case mock or arrange the container state to simulate an upgraded version, call aws.SetUp with force=true, and assert the expected outcome (no error and that the downgrade path/state-change was executed) using the same test helpers/mocks as the current test so the behavior is explicitly verified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/cmd/cli/command/cd.go`:
- Around line 198-203: Read the --force flag before the provider pre-check and
pass it into canIUseProvider so downgrades are allowed during the guard;
specifically, call cmd.Flags().GetBool("force") before invoking
canIUseProvider(ctx, session.Provider, "", 0, ...) and pass that boolean as the
allowUpgrade argument, then continue to call cli.InstallCD(ctx,
session.Provider, force) as before; this touches canIUseProvider, InstallCD and
the force variable used instead of the hardcoded false that interacts with
resolveVersion().
---
Nitpick comments:
In `@src/pkg/clouds/aws/ecs/cfn/setup_test.go`:
- Line 37: Add a new test case invoking aws.SetUp(ctx, testContainers, true)
alongside the existing call to aws.SetUp(..., false) to validate
forced-downgrade behavior; in that case mock or arrange the container state to
simulate an upgraded version, call aws.SetUp with force=true, and assert the
expected outcome (no error and that the downgrade path/state-change was
executed) using the same test helpers/mocks as the current test so the behavior
is explicitly verified.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87359425-2b6b-4ee6-82ed-cd25ea1d75f0
⛔ Files ignored due to path filters (1)
src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (34)
Makefilepkgs/defang/cli.nixsrc/cmd/cli/command/cd.gosrc/cmd/cli/command/commands.gosrc/cmd/crun/main.gosrc/go.modsrc/pkg/cli/client/byoc/aws/byoc.gosrc/pkg/cli/client/byoc/do/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc.gosrc/pkg/cli/client/byoc/gcp/byoc_test.gosrc/pkg/cli/client/playground.gosrc/pkg/cli/client/provider.gosrc/pkg/cli/install_cd.gosrc/pkg/clouds/aws/ecs/cfn/setup.gosrc/pkg/clouds/aws/ecs/cfn/setup_test.gosrc/pkg/clouds/driver.gosrc/pkg/crun/common.gosrc/pkg/crun/common_test.gosrc/pkg/crun/destroy.gosrc/pkg/crun/docker/common.gosrc/pkg/crun/docker/info.gosrc/pkg/crun/docker/run.gosrc/pkg/crun/docker/run_test.gosrc/pkg/crun/docker/setup.gosrc/pkg/crun/docker/stop.gosrc/pkg/crun/docker/tail.gosrc/pkg/crun/docker/teardown.gosrc/pkg/crun/factory.gosrc/pkg/crun/info.gosrc/pkg/crun/local/local.gosrc/pkg/crun/local/local_test.gosrc/pkg/crun/logs.gosrc/pkg/crun/run.gosrc/pkg/crun/stop.go
💤 Files with no reviewable changes (19)
- src/cmd/crun/main.go
- src/pkg/crun/logs.go
- src/pkg/crun/docker/run_test.go
- src/pkg/crun/destroy.go
- src/pkg/crun/docker/info.go
- src/pkg/crun/docker/teardown.go
- src/pkg/crun/docker/stop.go
- src/pkg/crun/common.go
- src/pkg/crun/stop.go
- src/pkg/crun/info.go
- src/pkg/crun/factory.go
- src/pkg/crun/docker/tail.go
- src/pkg/crun/docker/common.go
- src/pkg/crun/run.go
- src/pkg/crun/docker/setup.go
- src/pkg/crun/docker/run.go
- src/pkg/crun/common_test.go
- src/pkg/crun/local/local.go
- src/pkg/crun/local/local_test.go
Description
defang cd setup --forceallow one to update the CloudFormation template in an AWS account to match the CLI's. This is useful if the stack got accidentally upgraded by a nightly build, or the CLI must be downgrade to account for a regression, but the CloudFormation stack was too new.We should release a new CLI with this flag before merging #1984
Also removing dead
cruncode to avoid having to update it for this PR.Linked Issues
Checklist
Summary by CodeRabbit
New Features
Removed Features
Chores