Skip to content

Commit 0bc9b47

Browse files
committed
fix: fixed postgres sql bugs in stored
1 parent 8bada0b commit 0bc9b47

11 files changed

Lines changed: 241 additions & 48 deletions

File tree

go.mod

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ require (
88
github.com/dgraph-io/badger/v4 v4.9.1
99
github.com/docker/docker v28.5.2+incompatible
1010
github.com/docker/go-connections v0.6.0
11-
github.com/xraph/forge v1.7.0
11+
github.com/xraph/forge v1.7.1
1212
github.com/xraph/forgeui v1.4.1
1313
github.com/xraph/go-utils v1.1.1
14-
github.com/xraph/grove v1.5.3
15-
github.com/xraph/grove/drivers/mongodriver v1.5.3
16-
github.com/xraph/grove/drivers/pgdriver v1.5.3
17-
github.com/xraph/grove/drivers/sqlitedriver v1.5.3
14+
github.com/xraph/grove v1.5.5
15+
github.com/xraph/grove/drivers/mongodriver v1.5.5
16+
github.com/xraph/grove/drivers/pgdriver v1.5.5
17+
github.com/xraph/grove/drivers/sqlitedriver v1.5.5
1818
github.com/xraph/vessel v1.0.2
1919
go.jetify.com/typeid/v2 v2.0.0-alpha.3
2020
go.mongodb.org/mongo-driver/v2 v2.5.0

go.sum

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -525,20 +525,20 @@ github.com/xlab/treeprint v1.2.0 h1:HzHnuAF1plUN2zGlAFHbSQP2qJ0ZAD3XF5XD7OesXRQ=
525525
github.com/xlab/treeprint v1.2.0/go.mod h1:gj5Gd3gPdKtR1ikdDK6fnFLdmIS0X30kTTuNd/WEJu0=
526526
github.com/xraph/confy v0.5.2 h1:YlZDDG2sZWyiSm4yjXTLQ7RFdy/3izg/plJh1g9guEA=
527527
github.com/xraph/confy v0.5.2/go.mod h1:FPupuOi04fSgh8pKb9BOTPxyzq9WhIncfY9aDH0bLO8=
528-
github.com/xraph/forge v1.7.0 h1:Jt6DGizIFIq51CcL+3laKuYKc8Kqmmx9F6mtlnLJSEA=
529-
github.com/xraph/forge v1.7.0/go.mod h1:xO4temFTCXsrwLKYiGt+hygtkUvEOqeZdHIVjSqIacc=
528+
github.com/xraph/forge v1.7.1 h1:k5mLti4E0ChrVtWsMta0E9FabeUERl7ucL2Qn50wiJA=
529+
github.com/xraph/forge v1.7.1/go.mod h1:xO4temFTCXsrwLKYiGt+hygtkUvEOqeZdHIVjSqIacc=
530530
github.com/xraph/forgeui v1.4.1 h1:LHK1t/sZ+9zL+MNUZralO9/rc0f5UCa19dpbWTuRMNg=
531531
github.com/xraph/forgeui v1.4.1/go.mod h1:rH/+wb1tt2pXSHotWAvoP+Lt846xlIjuwPDSpS5K5mw=
532532
github.com/xraph/go-utils v1.1.1 h1:k5TOQ1qhXLdEX8W5F60mSuPcFOPc8sBsnhmjx/Kpr5g=
533533
github.com/xraph/go-utils v1.1.1/go.mod h1:tZN4SuGy9otCo6dETp7Cvkgyiy0BvaJaZbTBv4Euvo4=
534-
github.com/xraph/grove v1.5.3 h1:tu8BSqweSbsBGVy6IjSwTl7fHk7nW3MgkShKr2ttj9Q=
535-
github.com/xraph/grove v1.5.3/go.mod h1:bgjHNhnmyfEyzbdpcppRt+Zf24nNcbGKlo450Mi4giI=
536-
github.com/xraph/grove/drivers/mongodriver v1.5.3 h1:5JaPCLSVl9Fre13li9QMujfA+6EyqYKEsMkd/z5KMrc=
537-
github.com/xraph/grove/drivers/mongodriver v1.5.3/go.mod h1:xojoSuw3qSm3NIe3xBmasocRQ7oyju3XbjvdlCXVXIU=
538-
github.com/xraph/grove/drivers/pgdriver v1.5.3 h1:mxDOhVyj85oQLHEAOVBc5bcBfLgwt9GuDctnv5wMBhY=
539-
github.com/xraph/grove/drivers/pgdriver v1.5.3/go.mod h1:BBCrIuCBrRajE/dB5IKC0HshU/bUHPNS/etWtmX19qo=
540-
github.com/xraph/grove/drivers/sqlitedriver v1.5.3 h1:z+n6KFwl3JoUbvPYLiD8rZmM3TfoDsh59SN7Cd/zFSk=
541-
github.com/xraph/grove/drivers/sqlitedriver v1.5.3/go.mod h1:xzHewWROOPVn0Luu8/sWEAhSgmDnw3xmNrRw0xcefnM=
534+
github.com/xraph/grove v1.5.5 h1:Lqba0Ijywim5U5sY9wDBdcB/tAJQU+iv2DLSBRBXpss=
535+
github.com/xraph/grove v1.5.5/go.mod h1:bgjHNhnmyfEyzbdpcppRt+Zf24nNcbGKlo450Mi4giI=
536+
github.com/xraph/grove/drivers/mongodriver v1.5.5 h1:TiJ02LKSV/IeHUYDvpQTAlsGDprAiybrO5O2qUgBDuE=
537+
github.com/xraph/grove/drivers/mongodriver v1.5.5/go.mod h1:xojoSuw3qSm3NIe3xBmasocRQ7oyju3XbjvdlCXVXIU=
538+
github.com/xraph/grove/drivers/pgdriver v1.5.5 h1:Ia9PiZZuZK1xzWd3JFmASj2f1DGg1j1eQrqbYOXArok=
539+
github.com/xraph/grove/drivers/pgdriver v1.5.5/go.mod h1:BBCrIuCBrRajE/dB5IKC0HshU/bUHPNS/etWtmX19qo=
540+
github.com/xraph/grove/drivers/sqlitedriver v1.5.5 h1:L/CL1h9BM2xN1/BTXUn9t4VNBgWdQHTGXd8FjkrAe38=
541+
github.com/xraph/grove/drivers/sqlitedriver v1.5.5/go.mod h1:xzHewWROOPVn0Luu8/sWEAhSgmDnw3xmNrRw0xcefnM=
542542
github.com/xraph/vessel v1.0.2 h1:IeNTwxiFgqH2vW9lh8PNXr1SeGdEc7cxQ6sH7jMokfo=
543543
github.com/xraph/vessel v1.0.2/go.mod h1:5hgrMbuczxu2kRIM3iVJ3wPSb6HOvbMhV4nkRFaPNqs=
544544
github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 h1:ilQV1hzziu+LLM3zUTJ0trRztfwgjqKnBWNtSRkbmwM=

store/postgres/admin.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ func (s *Store) GetTenant(ctx context.Context, tenantID string) (*admin.Tenant,
4444
Name: model.Name,
4545
Status: admin.TenantStatus(model.Status),
4646
}
47+
unmarshalJSONB(model.Metadata, &tenant.Metadata)
4748

4849
return tenant, nil
4950
}
@@ -77,6 +78,7 @@ func (s *Store) GetTenantByExternalID(ctx context.Context, externalID string) (*
7778
Name: model.Name,
7879
Status: admin.TenantStatus(model.Status),
7980
}
81+
unmarshalJSONB(model.Metadata, &tenant.Metadata)
8082

