Skip to content

feat(guardrail): admin api#111

Merged
bzp2010 merged 1 commit into
mainfrom
bzp/feat-guardrail-admin-api
May 18, 2026
Merged

feat(guardrail): admin api#111
bzp2010 merged 1 commit into
mainfrom
bzp/feat-guardrail-admin-api

Conversation

@bzp2010
Copy link
Copy Markdown
Collaborator

@bzp2010 bzp2010 commented May 18, 2026

Summary by CodeRabbit

  • New Features

    • Added admin API endpoints for complete guardrails management (create, read, update, delete, list)
    • Guardrail deletion validates that it's not currently referenced by any policy
  • Tests

    • Added comprehensive test coverage for guardrails CRUD operations and validation scenarios

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

📝 Walkthrough

Walkthrough

This PR introduces complete CRUD admin endpoints for guardrails with HTTP handlers, OpenAPI documentation, config entity validation wiring, and comprehensive test coverage including schema validation and policy reference constraints.

Changes

Guardrail Admin CRUD Endpoints

Layer / File(s) Summary
Guardrail admin handlers and validation logic
src/admin/guardrails.rs
Five HTTP handlers implement list, get, post, put, and delete operations. Post generates UUIDs, put provides idempotent status codes, and delete checks for policy references. A shared update helper parses JSON, validates against schema, runs guardrail-specific validation, and persists with correct status codes (201 for new, 200 for updates).
Admin API module wiring
src/admin/mod.rs
Declares the guardrails submodule, registers the OpenAPI tag, documents CRUD routes in OpenAPI paths, and configures the Axum router to map /guardrails and /guardrails/{id} to handlers.
Config entity validation updates
src/config/entities/guardrails.rs
Renames the private validate function to validate_guardrail_definition with pub(crate) visibility to expose it for admin handlers. Updates GuardrailsStore::new to use the renamed validator callback.
Admin guardrail test coverage
tests/admin/guardrails.test.ts, tests/utils/admin.ts
Per-test isolated server setup with unique etcd prefixes. CRUD lifecycle, PUT idempotency (201 vs 200), schema validation error handling, and policy reference deletion prevention are all verified. Test utilities exports GUARDRAILS_URL constant.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • api7/aisix#103: Introduces the guardrails store and validation logic that this PR wires into the admin endpoints.
  • api7/aisix#107: Implements policy admin and validation that interoperates with guardrail references and deletion constraints.

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Security Check ❌ Error AWS credentials leaked in responses. Bedrock guardrails expose unredacted access_key_id, secret_access_key, session_token in all admin API responses. Redact secret fields in GuardrailConfig serialization. Use custom Serialize impl or response DTO. Follow bedrock.rs Debug impl pattern (lines 57-74) for redaction.
E2e Test Quality Review ⚠️ Warning Tests use real services but lack critical scenarios: no 404 tests, no auth edge cases for guardrails, missing invalid input/boundary tests. Implementation error handling correct. Add tests for: 404 responses, auth failures (invalid/missing tokens), invalid JSON/schema inputs, regex pattern validation. Guardrails endpoint needs explicit auth testing.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly describes the main change: adding admin API endpoints for guardrails management, which aligns with the changeset that introduces CRUD endpoints, validation, and comprehensive tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bzp/feat-guardrail-admin-api

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/config/entities/guardrails.rs (1)

60-60: ⚡ Quick win

Add tracing and docs to the newly exposed validator function.

Since validate_guardrail_definition is now pub(crate), please add #[fastrace::trace] and a /// doc comment above it.

As per coding guidelines, "Use #[fastrace::trace] decorator for distributed tracing on public functions" and "Use /// for doc comments on public items in Rust".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/config/entities/guardrails.rs` at line 60, Add a function-level doc
comment and the #[fastrace::trace] attribute to the newly exposed validator:
above pub(crate) fn validate_guardrail_definition(key: &str, value: &Guardrail)
-> Result<(), String> add a /// doc comment describing what the function does
(e.g., "Validate a guardrail definition key/value pair and return Err(String) on
validation failure") and place #[fastrace::trace] directly above the function
signature to enable distributed tracing for this now pub(crate) function.
src/admin/guardrails.rs (1)

24-24: ⚡ Quick win

Document public admin API items with /// comments.

OPENAPI_TAG and the public handlers are missing doc comments. Add concise /// docs to keep the public surface self-describing.

As per coding guidelines, "Use /// for doc comments on public items in Rust".

Also applies to: 36-36, 75-75, 106-106, 126-126, 145-145

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/admin/guardrails.rs` at line 24, The public constant OPENAPI_TAG and
several public handlers in this module lack triple-slash doc comments; add
concise /// docs above pub items (at least the pub const OPENAPI_TAG and every
pub fn/handler and other pub items flagged) that briefly describe their purpose
and behavior so the public API is self-describing; update the doc comments for
OPENAPI_TAG and each public handler function name found in this file (the public
handler functions and any other pub items at the commented locations) with
one-line descriptions and any short notes about expected inputs/outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/admin/guardrails.rs`:
- Around line 36-37: The public admin handler function `pub async fn
list(State(state): State<AppState>) -> Response` (and the other public handlers
noted in the review) must be annotated with the request-scoped tracing attribute
`#[fastrace::trace]`; add `#[fastrace::trace]` directly above each public
handler declaration (e.g., above `pub async fn list(...) -> Response`) so the
handlers are instrumented for distributed tracing and follow the project's
coding guidelines.

---

Nitpick comments:
In `@src/admin/guardrails.rs`:
- Line 24: The public constant OPENAPI_TAG and several public handlers in this
module lack triple-slash doc comments; add concise /// docs above pub items (at
least the pub const OPENAPI_TAG and every pub fn/handler and other pub items
flagged) that briefly describe their purpose and behavior so the public API is
self-describing; update the doc comments for OPENAPI_TAG and each public handler
function name found in this file (the public handler functions and any other pub
items at the commented locations) with one-line descriptions and any short notes
about expected inputs/outputs.

In `@src/config/entities/guardrails.rs`:
- Line 60: Add a function-level doc comment and the #[fastrace::trace] attribute
to the newly exposed validator: above pub(crate) fn
validate_guardrail_definition(key: &str, value: &Guardrail) -> Result<(),
String> add a /// doc comment describing what the function does (e.g., "Validate
a guardrail definition key/value pair and return Err(String) on validation
failure") and place #[fastrace::trace] directly above the function signature to
enable distributed tracing for this now pub(crate) function.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b9484ff5-8c06-48a1-acb3-96718c80d8b3

📥 Commits

Reviewing files that changed from the base of the PR and between d7fc71f and fef12a8.

📒 Files selected for processing (5)
  • src/admin/guardrails.rs
  • src/admin/mod.rs
  • src/config/entities/guardrails.rs
  • tests/admin/guardrails.test.ts
  • tests/utils/admin.ts

Comment thread src/admin/guardrails.rs
@bzp2010 bzp2010 merged commit bbcf85e into main May 18, 2026
3 checks passed
@bzp2010 bzp2010 deleted the bzp/feat-guardrail-admin-api branch May 18, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant