You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fortress@directory.apache.org by Jan Sindberg <js...@autorola.com> on 2016/01/12 13:14:12 UTC
Config - override properties bug?
In GlobalIds.java
public static final String TRUST_STORE = Config.getProperty( "trust.store" );
public static final String TRUST_STORE_PW = Config.getProperty( "trust.store.password" );
In Config.java:getExternalConfig()
...
config.setProperty( GlobalIds.TRUST_STORE, szValue );
...
By the time getExternalConfig is called, the fortress.properties has been read. This forces the code to evaluate TRUST_STORE = Config.getProperty( "trust.store" );
Now the value of some path becomes the key - the result is that an external value will never be used because nowhere in the code is the key known.
It should be fixed in GlobalIds so that the field is only the key, and all usage of this key (in ApacheDsDataProvider.java) should be replaced with
Config.getProperty("...");
There could be subtle issues with other static fields in GlobalIds which depends on Config.getProperty(...).
In our case we want to extend ConfigMgrImpl as a point for reading our own params. I know about fortress.user.properties, but one hardcoded file is too limited. We want to support multiple environments (for test, development and prod). All environments are started with an "autorola_env" system property. I can use that to read the right file or to use our config-tool which handles a special property-syntax that allows to set properties for multiple environments in one file. But with the current setup, the call to our ConfigMgrImpl comes in after GlobalIds have had its static initializers run (- and possibly too late for the DAO's too). I think that I will have to add a step similar to fortress.user.properties but with a dynamic filename. I guess I could fiddle with build and deploy to generate the right file at the right moment.
// Jan
Re: Config - override properties bug?
Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 12, 2016, at 1:42 PM, Jan Sindberg <js...@autorola.com> wrote:
>
>> Fra: Shawn McKinney [mailto:smckinney@apache.org]
>> Sendt: 12. januar 2016 15:54
>> Til: fortress@directory.apache.org
>> Emne: Re: Config - override properties bug?
>
>
> I have brought up two topics in this thread (I should remember not to have more than one pr. thread :-) ). The first I suspect is a bug: The documentation on configuration mentions that you can override trust.store and trust.store.pw through system properties - and the code shows intent of that but fails in actually supporting it. Can I add such findings to Jira directly? I hope that I will have time to propose a patch soon if we agree.
> I will repeat here for clarity:
>
> In GlobalIds.java
> public static final String TRUST_STORE = Config.getProperty( "trust.store" );
> public static final String TRUST_STORE_PW = Config.getProperty( "trust.store.password" );
>
> In Config.java:getExternalConfig()
> ...
> config.setProperty( GlobalIds.TRUST_STORE, szValue ); ...
>
> By the time getExternalConfig is called, the fortress.properties has been read. This forces the code to evaluate TRUST_STORE = Config.getProperty( "trust.store" ); Now the value of some path becomes the key - the result is that an external value will never be used because nowhere in the code is the key known.
>
> It should be fixed in GlobalIds so that the field is only the key, and all usage of this key (in ApacheDsDataProvider.java) should be replaced with Config.getProperty("..."); There could be subtle issues with other static fields in GlobalIds which depends on Config.getProperty(...).
Yes, please create a JIRA and we’ll address the issue there.
>
> On Jan 12, 2016, at 1:42 PM, Jan Sindberg <js...@autorola.com> wrote:
>
>
> The other topic is about supporting multiple environments (including developers locally). I will throw that into another thread.
Sounds good.
Thanks,
Shawn
SV: Config - override properties bug?
Posted by Jan Sindberg <js...@autorola.com>.
> Fra: Shawn McKinney [mailto:smckinney@apache.org]
> Sendt: 12. januar 2016 15:54
> Til: fortress@directory.apache.org
> Emne: Re: Config - override properties bug?
I have brought up two topics in this thread (I should remember not to have more than one pr. thread :-) ). The first I suspect is a bug: The documentation on configuration mentions that you can override trust.store and trust.store.pw through system properties - and the code shows intent of that but fails in actually supporting it. Can I add such findings to Jira directly? I hope that I will have time to propose a patch soon if we agree.
I will repeat here for clarity:
In GlobalIds.java
public static final String TRUST_STORE = Config.getProperty( "trust.store" );
public static final String TRUST_STORE_PW = Config.getProperty( "trust.store.password" );
In Config.java:getExternalConfig()
...
config.setProperty( GlobalIds.TRUST_STORE, szValue ); ...
By the time getExternalConfig is called, the fortress.properties has been read. This forces the code to evaluate TRUST_STORE = Config.getProperty( "trust.store" ); Now the value of some path becomes the key - the result is that an external value will never be used because nowhere in the code is the key known.
It should be fixed in GlobalIds so that the field is only the key, and all usage of this key (in ApacheDsDataProvider.java) should be replaced with Config.getProperty("..."); There could be subtle issues with other static fields in GlobalIds which depends on Config.getProperty(...).
The other topic is about supporting multiple environments (including developers locally). I will throw that into another thread.
Kind regards - Jan
Re: Config - override properties bug?
Posted by Shawn McKinney <sm...@apache.org>.
> On Jan 12, 2016, at 6:14 AM, Jan Sindberg <js...@autorola.com> wrote:
>
> In GlobalIds.java
> public static final String TRUST_STORE = Config.getProperty( "trust.store" );
> public static final String TRUST_STORE_PW = Config.getProperty( "trust.store.password" );
>
> In Config.java:getExternalConfig()
> ...
> config.setProperty( GlobalIds.TRUST_STORE, szValue );
> ...
>
> By the time getExternalConfig is called, the fortress.properties has been read. This forces the code to evaluate TRUST_STORE = Config.getProperty( "trust.store" );
> Now the value of some path becomes the key - the result is that an external value will never be used because nowhere in the code is the key known.
>
> It should be fixed in GlobalIds so that the field is only the key, and all usage of this key (in ApacheDsDataProvider.java) should be replaced with
> Config.getProperty("...");
> There could be subtle issues with other static fields in GlobalIds which depends on Config.getProperty(...).
>
> In our case we want to extend ConfigMgrImpl as a point for reading our own params. I know about fortress.user.properties, but one hardcoded file is too limited. We want to support multiple environments (for test, development and prod). All environments are started with an "autorola_env" system property. I can use that to read the right file or to use our config-tool which handles a special property-syntax that allows to set properties for multiple environments in one file. But with the current setup, the call to our ConfigMgrImpl comes in after GlobalIds have had its static initializers run (- and possibly too late for the DAO's too). I think that I will have to add a step similar to fortress.user.properties but with a dynamic filename. I guess I could fiddle with build and deploy to generate the right file at the right moment.
This subsystem (configuration) has been hard wired to pick up properties in the following order:
• fortress.properties file - found on the classpath of that name.
• Java system properties - to override any of the 13 properties listed above.
• LDAP configuration node - found by config coordinates set in the fortress.properties file itself.
The fortress.properties and ldap config node have been in use for some time. The system property usage has been added in the past year to allow deployers of fortress web and rest to deploy the war directly from maven repo and override its coordinates at last minute to work in local env. For example the war will have localhost:389 with uid of uid=admin with a pw of secret.
We would never run that way in production so this is a way to override those values w/out having to crack the seal on the .war file.
Your use case of having different sets of configurations for different types of envs could be solved by using the system properties.
For example you could set these system properties in your prod env:
fortress.config.realm=prod
fortress.host=prodhostname
fortress.port=389
fortress.admin.user=whatver
fortress.admin.pw=something
And these in test:
fortress.config.realm=test
fortress.host=testhostname
fortress.port=389
fortress.admin.user=admin
fortress.admin.pw=secret
and each env would have its own set of properties resident on a node on a particular server.
> On Jan 12, 2016, at 7:50 AM, Jan Sindberg <js...@autorola.com> wrote:
>
>> Fra: Jan Sindberg [mailto:jsi@autorola.com]
>> Sendt: 12. januar 2016 13:14
>> Til: fortress@directory.apache.org
>> Emne: Config - override properties bug?
>>
>> But with the current setup, the call to our ConfigMgrImpl comes in after GlobalIds have had its static initializers run (- and possibly too late for the DAO's too).
>> I think that I will have to add a step similar to fortress.user.properties but with a dynamic filename
> ...
> My bad, it comes before DAO's and ApacheDsDataProvider initialization - so my ConfigMgrImpl could work except for the GlobalIds.TRUST_STORE case mentioned.
It’s tricky how the config subsystem works which is one of its weaknesses. It is important to understand the capabilities and rationale, before we make any changes to how it works.
Having said that I don’t see the need to add a new properties file to the mix given that the same thing can be done using the configuration nodes and system properties. Have I missed something?
Thanks
Shawn
SV: Config - override properties bug?
Posted by Jan Sindberg <js...@autorola.com>.
> Fra: Jan Sindberg [mailto:jsi@autorola.com]
> Sendt: 12. januar 2016 13:14
> Til: fortress@directory.apache.org
> Emne: Config - override properties bug?
>
> But with the current setup, the call to our ConfigMgrImpl comes in after GlobalIds have had its static initializers run (- and possibly too late for the DAO's too).
> I think that I will have to add a step similar to fortress.user.properties but with a dynamic filename
...
My bad, it comes before DAO's and ApacheDsDataProvider initialization - so my ConfigMgrImpl could work except for the GlobalIds.TRUST_STORE case mentioned.
// Jan