HuasoFoundries/phpPgAdmin6

Consider changing configuration format

nreynis opened this issue · 4 comments

Right now the configuration file is using php with nested array, this makes it hard to do automated installation / vm provisionning:

cp /usr/share/php-pg-admin-6/{config.inc.php-dist,config.inc.php}
sed -r -i "s/\\\$conf\['servers'\]\[0\]\['defaultdb'\] = 'template1';/\\\$conf\['servers'\]\[0\]\['defaultdb'\] = '${DB_NAME}';/g" /usr/share/php-pg-admin-6/config.inc.php

Would you consider using a format that is easier to manage like yml?

I can't speak for the developer, but I'd suggest leaving the primary configuration in PHP. When running phpPgAdmin in a web browser, the configuration will need to be parsed for each request. Using native PHP code allows its bytecode to be cached in memory. It would negatively affect performance if PHP had to read and parse some other format every time a request was made. As a compromise, a tool could be made to translate YML, INI or some other format to PHP code during deployment.

Lastly, a related improvement to the configuration would be to use constants instead of runtime variables. This would allow the values to be cached in memory and avoid any runtime overhead. PHP arrays can be made constant to get this benefit. However, the actual performance improvement of this change would be pretty minor. Example below.

const CONF = [
   'servers' => [
      [
         'desc' => 'Server Name',
         'host' => 'localhost',
         ...
      ],
      ...
   ]
];

I'd leave performance issues aside for they should be negligible.

I'd have no problem in supporting yaml, json or even ini files out of the box. However the thing I have been considering above others is to use a .env file, which would be considered only if present, and use environment vars otherwise.

This approach would let you deploy this app to heroku, for example, without having to add sensitive data to version control.

However, I'm not sure there is a sensible way to treat arrays using environment vars, and the config's file behavior is to allow declaration of N servers non deterministically.

Yet another solution would be to roll your own config file declaring a single $conf variable, which could be the result of parsing a file (for example, with json)

<?php

$conf = json_parse(file_get_contents('conf.json'), true);

Having a one liner configuration file referencing a hardcoded file would be easier to handle automated deploys. What do you think?

Having a .env file which fallback to environment variable is a great idea and would be the most practical and versatile solution!

I don't really think there's a solution to handle tree like structures (let alone graphs) in environment variables.

Concretly we would need to encode this:

  root                           
    |                            
    +--server_group_0            
    |    |                       
    |    +--desc                 
    |    |                       
    |    +--server_0             
    |    |    |                  
    |    |    +--desc            
    |    |    +--host            
    |    |    +--port            
    |   ...   +--sslmode         
    |    |    +--defaultdb       
    |    |    +--pg_dump_path    
    |    |    +--pg_dump_all_path
    |    |                       
   ...   +--server_N             
    |         |                  
    |        ...                 
    |                            
    +--server_group_N            
              |                  
             ...                 

The low hanging fruit is to serialize desc, host, port, ssl_mode and default_db in a pseudo JDBC string:
desc:ssl_mode://host:port/default_db

But the rest (dump_paths and group structure) is more tricky.

Also plugins can have their own config blocks.

environment variables cannot handle arrays. The only workaround would be to have it store a json_encoded string, which might grow up to be huge.