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

After upgrading spring-cloud-config-client from 4.0.2 to 4.1.0 and Spring Boot from 3.1.5 to 3.2.0 properties from application.yml are missing #2376

Closed
anvo1115 opened this issue Jan 18, 2024 · 16 comments

Comments

@anvo1115
Copy link

Initially I created an issue for spring-boot but their team asked to create it to spring-cloud.
spring-projects/spring-boot#39238

We upgraded our spring-boot-starter-web library from 3.5.1 to 3.2.0 version , spring-cloud-config-client from 4.0.2 to 4.1.0
and some functional stopped working.

  1. We have a spring.factories file where we register
    org.springframework.boot.BootstrapRegistryInitializer=com.my.configuration.CustomBootstrapper

  2. In CustomBootstrapper we have custom call for config-server data loading:


public class CustomBootstrapper implements BootstrapRegistryInitializer {

    public CustomBootstrapper() {
    }

    @Override
    public void initialize(BootstrapRegistry registry) {
        registry.register(ConfigServerBootstrapper.LoaderInterceptor.class,
                context -> new CustomInterceptor(new CustomDataLoader()));
    }

    @AllArgsConstructor
    static final class CustomInterceptor implements ConfigServerBootstrapper.LoaderInterceptor {
        private final CustomDataLoader customDataLoader;

        @Override
        public ConfigData apply(ConfigServerBootstrapper.LoadContext loadContext) {
            return customDataLoader.doLoad(loadContext.getLoaderContext(), loadContext.getResource());
        }
    }
 }

(3) In class CustomDataLoader we try to get custom property from application.yml :

    public class CustomDataLoader extends ConfigServerConfigDataLoader {
    private static final Log logger = LogFactory.getLog(CustomDataLoader.class);

    public CustomDataLoader() {
        super(destination -> logger);
    }

    @Override
    protected Environment getRemoteEnvironment(ConfigDataLoaderContext context, ConfigServerConfigDataResource resource, String label, String state) {
        Binder binder = context.getBootstrapContext().get(Binder.class);
        BindResult<String> propertyBindResult = binder.bind("my-property", String.class);
       // other code
     }

// other code

Unfortunately we got null in propertyBindResult. But previously it returned the value set in application.yml.

@ryanjbaxter
Copy link
Contributor

There are some changes we made which effect when getRemoteEnvironment is called and what configuration property sources are loaded.

I think this PR would actually give you the functionality you need.

Then you could do something like

ConfigServerConfigDataLocationResolver.PropertyResolver resolver = context.getBootstrapContext()
				.get(ConfigServerConfigDataLocationResolver.PropertyResolver.class);
		String myProperty = resolver.get("my-property", String.class, "default-value");

If you could build my branch and test this out using Spring Cloud 2022.0.5-SNAPSHOT that would go a long ways to making sure that PR addresses your use case as well.

@anvo1115
Copy link
Author

@ryanjbaxter ,
I tested your changes. Unfortunately I always get "default-value".
But we expected to get the value from application.yml file.

@ryanjbaxter
Copy link
Contributor

Here is a sample I used to confirm things worked...maybe I am not doing the same thing.

demo.zip

@anvo1115
Copy link
Author

@ryanjbaxter , thank you for the demo. it worked on my side, too.
I found that in my project I use spring-boot spring-boot 3.1.8 and you have 3.1.9-SNAPSHOT.
Did you make changes there , too?

I'm trying to set all the dependencies as in your project.

@ryanjbaxter
Copy link
Contributor

No that shouldnt matter

@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.

@spring-cloud-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@anvo1115
Copy link
Author

anvo1115 commented Feb 11, 2024

I found how to reproduce the issue in your Demo.

Add to your test class DemoApplicationTests


@SpringBootTest(properties = {
		"spring.config.import=optional:configserver:http://localhost:8065"
			})

Than you will see that the default value was returned instead of value from application.properties.

@ryanjbaxter
Copy link
Contributor

When you add the properties property to the annotation src/main/resources/application.properties is no longer included in the Environment, that is why default-value is returned. If you don't add that property to the annotation it works as expected.

@anvo1115
Copy link
Author

Is it expected behaviour? Earlier in spring-cloud-config-client 4.0.2 and Spring Boot 3.1.5 everything worked anyway.
If you consider that this is right, will you merge your fix into spring-cloud-config-client 4.1.x ?

@ryanjbaxter
Copy link
Contributor

The change is merged into 4.1.x at this point.

Is this the expected behavior for the @SpringBootTest annotation? I am not sure, that's a question for the Boot team, but that is what I observe happening.

@anvo1115
Copy link
Author

I created an issue for spring-boot: spring-projects/spring-boot#39940

@anvo1115
Copy link
Author

@ryanjbaxter ,
When are you planning to release 4.1.x ?
I can see that only 4.0.5 is released.

@ryanjbaxter
Copy link
Contributor

The current plan is to have a release on March 26th

https://github.com/spring-cloud/spring-cloud-release/milestones

@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.

@spring-cloud-issues
Copy link

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants