Wojciech, there was a lot of work in the configuration area in the Tigase server. I am not sure if this ticket is still relevant or if perhaps there is a better way already implement for this?

While there are significant changes to the configuration (DSL) this particular functionality is not implemented. I think that %andrzej.wojcik would be better person to comment on how this fits into DSL.

I was thinking about providing a way to pass variables to DSL as it is a very good idea in case of cluster node related settings.

I was also thinking about possibility to pass some variable defined in DSL so it could be reused - but later on after some adjustments in DSL this is not needed any more.

The ticket is 4 years old, so I do not think it is a high priority right now. However, I think it could be very useful as you say in clustered setup but in particular for VMs on cloud like AWS and similar. Lots of information about VM instance is passed through environmental variables so this would be a good way to automatically setup Tigase configuration and adjust it to the environment.

Andrzej, it was you who worked on DSL for Tigase? I am assigning this ticket to you then.

I added support for using environment variables and system properties in configuration file.

To set property username to value of environment value USER you can now use env() function

username = env('USER')

To use system property you need to replace env() function with prop() function. Additionally prop() function accepts second parameter for passing default value if system property is not set.

To fix this particular issue and use variables in strings, I introduced computable variables. So now to set path property to ${HOME_DIR}/log you can use following line:

path = env('USER') + '/log'

While already working on this I decided to allow simple computations in config file (ie. to make values set in milliseconds more readable).

Now to set property timeout to 5 minutes you can write:

timeout = 5L * 60 * 1000

instead of:

timeout = 300000L

I updated DSL description in documentation.

%wojtek Please review this changes in description of DSL and let me know, if this format and description is OK or if I should improve some parts.

I've tested on the latest version from @origin/master@:

wojtek@atlantiscity.local ~/dev/tigase/tigase-server $ git status
On branch master
Your branch is up-to-date with 'origin/master'.
wojtek@atlantiscity.local ~/dev/tigase/tigase-server $ git pull
Already up-to-date.
wojtek@atlantiscity.local ~/dev/tigase/tigase-server $ git log --oneline --max-count=1 | cat
855b087b include skip-distribution-deploy to tigase-server-documentation module as well

And it seems not working:

• config admins:

admins = [ env('HOST_ADMIN') ]

  5 admins = [
  6     env('HOST_ADMIN')
  7 ]

• config from example from documentation:

user {
    name = env('HOST_ADMIN')
    home = prop('user.home')
    paths = [ prop('user.home'), prop('user.dir') ]
}

205 user {
206     home = prop('user.home')
207     name = env('HOST_ADMIN')
208     paths = [
209         prop('user.home'),
210         prop('user.dir')
211     ]
212 }

Possible improvements

• explanation of the difference between system property and environment variable;

PS. during the upgrade repository URL is converted to:

dataSource {
    default {
        uri = 'jdbc:mysql://…'
    }
}

2017-02-06 19:04:01.007 [main]             AbstractBeanConfigurator.configure()  WARNING: Field 'default' does not exists in bean 'dataSource'. Ignoring!

Wojciech Kapcia wrote:

I've tested on the latest version from @origin/master@:

[...]

And it seems not working:

• config admins:

[...] result

[...]

• config from example from documentation:

[...] result:

[...]

If you are looking at dumps of configuration file then there may be an issue (have you check it in any other way?), because when I was implementing support for variables I implemented serialization of variables, so dump stores same value as an input file (generally I did that as I think that this way ConfigWriter could be used in future in read->modyfy->save workflow which would allow us to be able to modify configuration from code and store it to file which could be later reused by Tigase after server restart). However I forgot that same class is responsible for generation of dump files.

I suppose I should update code, so that dumps will have variables resolved to an exact values, right?

Possible improvements

• explanation of the difference between system property and environment variable;

hm, this is fact is different properties and environment variables as stated in documentation of JDK (I used methods of System class). Will need to add this to documentation.

PS. during the upgrade repository URL is converted to:

[...] which results in following error:

[...] adding () to default solves the issue.

I think it is only a warning in log files but will solve it as I will work on documentation improvements.

Please review my comments and let me know what you think.

Andrzej Wójcik wrote:

Wojciech Kapcia wrote:

I've tested on the latest version from @origin/master@:

