chore: add rule id to deploydecision and get rules oapi #707

adityachoudhari26 · 2025-10-29T22:56:19Z

Summary by CodeRabbit

Release Notes

New Features
- Added a new API endpoint to retrieve a specific policy rule by workspace, policy, and rule ID
- Rule evaluation results now include the associated rule identifier

coderabbitai · 2025-10-29T22:56:30Z

Walkthrough

The PR adds a new API endpoint to retrieve a specific policy rule by ID and introduces rule ID tracking in policy evaluation results. Changes span OpenAPI specifications, generated Go bindings, HTTP handlers, and policy evaluation factory to support this new functionality.

Changes

Cohort / File(s)	Summary
OpenAPI Schema Definitions `apps/workspace-engine/oapi/openapi.json`	Generated OpenAPI schema reflecting new RuleId field in RuleEvaluation and GET endpoint for fetching rules by ID.
OpenAPI Specification Helpers `apps/workspace-engine/oapi/spec/lib/openapi.libsonnet`	Added `ruleIdParam()` helper method to generate path parameter definitions for rule ID.
API Endpoint Specification `apps/workspace-engine/oapi/spec/paths/policy.jsonnet`	Defined new GET endpoint at `/v1/workspaces/{workspaceId}/policies/{policyId}/rules/{ruleId}` with operationId `getRule`, supporting parameters, and response schemas.
API Schema Definitions `apps/workspace-engine/oapi/spec/schemas/policy.jsonnet`	Added `ruleId` property to RuleEvaluation schema and included it in required fields array.
Generated Go Bindings `apps/workspace-engine/pkg/oapi/oapi.gen.go`	Auto-generated code adding RuleId field to RuleEvaluation struct, GetRule method to ServerInterface, and associated handler middleware.
Rule Evaluation Builder `apps/workspace-engine/pkg/oapi/evaluation.go`	Added RuleId field initialization and `WithRuleId()` fluent setter method for RuleEvaluation builder pattern.
HTTP Handler Implementation `apps/workspace-engine/pkg/server/openapi/policies/policies.go`	Implemented GetRule handler to parse path parameters, validate workspace/policy existence, find matching rule by ID, and return 200/404 responses.
Policy Evaluation Factory `apps/workspace-engine/pkg/workspace/releasemanager/policy/factory.go`	Modified evaluation result collection across all rule scopes to attach originating rule ID via `WithRuleId()` before appending results.

Sequence Diagram

sequenceDiagram
    participant Client as Client
    participant Handler as GetRule Handler
    participant Workspace as Workspace Store
    participant Policy as Policy Lookup
    participant Rules as Rules Iteration

    Client->>Handler: GET /v1/workspaces/{id}/policies/{id}/rules/{id}
    Handler->>Workspace: Fetch workspace by ID
    alt Workspace not found
        Handler->>Client: 500 Internal Server Error
    else Workspace found
        Handler->>Policy: Find policy in workspace
        alt Policy not found
            Handler->>Client: 404 Not Found (policy)
        else Policy found
            Handler->>Rules: Iterate policy.Rules
            alt Matching rule found by ID
                rect rgb(200, 220, 240)
                note over Rules: Rule matches ruleId parameter
                end
                Handler->>Client: 200 OK + PolicyRule object
            else No matching rule
                Handler->>Client: 404 Not Found (rule)
            end
        end
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Areas requiring attention:
- Verify that WithRuleId() is correctly applied in factory.go across all evaluation result collection paths (environment/version/target, version, target, release, workspace scoped)
- Confirm that the handler in policies.go correctly validates workspace and policy before iterating rules
- Check that generated OpenAPI bindings in oapi.gen.go align with specification definitions in policy.jsonnet

Possibly related PRs

chore: implement version any approval rule #657: Also tracks rule IDs on rule evaluation results (VersionAnyApprovalRule implementation), complementing this PR's API exposure and evaluation result enrichment.

