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

Make sure all contents in readme is included for secret management module #802

Closed
wants to merge 6 commits into from
Closed

Make sure all contents in readme is included for secret management module #802

wants to merge 6 commits into from

Conversation

chenrujun
Copy link

@chenrujun chenrujun commented Jan 11, 2022

2. Fix indent error of list in yaml file.
2. Add contents about different authentication method.
2. Add content about special characters in property name.
3. Add content about case-sensitive setting.
@chenrujun chenrujun self-assigned this Jan 11, 2022
@chenrujun chenrujun added the azure-spring All azure-spring related issues label Jan 11, 2022
@chenrujun
Copy link
Author

Hi, @backwind1233 , @saragluna , please help to review this PR when you have time.

@@ -56,23 +57,25 @@ spring:
endpoint: ${AZURE_KEYVAULT_ENDPOINT}
----

If your application is authenticated by other methods like Managed Identity or Azure CLI, properties like `tenant-id`, `client-id`, `client-secret` is not necessary. But if these properties are configured, then these properties have higher priority. Please refer to link:authentication.html[Authentication section] to get more information.
Copy link
Member

Choose a reason for hiding this comment

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

properties like tenant-id, client-id, client-secret is not necessary --> properties like tenant-id, client-id, client-secret are not necessary

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

@@ -56,23 +57,25 @@ spring:
endpoint: ${AZURE_KEYVAULT_ENDPOINT}
----

If your application is authenticated by other methods like Managed Identity or Azure CLI, properties like `tenant-id`, `client-id`, `client-secret` is not necessary. But if these properties are configured, then these properties have higher priority. Please refer to link:authentication.html[Authentication section] to get more information.

===== Java code

[source,java]
----
@SpringBootApplication
public class KeyVaultSample implements CommandLineRunner {

Copy link
Member

Choose a reason for hiding this comment

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

Have you tested it?

Copy link
Author

Choose a reason for hiding this comment

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

No, it's just code snippet, not project, no need to test, I think.

spring.cloud.azure.keyvault.secret.property-sources[].case-sensitive=true
----


=== Samples

Please refer to link:https://github.com/Azure-Samples/azure-spring-boot-samples/tree/spring-cloud-azure_4.0[azure-spring-boot-samples] for more details.
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to link the keyvault samples here.

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

private String secretNameInKeyVault2;
@Value("${secret-name-in-key-vault-both}")
private String secretNameInKeyVaultBoth;
@Value("${sampleProperty1}")
Copy link
Member

Choose a reason for hiding this comment

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

Why do you change the names here, if you are willing to do this change, will you also update the names in samples?
image

Copy link
Author

Choose a reason for hiding this comment

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


`.` is not supported in secret name. If your application have property name which contain `.`, like `spring.datasource.url`, just replace `.` to `-` when save secret in Azure Key Vault. For example: Save `spring-datasource-url` in Azure Key Vault. In your application, you can still use `spring.datasource.url` to retrieve property value.

===== Use property placeholders
Copy link
Member

Choose a reason for hiding this comment

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

An other question is,
I think ==== is for title, do you think we should use the same pattern(Style) for titles, refer here.
image

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

And I created an issue to update all titles: Azure/azure-sdk-for-java#26458

@chenrujun chenrujun closed this Jan 13, 2022
@chenrujun
Copy link
Author

Closing this PR, use #803 instead. #803 is targeting to 4.0.0-beta.3 version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
azure-spring All azure-spring related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants