Skip to content
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

The doc comment for Keyvault certificate client model CertificateContentType could be improved #4394

Open
ahsonkhan opened this issue Mar 1, 2023 · 1 comment
Assignees

Comments

@ahsonkhan
Copy link
Contributor

The use of getsecret doesn't seem appropriate in this doc. This applies to both C++ and even .NET.

One fix might be to just copy what's in the swagger, verbatim.
Or, maybe it is supposed to be GetCertificate since CertificateClient doesn't have a GetSecret anyway.

/**
* @brief Content type of the certificate when downloaded from getsecret.
*
*/
class CertificateContentType final {

cc @heaths, @gearama

Looks like this doc comment was incorrectly copied from .NET:
https://github.com/Azure/azure-sdk-for-net/blob/cb3e8630ab0df6609b5ff6643e5ea0700e4a9816/sdk/keyvault/Azure.Security.KeyVault.Certificates/src/CertificateContentType.cs#L10

Azure/azure-sdk-for-net#7530

Here's the corresponding swagger:
https://github.com/Azure/azure-rest-api-specs/blob/3e27c70e7c02c07b458bc0e94716c3d82d3fdd19/specification/keyvault/data-plane/Microsoft.KeyVault/stable/7.3/certificates.json#L1387

      "description": "The content type of the secret. eg. 'application/x-pem-file' or 'application/x-pkcs12', "

@schaabs and @vhvb1989 what is the correct/intended documentation here?

Originally posted by @ahsonkhan in #4255 (comment)

@heaths
Copy link
Member

heaths commented Mar 1, 2023

The comment is technically correct. When you download the managed secret - the backing certificate, if you will - that content type does describe whether it's a base64-encoded PFX or ASCII PEM file. But, true, the docs could be reformatted to actually say something like "when the managed secret is downloaded using a SecretClient".

heaths added a commit to heaths/azure-sdk-for-net that referenced this issue Mar 1, 2023
heaths added a commit to Azure/azure-sdk-for-net that referenced this issue Mar 1, 2023
heaths added a commit to Azure/azure-sdk-for-net that referenced this issue Mar 2, 2023
* Clarify CertificateContentType comment

Relates to Azure/azure-sdk-for-cpp#4394

* Use Key Vault stable/7.4 API version

Also includes re-recorded tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants