Skip to content

feat: ADR GA - #906

Open
Hangyi (HangyiWang) wants to merge 33 commits into
Azure:previewfrom
HangyiWang:work/ga-2026-11-01
Open

Hangyi (HangyiWang) wants to merge 33 commits into
Azure:previewfrom
HangyiWang:work/ga-2026-11-01

Conversation

@HangyiWang

@HangyiWang Hangyi (HangyiWang) commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Service APIs

Service Plane API version
Device Registry / CMS Control 2026-11-01
IoT Hub Control, service data, device data 2026-11-01
DPS Control, service data, device data 2026-11-01

Command changes

  • Add GA ADR namespace/device, Hub/DPS link, managed identity, certificate authority/policy, and schema registry/schema/version commands, reusing supported release-preview implementations.
  • Protect ADR namespace unlink with ETag-based conditional writes; concurrent updates return HTTP 412 instead of being overwritten.
  • Preserve existing GA Hub/DPS authentication, hostname, key-output, table-output, and update behavior; honor the active cloud for ordinary Hub/DPS management. ADR/CMS remains public-cloud-only.
  • Exclude preview-only SU, MQTT broker, semantic-model, and ADR device-type-reference surfaces.
  • Accept relative paths in az iot du update calculate-hash without resolving symlink URIs.

Test-only command coverage follow-up

Adds happy-path coverage for 13 DPS and 16 Hub commands in test-only commits. Two production issues exposed by these tests received explicit approval for separate fixes, documented below. Tests verify meaningful updates, unchanged keys and unrelated objects, certificate replacement/readback, and cleanup. Manual failover uses a dedicated owned Hub; module methods use a real connected module responder in the owned SAS phase.

Coverage Live status in UPX centraluseuap, canary ARM
DPS: 13 command paths Passed across three focused scenarios. The management retry passed at a8a4aea3; linked-Hub show and registration deletion passed at 5dd136a0, with their code and dependencies unchanged.
Hub certificate show/update, consumer-group list/show, SKU/quota/stats: 7 paths Passed at a8a4aea3.
Hub enrichment update/delete: 2 paths Passed at bc8409a0, including real create/update/readback/delete after the TLS metadata filter fix.
Hub policies: 5 paths Policy lifecycle setup/call/teardown passed at ead9afa6, after the TLS projection and authoritative-output fixes.
Hub manual failover: 1 path Passed at 9eaa6ca7, including setup, call, teardown and native gates. Setup took 696.4s, failover 523.5s and teardown 65.6s. All four recorded IDs were independently absent; baseline preserved and private profile removed.
Hub module method: 1 path Passed in the native owned SAS phase at bc8409a0, with a real connected module responder and zero skips.

The first DPS management attempt exposed an incorrect test assumption about independently round-tripping permission names. The test now explicitly requests paired enrollment/registration permissions and retains exact returned-rights assertions. Both DPS attempts have complete native cleanup plus independent absent-resource checks; baseline resources were preserved and temporary profiles removed. The original failed attempt remains recorded.

Opus 5.5/high reviews addressed void-output handling, narrowly scoped failover admission, correlated failover-operation completion before cleanup, the legacy ADO selection boundary, and the subsequent failover test corrections. Legacy ADO retains its previous partial roster; receipt-dependent cases remain required by the native controller.

The failed original Hub cohort has six independently absent recorded IDs and a preserved baseline, but its failed native gates remain failed. The TLS-fix rerun has complete native cleanup, two independently absent IDs, a preserved baseline and a removed private profile.
The module-method run also passed its native gates and cleanup; eight recorded IDs are independently absent, the baseline is preserved and its temporary profile is removed.
The policy/failover cohort has four independently absent IDs and a preserved baseline; its failed teardown and native gates remain failed.

Separately approved GA fixes exposed by live coverage

  • bc8409a0: exclude GET-returned minTlsVersionMigration metadata from Hub PUT bodies. Preserve writable minTlsVersion, ETags and GET output. Opus 5.5/high review found no significant issues; 178 focused checks and linters passed. Live enrichment lifecycle passed.
  • ead9afa6: policy create/delete wait for successful writes, then read policies through the existing list-keys API instead of assuming the completed Hub response contains authorizationPolicies. Preserve policy/key output and error propagation. Opus 5.5/high review found no significant issues; 540 focused checks and linters passed. Live policy lifecycle passed.

Full qualification follow-up

  • 3721d01f commits the GA registration/hostname contract assertions previously used in local qualification.

  • 08661af9 fixes the storage test fixture's fresh managed-identity grants: use known object IDs regardless of caller type, avoiding Graph lookup/name hydration while retaining exact-ID cleanup and bounded visibility. Opus 5.5/high review, 401 focused unit tests and linters passed.

  • 3a70c034 retains strict polling URL rejection while reporting only failed validation categories and forbidden path character codepoints. It also checks policy create/delete return values in live coverage. Opus 5.5/high review, 732 focused unit tests and linters passed.

  • 9eaa6ca7 narrowly admits literal = in Azure-acknowledged opaque ARM polling paths. Scope, origin, correlated operation completion and no-replay guards remain enforced. Opus 5.5/high review included a red/green check proving the observed-DELETE regression fails without the fix.

All 29 added command paths have passed across the documented focused GA runs. This is not complete-suite qualification.

The complete rerun at 9eaa6ca7 started on 2026-10-17 at 21:11 UTC. It launched 33 Hub control, 53 Hub data and 68 DPS cases (154 total); the additional 17 ADR cases were not launched. Execution was stopped after the DPS local-auth update exceeded its 900-second phase budget and Azure rejected cleanup with HTTP 409 / 409301, reporting IotDpsState: Transitioning. A read-only check at 22:07 UTC still returned both state fields as Transitioning with disableLocalAuth=true.

Final evidence contains 110 recorded case passes, three Hub failures, five incomplete case receipts and 36 unreported cases. The Hub failures are storage endpoint authorization (400117), migrate-controlplane-with-create deployment failure, and monitor-feedback assertion failure; their root causes are not yet established. Native cohort gates remain failed/interrupted. The DPS count excludes a synthetic unexpected JUnit identity.

Shutdown also exposed a worker that outlived its controller; the worker has now exited. Recovery used copied ownership receipts and the native no-replay cleanup helper, preserving original execution gates. Independent verification at 22:07 UTC found 145 of 147 recorded IDs absent. Only the stuck owned DPS instance and its scoped role assignment remained; the baseline resource was preserved and temporary profiles were removed. This is the historical local-run snapshot, not the newer GitHub-run cleanup result.

The seven generated SDK defaults are 2026-11-01. Hub state export/import is a separate path: it invokes az group export, preserves the returned resources' apiVersion, and submits them through az deployment group create. The GitHub ownership receipts confirm 2026-05-01-preview deployment targets; the existing blank_hub_arm.json compatibility fixture also explicitly uses 2022-04-30-preview. The ownership transport does not replace those versions. These passing snapshot/deployment cases are not evidence of GA-only template operations; SDK defaults do not control an ARM template's resource versions.

CI for historical head 9eaa6ca7 was 64/64 green. The preview counterpart is #935 at 2ba1f894, with its last observed 27/27 green checks. It preserves preview-only contracts and preview's native poller. Preview live execution is still pending GA verification.

Native GitHub qualification: 2026-10-18

Full selected run 39081726111 used exact commit 9eaa6ca7, Python 3.13, centraluseuap, and canary ARM, selecting all Hub control, Hub data, DPS, and ADR tests.

Suite Result Native cleanup
DPS 68/68 passed: regular 36, service SAS 29, local-auth toggle 3 All three phase receipts complete
Hub control 33/33 passed across all four phases All four phase receipts complete
Hub data 53/53 passed: Entra state/config 7, Entra devices/protocol 37, SAS 9 All three phase receipts complete
ADR 14 passed, 3 failed Original failed result retained; see fix and rerun below

All 154 Hub/DPS cases have passing setup, call, and teardown receipts, with no skips or missing cases. The earlier local Hub storage, migration, feedback, and DPS local-auth failures did not recur; this does not by itself establish their earlier root causes. The original workflow remains failed because of ADR, and the final summary correctly propagates that failure rather than hiding it.

Commit f75e0aec fixes the two identified ADR issues:

  • The schema registry integration fixture now explicitly requests the selected region instead of inheriting the resource group's westus2 location.
  • Existing bounded fresh-grant recovery now recognizes the exact canonical DPS ResourceMiNotAuthorized / inbound 400315 envelope. It remains DPS-only, requires a grant created by this command, retains role/identity/snapshot checks and the two-retry limit, and does not retry arbitrary authorization failures or uncertain writes. The no-wait test readiness path recognizes the same exact envelope under its existing fresh-grant window.

The fix passed 771 focused unit tests and existing linters; Opus 5.5/high review found no blockers. Both the fork PR branch and Azure-owned branch contain the fix.

Full ADR rerun 39084646411 used exact f75e0aec with the same Python, region, and endpoint. It finished at 01:46 UTC with 15 passed, 2 failed, all 17 case receipts complete and no skips. The schema registry lifecycle passed with the explicit region fix. The link lifecycle's DPS add also succeeded with zero recovery updates, before failing later on Hub add; it did not exercise the newly recognized DPS error envelope in this run.

ADR failure at f75e0aec Evidence and disposition
TestADRLinkLifecycle.test_adr_link_lifecycle Hub add returned AdrMiNotAuthorized for namespace-managed-identity data-plane read access. Azure's message now includes (endpoint 'secondary' being linked, resource '<Hub ARM ID>'); the old test helper rejected that context and attempted zero recovery updates. This confirmed test-helper compatibility gap is corrected in cd9ab501; see the focused follow-up below.
TestADRLinkDelete.test_adr_link_hub_dps_delete DPS add produced five terminal InvalidLinkRequest / AdrMiNotAuthorized failures and exhausted the existing 600-second recovery deadline. Required outbound-identity Contributor assignments and repeated verification were confirmed, but the last actual PATCH was approximately 8m44s after grant creation. The run was blocked by service authorization; that does not prove propagation would never recover. The actual denied backend principal/token/cache decision is unavailable, so the ultimate cause remains unproven.

Further live reruns initially stopped on that blocker as requested. PR CI for historical f75e0aec was 77/77 green, independently of the failed integration workflows. The user subsequently authorized the test-helper correction and a separate longer-window diagnostic below. Results at different commits and diagnostic timeouts are not combined into an all-green full-suite claim.

Cleanup of the failed ADR scenarios initially left one DPS instance after a 409304 rejection at 01:19:08 UTC. An independent ARM activity-log check confirmed that DELETE terminally failed with Conflict, and the preceding unlink notification subsequently succeeded at 01:21:04. After verifying Active/Succeeded and namespace absence, recovery submitted one DELETE for that exact owned DPS instance. At 01:52 UTC, its GET and scoped-role GET both returned HTTP 404. Together with the nine independently absent roots checked earlier, all ten roots of the two failed ADR scenarios are absent. Original failed execution and cleanup evidence remains preserved. The separate older local-run DPS remainder is not included in this cleanup claim.

