You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
layers: move common-svc's auth into pplx-space-rpc-auth - #1516
Moves common-svc's auth module, template_lookup and the auth_layer_e2e test into a new l40_api crate, pplx-space-rpc-auth (crates/l40_api/rpc/auth). This is the first of three PRs that delete pplx-space-common-svc. The code moves unchanged apart from paths. Dependents import rpc-auth and kube-client directly. Their k8s, test-hooks and test-support features now enable the owning crates' features (rpc-auth/*, api-route/test-support, ecw-credential/test-support). common-svc drops auth, template_lookup, kube_client, and its k8s, test-support and test-hooks features (nothing left in it reads a test hook). Every binary that installs an auth layer needing TokenReview (api, internal-api, node-api, build-manager) gets rpc-auth/k8s in every release and Kind build. credmgr takes rpc-auth without k8s, since nothing it uses is gated on it; node-ctl-server takes SpaceBearerInterceptor from bearer-token and needs no rpc-auth; both take kube-client directly. The isolated-crates job now lints rpc-auth alone (it is added to ALSO in ci/scripts/lib/rust-isolated-crates.sh), which covers its build without k8s.
Event targets change for 19 events without an explicit target: pplx_space_common_svc::auth[::k8s|::build|::build::k8s] → pplx_space_rpc_auth[::…] (k8s.rs 9, build/k8s.rs 5, lib.rs 3, build.rs 1), and pplx_space_common_svc::template_lookup → pplx_space_rpc_auth::template_lookup (1). Metrics, spans, fields and wire format are unchanged.
Validation
cargo check --workspace --all-targets, and each touched crate alone with --all-targets.
Feature builds: rpc-auth with k8s, test-support, test-hooks and all three; api, internal-api-server and node-ctl-server with kubernetes; node-api-server and build-mgr-server with k8s,test-hooks; internal-api-svc and node-api-svc with k8s.
The CI entrypoints on this head against current main: clippy and isolated-crates pass; docs and Python tests pass; Rust tests fail only 40 tests that need process permissions the validation host lacks and that fail the same way on main.
Workspace clippy with all targets and all features, and common-svc's two CI clippy runs.
nextest: rpc-auth 81/81 with all features, 23/23 with default features. Dependents 7303/7306, and 4554/4555 with the Kubernetes features.
Two failures also fail on main at 88f9f3e: api routes::e2b::tests::detail_reports_authoritative_federated_ownership_not_metadata (snapshot) and net-gateway …interception_exemptions_relay_original_tls_only_after_admission.
common-svc::logging_process failed once under load and passes when run alone.
Dependency graphs are identical to main except for rpc-auth and common-svc's dropped k8s.
cargo arch: no required findings and no baseline change.
Severity and risk
Severity: does not apply (refactor). Risk: a feature remap that stops enabling TokenReview auth in a binary. The graph comparison shows rpc-auth/k8s in every binary that installs an auth layer needing TokenReview. Roll back by reverting.
Rollout
A downstream mirror that enables the removed pplx-space-common-svc/k8s feature needs the same one-line change:
drop it. pplx-space-internal-api-server/kubernetes already enables common-svc's k8s before this change and
rpc-auth's k8s after it, so dropping the line first is safe on either side of this change.
Automatic approval is disabled by the configured approval policy for this pull request. Review comments and requests for changes continue normally.
Commit: 40c6a00b1f61.
No actionable comments were generated in the recent review.
Walkthrough
The change moves RPC authentication and template lookup code from common-svc into a dedicated crate. API, build, node, template, sandbox, and credential-manager services update their dependencies and imports. Kubernetes client usage moves to pplx-space-kube-client. Common-svc removes the extracted modules, features, and dependencies. CI paths and source-reference comments are updated.
Possibly related PRs
perplexityai/SPACE#1515: Moves authentication and template lookup metrics into common-svc, including modules relocated by this change.
Merge Risk: High · up to 40c6a
RPC privilege checks move to a new crate. The move warrants high-risk handling despite no identified authorization failure.
Security architecture risk: Low · up to 40c6a
The inspected authentication and publication paths preserve their authorization checks. No introduced security regression was established, but verification of release configurations and deployed monitoring remains incomplete.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The relevant security scope spans shared RPC identity decisions, privileged template publication, and artifact transfer, including private releases. Compromise of an authorized service credential or local service identity remains consequential, but the inspected changes do not demonstrate broader caller authority or tenant reach than the base.
Trust Boundaries and Controls
observed — Forwarded template export retains two distinct controls. Node API requires authenticated transfer authority and replaces caller-supplied grant metadata before forwarding. Template manager invokes its existing peer allowlist first, then requires the forwarded grant when the authorized principal is node-api. Root and permitted local operators remain separate privileged paths, as before this PR.
Resilience and Maintainability Implications
inferred — The extracted TokenReview cache preserves its security-relevant transitions: keys include token digest and audience, expired entries are rejected, same-key reviews coordinate through a claim, and dropping that claim wakes waiters on completion, error, or cancellation. Infrastructure failures are not negative-cached. Cleanup behavior follows guard ownership; concurrency and cancellation tests were not executed in this review.
Hardening Proposals
proposed — If deployed authentication alerts or log filters match tracing targets exactly, update them for the rpc-auth namespace. Implicit event targets change with the module path; external consumers were unavailable, so missed security telemetry is a possibility to verify, not an observed regression.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title identifies the auth migration and meets the required format.
Description Follows The Pr Template
The description includes a summary, validation commands and results, severity and risk, and an AI assistance line that identifies the tool and its contributions.
Bug Fixes Name A Regression Test
This PR is a refactor, not an application bug fix. The diff moves authentication code to pplx-space-rpc-auth and updates imports and feature mappings. The regression-test requirement does not apply.
Weakened Safeguards Are Justified
The moved tests retain their assertions and test registration. The Kind path changes follow the moved files. The ALSO entry adds isolated lint coverage, with a reason in the adjacent comment.
Public-Safe Content
The title, description, and diff contain no identified content that matches the check’s failure conditions. The token references in the diff are code identifiers, not credential values.
Recent review info
Automatic approval policy
Commit: 40c6a00b1f61.
Setting
Effective value
Source
Automatic approvals
Disabled
Override 4
Merge risk
Low
Override 2
Security risk
Not set
Global configuration
Matching overrides (numbered as in the resolved configuration):
Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 0 remain after this review.
Moves common-svc's `auth` module, `template_lookup` and the
`auth_layer_e2e` test into a new l40_api crate, pplx-space-rpc-auth
(crates/l40_api/rpc/auth), as part of deleting common-svc. The code
moves unchanged apart from paths: `crate::auth::` becomes `crate::`,
`pub(in crate::auth)` and the root module's `pub(super)` become
`pub(crate)`, and the common-svc re-exports it used are replaced by
their owning crates.
common-svc drops the module, its `k8s`, `test-support` and
`test-hooks` features and the `kube_client` re-export; nothing left in
it reads a test hook. Dependents now take:
- `pplx_space_rpc_auth` for the auth layers and `template_lookup`;
- `pplx-space-kube-client` directly (optional behind `kubernetes`
where kube is optional);
- `rpc-auth/k8s`, `rpc-auth/test-hooks` and `rpc-auth/test-support`
where they enabled the common-svc features for auth, and
`api-route/test-support` and `ecw-credential/test-support` where
their tests use those helpers.
node-ctl-server takes `SpaceBearerInterceptor` from
pplx-space-bearer-token, its owner, instead of through rpc-auth.
The isolated-crates job lints rpc-auth alone (its ALSO list), so its
build without `k8s`, which no release binary uses, is still linted.
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
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.
shehan-pplx
left a comment
Summary
Moves common-svc's
authmodule,template_lookupand theauth_layer_e2etest into a new l40_api crate,pplx-space-rpc-auth(crates/l40_api/rpc/auth). This is the first of three PRs that deletepplx-space-common-svc. The code moves unchanged apart from paths. Dependents import rpc-auth and kube-client directly. Theirk8s,test-hooksandtest-supportfeatures now enable the owning crates' features (rpc-auth/*,api-route/test-support,ecw-credential/test-support). common-svc dropsauth,template_lookup,kube_client, and itsk8s,test-supportandtest-hooksfeatures (nothing left in it reads a test hook). Every binary that installs an auth layer needing TokenReview (api, internal-api, node-api, build-manager) getsrpc-auth/k8sin every release and Kind build. credmgr takes rpc-auth withoutk8s, since nothing it uses is gated on it; node-ctl-server takesSpaceBearerInterceptorfrom bearer-token and needs no rpc-auth; both take kube-client directly. The isolated-crates job now lints rpc-auth alone (it is added toALSOinci/scripts/lib/rust-isolated-crates.sh), which covers its build withoutk8s.Event targets change for 19 events without an explicit target:
pplx_space_common_svc::auth[::k8s|::build|::build::k8s]→pplx_space_rpc_auth[::…](k8s.rs9,build/k8s.rs5,lib.rs3,build.rs1), andpplx_space_common_svc::template_lookup→pplx_space_rpc_auth::template_lookup(1). Metrics, spans, fields and wire format are unchanged.Validation
cargo check --workspace --all-targets, and each touched crate alone with--all-targets.k8s,test-support,test-hooksand all three; api, internal-api-server and node-ctl-server withkubernetes; node-api-server and build-mgr-server withk8s,test-hooks; internal-api-svc and node-api-svc withk8s.api routes::e2b::tests::detail_reports_authoritative_federated_ownership_not_metadata(snapshot) andnet-gateway …interception_exemptions_relay_original_tls_only_after_admission.common-svc::logging_processfailed once under load and passes when run alone.k8s.cargo arch: no required findings and no baseline change.Severity and risk
Severity: does not apply (refactor). Risk: a feature remap that stops enabling TokenReview auth in a binary. The graph comparison shows
rpc-auth/k8sin every binary that installs an auth layer needing TokenReview. Roll back by reverting.Rollout
A downstream mirror that enables the removed
pplx-space-common-svc/k8sfeature needs the same one-line change:drop it.
pplx-space-internal-api-server/kubernetesalready enables common-svc'sk8sbefore this change andrpc-auth's
k8safter it, so dropping the line first is safe on either side of this change.Stack
AI assistance: Claude Code made the code changes and drafted this description.