8183
return tenant, nil
8284
}
@@ -106,6 +108,7 @@ func (s *Store) GetTenantBySlug(ctx context.Context, slug string) (*admin.Tenant
106108
Name: model.Name,
107109
Status: admin.TenantStatus(model.Status),
108110
}
111+
unmarshalJSONB(model.Metadata, &tenant.Metadata)
109112

110113
return tenant, nil
111114
}
@@ -161,6 +164,7 @@ func (s *Store) ListTenants(ctx context.Context, opts admin.ListTenantsOptions)
161164
Name: model.Name,
162165
Status: admin.TenantStatus(model.Status),
163166
}
167+
unmarshalJSONB(model.Metadata, &tenant.Metadata)
164168
items = append(items, tenant)
165169
}
166170

@@ -234,7 +238,14 @@ func (s *Store) CountTenantsByStatus(ctx context.Context, status admin.TenantSta
234238
func (s *Store) InsertAuditEntry(ctx context.Context, entry *admin.AuditEntry) error {
235239
model := toAuditEntryModel(entry)
236240

237-
_, err := s.pg.NewInsert(model).Exec(ctx)
241+
// The id column is BIGSERIAL, but the model tag is `id,pk` without
242+
// `autoincrement`, so grove would otherwise bind the zero value and emit
243+
// `id = 0` on every insert — the first row takes pk 0 and every subsequent
244+
// insert collides ("duplicate key value violates unique constraint"). Pin
245+
// the insert to the non-id columns so Postgres generates the sequence value.
246+
_, err := s.pg.NewInsert(model).
247+
Column("tenant_id", "actor_id", "action", "resource", "resource_id", "details", "created_at").
248+
Exec(ctx)
238249
if err != nil {
239250
return fmt.Errorf("postgres: insert audit entry failed: %w", err)
240251
}
@@ -303,6 +314,7 @@ func (s *Store) QueryAuditLog(ctx context.Context, opts admin.AuditQuery) (*admi
303314
Resource: model.Resource,
304315
ResourceID: model.ResourceID,
305316
}
317+
unmarshalJSONB(model.Details, &entry.Details)
306318
items = append(items, entry)
307319
}
308320

store/postgres/datacenter.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,18 +74,30 @@ func (s *Store) GetDatacenterBySlug(ctx context.Context, tenantID string, slug s
7474
func (s *Store) ListDatacenters(ctx context.Context, tenantID string, opts datacenter.ListOptions) (*datacenter.ListResult, error) {
7575
var models []datacenterModel
7676

77-
q := s.pg.NewSelect(&models).Where("tenant_id = $1 OR tenant_id = ''", tenantID)
78-
77+
// The OR must be parenthesized: chained Where clauses are AND-joined, and
78+
// without the parens SQL precedence reads it as
79+
// `tenant_id = $1 OR (tenant_id = '' AND <filters>)`, which leaks every
80+
// tenant row past the status/provider/region filters.
81+
q := s.pg.NewSelect(&models).Where("(tenant_id = $1 OR tenant_id = '')", tenantID)
82+
83+
// Each chained Where shares the query's positional placeholder space, so
84+
// the conditional filters must continue numbering from $2 — not restart at
85+
// $1, which bound 4 args to a single placeholder ("expected 1 arguments,
86+
// got 4"). Mirrors the argIdx pattern in instance.go List.
87+
argIdx := 1
7988
if opts.Status != "" {
80-
q = q.Where("status = $1", opts.Status)
89+
argIdx++
90+
q = q.Where(fmt.Sprintf("status = $%d", argIdx), opts.Status)
8191
}
8292

8393
if opts.Provider != "" {
84-
q = q.Where("provider_name = $1", opts.Provider)
94+
argIdx++
95+
q = q.Where(fmt.Sprintf("provider_name = $%d", argIdx), opts.Provider)
8596
}
8697

8798
if opts.Region != "" {
88-
q = q.Where("region = $1", opts.Region)
99+
argIdx++
100+
q = q.Where(fmt.Sprintf("region = $%d", argIdx), opts.Region)
89101
}
90102

91103
q = q.OrderExpr("created_at DESC")

store/postgres/deploy.go

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ package postgres
22

33
import (
44
"context"
5-
"database/sql"
6-
"errors"
75
"fmt"
86

97
ctrlplane "github.com/xraph/ctrlplane"
@@ -163,18 +161,17 @@ func (s *Store) ListReleases(ctx context.Context, tenantID string, instanceID id
163161
}
164162

165163
func (s *Store) NextReleaseVersion(ctx context.Context, tenantID string, instanceID id.ID) (int, error) {
164+
// Scalar aggregate: grove's SelectQuery.Scan only fills struct dests
165+
// (passing &int errors "dest must be a pointer to a struct" and leaks the
166+
// connection). Use a raw single-row query and COALESCE(MAX,0) so the
167+
// no-rows case naturally yields 0 → first version 1, with no sentinel
168+
// handling and no leaked connection.
166169
var maxVersion int
167170

168-
err := s.pg.NewSelect((*releaseModel)(nil)).
169-
Column("version").
170-
Where("tenant_id = $1 AND instance_id = $2", tenantID, instanceID.String()).
171-
OrderExpr("version DESC").
172-
Limit(1).
173-
Scan(ctx, &maxVersion)
174-
if errors.Is(err, sql.ErrNoRows) {
175-
return 1, nil
176-
}
177-
171+
err := s.pg.QueryRow(ctx,
172+
`SELECT COALESCE(MAX(version), 0) FROM cp_releases WHERE tenant_id = $1 AND instance_id = $2`,
173+
tenantID, instanceID.String(),
174+
).Scan(&maxVersion)
178175
if err != nil {
179176
return 0, fmt.Errorf("postgres: next release version failed: %w", err)
180177
}

store/postgres/health.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,14 @@ func (s *Store) ListChecks(ctx context.Context, tenantID string, instanceID id.I
7272
CreatedAt: model.CreatedAt,
7373
UpdatedAt: model.UpdatedAt,
7474
},
75-
TenantID: model.TenantID,
76-
InstanceID: id.MustParse(model.InstanceID),
77-
Name: model.Name,
78-
Type: health.CheckType(model.Type),
79-
Enabled: model.Enabled,
80-
Interval: time.Duration(model.Interval),
81-
Timeout: time.Duration(model.Timeout),
75+
TenantID: model.TenantID,
76+
InstanceID: id.MustParse(model.InstanceID),
77+
ServiceName: model.ServiceName,
78+
Name: model.Name,
79+
Type: health.CheckType(model.Type),
80+
Enabled: model.Enabled,
81+
Interval: time.Duration(model.Interval),
82+
Timeout: time.Duration(model.Timeout),
8283
}
8384
items = append(items, check)
8485
}
@@ -130,7 +131,13 @@ func (s *Store) DeleteCheck(ctx context.Context, tenantID string, checkID id.ID)
130131
func (s *Store) InsertResult(ctx context.Context, result *health.HealthResult) error {
131132
model := toHealthResultModel(result)
132133

133-
_, err := s.pg.NewInsert(model).Exec(ctx)
134+
// cp_health_results.id is BIGSERIAL, but healthResultModel.ID is a plain
135+
// int64 pk (no autoincrement tag), so the default insert binds id = 0 on
136+
// every row and the second insert collides on the primary key. List only
137+
// the data columns and let Postgres assign the serial id.
138+
_, err := s.pg.NewInsert(model).
139+
Column("check_id", "instance_id", "tenant_id", "status", "latency", "message", "status_code", "checked_at").
140+
Exec(ctx)
134141
if err != nil {
135142
return fmt.Errorf("postgres: insert health result failed: %w", err)
136143
}

store/postgres/migrations.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,5 +677,92 @@ CREATE INDEX IF NOT EXISTS idx_cp_bootstrap_workloads_state ON cp_bootstrap_work
677677
return err
678678
},
679679
},
680+
// cp_health_checks was created without a service_name column even
681+
// though healthCheckModel/health.HealthCheck both carry the field
682+
// (it targets a specific service inside a multi-service instance).
683+
// Inserting the full model failed with "column service_name does
684+
// not exist", so the store worked around it with a column whitelist
685+
// that silently dropped ServiceName. Add the column so it round-trips;
686+
// NULL is impossible because legacy rows default to ''.
687+
&migrate.Migration{
688+
Name: "add_service_name_to_cp_health_checks",
689+
Version: "20240101000022",
690+
Up: func(ctx context.Context, exec migrate.Executor) error {
691+
_, err := exec.Exec(ctx, `ALTER TABLE cp_health_checks ADD COLUMN IF NOT EXISTS service_name TEXT NOT NULL DEFAULT ''`)
692+
693+
return err
694+
},
695+
Down: func(ctx context.Context, exec migrate.Executor) error {
696+
_, err := exec.Exec(ctx, `ALTER TABLE cp_health_checks DROP COLUMN IF EXISTS service_name`)
697+
698+
return err
699+
},
700+
},
701+
// The multi-service refactor changed the deployment/release/template/
702+
// instance models to JSONB `services` (and friends) but the migrations
703+
// only dropped the legacy single-image columns — the replacement columns
704+
// were never added, so InsertDeployment/InsertRelease/InsertTemplate and
705+
// the instance writes failed with "column ... does not exist" (42703) on
706+
// a freshly-migrated DB. Add every column the current models expect.
707+
// All ADD COLUMN IF NOT EXISTS, so this is a safe no-op where a column
708+
// was already present.
709+
&migrate.Migration{
710+
Name: "reconcile_multi_service_columns",
711+
Version: "20240101000023",
712+
Up: func(ctx context.Context, exec migrate.Executor) error {
713+
stmts := []string{
714+
`ALTER TABLE cp_deployments ADD COLUMN IF NOT EXISTS services JSONB`,
715+
`ALTER TABLE cp_deployments ADD COLUMN IF NOT EXISTS service_progress JSONB`,
716+
`ALTER TABLE cp_releases ADD COLUMN IF NOT EXISTS services JSONB`,
717+
`ALTER TABLE cp_releases ADD COLUMN IF NOT EXISTS config JSONB`,
718+
`ALTER TABLE cp_releases ADD COLUMN IF NOT EXISTS metadata JSONB`,
719+
`ALTER TABLE cp_templates ADD COLUMN IF NOT EXISTS default_kind TEXT`,
720+
`ALTER TABLE cp_templates ADD COLUMN IF NOT EXISTS default_strategy TEXT`,
721+
`ALTER TABLE cp_templates ADD COLUMN IF NOT EXISTS services JSONB`,
722+
`ALTER TABLE cp_templates ADD COLUMN IF NOT EXISTS labels JSONB`,
723+
`ALTER TABLE cp_templates ADD COLUMN IF NOT EXISTS variables JSONB`,
724+
`ALTER TABLE cp_templates ADD COLUMN IF NOT EXISTS source JSONB`,
725+
`ALTER TABLE cp_instances ADD COLUMN IF NOT EXISTS kind TEXT`,
726+
`ALTER TABLE cp_instances ADD COLUMN IF NOT EXISTS services JSONB`,
727+
`ALTER TABLE cp_instances ADD COLUMN IF NOT EXISTS service_refs JSONB`,
728+
`ALTER TABLE cp_instances ADD COLUMN IF NOT EXISTS labels JSONB`,
729+
`ALTER TABLE cp_instances ADD COLUMN IF NOT EXISTS source JSONB`,
730+
}
731+
for _, stmt := range stmts {
732+
if _, err := exec.Exec(ctx, stmt); err != nil {
733+
return err
734+
}
735+
}
736+
737+
return nil
738+
},
739+
Down: func(ctx context.Context, exec migrate.Executor) error {
740+
stmts := []string{
741+
`ALTER TABLE cp_deployments DROP COLUMN IF EXISTS services`,
742+
`ALTER TABLE cp_deployments DROP COLUMN IF EXISTS service_progress`,
743+
`ALTER TABLE cp_releases DROP COLUMN IF EXISTS services`,
744+
`ALTER TABLE cp_releases DROP COLUMN IF EXISTS config`,
745+
`ALTER TABLE cp_releases DROP COLUMN IF EXISTS metadata`,
746+
`ALTER TABLE cp_templates DROP COLUMN IF EXISTS default_kind`,
747+
`ALTER TABLE cp_templates DROP COLUMN IF EXISTS default_strategy`,
748+
`ALTER TABLE cp_templates DROP COLUMN IF EXISTS services`,
749+
`ALTER TABLE cp_templates DROP COLUMN IF EXISTS labels`,
750+
`ALTER TABLE cp_templates DROP COLUMN IF EXISTS variables`,
751+
`ALTER TABLE cp_templates DROP COLUMN IF EXISTS source`,
752+
`ALTER TABLE cp_instances DROP COLUMN IF EXISTS kind`,
753+
`ALTER TABLE cp_instances DROP COLUMN IF EXISTS services`,
754+
`ALTER TABLE cp_instances DROP COLUMN IF EXISTS service_refs`,
755+
`ALTER TABLE cp_instances DROP COLUMN IF EXISTS labels`,
756+
`ALTER TABLE cp_instances DROP COLUMN IF EXISTS source`,
757+
}
758+
for _, stmt := range stmts {
759+
if _, err := exec.Exec(ctx, stmt); err != nil {
760+
return err
761+
}
762+
}
763+
764+
return nil
765+
},
766+
},
680767
)
681768
}

