Repository: juanfont/headscale

Issue:
[Bug] Automatic database migration from 0.23.0 to 0.24.0 does not work with postgres
### Is this a support request?

- [x] This is not a support request

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current Behavior

I tried to update my headscale instance, which uses postgres as the database, from 0.23.0 to 0.24.0 using the Debian packages provided as part of the releases on Github. However, after installing the new package, headscale failed to start due to problems with the database migration, with the following message in the logs:

```
Jan 17 16:48:47 hostname headscale[130595]: 2025-01-17T16:48:47Z INF Opening database database=postgres path="host=/run/postgresql dbname=headscale user=headscale"
Jan 17 16:48:47 hostname headscale[130595]: 2025-01-17T16:48:47Z FTL Migration failed: ERROR: constraint "uni_users_name" of relation "users" does not exist (SQLSTATE 42704) error="ERROR: constraint \"uni_users_name\" of relation \"users\" does not exist (SQLSTATE 42704)"
```

I was also left unable to blindly downgrade to 0.23.0, as the new version had already executed a migration successfully before encountering the error, leaving my database in an inconsistent state that would not have been supported by the old version.

### Expected Behavior

Headscale executes the database migration without errors, and then proceeds to function normally.

### Steps To Reproduce

1. Install headscale 0.23.0 using postgres as the database.
2. Update the installed version of headscale to 0.24.0, and attempt to start the headscale process again.

### Environment

```markdown
- OS: Debian 12
- Headscale version: 0.23.0
- Tailscale version: N/A
```

### Runtime environment

- [x] Headscale is behind a (reverse) proxy
- [ ] Headscale runs in a container

### Anything else?

I manually inspected the migrations table in the database and compared this with the code in 0.24.0, which indicates that this was a problem with migration `202407191627`. This is an automatic migration executed by gorm to update the schema of the `users` table -- I'm not sure if there have been changes in gorm, but I'm not familiar with the library so I didn't investigate that further.

As I mentioned above, my database was left in an inconsistent state which prevented me from downgrading (in fairness I should have backed up my database *before* performing the upgrade...). However, my database did have a uniqueness constraint for `users.name` as implied by the log message above, but in my database the constraint was called `users_name_key` instead of `uni_users_name`.

I used the following SQL to rename the constraint on the `users` table, and with this change the migration which had been causing me problems then executed correctly:
```
begin;
alter table users add constraint uni_users_name unique (name);
alter table users drop constraint users_name_key;
commit;
```

My headscale instance now appears to be working correctly, though I haven't tried adding new users or nodes to the Tailnet yet.

Discussion:
@kradalby: This is unfortunate, I will not have time to look into this until next week, but in the meantime if someone has a Postgres backup that can reproduce this issue, I would appreciate getting it.

As often mentioned we don't have the time to do the extensive testing for Postgres as we have so much other things to fix.

A personal rant;
I personally have less and less appreciation for Gorm, and if this turns out to be some sort of auto magic footgun that doesn't help.
I want to get rid of Gorm, but that means get rid of Postgres and only support one database, which we are not doing, so it will likely continue like this where we try to test things, but if no one tests the betas with the setups we can't test, then this will continue to happen. In this case I suspect the beta testers didn't run Postgres so we didn't discover it.@sysvinit: > This is unfortunate, I will not have time to look into this until next week, but in the meantime if someone has a Postgres backup that can reproduce this issue, I would appreciate getting it.

I do have pg_dumpall backups of the entire postgres server on that machine as part of the daily system backups (useful for restoring after a storage failure, less so for restoring individual tables in the heat of the moment). I could get you a copy of the headscale DB from my server from the night before I attempted the upgrade, though I'll need to find some free time next week to spin up a test instance of postgres which I can load the backup into first in order to extract the headscale parts (and censor things like IP addresses). What would be the best way to send you the DB dump?@kradalby: Great, email in my GitHub would be sufficient@sysvinit: Given you've found a fix already -- do you still need a copy of my headscale database out of my backups?@kradalby: No, thank you, I got one from another user and wrote a test based on that.@panteparak: Hi @kradalby 

