Fix azure.scopes defaulting to Azure Global when cloud-type=azure_china/azure_us_government#48683
Fix azure.scopes defaulting to Azure Global when cloud-type=azure_china/azure_us_government#48683
Conversation
…government Agent-Logs-Url: https://github.com/Azure/azure-sdk-for-java/sessions/5f56cd61-af29-4585-a48b-d7bc109977a6 Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
|
|
||
| ### Bugs Fixed | ||
|
|
||
| - Fixed `azure.scopes` using wrong default value for Azure China and Azure US Government when `spring.cloud.azure.profile.cloud-type` is set to `azure_china` or `azure_us_government`. The scopes are now correctly derived from the merged cloud type. ([#44945](https://github.com/Azure/azure-sdk-for-java/issues/44945)) |
Agent-Logs-Url: https://github.com/Azure/azure-sdk-for-java/sessions/2f82f4f9-ea2e-46a7-a2fc-062b403c19bd Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes passwordless JDBC azure.scopes resolution so that when spring.cloud.azure.profile.cloud-type is set (e.g., azure_china / azure_us_government) and JDBC-specific scopes aren’t explicitly configured, the effective default scope is derived from the merged cloud type (instead of incorrectly defaulting to Azure Global).
Changes:
- Update JDBC passwordless properties so
getScopes()returns the raw (possibly null) configured value, and addgetEffectiveScopes()for cloud-aware defaults; ensure JDBC URL generation uses the effective scopes. - Adjust
copyAzureCommonPropertiesIgnoreNullto avoid overwriting target scopes when the source scopes are null. - Update/add tests for China/US Gov scope behavior and update changelogs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/spring/spring-cloud-azure-core/src/main/java/com/azure/spring/cloud/core/implementation/util/AzurePasswordlessPropertiesUtils.java | Adds null-guard around copying scopes during passwordless property merge. |
| sdk/spring/spring-cloud-azure-core/CHANGELOG.md | Notes the change in scopes merge behavior. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/passwordless/properties/AzureJdbcPasswordlessProperties.java | Separates raw vs effective scopes and ensures toPasswordlessProperties() uses cloud-aware default scopes. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/passwordless/MergeAzureCommonPropertiesTest.java | Adds merge tests validating JDBC effective scopes behavior. |
| sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/jdbc/JdbcPropertiesBeanPostProcessorTest.java | Fixes US Gov expected scope and adds Azure China scenario coverage. |
| sdk/spring/spring-cloud-azure-autoconfigure/CHANGELOG.md | Adds a bug-fix entry for the cloud-type-specific default scopes behavior. |
| if (source.getScopes() != null) { | ||
| target.setScopes(source.getScopes()); |
There was a problem hiding this comment.
copyAzureCommonPropertiesIgnoreNull calls source.getScopes() twice (once in the null-check and again when setting). If getScopes() is computed (as it is for some implementations), this can be redundant and can even produce inconsistent results if the computation is not purely deterministic. Consider caching the value in a local variable and reusing it for the null-check and assignment.
| if (source.getScopes() != null) { | |
| target.setScopes(source.getScopes()); | |
| String[] scopes = source.getScopes(); | |
| if (scopes != null) { | |
| target.setScopes(scopes); |
| - Fixed `azure.scopes` using wrong default value for Azure China and Azure US Government when `spring.cloud.azure.profile.cloud-type` is set to `azure_china` or `azure_us_government`. The scopes are now correctly derived from the merged cloud type. ([#47096](https://github.com/Azure/azure-sdk-for-java/issues/47096)) | ||
|
|
There was a problem hiding this comment.
| assertEquals("https://ossrdbms-aad.database.chinacloudapi.cn/.default", result.getEffectiveScopes()); | ||
| // toPasswordlessProperties should include the correct cloud-type-aware scope | ||
| assertEquals("https://ossrdbms-aad.database.chinacloudapi.cn/.default", | ||
| result.toPasswordlessProperties().getProperty("azure.scopes")); | ||
| assertEquals(AzureProfileOptionsProvider.CloudType.AZURE_CHINA, result.getProfile().getCloudType()); |
There was a problem hiding this comment.
This test uses the hard-coded property key string "azure.scopes". To reduce brittleness if the underlying key changes, prefer using the canonical constant (e.g., AuthProperty.SCOPES.getPropertyKey()) when reading from the Properties produced by toPasswordlessProperties().
| } | ||
|
|
||
| @Test | ||
| void testJdbcPropertiesGetCorrectScopeFromGlobalCloudType() { |
There was a problem hiding this comment.
The method name testJdbcPropertiesGetCorrectScopeFromGlobalCloudType is misleading: it sets the cloud type to AZURE_CHINA and asserts the China-specific scope. Renaming the test to reflect the scenario (e.g., China/global properties cloud type) would make the intent clearer.
| void testJdbcPropertiesGetCorrectScopeFromGlobalCloudType() { | |
| void testJdbcPropertiesGetCorrectScopeFromChinaCloudTypeInGlobalProperties() { |
| if (source.getScopes() != null) { | ||
| target.setScopes(source.getScopes()); | ||
| } | ||
| target.setPasswordlessEnabled(source.isPasswordlessEnabled()); |
There was a problem hiding this comment.
The new null-guard relies on source.getScopes() returning null when scopes were not explicitly configured. Some PasswordlessProperties implementations compute a non-null default in getScopes() when the backing field is null (e.g., AzureRedisPasswordlessProperties#getScopes), so merging can still overwrite the target’s cloud-type-aware default with an Azure-global scope when the per-service profile.cloudType isn’t set. Consider using a way to distinguish “explicitly set scopes” vs “computed default” (or aligning other implementations with the raw/effective scopes split used for JDBC).
AzureJdbcPasswordlessProperties.getScopes()to returnthis.scopes(null when not explicitly set)getEffectiveScopes()that returns cloud-type-aware default when scopes is not explicitly settoPasswordlessProperties()inAzureJdbcPasswordlessPropertiesto usegetEffectiveScopes()AzurePasswordlessPropertiesUtils.copyAzureCommonPropertiesIgnoreNullto only copy scopes if non-nullshouldGetCloudTypeFromAzureUsGovtest to expect the correct US Government scopeshouldGetCorrectScopeFromAzureChinatest for Azure China scope fixtestJdbcPropertiesGetCorrectScopeFromGlobalCloudTypeandtestJdbcPropertiesExplicitScopesOverridesDefaulttestsazure.scopesis wrong when setspring.cloud.azure.profile.cloud-type=azure_china#47096