Bridge azure properties to spring starter to authenticate #5

moarychan · 2021-07-23T01:56:24Z

Fixes Azure#21971

Based on the target PR changes, try to use each starter property class and the super AzureProperties to construct TokenCredential to authenticate.

Changelog;

Add a new ChainedTokenCredential bean, then it will be used as the final token credential to each service client builder.
- The first is each module's TokenCredential, it's created by SpringMappingCredentialPropertiesProvider.
  MappingCredentialPropertiesProvider.java interface to mapping each module properties to
  MappingCredentialPropertiesProvider implementation, then rach module will use SpringEnvironmentCredentialBuilder
  object to build a starter's TokenCredential which will be used as a credential option.
  Provider DefaultQueueClientBuilderCustomizer to cover the common use-cases, and combine the
- The second is the Azure Spring's common TokenCredential, it's created from AzureDefaultTokenCredentialAutoConfiguration auto-configuration.
Use Configurer and Customizer pattern to apply each module service client builder definition.
The abstract class AbstractClientBuilderConfigurer to customize the special configured.

Below interfaces are credential customizer:
- AzureKeyCredentialClientBuilderCustomizer
- ConnectionStringClientBuilderCustomizer
- SharedKeyCredentialClientBuilderCustomizer
- TokenCredentialClientBuilderCustomizer

Todolist:

saragluna · 2021-07-23T06:28:26Z

...utoconfigure/src/main/java/com/azure/spring/AbstractMappingCredentialPropertiesProvider.java

