Skip to content

chore(deps): load sspi fork from git instead of vendoring third_party#338

Merged
robot-head merged 2 commits into
mainfrom
chore/sspi-fork-git-dep
May 30, 2026
Merged

chore(deps): load sspi fork from git instead of vendoring third_party#338
robot-head merged 2 commits into
mainfrom
chore/sspi-fork-git-dep

Conversation

@robot-head

Copy link
Copy Markdown
Owner

Summary

Replaces the vendored cargo package snapshot under third_party/sspi with a git dependency on our sspi fork branch, wired through [patch.crates-io] in the workspace Cargo.toml.

The fork is branched off the upstream sspi-v0.21.0 tag and carries our SASL/GSSAPI Kerberos work:

  • Keytab client credentials — authenticate as a Kerberos client from a pre-derived long-term key, no password (Credentials::Keytab).
  • Integrity-only Wrap tokens — accept GSS_Wrap tokens with conf_req_flag == FALSE (RFC 4121 §4.2.4), used by stock SASL/GSSAPI clients for the RFC 4752 security-layer negotiation.
  • Multi-SPN acceptor keys — validate an AP-REQ against any of several configured service principals from one keytab.

Upstreaming is tracked at Devolutions/sspi-rs#681.

Why

The vendored tree was a flattened cargo package snapshot (~37k lines, normalized Cargo.toml, packaging artifacts) that was awkward to maintain and rebase against upstream. A git fork branch is rebaseable onto future upstream releases and is the same source we're proposing upstream.

Changes

  • Cargo.toml[patch.crates-io] switched from path = "third_party/sspi" to git = "…/sspi-rs.git", branch = "crabka/sasl-gssapi".
  • third_party/sspi/ — removed (143 files).
  • crates/security/Cargo.toml, crates/security/src/gssapi/provider.rs — stale comments updated to reference the fork / PR.
  • Cargo.locksspi re-resolved to the git source, pinned to commit 0840ccd5.

Notes

  • Cargo.lock pins the exact fork commit, so builds stay reproducible even though the ref is a branch. After rebasing the fork onto a new upstream release, run cargo update sspi to re-pin.
  • cargo check -p crabka-security passes against the git source.

🤖 Generated with Claude Code

Replace the vendored `cargo package` snapshot under third_party/sspi with
a git dependency on our sspi fork branch, wired through `[patch.crates-io]`.
The fork (branched off the sspi-v0.21.0 tag) carries our SASL/GSSAPI
Kerberos work: keytab client credentials, integrity-only Wrap tokens, and
multi-SPN acceptor keys. Upstreaming tracked at Devolutions/sspi-rs#681.

Cargo.lock pins the exact fork commit, so builds stay reproducible despite
the ref being a branch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented May 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.24%. Comparing base (df73108) to head (fab6177).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #338       +/-   ##
===========================================
+ Coverage   76.35%   92.24%   +15.88%     
===========================================
  Files        1213     1215        +2     
  Lines      149948   159361     +9413     
===========================================
+ Hits       114497   146995    +32498     
+ Misses      35451    12366    -23085     
Flag Coverage Δ
broker-integration 49.70% <ø> (+0.21%) ⬆️
broker-jvm-acceptance 50.45% <ø> (+0.72%) ⬆️
client-core-integration 36.47% <ø> (ø)
jvm-differential 92.21% <ø> (+23.56%) ⬆️
log-integration 22.73% <ø> (ø)
unit 82.11% <ø> (+23.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@robot-head robot-head enabled auto-merge (squash) May 29, 2026 23:30
@codspeed-hq

codspeed-hq Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 267 untouched benchmarks
⏩ 16 skipped benchmarks1


Comparing chore/sspi-fork-git-dep (fab6177) with main (3e4fde5)2

Open in CodSpeed

Footnotes

  1. 16 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (2ebd9ea) during the generation of this report, so 3e4fde5 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

cargo-deny's [sources] denies unknown git sources; allow the sspi fork
URL now that sspi is pulled from git rather than vendored.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@robot-head robot-head disabled auto-merge May 30, 2026 00:12
@robot-head robot-head merged commit 1753ec4 into main May 30, 2026
30 of 32 checks passed
@robot-head robot-head deleted the chore/sspi-fork-git-dep branch May 30, 2026 00:12
@robot-head

Copy link
Copy Markdown
Owner Author

CI note: the red client-consumer-integration check is a pre-existing, unrelated flake

The only CI failure actually caused by this PR was cargo-deny (the new sspi git source needed allowlisting in deny.toml) — fixed in fab6177 and now green.

The remaining red is eager_rebalance_reacquires_and_primes (crates/client-consumer/tests/integration.rs:367, "m1 did not re-acquire both partitions"), a timing-sensitive consumer-rebalance test:

  • Not reachable from this changecrabka-client-consumer has no sspi dependency; the only transitive delta from the fork swap is md4 (sspi-internal NTLM), which never compiles into the consumer.
  • Passes 3/3 locally; only flakes under loaded, llvm-cov-instrumented CI runners that overrun the test's tight rebalance window.
  • Pre-existing on main — e.g. run 26667890038 (commit 3e4fde5c, on main) failed the same job with no relation to this PR.

Merging as-is; the flake is tracked separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant