From 081897ca90f49730d628791f0d94aa4bd720aa06 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 22:36:41 -0500 Subject: [PATCH 01/11] feat(databases): apply migrations through migration runs under a grant --- cmd/volcano/grant_test.go | 40 +++ docs/databases.md | 6 + internal/api/database_migration_runs.go | 79 +++++ internal/cmd/databases/migration/migration.go | 13 +- internal/cmd/databases/migration/runs.go | 237 ++++++++++++++ internal/cmd/databases/migration/runs_test.go | 305 ++++++++++++++++++ 6 files changed, 679 insertions(+), 1 deletion(-) create mode 100644 internal/api/database_migration_runs.go create mode 100644 internal/cmd/databases/migration/runs.go create mode 100644 internal/cmd/databases/migration/runs_test.go diff --git a/cmd/volcano/grant_test.go b/cmd/volcano/grant_test.go index 3e0aa71a..2e9d5dc1 100644 --- a/cmd/volcano/grant_test.go +++ b/cmd/volcano/grant_test.go @@ -37,6 +37,12 @@ type grantRun struct { // directory's link file, telemetry off and no prompts. No config loader is // injected, so the CLI reads its configuration the way it does in the VM. func runAsWorker(t *testing.T, handler http.HandlerFunc, args ...string) grantRun { + t.Helper() + return runAsWorkerWith(t, nil, handler, args...) +} + +// runAsWorkerWith is runAsWorker with setup run in the workspace first. +func runAsWorkerWith(t *testing.T, setup func(), handler http.HandlerFunc, args ...string) grantRun { t.Helper() resetInstructions(t) api.ResetLastRefusalForTest() @@ -65,6 +71,9 @@ func runAsWorker(t *testing.T, handler http.HandlerFunc, args ...string) grantRu t.Setenv("VOLCANO_PROJECT_ID", "") t.Setenv("VOLCANO_TELEMETRY_DISABLED", "1") t.Setenv("VOLCANO_NONINTERACTIVE", "1") + if setup != nil { + setup() + } deps := cliruntime.Deps{HTTPClient: server.Client()} root := rootcmd.New(deps) @@ -225,3 +234,34 @@ func TestGrantRefusesStorageObjectCommandsBeforeAnyRequest(t *testing.T) { assert.Contains(t, run.stderr, "storage object commands do not work with a delegated grant (pg-)") assert.Empty(t, run.requests) } + +// Migrations under a grant go through a server-side run. The CLI never reads +// the database, whose answer holds the connection string. +func TestGrantMigrationUpUsesAMigrationRun(t *testing.T) { + runsPath := "/projects/" + mainTestProjectAlpha + "/databases/app/migration-runs" + const runID = "44444444-4444-4444-8444-444444444444" + run := runAsWorkerWith(t, func() { + require.NoError(t, os.MkdirAll(filepath.Join("volcano", "migrations"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "001_init.sql"), []byte("CREATE TABLE t();"), 0o644)) + }, func(w http.ResponseWriter, r *http.Request) { + state := `{"id":"` + runID + `","status":"succeeded","phase":"apply","source":"api","warnings":[],` + + `"database":{"id":"66666666-6666-4666-8666-666666666666","name":"app"},` + + `"files":[{"name":"001_init.sql","outcome":"applied"}],` + + `"created_at":"2026-11-11T00:00:00Z","updated_at":"2026-11-11T00:00:00Z"}` + switch { + case r.Method == http.MethodPost && r.URL.Path == runsPath: + respondExitTestJSON(w, http.StatusAccepted, state) + case r.Method == http.MethodGet && r.URL.Path == runsPath+"/"+runID: + respondExitTestJSON(w, http.StatusOK, state) + default: + respondExitTestJSON(w, http.StatusForbidden, `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`) + } + }, "cloud", "databases", "migration", "up", "--all", "-d", "app") + + assert.Equal(t, 0, run.code, run.stderr) + assert.Equal(t, []string{"POST " + runsPath}, run.requests) + assert.Contains(t, run.stdout, "001_init.sql: applied") + for _, request := range run.requests { + assert.NotEqual(t, "GET /projects/"+mainTestProjectAlpha+"/databases/app", request) + } +} diff --git a/docs/databases.md b/docs/databases.md index 3dc92402..36919996 100644 --- a/docs/databases.md +++ b/docs/databases.md @@ -37,6 +37,12 @@ database proxy; the local `migrations deploy` command does not have a cloud variant. [`volcano deploy --with-migrations`](deploy.md) runs the same migrations as part of a whole-project deploy. +With a [delegated grant](authentication.md#delegated-grants) (`pg-`), `volcano cloud +databases migration up` sends the files to a server-side migration run instead +of connecting to the database. The run skips files the database's migration +history already records, and applies the rest after a rehearsal and a backup. +Where migration runs are not available, the command fails and applies nothing. + Each file runs as a single query, and the database proxy refuses a query with more than one statement, so put one SQL statement in each file. Files run in name order. diff --git a/internal/api/database_migration_runs.go b/internal/api/database_migration_runs.go new file mode 100644 index 00000000..57ac4d56 --- /dev/null +++ b/internal/api/database_migration_runs.go @@ -0,0 +1,79 @@ +package api + +import ( + "context" + "fmt" + "net/http" + "strings" + + "github.com/google/uuid" + + "github.com/Kong/volcano-cli/internal/apiclient" +) + +// MigrationRunInProgressCode is the 409 the API answers when the database +// already has a queued or running migration run. +const MigrationRunInProgressCode = "migration_run_in_progress" + +// FeatureUnavailableCode is the 404 the API answers when migration runs are +// not turned on where it runs. +const FeatureUnavailableCode = "feature_unavailable" + +// MigrationRunInProgressError is a run the API would not start because RunID +// is already queued or running on the database. +type MigrationRunInProgressError struct { + Err *Error + RunID uuid.UUID +} + +func (e *MigrationRunInProgressError) Error() string { + return fmt.Sprintf("%s (run %s)", e.Err.Error(), e.RunID) +} + +func (e *MigrationRunInProgressError) Unwrap() error { return e.Err } + +// CreateDatabaseMigrationRun starts a server-side migration run of files on +// databaseName. The server compares them with the database's history and +// applies the pending ones; nothing here connects to the database. +// idempotencyKey makes a retried request return the run the first one started, +// which the API answers with 200 instead of 202. +func (c *Client) CreateDatabaseMigrationRun( + ctx context.Context, projectID uuid.UUID, databaseName, idempotencyKey string, files []apiclient.DatabaseMigrationFile, +) (*apiclient.DatabaseMigrationRun, error) { + params := &apiclient.CreateDatabaseMigrationRunParams{} + if key := strings.TrimSpace(idempotencyKey); key != "" { + params.IdempotencyKey = &key + } + resp, err := c.client.CreateDatabaseMigrationRunWithResponse(ctx, projectID, strings.TrimSpace(databaseName), params, + apiclient.CreateDatabaseMigrationRunJSONRequestBody{Files: files}) + if err != nil { + return nil, err + } + switch { + case resp.JSON202 != nil: + return resp.JSON202, nil + case resp.JSON200 != nil: + return resp.JSON200, nil + case resp.JSON409 != nil: + apiErr := &Error{StatusCode: http.StatusConflict, Message: strings.TrimSpace(resp.JSON409.Error), fromAPI: true} + if resp.JSON409.Code != nil { + apiErr.Code = *resp.JSON409.Code + } + if apiErr.Code == MigrationRunInProgressCode && resp.JSON409.RunId != nil { + return nil, &MigrationRunInProgressError{Err: apiErr, RunID: *resp.JSON409.RunId} + } + return nil, apiErr + } + return nil, apiErrorFromGeneratedErrors(resp.StatusCode(), resp.Body, resp.JSON400, resp.JSON404, resp.JSON413) +} + +// GetDatabaseMigrationRun returns one migration run with its files. +func (c *Client) GetDatabaseMigrationRun( + ctx context.Context, projectID uuid.UUID, databaseName string, runID uuid.UUID, +) (*apiclient.DatabaseMigrationRun, error) { + resp, err := c.client.GetDatabaseMigrationRunWithResponse(ctx, projectID, strings.TrimSpace(databaseName), runID) + if err != nil { + return nil, err + } + return apiResult(resp.StatusCode(), resp.Body, resp.JSON200, resp.JSON404) +} diff --git a/internal/cmd/databases/migration/migration.go b/internal/cmd/databases/migration/migration.go index 95d5be8f..5540d7a3 100644 --- a/internal/cmd/databases/migration/migration.go +++ b/internal/cmd/databases/migration/migration.go @@ -15,6 +15,7 @@ import ( "github.com/Kong/volcano-cli/internal/apiclient" clidatabase "github.com/Kong/volcano-cli/internal/database" cliruntime "github.com/Kong/volcano-cli/internal/runtime" + clisession "github.com/Kong/volcano-cli/internal/session" ) type sqlExecutor func(context.Context, string, string) error @@ -67,7 +68,11 @@ The --database flag is required. The CLI connects directly to your database via pgproxy and executes the SQL files. WARNING: Migrations are executed without tracking. You are responsible for not -re-running non-idempotent migrations.`, +re-running non-idempotent migrations. + +With a delegated grant (pg-) in VOLCANO_TOKEN, the files go to a server-side +migration run instead: the CLI never reads the connection string, and the run +skips files the database's migration history already records.`, cliruntime.CommandPath(deps, "databases migration up --all -d mydb"), cliruntime.CommandPath(deps, "databases migration up -a -d mydb"), cliruntime.CommandPath(deps, "databases migration up -d mydb -f 001_create_users"), @@ -175,6 +180,12 @@ func runUp(ctx context.Context, opts upOptions) error { fmt.Fprintf(opts.out, "\nExecuting migration: %s\n", filepath.Base(migrationFiles[0])) } + // A delegated grant never reads the connection string: the server applies + // the files instead. See runs.go. + if sessions := clisession.NewFactory(opts.deps); usesMigrationRuns(sessions) { + return applyThroughRun(ctx, sessions, opts.database, migrationFiles, opts.out) + } + database, err := clidatabase.NewService(opts.deps).Get(ctx, opts.database) if err != nil { return err diff --git a/internal/cmd/databases/migration/runs.go b/internal/cmd/databases/migration/runs.go new file mode 100644 index 00000000..fc38b709 --- /dev/null +++ b/internal/cmd/databases/migration/runs.go @@ -0,0 +1,237 @@ +package migration + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "regexp" + "strings" + "time" + + "github.com/google/uuid" + + "github.com/Kong/volcano-cli/internal/api" + "github.com/Kong/volcano-cli/internal/apiclient" + "github.com/Kong/volcano-cli/internal/config" + clisession "github.com/Kong/volcano-cli/internal/session" +) + +// Server-side migration runs. +// +// Under a pg- delegated grant, `databases migration up` sends the files to +// Hosting's migration runs and follows the run, instead of reading the +// database's connection string and connecting to Postgres itself. A grant +// can't read a connection string (it is a credential that outlives the run), +// and the server picks the right database: in local mode the direct path used +// a fixed port, which could be another stack's database. +// +// There is no fallback to the direct path. Where migration runs are off (the +// `database_migrations` release gate), the command fails and says so. + +// runPollInterval is how often a run is read while it has not ended. Tests set +// it to zero. +var runPollInterval = 2 * time.Second + +// runWaitLimit bounds how long the command follows one run. The run carries on +// on the server after the command stops. +var runWaitLimit = 60 * time.Minute + +// maxRunReadFailures is how many reads in a row may fail on the network before +// the command stops following a run. +const maxRunReadFailures = 5 + +// trackedMigrationName is the name rule the API applies to a top-level file: +// lowercase ".sql", no directory. +var trackedMigrationName = regexp.MustCompile(`^[^/\\]+\.sql$`) + +// usesMigrationRuns reports whether up applies files through migration runs: +// when the credential is a delegated grant. +func usesMigrationRuns(sessions clisession.Factory) bool { + cfg, err := sessions.Config() + return err == nil && config.IsDelegatedGrant(cfg.Token()) +} + +// applyThroughRun applies files to database with a server-side migration run +// and follows the run to its end, printing what became of each file. +func applyThroughRun(ctx context.Context, sessions clisession.Factory, database string, files []string, out io.Writer) error { + project, err := sessions.CurrentProject() + if err != nil { + return err + } + request, err := migrationRunFiles(files) + if err != nil { + return err + } + + fmt.Fprintf(out, "\nUsing database: %s\n", database) + fmt.Fprintln(out, "Applying through a server-side migration run: files the database's history already records are skipped.") + + run, err := startRun(ctx, project, database, request, out) + if err != nil { + return err + } + run, err = followRun(ctx, project, database, run, out) + if err != nil { + return err + } + return reportRun(run, out) +} + +// migrationRunFiles reads files into a run request. Names go by base name, +// since up reads only the top level of volcano/migrations. +func migrationRunFiles(files []string) ([]apiclient.DatabaseMigrationFile, error) { + request := make([]apiclient.DatabaseMigrationFile, 0, len(files)) + for _, file := range files { + name := filepath.Base(file) + if !trackedMigrationName.MatchString(name) { + return nil, fmt.Errorf("migration %s: a migration run takes only names ending in lowercase .sql; rename it", name) + } + content, err := os.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("failed to read migration %s: %w", name, err) + } + request = append(request, apiclient.DatabaseMigrationFile{Name: name, Sql: string(content)}) + } + return request, nil +} + +// startRun starts the run. When the database already has a run going, it +// follows that run to its end and then starts this one: a run started by an +// earlier command whose grant expired while it waited ends first, and the new +// run skips whatever it applied. +func startRun( + ctx context.Context, project *clisession.ProjectSession, database string, files []apiclient.DatabaseMigrationFile, out io.Writer, +) (*apiclient.DatabaseMigrationRun, error) { + // One key per command: a retried request returns the run this command + // started, and running the command again starts a new one. + key := uuid.NewString() + for attempt := 0; ; attempt++ { + run, err := project.API.CreateDatabaseMigrationRun(ctx, project.ProjectID, database, key, files) + if err == nil { + fmt.Fprintf(out, "Started migration run %s.\n", run.Id) + return run, nil + } + inProgress, ok := errors.AsType[*api.MigrationRunInProgressError](err) + if !ok || attempt > 0 { + return nil, migrationRunStartError(database, err) + } + fmt.Fprintf(out, "Migration run %s is already running on %s; waiting for it to end first.\n", inProgress.RunID, database) + earlier, err := project.API.GetDatabaseMigrationRun(ctx, project.ProjectID, database, inProgress.RunID) + if err != nil { + return nil, fmt.Errorf("failed to read migration run %s: %w", inProgress.RunID, err) + } + if _, err := followRun(ctx, project, database, earlier, out); err != nil { + return nil, err + } + } +} + +// migrationRunStartError says why a run did not start. +func migrationRunStartError(database string, err error) error { + if api.Status(err) == http.StatusNotFound && api.Code(err) == api.FeatureUnavailableCode { + return fmt.Errorf("migration runs are not available on this Volcano API, and a delegated grant "+ + "can't connect to the database directly, so nothing was applied to %q: %w", database, err) + } + return fmt.Errorf("failed to start a migration run on %q: %w", database, err) +} + +// followRun reads run until it ends, printing each phase it enters. +func followRun( + ctx context.Context, project *clisession.ProjectSession, database string, run *apiclient.DatabaseMigrationRun, out io.Writer, +) (*apiclient.DatabaseMigrationRun, error) { + deadline := time.Now().Add(runWaitLimit) + phase := apiclient.DatabaseMigrationRunPhase("") + failures := 0 + for { + if run.Phase != phase { + phase = run.Phase + fmt.Fprintf(out, " %s: %s\n", run.Id, phase) + } + if runEnded(run) { + return run, nil + } + if time.Now().After(deadline) { + return nil, fmt.Errorf("stopped waiting for migration run %s after %s; it carries on on the server. "+ + "Run the command again to wait for it", run.Id, runWaitLimit) + } + if err := sleep(ctx, runPollInterval); err != nil { + return nil, err + } + next, err := project.API.GetDatabaseMigrationRun(ctx, project.ProjectID, database, run.Id) + switch { + case err == nil: + run, failures = next, 0 + case api.Status(err) == 0 && failures+1 < maxRunReadFailures: + // No answer from the API: a network failure, worth another read. + failures++ + default: + return nil, fmt.Errorf("stopped following migration run %s, which carries on on the server. "+ + "Run the command again to wait for it: %w", run.Id, err) + } + } +} + +func runEnded(run *apiclient.DatabaseMigrationRun) bool { + return run.Status == apiclient.DatabaseMigrationRunStatusSucceeded || run.Status == apiclient.DatabaseMigrationRunStatusFailed +} + +func sleep(ctx context.Context, d time.Duration) error { + if d <= 0 { + return ctx.Err() + } + timer := time.NewTimer(d) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + +// reportRun prints each file's outcome and the run's warnings, and returns an +// error for a failed run. A file's database error is printed, never put in the +// returned error: it can quote SQL and data. +func reportRun(run *apiclient.DatabaseMigrationRun, out io.Writer) error { + fmt.Fprintln(out) + if run.Files != nil { + for _, file := range *run.Files { + fmt.Fprintf(out, " %s: %s\n", file.Name, file.Outcome) + if file.Error != nil { + fmt.Fprintf(out, " %s\n", fileErrorText(file.Error)) + } + } + } + for _, warning := range run.Warnings { + fmt.Fprintf(out, "Warning: %s\n", warning.Message) + } + if run.BackupName != nil && *run.BackupName != "" { + fmt.Fprintf(out, "Backup: %s\n", *run.BackupName) + } + if run.Status == apiclient.DatabaseMigrationRunStatusSucceeded { + fmt.Fprintln(out, "Migrations deployed!") + return nil + } + if run.Error != nil { + return fmt.Errorf("migration run %s failed in %s: %s: %s", run.Id, run.Phase, run.Error.Code, run.Error.Message) + } + return fmt.Errorf("migration run %s failed in %s", run.Id, run.Phase) +} + +func fileErrorText(fileErr *apiclient.DatabaseMigrationFileError) string { + var details []string + if fileErr.Line != nil { + details = append(details, fmt.Sprintf("line %d", *fileErr.Line)) + } + if fileErr.Sqlstate != nil && *fileErr.Sqlstate != "" { + details = append(details, "SQLSTATE "+*fileErr.Sqlstate) + } + if len(details) == 0 { + return fileErr.Message + } + return fileErr.Message + " (" + strings.Join(details, ", ") + ")" +} diff --git a/internal/cmd/databases/migration/runs_test.go b/internal/cmd/databases/migration/runs_test.go new file mode 100644 index 00000000..7df68435 --- /dev/null +++ b/internal/cmd/databases/migration/runs_test.go @@ -0,0 +1,305 @@ +package migration + +import ( + "bytes" + "context" + "encoding/json" + "maps" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + cliconfig "github.com/Kong/volcano-cli/internal/config" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +const ( + testMigrationGrant = cliconfig.DelegatedGrantPrefix + "grant-secret" + testRunID = "44444444-4444-4444-8444-444444444444" + testEarlierRunID = "55555555-5555-4555-8555-555555555555" + migrationRunsPath = "/projects/" + migrationProjectID + "/databases/app/migration-runs" +) + +// fakeMigrationRuns is a recording migration-runs API. Each GET of a run +// returns the next state from its list, and stays on the last. +type fakeMigrationRuns struct { + t *testing.T + mu sync.Mutex + requests []string + bodies []map[string]any + keys []string + create []func(w http.ResponseWriter) + states map[string][]map[string]any +} + +func (f *fakeMigrationRuns) ServeHTTP(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + f.requests = append(f.requests, r.Method+" "+r.URL.Path) + assert.Equal(f.t, "Bearer "+testMigrationGrant, r.Header.Get("Authorization")) + switch { + case r.Method == http.MethodPost && r.URL.Path == migrationRunsPath: + var body map[string]any + require.NoError(f.t, json.NewDecoder(r.Body).Decode(&body)) + f.bodies = append(f.bodies, body) + f.keys = append(f.keys, r.Header.Get("Idempotency-Key")) + respond := f.create[0] + if len(f.create) > 1 { + f.create = f.create[1:] + } + respond(w) + case r.Method == http.MethodGet && len(r.URL.Path) > len(migrationRunsPath)+1: + id := r.URL.Path[len(migrationRunsPath)+1:] + states := f.states[id] + if len(states) == 0 { + writeRunJSON(w, http.StatusNotFound, map[string]any{"error": "not found"}) + return + } + state := states[0] + if len(states) > 1 { + f.states[id] = states[1:] + } + writeRunJSON(w, http.StatusOK, state) + default: + writeRunJSON(w, http.StatusForbidden, map[string]any{ + "error": "delegated grants cannot be used on this endpoint", "code": "grant_route_not_allowed", + }) + } +} + +func writeRunJSON(w http.ResponseWriter, status int, body any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(body) +} + +func runState(id, status, phase string, extra map[string]any) map[string]any { + state := map[string]any{ + "id": id, "status": status, "phase": phase, "source": "api", "warnings": []any{}, + "database": map[string]any{"id": "66666666-6666-4666-8666-666666666666", "name": "app"}, + "created_at": "2026-11-11T00:00:00Z", "updated_at": "2026-11-11T00:00:00Z", + } + maps.Copy(state, extra) + return state +} + +func acceptRun(id string) func(w http.ResponseWriter) { + return func(w http.ResponseWriter) { + writeRunJSON(w, http.StatusAccepted, runState(id, "queued", "plan", nil)) + } +} + +// setGrantMigrationTest makes a project with two migration files and the +// grant in VOLCANO_TOKEN, and polls without waiting. +func setGrantMigrationTest(t *testing.T) { + t.Helper() + setMigrationCommandTestHome(t) + t.Chdir(t.TempDir()) + require.NoError(t, os.MkdirAll(filepath.Join("volcano", "migrations"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "002_second.sql"), []byte("CREATE TABLE second;"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "001_first.sql"), []byte("CREATE TABLE first;"), 0o644)) + saveMigrationCommandTestConfig(t) + t.Setenv("VOLCANO_TOKEN", testMigrationGrant) + previous := runPollInterval + runPollInterval = 0 + t.Cleanup(func() { runPollInterval = previous }) +} + +func runGrantMigration(t *testing.T, fake *fakeMigrationRuns, args ...string) (string, error) { + t.Helper() + server := httptest.NewServer(fake) + t.Cleanup(server.Close) + executor := func(context.Context, string, string) error { + t.Error("a grant must never connect to the database directly") + return nil + } + cmd := newWithExecutor(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}, executor) + out, err := executeMigrationCommand(t, cmd, args...) + assert.NotContains(t, out, "grant-secret") + if err != nil { + assert.NotContains(t, err.Error(), "grant-secret") + } + for _, request := range fake.requests { + assert.NotEqual(t, "GET /projects/"+migrationProjectID+"/databases/app", request, "a grant must never read the connection string") + } + return out, err +} + +func TestMigrationUpUsesRunsUnderGrant(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){acceptRun(testRunID)}, states: map[string][]map[string]any{ + testRunID: { + runState(testRunID, "running", "rehearse", nil), + runState(testRunID, "succeeded", "apply", map[string]any{ + "backup_name": "before-migration-1", + "files": []any{ + map[string]any{"name": "001_first.sql", "outcome": "already_applied"}, + map[string]any{"name": "002_second.sql", "outcome": "applied"}, + }, + }), + }, + }} + + out, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.NoError(t, err) + + assert.Equal(t, []string{ + "POST " + migrationRunsPath, + "GET " + migrationRunsPath + "/" + testRunID, + "GET " + migrationRunsPath + "/" + testRunID, + }, fake.requests) + require.Len(t, fake.bodies, 1) + assert.Equal(t, []any{ + map[string]any{"name": "001_first.sql", "sql": "CREATE TABLE first;"}, + map[string]any{"name": "002_second.sql", "sql": "CREATE TABLE second;"}, + }, fake.bodies[0]["files"]) + assert.NotEmpty(t, fake.keys[0], "Idempotency-Key") + for _, want := range []string{ + "Started migration run " + testRunID, + testRunID + ": rehearse", + testRunID + ": apply", + "001_first.sql: already_applied", + "002_second.sql: applied", + "Backup: before-migration-1", + "Migrations deployed!", + } { + assert.Contains(t, out, want) + } + assert.NotContains(t, out, "without tracking") +} + +func TestMigrationUpUnderGrantSendsOneFile(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){func(w http.ResponseWriter) { + writeRunJSON(w, http.StatusOK, runState(testRunID, "succeeded", "apply", nil)) + }}} + + _, err := runGrantMigration(t, fake, "up", "-f", "002_second", "-d", "app") + require.NoError(t, err) + assert.Equal(t, []any{map[string]any{"name": "002_second.sql", "sql": "CREATE TABLE second;"}}, fake.bodies[0]["files"]) + assert.Equal(t, []string{"POST " + migrationRunsPath}, fake.requests, "a 200 replay that already ended needs no read") +} + +func TestMigrationUpUnderGrantReportsAFailedRun(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){acceptRun(testRunID)}, states: map[string][]map[string]any{ + testRunID: {runState(testRunID, "failed", "rehearse", map[string]any{ + "error": map[string]any{"code": "migration_failed", "message": "002_second.sql failed at line 1 (SQLSTATE 42P07)"}, + "files": []any{ + map[string]any{"name": "001_first.sql", "outcome": "applied"}, + map[string]any{"name": "002_second.sql", "outcome": "failed", "error": map[string]any{ + "message": `relation "second" already exists`, "sqlstate": "42P07", "line": 1, + }}, + }, + })}, + }} + + out, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.Error(t, err) + assert.Equal(t, "migration run "+testRunID+" failed in rehearse: migration_failed: 002_second.sql failed at line 1 (SQLSTATE 42P07)", err.Error()) + assert.Contains(t, out, `relation "second" already exists (line 1, SQLSTATE 42P07)`) + assert.NotContains(t, err.Error(), "already exists", "the database's own message stays out of the error") +} + +// Where migration runs are off, a grant fails and never falls back to the +// connection string. +func TestMigrationUpUnderGrantRefusesWhenRunsAreOff(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){func(w http.ResponseWriter) { + writeRunJSON(w, http.StatusNotFound, map[string]any{"error": "migration runs are not available", "code": "feature_unavailable"}) + }}} + + _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.Error(t, err) + assert.Contains(t, err.Error(), "migration runs are not available on this Volcano API") + assert.Contains(t, err.Error(), "nothing was applied") + assert.Equal(t, []string{"POST " + migrationRunsPath}, fake.requests) +} + +// A run an earlier command started is still going: wait for it, then start +// this command's run once. +func TestMigrationUpUnderGrantWaitsForARunInProgress(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){ + func(w http.ResponseWriter) { + writeRunJSON(w, http.StatusConflict, map[string]any{ + "error": "a migration run is in progress", "code": "migration_run_in_progress", "run_id": testEarlierRunID, + }) + }, + acceptRun(testRunID), + }, states: map[string][]map[string]any{ + testEarlierRunID: { + runState(testEarlierRunID, "running", "apply", nil), + runState(testEarlierRunID, "succeeded", "apply", nil), + }, + testRunID: {runState(testRunID, "succeeded", "apply", nil)}, + }} + + out, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.NoError(t, err) + assert.Contains(t, out, "Migration run "+testEarlierRunID+" is already running on app; waiting for it to end first.") + assert.Equal(t, []string{ + "POST " + migrationRunsPath, + "GET " + migrationRunsPath + "/" + testEarlierRunID, + "GET " + migrationRunsPath + "/" + testEarlierRunID, + "POST " + migrationRunsPath, + "GET " + migrationRunsPath + "/" + testRunID, + }, fake.requests) + require.Len(t, fake.keys, 2) + assert.Equal(t, fake.keys[0], fake.keys[1], "one command, one key") +} + +// The grant expires while the run goes on: the command says the run carries on. +func TestMigrationUpUnderGrantStopsOnAnExpiredGrant(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){acceptRun(testRunID)}, states: map[string][]map[string]any{}} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + fake.mu.Lock() + fake.requests = append(fake.requests, r.Method+" "+r.URL.Path) + fake.mu.Unlock() + writeRunJSON(w, http.StatusUnauthorized, map[string]any{"error": "invalid or expired token"}) + return + } + fake.ServeHTTP(w, r) + })) + t.Cleanup(server.Close) + cmd := newWithExecutor(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}, nil) + out, err := executeMigrationCommand(t, cmd, "up", "--all", "-d", "app") + require.Error(t, err) + assert.Contains(t, err.Error(), "stopped following migration run "+testRunID+", which carries on on the server") + assert.Contains(t, out, "Started migration run "+testRunID) + assert.NotContains(t, out, "grant-secret") +} + +func TestMigrationUpUnderGrantRefusesAnUppercaseExtension(t *testing.T) { + setGrantMigrationTest(t) + require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "003_third.SQL"), []byte("SELECT 1;"), 0o644)) + fake := &fakeMigrationRuns{t: t} + + _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.ErrorContains(t, err, "003_third.SQL: a migration run takes only names ending in lowercase .sql") + assert.Empty(t, fake.requests) +} + +// `deploy --with-migrations` applies through ApplyAll, which takes the same +// path under a grant. +func TestApplyAllUsesRunsUnderGrant(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){acceptRun(testRunID)}, states: map[string][]map[string]any{ + testRunID: {runState(testRunID, "succeeded", "apply", nil)}, + }} + server := httptest.NewServer(fake) + t.Cleanup(server.Close) + + var out bytes.Buffer + err := ApplyAll(t.Context(), cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}, "app", &out) + require.NoError(t, err) + assert.Equal(t, []string{"POST " + migrationRunsPath, "GET " + migrationRunsPath + "/" + testRunID}, fake.requests) +} From 637e75bb217cdced6cac6ade3c51569b506ffb08 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 22:44:51 -0500 Subject: [PATCH 02/11] test(deploy): check a grant's deploy against Hosting's grant route table --- internal/cmd/databases/migration/runs_test.go | 15 ++ internal/cmd/deploy/grant_test.go | 212 ++++++++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 internal/cmd/deploy/grant_test.go diff --git a/internal/cmd/databases/migration/runs_test.go b/internal/cmd/databases/migration/runs_test.go index 7df68435..6953d363 100644 --- a/internal/cmd/databases/migration/runs_test.go +++ b/internal/cmd/databases/migration/runs_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/Kong/volcano-cli/internal/api" cliconfig "github.com/Kong/volcano-cli/internal/config" cliruntime "github.com/Kong/volcano-cli/internal/runtime" ) @@ -303,3 +304,17 @@ func TestApplyAllUsesRunsUnderGrant(t *testing.T) { require.NoError(t, err) assert.Equal(t, []string{"POST " + migrationRunsPath, "GET " + migrationRunsPath + "/" + testRunID}, fake.requests) } + +// A spent budget keeps the API's error, so the CLI exits 6. +func TestMigrationUpUnderGrantKeepsTheBudgetRefusal(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){func(w http.ResponseWriter) { + writeRunJSON(w, http.StatusTooManyRequests, map[string]any{ + "error": "the delegated grant has no mutations left; ask the app for a new grant", "code": "grant_budget_exhausted", + }) + }}} + + _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.Error(t, err) + assert.True(t, api.GrantBudgetExhausted(err), "%v", err) +} diff --git a/internal/cmd/deploy/grant_test.go b/internal/cmd/deploy/grant_test.go new file mode 100644 index 00000000..b52b444b --- /dev/null +++ b/internal/cmd/deploy/grant_test.go @@ -0,0 +1,212 @@ +package deploy + +import ( + "net/http" + "slices" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Kong/volcano-cli/internal/api" + cliconfig "github.com/Kong/volcano-cli/internal/config" +) + +// grantRouteTable is Hosting's delegated grant route table, copied from +// DefaultGrantRoutes() in internal/middleware/delegated_grant_routes.go at +// Kong/volcano-hosting 1749144404 (C3, #3313). A pg- grant is refused on any +// other route with 403 grant_route_not_allowed. Update it with that file. +var grantRouteTable = []string{ + "GET /projects/{id}", "GET /projects/{id}/capabilities", "GET /projects/{id}/config", + "GET /projects/{id}/deployments", "POST /projects/{id}/logs/search", "POST /projects/{id}/logs/stream", + "GET /projects/{id}/functions", "GET /projects/{id}/functions/{functionId}", + "GET /projects/{id}/functions/{functionId}/deployments", "GET /projects/{id}/functions/{functionId}/schedulers", + "GET /projects/{id}/functions/{functionId}/schedulers/{schedulerId}", + "GET /projects/{id}/durable-functions", "GET /projects/{id}/durable-functions/{functionId}", + "GET /projects/{id}/frontends", "GET /projects/{id}/frontends/{frontendId}", + "GET /projects/{id}/frontends/{frontendId}/deployments", "GET /projects/{id}/frontends/{frontendId}/serving", + "POST /projects/{id}/functions", "POST /projects/{id}/functions/batch", "PATCH /projects/{id}/functions/{functionId}", + "POST /projects/{id}/functions/{functionId}/schedulers", + "PATCH /projects/{id}/functions/{functionId}/schedulers/{schedulerId}", + "DELETE /projects/{id}/functions/{functionId}/schedulers/{schedulerId}", + "POST /projects/{id}/durable-functions", + "PUT /projects/{id}/config", "POST /projects/{id}/variables", "PUT /projects/{id}/variables/{name}", + "DELETE /projects/{id}/variables/{name}", + "GET /projects/{id}/databases/{databaseName}/migrations", "GET /projects/{id}/databases/{databaseName}/migration-runs", + "GET /projects/{id}/databases/{databaseName}/migration-runs/{runId}", + "POST /projects/{id}/databases/{databaseName}/migration-runs", + "POST /projects/{id}/frontends", "POST /projects/{id}/frontends/{frontendId}/deployments", + "POST /projects/{id}/frontends/{frontendId}/redeploy", +} + +// grantMayCall reports whether a grant may call method and path. The public +// runtime catalog runs no auth middleware, so the grant branch never sees it. +func grantMayCall(method, path string) bool { + if method == http.MethodGet && path == "/functions/runtimes" { + return true + } + got := strings.Split(path, "/") + for _, entry := range grantRouteTable { + entryMethod, template, _ := strings.Cut(entry, " ") + want := strings.Split(template, "/") + if entryMethod != method || len(want) != len(got) { + continue + } + match := true + for i := range want { + if got[i] == "" && want[i] != "" || !strings.HasPrefix(want[i], "{") && want[i] != got[i] { + match = false + break + } + } + if match { + return true + } + } + return false +} + +// grantGate refuses what Hosting refuses a grant, records it, and passes the +// rest to next. +type grantGate struct { + mu sync.Mutex + refused []string + auth []string + // also lets a grant call these "METHOD path" requests too. + also []string + next http.HandlerFunc +} + +func (g *grantGate) ServeHTTP(w http.ResponseWriter, r *http.Request) { + g.mu.Lock() + g.auth = append(g.auth, r.Header.Get("Authorization")) + if !grantMayCall(r.Method, r.URL.Path) && !slices.Contains(g.also, r.Method+" "+r.URL.Path) { + g.refused = append(g.refused, r.Method+" "+r.URL.Path) + g.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`)) + return + } + g.mu.Unlock() + g.next(w, r) +} + +// sourceExportRead is the read `volcano cloud deploy` makes before deploying +// code, to refuse a project that deploys from Git. It is not on the grant +// table yet, so a whole-project deploy fails under a grant until Hosting adds +// it (as project.read). Per-resource deploys don't make it. +const sourceExportRead = "GET /projects/" + deployProjectID + "/source-export" + +func TestDeployUnderAGrantIsRefusedTheSourceModeRead(t *testing.T) { + setDeployCommandTestHome(t) + t.Setenv("VOLCANO_TOKEN", cliconfig.DelegatedGrantPrefix+"grant-secret") + writeFullProject(t) + fake := newFakeVolcano(t) + gate := &grantGate{next: fake.serve} + server := fake.start() + server.Config.Handler = gate + + out, err := executeDeployCommand(t, New(fake.cloudDeps(server))) + require.Error(t, err) + assert.Equal(t, []string{sourceExportRead}, gate.refused) + assert.Empty(t, fake.recorded(), "refused before any change") + assert.NotContains(t, out, "grant-secret") +} + +// With the source mode read allowed, every other request a whole-project +// deploy makes, migrations included, is on the grant table, and carries the +// grant. +func TestDeployUnderAGrantCallsOnlyGrantRoutesOtherwise(t *testing.T) { + setDeployCommandTestHome(t) + t.Setenv("VOLCANO_TOKEN", cliconfig.DelegatedGrantPrefix+"grant-secret") + writeFullProject(t) + fake := newFakeVolcano(t) + gate := &grantGate{next: fake.serve, also: []string{sourceExportRead}} + server := fake.start() + server.Config.Handler = gate + + out, err := executeDeployCommand(t, New(fake.cloudDeps(server))) + require.NoError(t, err, out) + assert.Empty(t, gate.refused) + for _, auth := range gate.auth { + assert.Equal(t, "Bearer "+cliconfig.DelegatedGrantPrefix+"grant-secret", auth) + } + assert.NotContains(t, out, "grant-secret") +} + +// `deploy --with-migrations` under a grant applies through a migration run and +// never reads the database, whose answer holds the connection string. +func TestDeployWithMigrationsUnderAGrantUsesAMigrationRun(t *testing.T) { + setDeployCommandTestHome(t) + t.Setenv("VOLCANO_TOKEN", cliconfig.DelegatedGrantPrefix+"grant-secret") + writeDeployProjectFile(t, "volcano/migrations/001_init.sql", "create table notes (id int);") + writeDeployProjectFile(t, "volcano/functions/hello.js", `exports.handler = async () => ({});`) + fake := newFakeVolcano(t) + runs := "/projects/" + deployProjectID + "/databases/app/migration-runs" + const runID = "44444444-4444-4444-8444-444444444444" + var runCalls []string + gate := &grantGate{also: []string{sourceExportRead}, next: func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, runs) { + runCalls = append(runCalls, r.Method+" "+r.URL.Path) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"id":"` + runID + `","status":"succeeded","phase":"apply","source":"api","warnings":[],` + + `"database":{"id":"66666666-6666-4666-8666-666666666666","name":"app"},` + + `"files":[{"name":"001_init.sql","outcome":"applied"}],` + + `"created_at":"2026-11-11T00:00:00Z","updated_at":"2026-11-11T00:00:00Z"}`)) + return + } + fake.serve(w, r) + }} + server := fake.start() + server.Config.Handler = gate + + out, err := executeDeployCommand(t, New(fake.cloudDeps(server)), "--with-migrations", "-d", "app") + require.NoError(t, err, out) + assert.Empty(t, gate.refused) + assert.Equal(t, []string{"POST " + runs}, runCalls) + assert.NotContains(t, fake.reads, "database app") + assert.Contains(t, out, "001_init.sql: applied") +} + +// A spent budget on any mutation keeps the API's error, so the CLI exits 6 +// and the worker can ask for a new grant. +func TestDeployUnderAGrantKeepsTheBudgetRefusalOnEveryMutation(t *testing.T) { + project := "/projects/" + deployProjectID + for _, route := range []string{ + "POST " + project + "/variables", + "POST " + project + "/functions/batch", + "POST " + project + "/durable-functions", + "POST " + project + "/frontends", + "PUT " + project + "/config", + } { + t.Run(route, func(t *testing.T) { + setDeployCommandTestHome(t) + t.Setenv("VOLCANO_TOKEN", cliconfig.DelegatedGrantPrefix+"grant-secret") + writeFullProject(t) + fake := newFakeVolcano(t) + var hit bool + gate := &grantGate{also: []string{sourceExportRead}, next: func(w http.ResponseWriter, r *http.Request) { + // The config step's dry run is a read in all but method. + if r.Method+" "+r.URL.Path == route && r.URL.Query().Get("dry_run") != "true" { + hit = true + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"error":"the delegated grant has no mutations left; ask the app for a new grant","code":"grant_budget_exhausted"}`)) + return + } + fake.serve(w, r) + }} + server := fake.start() + server.Config.Handler = gate + + out, err := executeDeployCommand(t, New(fake.cloudDeps(server))) + require.Error(t, err, out) + require.True(t, hit, "the deploy never reached %s", route) + assert.True(t, api.GrantBudgetExhausted(err), "%v", err) + }) + } +} From eefe9875c1c9f0919a5bbfbdd257b94175a795d7 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 23:20:57 -0500 Subject: [PATCH 03/11] feat(databases): record migration baselines through Hosting's baseline route --- cmd/volcano/grant_test.go | 33 ++++ internal/api/database_migration_runs.go | 42 +++- internal/cmd/databases/migration/baseline.go | 183 ++++++++++++++++++ .../cmd/databases/migration/baseline_test.go | 128 ++++++++++++ internal/cmd/databases/migration/migration.go | 4 +- internal/cmd/databases/migration/runs.go | 59 +++++- internal/cmd/databases/migration/runs_test.go | 31 ++- 7 files changed, 462 insertions(+), 18 deletions(-) create mode 100644 internal/cmd/databases/migration/baseline.go create mode 100644 internal/cmd/databases/migration/baseline_test.go diff --git a/cmd/volcano/grant_test.go b/cmd/volcano/grant_test.go index 2e9d5dc1..3363427f 100644 --- a/cmd/volcano/grant_test.go +++ b/cmd/volcano/grant_test.go @@ -265,3 +265,36 @@ func TestGrantMigrationUpUsesAMigrationRun(t *testing.T) { assert.NotEqual(t, "GET /projects/"+mainTestProjectAlpha+"/databases/app", request) } } + +// Baseline under a grant goes through Hosting's baseline route, with the files +// the caller names, and exits like other commands when Hosting refuses it. +func TestGrantBaselineUsesTheBaselineRoute(t *testing.T) { + baselinePath := "/projects/" + mainTestProjectAlpha + "/databases/app/migrations/baseline" + writeMigration := func() { + require.NoError(t, os.MkdirAll(filepath.Join("volcano", "migrations"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "001_init.sql"), []byte("CREATE TABLE t();"), 0o644)) + } + for _, tc := range []struct { + name string + status int + body string + code int + }{ + {"recorded", http.StatusOK, `{"recorded":["001_init.sql"]}`, 0}, + {"refused", http.StatusForbidden, `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`, exitPermissionDenied}, + {"budget", http.StatusTooManyRequests, `{"error":"the delegated grant has no mutations left; ask the app for a new grant","code":"grant_budget_exhausted"}`, exitGrantBudgetExhausted}, + } { + t.Run(tc.name, func(t *testing.T) { + run := runAsWorkerWith(t, writeMigration, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == baselinePath { + respondExitTestJSON(w, tc.status, tc.body) + return + } + respondExitTestJSON(w, http.StatusForbidden, `{"error":"delegated grants cannot be used on this endpoint","code":"grant_route_not_allowed"}`) + }, "cloud", "databases", "migration", "baseline", "-d", "app", "-f", "001_init", "--yes") + + assert.Equal(t, tc.code, run.code, run.stderr) + assert.Equal(t, []string{"POST " + baselinePath}, run.requests) + }) + } +} diff --git a/internal/api/database_migration_runs.go b/internal/api/database_migration_runs.go index 57ac4d56..244c8804 100644 --- a/internal/api/database_migration_runs.go +++ b/internal/api/database_migration_runs.go @@ -55,18 +55,44 @@ func (c *Client) CreateDatabaseMigrationRun( case resp.JSON200 != nil: return resp.JSON200, nil case resp.JSON409 != nil: - apiErr := &Error{StatusCode: http.StatusConflict, Message: strings.TrimSpace(resp.JSON409.Error), fromAPI: true} - if resp.JSON409.Code != nil { - apiErr.Code = *resp.JSON409.Code - } - if apiErr.Code == MigrationRunInProgressCode && resp.JSON409.RunId != nil { - return nil, &MigrationRunInProgressError{Err: apiErr, RunID: *resp.JSON409.RunId} - } - return nil, apiErr + return nil, migrationConflictError(resp.JSON409) } return nil, apiErrorFromGeneratedErrors(resp.StatusCode(), resp.Body, resp.JSON400, resp.JSON404, resp.JSON413) } +// BaselineDatabaseMigrations records files in databaseName's migration +// history as applied, without running them. The server skips files it already +// records with the same checksum, and refuses one recorded with another. +func (c *Client) BaselineDatabaseMigrations( + ctx context.Context, projectID uuid.UUID, databaseName string, files []apiclient.DatabaseMigrationFile, +) (*apiclient.DatabaseMigrationBaseline, error) { + resp, err := c.client.BaselineDatabaseMigrationsWithResponse(ctx, projectID, strings.TrimSpace(databaseName), + apiclient.BaselineDatabaseMigrationsJSONRequestBody{Files: files}) + if err != nil { + return nil, err + } + if resp.JSON200 != nil { + return resp.JSON200, nil + } + if resp.JSON409 != nil { + return nil, migrationConflictError(resp.JSON409) + } + return nil, apiErrorFromGeneratedErrors(resp.StatusCode(), resp.Body, resp.JSON400, resp.JSON404, resp.JSON413, resp.JSON503) +} + +// migrationConflictError is the error for a migration 409, naming the run in +// progress when that is the reason. +func migrationConflictError(conflict *apiclient.DatabaseMigrationConflictError) error { + apiErr := &Error{StatusCode: http.StatusConflict, Message: strings.TrimSpace(conflict.Error), fromAPI: true} + if conflict.Code != nil { + apiErr.Code = *conflict.Code + } + if apiErr.Code == MigrationRunInProgressCode && conflict.RunId != nil { + return &MigrationRunInProgressError{Err: apiErr, RunID: *conflict.RunId} + } + return apiErr +} + // GetDatabaseMigrationRun returns one migration run with its files. func (c *Client) GetDatabaseMigrationRun( ctx context.Context, projectID uuid.UUID, databaseName string, runID uuid.UUID, diff --git a/internal/cmd/databases/migration/baseline.go b/internal/cmd/databases/migration/baseline.go new file mode 100644 index 00000000..6b612170 --- /dev/null +++ b/internal/cmd/databases/migration/baseline.go @@ -0,0 +1,183 @@ +package migration + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "path/filepath" + "strings" + + "github.com/spf13/cobra" + + "github.com/Kong/volcano-cli/internal/api" + "github.com/Kong/volcano-cli/internal/confirm" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" + clisession "github.com/Kong/volcano-cli/internal/session" +) + +// newBaseline returns `databases migration baseline`: record files in a +// database's migration history as applied, without running them, through +// Hosting's baseline route. It never reads the connection string, so it works +// under a delegated grant. The caller names the files; nothing is detected. +// +// Flags match the baseline command in Kong/volcano-cli#365, so the two merge. +func newBaseline(deps cliruntime.Deps) *cobra.Command { + var database string + var names []string + var all, yes, jsonOutput bool + cmd := &cobra.Command{ + Use: "baseline", + Short: "Record migrations as applied without running them", + Long: fmt.Sprintf(`Record migration files in a database's migration history as applied, without +running them, so later migration runs skip them. + +Use it for files the database already reflects, such as ones applied with +'databases migration up' before migration runs tracked them. A file recorded +this way never runs, so name only files whose objects the database already +holds. When a migration run fails because an object already exists, the error +names the object and the file; baseline that file if the database already +holds everything it creates. + +Files the history already holds with the same content are skipped. A file +recorded with different content is refused (checksum_mismatch), and nothing is +recorded. Nothing reads the database's connection string, so this works with +a delegated grant (pg-). + +By default this command prompts for confirmation. Use --yes to skip the prompt. + +Usage: + %s + %s`, + cliruntime.CommandPath(deps, "databases migration baseline -d mydb --all"), + cliruntime.CommandPath(deps, "databases migration baseline -d mydb -f 001_create_users -f 002_add_posts")), + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + return runBaseline(cmd, baselineOptions{ + deps: deps, database: strings.TrimSpace(database), names: names, all: all, yes: yes, json: jsonOutput, + }) + }, + } + cmd.Flags().StringVarP(&database, "database", "d", "", "Database whose migration history to record in (required)") + cmd.Flags().BoolVarP(&all, "all", "a", false, "Record every migration file") + cmd.Flags().StringArrayVarP(&names, "file", "f", nil, "Record a migration by name or path; repeat for several") + cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Skip confirmation prompt") + cmd.Flags().BoolVar(&jsonOutput, "json", false, "Emit machine-readable JSON") + if err := cmd.MarkFlagRequired("database"); err != nil { + panic(err) + } + cmd.MarkFlagsMutuallyExclusive("all", "file") + cmd.MarkFlagsOneRequired("all", "file") + return cmd +} + +type baselineOptions struct { + deps cliruntime.Deps + database string + names []string + all bool + yes bool + json bool +} + +func runBaseline(cmd *cobra.Command, opts baselineOptions) error { + if opts.database == "" { + return errors.New("specify the database with --database/-d") + } + all, err := scanMigrations(filepath.Join("volcano", "migrations")) + if err != nil { + return fmt.Errorf("failed to scan migrations: %w", err) + } + selected, err := selectBaselineFiles(all, opts.names, opts.all) + if err != nil { + return err + } + files, err := migrationRunFiles(selected) + if err != nil { + return err + } + project, err := clisession.NewFactory(opts.deps).CurrentProject() + if err != nil { + return err + } + + if !opts.yes { + // With --json, stdout carries only the JSON document. + prompt := cmd.OutOrStdout() + if opts.json { + prompt = cmd.ErrOrStderr() + } + confirmed, err := confirmBaseline(cmd.InOrStdin(), prompt, opts.database, selected) + if err != nil || !confirmed { + return err + } + } + + baseline, err := project.API.BaselineDatabaseMigrations(cmd.Context(), project.ProjectID, opts.database, files) + if err != nil { + if api.Status(err) == http.StatusNotFound && api.Code(err) == api.FeatureUnavailableCode { + return fmt.Errorf("migration runs are not available on this Volcano API, so there is no migration history "+ + "to record a baseline in: %w", err) + } + return fmt.Errorf("failed to record a baseline in %q: %w", opts.database, err) + } + out := cmd.OutOrStdout() + if opts.json { + encoder := json.NewEncoder(out) + encoder.SetIndent("", " ") + return encoder.Encode(baseline) + } + if len(baseline.Recorded) == 0 { + fmt.Fprintf(out, "Nothing recorded: the migration history of %q already holds these files.\n", opts.database) + return nil + } + fmt.Fprintf(out, "Recorded %d file(s) as applied in %q without running them:\n", len(baseline.Recorded), opts.database) + for _, name := range baseline.Recorded { + fmt.Fprintf(out, " - %s\n", name) + } + return nil +} + +// confirmBaseline asks before recording. A command no person answers, such +// as a worker's, has to pass --yes. +func confirmBaseline(in io.Reader, w io.Writer, database string, files []string) (bool, error) { + if confirm.MarkedNonInteractive() { + return false, confirm.ErrCannotPrompt + } + listed := make([]string, len(files)) + for i, file := range files { + listed[i] = " - " + filepath.Base(file) + } + return confirm.Action(in, w, + fmt.Sprintf("A baseline records these files in the migration history of database '%s' as applied, "+ + "without running them. A file recorded this way never runs.\n%s", database, strings.Join(listed, "\n")), + fmt.Sprintf("Record %d file(s) as applied?", len(files))) +} + +// selectBaselineFiles picks the files a baseline records: all of them, or the +// ones named, each of which has to exist. +func selectBaselineFiles(all []string, names []string, everything bool) ([]string, error) { + if len(all) == 0 { + return nil, errors.New("no migration files found in volcano/migrations/") + } + if everything { + return all, nil + } + var selected []string + seen := map[string]bool{} + for _, name := range names { + if strings.TrimSpace(name) == "" { + return nil, errors.New("--file needs a migration name") + } + files, err := selectMigrationFiles(all, name, false) + if err != nil { + return nil, err + } + if !seen[files[0]] { + seen[files[0]] = true + selected = append(selected, files[0]) + } + } + return selected, nil +} diff --git a/internal/cmd/databases/migration/baseline_test.go b/internal/cmd/databases/migration/baseline_test.go new file mode 100644 index 00000000..9b51e4e2 --- /dev/null +++ b/internal/cmd/databases/migration/baseline_test.go @@ -0,0 +1,128 @@ +package migration + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/Kong/volcano-cli/internal/api" + "github.com/Kong/volcano-cli/internal/confirm" + cliruntime "github.com/Kong/volcano-cli/internal/runtime" +) + +const baselinePath = "/projects/" + migrationProjectID + "/databases/app/migrations/baseline" + +// baselineServer answers the baseline route with respond and fails any other +// request, the database read that holds the connection string included. +func baselineServer(t *testing.T, respond func(w http.ResponseWriter, files []any)) (*httptest.Server, *[]string) { + t.Helper() + var requests []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.Method+" "+r.URL.Path) + assert.Equal(t, "Bearer "+testMigrationGrant, r.Header.Get("Authorization")) + if r.Method != http.MethodPost || r.URL.Path != baselinePath { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + return + } + var body map[string][]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + respond(w, body["files"]) + })) + t.Cleanup(server.Close) + return server, &requests +} + +func runBaselineCommand(t *testing.T, server *httptest.Server, args ...string) (string, error) { + t.Helper() + cmd := newWithExecutor(cliruntime.Deps{HTTPClient: server.Client(), APIBaseURL: server.URL}, nil) + out, err := executeMigrationCommand(t, cmd, append([]string{"baseline"}, args...)...) + assert.NotContains(t, out, "grant-secret") + return out, err +} + +// Under a grant, baseline records the files the caller names through +// Hosting's baseline route, and never reads the connection string. +func TestBaselineUnderGrantRecordsTheNamedFiles(t *testing.T) { + setGrantMigrationTest(t) + var sent []any + server, requests := baselineServer(t, func(w http.ResponseWriter, files []any) { + sent = files + writeRunJSON(w, http.StatusOK, map[string]any{"recorded": []string{"002_second.sql"}}) + }) + + out, err := runBaselineCommand(t, server, "-d", "app", "-f", "002_second", "--yes") + require.NoError(t, err) + assert.Equal(t, []string{"POST " + baselinePath}, *requests) + assert.Equal(t, []any{map[string]any{"name": "002_second.sql", "sql": "CREATE TABLE second;"}}, sent) + assert.Contains(t, out, "Recorded 1 file(s) as applied in \"app\" without running them:\n - 002_second.sql\n") +} + +func TestBaselineAllAndJSON(t *testing.T) { + setGrantMigrationTest(t) + var sent []any + server, _ := baselineServer(t, func(w http.ResponseWriter, files []any) { + sent = files + writeRunJSON(w, http.StatusOK, map[string]any{"recorded": []string{}}) + }) + + out, err := runBaselineCommand(t, server, "-d", "app", "--all", "--yes", "--json") + require.NoError(t, err) + assert.Len(t, sent, 2) + assert.JSONEq(t, `{"recorded":[]}`, out) +} + +// No person answers a worker's command, so it has to pass --yes. +func TestBaselineWithoutYesRefusesWhenNoOneCanAnswer(t *testing.T) { + setGrantMigrationTest(t) + t.Setenv("VOLCANO_NONINTERACTIVE", "1") + server, requests := baselineServer(t, func(w http.ResponseWriter, _ []any) { + writeRunJSON(w, http.StatusOK, map[string]any{"recorded": []string{}}) + }) + + _, err := runBaselineCommand(t, server, "-d", "app", "--all") + require.ErrorIs(t, err, confirm.ErrCannotPrompt) + assert.Empty(t, *requests) +} + +func TestBaselineKeepsTheAPIRefusal(t *testing.T) { + for _, tc := range []struct { + name string + status int + body map[string]any + code string + want string + }{ + {"checksum mismatch", http.StatusConflict, map[string]any{"error": "001_first.sql is recorded with another checksum", "code": "checksum_mismatch"}, "checksum_mismatch", "001_first.sql is recorded with another checksum"}, + {"budget", http.StatusTooManyRequests, map[string]any{"error": "no mutations left", "code": "grant_budget_exhausted"}, "grant_budget_exhausted", "no mutations left"}, + {"not on the grant table", http.StatusForbidden, map[string]any{"error": "delegated grants cannot be used on this endpoint", "code": "grant_route_not_allowed"}, "grant_route_not_allowed", "cannot be used"}, + {"runs off", http.StatusNotFound, map[string]any{"error": "migration runs are not available", "code": "feature_unavailable"}, "feature_unavailable", "no migration history to record a baseline in"}, + } { + t.Run(tc.name, func(t *testing.T) { + setGrantMigrationTest(t) + server, _ := baselineServer(t, func(w http.ResponseWriter, _ []any) { writeRunJSON(w, tc.status, tc.body) }) + + _, err := runBaselineCommand(t, server, "-d", "app", "--all", "--yes") + require.Error(t, err) + assert.Equal(t, tc.status, api.Status(err)) + assert.Equal(t, tc.code, api.Code(err)) + assert.Contains(t, err.Error(), tc.want) + }) + } +} + +func TestBaselineRefusesAnUnknownFile(t *testing.T) { + setGrantMigrationTest(t) + require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "notes.txt"), []byte("x"), 0o644)) + server, requests := baselineServer(t, func(w http.ResponseWriter, _ []any) {}) + + _, err := runBaselineCommand(t, server, "-d", "app", "-f", "009_missing", "--yes") + require.ErrorContains(t, err, `migration "009_missing" not found`) + assert.Empty(t, *requests) +} diff --git a/internal/cmd/databases/migration/migration.go b/internal/cmd/databases/migration/migration.go index 5540d7a3..665895f1 100644 --- a/internal/cmd/databases/migration/migration.go +++ b/internal/cmd/databases/migration/migration.go @@ -52,6 +52,7 @@ func newWithExecutor(deps cliruntime.Deps, executor sqlExecutor) *cobra.Command Long: "Apply SQL migrations from the volcano/migrations directory.", } cmd.AddCommand(newUp(deps, executor)) + cmd.AddCommand(newBaseline(deps)) return cmd } @@ -183,7 +184,8 @@ func runUp(ctx context.Context, opts upOptions) error { // A delegated grant never reads the connection string: the server applies // the files instead. See runs.go. if sessions := clisession.NewFactory(opts.deps); usesMigrationRuns(sessions) { - return applyThroughRun(ctx, sessions, opts.database, migrationFiles, opts.out) + baselineCommand := cliruntime.CommandPath(opts.deps, "databases migration baseline -d "+opts.database) + return applyThroughRun(ctx, sessions, opts.database, migrationFiles, baselineCommand, opts.out) } database, err := clidatabase.NewService(opts.deps).Get(ctx, opts.database) diff --git a/internal/cmd/databases/migration/runs.go b/internal/cmd/databases/migration/runs.go index fc38b709..5892abb5 100644 --- a/internal/cmd/databases/migration/runs.go +++ b/internal/cmd/databases/migration/runs.go @@ -17,6 +17,7 @@ import ( "github.com/Kong/volcano-cli/internal/api" "github.com/Kong/volcano-cli/internal/apiclient" "github.com/Kong/volcano-cli/internal/config" + "github.com/Kong/volcano-cli/internal/sensitive" clisession "github.com/Kong/volcano-cli/internal/session" ) @@ -57,7 +58,11 @@ func usesMigrationRuns(sessions clisession.Factory) bool { // applyThroughRun applies files to database with a server-side migration run // and follows the run to its end, printing what became of each file. -func applyThroughRun(ctx context.Context, sessions clisession.Factory, database string, files []string, out io.Writer) error { +// baselineCommand is the command that records files as applied, named in the +// hint for a file that failed because its objects already exist. +func applyThroughRun( + ctx context.Context, sessions clisession.Factory, database string, files []string, baselineCommand string, out io.Writer, +) error { project, err := sessions.CurrentProject() if err != nil { return err @@ -78,7 +83,7 @@ func applyThroughRun(ctx context.Context, sessions clisession.Factory, database if err != nil { return err } - return reportRun(run, out) + return reportRun(run, baselineCommand, out) } // migrationRunFiles reads files into a run request. Names go by base name, @@ -194,9 +199,11 @@ func sleep(ctx context.Context, d time.Duration) error { } // reportRun prints each file's outcome and the run's warnings, and returns an -// error for a failed run. A file's database error is printed, never put in the -// returned error: it can quote SQL and data. -func reportRun(run *apiclient.DatabaseMigrationRun, out io.Writer) error { +// error for a failed run. That error carries each failed file's own database +// error, from the rehearsal or the database, which names the object, so the +// caller can decide what to baseline. It is marked sensitive: it can quote SQL +// and data, so usage telemetry never sends it. +func reportRun(run *apiclient.DatabaseMigrationRun, baselineCommand string, out io.Writer) error { fmt.Fprintln(out) if run.Files != nil { for _, file := range *run.Files { @@ -216,10 +223,48 @@ func reportRun(run *apiclient.DatabaseMigrationRun, out io.Writer) error { fmt.Fprintln(out, "Migrations deployed!") return nil } + summary := fmt.Sprintf("migration run %s failed in %s", run.Id, run.Phase) if run.Error != nil { - return fmt.Errorf("migration run %s failed in %s: %s: %s", run.Id, run.Phase, run.Error.Code, run.Error.Message) + summary += fmt.Sprintf(": %s: %s", run.Error.Code, run.Error.Message) } - return fmt.Errorf("migration run %s failed in %s", run.Id, run.Phase) + details := failedFileDetails(run, baselineCommand) + if details == "" { + return errors.New(summary) + } + return fmt.Errorf("%s\n%w", summary, sensitive.Wrap(errors.New(details))) +} + +// duplicateObjectStates are the SQLSTATEs Postgres answers when an object a +// file creates already exists. +var duplicateObjectStates = map[string]bool{ + "42P04": true, // duplicate_database + "42P06": true, // duplicate_schema + "42P07": true, // duplicate_table + "42701": true, // duplicate_column + "42710": true, // duplicate_object + "42712": true, // duplicate_alias + "42723": true, // duplicate_function +} + +// failedFileDetails is each failed file's database error, and for a file that +// failed because an object already exists, how to record it as applied. +func failedFileDetails(run *apiclient.DatabaseMigrationRun, baselineCommand string) string { + if run.Files == nil { + return "" + } + var lines []string + for _, file := range *run.Files { + if file.Error == nil { + continue + } + lines = append(lines, " "+file.Name+": "+fileErrorText(file.Error)) + if file.Error.Sqlstate != nil && duplicateObjectStates[*file.Error.Sqlstate] { + lines = append(lines, fmt.Sprintf(" The database already holds an object %s creates. If it holds everything "+ + "the file creates, record it as applied with `%s -f %s --yes`, then run this again.", + file.Name, baselineCommand, file.Name)) + } + } + return strings.Join(lines, "\n") } func fileErrorText(fileErr *apiclient.DatabaseMigrationFileError) string { diff --git a/internal/cmd/databases/migration/runs_test.go b/internal/cmd/databases/migration/runs_test.go index 6953d363..cd592359 100644 --- a/internal/cmd/databases/migration/runs_test.go +++ b/internal/cmd/databases/migration/runs_test.go @@ -18,6 +18,7 @@ import ( "github.com/Kong/volcano-cli/internal/api" cliconfig "github.com/Kong/volcano-cli/internal/config" cliruntime "github.com/Kong/volcano-cli/internal/runtime" + "github.com/Kong/volcano-cli/internal/sensitive" ) const ( @@ -203,9 +204,35 @@ func TestMigrationUpUnderGrantReportsAFailedRun(t *testing.T) { out, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") require.Error(t, err) - assert.Equal(t, "migration run "+testRunID+" failed in rehearse: migration_failed: 002_second.sql failed at line 1 (SQLSTATE 42P07)", err.Error()) assert.Contains(t, out, `relation "second" already exists (line 1, SQLSTATE 42P07)`) - assert.NotContains(t, err.Error(), "already exists", "the database's own message stays out of the error") + // The printed error names the object, so the caller can decide what to + // baseline, and says how. + assert.Equal(t, "migration run "+testRunID+" failed in rehearse: migration_failed: 002_second.sql failed at line 1 (SQLSTATE 42P07)\n"+ + ` 002_second.sql: relation "second" already exists (line 1, SQLSTATE 42P07)`+"\n"+ + " The database already holds an object 002_second.sql creates. If it holds everything the file creates, "+ + "record it as applied with `volcano databases migration baseline -d app -f 002_second.sql --yes`, then run this again.", + err.Error()) + // The database's message can quote SQL and data: usage telemetry never + // sends it. + assert.Contains(t, sensitive.Text(err), `relation "second" already exists`) +} + +// A file that fails for another reason gets its error, and no baseline hint. +func TestMigrationUpUnderGrantNamesOtherFileErrorsWithoutAHint(t *testing.T) { + setGrantMigrationTest(t) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){acceptRun(testRunID)}, states: map[string][]map[string]any{ + testRunID: {runState(testRunID, "failed", "apply", map[string]any{ + "error": map[string]any{"code": "migration_failed", "message": "001_first.sql failed (SQLSTATE 42601)"}, + "files": []any{map[string]any{"name": "001_first.sql", "outcome": "failed", "error": map[string]any{ + "message": `syntax error at or near "TABL"`, "sqlstate": "42601", + }}}, + })}, + }} + + _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.Error(t, err) + assert.Contains(t, err.Error(), `001_first.sql: syntax error at or near "TABL" (SQLSTATE 42601)`) + assert.NotContains(t, err.Error(), "baseline") } // Where migration runs are off, a grant fails and never falls back to the From 2dcb791ecde1541e356cb18000052ab69dc26fc0 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 23:22:01 -0500 Subject: [PATCH 04/11] refactor(databases): leave run conflicts and name checks to Hosting --- internal/cmd/databases/migration/migration.go | 3 +- internal/cmd/databases/migration/runs.go | 76 ++++--------------- internal/cmd/databases/migration/runs_test.go | 76 ++++--------------- 3 files changed, 31 insertions(+), 124 deletions(-) diff --git a/internal/cmd/databases/migration/migration.go b/internal/cmd/databases/migration/migration.go index 665895f1..d256eda1 100644 --- a/internal/cmd/databases/migration/migration.go +++ b/internal/cmd/databases/migration/migration.go @@ -184,8 +184,7 @@ func runUp(ctx context.Context, opts upOptions) error { // A delegated grant never reads the connection string: the server applies // the files instead. See runs.go. if sessions := clisession.NewFactory(opts.deps); usesMigrationRuns(sessions) { - baselineCommand := cliruntime.CommandPath(opts.deps, "databases migration baseline -d "+opts.database) - return applyThroughRun(ctx, sessions, opts.database, migrationFiles, baselineCommand, opts.out) + return applyThroughRun(ctx, sessions, opts.database, migrationFiles, opts.out) } database, err := clidatabase.NewService(opts.deps).Get(ctx, opts.database) diff --git a/internal/cmd/databases/migration/runs.go b/internal/cmd/databases/migration/runs.go index 5892abb5..463534c8 100644 --- a/internal/cmd/databases/migration/runs.go +++ b/internal/cmd/databases/migration/runs.go @@ -8,7 +8,6 @@ import ( "net/http" "os" "path/filepath" - "regexp" "strings" "time" @@ -45,10 +44,6 @@ var runWaitLimit = 60 * time.Minute // the command stops following a run. const maxRunReadFailures = 5 -// trackedMigrationName is the name rule the API applies to a top-level file: -// lowercase ".sql", no directory. -var trackedMigrationName = regexp.MustCompile(`^[^/\\]+\.sql$`) - // usesMigrationRuns reports whether up applies files through migration runs: // when the credential is a delegated grant. func usesMigrationRuns(sessions clisession.Factory) bool { @@ -58,11 +53,7 @@ func usesMigrationRuns(sessions clisession.Factory) bool { // applyThroughRun applies files to database with a server-side migration run // and follows the run to its end, printing what became of each file. -// baselineCommand is the command that records files as applied, named in the -// hint for a file that failed because its objects already exist. -func applyThroughRun( - ctx context.Context, sessions clisession.Factory, database string, files []string, baselineCommand string, out io.Writer, -) error { +func applyThroughRun(ctx context.Context, sessions clisession.Factory, database string, files []string, out io.Writer) error { project, err := sessions.CurrentProject() if err != nil { return err @@ -83,7 +74,7 @@ func applyThroughRun( if err != nil { return err } - return reportRun(run, baselineCommand, out) + return reportRun(run, out) } // migrationRunFiles reads files into a run request. Names go by base name, @@ -92,9 +83,6 @@ func migrationRunFiles(files []string) ([]apiclient.DatabaseMigrationFile, error request := make([]apiclient.DatabaseMigrationFile, 0, len(files)) for _, file := range files { name := filepath.Base(file) - if !trackedMigrationName.MatchString(name) { - return nil, fmt.Errorf("migration %s: a migration run takes only names ending in lowercase .sql; rename it", name) - } content, err := os.ReadFile(file) if err != nil { return nil, fmt.Errorf("failed to read migration %s: %w", name, err) @@ -104,35 +92,18 @@ func migrationRunFiles(files []string) ([]apiclient.DatabaseMigrationFile, error return request, nil } -// startRun starts the run. When the database already has a run going, it -// follows that run to its end and then starts this one: a run started by an -// earlier command whose grant expired while it waited ends first, and the new -// run skips whatever it applied. +// startRun starts the run. A run already going on the database is the API's +// 409, which names it; the caller waits for it and runs the command again. func startRun( ctx context.Context, project *clisession.ProjectSession, database string, files []apiclient.DatabaseMigrationFile, out io.Writer, ) (*apiclient.DatabaseMigrationRun, error) { - // One key per command: a retried request returns the run this command - // started, and running the command again starts a new one. - key := uuid.NewString() - for attempt := 0; ; attempt++ { - run, err := project.API.CreateDatabaseMigrationRun(ctx, project.ProjectID, database, key, files) - if err == nil { - fmt.Fprintf(out, "Started migration run %s.\n", run.Id) - return run, nil - } - inProgress, ok := errors.AsType[*api.MigrationRunInProgressError](err) - if !ok || attempt > 0 { - return nil, migrationRunStartError(database, err) - } - fmt.Fprintf(out, "Migration run %s is already running on %s; waiting for it to end first.\n", inProgress.RunID, database) - earlier, err := project.API.GetDatabaseMigrationRun(ctx, project.ProjectID, database, inProgress.RunID) - if err != nil { - return nil, fmt.Errorf("failed to read migration run %s: %w", inProgress.RunID, err) - } - if _, err := followRun(ctx, project, database, earlier, out); err != nil { - return nil, err - } + // A retried request with this key returns the run it started. + run, err := project.API.CreateDatabaseMigrationRun(ctx, project.ProjectID, database, uuid.NewString(), files) + if err != nil { + return nil, migrationRunStartError(database, err) } + fmt.Fprintf(out, "Started migration run %s.\n", run.Id) + return run, nil } // migrationRunStartError says why a run did not start. @@ -203,7 +174,7 @@ func sleep(ctx context.Context, d time.Duration) error { // error, from the rehearsal or the database, which names the object, so the // caller can decide what to baseline. It is marked sensitive: it can quote SQL // and data, so usage telemetry never sends it. -func reportRun(run *apiclient.DatabaseMigrationRun, baselineCommand string, out io.Writer) error { +func reportRun(run *apiclient.DatabaseMigrationRun, out io.Writer) error { fmt.Fprintln(out) if run.Files != nil { for _, file := range *run.Files { @@ -227,28 +198,16 @@ func reportRun(run *apiclient.DatabaseMigrationRun, baselineCommand string, out if run.Error != nil { summary += fmt.Sprintf(": %s: %s", run.Error.Code, run.Error.Message) } - details := failedFileDetails(run, baselineCommand) + details := failedFileDetails(run) if details == "" { return errors.New(summary) } return fmt.Errorf("%s\n%w", summary, sensitive.Wrap(errors.New(details))) } -// duplicateObjectStates are the SQLSTATEs Postgres answers when an object a -// file creates already exists. -var duplicateObjectStates = map[string]bool{ - "42P04": true, // duplicate_database - "42P06": true, // duplicate_schema - "42P07": true, // duplicate_table - "42701": true, // duplicate_column - "42710": true, // duplicate_object - "42712": true, // duplicate_alias - "42723": true, // duplicate_function -} - -// failedFileDetails is each failed file's database error, and for a file that -// failed because an object already exists, how to record it as applied. -func failedFileDetails(run *apiclient.DatabaseMigrationRun, baselineCommand string) string { +// failedFileDetails is each failed file's database error, which names the +// object, so the caller can decide what to baseline. +func failedFileDetails(run *apiclient.DatabaseMigrationRun) string { if run.Files == nil { return "" } @@ -258,11 +217,6 @@ func failedFileDetails(run *apiclient.DatabaseMigrationRun, baselineCommand stri continue } lines = append(lines, " "+file.Name+": "+fileErrorText(file.Error)) - if file.Error.Sqlstate != nil && duplicateObjectStates[*file.Error.Sqlstate] { - lines = append(lines, fmt.Sprintf(" The database already holds an object %s creates. If it holds everything "+ - "the file creates, record it as applied with `%s -f %s --yes`, then run this again.", - file.Name, baselineCommand, file.Name)) - } } return strings.Join(lines, "\n") } diff --git a/internal/cmd/databases/migration/runs_test.go b/internal/cmd/databases/migration/runs_test.go index cd592359..a36b4142 100644 --- a/internal/cmd/databases/migration/runs_test.go +++ b/internal/cmd/databases/migration/runs_test.go @@ -206,35 +206,15 @@ func TestMigrationUpUnderGrantReportsAFailedRun(t *testing.T) { require.Error(t, err) assert.Contains(t, out, `relation "second" already exists (line 1, SQLSTATE 42P07)`) // The printed error names the object, so the caller can decide what to - // baseline, and says how. + // baseline. assert.Equal(t, "migration run "+testRunID+" failed in rehearse: migration_failed: 002_second.sql failed at line 1 (SQLSTATE 42P07)\n"+ - ` 002_second.sql: relation "second" already exists (line 1, SQLSTATE 42P07)`+"\n"+ - " The database already holds an object 002_second.sql creates. If it holds everything the file creates, "+ - "record it as applied with `volcano databases migration baseline -d app -f 002_second.sql --yes`, then run this again.", + ` 002_second.sql: relation "second" already exists (line 1, SQLSTATE 42P07)`, err.Error()) // The database's message can quote SQL and data: usage telemetry never // sends it. assert.Contains(t, sensitive.Text(err), `relation "second" already exists`) } -// A file that fails for another reason gets its error, and no baseline hint. -func TestMigrationUpUnderGrantNamesOtherFileErrorsWithoutAHint(t *testing.T) { - setGrantMigrationTest(t) - fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){acceptRun(testRunID)}, states: map[string][]map[string]any{ - testRunID: {runState(testRunID, "failed", "apply", map[string]any{ - "error": map[string]any{"code": "migration_failed", "message": "001_first.sql failed (SQLSTATE 42601)"}, - "files": []any{map[string]any{"name": "001_first.sql", "outcome": "failed", "error": map[string]any{ - "message": `syntax error at or near "TABL"`, "sqlstate": "42601", - }}}, - })}, - }} - - _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") - require.Error(t, err) - assert.Contains(t, err.Error(), `001_first.sql: syntax error at or near "TABL" (SQLSTATE 42601)`) - assert.NotContains(t, err.Error(), "baseline") -} - // Where migration runs are off, a grant fails and never falls back to the // connection string. func TestMigrationUpUnderGrantRefusesWhenRunsAreOff(t *testing.T) { @@ -250,37 +230,21 @@ func TestMigrationUpUnderGrantRefusesWhenRunsAreOff(t *testing.T) { assert.Equal(t, []string{"POST " + migrationRunsPath}, fake.requests) } -// A run an earlier command started is still going: wait for it, then start -// this command's run once. -func TestMigrationUpUnderGrantWaitsForARunInProgress(t *testing.T) { +// A run already going on the database is Hosting's 409: the command fails and +// names that run, and the caller runs it again once that run ends. +func TestMigrationUpUnderGrantNamesARunInProgress(t *testing.T) { setGrantMigrationTest(t) - fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){ - func(w http.ResponseWriter) { - writeRunJSON(w, http.StatusConflict, map[string]any{ - "error": "a migration run is in progress", "code": "migration_run_in_progress", "run_id": testEarlierRunID, - }) - }, - acceptRun(testRunID), - }, states: map[string][]map[string]any{ - testEarlierRunID: { - runState(testEarlierRunID, "running", "apply", nil), - runState(testEarlierRunID, "succeeded", "apply", nil), - }, - testRunID: {runState(testRunID, "succeeded", "apply", nil)}, - }} + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){func(w http.ResponseWriter) { + writeRunJSON(w, http.StatusConflict, map[string]any{ + "error": "a migration run is in progress", "code": "migration_run_in_progress", "run_id": testEarlierRunID, + }) + }}} - out, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") - require.NoError(t, err) - assert.Contains(t, out, "Migration run "+testEarlierRunID+" is already running on app; waiting for it to end first.") - assert.Equal(t, []string{ - "POST " + migrationRunsPath, - "GET " + migrationRunsPath + "/" + testEarlierRunID, - "GET " + migrationRunsPath + "/" + testEarlierRunID, - "POST " + migrationRunsPath, - "GET " + migrationRunsPath + "/" + testRunID, - }, fake.requests) - require.Len(t, fake.keys, 2) - assert.Equal(t, fake.keys[0], fake.keys[1], "one command, one key") + _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.Error(t, err) + assert.Contains(t, err.Error(), "a migration run is in progress (run "+testEarlierRunID+")") + assert.Equal(t, http.StatusConflict, api.Status(err)) + assert.Equal(t, []string{"POST " + migrationRunsPath}, fake.requests) } // The grant expires while the run goes on: the command says the run carries on. @@ -306,16 +270,6 @@ func TestMigrationUpUnderGrantStopsOnAnExpiredGrant(t *testing.T) { assert.NotContains(t, out, "grant-secret") } -func TestMigrationUpUnderGrantRefusesAnUppercaseExtension(t *testing.T) { - setGrantMigrationTest(t) - require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "003_third.SQL"), []byte("SELECT 1;"), 0o644)) - fake := &fakeMigrationRuns{t: t} - - _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") - require.ErrorContains(t, err, "003_third.SQL: a migration run takes only names ending in lowercase .sql") - assert.Empty(t, fake.requests) -} - // `deploy --with-migrations` applies through ApplyAll, which takes the same // path under a grant. func TestApplyAllUsesRunsUnderGrant(t *testing.T) { From 0880d09914610c520d62f9430e3fd660fc225dac Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 23:30:48 -0500 Subject: [PATCH 05/11] style(databases): format the baseline command --- internal/cmd/databases/migration/baseline.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/databases/migration/baseline.go b/internal/cmd/databases/migration/baseline.go index 6b612170..7b5505fd 100644 --- a/internal/cmd/databases/migration/baseline.go +++ b/internal/cmd/databases/migration/baseline.go @@ -157,7 +157,7 @@ func confirmBaseline(in io.Reader, w io.Writer, database string, files []string) // selectBaselineFiles picks the files a baseline records: all of them, or the // ones named, each of which has to exist. -func selectBaselineFiles(all []string, names []string, everything bool) ([]string, error) { +func selectBaselineFiles(all, names []string, everything bool) ([]string, error) { if len(all) == 0 { return nil, errors.New("no migration files found in volcano/migrations/") } From 82acba6a8551a973a586ecc1a02864efd74cd348 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Wed, 11 Nov 2026 23:40:43 -0500 Subject: [PATCH 06/11] test(databases): drop an unused parameter --- internal/cmd/databases/migration/baseline_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/databases/migration/baseline_test.go b/internal/cmd/databases/migration/baseline_test.go index 9b51e4e2..12b12c5c 100644 --- a/internal/cmd/databases/migration/baseline_test.go +++ b/internal/cmd/databases/migration/baseline_test.go @@ -120,7 +120,7 @@ func TestBaselineKeepsTheAPIRefusal(t *testing.T) { func TestBaselineRefusesAnUnknownFile(t *testing.T) { setGrantMigrationTest(t) require.NoError(t, os.WriteFile(filepath.Join("volcano", "migrations", "notes.txt"), []byte("x"), 0o644)) - server, requests := baselineServer(t, func(w http.ResponseWriter, _ []any) {}) + server, requests := baselineServer(t, func(http.ResponseWriter, []any) {}) _, err := runBaselineCommand(t, server, "-d", "app", "-f", "009_missing", "--yes") require.ErrorContains(t, err, `migration "009_missing" not found`) From 9f990f30f6055a9536330b3221f209517980b649 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Thu, 12 Nov 2026 17:19:37 -0500 Subject: [PATCH 07/11] test(deploy): match the grant route table to Hosting's current table --- internal/cmd/deploy/grant_test.go | 47 ++++++++----------------------- 1 file changed, 12 insertions(+), 35 deletions(-) diff --git a/internal/cmd/deploy/grant_test.go b/internal/cmd/deploy/grant_test.go index b52b444b..b2c23ef3 100644 --- a/internal/cmd/deploy/grant_test.go +++ b/internal/cmd/deploy/grant_test.go @@ -2,7 +2,6 @@ package deploy import ( "net/http" - "slices" "strings" "sync" "testing" @@ -16,8 +15,9 @@ import ( // grantRouteTable is Hosting's delegated grant route table, copied from // DefaultGrantRoutes() in internal/middleware/delegated_grant_routes.go at -// Kong/volcano-hosting 1749144404 (C3, #3313). A pg- grant is refused on any -// other route with 403 grant_route_not_allowed. Update it with that file. +// Kong/volcano-hosting cf392bf5e5 (tkkhq/grant-acceptance-routes, C2, #3312). +// A pg- grant is refused on any other route with 403 grant_route_not_allowed. +// Update it with that file. var grantRouteTable = []string{ "GET /projects/{id}", "GET /projects/{id}/capabilities", "GET /projects/{id}/config", "GET /projects/{id}/deployments", "POST /projects/{id}/logs/search", "POST /projects/{id}/logs/stream", @@ -27,6 +27,7 @@ var grantRouteTable = []string{ "GET /projects/{id}/durable-functions", "GET /projects/{id}/durable-functions/{functionId}", "GET /projects/{id}/frontends", "GET /projects/{id}/frontends/{frontendId}", "GET /projects/{id}/frontends/{frontendId}/deployments", "GET /projects/{id}/frontends/{frontendId}/serving", + "GET /projects/{id}/source-export", "POST /projects/{id}/functions", "POST /projects/{id}/functions/batch", "PATCH /projects/{id}/functions/{functionId}", "POST /projects/{id}/functions/{functionId}/schedulers", "PATCH /projects/{id}/functions/{functionId}/schedulers/{schedulerId}", @@ -37,6 +38,7 @@ var grantRouteTable = []string{ "GET /projects/{id}/databases/{databaseName}/migrations", "GET /projects/{id}/databases/{databaseName}/migration-runs", "GET /projects/{id}/databases/{databaseName}/migration-runs/{runId}", "POST /projects/{id}/databases/{databaseName}/migration-runs", + "POST /projects/{id}/databases/{databaseName}/migrations/baseline", "POST /projects/{id}/frontends", "POST /projects/{id}/frontends/{frontendId}/deployments", "POST /projects/{id}/frontends/{frontendId}/redeploy", } @@ -74,15 +76,13 @@ type grantGate struct { mu sync.Mutex refused []string auth []string - // also lets a grant call these "METHOD path" requests too. - also []string - next http.HandlerFunc + next http.HandlerFunc } func (g *grantGate) ServeHTTP(w http.ResponseWriter, r *http.Request) { g.mu.Lock() g.auth = append(g.auth, r.Header.Get("Authorization")) - if !grantMayCall(r.Method, r.URL.Path) && !slices.Contains(g.also, r.Method+" "+r.URL.Path) { + if !grantMayCall(r.Method, r.URL.Path) { g.refused = append(g.refused, r.Method+" "+r.URL.Path) g.mu.Unlock() w.Header().Set("Content-Type", "application/json") @@ -94,13 +94,9 @@ func (g *grantGate) ServeHTTP(w http.ResponseWriter, r *http.Request) { g.next(w, r) } -// sourceExportRead is the read `volcano cloud deploy` makes before deploying -// code, to refuse a project that deploys from Git. It is not on the grant -// table yet, so a whole-project deploy fails under a grant until Hosting adds -// it (as project.read). Per-resource deploys don't make it. -const sourceExportRead = "GET /projects/" + deployProjectID + "/source-export" - -func TestDeployUnderAGrantIsRefusedTheSourceModeRead(t *testing.T) { +// Every request a whole-project deploy makes, the source mode read and +// migrations included, is on the grant table, and carries the grant. +func TestDeployUnderAGrantCallsOnlyGrantRoutes(t *testing.T) { setDeployCommandTestHome(t) t.Setenv("VOLCANO_TOKEN", cliconfig.DelegatedGrantPrefix+"grant-secret") writeFullProject(t) @@ -109,25 +105,6 @@ func TestDeployUnderAGrantIsRefusedTheSourceModeRead(t *testing.T) { server := fake.start() server.Config.Handler = gate - out, err := executeDeployCommand(t, New(fake.cloudDeps(server))) - require.Error(t, err) - assert.Equal(t, []string{sourceExportRead}, gate.refused) - assert.Empty(t, fake.recorded(), "refused before any change") - assert.NotContains(t, out, "grant-secret") -} - -// With the source mode read allowed, every other request a whole-project -// deploy makes, migrations included, is on the grant table, and carries the -// grant. -func TestDeployUnderAGrantCallsOnlyGrantRoutesOtherwise(t *testing.T) { - setDeployCommandTestHome(t) - t.Setenv("VOLCANO_TOKEN", cliconfig.DelegatedGrantPrefix+"grant-secret") - writeFullProject(t) - fake := newFakeVolcano(t) - gate := &grantGate{next: fake.serve, also: []string{sourceExportRead}} - server := fake.start() - server.Config.Handler = gate - out, err := executeDeployCommand(t, New(fake.cloudDeps(server))) require.NoError(t, err, out) assert.Empty(t, gate.refused) @@ -148,7 +125,7 @@ func TestDeployWithMigrationsUnderAGrantUsesAMigrationRun(t *testing.T) { runs := "/projects/" + deployProjectID + "/databases/app/migration-runs" const runID = "44444444-4444-4444-8444-444444444444" var runCalls []string - gate := &grantGate{also: []string{sourceExportRead}, next: func(w http.ResponseWriter, r *http.Request) { + gate := &grantGate{next: func(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(r.URL.Path, runs) { runCalls = append(runCalls, r.Method+" "+r.URL.Path) w.Header().Set("Content-Type", "application/json") @@ -189,7 +166,7 @@ func TestDeployUnderAGrantKeepsTheBudgetRefusalOnEveryMutation(t *testing.T) { writeFullProject(t) fake := newFakeVolcano(t) var hit bool - gate := &grantGate{also: []string{sourceExportRead}, next: func(w http.ResponseWriter, r *http.Request) { + gate := &grantGate{next: func(w http.ResponseWriter, r *http.Request) { // The config step's dry run is a read in all but method. if r.Method+" "+r.URL.Path == route && r.URL.Query().Get("dry_run") != "true" { hit = true From 6b8537d471acd26adcc96da6e05ffe0db06c8605 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Thu, 12 Nov 2026 17:25:41 -0500 Subject: [PATCH 08/11] test(deploy): copy the grant route table from Hosting's invoke branch --- internal/cmd/deploy/grant_test.go | 130 ++++++++++++++++++++---------- 1 file changed, 87 insertions(+), 43 deletions(-) diff --git a/internal/cmd/deploy/grant_test.go b/internal/cmd/deploy/grant_test.go index b2c23ef3..65801950 100644 --- a/internal/cmd/deploy/grant_test.go +++ b/internal/cmd/deploy/grant_test.go @@ -2,6 +2,8 @@ package deploy import ( "net/http" + "slices" + "strconv" "strings" "sync" "testing" @@ -13,47 +15,83 @@ import ( cliconfig "github.com/Kong/volcano-cli/internal/config" ) -// grantRouteTable is Hosting's delegated grant route table, copied from -// DefaultGrantRoutes() in internal/middleware/delegated_grant_routes.go at -// Kong/volcano-hosting cf392bf5e5 (tkkhq/grant-acceptance-routes, C2, #3312). -// A pg- grant is refused on any other route with 403 grant_route_not_allowed. -// Update it with that file. -var grantRouteTable = []string{ - "GET /projects/{id}", "GET /projects/{id}/capabilities", "GET /projects/{id}/config", - "GET /projects/{id}/deployments", "POST /projects/{id}/logs/search", "POST /projects/{id}/logs/stream", - "GET /projects/{id}/functions", "GET /projects/{id}/functions/{functionId}", - "GET /projects/{id}/functions/{functionId}/deployments", "GET /projects/{id}/functions/{functionId}/schedulers", - "GET /projects/{id}/functions/{functionId}/schedulers/{schedulerId}", - "GET /projects/{id}/durable-functions", "GET /projects/{id}/durable-functions/{functionId}", - "GET /projects/{id}/frontends", "GET /projects/{id}/frontends/{frontendId}", - "GET /projects/{id}/frontends/{frontendId}/deployments", "GET /projects/{id}/frontends/{frontendId}/serving", - "GET /projects/{id}/source-export", - "POST /projects/{id}/functions", "POST /projects/{id}/functions/batch", "PATCH /projects/{id}/functions/{functionId}", - "POST /projects/{id}/functions/{functionId}/schedulers", - "PATCH /projects/{id}/functions/{functionId}/schedulers/{schedulerId}", - "DELETE /projects/{id}/functions/{functionId}/schedulers/{schedulerId}", - "POST /projects/{id}/durable-functions", - "PUT /projects/{id}/config", "POST /projects/{id}/variables", "PUT /projects/{id}/variables/{name}", - "DELETE /projects/{id}/variables/{name}", - "GET /projects/{id}/databases/{databaseName}/migrations", "GET /projects/{id}/databases/{databaseName}/migration-runs", - "GET /projects/{id}/databases/{databaseName}/migration-runs/{runId}", - "POST /projects/{id}/databases/{databaseName}/migration-runs", - "POST /projects/{id}/databases/{databaseName}/migrations/baseline", - "POST /projects/{id}/frontends", "POST /projects/{id}/frontends/{frontendId}/deployments", - "POST /projects/{id}/frontends/{frontendId}/redeploy", +// The kinds of grant route, as Hosting's GrantRouteKind. A read and an invoke +// spend no mutation; a mutation spends one. +const ( + grantRead = "read" + grantInvoke = "invoke" + grantMutation = "mutation" +) + +// grantRouteTable is Hosting's delegated grant route table, "METHOD template" +// to kind, copied from DefaultGrantRoutes() in +// internal/middleware/delegated_grant_routes.go at Kong/volcano-hosting +// 2f42b4c314 (tkkhq/grant-acceptance-invoke, C6, #3329), the top of the grant +// chain. A pg- grant is refused on any other route with 403 +// grant_route_not_allowed. Update it with that file. +var grantRouteTable = map[string]string{ + "GET /projects/{id}": grantRead, + "GET /projects/{id}/capabilities": grantRead, + "GET /projects/{id}/config": grantRead, + "GET /projects/{id}/deployments": grantRead, + "POST /projects/{id}/logs/search": grantRead, + "POST /projects/{id}/logs/stream": grantRead, + "GET /projects/{id}/functions": grantRead, + "GET /projects/{id}/functions/{functionId}": grantRead, + "GET /projects/{id}/functions/{functionId}/deployments": grantRead, + "GET /projects/{id}/functions/{functionId}/schedulers": grantRead, + "GET /projects/{id}/functions/{functionId}/schedulers/{schedulerId}": grantRead, + "GET /projects/{id}/durable-functions": grantRead, + "GET /projects/{id}/durable-functions/{functionId}": grantRead, + "GET /projects/{id}/frontends": grantRead, + "GET /projects/{id}/frontends/{frontendId}": grantRead, + "GET /projects/{id}/frontends/{frontendId}/deployments": grantRead, + "GET /projects/{id}/frontends/{frontendId}/serving": grantRead, + "GET /projects/{id}/source-export": grantRead, + + "POST /projects/{id}/functions": grantMutation, + "POST /projects/{id}/functions/batch": grantMutation, + "PATCH /projects/{id}/functions/{functionId}": grantMutation, + "POST /projects/{id}/functions/{functionId}/schedulers": grantMutation, + "PATCH /projects/{id}/functions/{functionId}/schedulers/{schedulerId}": grantMutation, + "DELETE /projects/{id}/functions/{functionId}/schedulers/{schedulerId}": grantMutation, + "POST /projects/{id}/durable-functions": grantMutation, + + "POST /projects/{id}/durable-functions/{functionId}/executions": grantInvoke, + "GET /projects/{id}/durable-functions/{functionId}/executions/{executionId}": grantRead, + + "PUT /projects/{id}/config": grantMutation, + "POST /projects/{id}/variables": grantMutation, + "PUT /projects/{id}/variables/{name}": grantMutation, + "DELETE /projects/{id}/variables/{name}": grantMutation, + + "GET /projects/{id}/databases/{databaseName}/migrations": grantRead, + "GET /projects/{id}/databases/{databaseName}/migration-runs": grantRead, + "GET /projects/{id}/databases/{databaseName}/migration-runs/{runId}": grantRead, + "POST /projects/{id}/databases/{databaseName}/migration-runs": grantMutation, + "POST /projects/{id}/databases/{databaseName}/migrations/baseline": grantMutation, + + "POST /projects/{id}/frontends": grantMutation, + "POST /projects/{id}/frontends/{frontendId}/deployments": grantMutation, + "POST /projects/{id}/frontends/{frontendId}/redeploy": grantMutation, } -// grantMayCall reports whether a grant may call method and path. The public -// runtime catalog runs no auth middleware, so the grant branch never sees it. -func grantMayCall(method, path string) bool { - if method == http.MethodGet && path == "/functions/runtimes" { - return true +// grantDryRunReads are the mutations whose dry_run=true request is a read, as +// Hosting's GrantRoute.DryRunReads. Only config apply has one. +var grantDryRunReads = []string{"PUT /projects/{id}/config"} + +// grantRouteKind returns the kind of r on the grant table, and false when the +// table has no route for it. The public runtime catalog runs no auth +// middleware, so the grant branch never sees it. +func grantRouteKind(r *http.Request) (string, bool) { + if r.Method == http.MethodGet && r.URL.Path == "/functions/runtimes" { + return grantRead, true } - got := strings.Split(path, "/") - for _, entry := range grantRouteTable { - entryMethod, template, _ := strings.Cut(entry, " ") + got := strings.Split(r.URL.Path, "/") + for entry, kind := range grantRouteTable { + method, template, _ := strings.Cut(entry, " ") want := strings.Split(template, "/") - if entryMethod != method || len(want) != len(got) { + if method != r.Method || len(want) != len(got) { continue } match := true @@ -63,11 +101,16 @@ func grantMayCall(method, path string) bool { break } } - if match { - return true + if !match { + continue + } + if dryRun, err := strconv.ParseBool(r.URL.Query().Get("dry_run")); err == nil && dryRun && + slices.Contains(grantDryRunReads, entry) { + return grantRead, true } + return kind, true } - return false + return "", false } // grantGate refuses what Hosting refuses a grant, records it, and passes the @@ -82,7 +125,7 @@ type grantGate struct { func (g *grantGate) ServeHTTP(w http.ResponseWriter, r *http.Request) { g.mu.Lock() g.auth = append(g.auth, r.Header.Get("Authorization")) - if !grantMayCall(r.Method, r.URL.Path) { + if _, ok := grantRouteKind(r); !ok { g.refused = append(g.refused, r.Method+" "+r.URL.Path) g.mu.Unlock() w.Header().Set("Content-Type", "application/json") @@ -167,8 +210,9 @@ func TestDeployUnderAGrantKeepsTheBudgetRefusalOnEveryMutation(t *testing.T) { fake := newFakeVolcano(t) var hit bool gate := &grantGate{next: func(w http.ResponseWriter, r *http.Request) { - // The config step's dry run is a read in all but method. - if r.Method+" "+r.URL.Path == route && r.URL.Query().Get("dry_run") != "true" { + // The config step's dry run is a read on the grant table, so it + // spends nothing. + if kind, _ := grantRouteKind(r); r.Method+" "+r.URL.Path == route && kind == grantMutation { hit = true w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusTooManyRequests) From 8bf6de9c2359d14916617fdbe04623a314864076 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 08:11:55 -0500 Subject: [PATCH 09/11] fix(api): keep a migration run's body out of errors when the parser rejects it --- internal/api/database_migration_runs.go | 6 ++- internal/api/secret_redaction_test.go | 54 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/internal/api/database_migration_runs.go b/internal/api/database_migration_runs.go index 244c8804..a4776f8c 100644 --- a/internal/api/database_migration_runs.go +++ b/internal/api/database_migration_runs.go @@ -57,7 +57,9 @@ func (c *Client) CreateDatabaseMigrationRun( case resp.JSON409 != nil: return nil, migrationConflictError(resp.JSON409) } - return nil, apiErrorFromGeneratedErrors(resp.StatusCode(), resp.Body, resp.JSON400, resp.JSON404, resp.JSON413) + // A run the parser did not accept quotes database errors, which can quote + // data, so only an error body's own fields reach the error. + return nil, apiErrorFromGeneratedErrors(resp.StatusCode(), errorFieldsOnly(resp.Body), resp.JSON400, resp.JSON404, resp.JSON413) } // BaselineDatabaseMigrations records files in databaseName's migration @@ -101,5 +103,5 @@ func (c *Client) GetDatabaseMigrationRun( if err != nil { return nil, err } - return apiResult(resp.StatusCode(), resp.Body, resp.JSON200, resp.JSON404) + return apiResult(resp.StatusCode(), errorFieldsOnly(resp.Body), resp.JSON200, resp.JSON404) } diff --git a/internal/api/secret_redaction_test.go b/internal/api/secret_redaction_test.go index 25360eb5..215d4e29 100644 --- a/internal/api/secret_redaction_test.go +++ b/internal/api/secret_redaction_test.go @@ -184,3 +184,57 @@ func TestRedactedErrorsStillCarryTheStatus(t *testing.T) { assert.Equal(t, http.StatusOK, apiErr.StatusCode) assert.Contains(t, err.Error(), "200", "the status should still be reported") } + +// A migration run quotes the database's errors, which can quote data. A run +// the parser did not accept, such as one whose Content-Type was stripped on +// the way, must not become an error message, which usage telemetry sends. +func TestMigrationRunNeverPutsTheRunInAnError(t *testing.T) { + const quoted = "Key (email)=(alice@example.com) already exists" + body := `{"id":"11111111-1111-4111-8111-111111111111","status":"failed","phase":"apply",` + + `"files":[{"name":"001.sql","outcome":"failed","error":{"message":"` + quoted + `"}}]}` + for _, status := range []int{http.StatusAccepted, http.StatusOK} { + t.Run(http.StatusText(status), func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(server.Close) + client, err := NewClient(server.URL, "pg-grant", WithHTTPClient(server.Client())) + require.NoError(t, err) + + _, err = client.CreateDatabaseMigrationRun(context.Background(), redactionTestProjectID(t), "app", "key", nil) + require.Error(t, err, "an unparseable response must not read as success") + assert.NotContains(t, err.Error(), "alice") + assert.Equal(t, status, Status(err)) + + _, err = client.GetDatabaseMigrationRun(context.Background(), redactionTestProjectID(t), "app", + uuid.MustParse("11111111-1111-4111-8111-111111111111")) + require.Error(t, err) + assert.NotContains(t, err.Error(), "alice") + }) + } +} + +// Dropping the run must not cost a refusal its message or code, even one whose +// status the API does not declare for these routes. +func TestMigrationRunKeepsTheRefusalMessage(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte(`{"error":"the delegated grant has no mutations left","code":"grant_budget_exhausted"}`)) + })) + t.Cleanup(server.Close) + client, err := NewClient(server.URL, "pg-grant", WithHTTPClient(server.Client())) + require.NoError(t, err) + + _, err = client.CreateDatabaseMigrationRun(context.Background(), redactionTestProjectID(t), "app", "key", nil) + require.Error(t, err) + assert.Equal(t, http.StatusTooManyRequests, Status(err)) + assert.Equal(t, "grant_budget_exhausted", Code(err)) + assert.Contains(t, err.Error(), "the delegated grant has no mutations left") + + _, err = client.GetDatabaseMigrationRun(context.Background(), redactionTestProjectID(t), "app", uuid.New()) + require.Error(t, err) + assert.Equal(t, "grant_budget_exhausted", Code(err)) +} From 7dd72f54027d10dddc8d9737a23c2694fdb93ddb Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 08:12:17 -0500 Subject: [PATCH 10/11] fix(databases): tell a caller to wait for a run that carries on, not to rerun --- internal/cmd/databases/migration/runs.go | 9 +++++++-- internal/cmd/databases/migration/runs_test.go | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/internal/cmd/databases/migration/runs.go b/internal/cmd/databases/migration/runs.go index 463534c8..a6b4decb 100644 --- a/internal/cmd/databases/migration/runs.go +++ b/internal/cmd/databases/migration/runs.go @@ -40,6 +40,11 @@ var runPollInterval = 2 * time.Second // on the server after the command stops. var runWaitLimit = 60 * time.Minute +// rerunAfterRun tells a caller that stopped following a run what to do next. +// Running the command again starts a new run rather than following this one: +// while this one is going, Hosting refuses it with migration_run_in_progress. +const rerunAfterRun = "Wait for it to end before running the command again; the next run skips the files it applied" + // maxRunReadFailures is how many reads in a row may fail on the network before // the command stops following a run. const maxRunReadFailures = 5 @@ -132,7 +137,7 @@ func followRun( } if time.Now().After(deadline) { return nil, fmt.Errorf("stopped waiting for migration run %s after %s; it carries on on the server. "+ - "Run the command again to wait for it", run.Id, runWaitLimit) + "%s", run.Id, runWaitLimit, rerunAfterRun) } if err := sleep(ctx, runPollInterval); err != nil { return nil, err @@ -146,7 +151,7 @@ func followRun( failures++ default: return nil, fmt.Errorf("stopped following migration run %s, which carries on on the server. "+ - "Run the command again to wait for it: %w", run.Id, err) + "%s: %w", run.Id, rerunAfterRun, err) } } } diff --git a/internal/cmd/databases/migration/runs_test.go b/internal/cmd/databases/migration/runs_test.go index a36b4142..ff19ecce 100644 --- a/internal/cmd/databases/migration/runs_test.go +++ b/internal/cmd/databases/migration/runs_test.go @@ -266,10 +266,28 @@ func TestMigrationUpUnderGrantStopsOnAnExpiredGrant(t *testing.T) { out, err := executeMigrationCommand(t, cmd, "up", "--all", "-d", "app") require.Error(t, err) assert.Contains(t, err.Error(), "stopped following migration run "+testRunID+", which carries on on the server") + assert.Contains(t, err.Error(), "Wait for it to end before running the command again") + assert.NotContains(t, err.Error(), "again to wait for it", "running the command again does not follow this run") assert.Contains(t, out, "Started migration run "+testRunID) assert.NotContains(t, out, "grant-secret") } +// A run that outlasts the wait limit carries on; the command says to wait for +// it rather than run again, which would start a new run. +func TestMigrationUpUnderGrantStopsAtTheWaitLimit(t *testing.T) { + setGrantMigrationTest(t) + previous := runWaitLimit + runWaitLimit = 0 + t.Cleanup(func() { runWaitLimit = previous }) + fake := &fakeMigrationRuns{t: t, create: []func(http.ResponseWriter){acceptRun(testRunID)}} + + _, err := runGrantMigration(t, fake, "up", "--all", "-d", "app") + require.Error(t, err) + assert.Contains(t, err.Error(), "stopped waiting for migration run "+testRunID) + assert.Contains(t, err.Error(), "Wait for it to end before running the command again") + assert.NotContains(t, err.Error(), "again to wait for it") +} + // `deploy --with-migrations` applies through ApplyAll, which takes the same // path under a grant. func TestApplyAllUsesRunsUnderGrant(t *testing.T) { From 6c662fbb5cc43c856ae5ba5b478fc957c2656f14 Mon Sep 17 00:00:00 2001 From: Ted Kim Date: Fri, 13 Nov 2026 08:12:35 -0500 Subject: [PATCH 11/11] docs(databases): qualify the run's rehearsal and backup and document baseline --- docs/databases.md | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/docs/databases.md b/docs/databases.md index 36919996..5a2e27da 100644 --- a/docs/databases.md +++ b/docs/databases.md @@ -30,6 +30,7 @@ it using its connection string. | Change size (cloud) | `volcano cloud databases resize --type ` | | Apply local migrations | `volcano migrations deploy --all -d ` | | Apply cloud migrations | `volcano cloud databases migration up --all -d ` | +| Record cloud migrations as applied without running them | `volcano cloud databases migration baseline -d -f ` | Local migrations run from `volcano/migrations` against a local database. Cloud migrations use the cloud database command and connect through the managed @@ -40,8 +41,32 @@ migrations as part of a whole-project deploy. With a [delegated grant](authentication.md#delegated-grants) (`pg-`), `volcano cloud databases migration up` sends the files to a server-side migration run instead of connecting to the database. The run skips files the database's migration -history already records, and applies the rest after a rehearsal and a backup. -Where migration runs are not available, the command fails and applies nothing. +history already records. Before it applies the rest, it rehearses them on a +short-lived branch of the database and takes a backup, where the platform and +your plan offer branches and backups. When the run skips either step, the +command prints a `Warning:` line saying so, and the files are applied without +it. Where migration runs are not available, the command fails and applies +nothing. + +Files applied by an earlier `up` that connected to the database directly are +not in that history, so a run tries them again. When a run fails because an +object already exists, the error names the file. If the database already holds +everything that file creates, record the file as applied without running it, +then run `up` again: + +```bash +# Record named files; -f takes a name or a path and can repeat +volcano cloud databases migration baseline -d -f 001_create_users -f 002_add_posts + +# Record every file in volcano/migrations +volcano cloud databases migration baseline -d --all +``` + +Record only files the database already reflects: a recorded file never runs. +A file the history already records with the same content is skipped; one +recorded with different content is refused, and nothing is recorded. The +command asks for confirmation first; `--yes` skips the prompt, and `--json` +prints the result as JSON. Each file runs as a single query, and the database proxy refuses a query with more than one statement, so put one SQL statement in each file. Files run in