thaloco/banger
1
Fork 0
Code Pull requests Releases Packages Activity Actions

cleanup: drop pre-v0.1 migration scaffolding + legacy-behavior refs

Browse source 
banger hasn't shipped a public release — every "legacy", "pre-opt-in",
"previously", "migration note", "no longer" reference in the tree is
pinning against a state no real user's install has ever been in.
That scaffolding has weight: it's a coordinate system future readers
have to decode, and it keeps dead code alive.

Removed (code):
- internal/daemon/ssh_client_config.go
    - vmSSHConfigIncludeBegin / vmSSHConfigIncludeEnd constants and
      every `removeManagedBlock(existing, vm...)` call they enabled
      (legacy inline `Host *.vm` block scrub)
    - cleanupLegacySSHConfigDir (+ its caller in syncVMSSHClientConfig)
      — wiped a pre-opt-in sibling file under $ConfigDir/ssh
    - sameDirOrParent + resolvePathForComparison — only ever used
      by cleanupLegacySSHConfigDir
    - the "also check legacy marker" fallback in
      UserSSHIncludeInstalled / UninstallUserSSHInclude
- internal/store/migrations.go
    - migrateDropDeadImageColumns (migration 2) + its slice entry
    - dropColumnIfExists (orphaned after the above)
    - addColumnIfMissing + the whole "columns added across the pre-
      versioning lifetime" block at the end of migrateBaseline —
      subsumed into the baseline CREATE TABLE
    - `packages_path TEXT` column on the images table (the
      throwaway migration 2 dropped it, but there was never any
      reader)
- internal/daemon/vm.go
    - vmDNSRecordName local wrapper — was justified as "avoid
      pulling vmdns into every file"; three of four callers already
      imported vmdns directly, so inline the one stray call
- internal/cli/cli_test.go
    - TestLegacyRemovedCommandIsRejected (`tui` subcommand never
      shipped)

Removed / simplified (tests):
- ssh_client_config_test.go: dropped TestSameDirOrParentHandlesSymlinks,
  TestSyncVMSSHClientConfigPreservesUserKeyInLegacyDir,
  TestSyncVMSSHClientConfigNarrowsCleanupToLegacyFile,
  TestSyncVMSSHClientConfigLeavesUnexpectedLegacyContents,
  TestInstallUserSSHIncludeMigratesLegacyInlineBlock, plus the
  "legacy posture" regression strings in the remaining happy-path
  test; TestUninstallUserSSHIncludeRemovesBothMarkerBlocks collapsed
  to a single-block test
- migrations_test.go: dropped TestMigrateDropDeadImageColumns_AcrossInstallPaths,
  TestDropColumnIfExistsIsIdempotent; TestOpenReadOnlyDoesNotRunMigrations
  simplified to test against the baseline marker

Removed (docs):
- README.md "**Migration note.**" blockquote about the SSH-key path move
- docs/advanced.md parenthetical "(the old behaviour)"

Reworded (comments):
- Dropped "Previously this file also contained LogLevel DEBUG3..."
  history from vm_disk.go's sshdGuestConfig doc
- Dropped "Call sites that previously read vm.Runtime.{PID,...}"
  from vm_handles.go; now documents the current contract
- Dropped "Pre-v0.1 the defaults are" scaffolding in doctor_test.go
- Dropped "no longer does its own git inspection" phrasing in vm_run.go
- Dropped the "(also cleans up legacy inline block from pre-opt-in
  builds)" aside on the `ssh-config` CLI docstring
- Renamed test var `legacyKey` → `existingKey` in vm_test.go; its
  purpose was "pre-existing authorized_keys line," not banger-legacy

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Thales Maciel 2026-04-23 13:56:32 -03:00
parent 5791466498
commit 700a1e6e60
No known key found for this signature in database
GPG key ID: 33112E6833C34679
16 changed files with 54 additions and 735 deletions
Download patch file Download diff file Expand all files Collapse all files

108
internal/store/migrations.go
View file

