Repository navigation
feat(adr): add policy revoke-issuer & device CRUD/revoke commands with restructured tests - #782
Hangyi (HangyiWang) wants to merge 40 commits into
Conversation
* Eventhub sdk add * monitoring changes * event changes * add discovery logic * remove unused code * test changes * Add fixes for integration tests * remove readme file * Remove uamqp usage in the code entirely * Replace uampq from docs to pyamqp * Fix failing unit tests * Fix args compatibility issue * Fix linting errors * Addressing comments * Fix linting issues * Addressed comments * Add right exception * remove unwanted exception --------- Co-authored-by: Ketki <ketkinaik@microsoft.com>
* fix: move base64 validation inside validate block for passthrough cases When parsing connection strings with redacted SharedAccessKey=*** values (e.g., from IoT Hub properties), the base64 validation would fail since the redacted value is not valid base64. Move the validation inside the 'if validate:' block so it only runs when full validation is requested. Passthrough cases that only need to extract other properties (like EntityPath, Endpoint) will no longer fail. * fix: skip base64 validation for redacted SharedAccessKey (****) values * Remove defensive check --------- Co-authored-by: Ketki <ketkinaik@microsoft.com>
* Create release data * Improve formatting and remove internal changes --------- Co-authored-by: Ketki <ketkinaik@microsoft.com>
* fix: win/MSI pipeline fixes * fix conditional compile-time/runtime logic issue * fix windows extension path * test: try to repro windows install issue by reordering pipeline stages * fixup * revert pipeline testing changes for now * revert separation of python setup and dev requirements for now
Co-authored-by: Ketki <ketkinaik@microsoft.com>
Co-authored-by: Ketki <ketkinaik@microsoft.com>
Co-authored-by: Ketki <ketkinaik@microsoft.com>
Co-authored-by: Ketki <ketkinaik@microsoft.com>
* Try requirements options * Add range --------- Co-authored-by: Ketki <ketkinaik@microsoft.com>
Co-authored-by: Ketki <ketkinaik@microsoft.com>
…ang/device-revoke
…ang/device-revoke
|
Copilot could you review the PR and list the advice/actions/TODOs one by one |
|
Hangyi (@HangyiWang) I've opened a new pull request, #783, to work on those changes. Once the pull request is ready, I'll request review from you. |
| @@ -0,0 +1,46 @@ | |||
| # coding=utf-8 | |||
There was a problem hiding this comment.
Are there async operations being used anywhere? If not why include anything from aio?
There was a problem hiding this comment.
Nope - let me remove all aio folder, we only consume operations
| ) | ||
| return wait_for_terminal_state(poller, **kwargs) | ||
| result = wait_for_terminal_state(poller, **kwargs) | ||
| return result.serialize(keep_readonly=True) if result else result |
There was a problem hiding this comment.
What is the reason for this?
There was a problem hiding this comment.
This was changed a while ago, I don’t recall the exact reason, but I believe I needed to serialize it at the time to get some tests passing. If there are any objections, we can revert it.
| try: | ||
| result = wait_for_terminal_state(poller, **kwargs) | ||
| except HttpResponseError as e: | ||
| # The backend returns 200 OK with an empty body when the LRO completes, |
There was a problem hiding this comment.
Has this been raised with the service team?
There was a problem hiding this comment.
Not yet - this is a workaround for a while also, let me see if I can reproduce it and file a bug to them.
| try: | ||
| return self.client.credentials.get(resource_group_name=resource_group_name, namespace_name=namespace_name) | ||
| result = self.client.credentials.get(resource_group_name=resource_group_name, namespace_name=namespace_name) | ||
| return result.serialize(keep_readonly=True) |
There was a problem hiding this comment.
Another usage of result.serialize I'd like to grok
There was a problem hiding this comment.
yeah I can't recall exact reasons
| if attributes is not None: | ||
| properties["attributes"] = attributes | ||
| if policy_resource_id is not None: | ||
| properties["policy"] = {"resource_id": policy_resource_id} |
There was a problem hiding this comment.
How many of the above properties can be deleted or empty? For example, tags. For those that can, we should ensure update can support that.
There was a problem hiding this comment.
sotags attributes os_version can be deleted, not sure about policy_resource_id
| @@ -0,0 +1,8243 @@ | |||
| # coding=utf-8 | |||
There was a problem hiding this comment.
Why are we using models in our own clients ?
There was a problem hiding this comment.
Because generated SDK methods require them as typed request bodies? Not necessary but it's more convenient pattern in other places.
| @@ -0,0 +1,30 @@ | |||
| # coding=utf-8 | |||
New Commands
az iot adr ns policy revoke-issuerRevokes the current CA (issuer) certificate on a credential policy and triggers ICA rotation. After revocation, a new CA certificate is generated automatically. All device credentials issued by the old CA must be re-issued.
For BYOR policies, revocation transitions the policy back to
PendingActivationwith a new CSR that must be re-signed and re-activated viaactivate-byor.az iot adr ns device listLists all devices in an ADR namespace.
az iot adr ns device showShows details of a specific device.
az iot adr ns device updateUpdates device properties including enabled state, policy association, OS version, and attributes.
az iot adr ns device revokeRevokes all active credentials for a device, forcing re-authentication.
Integration Test Restructuring
Refactored ADR integration tests from 3 monolithic files into 5 focused modules with shared helpers:
test_adr_crud_int.pytest_adr_infra_int.pytest_adr_device_int.pytest_adr_byor_int.pytest_adr_policy_revoke_int.py