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

Wrong ordering of property sources in config client when using spring.config.import #2417

Closed
telperinion opened this issue May 16, 2024 · 5 comments · Fixed by #2437
Closed
Labels
Milestone

Comments

@telperinion
Copy link

Describe the bug
We are using a composite configuration at the config-server (at least 3 different backends(vault,file,git)) and have been moving from bootstrap configuration to import on the client side.

We are facing 2 issues:

  1. if the searchLocations in git for the configuration contains a "-default" (i.e. /configuration-default/{application}) the config client is using this as a priority property source and adding this even before an override repo.
  2. an -default in the file name (i.e application-default.yaml) has the same effekt rendering properties that are defined in a higher order repository (i.e. vault, or overrides) unusable in the application.

Is the 2. one intended or not (in https://docs.spring.io/spring-cloud-config/docs/current/reference/html/#config-data-import it is said that the default profile is used to load further profiles, but that the default profile has than precedence over the rest (even the override) is not documented there)?
If so, at least the first on is a bug in my eyes.

Sample
If needed(and somebody asks :) ) i will provide an sample project for this issue

@ryanjbaxter
Copy link
Contributor

Yes please provide a sample with example requests including that you expect the response and ordering of property sources should be.

@spring-cloud-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@telperinion
Copy link
Author

telperinion commented May 23, 2024

Sample Project can be found here: https://github.com/telperinion/spring-config-bug-mve

in this configuration the Config-Server returns the following json string:
{ "name": "config-client", "profiles": [ "master" ], "label": "default", "version": null, "state": null, "propertySources": [ { "name": "overrides", "source": { "test": "override" } }, { "name": "classpath:/primary/config-client/application.yaml", "source": { "test": "primary" } }, { "name": "classpath:/test-default/config-client/application.yaml", "source": { "test": "default" } } ] }

The 'test' property in the config-client-environment is 'default' not 'override' what would be expected.

@telperinion
Copy link
Author

telperinion commented Jun 7, 2024

the issue is connected to #2422 and so although to #2392

Unfortunatly we can't move to the solution of this issues as we cant move back to the bootstrap dependency

The first part of the issue is, that the profile determination of the propertie source is not specific enough (looking only for "_|-{profile}" in the propertie-source-name string will irregularly put sources with no profile to the profiled sources if the search path contains the pattern).

Second, if using the import dependency the override logic and the composite logic of the config server is disabled or completly deranged, this should be mentioned in the documentation (so i'm not really satisfied with this solution).
As for the override-logic there could be a quick fix, by just putting this source as profiled source to the top of the line.

For the rest it is really a problem. (for example not following a vault over git logic for the datasource configuration leading to production data in a test database)

@ryanjbaxter
Copy link
Contributor

The problem here two fold.

  1. The profile name (def) is a partial match to the -default in the property source name so we add the Option marking the property source as profile specific. This effects the way Boot orders the property source since profile specific property sources should be ranked higher.
  2. The overrides property source is never marked profile specific and IMO it should always be ranked profile specific so it it always ranked higher.

The logic in the PR is not perfect but honestly it can't be until we add this functionality #2291

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
3 participants