feat: improve APL ergonomics#71
Merged
Merged
Conversation
`parse_rule`'s no-colon branch only recognized bare `deny` / `allow` via
`try_bare_action`; the `deny('reason')` / `deny('reason', 'code')` call
form was reachable only as the action half of a conditional rule
(`predicate: deny('reason')`). That made it impossible to attach a
reason/code to an unconditional reaction — notably `on_deny:` /
`on_allow:` lists on PDP steps, which parse each item through
`parse_step -> parse_step_string -> parse_rule`.
Fall through to `try_parse_deny_call` after `try_bare_action` so an
unconditional `deny('reason')` / `deny('reason', 'code')` parses to an
`Always`-guarded `Deny`. A malformed `deny(...)` surfaces its own error
rather than being misread as a predicate downstream.
Add tests covering the reason-only and reason+code forms and the
malformed-call error path.
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
`run(name)` now invokes a named plugin everywhere `plugin(name)` does, so policies can read `- run(audit-log)` instead of `- plugin(audit-log)`: - policy-step / effect form (`parse_step_string`) — covers top-level `policy:` items and `do:` lists, and `detect_step_kind` recognizes `run(` so misrouted strings still get a clear step-kind error; - field-pipeline stage form (`args.x: "str | run(name)"`) — mirrors the step alias for symmetry. `plugin` remains the long form; both parse to the same `Step::Plugin` / `Stage::Plugin`. Errors name whichever verb the author used. Add tests for the step alias (string + compile paths), the field-stage alias, and the empty/malformed error path. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
APL config blocks previously had to be nested under an `apl:` key at every level (`route -> apl -> policy`, `global -> apl -> pdp`, ...). The visitor's `apl_subblock` now accepts APL terms written directly on the section when the wrapper is omitted (`route -> policy`), while the explicit `apl:` form still takes precedence. When `apl:` is absent, a synthetic block is assembled from the recognized APL keys present on the container — `policy`, `post_policy`, `args`, `result`, `pdp` — so structural keys (tool / identity / defaults / ...) are never misread. `plugins` is shape-gated: a map (the apl-override form) is included, a list (structural plugin-refs on RouteEntry / PolicyGroup) is left alone, avoiding a key-shape clash. Empty sections still return None (the "no contribution, skip" path). Applies uniformly to the global / defaults / policy-bundle / route visit sites. Add unit tests for the shape rules (wrapper precedence, flat keys, plugins map-vs-list, null/empty) and end-to-end tests for a wrapper-less route on both the allow and deny paths. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Three follow-up fixes to the optional-`apl:`-wrapper work:
1. Flat `plugins:` map broke config loading at route / defaults /
policy scope. The whole YAML deserializes into `CpexConfig` before
any visitor runs, and `plugins` is a `Vec<PluginRouteRef>` there — so
a wrapper-less override *map* (`plugins: { audit: { on_error: ... } }`)
failed the structural parse with "invalid type: map, expected a
sequence", reachable only under top-level `global:`. Give
`RouteEntry.plugins` and `PolicyGroup.plugins` a `deserialize_with`
that accepts either shape: a sequence stays the structural activation
list; a map deserializes to an empty `Vec` (it is APL-override data,
consumed by the visitor from the raw YAML). This mirrors the
`apl: { plugins: {...} }` wrapper form exactly — the map never
populated the structural list there either — so the flat and wrapped
forms are behaviorally identical. A scalar `plugins:` now yields a
shape-aware error instead of a cryptic serde message.
2. Flat `pdp:` was silently dropped at non-global scopes. Only
`visit_global` builds PDPs; a `pdp:` under a default / policy-bundle
/ route block was folded into the policy body and discarded with no
signal. Emit a `tracing::warn!` at those scopes (flat or wrapped) so
the footgun is visible.
3. `plugin()` / `run()` with an empty name was accepted in field
pipelines (`Stage::Plugin { name: "" }`), while the policy-step path
already rejected it. Add the same empty-name guard to `parse_stage`
so both paths fail with the same verb-named diagnostic.
Tests: unit coverage for the map-tolerant deserializer (list, map,
defaults/policies, scalar), the empty-name guard, and the pdp-warn
helper; end-to-end tests that drive a flat `plugins:` map through the
real `load_config_yaml` path — policy+map coexist and deny, defaults
inheritance, and flat-vs-wrapped equivalence — closing the gap the
isolated `apl_subblock` tests left open.
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Teryl Taylor <terylt@ibm.com>
terylt
approved these changes
Jun 18, 2026
terylt
left a comment
terylt
left a comment
Contributor
There was a problem hiding this comment.
LGTM. I made some small changes and you addressed the issues from my review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improves APL ergonomics while maintaining backwards compatibility with original syntax.
Changes
make the
apl:wrapper optionalAPL config blocks previously had to be nested under an
apl:key at every level (route -> apl -> policy,global -> apl -> pdp, ...). The visitor'sapl_subblocknow accepts APL terms written directly on the section when the wrapper is omitted (route -> policy), while the explicitapl:form still takes precedence.When
apl:is absent, a synthetic block is assembled from the recognized APL keys present on the container —policy,post_policy,args,result,pdp— so structural keys (tool / identity / defaults / ...) are never misread.pluginsis shape-gated: a map (the apl-override form) is included, a list (structural plugin-refs on RouteEntry / PolicyGroup) is left alone, avoiding a key-shape clash. Empty sections still return None (the "no contribution, skip" path).Applies uniformly to the global / defaults / policy-bundle / route visit sites. Add unit tests for the shape rules (wrapper precedence, flat keys, plugins map-vs-list, null/empty) and end-to-end tests for a wrapper-less route on both the allow and deny paths.
accept
runas an alias for pluginrun(name)now invokes a named plugin everywhereplugin(name)does, so policies can read- run(audit-log)instead of- plugin(audit-log):parse_step_string) — covers top-levelpolicy:items anddo:lists, anddetect_step_kindrecognizesrun(so misrouted strings still get a clear step-kind error;args.x: "str | run(name)") — mirrors the step alias for symmetry.pluginremains the long form; both parse to the sameStep::Plugin/Stage::Plugin. Errors name whichever verb the author used.Add tests for the step alias (string + compile paths), the field-stage alias, and the empty/malformed error path.
accept unconditional
deny('reason')as a bare actionparse_rule's no-colon branch only recognized baredeny/allowviatry_bare_action; thedeny('reason')/deny('reason', 'code')call form was reachable only as the action half of a conditional rule (predicate: deny('reason')). That made it impossible to attach a reason/code to an unconditional reaction — notablyon_deny:/on_allow:lists on PDP steps, which parse each item throughparse_step -> parse_step_string -> parse_rule.Fall through to
try_parse_deny_callaftertry_bare_actionso an unconditionaldeny('reason')/deny('reason', 'code')parses to anAlways-guardedDeny. A malformeddeny(...)surfaces its own error rather than being misread as a predicate downstream.Add tests covering the reason-only and reason+code forms and the malformed-call error path.