-
Notifications
You must be signed in to change notification settings - Fork 6
Add validate_policies function for schema validation #34
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.
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
Conversation
Add validate_policies(policies, schema) function that validates Cedar policies against a schema, exposing the cedar-policy Validator API. Changes: - Add validate_policies function in Rust (src/lib.rs) - Add ValidationResult and ValidationError Python classes - Add validate_policies Python wrapper function - Add 20 unit tests including Rust CLI parity tests - Add schema validation to integration tests (shouldValidate field) - Update README with validation documentation The function returns a ValidationResult with validation_passed boolean and a list of ValidationError objects. Supports both JSON and Cedar schema syntax formats.
|
Thanks for reviewing this and the consideration to add. I my CI pipeline I have to do a lot of work to validate my policies and introduce new dependacies. - name: Install Rust toolchain
uses: dtolnay/rust-toolchain@stable
- name: Cache Cedar CLI
uses: actions/cache@v4
with:
path: ~/.cargo/bin/cedar
key: cedar-cli-${{ env.CEDAR_CLI_VERSION }}-${{ runner.os }}
- name: Install Cedar CLI
run: |
if ! command -v cedar &> /dev/null; then
cargo install cedar-policy-cli --version "${{ env.CEDAR_CLI_VERSION }}"
else
echo "Cedar CLI already installed (from cache)"
fi
- name: Validate Cedar policies
run: |
# Generate schema and validate policies (run from source directory)
cd source && uv run python -m nc_auth_entities > /tmp/emu-schema.cedarschema
cedar validate --schema /tmp/emu-schema.cedarschema --policies policies.cedarIf we get |
|
Thank you for the PR @Iamrodos! I am at re:Invent now. I skimmed the code and the approach looks good. I will review it closely soon. |
|
@skuenzli Must say I am glad to not go to re:Invent anymore. I think I went for 9 or 10 years in a row. I think my first year was 2012. Have fun! |
| /// List of validation errors | ||
| errors: Vec<ValidationErrorSer>, | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| self.assertTrue(validation_result.validation_passed, | ||
| f"Expected policies to validate but got errors: " | ||
| f"{[str(e) for e in validation_result.errors]}") | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
|
||
| serde_json::to_string(&result).unwrap() | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| self.assertTrue( | ||
| any("BadType" in msg for msg in error_messages), | ||
| f"Expected error about 'BadType', got: {error_messages}" | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| unittest.main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌
|
This looks great! I love the extensive test cases. Thank you @Iamrodos |
This PR adds a
validate_policies(policies, schema)function that validates Cedar policies against a schema, exposing the cedar-policyValidatorAPI to Python users.Motivation
Currently, validating Cedar policies against a schema requires shelling out to the Cedar CLI (
cedar validate). This is inconvenient and adds an external dependency. By exposing the Validator API directly, users can:See the Cedar validation documentation for details on what the validator checks.
Usage
Design Decisions
The
validate_policiesfunction returns aValidationResultfor all error types (including parse errors) rather than raisingValueError. This differs from functions likeformat_policieswhich raise on invalid input.Rationale: The purpose of a validator is to check validity. A validator that throws exceptions for invalid input wouldn't be useful as a validator. Users expect to call
validate_policies()and check the result, not wrap it in try/except.Testing Approach
Three layers of testing ensure correctness:
New unit tests (
tests/unit/test_validate.py) - 20 tests covering the API, error handling, schema formats, and edge cases.Rust CLI parity tests - 8 of the unit tests use the same policy and schema files as the Rust CLI tests in
third_party/cedar/cedar-policy-cli/sample-data/. This ensures our validation produces the same results ascedar validate.Integration test validation - The existing integration tests now call
validate_policieswhen the test definition specifiesshouldValidate: true. This adds validation coverage to 22 additional test scenarios from the official Cedar integration test suite.Implementation assisted by AI tooling; design decisions, testing strategy, and code review performed by the author.