[...]

And it seems not working:

• config admins:

[...] result

[...]

• config from example from documentation:

[...] result:

[...]

If you are looking at dumps of configuration file then there may be an issue (have you check it in any other way?),

I tested with VHosts by trying to connect however it looks like the issue may be related to how it's handled:

wojtek@atlantiscity.local ~/dev/tmps/tigase-server-dists/tigase-issue #7.2.0-SNAPSHOT-b4648 $ export HOST_HOSTNAME=atlantiscity ; echo ${HOST_HOSTNAME}
atlantiscity
wojtek@atlantiscity.local ~/dev/tmps/tigase-server-dists/tigase-issue #7.2.0-SNAPSHOT-b4648 $ export HOST_ADMIN="admin@atlantiscity" ; echo ${HOST_ADMIN}
admin@atlantiscity
wojtek@atlantiscity.local ~/dev/tmps/tigase-server-dists/tigase-issue #7.2.0-SNAPSHOT-b4648 $ ./scripts/tigase.sh stop etc/my_tigase.conf ; sleep 1 ; rm logs/* ; sleep 1 ; ./scripts/tigase.sh start etc/my_tigase.conf
Shutting down Tigase: 74444
1. Waiting for the server to terminate...
2. Tigase terminated.
Starting Tigase:
Tigase running pid=74569
wojtek@atlantiscity.local ~/dev/tmps/tigase-server-dists/tigase-issue #7.2.0-SNAPSHOT-b4648 $ telnet localhost 5222
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.



<stream:stream to="atlantiscity">
<?xml version='1.0'?><stream:stream xmlns='jabber:client' xmlns:stream='http://etherx.jabber.org/streams' id='tigase-error-tigase' from='atlantiscity' version='1.0' xml:lang='en'><stream:error><host-unknown xmlns='urn:ietf:params:xml:ns:xmpp-streams'/></stream:error></stream:stream>Connection closed by foreign host.

However other configuration (admins, admins = [ env('HOST_ADMIN') ]@) worked (different sets of commands for the non-admin user). It works for both single item and multiple items, however for multiple items it has to be in the form of @admins = env('HOST_ADMIN') (for the @export HOST_ADMIN="admin@atlantiscity,admin@firefly"@), otherwise it will throw exception:

2017-02-17 17:56:56.901 [main]             AbstractBeanConfigurator.configure()  WARNING: Can't prepare value of property 'admins' of bean 'message-router': '[tigase.conf.ConfigReader$EnvironmentVariable@2313db84]'
java.lang.IllegalArgumentException: tigase.util.TigaseStringprepException: Illegal characters in string, domain = atlantiscity,admin@firefly
        at tigase.kernel.DefaultTypesConverter.convert(DefaultTypesConverter.java:303)
        at tigase.kernel.DefaultTypesConverter.convert(DefaultTypesConverter.java:79)
        at tigase.kernel.DefaultTypesConverter.convert(DefaultTypesConverter.java:245)
        at tigase.kernel.beans.config.AbstractBeanConfigurator.configure(AbstractBeanConfigurator.java:115)
        at tigase.component.DSLBeanConfiguratorWithBackwardCompatibility.configure(DSLBeanConfiguratorWithBackwardCompatibility.java:48)
        at tigase.kernel.beans.config.AbstractBeanConfigurator.configure(AbstractBeanConfigurator.java:164)
        at tigase.kernel.core.Kernel.initBean(Kernel.java:130)
        at tigase.kernel.core.Kernel.getInstance(Kernel.java:371)
        at tigase.server.Bootstrap.start(Bootstrap.java:110)
        at tigase.server.XMPPServer.start(XMPPServer.java:138)
        at tigase.server.XMPPServer.main(XMPPServer.java:119)