Focused ADR follow-up: 2026-10-18

cd9ab501 corrects only the no-wait integration test helper: the contextual Hub message must match the exact Hub hostname, endpoint name, resource ID, and AdrMiNotAuthorized code, with no extra error fields. Existing legacy messages, identity/settings preservation, and retry bounds remain unchanged. No production CLI code or timeout defaults changed. All 450 focused unit tests and existing linters passed; Opus 5.5/high review found no blocking findings. Both GA remote branches contain this commit.

The exact TestADRLinkLifecycle.test_adr_link_lifecycle case passed setup, call, and teardown in a focused native local run at cd9ab501, Python 3.10, centraluseuap, and canary ARM. It completed at 04:25 UTC using unmodified commands and normal timeouts. Independent GETs at 04:26 UTC confirmed all 15 recorded owned IDs absent, both baseline resources preserved, and the temporary profile removed. This run encountered no authorization failures: the lifecycle is live-green, while the new contextual recovery branch is covered by deterministic offline regressions.

A separate TestADRLinkDelete.test_adr_link_hub_dps_delete diagnostic ran from 04:26:50 to 04:45 UTC after fresh admission. A session-only wrapper supplied --timeout 1200 to its waited DPS link add; all remaining commands, identities, automatic grants, recovery behavior, and assertions were unchanged. This is a diagnostic, not qualification under the default timeout.

The original DPS outbound AdrMiNotAuthorized denial did not recur. Instead, DPS returned the distinct inbound ResourceMiNotAuthorized / 400315 error at 04:36:40. Existing f75e0aec recovery submitted one identical, identity-preserving PATCH at 04:37:20; DPS linking succeeded at 04:38:52. The complete DPS command took 207.410 seconds, and success was observed 199.574 seconds after the inbound namespace-read Contributor grant was accepted. This live-exercised the existing inbound-error recovery but never used the additional ten minutes. It does not establish that a longer default timeout would fix the original outbound denial.

The complete diagnostic case still failed, later on the second Hub add, before any unlink assertions. Its persisted LinkCommitFailed said: The namespace's managed identity is not authorized to link the Hub resource. Grant it access on the resource, then resubmit the request. (Hub resource reported: [AuthorizationFailed]). This redacted error is not recognized by the production recovery classifier; no recovery update was submitted for this Hub. The corresponding namespace PATCH correlation is 69b0c5f4-dd2d-4b5a-bb7e-506cd8fff4dd at 04:41:26 UTC. This is a separate failure, not the contextual Hub device-count message fixed in cd9ab501. No further production behavior/default change or additional live attempt has been made pending the user's next decision.

The diagnostic has complete failed-case receipts, with setup and teardown passed. Independent GETs at 04:46 UTC confirmed all 13 recorded owned IDs absent, both baseline resources preserved, and the temporary profile removed. Original failed receipts are retained.

The Hub helper correction is also mirrored in #935 as d15a749a, preserving preview-specific fresh-grant behavior. All 461 focused preview tests and linters passed, with no blocking Opus 5.5/high review findings. GA PR CI at cd9ab501 is 77/77 green. Preview PR CI at d15a749a is 24/24 green, with run 39097306705 completed successfully. Preview live testing remains pending; CI success does not replace live qualification.

Full GA retry at cd9ab50: 2026-10-18

At the user's request, run 39099115825 reran all four selected suites at exact cd9ab501c12016886cb28352bef03da544ceac11, Python 3.13, centraluseuap, and canary ARM. It used normal commands and timeouts, with no diagnostic overrides or additional code changes. The workflow finished failed.

Suite Passed Failed case reports Interrupted Not started
DPS 67 1 0 0
Hub control 32 1 0 0
Hub data 46 0 1 6
ADR 15 2 0 0
Total: 171 selected 160 4 1 6

No cases were skipped. The 160 passes include complete setup/call/teardown receipts. Historical passes at other commits are not used to fill the seven unqualified Hub data cases.

Failure Observation
DPS test_register_and_issue_certificate_contract[deadline] Registration completed as assigned, and certificate-chain/correlation assertions passed. The next operation-status call returned the same operation ID but status assigning. Failure is line 77's readback assertion, not the 180-second registration deadline; the whole test call took 47.9 seconds. The CLI returns that status response unchanged. The underlying readback inconsistency is not established.
Hub control test_iot_storage_endpoint_lifecycle Key-based storage endpoint creation passed. Creating the next endpoint with --identity [system] returned 400117, saying the storage operation was not authorized with this permission. Correlation a719813c-3c11-4e8e-93e5-83b009ad1aca, 05:30:35 UTC. Propagation is plausible, but not proven. Manual failover passed.
ADR TestADRLinkLifecycle.test_adr_link_lifecycle DPS linking passed. The secondary Hub returned the legacy resource-read AdrMiNotAuthorized; the helper recognized it and submitted one identity-preserving update. Subsequent reads remained Failed until the readiness deadline, without observed progress authorizing another update. This is not the contextual-message classification defect fixed by cd9ab501.
ADR TestADRLinkDelete.test_adr_link_hub_dps_delete DPS outbound AdrMiNotAuthorized recovered within the normal timeout. The first Hub add then failed with LinkInitiateFailed, reporting Hub code 400913. No unlink assertions were reached. This differs from the earlier second-Hub LinkCommitFailed/AuthorizationFailed; the underlying rejection is not explained by the available error.