+
+    @Override
+    public void mapAzureProperties(
+        CredentialProperties credentialProperties,


Personally I don't like this code style.

saragluna · 2021-07-23T06:37:12Z

...toconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosAutoConfiguration.java

@@ -44,20 +46,44 @@ protected String getDatabaseName() {
    }

    @Bean
+    @ConditionalOnMissingBean


Do we really need this to be a bean?

I mean the source of this key could only be the CosmosProperties.

For @saragluna, we need to check the key rotation logic here.

saragluna · 2021-07-23T06:45:00Z

...configure/src/main/java/com/azure/spring/autoconfigure/storage/StorageAutoConfiguration.java

+    public BlobServiceClientBuilder blobServiceClientBuilder(
+        StorageProperties storageProperties,
+        ObjectProvider<MappingCredentialPropertiesProvider> mappingPropertiesProviders,
+        ObjectProvider<TokenCredential> defaultTokenCredentials) {


why is this called defaultTokenCredentials? Will there be multiple default token crdentials?

saragluna · 2021-07-23T06:46:05Z

...-autoconfigure/src/main/java/com/azure/spring/autoconfigure/unity/SpringAzureProperties.java

@@ -0,0 +1,4 @@
+package com.azure.spring.autoconfigure.unity;
+
+public interface SpringAzureProperties {


Why this empty interface?

Per discussion, maybe we could remove this now.

saragluna · 2021-07-23T06:53:10Z

.../azure/spring/autoconfigure/unity/identity/AzureDefaultTokenCredentialAutoConfiguration.java

@@ -24,6 +28,7 @@
 * Auto-configuration for Azure Spring default token credential.
 */
 @Configuration
+@ConditionalOnProperty(name = AzureProperties.PREFIX)


Do we really need to depend on this property bean?

No, I will remove it

saragluna · 2021-07-23T07:48:02Z

.../azure/spring/autoconfigure/unity/identity/AzureDefaultTokenCredentialAutoConfiguration.java

@@ -87,6 +92,12 @@ public TokenCredential azureTokenCredential(List<SpringCredentialBuilderBase> cr
        return chainedTokenCredentialBuilder.build();
    }

+    @Bean
+    public MappingCredentialPropertiesProvider cosmosCredentialPropertiesProvider(


What if service bus and cosmos work together?

saragluna · 2021-07-23T08:05:57Z

...toconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosAutoConfiguration.java

+            return cosmosClientBuilder.credential(azureKeyCredential);
+        }
+
+        MappingCredentialPropertiesProvider propertiesProvider = mappingPropertiesProviders.orderedStream()


How about using this EnvironemtCredential and the Azure Default Token Credential to construct a chained token credential?

saragluna · 2021-07-23T08:12:51Z

...loud-autoconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosProperties.java

@@ -20,7 +21,7 @@
 */
 @Validated
 @ConfigurationProperties(CosmosProperties.PREFIX)
-public class CosmosProperties extends AzureProperties {
+public class CosmosProperties extends AzureProperties implements SpringAzureProperties {


No need of SpringAzureProperties.

saragluna · 2021-07-23T08:19:48Z

...igure/src/main/java/com/azure/spring/cloud/autoconfigure/storage/AzureStorageProperties.java

 import javax.validation.constraints.Pattern;

 /**
 * @author Warren Zhu
 */
 @Validated
 @ConfigurationProperties("spring.cloud.azure.storage")
-public class AzureStorageProperties {
+public class AzureStorageProperties extends AzureProperties implements SpringAzureProperties {


No need to implement SpringAzureProperties.

saragluna · 2021-07-23T08:26:14Z

...in/java/com/azure/spring/cloud/autoconfigure/storage/AzureStorageQueueAutoConfiguration.java

-                                                        StorageAccountManager storageAccountManager) {
-
-        final String accountName = storageProperties.getAccount();
+    public QueueClientBuilderConfigurer queueClientBuilderConfigurer(


Do we need each new type of configurer for each sdk builder?

interface AzureKeyCredentialBuilderCustomizer { keyCredential(); } interface AzureNamedKeyCredentialBuilderCustomizer { namedKeyCredential; } interface AzureSdkClientBuilderCustomizer { tokenCredential(); httpClient(); } class KVBuilderCustomizer implements AzureKeyCredentialBuilderCustomizer, AzureSdkClientBuilderCustomizer, AzureNamedKeyCredentialBuilderCustomizer { tokenCredential(); keyCredntial(); httpClient(); configure() { } } class SBBuilderCustomizer implements AzureKeyCredentialBuilderCustomizer, AzureSdkClientBuilderCustomizer, AzureNamedKeyCredentialBuilderCustomizer { tokenCredential(); keyCredntial(); httpClient(); configure() { } }

saragluna · 2021-08-02T01:51:20Z

...toconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosAutoConfiguration.java

+    }
+
+    @Bean(COSMOS_CHAINED_TOKEN_CREDENTIAL_BEAN_NAME)
+    @ConditionalOnMissingBean


Will @ConditionalOnMissingBean check the bean name?

saragluna · 2021-08-02T01:56:24Z

...toconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosAutoConfiguration.java

+
+    @Bean(COSMOS_CHAINED_TOKEN_CREDENTIAL_BEAN_NAME)
+    @ConditionalOnMissingBean
+    public ChainedTokenCredential cosmosChainedTokenCredential(TokenCredential defaultTokenCredential) {


This TokenCredential could accidentally pick up the TokenCredential defined in other auto-configuration classes.

saragluna · 2021-08-02T02:03:31Z

...toconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosAutoConfiguration.java

-    public AzureKeyCredential azureKeyCredential() {
-        return new AzureKeyCredential(cosmosProperties.getKey());
+    @ConditionalOnMissingBean
+    public ClientBuilderCustomizer<CosmosClientBuilder> cosmosClientBuilderCustomizers() {


How about putting this into the builder creation method?

saragluna · 2021-08-02T02:17:26Z

...igure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosClientBuilderConfigurer.java

+    @Override
+    public CosmosClientBuilder configure(CosmosClientBuilder builder) {
+        configureClientBuilder(builder);
+        if (azureKeyCredentialCustomizer != null) {


How about omitting the DefaultSkipCredentialCallback and setting the credential by precedence, first setting the lower precedence and higher precedence.

stliu · 2021-08-03T02:42:21Z

...toconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosAutoConfiguration.java

+        return Optional.ofNullable(properties.getKey())
+                       .filter(StringUtils::hasText)
+                       .map(AzureKeyCredential::new)
+                       .orElse(null);


stliu · 2021-08-03T02:44:25Z

...toconfigure/src/main/java/com/azure/spring/autoconfigure/cosmos/CosmosAutoConfiguration.java

+        if (cosmosAzureKeyCredential != null) {
+            return builder -> builder.credential(cosmosAzureKeyCredential);
+        }
+        return null;


we should discuss this design, I'm really not sure if it is the right way to do this

btw, you can use ObjectProvider to get rid of this @Autowired(required=false)

saragluna · 2021-09-06T01:38:40Z

Already merged in this PR Azure#23787. Closing this one now.

App configuration4.0 sync repo

Moary Chen added 3 commits July 23, 2021 09:12

temp saved for cosmos, storage and keyvault

94353ba

merge branch

6ec0e05

use mapping properties provider to bridge

0e5ec4c

saragluna self-requested a review July 23, 2021 06:27

saragluna self-assigned this Jul 23, 2021

saragluna reviewed Jul 23, 2021

View reviewed changes

Moary Chen added 4 commits July 27, 2021 11:35

Extract the abstract interface to customize the client builder

92f0f85

test cosmos

6fdd070

test and fix generic type issue

d45335a

Add TokenCredentialCallback interface and default implementation

79749ea

saragluna requested changes Aug 2, 2021

View reviewed changes

Moary Chen added 2 commits August 2, 2021 19:05

Fixes comments and test storage with SP and cert file

548b429

Delete unused file

c332fd2

stliu reviewed Aug 3, 2021

View reviewed changes

Update configuration

d8e972e

saragluna closed this Sep 6, 2021

moarychan deleted the feature/unify-azure-spring-credential branch November 17, 2021 01:56

saragluna pushed a commit that referenced this pull request Feb 3, 2023

Merge pull request #5 from mrm9084/AppConfiguration4.0SyncRepo

3856e38

App configuration4.0 sync repo

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bridge azure properties to spring starter to authenticate #5

Bridge azure properties to spring starter to authenticate #5

moarychan commented Jul 23, 2021 •

edited

Loading

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

moarychan Jul 26, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Jul 23, 2021

saragluna Aug 2, 2021

saragluna Aug 2, 2021

saragluna Aug 2, 2021

saragluna Aug 2, 2021

stliu Aug 3, 2021

stliu Aug 3, 2021

stliu Aug 3, 2021

saragluna commented Sep 6, 2021

		@@ -0,0 +1,4 @@
		package com.azure.spring.autoconfigure.unity;

		public interface SpringAzureProperties {

Bridge azure properties to spring starter to authenticate #5

Bridge azure properties to spring starter to authenticate #5

Conversation

moarychan commented Jul 23, 2021 • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saragluna commented Sep 6, 2021

moarychan commented Jul 23, 2021 •

edited

Loading