wordpress-charmers team mailing list archive

[Thomas Cuthbert <thomas.cuthbert@xxxxxxxxxxxxx>]

Thomas Cuthbert has proposed merging ~tcuthbert/charm-k8s-wordpress:xubuntu-container-fixes into charm-k8s-wordpress:master.

Requested reviews:
  Wordpress Charmers (wordpress-charmers)

For more details, see:
https://code.launchpad.net/~tcuthbert/charm-k8s-wordpress/+git/charm-k8s-wordpress-1/+merge/403642
-- 
Your team Wordpress Charmers is requested to review the proposed merge of ~tcuthbert/charm-k8s-wordpress:xubuntu-container-fixes into charm-k8s-wordpress:master.

diff --git a/Dockerfile b/Dockerfile
index 21e3b76..5f50716 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,11 +1,5 @@
 ARG DIST_RELEASE
-FROM ubuntu:${DIST_RELEASE}
-ARG DIST_RELEASE
-ARG VERSION
-
-LABEL maintainer="wordpress-charmers@xxxxxxxxxxxxxxxxxxx"
-# Used by Launchpad OCI Recipe to tag version
-LABEL org.label-schema.version=${VERSION:-5.6}
+FROM ubuntu:${DIST_RELEASE} as base
 
 # HTTPS_PROXY used when we RUN curl to download Wordpress itself
 ARG BUILD_DATE
@@ -42,6 +36,7 @@ RUN apt-get update && apt-get -y dist-upgrade \
             python3 \
             python3-yaml \
             ssl-cert \
+            wget \
         && sed -ri 's/^export ([^=]+)=(.*)$/: ${\1:=\2}\nexport \1/' "$APACHE_ENVVARS" \
         && . "$APACHE_ENVVARS" \
         && for dir in "$APACHE_LOCK_DIR" "$APACHE_RUN_DIR" "$APACHE_LOG_DIR"; do rm -rvf "$dir"; mkdir -p "$dir"; chown "$APACHE_RUN_USER:$APACHE_RUN_GROUP" "$dir"; chmod 777 "$dir";  done \
@@ -62,8 +57,23 @@ RUN a2enconf docker-php \
     && a2enmod rewrite \
     && a2enmod ssl
 
+
+FROM base as plugins
+
+# Download themes and plugins. This will eventually be separated into new container.
+COPY ./image-builder/src/fetcher.py /
+WORKDIR /var/www/html/wp-content/
+RUN mkdir themes plugins && /fetcher.py
+VOLUME /var/www/html/wp-content
+
+FROM base As install
+ARG VERSION
+
+# TODO: replace downloading the source wordpress code with copying it from the upstream wordpress container,
+# which should speed builds up:
+#   COPY --from=wordpress-${VERSION}:fpm /usr/src/wordpress /usr/src/wordpress
 # Install the main Wordpress code, this will be our only site so /var/www/html is fine
-RUN curl -o wordpress.tar.gz -fSL "https://wordpress.org/wordpress-${VERSION}.tar.gz"; \
+RUN wget -O wordpress.tar.gz -t 3 -r "https://wordpress.org/wordpress-${VERSION}.tar.gz"; \
     && tar -xzf wordpress.tar.gz -C /usr/src/ \
     && rm wordpress.tar.gz \
     && chown -R www-data:www-data /usr/src/wordpress \
@@ -71,17 +81,9 @@ RUN curl -o wordpress.tar.gz -fSL "https://wordpress.org/wordpress-${VERSION}.ta
     && mv /usr/src/wordpress /var/www/html
 
 COPY ./image-builder/files/ /files/
