From aaa71224c27055d5785c2ed053ef955b7b27806e Mon Sep 17 00:00:00 2001 From: Sean Keever <33592180+swkeever@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:25:19 -0500 Subject: [PATCH 1/3] feat(git): turn applying database migrations on push on or off Adds 'volcano git migrations on|off', which sets the project's apply_migrations Git deploy setting. Turning it on asks for confirmation (--yes skips it), since anyone who can push to the production branch can then run any SQL as the database owner. 'volcano git status' and the deploy summary now report it. --- docs/git.md | 34 +++++- internal/cmd/git/deploy_settings.go | 10 ++ internal/cmd/git/git.go | 1 + internal/cmd/git/helpers_test.go | 8 +- internal/cmd/git/migrations.go | 74 +++++++++++++ internal/cmd/git/migrations_test.go | 155 ++++++++++++++++++++++++++++ internal/cmd/git/status_test.go | 14 +++ internal/gitconnect/gitconnect.go | 10 ++ internal/output/git.go | 30 +++++- tests/e2e/localmode/git_test.go | 1 + 10 files changed, 328 insertions(+), 9 deletions(-) create mode 100644 internal/cmd/git/migrations.go create mode 100644 internal/cmd/git/migrations_test.go diff --git a/docs/git.md b/docs/git.md index 946108da..82930112 100644 --- a/docs/git.md +++ b/docs/git.md @@ -28,7 +28,7 @@ into an empty repository; Volcano pushes that first commit itself. - `root-directory` changes only the subdirectory the connected repository builds from, and works with a project access token, so a CI job or an agent can use it. -- `production-branch`, `auto-deploy`, `functions` and `frontend` each change +- `production-branch`, `auto-deploy`, `functions`, `migrations` and `frontend` each change one thing about what a push deploys, and also work with a project access token. - What a push actually deploys is governed by the project's Git deploy settings, @@ -47,6 +47,7 @@ into an empty repository; Volcano pushes that first commit itself. | Change the branch that deploys | `volcano git production-branch ` | | Turn deploying on push on or off | `volcano git auto-deploy on\|off` | | Choose whether functions deploy | `volcano git functions on\|off` | +| Choose whether a push applies database migrations | `volcano git migrations on\|off [--yes]` | | Choose the frontend a push deploys | `volcano git frontend [--root ]` | | Export the project's source to an empty repository | `volcano git export [--branch ]` | | Cancel an incomplete export | `volcano git export cancel` | @@ -61,7 +62,8 @@ volcano git status This reports the project's own binding and every Git deploy setting: the repository, the branch a push has to land on and whether it follows the repository's default branch or is pinned, the subdirectory it builds from, -whether auto-deploy is on, whether functions deploy, and the frontend with the +whether auto-deploy is on, whether functions deploy, whether a push applies +database migrations, and the frontend with the directory it builds from. It also reports the project's source of truth: Volcano, or the repository once an export has handed it over. It ends with what a push deploys. It does @@ -279,6 +281,30 @@ Turning either setting off removes nothing: what is already deployed stays up. The other deploy settings stay as they were, and the command says what a push deploys from now on. Both work with a project access token. +## Applying database migrations on push + +```bash +volcano git migrations on # asks for confirmation; --yes skips it +volcano git migrations off +``` + +While it is on, each push to the production branch applies the commit's +pending migrations, the SQL files under `volcano/migrations`, before anything +deploys. Each database gets a tracked migration run; when one fails, nothing +from the push deploys and the live version keeps serving. It is off until you +turn it on. + +Anyone who can push to the production branch can then run any SQL as the +database owner, so protect that branch first. Volcano applies only the files +its migration history does not list yet: if some were already applied another +way, record them first with `volcano databases migration baseline`. See +[Database migrations](databases.md) for the files and runs. + +Turning it off undoes nothing: applied migrations stay, and pushes stop +applying new ones. The other settings change nothing about it, and it changes +nothing about them. Where database migrations are not available, turning it on +fails and saves nothing. + ## Choosing the frontend a push deploys A connected repository deploys its functions on push. To deploy a frontend from @@ -399,8 +425,8 @@ integration is not configured. Run these commands against the cloud API. `volcano git disconnect` reads only the project's own binding, which does not touch GitHub, so it reports that the project has nothing connected instead. -`volcano git root-directory`, `production-branch`, `auto-deploy`, `functions` -and `frontend` report that the integration is not configured, since no +`volcano git root-directory`, `production-branch`, `auto-deploy`, `functions`, +`migrations` and `frontend` report that the integration is not configured, since no repository can be connected there for them to change. `volcano git export` reports that source export is not available: local mode serves functions from a directory on your machine, so Volcano stores no source to export. diff --git a/internal/cmd/git/deploy_settings.go b/internal/cmd/git/deploy_settings.go index 1a198b71..f0d77d21 100644 --- a/internal/cmd/git/deploy_settings.go +++ b/internal/cmd/git/deploy_settings.go @@ -20,6 +20,9 @@ type deploySetting struct { subcommand string change func(context.Context, gitconnect.Service) (*apiclient.ProjectGitDeploySettings, error) render func(io.Writer, output.GitBinding) + // confirm, when set, is asked once the repository is known and before + // anything is saved. Declining saves nothing and is not an error. + confirm func(*apiclient.ProjectGitConnection) (bool, error) } // runDeploySetting changes one deploy setting on a project with a repository @@ -36,6 +39,13 @@ func runDeploySetting(ctx context.Context, deps cliruntime.Deps, out io.Writer, return err } + if setting.confirm != nil { + confirmed, err := setting.confirm(connection) + if err != nil || !confirmed { + return err + } + } + settings, err := setting.change(ctx, service) if err != nil { return guide(deps, err) diff --git a/internal/cmd/git/git.go b/internal/cmd/git/git.go index f5d52896..71aa41c3 100644 --- a/internal/cmd/git/git.go +++ b/internal/cmd/git/git.go @@ -33,6 +33,7 @@ an empty repository once; Volcano pushes that first commit itself.`, cmd.AddCommand(newProductionBranch(deps)) cmd.AddCommand(newAutoDeploy(deps)) cmd.AddCommand(newFunctions(deps)) + cmd.AddCommand(newMigrations(deps)) cmd.AddCommand(newFrontend(deps)) cmd.AddCommand(newExport(deps)) cmd.AddCommand(newDisconnect(deps)) diff --git a/internal/cmd/git/helpers_test.go b/internal/cmd/git/helpers_test.go index 445bdd56..5176fd44 100644 --- a/internal/cmd/git/helpers_test.go +++ b/internal/cmd/git/helpers_test.go @@ -189,6 +189,7 @@ type gitAPI struct { autoDeploy bool deployFunctions bool + applyMigrations bool frontend string frontendRoot string // providerStatus overrides the response on the routes that reach the git @@ -549,7 +550,8 @@ func (a *gitAPI) serveDeploySettings(w http.ResponseWriter) { } // serveUpdateDeploySettings replaces every deploy setting with what it was -// sent, as the platform does: a field left out is cleared, not kept. +// sent, as the platform does: a field left out is cleared, not kept, except +// apply_migrations, which keeps its saved value when left out. func (a *gitAPI) serveUpdateDeploySettings(w http.ResponseWriter, r *http.Request) { a.mu.Lock() defer a.mu.Unlock() @@ -563,6 +565,9 @@ func (a *gitAPI) serveUpdateDeploySettings(w http.ResponseWriter, r *http.Reques a.deployFunctions, _ = a.settingsBody["deploy_functions"].(bool) a.frontend, _ = a.settingsBody["frontend_name"].(string) a.frontendRoot, _ = a.settingsBody["frontend_app_root"].(string) + if apply, ok := a.settingsBody["apply_migrations"].(bool); ok { + a.applyMigrations = apply + } writeGitJSON(a.t, w, http.StatusOK, a.deploySettings()) } @@ -570,6 +575,7 @@ func (a *gitAPI) deploySettings() map[string]any { settings := map[string]any{ "auto_deploy_enabled": a.autoDeploy, "deploy_functions": a.deployFunctions, + "apply_migrations": a.applyMigrations, "updated_at": "2026-08-18T00:00:00Z", } if a.frontend != "" { diff --git a/internal/cmd/git/migrations.go b/internal/cmd/git/migrations.go new file mode 100644 index 00000000..b67f1e99 --- /dev/null +++ b/internal/cmd/git/migrations.go @@ -0,0 +1,74 @@ +package git + +import ( + "context" + "fmt" + + "github.com/spf13/cobra" + + "github.com/Kong/volcano-cli/internal/apiclient" + "github.com/Kong/volcano-cli/internal/gitconnect" + "github.com/Kong/volcano-cli/internal/output" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +func newMigrations(deps cliruntime.Deps) *cobra.Command { + var yes bool + baseline := cliruntime.CommandPath(deps, "databases migration baseline") + cmd := &cobra.Command{ + Use: "migrations on|off", + Short: "Choose whether a push to the connected repository applies its database migrations", + Long: fmt.Sprintf(`Choose whether a push to the connected repository's production branch applies +the commit's pending migrations, the SQL files under volcano/migrations, before +it deploys anything. Each database gets a tracked migration run; when one fails, +nothing from the push deploys and the live version keeps serving. + +Anyone who can push to the production branch can then run any SQL as the +database owner, so protect that branch first. Turning it on asks for +confirmation; use --yes to skip the prompt. + +Volcano applies only the files its migration history does not list yet. If +some files were already applied another way, record them first: + %s + +Turning it off undoes nothing: applied migrations stay, and pushes stop +applying new ones. The other deploy settings stay as they were; see +'%s' for all of them. + +This works with a project access token, so CI or an agent can set it. The +project needs a connected repository first: see '%s'.`, + baseline, + cliruntime.CommandPath(deps, "git status"), + cliruntime.CommandPath(deps, "git connect")), + Example: " volcano git migrations on\n volcano git migrations on --yes\n volcano git migrations off", + Args: cobra.ArbitraryArgs, + ValidArgs: []string{"on", "off"}, + RunE: func(cmd *cobra.Command, args []string) error { + on, err := onOffArgument(args, "migrations") + if err != nil { + return err + } + setting := deploySetting{ + subcommand: "migrations", + change: func(ctx context.Context, service gitconnect.Service) (*apiclient.ProjectGitDeploySettings, error) { + return service.SetApplyMigrations(ctx, on) + }, + render: output.GitMigrationsSet, + } + if on { + setting.confirm = func(connection *apiclient.ProjectGitConnection) (bool, error) { + return ask(cmd.InOrStdin(), cmd.OutOrStdout(), yes, + fmt.Sprintf("Each push to %[1]s of %[2]s will apply the SQL files under volcano/migrations "+ + "to the project's databases as their owner, before it deploys. Anyone who can push "+ + "to %[1]s can then run any SQL on them, so protect that branch first.\n"+ + "Files already applied another way need a baseline first: %[3]s", + connection.ProductionBranch, connection.RepoFullName, baseline), + "Apply migrations on push?") + } + } + return runDeploySetting(cmd.Context(), deps, cmd.OutOrStdout(), setting) + }, + } + cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Skip the confirmation prompt when turning migrations on") + return cmd +} diff --git a/internal/cmd/git/migrations_test.go b/internal/cmd/git/migrations_test.go new file mode 100644 index 00000000..7721fcfd --- /dev/null +++ b/internal/cmd/git/migrations_test.go @@ -0,0 +1,155 @@ +package git + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + cliconfig "github.com/Kong/volcano-cli/internal/config" +) + +// Turning it on is confirmed first, then sends apply_migrations alongside the +// other settings exactly as they were read. +func TestMigrationsTurnOnAfterConfirmation(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.frontend = "web" + + out, err := executeGitCommand(t, api.serve(), nil, "yes\n", "migrations", "on") + require.NoError(t, err) + + assert.Contains(t, out, "Anyone who can push to main can then run any SQL on them, so protect that branch first.") + assert.Contains(t, out, "volcano databases migration baseline") + assert.Contains(t, out, "Apply migrations on push?") + assert.Equal(t, map[string]any{ + "auto_deploy_enabled": true, + "deploy_functions": true, + "frontend_name": "web", + "apply_migrations": true, + }, api.sentSettingsBody()) + assert.Contains(t, out, "Migrations turned on") + assert.Contains(t, out, "Nothing was applied. The next push to main applies its pending migrations before it deploys.") + assert.Contains(t, out, "A push to main applies its migrations, then deploys: functions, frontend web\n") +} + +func TestMigrationsOnDeclinedSavesNothing(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + + out, err := executeGitCommand(t, api.serve(), nil, "no\n", "migrations", "on") + require.NoError(t, err) + assert.Contains(t, out, "Cancelled.") + assert.Nil(t, api.sentSettingsBody()) +} + +// --yes is how CI and agents turn it on, with nobody to answer a prompt. +func TestMigrationsOnWithYesSkipsThePrompt(t *testing.T) { + setGitCommandTestHome(t) + setGitCommandTestToken(t, cliconfig.ProjectTokenPrefix+"token") + api := newGitAPI(t) + api.connected = "octo/storefront" + + out, err := executeGitCommand(t, api.serve(), nil, "", "migrations", "on", "--yes") + require.NoError(t, err) + assert.NotContains(t, out, "Apply migrations on push?") + assert.Equal(t, true, api.sentSettingsBody()["apply_migrations"]) +} + +// Turning it off needs no confirmation and undoes nothing already applied. +func TestMigrationsTurnOff(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.applyMigrations = true + + out, err := executeGitCommand(t, api.serve(), nil, "", "migrations", "off") + require.NoError(t, err) + + assert.NotContains(t, out, "Apply migrations on push?") + assert.Equal(t, map[string]any{ + "auto_deploy_enabled": true, + "deploy_functions": true, + "apply_migrations": false, + }, api.sentSettingsBody()) + assert.Contains(t, out, "Migrations turned off") + assert.Contains(t, out, "Nothing was undone. Applied migrations stay; pushes no longer apply new ones.") + assert.Contains(t, out, "A push to main deploys: functions\n") +} + +// The other setting commands leave apply_migrations out of the write, which the +// platform reads as "keep the saved value", so they never turn it off. +func TestOtherSettingsLeaveMigrationsAsSaved(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.applyMigrations = true + + out, err := executeGitCommand(t, api.serve(), nil, "", "functions", "off") + require.NoError(t, err) + assert.NotContains(t, api.sentSettingsBody(), "apply_migrations") + assert.True(t, api.applyMigrations) + assert.Contains(t, out, "neither functions nor a frontend is selected") +} + +func TestMigrationsPromiseNothingWhileAutoDeployIsOff(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.autoDeploy = false + + out, err := executeGitCommand(t, api.serve(), nil, "", "migrations", "on", "--yes") + require.NoError(t, err) + assert.NotContains(t, out, "The next push to main applies") + assert.Contains(t, out, "Auto-deploy is off, so a push to main will not deploy anything.") +} + +// Where migrations are unavailable the platform refuses turning them on; the +// CLI reports that and saves nothing else. +func TestMigrationsOnWhereTheyAreUnavailable(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.settingsWriteStatus = http.StatusBadRequest + api.settingsWriteMessage = "Database migrations are not available." + + _, err := executeGitCommand(t, api.serve(), nil, "", "migrations", "on", "--yes") + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to turn migrations on") + assert.False(t, api.applyMigrations) +} + +func TestMigrationsRefuseAnythingButOnOrOff(t *testing.T) { + for _, tc := range []struct { + args []string + want string + }{ + {[]string{"migrations"}, "say whether migrations should be on or off, such as: on"}, + {[]string{"migrations", "true"}, `migrations is either on or off, not "true"`}, + } { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + + _, err := executeGitCommand(t, api.serve(), nil, "", tc.args...) + require.Error(t, err, "%q", tc.args) + assert.Contains(t, err.Error(), tc.want, "%q", tc.args) + assert.Nil(t, api.sentSettingsBody(), "%q", tc.args) + } +} + +// Nothing connected is refused before the prompt, so nobody confirms a change +// that cannot be saved. +func TestMigrationsOnAProjectWithNothingConnected(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + + out, err := executeGitCommand(t, api.serve(), nil, "yes\n", "migrations", "on") + require.Error(t, err) + assert.Contains(t, err.Error(), "Connect one with volcano git connect") + assert.NotContains(t, out, "Apply migrations on push?") + assert.Nil(t, api.sentSettingsBody()) +} diff --git a/internal/cmd/git/status_test.go b/internal/cmd/git/status_test.go index c75715a3..bb74d519 100644 --- a/internal/cmd/git/status_test.go +++ b/internal/cmd/git/status_test.go @@ -181,10 +181,24 @@ func TestStatusListsEveryDeploySetting(t *testing.T) { assert.Contains(t, out, "Production branch: release (pinned)") assert.Contains(t, out, "Auto-deploy: on") assert.Contains(t, out, "Functions: off") + assert.Contains(t, out, "Migrations: off") assert.Contains(t, out, "Frontend: web (apps/web)") assert.Contains(t, out, "A push to release deploys: frontend web (apps/web)\n") } +func TestStatusSaysAPushAppliesMigrationsFirst(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.applyMigrations = true + + out, err := executeGitCommand(t, api.serve(), nil, "", "status") + require.NoError(t, err) + + assert.Contains(t, out, "Migrations: on") + assert.Contains(t, out, "A push to main applies its migrations, then deploys: functions\n") +} + func TestStatusSaysWhenTheBranchFollowsTheDefault(t *testing.T) { setGitCommandTestHome(t) api := newGitAPI(t) diff --git a/internal/gitconnect/gitconnect.go b/internal/gitconnect/gitconnect.go index bbcca04f..c9dbbacb 100644 --- a/internal/gitconnect/gitconnect.go +++ b/internal/gitconnect/gitconnect.go @@ -523,6 +523,16 @@ func (s Service) SetDeployFunctions(ctx context.Context, on bool) (*apiclient.Pr }) } +// SetApplyMigrations turns applying the commit's pending migrations before a +// push deploys on or off. Unlike the other settings, the platform keeps the +// saved value when the field is left out, so only this write sends it. +func (s Service) SetApplyMigrations(ctx context.Context, on bool) (*apiclient.ProjectGitDeploySettings, error) { + return s.changeDeploySettings(ctx, "failed to turn migrations "+onOff(on), + func(settings *apiclient.UpdateProjectGitDeploySettingsRequest) { + settings.ApplyMigrations = &on + }) +} + func onOff(on bool) string { if on { return "on" diff --git a/internal/output/git.go b/internal/output/git.go index fe4392c4..387e3df6 100644 --- a/internal/output/git.go +++ b/internal/output/git.go @@ -80,6 +80,7 @@ func GitStatus(w io.Writer, binding GitBinding) { if settings := binding.Settings; settings != nil { kv(w, on, "Auto-deploy", "%s", onOff(settings.AutoDeployEnabled)) kv(w, on, "Functions", "%s", onOff(settings.DeployFunctions)) + kv(w, on, "Migrations", "%s", onOff(settings.ApplyMigrations)) frontend := frontendTarget(settings) if frontend == "" { frontend = "none" @@ -247,6 +248,26 @@ func GitFunctionsSet(w io.Writer, binding GitBinding) { gitDeploySettings(w, binding) } +// GitMigrationsSet renders applying migrations on push just turned on or off, +// followed by what a push to the production branch deploys. +func GitMigrationsSet(w io.Writer, binding GitBinding) { + settings := binding.Settings + Success(w, "Migrations turned %s", onOff(settings.ApplyMigrations)) + gitBindingDetail(w, theme.On(w), binding) + switch { + case !settings.ApplyMigrations: + Note(w, "Nothing was undone. Applied migrations stay; pushes no longer apply new ones.") + case settings.AutoDeployEnabled: + Note(w, "Nothing was applied. The next push to %s applies its pending migrations before it deploys.", + binding.Connection.ProductionBranch) + default: + // Auto-deploy off: the settings below say no push deploys anything, so + // promising this one would contradict them. + Note(w, "Nothing was applied.") + } + gitDeploySettings(w, binding) +} + // GitProductionBranchSet renders the branch a push must now land on, and // whether it follows the repository's default branch or is pinned. func GitProductionBranchSet(w io.Writer, binding GitBinding) { @@ -342,10 +363,11 @@ func gitDeploySettings(w io.Writer, binding GitBinding) { Warning(w, "Auto-deploy is on, but neither functions nor a frontend is selected, so a push deploys nothing.") nextStep(w, on, binding, "Deploy functions with ", "functions", "on") } else { - fmt.Fprintf(w, "%s%s\n", - theme.Dim(fmt.Sprintf("A push to %s deploys: ", connection.ProductionBranch), on), - strings.Join(targets, ", "), - ) + lead := fmt.Sprintf("A push to %s deploys: ", connection.ProductionBranch) + if settings.ApplyMigrations { + lead = fmt.Sprintf("A push to %s applies its migrations, then deploys: ", connection.ProductionBranch) + } + fmt.Fprintf(w, "%s%s\n", theme.Dim(lead, on), strings.Join(targets, ", ")) } if frontendTarget(settings) == "" { nextStep(w, on, binding, "To deploy a frontend on push too, run ", "frontend", " --root ") diff --git a/tests/e2e/localmode/git_test.go b/tests/e2e/localmode/git_test.go index e4142c99..5a5f062e 100644 --- a/tests/e2e/localmode/git_test.go +++ b/tests/e2e/localmode/git_test.go @@ -26,6 +26,7 @@ func requireLocalModeLeavesGitToTheCloud(t *testing.T, binary string, env []stri {"git", "production-branch", "release"}, {"git", "auto-deploy", "on"}, {"git", "functions", "on"}, + {"git", "migrations", "on", "--yes"}, {"git", "frontend", "web", "--root", "apps/web"}, } { output, err := runVolcanoLocalModeE2EAllowFailure(t, binary, local, dir, args...) From d649bed56ffda342b2c4f1158ee5126cb73ce548 Mon Sep 17 00:00:00 2001 From: Sean Keever <33592180+swkeever@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:39:49 -0500 Subject: [PATCH 2/3] fix(git): recheck the binding before turning migrations on 'volcano git migrations on' now refuses to save when the repository connection changed while the confirmation was open, so a yes for one repository and branch never enables SQL-on-push for another. The deploy summary also says that a push with neither functions nor a frontend selected still applies its migrations, instead of saying it deploys nothing. --- docs/git.md | 8 +++++-- internal/cmd/git/auto_deploy.go | 4 +++- internal/cmd/git/deploy_settings.go | 10 +++++--- internal/cmd/git/frontend.go | 4 +++- internal/cmd/git/functions.go | 4 +++- internal/cmd/git/migrations.go | 6 +++-- internal/cmd/git/migrations_test.go | 36 ++++++++++++++++++++++++++++- internal/gitconnect/gitconnect.go | 13 ++++++++++- internal/output/git.go | 16 +++++++++++-- 9 files changed, 87 insertions(+), 14 deletions(-) diff --git a/docs/git.md b/docs/git.md index 82930112..63efcfd1 100644 --- a/docs/git.md +++ b/docs/git.md @@ -292,10 +292,14 @@ While it is on, each push to the production branch applies the commit's pending migrations, the SQL files under `volcano/migrations`, before anything deploys. Each database gets a tracked migration run; when one fails, nothing from the push deploys and the live version keeps serving. It is off until you -turn it on. +turn it on. Like every push deploy, it needs auto-deploy on; with neither +functions nor a frontend selected, a push applies its migrations and deploys no +code. Anyone who can push to the production branch can then run any SQL as the -database owner, so protect that branch first. Volcano applies only the files +database owner, so protect that branch first. The confirmation names the +repository and branch; if the connection changes before you answer, nothing is +saved. Volcano applies only the files its migration history does not list yet: if some were already applied another way, record them first with `volcano databases migration baseline`. See [Database migrations](databases.md) for the files and runs. diff --git a/internal/cmd/git/auto_deploy.go b/internal/cmd/git/auto_deploy.go index 144c06bf..48bffe14 100644 --- a/internal/cmd/git/auto_deploy.go +++ b/internal/cmd/git/auto_deploy.go @@ -36,7 +36,9 @@ project needs a connected repository first: see '%s'.`, } return runDeploySetting(cmd.Context(), deps, cmd.OutOrStdout(), deploySetting{ subcommand: "auto-deploy", - change: func(ctx context.Context, service gitconnect.Service) (*apiclient.ProjectGitDeploySettings, error) { + change: func( + ctx context.Context, service gitconnect.Service, _ *apiclient.ProjectGitConnection, + ) (*apiclient.ProjectGitDeploySettings, error) { return service.SetAutoDeploy(ctx, on) }, render: output.GitAutoDeploySet, diff --git a/internal/cmd/git/deploy_settings.go b/internal/cmd/git/deploy_settings.go index f0d77d21..422cf28b 100644 --- a/internal/cmd/git/deploy_settings.go +++ b/internal/cmd/git/deploy_settings.go @@ -18,8 +18,12 @@ type deploySetting struct { // subcommand is the command's own name, whose next step the output leaves // out. subcommand string - change func(context.Context, gitconnect.Service) (*apiclient.ProjectGitDeploySettings, error) - render func(io.Writer, output.GitBinding) + // change saves the setting. It is given the binding the command read and + // showed, for a change that must not land on any other. + change func( + context.Context, gitconnect.Service, *apiclient.ProjectGitConnection, + ) (*apiclient.ProjectGitDeploySettings, error) + render func(io.Writer, output.GitBinding) // confirm, when set, is asked once the repository is known and before // anything is saved. Declining saves nothing and is not an error. confirm func(*apiclient.ProjectGitConnection) (bool, error) @@ -46,7 +50,7 @@ func runDeploySetting(ctx context.Context, deps cliruntime.Deps, out io.Writer, } } - settings, err := setting.change(ctx, service) + settings, err := setting.change(ctx, service, connection) if err != nil { return guide(deps, err) } diff --git a/internal/cmd/git/frontend.go b/internal/cmd/git/frontend.go index 84d7a075..c16eefb1 100644 --- a/internal/cmd/git/frontend.go +++ b/internal/cmd/git/frontend.go @@ -100,7 +100,9 @@ func frontendArguments(args []string, root string, rootSet, none bool) (name, fr func runFrontend(ctx context.Context, opts frontendOptions) error { return runDeploySetting(ctx, opts.deps, opts.out, deploySetting{ subcommand: "frontend", - change: func(ctx context.Context, service gitconnect.Service) (*apiclient.ProjectGitDeploySettings, error) { + change: func( + ctx context.Context, service gitconnect.Service, _ *apiclient.ProjectGitConnection, + ) (*apiclient.ProjectGitDeploySettings, error) { return service.SetFrontend(ctx, opts.name, opts.root) }, render: output.GitFrontendSet, diff --git a/internal/cmd/git/functions.go b/internal/cmd/git/functions.go index bcff4037..34f1359e 100644 --- a/internal/cmd/git/functions.go +++ b/internal/cmd/git/functions.go @@ -37,7 +37,9 @@ project needs a connected repository first: see '%s'.`, } return runDeploySetting(cmd.Context(), deps, cmd.OutOrStdout(), deploySetting{ subcommand: "functions", - change: func(ctx context.Context, service gitconnect.Service) (*apiclient.ProjectGitDeploySettings, error) { + change: func( + ctx context.Context, service gitconnect.Service, _ *apiclient.ProjectGitConnection, + ) (*apiclient.ProjectGitDeploySettings, error) { return service.SetDeployFunctions(ctx, on) }, render: output.GitFunctionsSet, diff --git a/internal/cmd/git/migrations.go b/internal/cmd/git/migrations.go index b67f1e99..4eb6ffa6 100644 --- a/internal/cmd/git/migrations.go +++ b/internal/cmd/git/migrations.go @@ -50,8 +50,10 @@ project needs a connected repository first: see '%s'.`, } setting := deploySetting{ subcommand: "migrations", - change: func(ctx context.Context, service gitconnect.Service) (*apiclient.ProjectGitDeploySettings, error) { - return service.SetApplyMigrations(ctx, on) + change: func( + ctx context.Context, service gitconnect.Service, shown *apiclient.ProjectGitConnection, + ) (*apiclient.ProjectGitDeploySettings, error) { + return service.SetApplyMigrations(ctx, on, shown) }, render: output.GitMigrationsSet, } diff --git a/internal/cmd/git/migrations_test.go b/internal/cmd/git/migrations_test.go index 7721fcfd..8cc3867f 100644 --- a/internal/cmd/git/migrations_test.go +++ b/internal/cmd/git/migrations_test.go @@ -92,7 +92,41 @@ func TestOtherSettingsLeaveMigrationsAsSaved(t *testing.T) { require.NoError(t, err) assert.NotContains(t, api.sentSettingsBody(), "apply_migrations") assert.True(t, api.applyMigrations) - assert.Contains(t, out, "neither functions nor a frontend is selected") + assert.Contains(t, out, "so a push to main applies its migrations and deploys no code.") +} + +// With nothing selected to deploy, a push still applies migrations on its own, +// so the summary must not say a push deploys nothing. +func TestMigrationsOnWithNothingElseToDeploy(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.deployFunctions = false + + out, err := executeGitCommand(t, api.serve(), nil, "", "migrations", "on", "--yes") + require.NoError(t, err) + assert.Contains(t, out, "Nothing was applied. The next push to main applies its pending migrations.\n") + assert.Contains(t, out, "so a push to main applies its migrations and deploys no code.") + assert.NotContains(t, out, "a push deploys nothing") +} + +// The prompt names a repository and branch. If the binding moved while it was +// open, the user's yes would let a repository they never saw run SQL as the +// database owner. +func TestMigrationsOnRefusesWhenTheBindingMovedUnderIt(t *testing.T) { + setGitCommandTestHome(t) + api := newGitAPI(t) + api.connected = "octo/storefront" + api.connectedRepoID = gitRepositoryID + api.connectedAfterRead = "acme/something-else" + + out, err := executeGitCommand(t, api.serve(), nil, "yes\n", "migrations", "on") + require.Error(t, err) + assert.Contains(t, out, "octo/storefront") + assert.Contains(t, err.Error(), "changed while this command was running") + assert.Contains(t, err.Error(), "acme/something-else") + assert.Nil(t, api.sentSettingsBody(), "nothing may be saved for a binding the user did not confirm") + assert.False(t, api.applyMigrations) } func TestMigrationsPromiseNothingWhileAutoDeployIsOff(t *testing.T) { diff --git a/internal/gitconnect/gitconnect.go b/internal/gitconnect/gitconnect.go index c9dbbacb..e219d83c 100644 --- a/internal/gitconnect/gitconnect.go +++ b/internal/gitconnect/gitconnect.go @@ -526,7 +526,18 @@ func (s Service) SetDeployFunctions(ctx context.Context, on bool) (*apiclient.Pr // SetApplyMigrations turns applying the commit's pending migrations before a // push deploys on or off. Unlike the other settings, the platform keeps the // saved value when the field is left out, so only this write sends it. -func (s Service) SetApplyMigrations(ctx context.Context, on bool) (*apiclient.ProjectGitDeploySettings, error) { +// +// expected is the binding the caller read and showed. Turning this on lets +// whoever can push to that repository's production branch run SQL as the +// database owner, so the write is refused when the binding changed since: the +// user agreed to that repository and branch, not to whatever is bound now. +// Like Disconnect, this narrows the window rather than closing it. +func (s Service) SetApplyMigrations( + ctx context.Context, on bool, expected *apiclient.ProjectGitConnection, +) (*apiclient.ProjectGitDeploySettings, error) { + if err := s.confirmUnchanged(ctx, expected); err != nil { + return nil, err + } return s.changeDeploySettings(ctx, "failed to turn migrations "+onOff(on), func(settings *apiclient.UpdateProjectGitDeploySettingsRequest) { settings.ApplyMigrations = &on diff --git a/internal/output/git.go b/internal/output/git.go index 387e3df6..bd2b20c8 100644 --- a/internal/output/git.go +++ b/internal/output/git.go @@ -257,6 +257,11 @@ func GitMigrationsSet(w io.Writer, binding GitBinding) { switch { case !settings.ApplyMigrations: Note(w, "Nothing was undone. Applied migrations stay; pushes no longer apply new ones.") + case settings.AutoDeployEnabled && len(deployTargets(settings)) == 0: + // Nothing selected to deploy: a push still applies migrations on its + // own, which the settings below say too. + Note(w, "Nothing was applied. The next push to %s applies its pending migrations.", + binding.Connection.ProductionBranch) case settings.AutoDeployEnabled: Note(w, "Nothing was applied. The next push to %s applies its pending migrations before it deploys.", binding.Connection.ProductionBranch) @@ -359,10 +364,17 @@ func gitDeploySettings(w io.Writer, binding GitBinding) { } targets := deployTargets(settings) - if len(targets) == 0 { + switch { + case len(targets) == 0 && settings.ApplyMigrations: + // The platform runs a push with migrations and nothing else to deploy: + // it applies them and finishes with no deploys. + Warning(w, "Auto-deploy is on, but neither functions nor a frontend is selected, "+ + "so a push to %s applies its migrations and deploys no code.", connection.ProductionBranch) + nextStep(w, on, binding, "Deploy functions with ", "functions", "on") + case len(targets) == 0: Warning(w, "Auto-deploy is on, but neither functions nor a frontend is selected, so a push deploys nothing.") nextStep(w, on, binding, "Deploy functions with ", "functions", "on") - } else { + default: lead := fmt.Sprintf("A push to %s deploys: ", connection.ProductionBranch) if settings.ApplyMigrations { lead = fmt.Sprintf("A push to %s applies its migrations, then deploys: ", connection.ProductionBranch) From 74561397f5edf4e405671592c66ba477beece429 Mon Sep 17 00:00:00 2001 From: Sean Keever <33592180+swkeever@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:53:51 -0500 Subject: [PATCH 3/3] fix(git): name the cloud baseline command in the migrations prompt The top-level 'volcano databases' commands run against the local server, so following 'volcano databases migration baseline' would record history locally and leave the cloud databases unprepared. The prompt, help and docs now name 'volcano cloud databases migration baseline'. --- docs/git.md | 2 +- internal/cmd/git/migrations.go | 5 ++++- internal/cmd/git/migrations_test.go | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/git.md b/docs/git.md index 63efcfd1..90f9376a 100644 --- a/docs/git.md +++ b/docs/git.md @@ -301,7 +301,7 @@ database owner, so protect that branch first. The confirmation names the repository and branch; if the connection changes before you answer, nothing is saved. Volcano applies only the files its migration history does not list yet: if some were already applied another -way, record them first with `volcano databases migration baseline`. See +way, record them first with `volcano cloud databases migration baseline`. See [Database migrations](databases.md) for the files and runs. Turning it off undoes nothing: applied migrations stay, and pushes stop diff --git a/internal/cmd/git/migrations.go b/internal/cmd/git/migrations.go index 4eb6ffa6..313f7695 100644 --- a/internal/cmd/git/migrations.go +++ b/internal/cmd/git/migrations.go @@ -14,7 +14,10 @@ import ( func newMigrations(deps cliruntime.Deps) *cobra.Command { var yes bool - baseline := cliruntime.CommandPath(deps, "databases migration baseline") + // A push applies migrations to the cloud project's databases, so the + // baseline is the cloud one: the top-level databases commands run against + // the local server. + baseline := cliruntime.CommandPath(deps, "cloud databases migration baseline") cmd := &cobra.Command{ Use: "migrations on|off", Short: "Choose whether a push to the connected repository applies its database migrations", diff --git a/internal/cmd/git/migrations_test.go b/internal/cmd/git/migrations_test.go index 8cc3867f..23575ed4 100644 --- a/internal/cmd/git/migrations_test.go +++ b/internal/cmd/git/migrations_test.go @@ -22,7 +22,7 @@ func TestMigrationsTurnOnAfterConfirmation(t *testing.T) { require.NoError(t, err) assert.Contains(t, out, "Anyone who can push to main can then run any SQL on them, so protect that branch first.") - assert.Contains(t, out, "volcano databases migration baseline") + assert.Contains(t, out, "volcano cloud databases migration baseline") assert.Contains(t, out, "Apply migrations on push?") assert.Equal(t, map[string]any{ "auto_deploy_enabled": true,