[Cosmos] Move the key-auth live tests onto fixed, long-lived test accounts - #48459
Draft
Simon Moreno (simorenoh) wants to merge 7 commits into
Draft
[Cosmos] Move the key-auth live tests onto fixed, long-lived test accounts#48459Simon Moreno (simorenoh) wants to merge 7 commits into
Simon Moreno (simorenoh) wants to merge 7 commits into
Conversation
The Cosmos live tests are moving off per-run provisioned accounts and onto
fixed, self-owned accounts that are shared with other language SDKs and with
concurrent runs of this suite. Databases are the only account-scoped namespace
the tests control, so every database created against a live account now carries
a prefix identifying the run that created it.
Adds RESOURCE_PREFIX / RUN_ID / unique_database_id() to test_config.py. The run
id combines Build.BuildId with a random suffix: the build id groups every matrix
leg of a single pipeline run, which is what an out-of-band cleanup pass matches
on, while the suffix keeps parallel legs distinct.
Two call sites embed the generated id rather than using it verbatim so their
assertions still mean something: test_crud_database keeps a literal leading
space for the "id can begin with space" case, and test_resource_id keeps its
unicode and special-character payload.
Also fixes a latent bug in the create_database_if_not_exists negative tests,
which used a hardcoded "responses_test" id. Both calls now share one run-scoped
id, so the second call is guaranteed to be the one exercising the already-exists
path; previously a leftover database from any earlier run made the first call
take that path too, and the test silently stopped testing what it claimed.
Left alone deliberately: create_database_if_not_exists("test", ...) in
test_crud*.py runs through TimeoutTransport with passthrough disabled and never
reaches the service, test_proxy's database is emulator-only, and the
id_with_illegal* cases are negative tests where creation is expected to fail.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cb8202b3-2717-4b4a-92af-0a0a4cb223f3
…hQuery
Live tests toggle SDK feature flags through os.environ, but every one of them
"restored" the flag by writing an assumed default rather than the value it
replaced. On the circuit-breaker matrix legs the pipeline sets
AZURE_COSMOS_ENABLE_CIRCUIT_BREAKER=True for the whole job
(test-resources.bicep:107), so test_availability_strategy deleting the variable
and test_user_agent setting it to "False" silently disabled the feature under
test for every test that ran later in the same process.
Add test_config.set_environment_variables / restore_environment_variables, which
capture prior values and put them back exactly -- including removing variables
that were previously unset -- and route the toggles through them. Sites that
mutate before entering their try block now register the restore with
addCleanup so it runs even if setup fails.
Also fixes leaks that had no restore at all:
- test_aad{,_async}.test_account_scope_only and test_account_scope_fallback_on_error
set AZURE_COSMOS_AAD_SCOPE_OVERRIDE="" and never cleared it, so the leak was
order-dependent on the two tests that did clear it.
- test_session_token_unit.validate_different_session_token_false_progress_merge_scenarios
set AZURE_COSMOS_SESSION_TOKEN_FALSE_PROGRESS_MERGE and never cleared it.
Separately, register the cosmosSearchQuery marker in pytest.ini. It is used by
8 test files and selected by the macos311_search_query matrix leg but was never
declared.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cb8202b3-2717-4b4a-92af-0a0a4cb223f3
Three changes that only matter once the suite stops getting a freshly
provisioned account per run.
cosmos-staging.yml swept the account with an unfiltered
"for db in client.list_databases(): client.delete_database(db['id'])" plus an
equivalent Remove-AzCosmosDBSqlDatabase pipeline task. Against a long-lived
account shared with other languages' CI that deletes resources still in use, so
both cleanups are now scoped to the PythonSDKTest-<build id>- prefix this run
creates. (The pipeline looks vestigial -- it is an export of a classic pipeline
and nothing in the repo references it -- but making it safe is cheap and does
not depend on settling that question.)
test_backwards_compatibility{,_async} asserted len(list_databases()) > 0 after
creating a database, which passes on a shared account whether or not the
database under test exists. It now asserts the run's own database and container
appear in the listings, which is both isolation-safe and a stronger check.
Setup and teardown now go through test_config.retry_control_plane, which
retries 408/429/449/500/503 with jittered backoff, preferring the service's
x-ms-retry-after-ms hint. The account control plane is shared, so database and
container create/delete can fail purely because another run is busy on the same
account, and the SDK's built-in throttle retry gives up after ~30s. 400/401/403/
404/409 are deliberately not retried so negative tests still fail fast. An async
variant is provided for callers that create resources directly.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cb8202b3-2717-4b4a-92af-0a0a4cb223f3
… needs Ports azure-sdk-for-java#49735's account resolver so Python's Cosmos live tests can bind to the fixed sdkci-* accounts in the Cosmos-owned CosmosDB_Test_Subscription instead of provisioning an account per run. Per-run provisioning is what breaks every ~90 days when the live-test tenant is recreated and every service principal and RBAC assignment with it. Nothing is switched over yet: DisableAzureResourceCreation defaults to false and no matrix leg sets AccountSelector, so this is inert until sdk/cosmos/tests.yml opts in. sdk/cosmos/pipeline/resolve-cosmos-test-account.ps1 reads the same secret, in the same schema, naming the same accounts as the Java resolver, and applies the same validation. It is PowerShell rather than Java's bash+jq because Python's matrix has five windows-2022 legs and a macOS leg where neither bash nor jq is available; pwsh and ConvertFrom-Json need no install. It emits only ACCOUNT_HOST/ACCOUNT_KEY -- consistency, preferred locations and AZURE_COSMOS_ENABLE_CIRCUIT_BREAKER stay owned by live-platform-matrix.json, and a test asserts that. Keys use the double-set convention already in eng/common/TestResources/TestResources-Helpers.ps1: _ACCOUNT_KEY as a secret so the scrubber masks it, plus a plain ACCOUNT_KEY so it still reaches the test process, since issecret=true suppresses environment propagation. Added on top of the Java version: a case-sensitive selector match, so a matrix typo cannot silently bind a leg to the wrong account, and a rejection of values containing line breaks, which would otherwise truncate the logging command and print the rest of the key unscrubbed. live.tests.yml and archetype-sdk-tests.yml gain DisableAzureResourceCreation to skip the ARM deploy/remove pair. live.tests.yml also now forwards BeforeTestSteps/AfterTestSteps to build-test.yml: it declared both parameters but never passed them on, so anything a service supplied was silently dropped. Nothing in sdk/ sets them today, so wiring them up changes no existing behaviour -- but the resolver depends on it, because build-test.yml is where those steps run in the same job as the pytest task, which is what lets a published pipeline variable reach the tests as an environment variable. AAD legs are deliberately left on the provisioning path: test-resources.bicep creates a sqlRoleAssignment for testApplicationOid, and that principal is tenant-scoped. Legs with no AccountSelector skip the resolver step. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb8202b3-2717-4b4a-92af-0a0a4cb223f3
Adds AccountSelector to the 12 key-auth matrix legs and runs the resolver from
BeforeTestSteps. The resolver executes after the ARM deployment and republishes
ACCOUNT_HOST/ACCOUNT_KEY, so those legs talk to the fixed sdkci-* accounts while everything
else about the pipeline stays as it is.
Provisioning is deliberately left on. Turning it off is what removes the tenant coupling, but
DisableAzureResourceCreation is a stage-level parameter and the AAD legs share the stage --
they need the per-tenant sqlRoleAssignment that test-resources.bicep creates, so they cannot
stop provisioning. Overriding rather than disabling proves the accounts, the secret and the
resolver end to end without forcing that split first. The AAD legs set no AccountSelector, so
the resolver step skips itself for them and they keep the account they provisioned.
Selector map (verified against the matrix, not the group names -- several groups expand to
two or three jobs):
single-session cosmosQuery, cosmosSearchQuery, cosmosLong x2
single-session-split cosmosSplit x2
multiregion-tc-session cosmosCircuitBreakerMultiRegion,
cosmosPerPartitionAutomaticFailover
multimaster-multiregion-session cosmosCircuitBreaker, cosmosMultiRegion x3
The circuit-breaker and per-partition-failover lanes need no dedicated account: failures are
injected in-process by tests/_fault_injection_transport.py rather than by degrading the real
account, so they only need the right topology.
Selector values are mapped to "" in displayNames so they do not lengthen job names.
Two things to watch on the first run. The variable group must actually expose
sub-config-cosmos-azure-cloud-test-resources -- if it does not, $(...) expands to its own
literal macro text and the resolver reports "not valid JSON". And both cosmosSplit jobs share
one selector and run concurrently, each forcing splits and scaling to 30000 RU; that is
container-scoped so correctness holds, but it is real load on a shared account.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cb8202b3-2717-4b4a-92af-0a0a4cb223f3
|
Azure Pipelines: Successfully started running 2 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Member
Author
|
/azp run python - cosmos - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Route AAD-credential regression tests to the AAD lane, and give availability-strategy hedging enough timing margin for shared accounts. Also register the resolver vocabulary with cspell. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb8202b3-2717-4b4a-92af-0a0a4cb223f3
Member
Author
|
/azp run python - cosmos - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Use a new container ID for every sync test invocation so a persistent data client cannot reuse session state after another client deletes and recreates the container. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: cb8202b3-2717-4b4a-92af-0a0a4cb223f3
Member
Author
|
/azp run python - cosmos - tests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
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
1 participant
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.
Towards #48236.
The Cosmos live tests provision a Cosmos account per run via ARM, which couples them to the live-test subscription. That subscription now rotates on a ~90-day "ephemeral tenant" cycle, and every rotation takes the tests with it. Java already moved off this (Azure/azure-sdk-for-java#49735) onto a set of fixed accounts the Cosmos team owns; this is the Python port of that work.
Those accounts already exist — resource group
sdk-ciinCosmosDB_Test_Subscription, 18 accounts namedsdkci-<selector>, provisioned for the Java migration. Python needs 4 of them.What's here
A resolver.
sdk/cosmos/pipeline/resolve-cosmos-test-account.ps1reads a versioned JSON blob out of an ADO variable-group secret, looks up an account by selector, and publishesACCOUNT_HOST/ACCOUNT_KEY/SECONDARY_ACCOUNT_KEY. It's a PowerShell port rather than a copy of Java's bash+jq script because Python's matrix includes Windows and macOS legs.Twelve matrix legs bound to selectors.
live-platform-matrix.jsongains anAccountSelectoron each key-auth leg;tests.ymlgains aBeforeTestStepsblock that runs the resolver.Tests that can share an account. The bulk of the diff. Provisioning per run meant every test had an account to itself, and a fair amount of the suite quietly depended on that.
Staged on purpose
This PR does not turn provisioning off.
BeforeTestStepsruns after the deployment, so the resolver republishesACCOUNT_HOST/ACCOUNT_KEYover what the deployment set — same names, same double-set convention asTestResources-Helpers.ps1, so the later write wins. The key-auth legs run against the fixed accounts; an unused account still gets deployed alongside.The reason is the AAD legs.
DisableAzureResourceCreationis a compile-time stage parameter, andarchetype-sdk-tests.ymlemits one stage per (package × cloud) — Cosmos is one package and one cloud, so all 20 jobs share a stage and the flag can't vary per leg. The 7cosmosAAD*legs need the per-tenantsqlRoleAssignmentthattest-resources.bicepcreates, so they can't move yet. Java never hit this: its Cosmos matrix has no AAD legs, and it composes stages directly.So this proves the binding works first. Turning provisioning off is a follow-up, and needs one of: a second pipeline for the AAD lane, a data-plane role assignment on
sdk-ciso AAD can use fixed accounts too, or aConditionparameter on theeng/commondeploy/remove templates. Options are written up insdk/cosmos/pipeline/README.md.Making the tests shareable
Anything a shared account makes unsafe, in five commits:
test_database, so two concurrent runs would collide. Everything now goes throughtest_config.unique_database_id(), which prefixes withPythonSDKTest-<build id or local uuid>-. Also gives the cleanup a prefix to filter on, and keeps Python's resources inside Python-owned databases so container-level scaling doesn't reach other languages' work.retry_control_plane(and an async twin) retries 408/429/449/500/503 with jittered backoff, honoursx-ms-retry-after-ms/Retry-After, and deliberately doesn't retry 4xx so negative tests still fail fast.test_backwards_compatibility{,_async}assertedlen(list_databases()) > 0— true on a fresh account, meaningless on a shared one. Now checks its own ids are present.cosmos-staging.ymlhad two teardown steps that deleted every database in the account. Both now filter to this run's prefix.os.environweren't putting it back, so state leaked between tests in a process. Notablytest_availability_strategywas deletingAZURE_COSMOS_ENABLE_CIRCUIT_BREAKERandtest_user_agentwas setting it to"False"— on the circuit-breaker legs, where the pipeline sets it for the whole job, that disabled the feature under test for everything that ran afterwards. Pre-existing, unrelated to account sharing, but it surfaced while auditing shared state.Also fixed here
eng/pipelines/templates/jobs/live.tests.ymldeclaredBeforeTestStepsandAfterTestStepsand never forwarded them tobuild-test.yml, which declares and uses both. Silently dropped for every service. No service undersdk/currently sets them, so this was latent rather than broken — but the resolver depends on it.Verification
pwsh ./sdk/cosmos/pipeline/resolve-cosmos-test-account.tests.ps1). Covers selector lookup, version gating, missing/malformed fields, non-httpsendpoints, case sensitivity, and the shape of the emitted logging commands.displayNames.Two selector notes worth flagging for review: the multi-region legs borrow
sdkci-multiregion-tc-session, which was provisioned for Java's not-yet-wired thin-client stages, and bothcosmosSplitlegs sharesingle-session-splitand run concurrently — each forces partition splits and scales to 30k RU, so that's 60k RU on one shared account on top of Java's load. Container-scoped, so correctness holds, but it's the load case to watch on the first run.Two things I chose not to fold in, since both want their own change: teardown that deletes only what a run created (there's a known container leak in
test_backwards_compatibility), andconftest.py, which builds aCosmosClientat import and so needs a reachable account for any pytest invocation in that directory. The obvious fix for the latter — moving provisioning intopytest_collection_modifyitems— would silently break the multi-region tests, because three modules capture region info into module-level constants at import time and would freeze at their defaults.