refactor(builtins): consolidate bundled extensions into one published crate - #140
Conversation
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Add crates/builtins (praxis-policy-builtins) with the merged dependency table, one feature per bundled extension, and the module groups the implementations will move into. Nothing has moved yet; the crate is publish = false until it holds real code, so a release tag cut mid-move cannot upload an empty package. Restore the gates the new crate would otherwise fall out of. It is default = [], so a default-feature run compiles none of its modules: make lint and make doc gain an --all-features pass, msrv and release packaging likewise, and a new check-features target compiles the crate with no features and with each one alone. That last case is the only one that catches a feature body missing its own dep: edge, since --all-features masks it and the default build compiles nothing. It runs as its own CI job because the workflows invoke targets directly rather than make ci. Repair the publish order, which has been failing its own publishable-workspace guard since praxis-policy-plugin-identity-api-key was added without an ORDER entry. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Relocate praxis-policy-plugin-identity-jwt to praxis-policy-builtins::plugins::identity_jwt behind the jwt feature, and retire its crate. Its four test roots become submodules of one linked harness, gated on the feature so they do not link in a build that cannot run them. Reshape the facade's registration seam, which does not survive the path change on its own: register_builtins! matches a single identifier and uses it for both KIND and the factory type, so a four-segment module path cannot be substituted and $krate:path is not allowed before `::`. Each feature now aliases its module and the matcher stays as it was, which keeps registration keyed off the extension's own KIND const. Operator-facing messages and the assertion that checks them now name praxis-policy-builtins; the old crate name would point at something future releases do not publish. Paths inside the module are absolute rather than super-relative so they resolve identically from nested test modules, and RefreshGate's internals move to pub(super) to keep the boundary the retired crate had. 185 tests, name-for-name unchanged. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Relocate praxis-policy-plugin-identity-api-key to praxis-policy-builtins::plugins::identity_api_key behind the api-key feature, and retire its crate. Its directory harness carries over as the declared api_key test target, so the layout is unchanged. All three kind strings are preserved: identity/api-key for the plugin, and file and http for the directory backends it selects between. 81 tests, name-for-name unchanged. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Relocate praxis-policy-plugin-delegator-oauth and
praxis-policy-plugin-elicitation-ciba to
praxis-policy-builtins::plugins::{delegator_oauth,elicitation_ciba} behind
the oauth and elicitation-ciba features, and retire both crates. They move
together because they share the base64 and zeroize declarations, so the
merged entries are verified once rather than half-checked twice.
Both keep their module-internal boundary: pub(crate) becomes
pub(in crate::plugins::<module>), which is what the retired crate scope
meant. pub(super) would have been too narrow for the cache items a sibling
of cache consumes.
Drop the outer doc comments from the module declarations. A /// on the
declaration merges with the module file's own //! block and makes its
intra-doc links resolve in the parent's scope, so every module-relative
link in the moved docs failed to resolve. The crate-level feature table
keeps plain code spans for the same reason inverted: default = [] compiles
no module, and make doc runs a default-features pass.
154 tests, name-for-name unchanged.
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Relocate the three PDP crates to praxis-policy-builtins::pdps::* behind the cedar, cel and opa features, and retire them. Cedar's six test roots plus CEL's and OPA's one each become submodules of three feature-gated harnesses. praxis-policy-pdp-diff and ppe-benches are rewired in this same commit, not a later one: both held non-optional workspace dependencies on the retired crates, so removing those entries on their own leaves `cargo metadata` unable to resolve the workspace, which takes every cargo command down rather than just those two crates. Cedar's fixture reader builds its path from CARGO_MANIFEST_DIR, which now points at the consolidated crate, so it reads tests/cedar/fixtures. regorus keeps its curated feature set with http, net, opa-runtime and jsonschema still excluded, and a cedar-only build still resolves without praxis-policy-apl-runtime on its normal edges. 180 tests, name-for-name unchanged. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Relocate praxis-policy-secrets-vault to praxis-policy-builtins::secrets::vault behind the secrets-vault feature, and retire its crate. Its modules stay private and only the four items the facade re-exports remain reachable. The feature keeps its distinctive shape: it is the one builtin feature that does not imply _builtin, because a Vault provider needs an HttpTransport the host supplies and install_builtins cannot register it unattended. A facade built with secrets-vault alone still exposes no install_builtins. pub(crate) becomes pub(in crate::secrets::vault). This module handles Vault tokens and resolved secret material, so its boundary is kept as the retired crate had it rather than widened to every other extension. 40 tests, name-for-name unchanged. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Relocate praxis-policy-session-valkey to praxis-policy-builtins::session::valkey behind the valkey feature, and remove the last of the top-level builtins/ tree. All nine bundled extensions now live in one crate. Valkey is the only extension needing praxis-policy-apl-runtime, and it is the only feature that pulls it on a normal edge; a build with no features resolves no redis, deadpool-redis or TLS crate at all. That is what now holds the Redis stack out of an everyday build, so the default-members note explaining the old membership exclusion is replaced rather than dropped: the mechanism changed from membership to features. Integration cases stay ignored by default and still honour VALKEY_TEST_URL, the testcontainers fallback and VALKEY_TESTS_OPTIONAL. Measured on this machine, touching one extension source file costs about 1s incremental under --all-features and nothing under default features, against 3s for an engine crate, so consolidating nine compilation units into one does not show up as a meaningful incremental cost. 21 tests, name-for-name unchanged. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Engine-crate comments and rustdoc referred to the nine bundled extensions by package name. Those packages no longer exist, so each reference now names the extension itself, and the runtime's registration example points at the consolidated path. Cross-references inside the consolidated crate get the same treatment: the OPA module described itself against praxis-policy-pdp-cel and praxis-policy-pdp-cedar-direct, which are sibling modules now, not other crates. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Make praxis-policy-builtins publishable now that the implementations live in it, and collapse the publish order to seven entries with the crate placed after praxis-policy-apl-runtime, its deepest optional upstream. Add the order assertions the existing guard cannot make. That guard compares sorted sets, so it proves membership and says nothing about sequence — and sequence is what a serial publish depends on, since a crate uploaded before something it requires fails mid-run with its version already consumed. Verified by deliberately breaking each case. Run the packaging check and the publish dry run in ordinary CI. Neither was exercised outside a tagged release, which is how the order drifted for a whole release cycle unnoticed, and neither uploads anything. Gate the tag on facade API compatibility with all features enabled. A default-feature run would inspect none of the re-exports it exists to protect, because the facade is default = []. It is a floor rather than a proof: semver-checks coverage of cross-crate re-exports is incomplete, and that is the shape this change alters. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Rewrite the bundled-extensions tables around praxis-policy-builtins and its nine features, and add the migration mapping. The guidance says to replace an old dependency rather than add alongside it: keeping both links two copies of the same implementation registering the same kind, and the registry is last-write-wins, so the stale copy can win silently instead of failing. Both tables were already incomplete before this change, omitting api-key and secrets-vault, so they are corrected rather than reproduced. The claim mapping guide's links pointed at a claim_map_config.rs path that stopped existing when the mapper moved to praxis-policy-core; they now point where the file actually lives. deny.toml's advisory ignores stay dev-only and their reachability is unchanged, but their scoping prose named crates that are gone and a testcontainers edge whose blast radius widened. Both copies of safety-invariants.md are edited so they stay byte-identical; merging them is separate work. docs/dev/port-provenance.md keeps its builtins/ path: it records an import as it happened. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Promoting each retired crate's header block to module rustdoc carried a stale scope note in the OAuth delegator into rendered documentation, where it claimed the HTTP exchange logic and integration tests were still to come. It was an invisible plain comment before and it was already false; the module has full exchange coverage. Five other comments said "this crate" about behaviour belonging to one extension. Read against the consolidated crate they assert something about all nine, and one was outright wrong: the OPA input builder said the crate stays apl-core-only at compile time, which is now a property of the feature and its normal edges, not of the crate. The remaining "this crate" references are accurate as written, since the kind strings and the direct-dependency guidance really do describe praxis-policy-builtins. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Comments and rustdoc across the engine crates cited unit and requirement ids from the plans that produced them. Those ids mean nothing to a reader of the code and the surrounding sentences carry the same information without them, so each one is reworded rather than annotated: three test section banners, one rustdoc line on an engine test, a claim-root comment that now names the rejection the code returns, and a cap assertion. The Cedar entity builder's TODO goes the same way. It recorded a task and pointed at a tracking note, which is state rather than explanation; what it actually explained is kept, namely that the CMF bag-key prefixes are written literally because naming them by symbol would need a dependency the cedar feature deliberately does not take. Product requirement labels in docs/content and the negative example in CONTRIBUTING are left alone: the first are a published document's own vocabulary, and the second documents this rule. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
terylt
left a comment
There was a problem hiding this comment.
Nice work! Overall looks good.. below are some minor comments:
1. make semver-facade only runs at tag time
The release-readiness job exists so that packaging and publish order break in CI rather than at release. semver-facade is the check most likely to surprise, since the facade's re-exports all changed paths, but it only runs in release.yaml. Could it go in release-readiness too, or has it been run against 0.3.1 already?
2. Config errors name the crate, not the extension
plugin 'x' (praxis-policy-builtins) config parse failed no longer says which extension failed. The changelog already flags the text change, so it seems worth making the new text useful: the kind (identity/jwt, delegator/oauth, elicitation/ciba) identifies the extension and stays stable if the crate layout changes again.
3. The all feature has no users
Nothing in the workspace enables praxis-policy-builtins/all. ppe-pdp-diff and ppe-benches enable cedar, cel and opa, and docs.rs uses all-features = true, so the comment naming them as consumers is not accurate. Once published it is public API, so I'd drop it unless there is a host that wants it.
Nits
crates/ppe/Cargo.toml, thehttp-hypercomment: "unlike every other feature above" is no longer true, "a 17th crate" is stale, and "runs long minutes" lost its number. "One feature per builtin crate" above it is also stale.crates/ppe-apl-cmf/src/security.rs:16still points atbuiltins/pdps/cedar-direct/src/entities.rs.- The Makefile and
Cargo.tomlcomments say a default-feature build compiles none of the extensions. In a workspace build,ppe-pdp-diffturns oncedar,celandopathrough feature unification, so those three do compile. The all-features passes are still needed for the rest.
Follow-up, not for this PR
The migration note warns that keeping an old crate alongside the new one registers the same kind twice, and the registry is last-write-wins. Refusing a duplicate kind at registration would turn that silent failure into a loud one.
Name the plugin kind in config-load errors instead of the crate. All nine extensions ship in one crate now, so naming it identified nothing; the kind is what an operator writes in config and it survives a further layout change. Drop the `all` feature. Nothing enables it — the differential harness and benches ask for cedar, cel and opa, and docs.rs uses all-features — and the comment claiming them as consumers was wrong. It would have been permanent public API after the first publish. Run the facade compatibility check on pull requests as well as at tag time. It is the check most likely to surprise, since every re-export path moved. It passes against the published 0.3.1: 196 checks, no update required. Correct four comments claiming a default-feature build compiles no extension. In a workspace build the differential harness turns on cedar, cel and opa through feature unification, so the all-features passes are what cover the other six rather than all nine. The two packaging comments are left: verification builds each crate from its own tarball, where unification does not reach. Also fix a stale builtins/ path in the CMF security note, which the earlier sweep missed by grepping crate names rather than paths, and the http-hyper rationale, which still counted crates that no longer exist. Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
|
All addressed.
Nits: Agreed on refusing a duplicate kind at registration, we should open an issue for it. |
Move the standalone praxis-policy-plugin-quota crate into praxis-policy-builtins as plugins::quota, matching the sibling plugins, and wire it through the builtins feature and registration table rather than a separate published crate (review on praxis-proxy#116, after praxis-proxy#140 consolidated the bundled extensions into one crate). The plugin adds no new dependency to the builtins crate: its deps (core, bytes, serde, tracing) are already present, so it pulls in no crypto. Signed-off-by: Sam Batschelet <sbatsche@redhat.com>
Move the standalone praxis-policy-plugin-quota crate into praxis-policy-builtins as plugins::quota, matching the sibling plugins, and wire it through the builtins feature and registration table rather than a separate published crate (review on praxis-proxy#116, after praxis-proxy#140 consolidated the bundled extensions into one crate). The plugin adds no new dependency to the builtins crate: its deps (core, bytes, serde, tracing) are already present, so it pulls in no crypto. Signed-off-by: Sam Batschelet <sbatsche@redhat.com>
Description
Coalesce the builtin crates into one pubblished crate,
praxis-policy-builtins, with a Cargo feature per extension.Closes #137
Notes
install_builtins, its re-exports and all policykindstrings are unchanged, so hosts using the facade need no change. Publishable packages drop from 15 to 7.identity-api-keywas added without an ORDER entry, and adds order assertions it could not make.