-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for Secure Key Release #23276
Conversation
…d required models for this new operations; modified other related models as well. Updated CHANGELOG.
...re-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyAsyncClient.java
Show resolved
Hide resolved
...keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyImportRequestParameters.java
Show resolved
Hide resolved
...urity-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyRequestAttributes.java
Outdated
Show resolved
Hide resolved
...ty-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/models/CreateKeyOptions.java
Outdated
Show resolved
Hide resolved
...keyvault-keys/src/main/java/com/azure/security/keyvault/keys/models/CreateRsaKeyOptions.java
Show resolved
Hide resolved
...urity-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/models/KeyProperties.java
Show resolved
Hide resolved
...ty-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/models/KeyReleasePolicy.java
Outdated
Show resolved
Hide resolved
@@ -2,6 +2,9 @@ | |||
|
|||
## 4.4.0-beta.2 (Unreleased) | |||
|
|||
### Features Added | |||
- Added `Exportable` and `ReleasePolicy` to `CreateKeyOptions`, `ImportKeyOptions`, and `KeyProperties` to support Secure Key Release for Key Vault and Managed HSM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you been able to get SKR working on AKV? It works on HSM, but when I try to create an exportable key -- on a premium SKU vault -- I get back this exception:
(BadParameter) AKV.SKR.1001: The exportable attribute is only supported with premium vaults and API version >= '7.3-preview'.
(This may be something to bring up with the service team, since my request met both the error response's criteria and still failed.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @maorleger or @heaths brought this up last week. I am currently writing some tests but I suspect I will see the same error message. I'll let you know what I actually get from the service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked about this directly with @mccoyp, but I'm putting it here for the record. Azure Key vault does not support releasing keys as of this moment but it's in the roadmap. Support for this feature is only available for Managed HSMs.
…a required constructor parameter on `KeyReleasePolicy`.
- Added an empty constructor to fix a serialization issue in KeyReleasePolicy. - Changed `vaultBaseUrl` to `url` in `KeyService.release()`. - Added ``.setReleasePolicy()` to `CreateEcKetOptions` and `CreateOctKetOptions`.
…elines from failing.
...re-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyAsyncClient.java
Show resolved
Hide resolved
...re-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyAsyncClient.java
Show resolved
Hide resolved
...re-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyAsyncClient.java
Outdated
Show resolved
Hide resolved
...t/azure-security-keyvault-keys/src/main/java/com/azure/security/keyvault/keys/KeyClient.java
Outdated
Show resolved
Hide resolved
...t-keys/src/main/java/com/azure/security/keyvault/keys/implementation/models/RandomBytes.java
Show resolved
Hide resolved
} | ||
|
||
/** | ||
* Set the {@link OffsetDateTime notBefore} UTC time. | ||
* Set the key size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No "in bits" like the other classes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch.
@MethodSource("getTestParameters") | ||
public void releaseKey(HttpClient httpClient, KeyServiceVersion serviceVersion) { | ||
// TODO: Remove assumption once Key Vault allows for creating exportable keys. | ||
Assumptions.assumeTrue(isManagedHsmTest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is client-side validation and you should let the service error. We don't want tight coupling between SDK and service (it's in our guidelines). Only in rare cases - like null checks for required URL path parameters - do we do client-side validation of service client methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do let the service error when it comes down to the public APIs. I added this to a test case just to avoid having it break the live tests. Should I remove it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend it, yes. If/when the service does change - especially unexpectedly - you'll know when it doesn't match the recordings, assuming you validate your request bodies like we do in .NET by default.
Create new version 2022-09-30-preview for MySQL (Azure#23276) * add base folder 2022-09-30-preview from 2021-12-01-preview * update version and add new file backupandexport.json * update readme * add common-types * fix readme * copy 2021-12-01-preview folder to init 2022-09-30-preview * update version, readme and add new api in FlexibleServers.json * fix spell check * fix linter * fix linter * fix linter for flexible server * add location in header to fix linter * add base for serviceoperation * update api version * add new api for service operation * update readme * fix model validation * fix prettier * update to v5 for apiVersionParameter and resourceGroupName * fix linter * fix lro errorResponse * enableScaling and revert configuration * fix enableStatusEnum * sync backupExport * update readme * remove service operation * update backups * udpate backup name * fix final-state-via * fix lro option * revert replication role * use cloudError * Update readme.python.md * fix customer word * add default value for EnableStatusEnum * add logondisk * fix description --------- Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com>
Will add tests and recordings soon.