wordpress-charmers team mailing list archive

[Tom Haddon <tom.haddon@xxxxxxxxxxxxx>]

Some inline comments.

Diff comments:

> diff --git a/Dockerfile b/Dockerfile
> index 24b6b0b..7abcf33 100644
> --- a/Dockerfile
> +++ b/Dockerfile
> @@ -50,10 +50,22 @@ COPY --chown=www-data:www-data ./files/themes/ /var/www/html/wp-content/themes/
>  COPY ./files/wp-info.php /var/www/html/
>  COPY ./files/wp-config.php /var/www/html/
>  
> +# Copy our helper scripts into their own directory
> +COPY ./files/_add_option.php /srv/wordpress-helpers/
> +COPY ./files/_enable_option.php /srv/wordpress-helpers/
> +COPY ./files/_get_option.php /srv/wordpress-helpers/

If you had all these in the same directory you could just have one COPY line (same above for the ones you're putting in /var/www/html). Is there a reason not to do that, as it seems cleaner?

> +
> +# And their wrapper:
> +COPY ./files/plugin_handler.py /srv/wordpress-helpers/
> +RUN chmod 0755 /srv/wordpress-helpers/plugin_handler.py
> +
> +# And its service definition
> +COPY ./files/wordpress-plugin-handler.service /usr/lib/systemd/system/
> +
>  # entrypoint script will configure Wordpress based on env variables
>  COPY ./files/docker-entrypoint.sh /usr/local/bin/
> -
>  RUN chmod 0755 /usr/local/bin/docker-entrypoint.sh
> +
>  # Port 80 only, TLS will terminate elsewhere
>  EXPOSE 80
>  
> diff --git a/files/plugin_handler.py b/files/plugin_handler.py
> new file mode 100644
> index 0000000..1b23891
> --- /dev/null
> +++ b/files/plugin_handler.py
> @@ -0,0 +1,123 @@
> +#!/usr/bin/python3
> +
> +# get the env vars, if neither are set then nothing to do
> +# if either/both are set:
> +# # wait for localhost:80
> +# # set the first one
> +# # set the second one
> +# # delete ourselves and go away forever
> +
> +import logging
> +import os
> +import subprocess
> +import urllib.request
> +from time import sleep
> +from yaml import safe_load
> +
> +helpers_path = '/srv/wordpress-helpers'
> +install_path = '/var/www/html'
> +
> +logging.basicConfig(filename='/var/log/wordpress-plugin-handler.log', level=logging.DEBUG)
> +
> +
> +def call_php_helper(helper, stdin='', *args):
> +    path = os.path.join(helpers_path, helper)
> +    cmd = ['php', path]
> +    cmd.extend([str(arg) for arg in args])
> +    logging.info(cmd)
> +    process = subprocess.Popen(
> +        cmd,
> +        cwd=install_path,
> +        stdin=subprocess.PIPE,
> +        stdout=subprocess.PIPE,
> +        stderr=subprocess.STDOUT,
> +    )
> +    return process.communicate(stdin)[0]  # spit back stdout+stderr combined
> +
> +
> +def enable_plugin(*plugins):
> +    logging.info("Enabling plugins: {}".format(plugins))
> +    logging.info(call_php_helper('_enable_plugin.php', '', *plugins))
> +
> +
> +def get_option(key):
> +    value = call_php_helper('_get_option.php', '', key)
> +    return safe_load(value)
> +
> +
> +def add_option(key, value):
> +    # Ensure we don't overwrite settings
> +    if not get_option(key):
> +        logging.info('Adding option: {}'.format(key))
> +        call_php_helper('_add_option.php', '', key, value)
> +    else:
> +        logging.info('Option "{}" already in place, skipping.'.format(key))
> +
> +
> +def encode_team_map(team_map):
> +    # example: site-sysadmins=administrator,site-editors=editor,site-executives=editor

This really needs a unit test I think, if nothing else as a way of documenting the format we're expecting to see.

> +    team_map_lines = []
> +    i = 0
> +    team_map_lines.append("a:{}:{".format(len(team_map.split(','))))
> +    for mapping in team_map.split(','):
> +        i = i + 1
> +        team, role = mapping.split('=', 2)
> +        team_map_lines.append('i={};'.format(i))
> +        team_map_lines.append('0:8:"stdClass":4:{')
> +        team_map_lines.append('s:2:"id";')
> +        team_map_lines.append('i:1;')
> +        team_map_lines.append('s:4:"team";')
> +        team_map_lines.append('s:{}:"{}";'.format(len(team), team))
> +        team_map_lines.append('s:4:"role";')
> +        team_map_lines.append('s:{}:"{}";'.format(len(role), role))
> +        team_map_lines.append('s:6:"server";')
> +        team_map_lines.append('s:1:"0";')
> +        team_map_lines.append('}')
> +    team_map_lines.append('}')
> +
> +    return ''.join(team_map_lines)
> +
> +
> +def enable_akismet(key):
> +    enable_plugin('akismet/akismet.php')
> +    add_option('akismet_strictness', '0')
> +    add_option('akismet_show_user_comments_approved', '0')
> +    add_option('wordpress_api_key', key)
> +
> +
> +def enable_openid(team_map):
> +    encoded_team_map = encode_team_map(team_map)
> +    enable_plugin('openid/openid.php')
> +    add_option('openid_required_for_registration', '1')
> +    add_option('openid_teams_trust_list', encoded_team_map)
> +
> +
> +def wait_for_wordpress():
> +    url = 'http://localhost'
> +    sleep_time = 10
> +    success = False
> +    while success is not True:

I think it'd be worth having an upper limit on this (could be an arg to the function).

> +        try:
> +            response = urllib.request.urlopen(url, timeout=sleep_time)
> +        except Exception:
> +            logging.info('Waiting for Wordpress to accept connections')
> +            sleep(sleep_time)
> +        else:
> +            if response.status == 200:
> +                success = True
> +            else:
> +                logging.info('Waiting for Wordpress to return HTTP 200 (got {})'.format(response.status))
> +                sleep(sleep_time)
> +    return True
> +
> +
> +if __name__ == '__main__':
> +    wait_for_wordpress()
> +
> +    key = os.getenv('WP_PLUGIN_AKISMET_KEY')
> +    if key:
> +        enable_akismet(key)
> +
> +    team_map = os.getenv('WP_PLUGIN_OPENID_TEAM_MAP')
> +    if team_map:
> +        enable_openid(team_map)


-- 
https://code.launchpad.net/~barryprice/charm-k8s-wordpress/+git/wordpress-k8s-image-builder/+merge/380153
Your team Wordpress Charmers is requested to review the proposed merge of ~barryprice/charm-k8s-wordpress/+git/wordpress-k8s-image-builder:master into ~wordpress-charmers/charm-k8s-wordpress/+git/wordpress-k8s-image-builder:master.