Address Copilot and CodeRabbit review feedback
- commit
- 3e72060fbd8859b06149da6184fc8575214c1039
- author
- brice · · 2026-02-08 17:02 UTC
- parent
- 3a8691d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +43 -32
M docs/config/server.md +9 -3
| @@ -77,11 +77,17 @@ tls_enabled = true | |||
| 77 | 77 | tls_port = 8443 | |
| 78 | 78 | ``` | |
| 79 | 79 | ||
| 80 | - | With firewalld and Tailscale (`tailscale0` in `trusted` zone): | |
| 80 | + | With firewalld and Tailscale: | |
| 81 | 81 | ||
| 82 | 82 | ```bash | |
| 83 | - | sudo firewall-cmd --zone=trusted --add-rich-rule='rule family=ipv4 forward-port port=443 protocol=tcp to-port=8443' | |
| 84 | - | sudo firewall-cmd --zone=trusted --add-rich-rule='rule family=ipv6 forward-port port=443 protocol=tcp to-port=8443' | |
| 83 | + | # Verify tailscale0 is in trusted zone | |
| 84 | + | sudo firewall-cmd --get-zone-of-interface=tailscale0 | |
| 85 | + | ||
| 86 | + | # If needed, place tailscale0 in trusted | |
| 87 | + | sudo firewall-cmd --permanent --zone=trusted --add-interface=tailscale0 | |
| 88 | + | sudo firewall-cmd --reload | |
| 89 | + | ||
| 90 | + | # Forward 443 -> 8443 permanently (IPv4 + IPv6) | |
| 85 | 91 | sudo firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv4 forward-port port=443 protocol=tcp to-port=8443' | |
| 86 | 92 | sudo firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv6 forward-port port=443 protocol=tcp to-port=8443' | |
| 87 | 93 | sudo firewall-cmd --reload | |
| @@ -77,11 +77,17 @@ tls_enabled = true | |||||
| 77 | tls_port = 8443 | 77 | tls_port = 8443 | ||
| 78 | ``` | 78 | ``` | ||
| 79 | 79 | ||||
| 80 | - | With firewalld and Tailscale (`tailscale0` in `trusted` zone): | 80 | + | With firewalld and Tailscale: |
| 81 | 81 | ||||
| 82 | ```bash | 82 | ```bash | ||
| 83 | - | sudo firewall-cmd --zone=trusted --add-rich-rule='rule family=ipv4 forward-port port=443 protocol=tcp to-port=8443' | 83 | + | # Verify tailscale0 is in trusted zone |
| 84 | - | sudo firewall-cmd --zone=trusted --add-rich-rule='rule family=ipv6 forward-port port=443 protocol=tcp to-port=8443' | 84 | + | sudo firewall-cmd --get-zone-of-interface=tailscale0 |
| 85 | + | ||||
| 86 | + | # If needed, place tailscale0 in trusted | |||
| 87 | + | sudo firewall-cmd --permanent --zone=trusted --add-interface=tailscale0 | |||
| 88 | + | sudo firewall-cmd --reload | |||
| 89 | + | ||||
| 90 | + | # Forward 443 -> 8443 permanently (IPv4 + IPv6) | |||
| 85 | sudo firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv4 forward-port port=443 protocol=tcp to-port=8443' | 91 | sudo firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv4 forward-port port=443 protocol=tcp to-port=8443' | ||
| 86 | sudo firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv6 forward-port port=443 protocol=tcp to-port=8443' | 92 | sudo firewall-cmd --permanent --zone=trusted --add-rich-rule='rule family=ipv6 forward-port port=443 protocol=tcp to-port=8443' | ||
| 87 | sudo firewall-cmd --reload | 93 | sudo firewall-cmd --reload | ||
M internal/usecase/backup/detector.go +4 -8
| @@ -44,20 +44,16 @@ func hasPort(ports []int, p int) bool { | |||
| 44 | 44 | // buildPostgresInfo constructs a DBInfo for a PostgreSQL attachment. | |
| 45 | 45 | func buildPostgresInfo(domainName string, a domain.Attachment) domain.DBInfo { | |
| 46 | 46 | port := 5432 | |
| 47 | - | hasPostgresPort := false | |
| 48 | - | firstPositive := 0 | |
| 49 | 47 | for _, p := range a.Ports { | |
| 50 | 48 | if p == 5432 { | |
| 51 | - | hasPostgresPort = true | |
| 49 | + | port = 5432 | |
| 52 | 50 | break | |
| 53 | 51 | } | |
| 54 | - | if p > 0 && firstPositive == 0 { | |
| 55 | - | firstPositive = p | |
| 52 | + | if p > 0 && port == 5432 { | |
| 53 | + | port = p | |
| 56 | 54 | } | |
| 57 | 55 | } | |
| 58 | - | if !hasPostgresPort && firstPositive > 0 { | |
| 59 | - | port = firstPositive | |
| 60 | - | } | |
| 56 | + | ||
| 61 | 57 | return domain.DBInfo{ | |
| 62 | 58 | Type: domain.DBTypePostgreSQL, | |
| 63 | 59 | Version: postgresVersionFromImage(a.Image), | |
| @@ -44,20 +44,16 @@ func hasPort(ports []int, p int) bool { | |||||
| 44 | // buildPostgresInfo constructs a DBInfo for a PostgreSQL attachment. | 44 | // buildPostgresInfo constructs a DBInfo for a PostgreSQL attachment. | ||
| 45 | func buildPostgresInfo(domainName string, a domain.Attachment) domain.DBInfo { | 45 | func buildPostgresInfo(domainName string, a domain.Attachment) domain.DBInfo { | ||
| 46 | port := 5432 | 46 | port := 5432 | ||
| 47 | - | hasPostgresPort := false | |||
| 48 | - | firstPositive := 0 | |||
| 49 | for _, p := range a.Ports { | 47 | for _, p := range a.Ports { | ||
| 50 | if p == 5432 { | 48 | if p == 5432 { | ||
| 51 | - | hasPostgresPort = true | 49 | + | port = 5432 |
| 52 | break | 50 | break | ||
| 53 | } | 51 | } | ||
| 54 | - | if p > 0 && firstPositive == 0 { | 52 | + | if p > 0 && port == 5432 { |
| 55 | - | firstPositive = p | 53 | + | port = p |
| 56 | } | 54 | } | ||
| 57 | } | 55 | } | ||
| 58 | - | if !hasPostgresPort && firstPositive > 0 { | 56 | + | |
| 59 | - | port = firstPositive | |||
| 60 | - | } | |||
| 61 | return domain.DBInfo{ | 57 | return domain.DBInfo{ | ||
| 62 | Type: domain.DBTypePostgreSQL, | 58 | Type: domain.DBTypePostgreSQL, | ||
| 63 | Version: postgresVersionFromImage(a.Image), | 59 | Version: postgresVersionFromImage(a.Image), | ||
M internal/usecase/backup/detector_test.go +23 -14
| @@ -154,18 +154,27 @@ func TestDetectDatabaseFromAttachment_UnknownImageNoPostgresPort(t *testing.T) { | |||
| 154 | 154 | } | |
| 155 | 155 | ||
| 156 | 156 | func TestLooksLikePostgres(t *testing.T) { | |
| 157 | - | // Positive cases | |
| 158 | - | assert.True(t, looksLikePostgres("postgres")) | |
| 159 | - | assert.True(t, looksLikePostgres("postgres:16")) | |
| 160 | - | assert.True(t, looksLikePostgres("my-postgres-db")) | |
| 161 | - | assert.True(t, looksLikePostgres("postgresql")) | |
| 162 | - | assert.True(t, looksLikePostgres("pgsql")) | |
| 163 | - | assert.True(t, looksLikePostgres("mon-fauteuil-prod-pgsql")) | |
| 164 | - | assert.True(t, looksLikePostgres("postgis/postgis")) | |
| 165 | - | ||
| 166 | - | // Negative cases | |
| 167 | - | assert.False(t, looksLikePostgres("mysql")) | |
| 168 | - | assert.False(t, looksLikePostgres("redis")) | |
| 169 | - | assert.False(t, looksLikePostgres("custom-db")) | |
| 170 | - | assert.False(t, looksLikePostgres("")) | |
| 157 | + | tests := []struct { | |
| 158 | + | name string | |
| 159 | + | image string | |
| 160 | + | want bool | |
| 161 | + | }{ | |
| 162 | + | {name: "postgres base image", image: "postgres", want: true}, | |
| 163 | + | {name: "postgres with version tag", image: "postgres:16", want: true}, | |
| 164 | + | {name: "name contains postgres", image: "my-postgres-db", want: true}, | |
| 165 | + | {name: "postgresql alias", image: "postgresql", want: true}, | |
| 166 | + | {name: "pgsql alias", image: "pgsql", want: true}, | |
| 167 | + | {name: "project specific pgsql alias", image: "mon-fauteuil-prod-pgsql", want: true}, | |
| 168 | + | {name: "postgis alias", image: "postgis/postgis", want: true}, | |
| 169 | + | {name: "mysql negative", image: "mysql", want: false}, | |
| 170 | + | {name: "redis negative", image: "redis", want: false}, | |
| 171 | + | {name: "custom negative", image: "custom-db", want: false}, | |
| 172 | + | {name: "empty negative", image: "", want: false}, | |
| 173 | + | } | |
| 174 | + | ||
| 175 | + | for _, tc := range tests { | |
| 176 | + | t.Run(tc.name, func(t *testing.T) { | |
| 177 | + | assert.Equal(t, tc.want, looksLikePostgres(tc.image)) | |
| 178 | + | }) | |
| 179 | + | } | |
| 171 | 180 | } | |
| @@ -154,18 +154,27 @@ func TestDetectDatabaseFromAttachment_UnknownImageNoPostgresPort(t *testing.T) { | |||||
| 154 | } | 154 | } | ||
| 155 | 155 | ||||
| 156 | func TestLooksLikePostgres(t *testing.T) { | 156 | func TestLooksLikePostgres(t *testing.T) { | ||
| 157 | - | // Positive cases | 157 | + | tests := []struct { |
| 158 | - | assert.True(t, looksLikePostgres("postgres")) | 158 | + | name string |
| 159 | - | assert.True(t, looksLikePostgres("postgres:16")) | 159 | + | image string |
| 160 | - | assert.True(t, looksLikePostgres("my-postgres-db")) | 160 | + | want bool |
| 161 | - | assert.True(t, looksLikePostgres("postgresql")) | 161 | + | }{ |
| 162 | - | assert.True(t, looksLikePostgres("pgsql")) | 162 | + | {name: "postgres base image", image: "postgres", want: true}, |
| 163 | - | assert.True(t, looksLikePostgres("mon-fauteuil-prod-pgsql")) | 163 | + | {name: "postgres with version tag", image: "postgres:16", want: true}, |
| 164 | - | assert.True(t, looksLikePostgres("postgis/postgis")) | 164 | + | {name: "name contains postgres", image: "my-postgres-db", want: true}, |
| 165 | - | 165 | + | {name: "postgresql alias", image: "postgresql", want: true}, | |
| 166 | - | // Negative cases | 166 | + | {name: "pgsql alias", image: "pgsql", want: true}, |
| 167 | - | assert.False(t, looksLikePostgres("mysql")) | 167 | + | {name: "project specific pgsql alias", image: "mon-fauteuil-prod-pgsql", want: true}, |
| 168 | - | assert.False(t, looksLikePostgres("redis")) | 168 | + | {name: "postgis alias", image: "postgis/postgis", want: true}, |
| 169 | - | assert.False(t, looksLikePostgres("custom-db")) | 169 | + | {name: "mysql negative", image: "mysql", want: false}, |
| 170 | - | assert.False(t, looksLikePostgres("")) | 170 | + | {name: "redis negative", image: "redis", want: false}, |
| 171 | + | {name: "custom negative", image: "custom-db", want: false}, | |||
| 172 | + | {name: "empty negative", image: "", want: false}, | |||
| 173 | + | } | |||
| 174 | + | ||||
| 175 | + | for _, tc := range tests { | |||
| 176 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 177 | + | assert.Equal(t, tc.want, looksLikePostgres(tc.image)) | |||
| 178 | + | }) | |||
| 179 | + | } | |||
| 171 | } | 180 | } | ||
M internal/usecase/container/monitor.go +3 -3
| @@ -120,7 +120,7 @@ func (m *Monitor) checkContainer(ctx context.Context, log zerowrap.Logger, domai | |||
| 120 | 120 | return | |
| 121 | 121 | } | |
| 122 | 122 | // Crashed (non-zero exit): restart if not in backoff. | |
| 123 | - | m.handleCrash(ctx, log, domainName, tracked.ID, now) | |
| 123 | + | m.handleCrash(ctx, log, domainName, tracked.ID, inspected.ExitCode, now) | |
| 124 | 124 | } | |
| 125 | 125 | } | |
| 126 | 126 | ||
| @@ -157,7 +157,7 @@ func (m *Monitor) handleRunning(ctx context.Context, log zerowrap.Logger, domain | |||
| 157 | 157 | m.mu.Unlock() | |
| 158 | 158 | } | |
| 159 | 159 | ||
| 160 | - | func (m *Monitor) handleCrash(ctx context.Context, log zerowrap.Logger, domainName, containerID string, now time.Time) { | |
| 160 | + | func (m *Monitor) handleCrash(ctx context.Context, log zerowrap.Logger, domainName, containerID string, exitCode int, now time.Time) { | |
| 161 | 161 | m.mu.Lock() | |
| 162 | 162 | rec := m.history[domainName] | |
| 163 | 163 | if rec == nil { | |
| @@ -213,7 +213,7 @@ func (m *Monitor) handleCrash(ctx context.Context, log zerowrap.Logger, domainNa | |||
| 213 | 213 | } | |
| 214 | 214 | ||
| 215 | 215 | log.Warn().Str("domain", domainName).Str("container_id", containerID). | |
| 216 | - | Int("exit_code", -1). // exact code already logged by inspect | |
| 216 | + | Int("exit_code", exitCode). | |
| 217 | 217 | Msg("monitor: restarting crashed container") | |
| 218 | 218 | ||
| 219 | 219 | if err := m.service.runtime.StartContainer(ctx, containerID); err != nil { | |
| @@ -120,7 +120,7 @@ func (m *Monitor) checkContainer(ctx context.Context, log zerowrap.Logger, domai | |||||
| 120 | return | 120 | return | ||
| 121 | } | 121 | } | ||
| 122 | // Crashed (non-zero exit): restart if not in backoff. | 122 | // Crashed (non-zero exit): restart if not in backoff. | ||
| 123 | - | m.handleCrash(ctx, log, domainName, tracked.ID, now) | 123 | + | m.handleCrash(ctx, log, domainName, tracked.ID, inspected.ExitCode, now) |
| 124 | } | 124 | } | ||
| 125 | } | 125 | } | ||
| 126 | 126 | ||||
| @@ -157,7 +157,7 @@ func (m *Monitor) handleRunning(ctx context.Context, log zerowrap.Logger, domain | |||||
| 157 | m.mu.Unlock() | 157 | m.mu.Unlock() | ||
| 158 | } | 158 | } | ||
| 159 | 159 | ||||
| 160 | - | func (m *Monitor) handleCrash(ctx context.Context, log zerowrap.Logger, domainName, containerID string, now time.Time) { | 160 | + | func (m *Monitor) handleCrash(ctx context.Context, log zerowrap.Logger, domainName, containerID string, exitCode int, now time.Time) { |
| 161 | m.mu.Lock() | 161 | m.mu.Lock() | ||
| 162 | rec := m.history[domainName] | 162 | rec := m.history[domainName] | ||
| 163 | if rec == nil { | 163 | if rec == nil { | ||
| @@ -213,7 +213,7 @@ func (m *Monitor) handleCrash(ctx context.Context, log zerowrap.Logger, domainNa | |||||
| 213 | } | 213 | } | ||
| 214 | 214 | ||||
| 215 | log.Warn().Str("domain", domainName).Str("container_id", containerID). | 215 | log.Warn().Str("domain", domainName).Str("container_id", containerID). | ||
| 216 | - | Int("exit_code", -1). // exact code already logged by inspect | 216 | + | Int("exit_code", exitCode). |
| 217 | Msg("monitor: restarting crashed container") | 217 | Msg("monitor: restarting crashed container") | ||
| 218 | 218 | ||||
| 219 | if err := m.service.runtime.StartContainer(ctx, containerID); err != nil { | 219 | if err := m.service.runtime.StartContainer(ctx, containerID); err != nil { | ||
M internal/usecase/container/service.go +1 -1
| @@ -862,7 +862,7 @@ func (s *Service) AutoStart(ctx context.Context, routes []domain.Route) error { | |||
| 862 | 862 | } | |
| 863 | 863 | ||
| 864 | 864 | var started, errCount int | |
| 865 | - | for range len(pending) { | |
| 865 | + | for i := 0; i < len(pending); i++ { | |
| 866 | 866 | res := <-results | |
| 867 | 867 | if res.err != nil { | |
| 868 | 868 | log.Warn().Err(res.err).Str("domain", res.route.Domain).Msg("failed to auto-start container") | |
| @@ -862,7 +862,7 @@ func (s *Service) AutoStart(ctx context.Context, routes []domain.Route) error { | |||||
| 862 | } | 862 | } | ||
| 863 | 863 | ||||
| 864 | var started, errCount int | 864 | var started, errCount int | ||
| 865 | - | for range len(pending) { | 865 | + | for i := 0; i < len(pending); i++ { |
| 866 | res := <-results | 866 | res := <-results | ||
| 867 | if res.err != nil { | 867 | if res.err != nil { | ||
| 868 | log.Warn().Err(res.err).Str("domain", res.route.Domain).Msg("failed to auto-start container") | 868 | log.Warn().Err(res.err).Str("domain", res.route.Domain).Msg("failed to auto-start container") | ||
M wiki/guides/remote-cli.md +3 -3
| @@ -18,15 +18,15 @@ Manage Gordon instances remotely using the CLI with the `--remote` flag or saved | |||
| 18 | 18 | ||
| 19 | 19 | ## Understanding the Remote URL | |
| 20 | 20 | ||
| 21 | - | Default: use the Gordon registry domain configured on the remote instance. | |
| 21 | + | Default: use the Gordon domain configured on the remote instance. | |
| 22 | 22 | ||
| 23 | 23 | ```toml | |
| 24 | 24 | # On the remote Gordon server | |
| 25 | 25 | [server] | |
| 26 | - | registry_domain = "reg.example.com" # Use this as --remote URL | |
| 26 | + | gordon_domain = "gordon.example.com" # Use this as --remote URL | |
| 27 | 27 | ``` | |
| 28 | 28 | ||
| 29 | - | The CLI connects to `https://reg.example.com/admin/*` endpoints. | |
| 29 | + | The CLI connects to `https://gordon.example.com/admin/*` endpoints. | |
| 30 | 30 | ||
| 31 | 31 | Private tailnet setup (recommended for hardened VPS): keep using the HTTPS domain, point that DNS record to the VPS tailnet IP, and set insecure TLS for self-signed certs. | |
| 32 | 32 | ||
| @@ -18,15 +18,15 @@ Manage Gordon instances remotely using the CLI with the `--remote` flag or saved | |||||
| 18 | 18 | ||||
| 19 | ## Understanding the Remote URL | 19 | ## Understanding the Remote URL | ||
| 20 | 20 | ||||
| 21 | - | Default: use the Gordon registry domain configured on the remote instance. | 21 | + | Default: use the Gordon domain configured on the remote instance. |
| 22 | 22 | ||||
| 23 | ```toml | 23 | ```toml | ||
| 24 | # On the remote Gordon server | 24 | # On the remote Gordon server | ||
| 25 | [server] | 25 | [server] | ||
| 26 | - | registry_domain = "reg.example.com" # Use this as --remote URL | 26 | + | gordon_domain = "gordon.example.com" # Use this as --remote URL |
| 27 | ``` | 27 | ``` | ||
| 28 | 28 | ||||
| 29 | - | The CLI connects to `https://reg.example.com/admin/*` endpoints. | 29 | + | The CLI connects to `https://gordon.example.com/admin/*` endpoints. |
| 30 | 30 | ||||
| 31 | Private tailnet setup (recommended for hardened VPS): keep using the HTTPS domain, point that DNS record to the VPS tailnet IP, and set insecure TLS for self-signed certs. | 31 | Private tailnet setup (recommended for hardened VPS): keep using the HTTPS domain, point that DNS record to the VPS tailnet IP, and set insecure TLS for self-signed certs. | ||
| 32 | 32 | ||||