Skip to content

chore: migrate deployment variable values to db #817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adityachoudhari26 merged 2 commits into from
Feb 26, 2026
Merged

chore: migrate deployment variable values to db #817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
848fb87
chore: migrate deployment variable values to db
adityachoudhari26 Feb 26, 2026
3618545
cleanup
adityachoudhari26 Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions apps/workspace-engine/pkg/db/deployment_variables.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions apps/workspace-engine/pkg/db/models.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 29 additions & 0 deletions apps/workspace-engine/pkg/db/queries/deployment_variables.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,32 @@ SELECT dv.id, dv.deployment_id, dv.key, dv.description, dv.default_value
FROM deployment_variable dv
INNER JOIN deployment d ON d.id = dv.deployment_id
WHERE d.workspace_id = $1;

-- name: GetDeploymentVariableValueByID :one
SELECT id, deployment_variable_id, value, resource_selector, priority
FROM deployment_variable_value
WHERE id = $1;

-- name: ListDeploymentVariableValuesByVariableID :many
SELECT id, deployment_variable_id, value, resource_selector, priority
FROM deployment_variable_value
WHERE deployment_variable_id = $1
ORDER BY priority DESC, id;

-- name: UpsertDeploymentVariableValue :one
INSERT INTO deployment_variable_value (id, deployment_variable_id, value, resource_selector, priority)
VALUES ($1, $2, $3, $4, $5)
ON CONFLICT (id) DO UPDATE
SET deployment_variable_id = EXCLUDED.deployment_variable_id, value = EXCLUDED.value,
resource_selector = EXCLUDED.resource_selector, priority = EXCLUDED.priority
RETURNING *;

-- name: DeleteDeploymentVariableValue :exec
DELETE FROM deployment_variable_value WHERE id = $1;

-- name: ListDeploymentVariableValuesByWorkspaceID :many
SELECT dvv.id, dvv.deployment_variable_id, dvv.value, dvv.resource_selector, dvv.priority
FROM deployment_variable_value dvv
INNER JOIN deployment_variable dv ON dv.id = dvv.deployment_variable_id
INNER JOIN deployment d ON d.id = dv.deployment_id
WHERE d.workspace_id = $1;
8 changes: 8 additions & 0 deletions apps/workspace-engine/pkg/db/queries/schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,14 @@ CREATE TABLE deployment_variable (
default_value JSONB
);

CREATE TABLE deployment_variable_value (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
deployment_variable_id UUID NOT NULL REFERENCES deployment_variable(id) ON DELETE CASCADE,
value JSONB NOT NULL,
resource_selector TEXT,
priority BIGINT NOT NULL DEFAULT 0
);

CREATE TABLE resource_variable (
resource_id UUID NOT NULL REFERENCES resource(id) ON DELETE CASCADE,
key TEXT NOT NULL,
Expand Down
5 changes: 5 additions & 0 deletions apps/workspace-engine/pkg/db/sqlc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,8 @@ sql:
- column: "deployment_variable.default_value"
go_type:
type: "[]byte"

# DeploymentVariableValue
- column: "deployment_variable_value.value"
go_type:
type: "[]byte"
31 changes: 23 additions & 8 deletions apps/workspace-engine/pkg/workspace/store/deployment_variable_values.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,52 @@ package store
import (
"context"
"workspace-engine/pkg/oapi"
"workspace-engine/pkg/workspace/store/repository/memory"
"workspace-engine/pkg/workspace/store/repository"

"github.com/charmbracelet/log"
)

func NewDeploymentVariableValues(store *Store) *DeploymentVariableValues {
return &DeploymentVariableValues{repo: store.repo, store: store}
return &DeploymentVariableValues{
repo: store.repo.DeploymentVariableValues(),
store: store,
}
}

type DeploymentVariableValues struct {
repo *memory.InMemory
repo repository.DeploymentVariableValueRepo
store *Store
}

func (d *DeploymentVariableValues) SetRepo(repo repository.DeploymentVariableValueRepo) {
d.repo = repo
}

func (d *DeploymentVariableValues) Items() map[string]*oapi.DeploymentVariableValue {
return d.repo.DeploymentVariableValues.Items()
return d.repo.Items()
}

func (d *DeploymentVariableValues) Get(id string) (*oapi.DeploymentVariableValue, bool) {
return d.repo.DeploymentVariableValues.Get(id)
return d.repo.Get(id)
}

func (d *DeploymentVariableValues) Upsert(ctx context.Context, id string, deploymentVariableValue *oapi.DeploymentVariableValue) {
d.repo.DeploymentVariableValues.Set(id, deploymentVariableValue)
if err := d.repo.Set(deploymentVariableValue); err != nil {
log.Error("Failed to upsert deployment variable value", "error", err)
return
}
d.store.changeset.RecordUpsert(deploymentVariableValue)
}

func (d *DeploymentVariableValues) Remove(ctx context.Context, id string) {
deploymentVariableValue, ok := d.repo.DeploymentVariableValues.Get(id)
deploymentVariableValue, ok := d.repo.Get(id)
if !ok || deploymentVariableValue == nil {
return
}

d.repo.DeploymentVariableValues.Remove(id)
if err := d.repo.Remove(id); err != nil {
log.Error("Failed to remove deployment variable value", "error", err)
return
}
d.store.changeset.RecordDelete(deploymentVariableValue)
}
12 changes: 7 additions & 5 deletions apps/workspace-engine/pkg/workspace/store/deployment_variables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,13 @@ func (d *DeploymentVariables) Remove(ctx context.Context, id string) {
}

func (d *DeploymentVariables) Values(variableId string) map[string]*oapi.DeploymentVariableValue {
values := make(map[string]*oapi.DeploymentVariableValue)
for _, value := range d.store.DeploymentVariableValues.Items() {
if value.DeploymentVariableId == variableId {
values[value.Id] = value
}
dvvs, err := d.store.DeploymentVariableValues.repo.GetByVariableID(variableId)
if err != nil {
return make(map[string]*oapi.DeploymentVariableValue)
}
Comment on lines +56 to +59
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Don’t silently swallow repository errors in Values.

Returning an empty map on failure masks data-access issues and can lead to incorrect variable resolution behavior without any signal.

🔧 Suggested fix 
 func (d *DeploymentVariables) Values(variableId string) map[string]*oapi.DeploymentVariableValue {
 	dvvs, err := d.store.DeploymentVariableValues.repo.GetByVariableID(variableId)
 	if err != nil {
+		log.Error("Failed to list deployment variable values by variable ID", "variableId", variableId, "error", err)
 		return make(map[string]*oapi.DeploymentVariableValue)
 	}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/workspace-engine/pkg/workspace/store/deployment_variables.go` around
lines 56 - 59, The Values method is silently swallowing repository errors by
returning an empty map when
d.store.DeploymentVariableValues.repo.GetByVariableID(variableId) fails; change
Values to propagate the error instead of hiding it: update the Values method
signature to return (map[string]*oapi.DeploymentVariableValue, error) if it does
not already, and on err return nil and the wrapped error (or the original error)
so callers can handle/log it; update all callers of Values to handle the
returned error accordingly.

values := make(map[string]*oapi.DeploymentVariableValue, len(dvvs))
for _, v := range dvvs {
values[v.Id] = v
}
return values
}
54 changes: 54 additions & 0 deletions apps/workspace-engine/pkg/workspace/store/repository/db/deploymentvariables/mapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,57 @@ func ToVariableUpsertParams(e *oapi.DeploymentVariable) (db.UpsertDeploymentVari
DefaultValue: defaultValue,
}, nil
}

func ValueToOapi(row db.DeploymentVariableValue) *oapi.DeploymentVariableValue {
var resourceSelector *oapi.Selector
if row.ResourceSelector.Valid && row.ResourceSelector.String != "" {
sel := &oapi.Selector{}
celJSON, _ := json.Marshal(oapi.CelSelector{Cel: row.ResourceSelector.String})
_ = sel.UnmarshalJSON(celJSON)
resourceSelector = sel
}

value := oapi.Value{}
_ = value.UnmarshalJSON(row.Value)

return &oapi.DeploymentVariableValue{
Id: row.ID.String(),
DeploymentVariableId: row.DeploymentVariableID.String(),
Value: value,
ResourceSelector: resourceSelector,
Priority: row.Priority,
}
}

func ToValueUpsertParams(e *oapi.DeploymentVariableValue) (db.UpsertDeploymentVariableValueParams, error) {
id, err := uuid.Parse(e.Id)
if err != nil {
return db.UpsertDeploymentVariableValueParams{}, fmt.Errorf("parse id: %w", err)
}

dvid, err := uuid.Parse(e.DeploymentVariableId)
if err != nil {
return db.UpsertDeploymentVariableValueParams{}, fmt.Errorf("parse deployment_variable_id: %w", err)
}

var resourceSelector pgtype.Text
if e.ResourceSelector != nil {
cel, err := e.ResourceSelector.AsCelSelector()
if err == nil && cel.Cel != "" {
resourceSelector = pgtype.Text{String: cel.Cel, Valid: true}
}
}
Comment on lines +101 to +107
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify selector union behavior and mapper error handling paths.

# 1) Inspect Selector union helpers in generated OAPI to confirm AsCelSelector failure modes.
echo "=== OAPI Selector union and AsCelSelector ==="
rg -n -C3 'type Selector|AsCelSelector\(|UnmarshalJSON\(' apps/workspace-engine/pkg/oapi/oapi.gen.go | head -100

# 2) Inspect current mapper handling around AsCelSelector in this file.
echo -e "\n=== Mapper AsCelSelector handling ==="
rg -n -C4 'ToValueUpsertParams|AsCelSelector|resourceSelector' apps/workspace-engine/pkg/workspace/store/repository/db/deploymentvariables/mapper.go

# 3) Check where deployment variable values are built/validated before persistence.
echo -e "\n=== Deployment variable validation and building ==="
rg -n -C3 'DeploymentVariableValue|ResourceSelector' apps/workspace-engine/pkg/workspace/store/repository/db/deploymentvariables/mapper.go | head -80

Repository: ctrlplanedev/ctrlplane

Length of output: 5765

🏁 Script executed:

#!/bin/bash
# Check callers of ToValueUpsertParams to understand usage context
rg -n 'ToValueUpsertParams' apps/workspace-engine/pkg --type go -A 3 -B 1

Repository: ctrlplanedev/ctrlplane

Length of output: 1382

Handle AsCelSelector() errors explicitly to maintain consistency with other field validations.

On lines 103–107, AsCelSelector() errors are silently ignored while ID, DeploymentVariableID, and Value errors are explicitly propagated (lines 93, 98, 111). This inconsistency allows invalid selectors to persist as NULL without error, potentially dropping resource constraints unexpectedly. Since the function signature already supports error returns and the caller explicitly handles errors (repo.go:125), validation failures should be surfaced rather than masked.

Proposed fix 
 var resourceSelector pgtype.Text
 if e.ResourceSelector != nil {
   cel, err := e.ResourceSelector.AsCelSelector()
-  if err == nil && cel.Cel != "" {
-    resourceSelector = pgtype.Text{String: cel.Cel, Valid: true}
-  }
+  if err != nil {
+    return db.UpsertDeploymentVariableValueParams{}, fmt.Errorf("parse resource_selector: %w", err)
+  }
+  if cel.Cel != "" {
+    resourceSelector = pgtype.Text{String: cel.Cel, Valid: true}
+  }
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@apps/workspace-engine/pkg/workspace/store/repository/db/deploymentvariables/mapper.go`
around lines 101 - 107, The mapping code suppresses errors from
e.ResourceSelector.AsCelSelector(), causing invalid selectors to be dropped
silently; change the logic in mapper.go to mirror other field validations (like
ID/DeploymentVariableID/Value) by checking the error returned from
AsCelSelector() and returning/propagating it (or wrapping it) instead of
ignoring it, and only set resourceSelector = pgtype.Text{String: cel.Cel, Valid:
true} when err == nil and cel.Cel != "" so invalid selector parsing surfaces to
the caller (repo.go:125) for proper handling.


valueBytes, err := e.Value.MarshalJSON()
if err != nil {
return db.UpsertDeploymentVariableValueParams{}, fmt.Errorf("marshal value: %w", err)
}

return db.UpsertDeploymentVariableValueParams{
ID: id,
DeploymentVariableID: dvid,
Value: valueBytes,
ResourceSelector: resourceSelector,
Priority: e.Priority,
}, nil
}
Loading
Loading