[Key Vault] Create Security Domain library #37929

mccoyp · 2024-10-16T17:42:01Z

Description

This is a new library that is mostly generated directly from TypeSpec. The goal of this library is to provide a track 2 Security Domain client for the CLI, as well as external customers. There are currently no plans for shipping this library in other languages.

Initial preview timeline: public preview release by April 8th, 2025.

Customer library request: Azure/azure-cli#29998

All SDK Contribution checklist:

The pull request does not introduce [breaking changes]
CHANGELOG is updated for new features, bug fixes or other significant changes.
I have read the contribution guidelines.

General Guidelines and Best Practices

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

azure-sdk · 2024-10-16T18:09:01Z

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-keyvault-securitydomain

...yvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/operations/_operations.py

annatisch · 2024-10-16T21:54:43Z

...yvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/operations/_operations.py

+        """
+
+    @distributed_trace
+    def begin_download(


I'm rather confused by this method - it's very unusual to have a download function that is also a poller?
I see the _initial method is returning Iterable[bytes] like I would expect from a download, but this poller seems to return a result of None.....
If this method is not actually downloading any data locally, then we should probably find a different name.

The return type here actually seems to be a generation bug -- the poller is supposed to return a SecurityDomainObject (and the track 1 library that the CLI uses does this correctly). Azure PowerShell and the CLI even accept an output file parameter for this operation, so returning None is definitely unexpected. I'm looking into that now

After some more testing, this is a weird method. The initial response contains the SecurityDomainObject, even when get_download_status still returns "InProgress". The actual download operation doesn't seem to be an LRO, but the operation is marked as one because HSM activation, which is started by the operation, does take some time to complete.

This may have to do with this comment in the CLI's vendored package, stating the operation was apparently changed from returning a 200 to a 202 in the 7.2 API version: https://github.com/Azure/azure-cli/blob/424e80464cdba4c241fade9d966cce61ebe4513a/src/azure-cli/azure/cli/command_modules/keyvault/vendored_sdks/azure_keyvault_t1/v7_2/key_vault_client.py#L118-L122

The REST API documentation is correct -- the download endpoint just returns a value of the security domain. ~~To make this work with our LRO logic though, I think we'd have to make this return SecurityDomainOperationStatus, and add a value property to that model.~~ @heaths, @annatisch, what do you think?

EDIT: The get_download_status/downloadPending endpoint doesn't return a value containing the security domain -- it just returns the status of HSM activation. This operation wants to return the initial response as the final result once the status is "Success". I guess we still need a SecurityDomainObject return type, but I need to find a way to get this to work with TypeSpec...

I created a custom polling method to correctly handle this return type (and the non-standard "Success" status), and plugged it into an override for the begin_download operation: 4aac19c. The client method now agrees with the service specification and past behavior, though I'll need to sync with Laurent on whether there's anything we can do to correct the return type in the generated methods. Either way, we still need the custom polling to handle the LRO flow here.

...yvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/operations/_operations.py

annatisch · 2024-10-16T21:58:37Z

...yvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/operations/_operations.py

+        return deserialized  # type: ignore
+
+    @overload
+    def begin_upload(


Not entirely sure if upload is the best name here - again it's a bit weird to pair this with a poller, but I'm assuming the "upload" triggers some process that the poller then monitors?

For operation context, this uploads a security domain to an HSM. The CLI's documentation says this "start[s] to restore the HSM" -- the same command in Azure PowerShell uses Import as the verb. I don't have the full context, but it seems like the long-running process is restoring an HSM to some state

I found a good documentation page that walks through the process of recovering a lost HSM by uploading its security domain to a new HSM: https://learn.microsoft.com/azure/key-vault/managed-hsm/disaster-recovery-guide.

To upload a security domain, you need to have activated a previous HSM and downloaded its security domain. Then the transfer key is fetched from a new, inactive HSM -- which is used to encrypt the original security domain. The encrypted domain is can then be uploaded to the new HSM, activating it in the process. Testing it locally, the upload operation takes about a minute; the operation is considered successful once the HSM is active.

Based on that context, upload makes some sense as a verb. restore could possibly be an option, like the CLI documentation mentions, but restoring is already used as a verb in azure-keyvault-administration to refer to restoring the backed-up contents of an HSM. export/import or download/upload seem like the best options to me

...yvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/operations/_operations.py

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/aio/_client.py

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_patch.py

github-actions · 2025-01-10T05:00:43Z

Hi @mccoyp. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

xuzhang3 · 2025-02-06T05:27:16Z

@mccoyp any update of this SDK?

mccoyp · 2025-02-06T19:06:57Z

@xuzhang3 Our current estimate is that we'd like to release an initial Python preview in early April.

kristapratico · 2025-03-28T23:06:05Z

sdk/keyvault/azure-keyvault-securitydomain/README.md

+
+
+# Azure Keyvault Securitydomain client library for Python
+<!-- write necessary description of service -->


Looks like readme needs to be filled out

sdk/keyvault/azure-keyvault-securitydomain/apiview-properties.json

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_client.py

kristapratico · 2025-03-28T23:15:45Z

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_client.py

+    :paramtype api_version: str
+    """
+
+    def __init__(self, vault_base_url: str, credential: "TokenCredential", **kwargs: Any) -> None:


I noticed that other keyvault libraries use vault_url. Is there a reason this library should have a different name?

The user-facing client, SecurityDomainClient, exposes vault_url instead to match other libraries. Generated clients across KV consistently use vault_base_url because of the spec definition, but we always adjust this in the convenience layer.

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_internal/polling.py

kristapratico · 2025-03-29T01:59:11Z

...vault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_internal/async_polling.py

+PollingReturnType_co = TypeVar("PollingReturnType_co", covariant=True)
+
+
+class AsyncPollingTerminationMixin(AsyncLROBasePolling):


Do we inherit from AsyncLROBasePolling here b/c of finished? Could finished be defined on AsyncSecurityDomainDownloadPollingMethod and AsyncSecurityDomainUploadPollingMethod?

Since the polling methods that use this mixin already inherit from AsyncLROBasePolling as well, we could possibly drop this inheritance -- we would just need to implement _SansIOLROBasePolling.status here. I defined this in a mixin to share the functionality across the two polling methods that use it (the same goes for the AsyncNoPollingMixin)

...vault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_internal/async_polling.py

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_patch.py

kristapratico · 2025-03-29T02:13:42Z

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_patch.py

+        certificate_info_object: Union[CertificateInfoObject, JSON, IO[bytes]],
+        *,
+        content_type: str = "application/json",
+        polling: Optional[Literal[False]] = None,


I'm not sure if we've ever exposed polling as a public, documented keyword. Curious what our architects think.

Yeah, this is a main point where I think the design could be improved. If we don't split up begin_download into two methods (one for getting the security domain immediately, and one for polling on HSM activation) then I think we should probably change this to something like skip_activation_polling (or something with a better name). cc @annatisch

Update: I changed this to skip_activation_polling for now. Clearer on what the argument does, and doesn't expose the polling kwarg

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_patch.py

kashifkhan · 2025-04-09T18:34:49Z

sdk/keyvault/azure-keyvault-securitydomain/README.md

+
+#### Prequisites
+
+- Python 3.8 or later is required to use this package.


kashifkhan · 2025-04-09T19:37:51Z

sdk/keyvault/azure-keyvault-securitydomain/setup.py

+        "Programming Language :: Python :: 3.9",
+        "Programming Language :: Python :: 3.10",
+        "Programming Language :: Python :: 3.11",
+        "Programming Language :: Python :: 3.12",


py 313 will need to be added here :)

sdk/keyvault/azure-keyvault-securitydomain/README.md

mccoyp added KeyVault Client This issue points to a problem in the data-plane of the library. labels Oct 16, 2024

mccoyp mentioned this pull request Oct 16, 2024

[Key Vault] TypeSpec for Security Domain library Azure/azure-rest-api-specs#31060

Open

3 tasks

annatisch reviewed Oct 16, 2024

View reviewed changes

mccoyp force-pushed the kv-securitydomain branch from 98bf2c5 to ac1bf76 Compare October 23, 2024 06:33

evelyn-ys reviewed Nov 4, 2024

View reviewed changes

sdk/keyvault/azure-keyvault-securitydomain/azure/keyvault/securitydomain/_patch.py Outdated Show resolved Hide resolved

