Skip to content

Record FedAuth token deterministically in mock server - #89

Closed
saurabh500 wants to merge 10 commits into
mainfrom
saurabh500-saurabh500-mock-tds-deterministic-token
Closed

Record FedAuth token deterministically in mock server#89
saurabh500 wants to merge 10 commits into
mainfrom
saurabh500-saurabh500-mock-tds-deterministic-token

Conversation

@saurabh500

Copy link
Copy Markdown
Contributor

Summary

Fixes two design flaws in mssql-mock-tds that made downstream FedAuth regression tests (microsoft/mssql-python#652: est_unique_access_token_transmitted_exactly, est_distinct_tokens_on_sequential_connects) flaky/failing only on Linux.

Stacked on #80 — base branch is saurabh500-publish-python-pkg-ado-feed, not main.

The two flaws

  1. Token recorded only at connection teardown, asynchronously. Each connection is handled in a spawned task; the token was captured into the per-connection ConnectionProcessor during login but only copied into the shared, queryable ConnectionStore after the read loop broke on client EOF. That insert ran on the server task after the client's close() had already returned, so tests had no barrier to wait on and papered over it with time.sleep.
  2. Store keyed by client socket address. ConnectionStore was HashMap<SocketAddr, ConnectionInfo>. Two sequential connects from the same client process frequently reuse the same ephemeral local port on Linux, so the second connection's insert overwrote the first under the same key — has_received_token(first) then returned False.

The fix

  • Record eagerly, before LoginAck. ConnectionProcessor now holds an Option<Arc<Mutex<ConnectionStore>>> and upserts its state into the shared store in both FedAuth paths (inline-token Login7 and challenged FedAuthToken) before the response bytes are returned. Because the client blocks on the LoginAck read, the token is guaranteed visible the moment connect() returns — no teardown dependency, no sleeps. The existing teardown store.store() calls remain as a harmless final upsert.
  • Re-key by a unique per-connection id. An AtomicU64 counter on the server assigns a conn_id at accept() time, threaded through into the ConnectionProcessor. ConnectionStore now uses BTreeMap<u64, ConnectionInfo> so iteration and .values().last() are ordered and get_last_access_token() deterministically returns the most recent connection. store() upserts by conn_id (eager + teardown records for one connection update the same entry). ConnectionInfo.addr is retained for info; ConnectionStore::get now takes a conn_id.
  • Removed the time.sleep workarounds in mssql-py-core/tests/rs-only-tests/test_mock_server_fedauth.py; conn.close() calls kept.

Validation

  • cargo bclippy — green (-D warnings).
  • rustfmt --check on the changed file — green.
  • cargo nextest for mssql-mock-tds unit tests (16/16), mssql-tds test_mock_server_fedauth (all pass), test_redirection (all pass), and test_mock_server user-agent interception (pass). These Rust integration tests exercise the exact new eager-record + BTreeMap store code over TLS on Windows.
  • Both Python bindings (mssql-mock-tds-py, mssql-py-core) build via maturin develop and import cleanly.

Note: The in-repo Python fedauth tests could not be run to green on Windows due to a pre-existing, Windows-only native-tls/schannel handshake limitation with the self-signed test cert (connect times out during the TLS handshake, before any login/token logic). This is orthogonal to this change; the downstream tests run on Linux, and the Rust integration tests validate the same code paths.

Copilot AI and others added 10 commits June 29, 2026 11:17
Change the PyPI distribution name and Python import module from
mssql-mock-tds-py / mssql_mock_tds_py to mssql-mock-tds / mssql_mock_tds.
The folder and Cargo package name stay mssql-mock-tds-py to avoid colliding
with the existing mssql-mock-tds crate it depends on. Use a #[pyo3(name)]
override so the pymodule fn keeps a distinct identifier from the dependency
crate.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add sandbox OneBranch pipeline to publish mock TDS artifacts

Adds .pipeline/OneBranch/PublishFeeds-Sandbox.yml, a manual-only,
test-only pipeline that builds and (opt-in) publishes the mock TDS
Python wheels and crate to the mssql-rs_Public Azure Artifacts feed.
Publishing is gated behind publishPython/publishCrate parameters, both
default false (build + dry run only). Adds a single-abi3-wheel build
script for the Linux container build.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Extend sandbox pipeline with ARM64 and macOS wheel jobs

Adds Linux ARM64, Windows ARM64, and macOS universal2 mock wheel build
jobs to the sandbox PublishMockPython stage and wires them into the
UploadPython dependsOn. macOS builds with a new vendored-openssl
passthrough feature on mssql-mock-tds-py so the universal2 wheel
statically links OpenSSL instead of Homebrew's libssl. Linux jobs keep
system libssl (auditwheel skip); Windows uses SChannel. Still
sandbox/test-only and opt-in.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix OneBranch template resolution in sandbox pipeline

The sandbox pipeline inlines its stages under extends.parameters.stages
while extending the OneBranch governed template, so unqualified template:
includes resolved against the GovernedTemplates repo and failed (e.g.
cargo-authenticate-template.yml not found). Suffix every internal
/.pipeline/... template include with @self so they resolve against this
repo.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Conform sandbox artifact names to OneBranch convention

The governed OneBranch job template requires PublishPipelineArtifact
names to be drop_<Stage>_<Job>_<ob_artifactSuffix>. Rename the five
mock wheel artifacts accordingly so stage validation passes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Stamp sandbox wheel version as SemVer prerelease

cargo metadata rejects the PEP 440 dotted '.dev' suffix in Cargo.toml
('unexpected character . after patch version number'), failing maturin
before the build. Use the SemVer-valid '-dev' form for both pyproject.toml
and Cargo.toml; PEP 440 normalizes it back to '.dev' for the wheel, and
the matching versions satisfy maturin's pyproject/Cargo consistency check.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix sandbox macOS PEP 668 install and inline crate version stamping

macOS wheel build failed at 'pip install --user pipx' with
externally-managed-environment (PEP 668) on the Homebrew Python. Bootstrap
pipx with --break-system-packages only when missing, and install the wheel
package the same way for the re-tag step.

cargo publish --dry-run failed because the shared stamp-crate-versions.ps1
uses a global ^version regex that also rewrote the [dependencies.uuid]
version field to the prerelease version, corrupting the uuid requirement.
Stamp inline instead, replacing only the first ^version line (the [package]
version) in mssql-tds and mssql-mock-tds.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix macOS sandbox wheel version stamp for BSD sed

The macOS Stamp step used the GNU-only empty-regex reuse form
s//.../ which BSD sed rejects with "first RE may not be empty".
Replace it with an explicit substitution pattern. Each mock manifest
has exactly one line-starting version = (the package version), so a
plain substitution stamps only that line.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Invoke wheel as a module in macOS re-tag step

The --user/Homebrew wheel install does not place the wheel console
script on PATH, so wheel tags failed with exit 127 (command not
found). Call it as python3 -m wheel tags instead.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Use UsePythonVersion for macOS mock wheel job

Pin Python 3.12 via UsePythonVersion@0 instead of relying on the
agent's externally-managed Homebrew python3. The selected interpreter
is managed, writable, and on PATH, so the PEP 668
--break-system-packages workarounds and the off-PATH wheel/maturin
console-script issues go away. Install maturin and wheel with
python -m pip against that interpreter and re-tag with python -m wheel.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Run mock wheel upload job on the custom Windows pool

The UploadPython job used the default governed Windows pool, whose
build container has an empty Python tools cache and no outbound
network, so UsePythonVersion@0 failed trying to download Python 3.12
(WinError 10013). Use the same custom pool/image as the Windows build
jobs (RUST-1ES-POOL-WUS3 / RUST-Win22-Sql25-1P), which ships Python
3.12 and can pip install twine.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Drop unsupported --skip-existing from twine upload

Azure Artifacts' PyPI endpoint rejects twine's --skip-existing flag
(UnsupportedConfiguration). Each run stamps a unique .dev<date><BuildId>
wheel version, so there is no existing version to skip. Remove the flag.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Build self-contained musllinux_1_2 wheels (Alpine) for both architectures
using the repo's musllinux_*_rust container images. musl wheels statically
vendor OpenSSL (MATURIN_FEATURES=vendored-openssl) so they don't depend on
Alpine's libssl at the consumer side; the container build script now honors
an optional MATURIN_FEATURES env. Wire both jobs into UploadPython.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When the releaseVersion parameter is set, the seven Python wheel build
jobs stamp the base version from pyproject.toml as-is (e.g. 1.0.0)
instead of appending the .dev<date><buildId> prerelease suffix. Default
remains the .dev prerelease so existing behavior is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add a PreflightVersionCheck job that gates all seven wheel builds. When
publishing a release (releaseVersion + publishPython both true), it
authenticates pip to the feed and queries the PEP 503 simple index for
mssql-mock-tds; if the base version from pyproject.toml is already
published, the pipeline fails in seconds instead of building seven
wheels and only hitting the duplicate-version rejection at twine upload.

The check is best-effort on lookup failure (warns, continues) since the
upload-time rejection remains the authoritative guard. For dev/dry-run
runs the job is a no-op.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The mssql-rs_Public feed allows anonymous reads, so drop PipAuthenticate
from the Python preflight and pass the public simple-index URL directly.

Extend releaseVersion to the crate stage: when set, mssql-tds and
mssql-mock-tds publish their base version (e.g. 1.0.0) instead of the
-dev.<date>.<buildId> prerelease. Add a PreflightVersionCheck-style guard
as the first step of PublishCrate that queries the public Cargo sparse
index and fails fast if either crate version is already published, before
cargo-authenticate and the rust install. Best-effort on lookup failure;
cargo's duplicate-version rejection remains the authoritative guard.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The mock server previously recorded connection tokens into the shared
ConnectionStore only at connection teardown, and keyed the store by the
client socket address. Both flaws made downstream fedauth regression
tests flaky on Linux: tokens were not visible when connect() returned
(no barrier, papered over with sleeps), and sequential connects reusing
the same ephemeral port overwrote each other under the same key.

Record the token eagerly during login, before the LoginAck is sent, by
giving ConnectionProcessor a handle to the shared store. Since the client
blocks on LoginAck, the token is guaranteed visible once connect()
returns. Re-key ConnectionStore by a unique per-connection id from an
AtomicU64 counter using a BTreeMap so iteration is ordered and the most
recent connection is deterministic. Drop the time.sleep workarounds in
the Python fedauth tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assert the FedAuth token is visible in the shared ConnectionStore while the
client connection is still open, guarding against a regression to teardown-only
recording. Mirrors the downstream ODBC pooled-connection scenario where close()
does not send EOF.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@saurabh500
saurabh500 changed the base branch from saurabh500-publish-python-pkg-ado-feed to main July 6, 2026 19:33
@saurabh500

Copy link
Copy Markdown
Contributor Author

Superseded by #91, which targets main directly and contains only the mock-server token-capture changes (this branch was stacked on #80 and pulled in unrelated commits when retargeted).

@saurabh500 saurabh500 closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants