Skip to content

fix: resource search supports direct CEL expressions #981

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
Apr 13, 2026
+18 −2
Merged

fix: resource search supports direct CEL expressions #981

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55c4884
fix: resource search supports direct CEL expressions
adityachoudhari26 Apr 13, 2026
f5adcc9
cleanup
adityachoudhari26 Apr 13, 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
20 changes: 18 additions & 2 deletions apps/web/app/routes/ws/resources.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useState } from "react";
import { parse } from "cel-js";
import { Plus, Search } from "lucide-react";
import { useSearchParams } from "react-router";
import { useDebounce } from "react-use";
Expand Down Expand Up @@ -50,15 +51,30 @@ function useSearch() {
return { search, setSearch, searchDebounced };
}

function buildResourceSelector(search: string): string {
if (search === "") return "true";

if (search.includes("resource.")) {
try {
const result = parse(search);
if (result.isSuccess) return search;
} catch {
// not valid CEL, fall through to wrapping
}
}
Comment on lines +57 to +64
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't restrict direct CEL mode to resource. queries only.

Line 57 makes CEL parsing conditional on resource., so valid raw selectors using other supported prefixes will be rewritten into the fallback .contains(...) query instead of being executed as CEL. That makes the new feature partial and silently changes query semantics.

Broaden the CEL-path detection beyond a single prefix, or make CEL mode explicit instead of inferring it from resource.. Based on learnings: selectorToCel handles entity-type prefixes like resource. and deployment. in raw CEL expressions.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/web/app/routes/ws/resources.tsx` around lines 57 - 64, The current
conditional restricts raw CEL parsing to strings containing "resource.", causing
valid CEL expressions with other prefixes (e.g., "deployment.") to be wrapped
incorrectly; change the logic in the block that uses parse(search) so you
attempt CEL parse more broadly—either remove the search.includes("resource.")
guard and try parse(search) for any input (returning search when
result.isSuccess), or switch to an explicit CEL-mode flag using the existing
selectorToCel conversion path; update the code around the parse(search) call and
any related branching so successful CEL parses (parse, result.isSuccess) for
other entity prefixes are accepted instead of being rewritten.

Copy link
Copy Markdown
Member Author

@adityachoudhari26 adityachoudhari26 Apr 13, 2026

Choose a reason for hiding this comment

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

this is restricted to the resource page so this is fine

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Apr 13, 2026

Choose a reason for hiding this comment

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

@adityachoudhari26, that's a fair point — since this is the resource-specific page, only resource. prefixed CEL expressions are semantically meaningful here. The restriction is intentional and correct. I'll withdraw the concern.

✏️ Learnings added 
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 981
File: apps/web/app/routes/ws/resources.tsx:57-64
Timestamp: 2026-04-13T18:27:02.578Z
Learning: In `apps/web/app/routes/ws/resources.tsx`, the `getCleanedSearch` function intentionally restricts direct CEL parsing to strings containing `"resource."`. This is by design because the page is the Resources page and only `resource.*` prefixed CEL expressions are relevant there. Do not flag the missing support for other prefixes (e.g., `deployment.`) as an issue.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 845
File: apps/workspace-engine/pkg/workspace/store/relationship_indexes.go:180-180
Timestamp: 2026-03-12T22:11:19.490Z
Learning: In `apps/workspace-engine/pkg/workspace/store/relationship_indexes.go`, the `selectorToCel` function uses `strings.ReplaceAll` to replace entity-type prefixes (e.g., "resource.", "deployment.") in raw CEL expressions. This behavior was present pre-refactor and is intentionally preserved. Do not flag it as a new issue in this file.

Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 237
File: apps/webservice/src/app/[workspaceSlug]/(app)/_components/deployment-resource-drawer/DeploymentResourceDrawer.tsx:43-50
Timestamp: 2024-11-27T23:16:35.580Z
Learning: In `DeploymentResourceDrawer.tsx`, the `isOpen` variable already checks whether `deploymentId`, `environmentId`, and `resourceId` are non-null, so additional null checks in query `enabled` conditions are not necessary.

Learnt from: CR
Repo: ctrlplanedev/ctrlplane PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-09T16:35:48.799Z
Learning: Policies are CEL-based declarative rules with a `selector` field (CEL expression) matching resources/environments

Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 637
File: packages/events/src/kafka/client.ts:10-16
Timestamp: 2025-08-01T04:41:41.345Z
Learning: User adityachoudhari26 prefers not to add null safety checks for required environment variables when they are guaranteed to be present in their deployment configuration, similar to their preference for simplicity over defensive programming in test code.

Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 601
File: e2e/tests/api/policies/retry-policy.spec.ts:23-24
Timestamp: 2025-06-24T23:52:50.732Z
Learning: The user adityachoudhari26 prefers not to add null safety checks or defensive programming in test code, particularly in e2e tests, as they prioritize simplicity and focus on the main functionality being tested rather than comprehensive error handling within the test itself.

Learnt from: adityachoudhari26
Repo: ctrlplanedev/ctrlplane PR: 818
File: apps/workspace-engine/pkg/db/queries/schema.sql:252-282
Timestamp: 2026-02-26T23:01:30.641Z
Learning: User adityachoudhari26 prefers to defer adding database indexes on foreign keys until there's evidence they're needed, considering such additions premature optimization at the initial schema design stage.


const escaped = search.replace(/\\/g, "\\\\").replace(/'/g, "\\'");
return `resource.name.contains('${escaped}') || resource.identifier.contains('${escaped}')`;
}

export default function Resources() {
const { workspace } = useWorkspace();

const { search, setSearch, searchDebounced } = useSearch();
const { kind } = useKindFilter();
const { data: resources } = trpc.resource.list.useQuery(
{
workspaceId: workspace.id,
selector: `resource.name.contains('${searchDebounced}') || resource.identifier.contains('${searchDebounced}')`,
selector: buildResourceSelector(searchDebounced),
kind,
limit: 200,
offset: 0,
Expand Down
Loading