Caused by: tigase.util.TigaseStringprepException: Illegal characters in string, domain = atlantiscity,admin@firefly
        at tigase.util.XMPPStringPrepSimple.nameprep(XMPPStringPrepSimple.java:47)
        at tigase.xmpp.BareJID.bareJIDInstance(BareJID.java:128)
        at tigase.xmpp.BareJID.bareJIDInstance(BareJID.java:98)
        at tigase.kernel.DefaultTypesConverter.convert(DefaultTypesConverter.java:136)
        at tigase.kernel.DefaultTypesConverter.convert(DefaultTypesConverter.java:79)
        at tigase.kernel.DefaultTypesConverter.convert(DefaultTypesConverter.java:245)
        at tigase.kernel.beans.config.AbstractBeanConfigurator.configure(AbstractBeanConfigurator.java:115)
        at tigase.component.DSLBeanConfiguratorWithBackwardCompatibility.configure(DSLBeanConfiguratorWithBackwardCompatibility.java:48)
        at tigase.kernel.beans.config.AbstractBeanConfigurator.configure(AbstractBeanConfigurator.java:164)
        at tigase.kernel.core.Kernel.initBean(Kernel.java:130)
        at tigase.kernel.core.Kernel.getInstance(Kernel.java:371)
        at tigase.server.Bootstrap.start(Bootstrap.java:110)
        at tigase.server.XMPPServer.start(XMPPServer.java:138)
        at tigase.server.XMPPServer.main(XMPPServer.java:119)

because when I was implementing support for variables I implemented serialization of variables, so dump stores same value as an input file (generally I did that as I think that this way ConfigWriter could be used in future in read->modyfy->save workflow which would allow us to be able to modify configuration from code and store it to file which could be later reused by Tigase after server restart). However I forgot that same class is responsible for generation of dump files.

For the dump file I think we should use effective values.

Also, except for the WebInstaller, I think we shouldn't touch the config file (and if we want updateable configuration I think we should lean towards db-stored config [there was a storage for that already]) which could be shared among nodes and updated by any of them… actually I think it would be quite good, same level of usefulness in cluster as with automatic cluster-nodes discovery... at the lowest we could pass only a main repository url and then rest of the config would be read from that repo... but I think I got carried away ;) ).

I suppose I should update code, so that dumps will have variables resolved to an exact values, right?

+1

Possible improvements

• explanation of the difference between system property and environment variable;

hm, this is fact is different properties and environment variables as stated in documentation of JDK (I used methods of System class). Will need to add this to documentation.

I know where it came from, but not every user of the Tigase has to be a Java developer and familiar with that hence including at least reference will be helpful.

Wojciech Kapcia wrote:

Andrzej Wójcik wrote:

Wojciech Kapcia wrote:

I've tested on the latest version from @origin/master@:

[...]

And it seems not working:

• config admins:

[...] result

[...]

• config from example from documentation:

[...] result:

[...]

If you are looking at dumps of configuration file then there may be an issue (have you check it in any other way?),

I tested with VHosts by trying to connect however it looks like the issue may be related to how it's handled:

[...]

I'm not sure why VHosts are not working. I think this property should work in a same way as other properties. Maybe it is somehow related to --vhosts being system property and is system property is used instead of configuration variable. Either way it is not different issue than support of variables in init.properties file.

However other configuration (admins, admins = [ env('HOST_ADMIN') ]@) worked (different sets of commands for the non-admin user). It works for both single item and multiple items, however for multiple items it has to be in the form of @admins = env('HOST_ADMIN') (for the @export HOST_ADMIN="admin@atlantiscity,admin@firefly"@), otherwise it will throw exception:

[...]

Yes, and it is correct as you cannot set a multiple items in a list at once - you need to provide a whole list. This works, so I think it is ok. (it works in same way as Groovy would work).

You should be able to use it in following ways:

admins = [ env('HOST_ADMIN') ]
admins = [ 'admin@examle.com', env('HOST_ADMIN') ]
admins = env('HOST_ADMIN')

where in first two cases you passes single JID and in last one comma separated list of JIDs.

because when I was implementing support for variables I implemented serialization of variables, so dump stores same value as an input file (generally I did that as I think that this way ConfigWriter could be used in future in read->modyfy->save workflow which would allow us to be able to modify configuration from code and store it to file which could be later reused by Tigase after server restart). However I forgot that same class is responsible for generation of dump files.

For the dump file I think we should use effective values.

Also, except for the WebInstaller, I think we shouldn't touch the config file (and if we want updateable configuration I think we should lean towards db-stored config [there was a storage for that already]) which could be shared among nodes and updated by any of them… actually I think it would be quite good, same level of usefulness in cluster as with automatic cluster-nodes discovery... at the lowest we could pass only a main repository url and then rest of the config would be read from that repo... but I think I got carried away ;) ).