store/postgres/models.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,7 @@ func toMetricModel(m *telemetry.Metric) *metricModel {
447447
Name: m.Name,
448448
Type: string(m.Type),
449449
Value: m.Value,
450+
Labels: marshalJSONB(m.Labels),
450451
Timestamp: m.Timestamp,
451452
}
452453
}
@@ -458,6 +459,7 @@ func toLogEntryModel(log *telemetry.LogEntry) *logEntryModel {
458459
Level: log.Level,
459460
Message: log.Message,
460461
Source: log.Source,
462+
Attributes: marshalJSONB(log.Fields),
461463
Timestamp: log.Timestamp,
462464
}
463465
}
@@ -472,6 +474,7 @@ func toTraceModel(trace *telemetry.Trace) *traceModel {
472474
Operation: trace.Operation,
473475
Duration: int64(trace.Duration),
474476
Status: trace.Status,
477+
Attributes: marshalJSONB(trace.Attributes),
475478
Timestamp: trace.Timestamp,
476479
}
477480
}
@@ -552,6 +555,7 @@ func toTenantModel(tenant *admin.Tenant) *tenantModel {
552555
Slug: tenant.Slug,
553556
Name: tenant.Name,
554557
Status: string(tenant.Status),
558+
Metadata: marshalJSONB(tenant.Metadata),
555559
CreatedAt: tenant.CreatedAt,
556560
UpdatedAt: tenant.UpdatedAt,
557561
}
@@ -564,6 +568,7 @@ func toAuditEntryModel(entry *admin.AuditEntry) *auditEntryModel {
564568
Action: entry.Action,
565569
Resource: entry.Resource,
566570
ResourceID: entry.ResourceID,
571+
Details: marshalJSONB(entry.Details),
567572
CreatedAt: entry.CreatedAt,
568573
}
569574
}

store/postgres/network.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,15 @@ func (s *Store) GetCertificate(ctx context.Context, tenantID string, certID id.I
231231
func (s *Store) ListCertificates(ctx context.Context, tenantID string, instanceID id.ID) ([]network.Certificate, error) {
232232
var models []certificateModel
233233

234+
// cp_certificates has no instance_id column — certificates link to an
235+
// instance only transitively through their domain (cp_domains.instance_id).
236+
// Filtering on a non-existent instance_id column errors out with
237+
// "column instance_id does not exist", so resolve the instance's domains
238+
// via a subquery and match certificates by domain_id. The `?` placeholders
239+
// are renumbered to $1,$2,$3 by grove's appendWheres.
234240
err := s.pg.NewSelect(&models).
235-
Where("tenant_id = $1 AND instance_id = $2", tenantID, instanceID.String()).
241+
Where("tenant_id = ? AND domain_id IN (SELECT id FROM cp_domains WHERE tenant_id = ? AND instance_id = ?)",
242+
tenantID, tenantID, instanceID.String()).
236243
Scan(ctx)
237244
if err != nil {
238245
return nil, fmt.Errorf("postgres: list certificates failed: %w", err)

0 commit comments

Comments
 (0)