Skip to content

Commit bd06bab

Browse files
committed
fix: address PostgREST authenticator review findings
- Hoist parsed PostgREST config outside the service type switch so the per-node authenticator loop reuses the already-validated result instead of re-parsing and silently discarding errors, which could cause per-node authenticators to use a different anon role than the primary. - Fix double `%w` in `PostgRESTAuthenticatorResource.Refresh` — replace with `errors.Join` so both `resource.ErrNotFound` and the original error are independently unwrappable via `errors.Is`/`errors.As`.
1 parent 0c96852 commit bd06bab

2 files changed

Lines changed: 10 additions & 4 deletions

File tree

server/internal/orchestrator/swarm/orchestrator.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,10 @@ func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceIn
462462
// Service-type-specific resources.
463463
var serviceSpecificResources []resource.Resource
464464

465+
// Hoisted PostgREST config — parsed once and reused in both the primary
466+
// resource block and the per-node authenticator loop below.
467+
var parsedPostgRESTConfig *database.PostgRESTServiceConfig
468+
465469
switch spec.ServiceSpec.ServiceType {
466470
case "mcp":
467471
mcpConfig, errs := database.ParseMCPServiceConfig(spec.ServiceSpec.Config, false)
@@ -489,10 +493,12 @@ func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceIn
489493
serviceSpecificResources = []resource.Resource{dataDir, mcpConfigResource}
490494

491495
case "postgrest":
492-
postgrestConfig, errs := database.ParsePostgRESTServiceConfig(spec.ServiceSpec.Config)
496+
var errs []error
497+
parsedPostgRESTConfig, errs = database.ParsePostgRESTServiceConfig(spec.ServiceSpec.Config)
493498
if len(errs) > 0 {
494499
return nil, fmt.Errorf("failed to parse PostgREST service config: %w", errors.Join(errs...))
495500
}
501+
postgrestConfig := parsedPostgRESTConfig
496502

497503
preflight := &PostgRESTPreflightResource{
498504
ServiceID: spec.ServiceSpec.ServiceID,
@@ -594,14 +600,13 @@ func (o *Orchestrator) GenerateServiceInstanceResources(spec *database.ServiceIn
594600
},
595601
)
596602
if spec.ServiceSpec.ServiceType == "postgrest" {
597-
postgrestConfig, _ := database.ParsePostgRESTServiceConfig(spec.ServiceSpec.Config)
598603
orchestratorResources = append(orchestratorResources,
599604
&PostgRESTAuthenticatorResource{
600605
ServiceID: spec.ServiceSpec.ServiceID,
601606
DatabaseID: spec.DatabaseID,
602607
DatabaseName: spec.DatabaseName,
603608
NodeName: nodeInst.NodeName,
604-
DBAnonRole: postgrestConfig.DBAnonRole,
609+
DBAnonRole: parsedPostgRESTConfig.DBAnonRole,
605610
UserRoleID: perNodeRWID,
606611
},
607612
)

server/internal/orchestrator/swarm/postgrest_authenticator_resource.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package swarm
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67

78
"github.com/jackc/pgx/v5"
@@ -97,7 +98,7 @@ func (r *PostgRESTAuthenticatorResource) Refresh(ctx context.Context, rc *resour
9798
r.authenticatorUsername(),
9899
).Scan(&noInherit)
99100
if err != nil {
100-
return fmt.Errorf("%w: role %q not found: %w", resource.ErrNotFound, r.authenticatorUsername(), err)
101+
return fmt.Errorf("%w", errors.Join(resource.ErrNotFound, fmt.Errorf("role %q not found: %w", r.authenticatorUsername(), err)))
101102
}
102103
if !noInherit {
103104
return fmt.Errorf("%w: role %q does not have NOINHERIT", resource.ErrNotFound, r.authenticatorUsername())

0 commit comments

Comments
 (0)