Yes, it would be nice, but I wanted to have some kind of semi-automatic reconfiguration, so it would be possible to apply some changes to server configuration at runtime and later decide that it works fine and we want to keep them - and for that we would need possibility to save config without exact values.

I suppose I should update code, so that dumps will have variables resolved to an exact values, right?

+1

Done.

Possible improvements

• explanation of the difference between system property and environment variable;

hm, this is fact is different properties and environment variables as stated in documentation of JDK (I used methods of System class). Will need to add this to documentation.

I know where it came from, but not every user of the Tigase has to be a Java developer and familiar with that hence including at least reference will be helpful.

Added links to documentation related to system properties and environment variables in JDK documentation.

Andrzej Wójcik wrote:

Wojciech Kapcia wrote:

I tested with VHosts by trying to connect however it looks like the issue may be related to how it's handled:

[...]

I'm not sure why VHosts are not working. I think this property should work in a same way as other properties. Maybe it is somehow related to --vhosts being system property and is system property is used instead of configuration variable. Either way it is not different issue than support of variables in init.properties file.

I've run a quick tests and provided you with the steps to reproduce - I haven't went through the code trying to fix the issue.

However other configuration (admins, admins = [ env('HOST_ADMIN') ]@) worked (different sets of commands for the non-admin user). It works for both single item and multiple items, however for multiple items it has to be in the form of @admins = env('HOST_ADMIN') (for the @export HOST_ADMIN="admin@atlantiscity,admin@firefly"@), otherwise it will throw exception:

[...]

Yes, and it is correct as you cannot set a multiple items in a list at once - you need to provide a whole list. This works, so I think it is ok. (it works in same way as Groovy would work).

Well, Tigase is intended for all folks and expecting Groovy familiarity to setup a configuration file seems odd.

You should be able to use it in following ways:

[...]

where in first two cases you passes single JID and in last one comma separated list of JIDs.

IMHO should be explicitly stated in the documentation.

I suppose I should update code, so that dumps will have variables resolved to an exact values, right?

+1

Done.

Thanks

Possible improvements

• explanation of the difference between system property and environment variable;

hm, this is fact is different properties and environment variables as stated in documentation of JDK (I used methods of System class). Will need to add this to documentation.

I know where it came from, but not every user of the Tigase has to be a Java developer and familiar with that hence including at least reference will be helpful.

Added links to documentation related to system properties and environment variables in JDK documentation.

Thanks.

I've added information about usage of variables/properties with comma separated values to documentation to section about DSL.

Looks good.

Wojciech Kapcia wrote:

Andrzej Wójcik wrote:

Wojciech Kapcia wrote:

I tested with VHosts by trying to connect however it looks like the issue may be related to how it's handled:

[...]

I'm not sure why VHosts are not working. I think this property should work in a same way as other properties. Maybe it is somehow related to --vhosts being system property and is system property is used instead of configuration variable. Either way it is not different issue than support of variables in init.properties file.

I've run a quick tests and provided you with the steps to reproduce - I haven't went through the code trying to fix the issue.

While working on #5591 I've run into the problem with this hence re-openning the issue as the bit above slipped.

Basically using this functionality in system properties (with dashes) doesn't work. After running a couple of tests:

• using --virt-hosts = env("CONFIG_BASE_DIR") will result in having tigase.conf.ConfigReader$EnvironmentVariable in tigase/db/comp/ConfigRepository.java:68

• using --virt-hosts = 'localhost,a.localhost:clientCertCA=' + env("CONFIG_BASE_DIR") + '/certs/root_ca.pem:clientCertRequired=true' will result in having tigase.conf.ConfigReader$CompositeVariable in the same place.

Looks like we should either:

• retrieve the configuration option from the ConfigHolder/have it injected (but how? it's not exactly documented);

• set System property properly after parsing the value.

What do you think? I would say that going forward we should eliminate as much as possible using system properties?

I've added support for expanding variables to System properties.

I agree with you - we should stop using System properties. Everything with -- prefix should be gone!

Andrzej Wójcik wrote:

I've added support for expanding variables to System properties.

Thank you, works OK now.