lucasdiedrich / ojs

Open Journal Systems (OJS) is a journal management and publishing system.
GNU General Public License v3.0
19 stars 62 forks source link

Feature request : generate dynamically config.inc.php inside the container #17

Open Potomac54 opened 5 years ago

Potomac54 commented 5 years ago

Hello,

currently the config.inc.php is persistent and comes from the user, with a docker volume, see the docker-compose.yml :

volumes:

but we don't really need to use a volume, because a better solution is to generate dynamically the config.inc.php file from the config.TEMPLATE.inc.php provided in OJS source code, this generation will be done with the help of supervisord,

Just use sed commands to replace default settings in config.TEMPLATE.inc.php, by some settings defined in docker environment variables (from the Dockerfile) :

    SERVERNAME="localhost"      \
    OJS_BASE_URL="http://localhost/ojs"    \    
    OJS_DB_HOST="db"     \
    OJS_DB_USER="ojs"           \
    OJS_DB_PASSWORD="ojs"       \
    OJS_DB_NAME="ojs"           \
    OJS_SMTP_SERVER="personal.smpt.org" \
    OJS_SMTP_PORT="587" \
    OJS_SMTP_AUTH="tls" \
    OJS_SMTP_USER="my_login" \
    OJS_SMTP_PASSWORD="my_password" \
    OJS_ADMIN_NAME="admin" \
    OJS_ADMIN_PASSWORD="admin" \
    OJS_ADMIN_EMAIL="admin@admin.org" \

etc...

then create a bash script which purposes is to create dynamically config.inc.php, by using docker environment variables, this script will be launched from ojs-pre-start script, each time the OJS image is running :

cp /var/www/html/config.TEMPLATE.inc.php /var/www/html/config.inc.php chown apache:apache /var/www/html/config.inc.php

#sed operations
# set driver, host, username, password, database name in config.inc.php  
sed -i "s/driver = mysql/driver = mysqli/g" /var/www/html/config.inc.php
sed -i "s/host = localhost/host = ${OJS_DB_HOST}/g" /var/www/html/config.inc.php   
sed -i "s/username = ojs/username = ${OJS_DB_USER}/g" /var/www/html/config.inc.php
sed -i "s/password = ojs/password = ${OJS_DB_PASSWORD}/g" /var/www/html/config.inc.php
sed -i "s/name = ojs/name = ${OJS_DB_NAME}/g" /var/www/html/config.inc.php

# database charset
sed -i "s/database_charset = Off/database_charset = utf8/g" /var/www/html/config.inc.php

etc... (for each important option you have to do a sed command, with the appropriate docker environment variable)

and at the end, when all sed commands are done :

set installation on

sed -i "s/installed = Off/installed = On/g" /var/www/html/config.inc.php

We don't need to have a persistent config.inc.php file (because these settings will never change after a successful installation of OJS), all we need is to define environment variables related to OJS config options in Dockerfile, in order to create dynamically the config.inc.php at each run of OJS image, by using supervisord (ojs-pre-start script)

lucasdiedrich commented 5 years ago

Hi @Potomac54 ,