evelyn-ys mentioned this pull request Nov 4, 2024

{Keyvault} az keyvault security-domain: Migrate security domain to use track2 SDK Azure/azure-cli#30252

Merged

3 tasks

p3ck mentioned this pull request Nov 22, 2024

WIP - Add HSM support to Key Vault ansible-collections/azure#1717

Open

github-actions bot added the no-recent-activity There has been no recent activity on this issue. label Jan 10, 2025

mccoyp removed the no-recent-activity There has been no recent activity on this issue. label Jan 10, 2025

mccoyp added 16 commits March 24, 2025 21:59

Initialize package directory w/ tsp-location

672730b

Initial library generation

3ca9f25

Spec modifications

848f51a

Patch sync client for auth, send_request

5d230b0

Add _internal dir for auth support

bf46162

Custom sync poller

130e164

Regenerate w/ client customizations

afc1825

Correct return type, polling in begin_download

e241d3b

Override sync begin_download overloads

461a8e2

Add async patched client and polling

d485cd5

Fix PipelineClient references in send_request

5964dbe

Support upload polling

a088536

Simplify polling logic

65cd850

Return status from upload, None from download in gen client

e6d394a

Run black

f9b19bc

Rename generated client to avoid conflicts

a135c8d

mccoyp added 4 commits March 24, 2025 21:59

Accept polling=False to bypass polling

5ccd16f

Pylint

0b5fc0c

Custom NoPolling implementations for return types

6006a54

Generate on latest TSP

0e3b36c

mccoyp force-pushed the kv-securitydomain branch from 000f7af to 0e3b36c Compare March 26, 2025 00:48

mccoyp added 2 commits March 28, 2025 21:10

Re-implement polling for async

97e3fab

Pylint

0d9715d

kristapratico reviewed Mar 31, 2025

View reviewed changes

Address feedback

1fb3eb4

kashifkhan reviewed Apr 9, 2025

View reviewed changes

sdk/keyvault/azure-keyvault-securitydomain/README.md Outdated Show resolved Hide resolved

README, tests, samples, changelog date

5e20551

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Key Vault] Create Security Domain library #37929

[Key Vault] Create Security Domain library #37929

mccoyp commented Oct 16, 2024 •

edited

Loading

azure-sdk commented Oct 16, 2024 •

edited

Loading

annatisch Oct 16, 2024

mccoyp Oct 18, 2024

mccoyp Oct 23, 2024 •

edited

Loading

mccoyp Oct 24, 2024

annatisch Oct 16, 2024

mccoyp Oct 18, 2024 •

edited

Loading

mccoyp Oct 23, 2024

github-actions bot commented Jan 10, 2025

xuzhang3 commented Feb 6, 2025

mccoyp commented Feb 6, 2025

kristapratico Mar 28, 2025

kristapratico Mar 28, 2025

mccoyp Apr 3, 2025

kristapratico Mar 29, 2025

mccoyp Apr 3, 2025

kristapratico Mar 29, 2025

mccoyp Apr 3, 2025

mccoyp Apr 4, 2025

kashifkhan Apr 9, 2025

kashifkhan Apr 9, 2025



		# Azure Keyvault Securitydomain client library for Python
		<!-- write necessary description of service -->

		PollingReturnType_co = TypeVar("PollingReturnType_co", covariant=True)


		class AsyncPollingTerminationMixin(AsyncLROBasePolling):


		#### Prequisites

		- Python 3.8 or later is required to use this package.

[Key Vault] Create Security Domain library #37929

Are you sure you want to change the base?

[Key Vault] Create Security Domain library #37929

Conversation

mccoyp commented Oct 16, 2024 • edited Loading

Description

All SDK Contribution checklist:

General Guidelines and Best Practices

azure-sdk commented Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mccoyp Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mccoyp Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 10, 2025

xuzhang3 commented Feb 6, 2025

mccoyp commented Feb 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mccoyp commented Oct 16, 2024 •

edited

Loading

azure-sdk commented Oct 16, 2024 •

edited

Loading

mccoyp Oct 23, 2024 •

edited

Loading

mccoyp Oct 18, 2024 •

edited

Loading