-COPY ./image-builder/src/fetcher.py .
-RUN mkdir -p /files/themes /files/plugins
-RUN ./fetcher.py
-# Copy our collected themes and plugins into the appropriate paths
-RUN cp -r /files/plugins/* /var/www/html/wp-content/plugins/
-RUN cp -r /files/themes/* /var/www/html/wp-content/themes/
-
 # wp-info.php contains template variables which our ENTRYPOINT script will populate
 RUN install -D /files/wp-info.php /var/www/html/wp-info.php
 RUN install -D /files/wp-config.php /var/www/html/wp-config.php
-RUN chown -R www-data:www-data /var/www/html
 
 # Copy our helper scripts and their wrapper into their own directory
 RUN install /files/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
@@ -97,10 +99,19 @@ RUN chmod 0755 /srv/wordpress-helpers/plugin_handler.py
 RUN chmod 0755 /srv/wordpress-helpers/ready.sh
 RUN chmod 0755 /usr/local/bin/docker-entrypoint.sh
 
-RUN rm -r /files
+FROM install as wordpress
+ARG VERSION
+
+LABEL maintainer="wordpress-charmers@xxxxxxxxxxxxxxxxxxx"
+# Used by Launchpad OCI Recipe to tag version
+LABEL org.label-schema.version=${VERSION:-5.7}
 
 # Port 80 only, TLS will terminate elsewhere
 EXPOSE 80
 
+# Copy plugins from the plugin stage into the WordPress content directory.
+COPY ./image-builder/src/fetcher.py /
+COPY --chown=www-data:www-data --from=plugins /var/www/html/wp-content/plugins/ /var/www/html/wp-content/plugins/
+COPY --chown=www-data:www-data --from=plugins /var/www/html/wp-content/themes/ /var/www/html/wp-content/themes/
 ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
 CMD apachectl -D FOREGROUND
diff --git a/image-builder/files/docker-entrypoint.sh b/image-builder/files/docker-entrypoint.sh
index 545d54e..42caf39 100644
--- a/image-builder/files/docker-entrypoint.sh
+++ b/image-builder/files/docker-entrypoint.sh
@@ -14,6 +14,15 @@ done
 # If we have passed in SWIFT_URL, then append swift proxy config.
 [ -z "${SWIFT_URL-}" ] || a2enconf docker-php-swift-proxy
 
+# TODO: this will eventually be called directly by the charm.
+(
+    cd /tmp
+    /fetcher.py
+    find /tmp -type f \( -path '/tmp/plugins/*' -o -path '/tmp/themes/*' \) -printf "%p\0/var/www/html/wp-content/%P\0" |
+        xargs -rn2 -0 install -DT && rm -fr /tmp/* &&
+        rm -fr /tmp/*
+)
+
 nohup bash -c "/srv/wordpress-helpers/plugin_handler.py &"
 
 # Match against either php 7.2 (bionic) or 7.4 (focal).
diff --git a/image-builder/src/fetcher.py b/image-builder/src/fetcher.py
index b7085f2..7d77fd2 100755
--- a/image-builder/src/fetcher.py
+++ b/image-builder/src/fetcher.py
@@ -3,8 +3,12 @@
 import os
 import shutil
 import subprocess
+import sys
 import urllib.request
 import zipfile
+from typing import Mapping, List
+from urllib.parse import urlparse
+from yaml import safe_load
 
 
 zip_plugins_to_get = {
@@ -98,11 +102,11 @@ def get_plugins(zip_plugins, branch_plugins):
         current_zip = current_zip + 1
         print('Downloading {} of {} zipped plugins: {} ...'.format(current_zip, total_zips, zip_plugin))
         url = 'https://downloads.wordpress.org/plugin/{}.latest-stable.zip'.format(zip_plugin)
-        file_name = os.path.join(os.getcwd(), 'files/plugins', os.path.basename(url))
+        file_name = os.path.join(os.getcwd(), 'plugins', os.path.basename(url))
         with urllib.request.urlopen(url) as response, open(file_name, 'wb') as out_file:
             shutil.copyfileobj(response, out_file)
         with zipfile.ZipFile(file_name, 'r') as zip_ref:
-            zip_ref.extractall(os.path.join(os.getcwd(), 'files/plugins'))
+            zip_ref.extractall(os.path.join(os.getcwd(), 'plugins'))
         os.remove(file_name)
 
     total_branches = len(branch_plugins)
@@ -116,7 +120,7 @@ def get_plugins(zip_plugins, branch_plugins):
             basename = basename[3:]
         if basename.startswith('wp-plugin-'):
             basename = basename[10:]
-        dest = os.path.join(os.getcwd(), 'files/plugins', basename)
+        dest = os.path.join(os.getcwd(), 'plugins', basename)
         if url.startswith('lp:'):
             cmd = ['bzr', 'branch', url, dest]
         elif url.startswith('https://git'):
@@ -139,7 +143,7 @@ def get_themes(branch_themes):
             basename = basename[3:]
         if basename.startswith('wp-theme-'):
             basename = basename[9:]
-        dest = os.path.join(os.getcwd(), 'files/themes/', basename)
+        dest = os.path.join(os.getcwd(), 'themes/', basename)
         if url.startswith('lp:'):
             cmd = ['bzr', 'branch', url, dest]
         elif url.startswith('https://git'):
@@ -150,6 +154,112 @@ def get_themes(branch_themes):
         _ = subprocess.check_output(cmd, universal_newlines=True, stderr=subprocess.STDOUT)
 
 
-if __name__ == '__main__':
+class Plugin:
+    name: str
+    _protocol: str
+    _url: str
+    _owner = "www-data"
+    _group = "www-data"
+
+    def __init__(self, name: str, url: str):
+        self.name = name
+        self.url = url
+
+    def __str__(self) -> str:
+        return self.name
+
+    def __repr__(self) -> str:
+        kwargs = {"name": self.name, "url": self.url}
+        return "<Plugin>(name={name}, url={url})".format(**kwargs)
+
+    @property
+    def url(self):
+        return self._url
+
+    @url.setter
+    def url(self, url: str):
+        u = urlparse(url)
+        self._url = u.geturl()
+        self._protocol = u.scheme.split("+")[0]
+        self._protocol = self._protocol.replace("https", "git")
+
+    def is_available(self):
+        print("NOT IMPLEMENTED")
+
+    def is_bzr(self):
+        return self._protocol == "lp"
+
+    def is_git(self):
+        return self._protocol == "git"
+
+    def sync(self, dest_dir: str):
+        plugin_path = os.path.join(dest_dir, self.name)
+        try:
+            self.__call_sync(plugin_path)
+        except subprocess.CalledProcessError as ce:
+            shutil.rmtree(plugin_path)
+            print("ERROR: {0}: failed to sync plugin: {1}, cleaning up plugin dir: {2}".format(str(ce), self,
+                                                                                               plugin_path))
+        else:
+            self.__chown_path(plugin_path)
+
+    def __build_cmd(self, dest_dir: str) -> List[str]:
+        cmds = {
+            "git": ["git", "clone", self.url, dest_dir],
+            "lp": ["bzr", "branch", self.url, dest_dir],
+        }
+        return cmds[self._protocol]
+
+    def __call_sync(self, plugin_path: str, verbose=True):
+        subprocess_kwargs = {}
+        subprocess_kwargs["check"] = True
+        subprocess_kwargs["universal_newlines"] = True
+        if verbose:
+            subprocess_kwargs["stderr"] = subprocess.STDOUT
+
+        dirname = os.path.dirname(plugin_path)
+        if not os.path.exists(dirname):
+            self.__make_path(dirname)
+        print("DEBUG: syncing plugin at path: {0}".format(plugin_path))
+        rv = subprocess.run(self.__build_cmd(plugin_path), **subprocess_kwargs)
+        return rv.returncode
+
+    def __chown_path(self, path: str):
+        for dirpath, _, filenames in os.walk(path):
+            shutil.chown(dirpath, "www-data", "www-data")
+            for filename in filenames:
+                shutil.chown(os.path.join(dirpath, filename), self._owner, self._group)
+
+    def __make_path(self, path: str):
+        print("DEBUG: creating directory: {0}".format(path))
+        os.makedirs(path, mode=0o755)
+
+
+def sync_additional_plugins(additional_plugins: Mapping[str, str]):
+    """
+    Sync any additional WordPress plugins specified from the Container
+    environment variable: WORDPRESS_ADDITIONAL_PLUGINS.
+
+    WORDPRESS_ADDITIONAL_PLUGINS must be a valid YAML formatted map string.
+    """
+    dest_path = os.path.join(os.getcwd(), "plugins")
+    total_plugins = len(additional_plugins)
+    count = 0
+    for name, url in additional_plugins.items():
+        count += 1
+        plugin = Plugin(name, url)
+        print("Syncing {0} of {1} WordPress plugins: {2} ...".format(count, total_plugins, plugin))
+        plugin.sync(dest_path)
+
+
+if __name__ == "__main__":
+    additional_plugins = os.environ.get("WORDPRESS_ADDITIONAL_PLUGINS")
+    if additional_plugins:
+        plugins = safe_load(additional_plugins.encode("utf-8"))
+        sync_additional_plugins(plugins)
+    # TODO: Script should have command-line flags instead of this.
+    if any([e for e in os.environ.keys() if e.startswith("WORDPRESS")]):
+        print("DEBUG: we are running inside a container, no need to sync base plugins")
+        sys.exit(0)
     get_plugins(zip_plugins_to_get, branch_plugins_to_get)
     get_themes(branch_themes_to_get)
diff --git a/image-builder/tests/unit/test_fetcher.py b/image-builder/tests/unit/test_fetcher.py
new file mode 100644
index 0000000..edced66
--- /dev/null
+++ b/image-builder/tests/unit/test_fetcher.py
@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+import unittest
+from unittest.mock import patch
+
+from fetcher import Plugin
+from fetcher import sync_additional_plugins
+
+
+class TestPlugin(unittest.TestCase):
+    test_name = "testPlugin"
+    test_url = "lp:~testUser/testProject/testBranch"
+
+    def setUp(self):
+        test_name = self.test_name
+        test_url = self.test_url
+        self.plugin = Plugin(test_name, test_url)
+
+    def test__init__(self):
+        plugin = Plugin(TestPlugin.test_name, TestPlugin.test_url)
+        name = plugin.name
+        protocol = plugin._protocol
+        url = plugin.url
+        self.assertEqual(name, TestPlugin.test_name)
+        self.assertEqual(protocol, TestPlugin.test_url.split(":")[0])
+        self.assertEqual(url, TestPlugin.test_url)
+
+    def test_is_available(self):
+        self.plugin.is_available()
+
+    def test_is_bzr(self):
+        self.assertTrue(self.plugin.is_bzr())
+
+    def test_is_git(self):
+        self.assertFalse(self.plugin.is_git())
+
+    @patch.object(Plugin, "_Plugin__make_dest")
+    @patch.object(Plugin, "_Plugin__call_sync")
+    def test_sync(self, mock_call_sync, mock_make_dest):
+
+        mock_call_sync.return_value = None
+        self.plugin.sync("test/path")
+
+        assert mock_call_sync.called
+        assert mock_make_dest.called
+
+        mock_call_sync.return_value = True
+        with self.assertRaises(RuntimeError) as cm:
+            self.plugin.sync("test/path")
+            self.assertTrue("failed to sync plugin" in str(cm.exception))
diff --git a/tox.ini b/tox.ini
index 0736115..9abc7b0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -14,12 +14,12 @@ passenv =
 
 [testenv:unit]
 commands =
-    pytest --ignore image-builder --ignore {toxinidir}/tests/integration \
+    pytest --ignore image-builder/files --ignore {toxinidir}/tests/integration \
       {posargs:-v  --cov=src --cov-report=term-missing --cov-branch}
 deps = -r{toxinidir}/tests/unit/requirements.txt
        -r{toxinidir}/requirements.txt
 setenv =
-  PYTHONPATH={toxinidir}/src:{toxinidir}/build/lib:{toxinidir}/build/venv
+  PYTHONPATH={toxinidir}/src:{toxinidir}/build/lib:{toxinidir}/build/venv:{toxinidir}/image-builder/src:{toxinidir}/image-builder/files
   TZ=UTC
 
 [testenv:integration]