[BUG] Spring application.properties azure-spring-boot-sample-keyvault-certificates uses camel case. Others use hyphens

[edburns]

Describe the bug
Consider sdk/spring/azure-spring-boot-samples/azure-spring-boot-sample-keyvault-certificates/src/main/resources/application.properties. This file uses camel case for property names, while the other samples use hyphens in the same context.

[edburns]

Examples

            app.property("azure.activedirectory.client-id", clientId);
./azure-spring-boot-test-aad/src/test/java/com/azure/test/aad/groups/AADGroupsCountIT.java
            app.property("azure.keyvault.client-id", CLIENT_SECRET_ACCESS.clientId());
            app.property("azure.keyvault.client-id", CLIENT_SECRET_ACCESS.clientId());
./azure-spring-boot-test-keyvault/src/test/java/com/azure/test/keyvault/ActuatorIT.java

[edburns]

See also this related change in the end-to-end sample.

[edburns]

Hello @mnriem here is another question. Even with adjusting for using the hyphenated instead of camel case, I have a question about this property:

azure.keyvault.client-secret

Other places use client-key to represent this property. For example:

KeyVaultCertificatesEnvironmentPostProcessor.java
azure.keyvault.client-id=<the client ID with access to Azure Key Vault>
azure.keyvault.client-secret=<the client secret associated wit the client ID>
azure.keyvault.client-id=<the client ID with access to Azure Key Vault>
azure.keyvault.client-secret=<the client secret associated wit the client ID>
./azure-spring-boot-starter-keyvault-certificates/README.md
        testProperties.put("azure.keyvault.client-id", "aaaa-bbbb-cccc-dddd");
./azure-spring-boot/src/test/java/com/azure/spring/keyvault/KeyVaultEnvironmentPostProcessorTest.java
#azure.keyvault.client-id=put-your-azure-client-id-here
#azure.keyvault.client-key=put-your-azure-client-key-here
./azure-spring-boot-test-application/src/main/resources/application.properties
azure.keyvault.client-id=put-your-azure-client-id-here
azure.keyvault.client-key=put-your-azure-client-key-here
azure.keyvault.client-id=put-your-azure-client-id-here
./azure-spring-boot-starter-keyvault-secrets/README.md
azure.keyvault.client-id=put-your-azure-client-id-here
azure.keyvault.client-key=put-your-azure-client-key-here
azure.keyvault.client-id=put-your-azure-client-id-here
./azure-spring-boot-starter-keyvault-secrets/javadocTemp/README.md
azure.keyvault.client-id=put-your-azure-client-id-here
azure.keyvault.client-key=put-your-azure-client-key-here
azure.keyvault.client-id=put-your-azure-client-id-here
./azure-spring-boot-starter-keyvault-secrets/sourceTemp/README.md
            app.property("azure.keyvault.client-id", CLIENT_SECRET_ACCESS.clientId());
            app.property("azure.keyvault.client-key", CLIENT_SECRET_ACCESS.clientSecret());         

So might it not be better to do the same for certificates?

[edburns]

@jialindai wrote:

Hi Ed,

To use data binding, one simple approach is to use @ConfigurationProperties annotation, some explains can be found at:

https://www.baeldung.com/configuration-properties-in-spring-boot
https://www.baeldung.com/spring-enable-config-properties

The underlying API is data binder: https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/validation/DataBinder.html

It has one bind() API to bind PropertyValues into an object.

In general, the @ConfigurationProperties approach is preferred.

Regards,
Jialin

[edburns]

Hello @jialindai

Thanks for your suggestion about using @ConfigurationProperties. Due to the layered nature of this feature, with the proprietary Spring support on top of the standard JCA KeyStore, using @ConfiurationProperties will only benefit those using the feature from Spring. I think this is acceptable, but I want to explicitly mention it.

I am unsure if the approach you suggest of using @ConfigurationProperties is feasible. I would like @mnriem to offer his opinion. As far as I can tell, we must continue to use system properties directly because the underlying JCA KeyStore does not know anything about Spring, as it is a proprietary non-standard technology.

If this is true, I am happy to do the little bit of coding work.

Rationale

Here is my sketch of the proposed modifications to enable the tolerance
of property names supported by @ConfigurationProperties, as described
in the Baeldung article you cited.
With these modifications, we can support Spring property declarations
that treat the following as identical.

azure.keyvault.client-id
azure.keyvault.client_id
azure.keyvault.CLIENT_ID
azure.keyvault.CLIENT-ID
azure.keyvault.clientId
azure.keyvault.clientid

The JCA entry point will still use the single preferred usage of
azure.keyvault.client-id as a system property. They will not have the
benefit of this tolerance.

Proposed Modifications

Consider this code, in KeyVaultCertificatesEnvironmentPostProcessor.

String aadAuthenticationUrl = environment.getProperty("azure.keyvault.aad-authentication-url");
if (aadAuthenticationUrl != null) {
    systemProperties.put("azure.keyvault.aad-authentication-url", aadAuthenticationUrl);
}

I propose we use the @ConfigurationProperties mechanism to cause a
properties pojo bean, say certProperties to be made available to the
KeyVaultCertificatesEnvironmentPostProcessor and used like so:

if (null != certProperties.getAadAuthenticationUrl()) {
    systemProperties.put("azure.keyvault.aad-authentication-url", aadAuthenticationUrl);
}

with a similar usage for the other properties.

I would like comments from @jialindai and @mnriem on this before I proceed.

[jialindai]

@edburns, I think above is a good idea. It makes Spring config file more flexible in property names.

[mnriem]

@edburns Go for it. Make sure you test the variations ;)

[jialindai]

We may want to keep both camel case and hyphen case in system properties, so that we don't break customers using our beta version.

[edburns]

@jialindai We can keep this closed because I am working on it now in #18273 .