Properties not resolved on FactoryBean initialization

[stahloss]

I've been using the Spring Cloud Config Client to connect to a config server on PCF. Unfortunately I'm using some legacy code that loads a keystore in an afterPropertiesSet of an InitializeBean, which is also a FactoryBean.

I noticed that when using the Config Client, the properties are not resolved even when they're available in an application.properties. Strange thing is that one of the properties, location of type Resource is resolved.

My question is: Is this expected behavior and am I just missing something, or should all the properties be available when the bean is initialized? I'm guessing the latter so it seems like a bug to me and that's why I'm submitting it here.

Here's a link to a demo project illustrating the behavior:
https://github.com/D0rmouse/spring-cloud-config-1167

You'll see the following output when running the application:

Location: class path resource [somekeystore.jks]
Password: [$, {, s, s, l, ., k, e, y, s, t, o, r, e, ., p, a, s, s, w, o, r, d, }]
Type: ${ssl.keystore.type}

After removing the spring-cloud-starter-config dependency and running again the output will be:

Location: class path resource [somekeystore.jks]
Password: [c, h, a, n, g, e, i, t]
Type: jks

I would expect both outputs to be the same.

[ryanjbaxter]

I'm frankly a little confused about where ${ssl.keystore.password} and ${ssl.kystore.type} are coming from since the bean definition does not even contain these values.

[ryanjbaxter]

@spencergibb @dsyer any thoughts?

[dsyer]

I’d say it’s expected. Decryption has to be applied to application.properties so that keystore property has to be loaded earlier. Try bootstrap.properties. I’m pretty sure this is documented.

[dsyer]

Wait. That comment only applies to regular cloud context behaviour. This is a custom bean that defines a key store. I’m not sure it’s even anything to do with spring cloud.

[dsyer]

I'm frankly a little confused about where ${ssl.keystore.password} and ${ssl.kystore.type} are coming from since the bean definition does not even contain these values.

It’s in the XML (the bean definition). And it looks like it contains those placeholders.

[ryanjbaxter]

It’s in the XML (the bean definition). And it looks like it contains those placeholders.

@dsyer Now I feel like a dummy. My IDE was being smart and automatically put the correct values in the editor instead of showing me the real values. So I never saw the properties in the XML file. At least that makes more sense now :)

I’m not sure it’s even anything to do with spring cloud.

Without Spring Cloud Config on the classpath it works as expected.

It seems to have something to do with implementing FactoryBean, if the class does not implement that interface the properties seems to resolve fine.

[dsyer]

I see. It's this code in RefreshAutoConfiguration:

if (this.environment == null && registry instanceof BeanFactory) {
	this.environment = ((BeanFactory) registry)
			.getBean(Environment.class);
}

That looks like a really bad thing to do because getBean(Class) causes a huge instantiation cascade of all FactoryBeans. There must be a better way.

Of course it's not an issue in spring-cloud-config, but it is in spring-cloud-commons.

[stahloss]

Thanks so far. Setting the spring.cloud.refresh.enabled property to false or disabling the RefreshAutoConfiguration is a work-around as long as refreshing is not needed.

EDIT: This seems to only work for properties that are loaded from a local property source. Properties loaded from a Config Server will still not be resolved after disabling the refresh auto config.

[ryanjbaxter]

This only is a problem in Finchley, Edgware releases work fine.

[stahloss]

Disabling the RefreshAutoConfiguration does not resolve the issue for remote property sources, meaning the same bug seems to be present elsewhere in the software.

I tried spring boot 1.5 with Edgware and it also failed to resolve properties retrieved from a Config Server for FeactoryBeans when they were intialized.

I'll see if I can update the application this weekend to demonstrate the behavior.

EDIT: Added a Config Server to the demo project for easy testing. It's seems that disabling the refresh auto config does allow for correct remote property resolving, so I have to check my production setup to find out what could be causing them not to be resolved there...

[ryanjbaxter]

@D0rmouse see spring-cloud/spring-cloud-commons#436. You can try building and installing that branch and then using Finchley.BUIL-SNAPSHOT and seeing if it fixes the issue. It appears to work for me.

