-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix: Skip App Configuration validation when feature is disabled #47588
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
|
Hi, @mrm9084 , could you please help to review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where App Configuration validates connection strings even when spring.cloud.azure.appconfiguration.enabled=false, preventing applications from running tests or local development without providing unnecessary credentials.
Key changes:
- Added early check for the
enabledproperty inisResolvable()before validating store configuration - Added comprehensive test coverage for the disabled state
- Updated all existing tests to mock the enabled property check
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
AzureAppConfigDataLocationResolver.java |
Added early return in isResolvable() when enabled=false, preventing validation of connection strings |
AzureAppConfigDataLocationResolverTest.java |
Added new test for disabled state and updated all existing tests to mock the enabled property binding |
|
@copilot Update sdk/spring/CHANGELOG.md, following existing pattern in sdk/spring/CHANGELOG.md. |
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Plan to fix App Configuration validation when disabled
spring.cloud.azure.appconfiguration.enabledis falsevalidateAndInit()is called inAzureAppConfigDataLocationResolver.loadProperties()without checking if app configuration is enabledAzureAppConfigDataLocationResolver.isResolvable()to check if app configuration is enabled before validating store configurationSummary of Changes
The fix modifies
AzureAppConfigDataLocationResolver.isResolvable()to check thespring.cloud.azure.appconfiguration.enabledproperty before attempting to validate store configuration. Whenenabledisfalse, the resolver returnsfalseearly, preventing any validation of connection strings or endpoints.Key changes:
isResolvable()whenspring.cloud.azure.appconfiguration.enabledis falsetestIsResolvableWhenAppConfigurationIsDisabled()to verify the fixImpact:
spring.cloud.azure.appconfiguration.enabledis false, the application will no longer attempt to validate connection stringsOriginal prompt
This section details on the original issue you should resolve
<issue_title>[BUG] App Configuration spring.cloud.azure.appconfiguration.stores[0].connection-string is validated when spring.cloud.azure.appconfiguration.enabled is false</issue_title>
<issue_description>Describe the bug
spring.cloud.azure.appconfiguration.stores[0].connection-stringis validated whenspring.cloud.azure.appconfiguration.enabledis falseThe connection string, being a secret, is typically specified from outside (program argument, env var, etc).
When running tests, and when running the app locally with feature flags under
feature-management.feature-flags,spring.cloud.azure.appconfiguration.enabledis false and the connection string is left empty. The connection string should not have to be specified and should not be validated since it's not needed in these cases.Exception or Stack Trace