Just a heads up, somehow the migration in the fix PR have no effect and produce the same error.
I had to copy out the [SQL statement ](https://github.com/juanfont/headscale/blob/v0.24.2/hscontrol/db/db.go#L486-L506)AS IS and run manually on the DB to fix it.

Upgrading from 0.22.3 -> 0.24.2 with Postgres.

If you wanted to investigate further, I could sent you my SQL dump v0.22.3.@kradalby: Yes please, it worked from 0.23, but maybe the step from 0.22 was different, 0.23 is the time we introduced migrations so it would not surprise me. Email is in my profile@panteparak: @kradalby 
Sorry for the late reply, any tips on sanitising the db dump?, From what I can see, I should strip out IPs, nodekeys, discokeys. Anything else should be done? or is there a script clean this all up.@panteparak: @kradalby i've send my Postgres DB Dump to your email. Let me know if you need other info.@haatveit: I've (finally) upgraded from 0.21.0 by starting up every release along the way in case it was needed, but haven't been able to get releases past 0.24.2 working.


On 0.23.0 I got this on first startup after migrations:

```console
2025-02-25T21:50:20Z FTL home/runner/work/headscale/headscale/cmd/headscale/cli/serve.go:29 > Headscale ran into an error and had to shut down. error="failed to load ACL policy: loading nodes from database to validate policy: ERROR: cached plan must not change result type (SQLSTATE 0A000)"
```

then it worked next time I started the service:

From 0.24.0 onward I get this:

```console
2025-02-25T21:51:43Z FTL Migration failed: ERROR: constraint "uni_users_name" of relation "users" does not exist (SQLSTATE 42704) error="ERROR: constraint \"uni_users_name\" of relation \"users\" does not exist (SQLSTATE 42704)"
```

If I manually add the constraint via psql, I can run 0.24.0 through 0.24.2:

```sql
alter table users add constraint uni_users_name unique (name);
```

But if I try to upgrade to 0.24.3 or newer, I get this error on first startup:

```console
2025-02-25T22:11:47Z FTL Migration failed: automigrating types.Node: ERROR: constraint "uni_users_name" of relation "users" does not exist (SQLSTATE 42704) error="automigrating types.Node: ERROR: constraint \"uni_users_name\" of relation \"users\" does not exist (SQLSTATE 42704)"
```

and it removes the `uni_users_name` constraint again. I've looked at the [diff between 0.24.2 and 0.24.3](https://github.com/juanfont/headscale/compare/v0.24.2...v0.24.3) but don't really get why that is happening.@kradalby: I haven’t had a close look, but starting every release would be counter productive. Since there are migration fixes in the fix releases, you should jump to the latest release or at least the latest fix release.

We have not had a database that old in a bit, so if going from 0.21 straight to 0.24.3 or 0.25.1 doesn’t work I would appreciate a scrubbed copy I can write a test against.@nicka101: For me, simply spamming the Alter table in Postgres manually to add the `uni_users_name` constraint back while headscale is starting allows the migrations to succeed and headscale to start on 0.25.1@bazaah: The PreAuthKey and/or Node automigrates here: https://github.com/juanfont/headscale/compare/v0.24.2...v0.24.3#diff-826f8967b23df1209318fba82f94d8ed3eb12136b6cb2d070ddb76d9a059c336R590-R597 are somehow responsible for the `uni_users_name` issue. They seemingly race to create problem, as I see both of them in the logs

```
FTL Migration failed: automigrating types.Node: ERROR: constraint "uni_users_name" of relation "users" does not exist (SQLSTATE 42704)
                      ^^^^^^^^^^^^^^^^^^^^^^^^
```

(or)

```
FTL Migration failed: automigrating types.PreAuthKey: ERROR: constraint "uni_users_name" of relation "users" does not exist (SQLSTATE 42704)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

I eventually just needed to `INSERT INTO migrations (id) VALUES (202501311657);` to get a functional instance again. Is there anything I should run by hand to replicate the expected schema changes from https://github.com/juanfont/headscale/pull/2396?

My migration path was: 0.22.3 -> 0.23.0 -> 0.24.0 -> 0.24.2 -> 0.24.3, in case it matters.

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6a6da00..a06a2ad1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,8 +11,12 @@
 
 ### Changes
 
+- Fix migration issue with user table for PostgreSQL
+  [#2367](https://github.com/juanfont/headscale/pull/2367)
 - Relax username validation to allow emails
   [#2364](https://github.com/juanfont/headscale/pull/2364)
+- Remove invalid routes and add stronger constraints for routes to avoid API panic
+  [#2371](https://github.com/juanfont/headscale/pull/2371)
 
 ## 0.24.0 (2025-01-17)
 
diff --git a/cmd/headscale/cli/routes.go b/cmd/headscale/cli/routes.go
index e39b407f..ef289497 100644
--- a/cmd/headscale/cli/routes.go
+++ b/cmd/headscale/cli/routes.go
@@ -251,10 +251,15 @@ func routesToPtables(routes []*v1.Route) pterm.TableData {
 			isPrimaryStr = strconv.FormatBool(route.GetIsPrimary())
 		}
 
+		var nodeName string
+		if route.GetNode() != nil {
+			nodeName = route.GetNode().GetGivenName()
+		}
+
 		tableData = append(tableData,
 			[]string{
 				strconv.FormatUint(route.GetId(), Base10),
-				route.GetNode().GetGivenName(),
+				nodeName,
 				route.GetPrefix(),
 				strconv.FormatBool(route.GetAdvertised()),
 				strconv.FormatBool(route.GetEnabled()),
diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go
index 0d9120c2..36955e22 100644
--- a/hscontrol/db/db.go
+++ b/hscontrol/db/db.go
@@ -478,6 +478,38 @@ func NewHeadscaleDatabase(
 				// populate the user with more interesting information.
 				ID: "202407191627",
 				Migrate: func(tx *gorm.DB) error {
+					// Fix an issue where the automigration in GORM expected a constraint to
+					// exists that didnt, and add the one it wanted.
+					// Fixes https://github.com/juanfont/headscale/issues/2351
+					if cfg.Type == types.DatabasePostgres {
+						err := tx.Exec(`
+BEGIN;
+DO $$
+BEGIN
+    IF NOT EXISTS (
+        SELECT 1 FROM pg_constraint
+        WHERE conname = 'uni_users_name'
+    ) THEN
+        ALTER TABLE users ADD CONSTRAINT uni_users_name UNIQUE (name);
+    END IF;
+END $$;
+
+DO $$
+BEGIN
+    IF EXISTS (
+        SELECT 1 FROM pg_constraint
+        WHERE conname = 'users_name_key'
+    ) THEN
+        ALTER TABLE users DROP CONSTRAINT users_name_key;
+    END IF;
+END $$;
+COMMIT;
+`).Error
+						if err != nil {
+							return fmt.Errorf("failed to rename constraint: %w", err)
+						}
+					}
+
 					err := tx.AutoMigrate(&types.User{})
 					if err != nil {
 						return err
@@ -521,6 +553,27 @@ func NewHeadscaleDatabase(
 				},
 				Rollback: func(db *gorm.DB) error { return nil },
 			},
+			{
+				// Add a constraint to routes ensuring they cannot exist without a node.
+				ID: "202501221827",
+				Migrate: func(tx *gorm.DB) error {
+					// Remove any invalid routes associated with a node that does not exist.
+					if tx.Migrator().HasTable(&types.Route{}) && tx.Migrator().HasTable(&types.Node{}) {
+						err := tx.Exec("delete from routes where node_id not in (select id from nodes)").Error
+						if err != nil {
+							return err
+						}
+					}
+
+					err := tx.AutoMigrate(&types.Route{})
+					if err != nil {
+						return err
+					}
+
+					return nil
+				},
+				Rollback: func(db *gorm.DB) error { return nil },
+			},
 		},
 	)
 
diff --git a/hscontrol/types/routes.go b/hscontrol/types/routes.go
index 4ef3621f..12559fa6 100644
--- a/hscontrol/types/routes.go
+++ b/hscontrol/types/routes.go
@@ -13,7 +13,7 @@ import (
 type Route struct {
 	gorm.Model
 
-	NodeID uint64
+	NodeID uint64 `gorm:"not null"`
 	Node   *Node
 
 	// TODO(kradalby): change this custom type to netip.Prefix
@@ -79,7 +79,6 @@ func (rs Routes) Proto() []*v1.Route {
 	for _, route := range rs {
 		protoRoute := v1.Route{
 			Id:         uint64(route.ID),
-			Node:       route.Node.Proto(),
 			Prefix:     route.Prefix.String(),
 			Advertised: route.Advertised,
 			Enabled:    route.Enabled,
@@ -88,6 +87,10 @@ func (rs Routes) Proto() []*v1.Route {
 			UpdatedAt:  timestamppb.New(route.UpdatedAt),
 		}
 
+		if route.Node != nil {
+			protoRoute.Node = route.Node.Proto()
+		}
+
 		if route.DeletedAt.Valid {
 			protoRoute.DeletedAt = timestamppb.New(route.DeletedAt.Time)
 		}



Test patch:
diff --git a/hscontrol/db/db_test.go b/hscontrol/db/db_test.go
index c3d9a835..0672c252 100644
--- a/hscontrol/db/db_test.go
+++ b/hscontrol/db/db_test.go
@@ -6,6 +6,7 @@ import (
 	"io"
 	"net/netip"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"slices"
 	"sort"
@@ -23,7 +24,10 @@ import (
 	"zgo.at/zcache/v2"
 )
 
-func TestMigrations(t *testing.T) {
+// TestMigrationsSQLite is the main function for testing migrations,
+// we focus on SQLite correctness as it is the main database used in headscale.
+// All migrations that are worth testing should be added here.
+func TestMigrationsSQLite(t *testing.T) {
 	ipp := func(p string) netip.Prefix {
 		return netip.MustParsePrefix(p)
 	}
@@ -375,3 +379,58 @@ func TestConstraints(t *testing.T) {
 		})
 	}
 }
+
+func TestMigrationsPostgres(t *testing.T) {
+	tests := []struct {
+		name     string
+		dbPath   string
+		wantFunc func(*testing.T, *HSDatabase)
+	}{
+		{
+			name:   "user-idx-breaking",
+			dbPath: "testdata/pre-24-postgresdb.pssql.dump",
+			wantFunc: func(t *testing.T, h *HSDatabase) {
+				users, err := Read(h.DB, func(rx *gorm.DB) ([]types.User, error) {
+					return ListUsers(rx)
+				})
+				require.NoError(t, err)
+
+				for _, user := range users {
+					assert.NotEmpty(t, user.Name)
+					assert.Empty(t, user.ProfilePicURL)
+					assert.Empty(t, user.Email)
+				}
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			u := newPostgresDBForTest(t)
+
+			pgRestorePath, err := exec.LookPath("pg_restore")
+			if err != nil {
+				t.Fatal("pg_restore not found in PATH. Please install it and ensure it is accessible.")
+			}
+
+			// Construct the pg_restore command
+			cmd := exec.Command(pgRestorePath, "--verbose", "--if-exists", "--clean", "--no-owner", "--dbname", u.String(), tt.dbPath)
+
+			// Set the output streams
+			cmd.Stdout = os.Stdout
+			cmd.Stderr = os.Stderr
+
+			// Execute the command
+			err = cmd.Run()
+			if err != nil {
+				t.Fatalf("failed to restore postgres database: %s", err)
+			}
+
+			db = newHeadscaleDBFromPostgresURL(t, u)
+
+			if tt.wantFunc != nil {
+				tt.wantFunc(t, db)
+			}
+		})
+	}
+}
diff --git a/hscontrol/db/suite_test.go b/hscontrol/db/suite_test.go
index fb7ce1df..e9c71823 100644
--- a/hscontrol/db/suite_test.go
+++ b/hscontrol/db/suite_test.go
@@ -78,13 +78,11 @@ func newSQLiteTestDB() (*HSDatabase, error) {
 func newPostgresTestDB(t *testing.T) *HSDatabase {
 	t.Helper()
 
-	var err error
-	tmpDir, err = os.MkdirTemp("", "headscale-db-test-*")
-	if err != nil {
-		t.Fatal(err)
-	}
+	return newHeadscaleDBFromPostgresURL(t, newPostgresDBForTest(t))
+}
 
-	log.Printf("database path: %s", tmpDir+"/headscale_test.db")
+func newPostgresDBForTest(t *testing.T) *url.URL {
+	t.Helper()
 
 	ctx := context.Background()
 	srv, err := postgrestest.Start(ctx)
@@ -100,10 +98,16 @@ func newPostgresTestDB(t *testing.T) *HSDatabase {
 	t.Logf("created local postgres: %s", u)
 	pu, _ := url.Parse(u)
 
+	return pu
+}
+
+func newHeadscaleDBFromPostgresURL(t *testing.T, pu *url.URL) *HSDatabase {
+	t.Helper()
+
 	pass, _ := pu.User.Password()
 	port, _ := strconv.Atoi(pu.Port())
 
-	db, err = NewHeadscaleDatabase(
+	db, err := NewHeadscaleDatabase(
 		types.DatabaseConfig{
 			Type: types.DatabasePostgres,
 			Postgres: types.PostgresConfig{
diff --git a/hscontrol/db/testdata/pre-24-postgresdb.pssql.dump b/hscontrol/db/testdata/pre-24-postgresdb.pssql.dump
new file mode 100644
index 00000000..7f8df28b
Binary files /dev/null and b/hscontrol/db/testdata/pre-24-postgresdb.pssql.dump differ