[stahloss]

I'm using io.pivotal.spring.cloud:spring-cloud-services-starter-config-client instead of org.springframework.cloud:spring-cloud-starter-config which seems to prevent me from disabling the RefreshAutoConfiguration on PCF, so I'm hoping your fix will work!

I'm from Europe, so it will be tomorrow before I'll test it. Will let you know asap.

[ryanjbaxter]

You will have to switch to the spring cloud jars and not scs.

[stahloss]

I can confirm the fix works on the demo project with the Spring Cloud as well as the io.pivotal dependencies.

When I deploy to PCF with the fix and use a PCF Config Server the properties are still not resolved when the FactoryBean is initialized.

Maybe I go for connecting to the Config Server manually using only Spring Cloud dependencies and see what happens.

EDIT: Below the warning message from the PCF app log:

WARN 23 --- [ main] o.s.b.f.s.DefaultListableBeanFactory : Bean creation exception on non-lazy FactoryBean type check: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'xxxx' defined in class path resource [xxxx]: Cannot create inner bean 'httpSenderBase$child#117159c0' of type [xxxx] while setting bean property 'messageSender'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'httpSenderBase$child#117159c0' defined in class path resource [xxxx]: Cannot resolve reference to bean 'httpSenderSchemeList' while setting bean property 'schemes'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'httpSenderSchemeList': Cannot create inner bean 'httpsScheme' of type [org.apache.http.conn.scheme.Scheme] while setting bean property 'sourceList' with key [0]; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'httpsScheme' defined in class path resource [generic-access-component-ssl-context.xml]: Cannot resolve reference to bean 'sslSocketFactory' while setting constructor argument; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'sslSocketFactory' defined in class path resource [generic-access-component-ssl-context.xml]: Cannot resolve reference to bean 'sslKeystore' while setting constructor argument; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'sslKeystore' defined in class path resource [generic-access-component-ssl-context.xml]: Invocation of init method failed; nested exception is java.security.KeyStoreException: ${ssl.keystore.type} not found

[ryanjbaxter]

I cant say for sure what it happening on PCF. In either case that is something you will need to talk to the SCS team about. Thanks for testing the fix!

[ryanjbaxter]

Fixed via spring-cloud/spring-cloud-commons#436

[stahloss]

There's something about this issue that keeps bugging me... I now think the fix is done in the wrong place, and here's why:

When you start the demo project with the config server (without @ryanjbaxter's fix), you can clearly see in the log that the properties are loaded before the FactoryBean is initialized.

So I think the real issue here is that these properties are not available to the context loading the FactoryBean, but they are actually already loaded and should be available. Fixing this seems like the right thing to do.
I'm not sure if this is techinically feasibly easiliy, because of lack of knowledge about internals on my part, but conceptually, it should work like this:

1. Load properties from either local or remote property source.
2. Initialize or update context that loads (Factory)Beans.

Now, all properties are available to the FactoryBeans and initialization should proceed without problems.
This would also fix any issues caused by external context update triggers, what is probably the cause of my application still being affected by the bug on PCF.

[dsyer]

I couldn't reproduce your issue with remote configuration and a FactoryBean so I'm not really sure what to make of it, and I doubt that this is the right place to start looking for fixes. Could you make a really simple sample app that shows the problem?

[stahloss]

When you take the sample application, start ConfigServerApplication and then start DemoApplication you'll see

c.c.c.ConfigServicePropertySourceLocator : Environment demoapp has 1 property sources with 3 properties.

before

com.example.demo.KeyStoreFactoryBean : Password: [$, {, s, s, l, ., k, e, y, s, t, o, r, e, ., p, a, s, s, w, o, r, d, }]

(of course, make sure @ryanjbaxter's fixed spring-cloud-context isn't on the classpath when running)

So the properties are loaded, but not available to KeyStoreFactoryBean when the afterPropertiesSet method is called.

This clearly illustrates the properties could be available to KeyStoreFactoryBean but somehow aren't.

I get that this may not be the place to address this issue, but what would be then? spring-framework?

Or should the advice be just to not use property driven FactoryBeans because there seems to be no way the system can guarantee the properties will be availble when they're loaded?

[dsyer]

If you don’t have Ryan’s bugfix how will we know this is a new issue? In any case we should move the discussion to spring-cloud-commons because that’s where all the features are implemented.

[stahloss]

I have a feeling you haven't read what I posted, please re-read my previous two posts. I'm not talking about a new issue, but rather a different solution to this one.

I'm saying that Ryan's fix does fix the bug, you can read I've tested it myself, but probably it could be fixed more genericly.

Now you've prevented an instantiation cascade, while it would be better to provide the properties to the AbstractFactory that instantiates the FactoryBeans, because clearly they are already loaded...

[dsyer]

I certainly did read everything you wrote, but now I'm just confused. You seem to be saying that even with the newest snapshots you are seeing a problem that I can't reproduce. Please open an issue in spring-cloud-commons so we can continue the analysis.

[stahloss]

No not with the newest snapshots... I'm talking about the unfixed situation and how the log output in that situation shows that it could be fixed in a different, more generic way,

[dsyer]

I'm not sure I follow then (and nothing in the logs jumps out at me as wrong). We definitely needed to fix that cascade anyway, so what's wrong with the solution we have?

[stahloss]

The logs (from the unfixed output) show me that properties are available before the FactoryBean is initialized, because they have been fetched from the Config Server, but somehow they are not available to the FactoryBean. This is wrong.

The cascade fix is ok for this scenario, but it doesn't fix the root issue, which is what I stated above. It should work even with the cascade, because the properties should already be available.

So chronologically the properties are fetched from the Config Server and the cascade is triggered, but the properties are not available to the FactoryBean. Why is that? They are fetched and available right, so they should resolve.

[ryanjbaxter]

You are welcome to open an issue for that problem in spring cloud commons, but since it works now with the code the way it is (which is a fix we want regardless of what you are saying is true or not), it will be a low priority for us.

[stahloss]

Wow that's really easy. So you probably didn't even run the app and see that the properties are loaded before the bean is instantiated. You don't even believe me...

Again: You 'fixed' the problem by preventing the cascade, but what you should have done is made sure the Factory instantiating the FactoryBean is aware of the properties that have already been fetched...

[ryanjbaxter]

Sorry we are very busy with many projects to maintain, we dont have time to run every single app and test every single thing. I did not say I didnt believe you, I was just instructing you what the correct course of action is to move forward.

[dsyer]

FWIW I ran it. I didn't see a problem. Seems like it's not really an issue to me. If you think it is then spring-cloud-commons is the place to discuss it.

[stahloss]

I understand you're busy and appreciate the instruction and I'll take it up with the commons people.

I don't understand how it is you think that loading FactoryBeans without resolved properties while they clearly can be is not really an issue.

Any cascade triggered from anywhere too early will become an issue until this is addressed, as I've already encountered.

[spencergibb]

Same folks, code just lives in Commons

[stahloss]

Hi,

I've tested with the following versions. The issue is not resolved.

• Spring Boot 2.1.1.RELEASE
• Spring Cloud Greenwich.M3

I read Greenwich.M3 should contain spring-cloud-commons 2.1.0.M2, in which the issue should be resolved right?
The demo project is updated.

I also the FactoryBean issue has become more of an issue in this release than it was before in my production project.

[ryanjbaxter]

It is in there
https://github.com/spring-cloud/spring-cloud-commons/blob/v2.1.0.RC2/spring-cloud-context/src/main/java/org/springframework/cloud/autoconfigure/RefreshAutoConfiguration.java

But as we said above this is not the repo this issue, please open an issue in spring cloud commons.

[stahloss]

Yes, but the demo project and the initial issue was submitted and solved here and this fix doesn't seem to work. This fix should at least fix the scenario in the demo project. It doesn't address the real issue, which is already created in spring-cloud-commons.

[spencergibb]

Then that is the issue to track