Hub data's entra-devices-protocol phase exhausted its 4500-second (75-minute) phase budget after 30 passes, interrupting TestIoTHubModules.test_iothub_module_identity during module query visibility polling. That test had run approximately 618 seconds; its own 30-minute query-visibility allowance was not exhausted. Earlier device-identity and nested-edge cases passed after 907.5 and 1204.3 seconds respectively. The five-minute traceback dump is diagnostic output, not a five-minute test failure. Shutdown subsequently encountered a closed-progress-stream ValueError during cleanup.

The six unstarted cases are TestIoTHubModules.test_iothub_module_renew_key, test_iothub_module_connection_string_show, test_iothub_module_generate_sas_token, test_iothub_module_hostname_type_permutations, and TestHubPreview.test_identity_roundtrip, test_responding_digital_twin. Thus Hub data has seven unqualified cases, not seven assertion failures.

Cleanup evidence is retained without rewriting failed native receipts. All three DPS phase cleanup receipts completed. Hub control completed three of four native cleanup receipts; independent read-only GETs confirmed all 20 recorded IDs, including six roots, absent for its failed phase. Hub data completed two of three native cleanup receipts; its interrupted phase had four roots absent but one scoped role still unattested. Independent read-only GETs subsequently confirmed all eight recorded IDs, including the four roots and all four scoped roles, absent. The two failed ADR scenarios reported cleanup completion, and independent GETs confirmed all ten scenario roots absent. The older local-run DPS remainder was not touched and is not included in these claims.

The manual integration gate and final workflow gate correctly failed. Regular PR CI remains independently green: 79/79 successful checks for this GA PR at cd9ab501, and 24/24 for #935 at d15a749a. Preview live testing remains pending. No retry broadening, timeout change, production fix, or further dispatch was made in response to this run; full-green qualification remains blocked.

Authorized readiness fixes and focused retries: 2026-10-18

The subsequent read-only investigation distinguished two test-side readiness gaps from two instrumented retries. The earlier Hub 400913 was decoded from its Activity Log: the selected inbound UAMI was denied four ADR management actions despite an accepted Contributor grant on the exact namespace. That finding supersedes the earlier unexplained-error note; it did not justify a blanket 400913 retry.

The user authorized these test-only changes, now pushed to both GA remotes:

  • e195cbe7: hand a recognized ADR persisted failure to the existing waited, identity-preserving recovery command using only the remaining helper deadline. Do not replay recovery based on stale GETs; the original 600-second helper budget remains.
  • b190afc0: wait 180 seconds after each newly observed storage role grant, for the exact Hub system principal and UAMI at the exact storage-account scope. Existing grants do not add a wait. Endpoint errors still propagate without replay.
  • 52e300ee: move the two query-heavy device/module identity cases into a separate required entra-identities HubData phase. All four HubData phases retain their existing per-phase bounds, the overall job cap is unchanged, and the complete selection remains 171 cases.
  • 11fabc09: account for the two storage fixture waits in that lifecycle's pytest budget: 900 + 2 * 180 = 1260 seconds, including fixtures. CLI timeouts, phase budgets, command coverage, and all assertions are unchanged.
Focused case Commit Final result
DPS test_register_and_issue_certificate_contract[deadline] 52e300ee Passed, including native debug gates and cleanup. Registration and the subsequent GET for the same operation both returned assigned using data API 2026-11-01; no diagnostic convergence reads were needed.
Hub test_iot_storage_endpoint_lifecycle 11fabc09 Passed, including all 19 lifecycle commands, six endpoint-property assertions, setup/teardown, native debug gates, and cleanup. Pytest completed in 995.78 seconds. Neither the earlier storage authorization error nor the timeout recurred.
ADR TestADRLinkDelete.test_adr_link_hub_dps_delete 52e300ee Passed the complete scenario, including Hub/DPS unlink, idempotency/error assertions, and teardown. The first-Hub 400913 did not recur.
ADR TestADRLinkLifecycle.test_adr_link_lifecycle 52e300ee Failed at the earlier DPS-primary link, before reaching the original secondary-Hub step. The persisted error remained outbound AdrMiNotAuthorized after the waited recovery exhausted its remaining 566 seconds.

The first storage retry at 52e300ee passed all six endpoint-property checks but hit the original 900-second whole-test deadline during the final endpoint deletion: setup took 602.13 seconds and the call had used 297.88 seconds. This exposed the missing allowance for the newly added fixture waits. Its failed receipt is preserved; the fresh 11fabc09 run above verifies the correction.

For the remaining ADR failure, the requested outbound UAMI remained persisted, and its exact-scope DPS Contributor grant was accepted at 22:12:41 UTC. The initial namespace PATCH was accepted at 22:12:49, followed by five tracked recovery PATCHes at 22:13:15, 22:14:22, 22:15:59, 22:18:37, and 22:21:15. The final error at 22:22:40 still denied the namespace identity read access to the linked DPS. The initial PATCH correlation is 7271ed0a-9eaa-4158-bf69-c12b65002d33; the last recovery correlation is cf35f4e2-2880-4b1b-8046-c1f4f7dc092d. This demonstrates persistent service authorization rejection within the existing budget, not proof of effective authorization or proof that a longer wait would resolve it. No further production change, broader retry classifier, or ADR timeout increase was made.

All attempts used Python 3.10, centraluseuap, canary ARM, isolated CLI profiles, and committed GA source. Session diagnostics did not alter responses or assertions. The initial four-case cohort has all 41 recorded ARM IDs independently absent; the storage-only retry also has every recorded ID independently absent. Baseline resources were preserved, native cleanup/teardown evidence retained, and temporary profiles removed.

Compatible fixes are also pushed to #935, ending at ac78916f. Preview-specific fresh-grant behavior is preserved; no preview live run was performed. Regular PR CI is green for GA 11fabc09 and preview ac78916f, separately from integration qualification.

Current qualification: three of the four focused cases have passed across the two recorded GA commits; this is not full-suite qualification. The remaining ADR authorization failure blocks the planned full GitHub rerun. No full workflow was dispatched after run 39099115825; its 160 passed / 4 failed / 1 interrupted / 6 unstarted accounting remains the latest complete-workflow result. The new HubData partition still requires a full live run.

Hangyi (HangyiWang) and others added 2 commits October 1, 2026 01:24
Generate ADR/CMS, Hub management, DPS management and DPS data clients for 2026-11-01, and Hub data clients for 2026-11-01-preview. Preserve modeless synchronous generation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port supported commands and tests from release/1.0.0-preview, apply the stable API exclusions and compatibility adapters, and document generation provenance and offline qualification. Preserve baseline Hub, DPS and Device Update behavior.

Validation: 9726 unit tests passed; Flake8 and Pylint passed; the 1.0.0 wheel and native CLI help were verified. No live integration qualification was performed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@HangyiWang Hangyi (HangyiWang) changed the title feat: prepare azure-iot 1.0.0 GA SDK and command surface feat: adr ns GA Oct 1, 2026
Hangyi (HangyiWang) and others added 5 commits October 1, 2026 04:12
Preserve the implementation plan and qualification report as local session artifacts only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore release ADU path handling and owned Hub fixtures with their regression tests. Adapt namespace cleanup and real SDK 404 proofs to supported GA children, keep preview-only Hub skips out of GA, and remove mechanical test artifacts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reuse release/1.0.0-preview at 734e82d, including concurrent owned Hub/DPS phases, ADR worker groups, locked installs, readiness recovery, and the missing CI regression coverage. Preserve GA SDKs, API pins, supported command scope, and existing cleanup safeguards.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore exact source parity for SAS phase tests and the DPS canary response URL while preserving GA API assertions and public endpoint defaults.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@HangyiWang Hangyi (HangyiWang) changed the title feat: adr ns GA feat: adr GA Oct 8, 2026
Port supported release-preview schema commands and test infrastructure. Preserve stable Hub and DPS command behavior, refresh Hub SDKs, and add offline and live-scenario regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@HangyiWang
Hangyi (HangyiWang) marked this pull request as ready for review October 9, 2026 23:04
Copilot AI balanced review requested due to automatic review settings October 9, 2026 23:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Link removal still performs an unconditional stale full-resource PUT that can overwrite concurrent namespace changes.

1 open finding
What changed in this PR

Prepares the Azure IoT extension 1.0.0 GA surface, regenerating ADR, IoT Hub, and DPS SDK integrations for API version 2026-11-01.

Changes:

  • Promotes supported commands from preview and updates generated modeless SDKs.
  • Adds authentication, validation, integration-target, and release safeguards.
  • Adds relative-path support for Device Update hash calculation.
File Description
setup.py Raises SDK dependency floors and marks the package stable.
README.md Documents 1.0.0 scope and installation requirements.
docs/​*, linter_exclusions.yml Updates installation and command-lint guidance.
scripts/​select-openssl.sh Selects OpenSSL 3 for macOS tests.
dev_requirements, CredScanSuppressions.json Updates test and security tooling.
.github/​workflows/​* Pins actions and updates CI/release qualification.
.azure-devops/​* Updates CLI setup and integration execution.
.github/​skills/​generate-typespec-sdk/​SKILL.md Updates SDK-generation guidance.
azext_iot/​sdk/​iothub/​** Replaces legacy Hub clients with modeless GA SDKs.
azext_iot/​sdk/​dps/​** Replaces legacy DPS clients with modeless GA SDKs.
azext_iot/​sdk/​deviceregistry/​** Adds GA ADR operation groups and API version.
azext_iot/​adr/​** Adds GA command arguments, routing, and endpoint persistence.
azext_iot/​iothub/​** Adapts Hub providers, authentication, and state handling.
azext_iot/​dps/​** Adds GA registration, enrollment, authentication, and CSR behavior.
azext_iot/​core/​** Adds DPS validation and modeless output transforms.
azext_iot/​deviceupdate/​commands_update.py Resolves relative paths before producing file URIs.
azext_iot/​{commands.py,constants.py,__init__.py,azext_metadata.json} Promotes extension metadata and command registration to GA.
azext_iot/​tests/​** Expands unit and integration coverage for the GA migration.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread azext_iot/adr/providers/link.py Outdated
@zzcodework

Copy link
Copy Markdown
Contributor

Azure SRE Agent - automated review

Reviewing full diff (head 11526e99)