Poem

🐰 A new endpoint hops into the warren, /
To fetch rules by their ID with precision, /
No more searching the whole burrow through, /
RuleId in results, now clear and true, /
The factory marks each evaluation bright! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "add rule id to deploydecision and get rules oapi" directly corresponds to the main changes in the changeset. The PR adds a new `ruleId` field to the `RuleEvaluation` object (representing the deployment decision evaluation) and introduces a new GET endpoint to retrieve specific policy rules by ID. The title is specific and concise, clearly identifying both the schema enhancement and the new API endpoint, making it understandable to someone reviewing commit history. While some terminology like "deploydecision" and "oapi" could be slightly more explicit, the title conveys the essential nature of the changes without ambiguity.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch add-rule-id

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2025-10-29T22:58:19Z

📊 DB Package Test Coverage

pkg/db coverage: 57.0%

View detailed coverage report in artifacts

github-actions · 2025-10-29T22:59:15Z

📊 Code Coverage Report

workspace-engine coverage: 43.7%

View detailed coverage report in artifacts

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

apps/workspace-engine/oapi/openapi.json (1)

3144-3211: New endpoint getRule looks consistent.

Parameters and responses mirror existing patterns. Consider adding a tag for grouping (e.g., "Policies") for better client SDK grouping; optional.
apps/workspace-engine/oapi/spec/paths/policy.jsonnet (1)
69-83: Path addition LGTM; add a tag for grouping (optional).

Spec reads clean and consistent. If you want SDK grouping, add a "Policies" tag.

Apply this minimal tweak if desired:
   '/v1/workspaces/{workspaceId}/policies/{policyId}/rules/{ruleId}': {
     get: {
-      summary: 'Get rule',
+      summary: 'Get rule',
+      tags: ['Policies'],
       operationId: 'getRule',
Also, confirm openapi.ruleIdParam() exists in ../lib/openapi.libsonnet.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0dd36e0 and be27bb6.

📒 Files selected for processing (8)

apps/workspace-engine/oapi/openapi.json (2 hunks)
apps/workspace-engine/oapi/spec/lib/openapi.libsonnet (1 hunks)
apps/workspace-engine/oapi/spec/paths/policy.jsonnet (1 hunks)
apps/workspace-engine/oapi/spec/schemas/policy.jsonnet (1 hunks)
apps/workspace-engine/pkg/oapi/evaluation.go (2 hunks)
apps/workspace-engine/pkg/oapi/oapi.gen.go (4 hunks)
apps/workspace-engine/pkg/server/openapi/policies/policies.go (2 hunks)
apps/workspace-engine/pkg/workspace/releasemanager/policy/factory.go (6 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

apps/workspace-engine/**/*.go

📄 CodeRabbit inference engine (apps/workspace-engine/CLAUDE.md)

apps/workspace-engine/**/*.go: Do not add extraneous inline comments that state the obvious
Do not add comments that simply restate what the code does
Do not add comments for standard Go patterns (e.g., noting WaitGroup or semaphore usage)
Write comments that explain why, document complex logic/algorithms, provide non-obvious context, include TODO/FIXME, and document exported functions/types/methods

Files:

apps/workspace-engine/pkg/oapi/evaluation.go
apps/workspace-engine/pkg/oapi/oapi.gen.go
apps/workspace-engine/pkg/workspace/releasemanager/policy/factory.go
apps/workspace-engine/pkg/server/openapi/policies/policies.go

**/*.{js,jsx,ts,tsx,json,md,yml,yaml}

📄 CodeRabbit inference engine (CLAUDE.md)

Formatting: Prettier is used with @ctrlplane/prettier-config

Files:

apps/workspace-engine/oapi/openapi.json

🧠 Learnings (8)

📓 Common learnings

Learnt from: adityachoudhari26
PR: ctrlplanedev/ctrlplane#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
PR: ctrlplanedev/ctrlplane#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.