Actually, use external volume to map configuration files is the default behavior and should not ever be excluded, this approach is used by the most downloaded container, when you speak WE i think you`re only thinking on your use case, but mine for example, i like to use external volumes with replicated data.

And yes, this is an awesome request, i already thought about it over #4, but as i had lack of feedback for this, please feel free to make and PR.

Thanks.

marcbria commented 5 years ago

We don't need to have a persistent config.inc.php file (because these settings will never change after a successful installation of OJS), all we need is to define environment variables related to OJS config options in Dockerfile, in order to create dynamically the config.inc.php at each run of OJS image, by using supervisord (ojs-pre-start script)

@Potomac54 if we are talking about the first config.inc.php file, I like your proposal, but once is installed (even updated), this config need to be persistent. Lots of variables of this config file can be modified in production and we don't want them to change everytime the container is restarted.

Potomac commented 5 years ago

@marcbria @lucasdiedrich :
I added a pull request which implements automatic installation (no need anymore of OJS_CLI_INSTALL) and automatic recreation of the config file (no volume needed, just set docker environment variables for the customization of config.inc.php, and you can custom recreate_config script)

https://github.com/lucasdiedrich/ojs/pull/31/commits/cd4f3aa350538e8198f3662ae47f29326c214409

marcbria commented 5 years ago

As discussed in a former post with Lucas, I'm ok wiht the install var (OJS_CLI_INSTALL=1) in docker-compose if is keep to 0 in dockerfile. I'm unsure about the config.inc.php. In most of the cases you don't want to reset your config file every time you rise the docker and the config file is something you want to keep persistent, out of the container, so I will take a look to your suggestion, but I think is better keeping it as a volume.

@Potomac54 does it make sense to you? @lucasdiedrich what do you think?

Potomac commented 5 years ago

@marcbria ,

a volume for a config file seems to me overkill, because the content of the config.inc.php will likely never change, once it is configured during the installation of OJS,

with my solution if you want to modify the config file you have just to set new values to environment variables in your docker-compose.yml, and you can also add new sed commands in recreate_config script if new configuration options are available in futur versions of OJS

a volume is necessary for the database, the upload file directory (/var/www/files) and public directory (/var/www/html/public) because their content will change often during the life of the container (new submission file, image uploaded by the user when adding messages, new attached files, new reviews etc...)

but I understand your point of view, no problem if you want to keep the current situation (volume for config.inc.php file),

I think also that most of the settings present in config.inc.php could be stored in the database, I think there are too much settings in the config.inc.php, it would be interesting to ask to OJS developpers if they can reduce the number of options in the config.inc.php file, and move the less important options in the database ?

config.inc.php should just contain vital options like credentials for database, some web applications use a short config file (few options) and store the rest of the options in the database, what do you think ?

marcbria commented 5 years ago

Hi @Potomac54,

Thanks for your comments.

I'm full open to improvements, so let me insist on this point to be sure I'm not missing something.

If I catch you, the proposal in a generic sed/awk to create the config.inc.php file based on the variables of your .env file, isn't it? It means this sed need to escape those vars and syntax in the .env file will be really annoying or/and this sed need to be smart enough to deal with every charset and variable type.

And the same happens with php.ini or apache config files... so we will we need a generator for them?

I mean, from my experience, when testing/development, but even in production, config files (ojs, apache, php...) need to be changed more often than on will expect so this is why I think we need them to be handy. And config data is also something valuable to keep persistent.

What do you think?

My be, instead of keeping a volume for each config file we can copy them on each container restart from the ${PWD}/config/* folder?

In the other hand I full agree about the 3 volumes you mention. Thus are the ones that we have added to the docker-compose with comments.

About the config variables, I see benefits in keeping them in a file as well as in moving them to the database, but this discussion is not directly related with docker, so if you like to talk about this, please open a discussion in the PKP forum and we can follow the conversation there.

Potomac commented 5 years ago

@marcbria

not sure to understand your question, but I can explain again my solution, it uses environment variables you can see in Dockerfile, you can redefine them in your docker-compose.yml, just set the values you want :

        SERVERNAME="localhost"      \
        OJS_BASE_URL="http://localhost"    \             
        OJS_DB_HOST="db"     \
        OJS_DB_USER="ojs"           \
        OJS_DB_PASSWORD="ojs"       \
        OJS_DB_NAME="ojs"           \
        OJS_WEB_CONF="/etc/apache2/conf.d/ojs.conf" \
        OJS_CONF="/var/www/html/config.inc.php" \
        OJS_SMTP_SERVER="smtp.example.fr" \
        OJS_SMTP_PORT="587" \
        OJS_SMTP_AUTH="tls" \
        OJS_SMTP_USER="smtp" \
        OJS_SMTP_PASSWORD="password" \
        OJS_ADMIN_NAME="admin" \
        OJS_ADMIN_PASSWORD="admin" \
        OJS_ADMIN_EMAIL="myemail@email.fr" \
        OJS_PRIMARY_LOCALE="en_US" \
        OJS_SECONDARY_LOCALE="fr_FR" \
        OJS_TIMEZONE="Paris" \

you can change the default values in Dockerfile, or in your docker-compose.yml by adding the desired environment variable you want to set :

ojs:                    

    environment:  # 4 examples here of redefinition of environment variables
      SERVERNAME: ${PROJECT_DOMAIN}
      OJS_DB_HOST: 'db'
      HTTPS: "off"
      OJS_PRIMARY_LOCALE="en_US" \ 

and then the recreate_config script will use the values of these environment variables for doing the sed commands, no need to modify the recreate_config script, it's fully automatic,

every time the container is started your config.inc.php will be recreated with the values you set in these environment variables

Potomac commented 5 years ago

@marcbria : here is what recreate_config script does :

then several sed commands are done on /var/www/html/config.inc.php, by using environment variables defined in Dockerfile, in order to create a custom config, example :

    # set driver, host, username, password, database name in config.inc.php  
    sed -i "s/driver = mysql/driver = mysqli/g" /var/www/html/config.inc.php
    sed -i "s/host = localhost/host = ${OJS_DB_HOST}/g" /var/www/html/config.inc.php   
    sed -i "s/username = ojs/username = ${OJS_DB_USER}/g" /var/www/html/config.inc.php
    sed -i "s/password = ojs/password = ${OJS_DB_PASSWORD}/g" /var/www/html/config.inc.php
    sed -i "s/name = ojs/name = ${OJS_DB_NAME}/g" /var/www/html/config.inc.php

the recreate_config script : https://raw.githubusercontent.com/Potomac/ojs/php7-test/files/usr/local/bin/recreate_config

it's not a perfect solution because the script doesn't cover all options of config.inc.php, I cover the most important options, if you need to set an option non present in recreate_config then you have to add a sed command,

but for 95% of users the solution should be enough, the most important options in config.inc.php are :

by default I use utf-8 for charset, but the user can change to a different value in recreate_config script,

the main drawback of recreate_config is that you have to check if new configuration options are provided in config.TEMPLATE.inc.php each time a new version of OJS is released, and check also that the sed commands still work on new version of OJS,

instead of using sed commands : a more clean solution would be to have a cli utility, from OJS developpers, in order to generate a configuration file by passing arguments (environment variables), something like :

config_generate --db-name=$OJS_DB_NAME --db-password=$OJS_DB_PASSWORD --smtp-server=$OJS_SMTP_SERVER etc...

Potomac commented 5 years ago

@lucasdiedrich @marcbria : I opened a bug report in OJS github, in order to have a very short config.inc.php file, currently I feel that that 90% of options in this file could be easily moved to the database, most of these options don't have to be in a text file :

https://github.com/pkp/pkp-lib/issues/4687

marcbria commented 5 years ago

Hi @Potomac54

Thanks for your explanation. Comments under your comments:

it's not a perfect solution because the script doesn't cover all options of config.inc.php, ... but for 95% of users the solution should be enough

This is what I was worried about. The official dockerfile need to cover100% of the contexts... at last, the ones we can think about. I mean, we can not implement solutions that we know (by design) that won't work in some situations.

the main drawback of recreate_config is that you have to check if new configuration options are provided in config.TEMPLATE.inc.php each time a new version of OJS is released, and check also that the sed commands still work on new version of OJS,

I like the idea of a config-generator (I added a feature like this in my mojo & dojo scripts) but as an addition... I mean, right now, I think we need to keep the volume (or a copy of the local config.inc.php) as the first method and, if not found, go with the generator.

Talking about the generator, thanks for for your work, but please, take a look to @arturluizbr script: https://github.com/arturluizbr/docker-pkp-ojs/blob/master/files/config-creator.php

I see two main benefits over your approach:

What do you think?

instead of using sed commands : a more clean solution would be to have a cli utility, from OJS developpers, in order to generate a configuration file by passing arguments (environment variables)

I think this discussion also need to be taken in pkp's forum first. If they accept adding an script for this, we can call it from the dockerfile.

Potomac commented 5 years ago

Hello @marcbria

I like the idea of a config-generator (I added a feature like this in my mojo & dojo scripts) but as an addition... I mean, right now, I think we need to keep the volume (or a copy of the local config.inc.php) as the first method and, if not found, go with the generator.

your idea is interesting, we could define an environment variable in Dockerfile, like "OJS_USE_VOLUME_CONFIG", if it is set to "true" then the config generator step will be skipped, and the volume for config.inc.php will be used,

it requires that the user must add a line "volume" in his docker-compose.yml file for config.inc.php, if he wants to set to "true" this environment variable "OJS_USE_VOLUME_CONFIG",

note that if OJS developers decide in a future to simplify the config.inc.php file (by moving 99% of the options to the database) then the problem will be fixed, the config.inc.php will have just the settings for database and base url, which can make the config generator the best solution for handling ojs config, with environment variables

marcbria commented 5 years ago

Why not keeping it as now, with config files as volumes commented?