@ -24,15 +24,10 @@ type migration struct {
// entries — installed DBs key off the id column.
var migrations = []migration{
{id: 1, name: "baseline", up: migrateBaseline},
{id: 2, name: "drop_dead_image_columns", up: migrateDropDeadImageColumns},
}
// runMigrations ensures schema_migrations exists, then applies every
// migration whose id hasn't been recorded yet, in id order. Existing
// dev databases (schema set up by the pre-versioning inline migrate()
// helper) see the baseline SQL as a no-op because every statement is
// `CREATE TABLE IF NOT EXISTS`; the row that records id=1 is what
// brings them into the new system.
// migration whose id hasn't been recorded yet, in id order.
func runMigrations(db *sql.DB) error {
if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations (
id INTEGER PRIMARY KEY,
@ -105,11 +100,7 @@ func applyMigration(db *sql.DB, m migration) error {
return tx.Commit()
}
// migrateBaseline captures the schema as it stood when the versioned
// migration system was introduced. Uses IF NOT EXISTS on every object
// so existing dev databases — whose tables were set up by the old
// inline migrate() — pass through cleanly and only the
// schema_migrations row gets added.
// migrateBaseline creates the full current schema.
func migrateBaseline(tx *sql.Tx) error {
stmts := []string{
`CREATE TABLE IF NOT EXISTS images (
@ -122,7 +113,6 @@ func migrateBaseline(tx *sql.Tx) error {
kernel_path TEXT NOT NULL,
initrd_path TEXT,
modules_dir TEXT,
packages_path TEXT,
build_size TEXT,
seeded_ssh_public_key_fingerprint TEXT,
docker INTEGER NOT NULL DEFAULT 0,
@ -149,99 +139,5 @@ func migrateBaseline(tx *sql.Tx) error {
return err
}
}
// Columns added to the images table across the pre-versioning
// lifetime of the project. New installs get them from the CREATE
// TABLE above; upgraders from an ancient snapshot (pre-
// ensureColumnExists) pick them up here. Idempotent either way.
for _, col := range []struct{ table, name, typ string }{
{"images", "work_seed_path", "TEXT"},
{"images", "seeded_ssh_public_key_fingerprint", "TEXT"},
} {
if err := addColumnIfMissing(tx, col.table, col.name, col.typ); err != nil {
return err
}
}
return nil
}
// migrateDropDeadImageColumns removes image-table columns that the
// store never reads or writes. `packages_path` was introduced for a
// build pipeline that no longer exists; the baseline migration still
// creates it for historical fidelity, and this migration drops it on
// new installs + any upgrader that still carries it. Idempotent via
// dropColumnIfExists so running the migration twice (or against a
// DB where the column was already gone) is a no-op.
func migrateDropDeadImageColumns(tx *sql.Tx) error {
return dropColumnIfExists(tx, "images", "packages_path")
}
// dropColumnIfExists is SQLite's "ALTER TABLE DROP COLUMN IF EXISTS"
// (which the dialect lacks) as a library function. modernc.org/sqlite
// bundles SQLite 3.42+, which supports plain DROP COLUMN — we add the
// existence guard so the statement is idempotent across repeat runs
// and legacy DBs that never had the column in the first place.
func dropColumnIfExists(tx *sql.Tx, table, column string) error {
rows, err := tx.Query(fmt.Sprintf("PRAGMA table_info(%s)", table))
if err != nil {
return err
}
defer rows.Close()
var found bool
for rows.Next() {
var (
cid int
name string
valueType string
notNull int
defaultV sql.NullString
pk int
)
if err := rows.Scan(&cid, &name, &valueType, &notNull, &defaultV, &pk); err != nil {
return err
}
if name == column {
found = true
}
}
if err := rows.Err(); err != nil {
return err
}
if !found {
return nil
}
_, err = tx.Exec(fmt.Sprintf("ALTER TABLE %s DROP COLUMN %s", table, column))
return err
}
// addColumnIfMissing is SQLite's "ALTER TABLE ADD COLUMN IF NOT EXISTS"
// (which the dialect lacks) as a library function. Used inside
// migrations when a column needs to survive a database that went
// through some historical path where the column was added later.
func addColumnIfMissing(tx *sql.Tx, table, column, columnType string) error {
rows, err := tx.Query(fmt.Sprintf("PRAGMA table_info(%s)", table))
if err != nil {
return err
}
defer rows.Close()
for rows.Next() {
var (
cid int
name string
valueType string
notNull int
defaultV sql.NullString
pk int
)
if err := rows.Scan(&cid, &name, &valueType, &notNull, &defaultV, &pk); err != nil {
return err
}
if name == column {
return nil
}
}
if err := rows.Err(); err != nil {
return err
}
_, err = tx.Exec(fmt.Sprintf("ALTER TABLE %s ADD COLUMN %s %s", table, column, columnType))
return err
}

187
internal/store/migrations_test.go
View file

@ -141,98 +141,18 @@ func TestApplyMigrationRollsBackOnBodyError(t *testing.T) {
}
}
// TestMigrateDropDeadImageColumns_AcrossInstallPaths verifies the
// drop-column migration is correct on both paths it can land on:
// a fresh install (baseline created the column, migration 2 drops
// it) and a legacy DB that somehow lost or never had the column
// (migration 2 is a no-op). Runs migrations end-to-end so the
// invariant-check is the real system, not the helper in isolation.
func TestMigrateDropDeadImageColumns_AcrossInstallPaths(t *testing.T) {
hasColumn := func(t *testing.T, db *sql.DB, table, column string) bool {
t.Helper()
rows, err := db.Query("PRAGMA table_info(" + table + ")")
if err != nil {
t.Fatalf("PRAGMA table_info: %v", err)
}
defer rows.Close()
for rows.Next() {
var (
cid int
name string
valueType string
notNull int
defaultV sql.NullString
pk int
)
if err := rows.Scan(&cid, &name, &valueType, &notNull, &defaultV, &pk); err != nil {
t.Fatalf("scan table_info row: %v", err)
}
if name == column {
return true
}
}
if err := rows.Err(); err != nil {
t.Fatalf("rows.Err: %v", err)
}
return false
}
t.Run("fresh install drops packages_path", func(t *testing.T) {
db := openRawDB(t)
if err := runMigrations(db); err != nil {
t.Fatalf("runMigrations: %v", err)
}
if hasColumn(t, db, "images", "packages_path") {
t.Fatal("packages_path column survived migration 2 on fresh install")
}
})
t.Run("legacy DB without column is a no-op", func(t *testing.T) {
db := openRawDB(t)
// Simulate a DB whose baseline was applied against a modified
// schema that never had packages_path: seed schema_migrations,
// run baseline, drop the column out-of-band, then run
// runMigrations and expect migration 2 to succeed regardless.
if _, err := db.Exec(`CREATE TABLE IF NOT EXISTS schema_migrations (
id INTEGER PRIMARY KEY,
name TEXT NOT NULL,
applied_at TEXT NOT NULL
)`); err != nil {
t.Fatalf("seed schema_migrations: %v", err)
}
if err := applyMigration(db, migrations[0]); err != nil {
t.Fatalf("apply baseline: %v", err)
}
if _, err := db.Exec("ALTER TABLE images DROP COLUMN packages_path"); err != nil {
t.Fatalf("pre-drop packages_path: %v", err)
}
if err := runMigrations(db); err != nil {
t.Fatalf("runMigrations after manual pre-drop: %v", err)
}
if hasColumn(t, db, "images", "packages_path") {
t.Fatal("packages_path reappeared after runMigrations")
}
})
}
// TestOpenReadOnlyDoesNotRunMigrations pins the doctor contract:
// OpenReadOnly must not mutate the DB. We create a DB without the
// schema_migrations row for migration 2 present (simulating a
// daemon-not-yet-run state), open it read-only, and confirm no row
// was added and no column dropped.
// OpenReadOnly must not mutate the DB. Seed a DB whose baseline
// migration row has been forcibly removed (simulating a "behind"
// state), open it read-only, and confirm nothing was re-applied.
func TestOpenReadOnlyDoesNotRunMigrations(t *testing.T) {
path := filepath.Join(t.TempDir(), "state.db")
// Seed the file by running full Open once, then roll migration 2
// backwards manually so the DB is "behind" current code.
full, err := Open(path)
if err != nil {
t.Fatalf("Open: %v", err)
}
if _, err := full.db.Exec("ALTER TABLE images ADD COLUMN packages_path TEXT"); err != nil {
t.Fatalf("re-add packages_path: %v", err)
}
if _, err := full.db.Exec("DELETE FROM schema_migrations WHERE id = 2"); err != nil {
t.Fatalf("remove migration 2 marker: %v", err)
if _, err := full.db.Exec("DELETE FROM schema_migrations WHERE id = 1"); err != nil {
t.Fatalf("remove baseline marker: %v", err)
}
_ = full.Close()
@ -242,39 +162,12 @@ func TestOpenReadOnlyDoesNotRunMigrations(t *testing.T) {
}
defer ro.Close()
// Migration 2 marker must still be absent; packages_path must
// still exist.
var migCount int
if err := ro.db.QueryRow("SELECT COUNT(*) FROM schema_migrations WHERE id = 2").Scan(&migCount); err != nil {
if err := ro.db.QueryRow("SELECT COUNT(*) FROM schema_migrations WHERE id = 1").Scan(&migCount); err != nil {
t.Fatalf("query schema_migrations: %v", err)
}
if migCount != 0 {
t.Fatal("OpenReadOnly recorded migration 2 — the open path mutated the DB")
}
rows, err := ro.db.Query("PRAGMA table_info(images)")
if err != nil {
t.Fatalf("PRAGMA table_info: %v", err)
}
defer rows.Close()
var sawColumn bool
for rows.Next() {
var (
cid int
name string
valueType string
notNull int
defaultV sql.NullString
pk int
)
if err := rows.Scan(&cid, &name, &valueType, &notNull, &defaultV, &pk); err != nil {
t.Fatalf("scan: %v", err)
}
if name == "packages_path" {
sawColumn = true
}
}
if !sawColumn {
t.Fatal("packages_path disappeared — OpenReadOnly ran the drop migration")
t.Fatal("OpenReadOnly re-recorded a migration row — the open path mutated the DB")
}
}
@ -351,72 +244,6 @@ func TestRunMigrationsIgnoresUnknownAppliedIDs(t *testing.T) {
}
}
// TestDropColumnIfExistsIsIdempotent pins the "run twice, no harm"
// property. A daemon that restarts after a successful migration 2
// on a fresh install shouldn't fail because the column is already
// gone. migrateDropDeadImageColumns calls dropColumnIfExists, which
// must silently succeed when the column is absent.
func TestDropColumnIfExistsIsIdempotent(t *testing.T) {
db := openRawDB(t)
// Set up a tiny table with a known column we're going to drop.
if _, err := db.Exec(`CREATE TABLE throwaway (keeper TEXT, victim TEXT)`); err != nil {
t.Fatalf("CREATE: %v", err)
}
run := func(label string) error {
tx, err := db.Begin()
if err != nil {
t.Fatalf("%s Begin: %v", label, err)
}
if err := dropColumnIfExists(tx, "throwaway", "victim"); err != nil {
_ = tx.Rollback()
return err
}
return tx.Commit()
}
if err := run("first"); err != nil {
t.Fatalf("first dropColumnIfExists: %v", err)
}
// Second call against a table that no longer has the column.
if err := run("second"); err != nil {
t.Fatalf("second dropColumnIfExists (column already gone): %v", err)
}
// The keeper column must still be there; victim is gone.
rows, err := db.Query("PRAGMA table_info(throwaway)")
if err != nil {
t.Fatalf("PRAGMA: %v", err)
}
defer rows.Close()
var haveKeeper, haveVictim bool
for rows.Next() {
var (
cid int
name string
valueType string
notNull int
defaultV sql.NullString
pk int
)
if err := rows.Scan(&cid, &name, &valueType, &notNull, &defaultV, &pk); err != nil {
t.Fatalf("scan: %v", err)
}
switch name {
case "keeper":
haveKeeper = true
case "victim":
haveVictim = true
}
}
if !haveKeeper {
t.Fatal("keeper column disappeared — dropColumnIfExists is too aggressive")
}
if haveVictim {
t.Fatal("victim column survived — dropColumnIfExists didn't actually drop")
}
}
func TestRunMigrationsRejectsDuplicateID(t *testing.T) {
db := openRawDB(t)
orig := migrations