spring-cloud / spring-cloud-config

External configuration (server and client) for Spring Cloud
Apache License 2.0
1.96k stars 1.29k forks source link

Feature Request - Config Server returns a 400 if it cannot find a file of the application's name #1057

Open indraneelb1903 opened 6 years ago

indraneelb1903 commented 6 years ago

Consider the following scenario.

We have a single Config Server and about 30-40 different applications receive their configurations from the Config Server . Out of this about 8 applications have a Kafka Consumer running inside them , and 7 applications(consumers) are listening to a topic belong to the same consumer group. The name of the group is given by the property , lets say consume.group. As , this config is shared by 7 applications , we decide to put it in application.yml/properties for convenience . Now, the remaining(8th) application(consumer) is consuming messages from same topic but with a different consumer group name , so we decide to create a file called test8-dev.yml/properties(test8 is name of application and dev is profile). Here, we insert the property consume.group , so this value overrides the property in application.yml.

Now the issue which could happen in the above case is if the property spring.application.name is not given(human-error) or it is mispelled or the file test8-dev.yml/properties is not found(maybe accidently removed), is that the application by the name of test8 will consume belonging to a consumer group , it should not belong to and would try to accomplish some other goal. This will make the consumer group lose some messages as those messages are being consumed by an application(test8) for whom they are not meant for.

I decided to go with the Kafka Consumer example , to show the gravity of what the above situation could do.

I propose that we add an optional header in the Config Client when it makes a request , indicating that this application expects to get properties from a file by the application's name . In such a case ,If the config Server does not find a file by the name of application , it will return a 400(Bad request) with the error message. This would be better , as the application won't start because properties won't be found , and the person deploying would automatically know that something is wrong ( because the application fails to start ) and no damage would be done .

spencergibb commented 6 years ago

We already have a fail fast setting

indraneelb1903 commented 6 years ago

Isn't fail-fast property meant for when it cannot connect to the Config Server? . The above issue is not about connecting . The test8 application in the above example gets all the properties but not the one(consume.group) it needs to .

indraneelb1903 commented 6 years ago

I should have been more specific when I said application would not start in the last few lines , this is not because of fail-fast but because variables with @Value annotations won't be resolved ( as properties are not present in Spring's environment ) , as the Config Server returned no properties ( 400 error )

spencergibb commented 6 years ago

You want to require that a file has to be loaded by application name, the common files are not enough

indraneelb1903 commented 6 years ago

Yes because I want to override the property present in application.yml/properties .

indraneelb1903 commented 6 years ago

If the file by application name is not found because it is accidently deleted or name of application is mispelled or spring.application.name property is not specified , then return a 400.

indraneelb1903 commented 6 years ago

My bad, by mistake.

bzlat commented 6 years ago

I think this is already taken care of (see spring.cloud.config.server.accept-empty here spring-cloud-config.adoc and acceptEmpty in ConfigServerProperties and EnvironmentController).

spencergibb commented 6 years ago

Indeed, looks like the acceptEmpty property

indraneelb1903 commented 6 years ago

This is on the config server side , wouldn’t this apply for every config client? What if I want it selective?

indraneelb1903 commented 6 years ago

https://github.com/spring-cloud/spring-cloud-config/pull/960 Lets say your application's name is foo and profile is dev , so it expects properties from a file called foo-dev.properties. Looking at the pull request above , I don't think it will send a 404 if foo-dev.properties is not found , it will send 404 , if property sources are empty , that means it did not find anything from even the common(application.properties) config files . Feel free to correct me if i am wrong.

spencergibb commented 6 years ago

Ok, I'll reopen and wait for votes.