Scope note (important): this PR is 522 files / +131847 / -33906. The bulk is regenerated SDK (azext_iot/sdk/iothub, azext_iot/sdk/dps, azext_iot/sdk/deviceregistry — 153 files) and test recordings/fixtures under azext_iot/tests/. I reviewed the hand-written, non-generated surface selectively — azext_iot/common/arm.py, azext_iot/common/embedded_cli.py, the new azext_iot/adr/providers/* modules (link_recovery.py, link_preflight.py, link_persistence.py, certificate_activation.py), azext_iot/iothub/providers/base.py, azext_iot/core/_validators.py, azext_iot/deviceupdate/commands_update.py, setup.py, and HISTORY.rst — and did not read the generated SDK, the recordings, or the full diff of the largest files (azext_iot/core/custom.py +816/-411, azext_iot/operations/dps.py +520/-312, azext_iot/adr/_help.py +955/-105, azext_iot/iothub/providers/state.py +303/-132). Treat the absence of findings in those files as "not reviewed", not "clean".

Blocking

None found in the reviewed surface.

Suggestions

  • azext_iot/common/embedded_cli.py — as_json() now calls raise_for_error() first, so a failed command raises the underlying service exception (get_error()) instead of the previous CLIInternalError. That is a better error, but it is a behavioral change for any caller that catches CLIInternalError around as_json(). I checked the two except CLIInternalError sites in the extension (azext_iot/common/utility.py, azext_iot/iothub/providers/helpers/edge_device_config.py) and neither wraps as_json(), so this looks safe in-repo — worth a conscious confirmation rather than an accident, and it is not called out in HISTORY.rst.
  • azext_iot/common/embedded_cli.py — _ensure_subscription now skips --subscription for the command groups {"account", "ad", "extension"} by taking command.lstrip().split(maxsplit=1)[0]. This is correct for every current in-repo caller (they pass unprefixed commands such as "account show", "group show --name ..."), but it silently stops matching if any future caller passes a leading az . Anchoring on a normalized first token, or asserting the no-az convention in the docstring, would make the invariant explicit.

Nits

  • azext_iot/deviceupdate/commands_update.py — Path(file_metadata.path).resolve().as_uri() fixes the relative-path crash, but resolve() also follows symlinks, so the emitted uri for a symlinked input no longer reflects the path the user passed. If only absolutization is wanted, Path(...).absolute() avoids that. Also, the from pathlib import Path is function-local while the module imports elsewhere are top-level.
  • scripts/select-openssl.sh — runs brew --prefix openssl@3 under set -euo pipefail, so on a non-macOS/non-brew machine it fails with a bare brew: command not found rather than a useful message. A short guard (command -v brew || { echo "macOS/Homebrew only"; exit 1; }) would make the intent obvious.

Things I specifically checked and found correct

  • adapt_modeless_lro_poller relies on NameError.name (Python 3.10+); setup.py declares python_requires=">=3.10", so this is fine.
  • IotHubProvider._begin_hub_update does pass **hub_etag_arguments(self.hub_resource), so the message_endpoint / message_route read-modify-PUT path has optimistic concurrency. hub_description_for_write strips service-owned projections while preserving authorizationPolicies, so a full PUT does not drop SAS policies.
  • HISTORY.rst has a complete 1.0.0 (Unreleased) entry covering the API-version move, the dependency-floor bumps, and the excluded preview surfaces; setup.py dependency floors match what the entry claims.
  • link_recovery.py is carefully bounded: it only treats AdrMiNotAuthorized / a fully-matched LinkInitiateFailed envelope as recoverable, re-verifies grants twice, refuses to re-PATCH if the namespace changed, and preserves the original service error as the exception cause.

This is an automated review and may be incomplete; given the size of this PR it is explicitly a partial review of the non-generated surface. Please verify findings independently.

@HangyiWang Hangyi (HangyiWang) changed the title feat: adr GA feat: adr ga Oct 12, 2026
@HangyiWang Hangyi (HangyiWang) changed the title feat: adr ga feat: ADR GA Oct 12, 2026
Require response-header ETags for ADR unlink replacement writes, preserve ADU symlink URIs, retain original DPS registration diagnostics, and report missing Homebrew explicitly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 12, 2026 03:53
@HangyiWang

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review recommendations in 86969d5:

  • ADU calculate-hash uses absolute(), not resolve(), so relative inputs work without rewriting symlink URIs; added absolute/relative symlink regressions.
  • Kept EmbeddedCLI original-error propagation, documented it in HISTORY, and removed the two DPS integration diagnostic wrappers that masked the original failure.
  • Kept the documented no-leading-az invoke contract; no partial normalization was added.
  • Added an explicit missing-Homebrew diagnostic while retaining fail-closed OpenSSL capability checks.

Local validation: 3,040 relevant GA unit cases passed, flake8 passed, pylint 10/10, and all changed executable production Python lines were covered. Integration suites were not run.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Ordinary management routing violates the active-cloud contract, and malformed generated LRO callbacks raise NameError after successful operations.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread azext_iot/_factory.py
Comment on lines +131 to +132
get_adr_arm_endpoint(cli_ctx.cloud.endpoints.resource_manager),
api_version=_ADR_IOT_HUB_API_VERSION,
@zzcodework

Copy link
Copy Markdown
Contributor

Azure SRE Agent - automated review

Reviewing changes since 11526e99 (head 86969d5e, 1 commit, 10 files / +155 -33)

This increment answers the previous review's two nits and Copilot's unlink-concurrency comment. I verified each claim rather than taking the summary at face value.

Blocking

  1. azext_iot/adr/providers/link.py — the new If-Match protection is the same code that is also open as fix(adr): protect namespace unlink with response ETags #925 on release/1.1.0-preview; I reviewed it in detail there and the plumbing is correct (verified against this head's azext_iot/sdk/deviceregistry/operations/_operations.py: NamespacesOperations.get invokes cls(pipeline_response, deserialized, {}), and begin_create_or_replace pops headers and forwards it through _create_or_replace_initial to the request builder, so If-Match reaches the wire). Carrying the one finding that matters here:

    The guard raises whenever the GET response has no usable ETag header, and there is no fallback — the generated namespace schema declares no etag body property at all (0 occurrences in the whole NamespacesOperations span), which is why the body-etag path is explicitly rejected by test_delete_real_sdk_requires_header_even_when_body_has_etag. So if the RP does not emit that header, the entire az iot adr ns link hub|dps|su remove surface becomes unusable rather than merely unprotected. Every new test supplies the header itself (Mock return tuples or responses headers), so none of them is evidence about the live service.

    Your comment says "Integration suites were not run" — that is precisely the gap. azext_iot/tests/adr/test_adr_link_delete_int.py::test_adr_link_hub_dps_delete and ::test_adr_link_su_delete both drive the real link {kind} remove; please attach an ADR int run on 86969d5e (or on fix(adr): protect namespace unlink with response ETags #925's a1458be4, which carries identical code) before this merges. Note this branch's variant wraps the result in adapt_modeless_lro_poller(poller).result(), so the inline-200 path is exercised too.

Suggestions

  1. azext_iot/adr/providers/link.py — carried from fix(adr): protect namespace unlink with response ETags #925: the ETag is captured before ensure_unlink_reader(...), which is the slow step (role-assignment create plus propagation), and the precondition is then held across that whole window with retry_total=0 and no retry. Re-reading the namespace immediately before building the PUT body would shrink the race to body construction; preflight does not need the ETag. Your own test_delete_real_sdk_preserves_concurrent_changes_and_can_be_rerun models exactly this window by mutating inside ensure_unlink_reader. Also, the 412 surfaces as a bare HttpResponseError (the initial call's error_map covers 401/404/409/304 only), so the "inspect and rerun" remedy exists only in _help.py, not in the error the user actually sees.

  2. azext_iot/deviceupdate/commands_update.py — resolve() → absolute() is the right call for the symlink behaviour you wanted, and test_calculate_hash_preserves_symlink_uri locks both the relative and absolute inputs. One residual: Path.absolute() does not normalize .., so az iot du update calculate-hash --file-path sub/../payload.bin now emits file:///<cwd>/sub/../payload.bin (confirmed on CPython 3.12: Path('sub/../f.bin').absolute().as_uri() keeps the .., whereas Path(os.path.abspath('sub/../f.bin')).as_uri() collapses it). os.path.abspath normalizes without resolving symlinks, so it gives you both properties; worth it since this URI is written into an import manifest. The HISTORY.rst wording ("without resolving symlinks") is accurate either way.

Nits

None blocking in the rest of the increment.

Checked and clean

  • azext_iot/tests/dps/device_registration/test_iot_device_registration_{group,individual}_int.py — removing the try/except CLIInternalError → AssertionError wrappers is a strict improvement (the original service error now survives instead of being flattened into a generic message). I confirmed CLIInternalError has zero remaining references in both files at this head, so dropping the import is safe and will not trip flake8 or raise NameError.
  • scripts/select-openssl.sh — the command -v brew guard is placed before brew --prefix openssl@3, so it fires ahead of the first use and preserves the fail-closed capability checks below it. test_native_openssl_selection_explains_missing_homebrew proves the contract properly: it invokes bash by absolute path with PATH pointed at an empty tmp_path, and asserts exit 1, the message on stderr, and empty stdout — so a future regression that printed the path anyway would fail.
  • HISTORY.rst carries entries for both the ADR concurrency change and the ADU wording change; _help.py's remove text matches the new 412 behaviour.
  • The unit-test migration in test_adr_link_delete_unit.py is complete — every namespaces.get.return_value moved to the (resource, ETAG) tuple, the kwarg-set assertion was widened to include headers, and the real-SDK tests assert If-Match on the PUT.

Unrelated to this increment but still open: Copilot's inline comment on azext_iot/_factory.py about AZURE_IOT_ADR_ARM_ENDPOINT affecting non-ADR hub management calls (posted after this push). I am not restating it — flagging only that it is unresolved on the full diff.

Automated review — scoped to the increment only; the rest of this 522-file PR was covered by the previous partial review and is not re-reviewed here.

Freeze requested and observed provisioning fields individually so successful authorization recovery can populate omitted fields without hiding subsequent drift. Share the comparison with integration readiness checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 23:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Broad SDK, authentication, and release changes require human review; tests and live GA qualification were not performed.

1 open issue

Review effort: Balanced

@zzcodework

Copy link
Copy Markdown
Contributor

Azure SRE Agent - automated review

Reviewing changes since 86969d5e (head 6a2aa690, 1 commit, 5 files / +130 -12)

No blockers. 0 new suggestions.

This increment refactors LinkRecovery.inspect's endpoint comparison into a shared
endpoint_settings_match(actual, expected) helper (topology.py), applied to both the
self.expected and self.snapshot comparisons (previously snapshot used raw strict equality).
Traced the new leniency rule end to end: for non-Hub endpoint types it's still a plain
actual == expected after endpoint_update_body projection; for Hub it additionally allows
actual's provisioning dict to gain only availability/allocationWeight beyond what's
expected (actual_settings.keys() - expected_settings.keys() - {"availability", "allocationWeight"} must be empty) while every originally-requested key must still match. This
closes a real gap in the old code, which only checked expected_settings keys against actual
and silently ignored any extra/unknown key the service added.

Verified the 4 new unit cases match the implementation: non-dict/non-dict-coercible settings
([], "invalid") fail the isinstance guard; {"unknownSetting": 1} fails the new key-diff
guard; and the dps-kind test confirms the Hub-only leniency gate via endpoint_is_type(expected, IOT_HUB_ENDPOINT_TYPE) — a provisioning block newly appearing on a non-Hub endpoint still
hits the strict-equality branch and raises. No HISTORY.rst entry, consistent with this being an
internal fix to not-yet-shipped ADR GA code in this PR.

Automated review, unverified. 0 clean checks recorded in state.

Preserve every selected case and existing timeout while giving the two identity round trips an independent owned phase and 75-minute runtime budget.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 18, 2026 19:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Broad SDK/authentication changes and unresolved live qualification failures require final human review.

1 open issue

Review effort: Balanced

@zzcodework

Copy link
Copy Markdown
Contributor

Azure SRE Agent - automated review

No blockers. 1 suggestion on 52e300e (changes since cd9ab501, test-only).

  • iothub/conftest.py: provisioned_storage_with_identity_module now calls assign_role_with_propagation twice, so the two grants sleep 180s each (6 min serial) in module setup. Granting both first and sleeping once would keep the same guarantee and save ~3 min per module.

Checked: new entra-identities phase is wired through manifest, ci_budgets.json, shard-count tests and docs. Phases run concurrently, so the runner budget stays 75+15+5 min plus 15 setup, under the 120 min job timeout.

Automated review, unverified. 4 clean checks recorded in state.

Preserve the existing 900-second lifecycle allowance in addition to the two fresh-role propagation waits. Keep the shared phase and CLI timeouts unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 18, 2026 22:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Live qualification retains four failed cases and seven unqualified Hub data cases, requiring final human review.

1 open issue

Review effort: Balanced

Hangyi (HangyiWang) and others added 2 commits October 19, 2026 03:47
Align wait errors, ADR unlink completion and ETags, DPS certificate ETag lookup, authentication and key-output defaults, allocation validation, hostname discovery, and linked-hub no-op validation. Preserve GA SDK compatibility and reliability coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the device hostname when available and retain the classic fallback. Cover managed-identity create, conversion and reuse for both hostname shapes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 06:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Broad SDK changes and incomplete live qualification, including persistent ADR authorization failures, require human review.

1 open issue

Review effort: Balanced

Keep the real timer and handler assertions, observe two fresh ticks with a bounded wait, and cover deferred signal delivery explicitly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 07:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Broad SDK and authentication changes require human review; live qualification remains incomplete.

1 open issue

Review effort: Balanced

Hangyi (HangyiWang) and others added 2 commits October 19, 2026 18:08
Batch independent role grants, reuse the phase-owned Hub for read-only checks, skip private ARM-only data grants, and overlap acknowledged pool creates with bounded owned-resource readiness checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the offline receipt scope fixed while exercising CI default resource-group settings. Integration fixture code and the running live snapshot are unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 20:10
@HangyiWang

Hangyi (HangyiWang) commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor Author

IoT Hub fixture setup improvements and local results

Added the six audited setup improvements in fec6dbde, with an offline unit-fixture scope correction in c8efc831:

  • Batch caller-role grants before one existing 120-second wait and storage-role grants before one existing 180-second wait.
  • Skip the data-plane grant/wait only for exactly ARM-only isolated-Hub metadata.
  • Reuse the strictly validated phase-owned shared Hub for the missing-destination negative case and four read-only TLS/hostname cases.
  • Overlap independent two-Hub creates, then perform bounded GET-only reconciliation of their exact owned records.

Ownership, cleanup, authentication isolation, assertions, node IDs, readiness requirements, and explicit propagation windows remain intact. No production command changes, mutating retries, or ambiguous write replay were added.

Offline: 1,609 targeted unit/dependent tests passed at c8efc831; flake8, pylint, Python header checks, and diff checks passed.

Local live: 14/15 affected cases passed at frozen fec6dbdee8ff524c73d73015ae8fa5271853f8fc. The only subsequent source change is the two-line offline unit-fixture scope correction, not live integration behavior.

Cohort Result
Control-plane state 4/4 passed
Hostnames/TLS, utility, and ARM-only failover 6/6 passed
Data-plane state/missing-destination 3/3 passed
Isolated device identity 1/1 passed
Storage endpoint lifecycle 0/1: IH400117 during system-assigned storage endpoint authentication

Storage error evidence: 2026-10-19 18:27:42Z, correlation 4918ff5b-8a8f-466e-acca-0d40226f88ec. Setup and teardown passed; the case body failed. This error has occurred previously, but this run alone cannot rule out sensitivity to the incidental identity-aging time removed by batching. No propagation window was shortened or failure suppressed.

Cleanup: independent GET-only verification confirmed all 53 recorded resource/child IDs returned 404, the original protected baseline was preserved, no extra resources remained, and temporary Azure profiles were removed. The failed storage native receipt remains failed.

Measured overlap: Activity Log create intervals show 171.15 seconds of overlap for the control-state Hub pair and 168.84 seconds for the data-state pair, roughly 2m50s per pool. This is overlapping provisioning work, not measured full-workflow savings.

The adapted release/1.1.0-preview port is #937 and was also qualified locally: 1,530 targeted unit tests passed; 10/14 affected live cases passed, with four control-state cases blocked by one shared IH400117 setup failure. Its standalone storage case passed. All 38 preview recorded IDs were independently confirmed absent. Both live runs are focused/debug qualification, not full-suite qualification.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Broad SDK and authentication changes, incomplete live qualification, and remaining ADR authorization failures require human review.

1 open issue

Review effort: Balanced

@HangyiWang

Copy link
Copy Markdown
Contributor Author

GitHub CI is now green at the unchanged head c8efc831: run 39317067066, attempt 2 completed successfully with 63 jobs. All tests passed in the first attempt, but one macOS shard failed to upload its evidence because the artifact service DNS lookup returned ENOTFOUND. A single failed-job/dependent-gate rerun passed without source changes. The preview port, #937, is also green (23 jobs). Local integration qualification remains GA 14/15 and preview 10/14, with the recorded IH400117 failures and independently verified cleanup as documented above.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 22:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

ADR authorization failures and incomplete full-suite qualification require final human review.

1 open issue

Review effort: Balanced

This branch has not been deployed

No deployments
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.

3 participants