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

Support external configuration files #422

Open
alexandre-touret opened this issue May 6, 2019 · 31 comments
Open

Support external configuration files #422

alexandre-touret opened this issue May 6, 2019 · 31 comments
Assignees
Labels
use case 💡 An issue which illustrates a desired use case for the specification
Milestone

Comments

@alexandre-touret
Copy link

alexandre-touret commented May 6, 2019

Hello,
In addition of this issue, it could be nice to support external configuration files. I usually externalize all the configuration files from WAR/JAR in order to handle multi environment deployments with the same artifact ( DEV -> PROD ).

For instance, when we run the application, I could apply the configuration files as following :

java -jar my_app.jar -Dconfig=microservice-config.properties

Thanks
Regards
Alexandre Touret

@pilhuhn
Copy link

pilhuhn commented May 6, 2019

Like for #418 please use a prefix of e.g. -Dconfig.fromFile and not only `-Dconfig´ in case this is implemented.

@dmlloyd
Copy link
Contributor

dmlloyd commented Sep 27, 2019

Relying on a system property can be problematic, if more than one application exists in the run time. Does the external file then apply to all configurations? How does one exclude it? Etc.

I think the container ought to be responsible for providing this feature because sometimes it will make sense and sometimes it will not.

@Emily-Jiang
Copy link
Member

We discussed this issue at today's hangout with @jmesnil @kenfinnigan. This sounds a good approach to feed in config properties at different stage.

@Emily-Jiang
Copy link
Member

What is the default ordinal for this file if the file itself does not define the property ordinal? @jmesnil suggested to use 400-1.
Thoughts?

@alexandre-touret
Copy link
Author

Hello,
Thanks for your feedback.

I have a question : Why 400-1 and not 500 ?

Regards

@dmlloyd
Copy link
Contributor

dmlloyd commented Oct 11, 2019

You want more space than just one to allow user-defined sources to be inserted in between config sources. The point of specifying spacing between config source ordinals in the specification is to do exactly that.

@dmlloyd
Copy link
Contributor

dmlloyd commented Oct 11, 2019

Also, this feature really can't be implemented until the above questions are answered.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Oct 11, 2019

Relying on a system property can be problematic, if more than one application exists in the run time. Does the external file then apply to all configurations? How does one exclude it? Etc.

If more than one apps for this server, this configuration should applicable to each app. Each application should create one config source object of course.

@dmlloyd dmlloyd added the use case 💡 An issue which illustrates a desired use case for the specification label Mar 4, 2020
@dmlloyd
Copy link
Contributor

dmlloyd commented Mar 4, 2020

We don't presently have a way for a configuration source to use other configuration sources to configure themselves. See #470 for more info.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Mar 5, 2020

I have a question : Why 400-1 and not 500 ?

Since system property is having the ordinal of 400, when you pass in the configsource via the command line, you might want to override the system properties. Personally, I think having the default ordinal of 500 makes sense. By the way, you can config this ordinal by adding a property of config_ordinal in the config source.

@Emily-Jiang Emily-Jiang added this to the MP Config 2.0 milestone Apr 2, 2020
@Emily-Jiang Emily-Jiang self-assigned this Apr 2, 2020
@Emily-Jiang
Copy link
Member

maybe use 450 as the ordinal_value to allow more config sources to squeeze in. How about -Dconfig.location=, which could be a file or a folder. Thoughts?

@Emily-Jiang
Copy link
Member

@LittleWing we discussed today. Will #418 solve your issue? If not, can you specify your use case clearly so that we can understand what you are trying to achieve?

@alexandre-touret
Copy link
Author

Hello @Emily-Jiang .
Thanks for your feedback.
From what I understood #418 only mentions internal configuration files. I think that project stages is an interesting feature. However, the use cas I described is to use external files (for instance provided by config maps in K8S).
In addition to your last comment , I guess that defining the property -Dconfig.location= as a file location could be a good idea.

I'm available for further informations if needed.

Regards

@radcortez
Copy link
Contributor

Hi @LittleWing

Not exactly. While #418, gives the idea to apply profiles to files, it wouldn't be specced that way because of obvious limitations. The current proposal is to support them at the configuration level, so this will allow you to specify any profile in any ConfigSource and not only file based config sources.

@alexandre-touret
Copy link
Author

OK got it.
It sounds good for me.

@radcortez
Copy link
Contributor

With the current proposal of #418, will it be enough for your use case @LittleWing ?

@alexandre-touret
Copy link
Author

If the #418 proposal enables using any ConfigSource even from external sources then I will be "enough".

Thanks

@radcortez
Copy link
Contributor

Yes, that the idea. Thank you :)

@smillidge
Copy link
Contributor

smillidge commented Apr 23, 2020

@LittleWing this is easily done with a custom ConfigSource that reads property files. Payara Micro supports that through --systemproperties <file-path>

I suppose a bigger question is whether we want to specify more config sources?

@OndroMih
Copy link
Contributor

@LittleWing, I don't see how #418 solves this? #418 adds an option to specify different values for different profiles for the existing config sources. It won't allow reading values from an external config file. For that, we would have to define an additional standard config source so that you don't have to rely on a non-standard config source like @smillidge described.

#418 only allows creating an additional properties file inside the app that would be used when a specific profile is selected. Alternative proposal would allow specifying profile-specific values in config sources. There are only 3 standard config sources: properties files inside the app, system properties and environment variables.

Or am I wrong and I just don't see how #418 solves your usecase? Can you explain?

@alexandre-touret
Copy link
Author

Hello,
Maybe I'm wrong, but @radcortez said in this comment that #418 could fit my needs.
If I can enable loading some configuration values from an external file ( for instance a file provided by a k8s config map) by enabling a profile or whatever, it's OK for me.
The payara micro feature described by @smillidge is good also.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Apr 24, 2020

If I can enable loading some configuration values from an external file ( for instance a file provided by a k8s config map) by enabling a profile or whatever, it's OK for me.

You can do it today by specifying config properties in k8s config map and directly map the configmap to env, which makes all config properties available to your microservices. e.g.

kubectl create configmap greeting-config --from-literal message=Hello

In k8s deployment yaml:

env:
- name: GREETING
valueFrom:
configMapKeyRef:
name: greeting-config
key: message

What I would also want to mention here. There is another support I would like to propose for #418 is to support config source overriden at a file level.
Considering you have a jar with META-INF/ containing the following 3 files:

microprofile-config.properties
microprofile-config-dev.properties
microprofle-config-live.properties

if mp.config.profile=dev, the file microprofile-config-dev.properties will be loaded while the other two will be ignored. In this way, end users can easily support different values for config properties based on the project profile.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Apr 24, 2020

I think what you proposed is to automatically load a config source without doing the service loader patter and directly load that all properties contained in a config source. At the moment, you can use -Dmy.config.prop1=blah, which allows you to add system properties on microservice startup.

You want to load a whole file and treat all properties inside the file as config properties instead of specifying individual config properties. @smillidge mentioned to load the files as system properties in Payara, we could also load the config source as a config source other than system properties with the specified config source ordinal.

@alexandre-touret
Copy link
Author

Hello,

I think what you proposed is to automatically load a config source without doing the service loader patter and directly load that all properties contained in a config source. At the moment, you can use -Dmy.config.prop1=blah, which allows you to add system properties on microservice startup.

For few properties it's OK, when you have several properties, it could be a nightmare to deal with.

You want to load a whole file and treat all properties inside the file as config properties instead of specifying individual config properties.
I "only" want that feature. As I said in the issue description, I would like to specify the config file directly from the command line:

java -jar my_app.jar -Dconfig=microservice-config.properties

@radcortez
Copy link
Contributor

To understand better, should a file passed in -Dconfig just overrides the location of microprofile-config.properties and still have the same ordinal in the config sources, or should it also override any property?

For what it seems, you want the ability to pass a file, but also override?

@alexandre-touret
Copy link
Author

Hello,
Frankly I didn't consider these constraints.
I only wanted to pass a file. However, I think that the ability to override a whole file or some properties is interesting.

Just as a comparison, the existing spring boot analog feature is really useful on a daily basis (for devs and ops).

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Jun 19, 2020

To understand better, should a file passed in -Dconfig just overrides the location of microprofile-config.properties and still have the same ordinal in the config sources, or should it also override any property?

From my understanding, -Dconfig=fully.qualified.propertie.file. It supplies a fully qualified property file and it has no clash with microprofile-config.properties. It just adds additional properties. The file itself has its own ordinal (containing config_ordinal property inside the property file).

For what it seems, you want the ability to pass a file, but also override?

It just provides a way to pass in the config file instead of creating service loader for it.

@Emily-Jiang
Copy link
Member

Emily-Jiang commented Jun 19, 2020

To summarise this issue, I would like to make the following proposal is:

  1. Reserve a special config property included.config.location with the fully qualified property file
  2. When a property is found, while the property can be specified as a system property or environment variable or a property in other config sources, MP Config automatically load that property file. The properties inside included.config.location will be merged into the parent. If it contains the same property as the parent property, the property inside included.config.location overrides the same property in the parent parent. If included.config.location contains config_ordinal, that property will be ignored.
    In this way, when you use java -jar myjar.jar -Dnested.config.location=/etc/myworld.properties, the properties inside that file will be treated as other system property, as it inherits the parent nature.

e.g.
system properties

java.version=1.8
os.name=Mac OS X
included.config.location=/etc/myExtraConfig.properties, /etc/myAnoterConfig.properties

/etc/myExtraConfig.properties

client.name=Bob
client.age=21

The system property config source will have the following properties

java.version=1.8
os.name=Mac OS X
client.name=Bob
client.age=21

@alexandre-touret does this solve your problem?

@alexandre-touret
Copy link
Author

Hello,
Sorry for the delay.

Thanks @Emily-Jiang for your proposal.
It solves my problem.

Regards

@jbee
Copy link
Contributor

jbee commented Aug 21, 2020

@Emily-Jiang you did not fully update your example in #422 (comment) consistently using included.config.location as the special property reserved for includes of one or more fully qualified files.

@Emily-Jiang
Copy link
Member

@jbee thanks, I have now updated all occurences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
use case 💡 An issue which illustrates a desired use case for the specification
Projects
None yet
Development

No branches or pull requests

8 participants