Skip to content

refactor: replace selectors with a single selector field in policy sc… #794

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
jsbroks merged 9 commits into from
Feb 8, 2026
Merged

refactor: replace selectors with a single selector field in policy sc… #794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cb40d5f
refactor: replace selectors with a single selector field in policy sc…
jsbroks Feb 8, 2026
dba161b
fix: update MatchPolicy logic to ensure empty selectors do not match
jsbroks Feb 8, 2026
460772e
feat: add tests for policy selector migration handling
jsbroks Feb 8, 2026
7f5e5fe
refactor: remove deprecated API paths from schema
jsbroks Feb 8, 2026
8229754
fix: ensure rules are always defined in policy upsert
jsbroks Feb 8, 2026
e16ebe9
engine(persistence): preserve CEL precedence by parenthesizing AND pa…
cursoragent Feb 8, 2026
305c166
web: switch policy target display to single CEL selector; remove lega…
cursoragent Feb 8, 2026
68277cb
fix(tests): clean up whitespace in policy test cases for improved rea…
jsbroks Feb 8, 2026
efeb147
fix(persistence): handle single AND parts in policy selector migratio…
jsbroks Feb 8, 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
53 changes: 16 additions & 37 deletions apps/api/openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,9 @@
},
"type": "array"
},
"selectors": {
"items": {
"$ref": "#/components/schemas/PolicyTargetSelector"
},
"type": "array"
"selector": {
"description": "CEL expression for matching release targets. Use \"true\" to match all targets.",
"type": "string"
}
},
"required": [
Expand Down Expand Up @@ -1193,11 +1191,9 @@
},
"type": "array"
},
"selectors": {
"items": {
"$ref": "#/components/schemas/PolicyTargetSelector"
},
"type": "array"
"selector": {
"description": "CEL expression for matching release targets. Use \"true\" to match all targets.",
"type": "string"
},
"workspaceId": {
"type": "string"
Expand All @@ -1208,7 +1204,7 @@
"name",
"createdAt",
"workspaceId",
"selectors",
"selector",
"rules",
"metadata",
"priority",
Expand Down Expand Up @@ -1274,26 +1270,6 @@
],
"type": "object"
},
"PolicyTargetSelector": {
"properties": {
"deploymentSelector": {
"$ref": "#/components/schemas/Selector"
},
"environmentSelector": {
"$ref": "#/components/schemas/Selector"
},
"id": {
"type": "string"
},
"resourceSelector": {
"$ref": "#/components/schemas/Selector"
}
},
"required": [
"id"
],
"type": "object"
},
"PropertyMatcher": {
"properties": {
"fromProperty": {
Expand Down Expand Up @@ -2085,15 +2061,18 @@
},
"type": "array"
},
"selectors": {
"items": {
"$ref": "#/components/schemas/PolicyTargetSelector"
},
"type": "array"
"selector": {
"description": "CEL expression for matching release targets. Use \"true\" to match all targets.",
"type": "string"
}
},
"required": [
"name"
"name",
"selector",
"rules",
"priority",
"enabled",
"metadata"
],
"type": "object"
},
Expand Down
33 changes: 11 additions & 22 deletions apps/api/openapi/schemas/policies.jsonnet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ local openapi = import '../lib/openapi.libsonnet';
description: { type: 'string' },
priority: { type: 'integer' },
enabled: { type: 'boolean' },
selectors: {
type: 'array',
items: openapi.schemaRef('PolicyTargetSelector'),
selector: {
type: 'string',
description: 'CEL expression for matching release targets. Use "true" to match all targets.',
},
rules: {
type: 'array',
Expand All @@ -27,15 +27,15 @@ local openapi = import '../lib/openapi.libsonnet';

UpsertPolicyRequest: {
type: 'object',
required: ['name'],
required: ['name', 'selector', 'rules', 'priority', 'enabled', 'metadata'],
properties: {
name: { type: 'string' },
description: { type: 'string' },
priority: { type: 'integer' },
enabled: { type: 'boolean' },
selectors: {
type: 'array',
items: openapi.schemaRef('PolicyTargetSelector'),
selector: {
type: 'string',
description: 'CEL expression for matching release targets. Use "true" to match all targets.',
},
rules: {
type: 'array',
Expand All @@ -60,7 +60,7 @@ local openapi = import '../lib/openapi.libsonnet';

Policy: {
type: 'object',
required: ['id', 'name', 'createdAt', 'workspaceId', 'selectors', 'rules', 'metadata', 'priority', 'enabled'],
required: ['id', 'name', 'createdAt', 'workspaceId', 'selector', 'rules', 'metadata', 'priority', 'enabled'],
properties: {
id: { type: 'string' },
name: { type: 'string' },
Expand All @@ -69,9 +69,9 @@ local openapi = import '../lib/openapi.libsonnet';
workspaceId: { type: 'string' },
priority: { type: 'integer' },
enabled: { type: 'boolean' },
selectors: {
type: 'array',
items: openapi.schemaRef('PolicyTargetSelector'),
selector: {
type: 'string',
description: 'CEL expression for matching release targets. Use "true" to match all targets.',
},
rules: {
type: 'array',
Expand All @@ -85,17 +85,6 @@ local openapi = import '../lib/openapi.libsonnet';
},
},

PolicyTargetSelector: {
type: 'object',
required: ['id'],
properties: {
id: { type: 'string' },
deploymentSelector: openapi.schemaRef('Selector'),
environmentSelector: openapi.schemaRef('Selector'),
resourceSelector: openapi.schemaRef('Selector'),
},
},

PolicyRule: {
type: 'object',
required: ['id', 'policyId', 'createdAt'],
Expand Down
14 changes: 6 additions & 8 deletions apps/api/src/routes/v1/workspaces/policies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ const upsertPolicy: AsyncTypedHandler<
{ params: { path: { workspaceId, policyId } } },
);

const selectors = Array.isArray(body.selectors) ? body.selectors : [];

const policyIdResult = z.string().uuid().safeParse(policyId);
if (!policyIdResult.success)
throw new ApiError("Invalid policy ID: must be a valid UUID v4", 400);
Expand All @@ -82,11 +80,11 @@ const upsertPolicy: AsyncTypedHandler<
createdAt: existingPolicy.data?.createdAt ?? new Date().toISOString(),
name: body.name,
description: body.description,
priority: body.priority ?? 0,
enabled: body.enabled ?? true,
metadata: body.metadata ?? {},
rules: body.rules ?? [],
selectors,
priority: body.priority,
enabled: body.enabled,
metadata: body.metadata,
rules: body.rules,
selector: body.selector,
};

// Determine if this is a create or update
Expand Down Expand Up @@ -142,7 +140,7 @@ const createPolicy: AsyncTypedHandler<
priority: 0,
createdAt: new Date().toISOString(),
metadata: {},
selectors: [],
selector: "true",
...body,
rules: body.rules ?? [],
};
Expand Down
23 changes: 10 additions & 13 deletions apps/api/src/types/openapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,8 @@ export interface components {
name: string;
priority?: number;
rules?: components["schemas"]["PolicyRule"][];
selectors?: components["schemas"]["PolicyTargetSelector"][];
/** @description CEL expression for matching release targets. Use "true" to match all targets. */
selector?: string;
};
CreateRelationshipRuleRequest: {
description?: string;
Expand Down Expand Up @@ -1212,7 +1213,8 @@ export interface components {
name: string;
priority: number;
rules: components["schemas"]["PolicyRule"][];
selectors: components["schemas"]["PolicyTargetSelector"][];
/** @description CEL expression for matching release targets. Use "true" to match all targets. */
selector: string;
workspaceId: string;
};
PolicyRequestAccepted: {
Expand All @@ -1232,12 +1234,6 @@ export interface components {
verification?: components["schemas"]["VerificationRule"];
versionCooldown?: components["schemas"]["VersionCooldownRule"];
};
PolicyTargetSelector: {
deploymentSelector?: components["schemas"]["Selector"];
environmentSelector?: components["schemas"]["Selector"];
id: string;
resourceSelector?: components["schemas"]["Selector"];
};
PropertyMatcher: {
fromProperty: string[];
/** @enum {string} */
Expand Down Expand Up @@ -1515,15 +1511,16 @@ export interface components {
};
UpsertPolicyRequest: {
description?: string;
enabled?: boolean;
enabled: boolean;
/** @description Arbitrary metadata for the policy (record<string, string>) */
metadata?: {
metadata: {
[key: string]: string;
};
name: string;
priority?: number;
rules?: components["schemas"]["PolicyRule"][];
selectors?: components["schemas"]["PolicyTargetSelector"][];
priority: number;
rules: components["schemas"]["PolicyRule"][];
/** @description CEL expression for matching release targets. Use "true" to match all targets. */
selector: string;
};
Comment on lines 1512 to 1524
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for callers constructing or using UpsertPolicyRequest payloads
rg -n --type=ts -C3 'UpsertPolicyRequest|requestPolicyUpsert|upsertPolicy' -g '!**/openapi.ts' | head -100

Repository: ctrlplanedev/ctrlplane

Length of output: 5300

🏁 Script executed:

#!/bin/bash
# Also search for any test files or examples that might construct UpsertPolicyRequest
rg -n 'UpsertPolicyRequest' --type=ts --type=js -C2

Repository: ctrlplanedev/ctrlplane

Length of output: 1326

🏁 Script executed:

#!/bin/bash
# Check if there are any type definitions or interfaces that extend or reference UpsertPolicyRequest
rg -n 'extends.*UpsertPolicyRequest|implements.*UpsertPolicyRequest' --type=ts

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

#!/bin/bash
# Search for API calls that might be affected, looking for patterns like axios/fetch calls with policy upsert
rg -n 'PUT.*policy|PATCH.*policy' --type=ts --type=js -C2 | head -80

Repository: ctrlplanedev/ctrlplane

Length of output: 3369

🏁 Script executed:

#!/bin/bash
# Look at the upsertPolicyFixtures implementation
sed -n '549,600p' e2e/api/entities-builder.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 1437

🏁 Script executed:

#!/bin/bash
# Examine the upsertPolicy route handler
sed -n '58,120p' apps/api/src/routes/v1/workspaces/policies.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 1891

🏁 Script executed:

#!/bin/bash
# Search for actual policy payload constructions in fixtures
rg -A 10 'policies:' e2e/fixtures/ --type=yaml --type=yml

Repository: ctrlplanedev/ctrlplane

Length of output: 93

🏁 Script executed:

#!/bin/bash
# Check if there's a fixture file referenced
find e2e -name "*.yaml" -o -name "*.yml" | head -20

Repository: ctrlplanedev/ctrlplane

Length of output: 990

🏁 Script executed:

#!/bin/bash
# Check one of the policy fixture files to see actual payload structure
cat e2e/tests/api/policies/approval-policy.spec.yaml | head -100

Repository: ctrlplanedev/ctrlplane

Length of output: 1041

🏁 Script executed:

#!/bin/bash
# Search for actual PUT requests to the upsert endpoint
rg -n 'PUT.*policies' e2e --type=ts -C5

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

#!/bin/bash
# Check if there are integration tests actually using the PUT upsert endpoint
rg -n 'api\.PUT.*policies' e2e --type=ts -A 10 | head -80

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

#!/bin/bash
# Let's look closer at what fields the handler actually requires vs what's optional
sed -n '58,95p' apps/api/src/routes/v1/workspaces/policies.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 1229

🏁 Script executed:

#!/bin/bash
# Check the CreatePolicy endpoint to see what it requires for comparison
rg -n -A 30 'const createPolicy:' apps/api/src/routes/v1/workspaces/policies.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 870

UpsertPolicyRequest requires all major fields; this is a breaking API change from create behavior.

The enabled, metadata, priority, rules, and selector fields are now all required in upsert payloads. Unlike the createPolicy handler (which provides defaults: enabled: true, priority: 0, metadata: {}, selector: "true"), the upsertPolicy handler has no such defaults—it extracts fields directly from the request body. Clients that previously relied on optional defaults for partial upsert payloads will receive validation errors. This is an intentional asymmetry between create and upsert semantics, but callers (SDKs, internal services, or direct API consumers) must be updated to provide all required fields.

🤖 Prompt for AI Agents 
In `@apps/api/src/types/openapi.ts` around lines 1512 - 1524, The OpenAPI type
UpsertPolicyRequest currently requires enabled, metadata, priority, rules, and
selector which breaks backward compatibility; either make these fields optional
in apps/api/src/types/openapi.ts (change enabled, metadata, priority, rules,
selector to optional with same types) or update the upsert handler
(upsertPolicy) to apply the same defaults as createPolicy (enabled: true,
priority: 0, metadata: {}, selector: "true") when those properties are missing;
pick one approach and keep the other consistent (type vs runtime) so partial
upsert payloads from existing clients continue to work.

UpsertRelationshipRuleRequest: {
description?: string;
Expand Down
23 changes: 10 additions & 13 deletions apps/web/app/api/openapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,8 @@ export interface components {
name: string;
priority?: number;
rules?: components["schemas"]["PolicyRule"][];
selectors?: components["schemas"]["PolicyTargetSelector"][];
/** @description CEL expression for matching release targets. Use "true" to match all targets. */
selector?: string;
};
CreateRelationshipRuleRequest: {
description?: string;
Expand Down Expand Up @@ -1212,7 +1213,8 @@ export interface components {
name: string;
priority: number;
rules: components["schemas"]["PolicyRule"][];
selectors: components["schemas"]["PolicyTargetSelector"][];
/** @description CEL expression for matching release targets. Use "true" to match all targets. */
selector: string;
workspaceId: string;
};
PolicyRequestAccepted: {
Expand All @@ -1232,12 +1234,6 @@ export interface components {
verification?: components["schemas"]["VerificationRule"];
versionCooldown?: components["schemas"]["VersionCooldownRule"];
};
PolicyTargetSelector: {
deploymentSelector?: components["schemas"]["Selector"];
environmentSelector?: components["schemas"]["Selector"];
id: string;
resourceSelector?: components["schemas"]["Selector"];
};
PropertyMatcher: {
fromProperty: string[];
/** @enum {string} */
Expand Down Expand Up @@ -1515,15 +1511,16 @@ export interface components {
};
UpsertPolicyRequest: {
description?: string;
enabled?: boolean;
enabled: boolean;
/** @description Arbitrary metadata for the policy (record<string, string>) */
metadata?: {
metadata: {
[key: string]: string;
};
name: string;
priority?: number;
rules?: components["schemas"]["PolicyRule"][];
selectors?: components["schemas"]["PolicyTargetSelector"][];
priority: number;
rules: components["schemas"]["PolicyRule"][];
/** @description CEL expression for matching release targets. Use "true" to match all targets. */
selector: string;
Comment on lines 1512 to +1523
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 8, 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:

rg -n 'PolicyTargetSelector' --type=ts

Repository: ctrlplanedev/ctrlplane

Length of output: 435

🏁 Script executed:

cat -n apps/web/app/api/openapi.ts | sed -n '1512,1523p'

Repository: ctrlplanedev/ctrlplane

Length of output: 667

🏁 Script executed:

# Also check if PolicyTargetSelector is defined anywhere in the file
rg -n 'PolicyTargetSelector' apps/web/app/api/openapi.ts -C3

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

# Check a wider context around lines 1512-1523 to see the old structure (if any) and confirm the current state
cat -n apps/web/app/api/openapi.ts | sed -n '1500,1550p'

Repository: ctrlplanedev/ctrlplane

Length of output: 2229

🏁 Script executed:

cd apps/web/app/api && git diff HEAD -- openapi.ts | head -150

Repository: ctrlplanedev/ctrlplane

Length of output: 48

🏁 Script executed:

# Also check if there's a commit message or PR info that confirms what changed
git log --oneline -20 -- apps/web/app/api/openapi.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 149

🏁 Script executed:

git show cb40d5f9:apps/web/app/api/openapi.ts | sed -n '1500,1550p'

Repository: ctrlplanedev/ctrlplane

Length of output: 1872

🏁 Script executed:

# Show the actual commit diff for the UpsertPolicyRequest section
git show cb40d5f9 -- apps/web/app/api/openapi.ts | grep -A50 "UpsertPolicyRequest"

Repository: ctrlplanedev/ctrlplane

Length of output: 3649

🏁 Script executed:

# Show the actual diff (before and after) for this commit
git show cb40d5f9 -- apps/web/app/api/openapi.ts | grep -B20 -A20 "UpsertPolicyRequest" | head -100

Repository: ctrlplanedev/ctrlplane

Length of output: 2850

🏁 Script executed:

# Check the previous version of the file to see what UpsertPolicyRequest looked like before
git show cb40d5f9^:apps/web/app/api/openapi.ts 2>/dev/null | grep -A30 "UpsertPolicyRequest:" || echo "Pattern not found or commit has no parent"

Repository: ctrlplanedev/ctrlplane

Length of output: 107

🏁 Script executed:

# Check git log to see if there's a prior version with 'selectors' instead of 'selector'
git log --all --oneline -p -- apps/web/app/api/openapi.ts | grep -B5 -A15 "selectors.*PolicyTargetSelector" | head -50

Repository: ctrlplanedev/ctrlplane

Length of output: 1666

🏁 Script executed:

# Also check if this file is auto-generated by looking for a generation script or header
head -20 apps/web/app/api/openapi.ts

Repository: ctrlplanedev/ctrlplane

Length of output: 570

Breaking API change: UpsertPolicyRequest introduces multiple required fields and replaces selectors with selector.

This is a breaking change for existing API consumers:

  • priority and rules — promoted from optional to required.
  • enabled and metadata — new required fields.
  • selectors?: PolicyTargetSelector[] → selector: string — field renamed, type changed from array to string, and required.

Existing clients calling the upsert endpoint without these fields will receive validation errors. Ensure all known consumers are updated and that API versioning or migration documentation addresses this breaking change. Note: PolicyTargetSelector remains in use in the UI layer; only the OpenAPI schema was regenerated.

🤖 Prompt for AI Agents 
In `@apps/web/app/api/openapi.ts` around lines 1512 - 1523, The OpenAPI schema
change makes UpsertPolicyRequest breaking by making enabled, metadata, priority,
and rules required and by replacing selectors?: PolicyTargetSelector[] with
selector: string; revert or adjust the schema so existing clients keep
compatibility: either mark enabled, metadata, priority, and rules as optional
again in the UpsertPolicyRequest schema or add a new versioned request type
(e.g., UpsertPolicyRequestV2) while preserving the original UpsertPolicyRequest
with selectors?: PolicyTargetSelector[]; update any server-side validation logic
that enforces required fields (the upsert endpoint handler) to accept the
original optional fields or branch on request version, and ensure
selector/selectors mapping is handled (convert incoming selectors array to the
new selector string if supporting both) so current consumers don’t break.

};
UpsertRelationshipRuleRequest: {
description?: string;
Expand Down
41 changes: 8 additions & 33 deletions apps/web/app/routes/ws/deployments/page.$deploymentId.policies.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ type ReleaseTargetWithState =
WorkspaceEngine["schemas"]["ReleaseTargetWithState"];
type PolicyRule = WorkspaceEngine["schemas"]["PolicyRule"];
type Selector = WorkspaceEngine["schemas"]["Selector"];
type PolicyTargetSelector =
WorkspaceEngine["schemas"]["PolicyTargetSelector"];

const releaseTargetKey = (releaseTarget: ReleaseTarget) =>
`${releaseTarget.resourceId}-${releaseTarget.environmentId}-${releaseTarget.deploymentId}`;
Expand Down Expand Up @@ -166,31 +164,10 @@ const getRuleDetails = (rule: PolicyRule): string[] => {
return [];
};

const getPolicyTargetDescriptions = (selectors: PolicyTargetSelector[]) => {
return selectors
.map((selector, idx) => {
const parts: string[] = [];
if (selector.deploymentSelector != null) {
parts.push(
`deployment: ${truncateText(formatSelector(selector.deploymentSelector))}`,
);
}
if (selector.environmentSelector != null) {
parts.push(
`environment: ${truncateText(formatSelector(selector.environmentSelector))}`,
);
}
if (selector.resourceSelector != null) {
parts.push(
`resource: ${truncateText(formatSelector(selector.resourceSelector))}`,
);
}
if (parts.length === 0) return null;
return { id: selector.id ?? `selector-${idx}`, value: parts.join(" | ") };
})
.filter(
(item): item is { id: string; value: string } => item != null,
);
const getPolicySelectorDescription = (selector?: string): string[] => {
const value = selector?.trim();
if (!value || value === "true") return [];
return [value];
};

type PolicyResourceRowProps = {
Expand Down Expand Up @@ -243,9 +220,7 @@ const PolicyReleaseTargetsGroup: React.FC<PolicyReleaseTargetsGroupProps> = ({
name: getRuleDisplay(rule),
details: getRuleDetails(rule),
}));
const selectorSummaries = getPolicyTargetDescriptions(
policy.policy.selectors ?? [],
);
const selectorSummaries = getPolicySelectorDescription(policy.policy.selector);
return (
<Fragment>
<TableRow>
Expand Down Expand Up @@ -293,10 +268,10 @@ const PolicyReleaseTargetsGroup: React.FC<PolicyReleaseTargetsGroupProps> = ({
{selectorSummaries.length > 0 && (
<div className="space-y-1 text-xs text-muted-foreground">
<div className="text-xs font-medium uppercase tracking-wide text-foreground">
Targets
Target selector (CEL)
</div>
{selectorSummaries.map((selector) => (
<div key={selector.id}>{selector.value}</div>
{selectorSummaries.map((value, idx) => (
<div key={idx}>{value}</div>
))}
</div>
)}
Expand Down
Loading
Loading