Merge ~barryprice/charm-k8s-wordpress/+git/charm-k8s-wordpress:master into charm-k8s-wordpress:master

[Barry Price]

Handle redirects better, detect and warn about missing DB config, add registry authentication support, fix status.maintenance() syntax error

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Stuart Bishop (stub)]

This looks good. One comment inline for some minor code shuffling. Try not to include black code reformats together with other reviews.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 972e302aea795b274e3846923a7049081e7d9472

diff --git a/config.yaml b/config.yaml
index 842b2e2..a622726 100644
--- a/config.yaml
+++ b/config.yaml
@@ -3,6 +3,14 @@ options:
     type: string
     description: "The docker image to install. Required. Defaults to Dockerhub wordpress:php7.3"
     default: "wordpress:php7.3"
+  image_user:
+    type: string
+    description: "Username to use for the configured image registry, if required"
+    default: ""
+  image_pass:
+    type: string
+    description: "Password to use for the configured image registry, if required"
+    default: ""
   ports:
     type: string
     description: >
diff --git a/reactive/wordpress.py b/reactive/wordpress.py
index 39ea6dc..d54ac58 100644
--- a/reactive/wordpress.py
+++ b/reactive/wordpress.py
@@ -3,7 +3,7 @@ import os
 import re
 import requests
 from pprint import pprint
-from urllib.parse import urlunparse
+from urllib.parse import urlparse, urlunparse
 from yaml import safe_load
 
 from charmhelpers.core import host, hookenv
@@ -14,7 +14,7 @@ from charms.reactive import hook, when, when_not
 
 @hook("upgrade-charm")
 def upgrade_charm():
-    status.maintenance("maintenance", "Upgrading charm")
+    status.maintenance("Upgrading charm")
     reactive.clear_flag("wordpress.configured")
 
 
@@ -108,14 +108,27 @@ def make_pod_spec():
     # PodSpec v1? https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.13/#podspec-v1-core
     spec = {
         "containers": [
-            {"name": hookenv.charm_name(), "image": config["image"], "ports": ports, "config": container_config}
+            {
+                "name": hookenv.charm_name(),
+                "imageDetails": {"imagePath": config["image"]},
+                "ports": ports,
+                "config": container_config,
+            }
         ]
     }
     out = io.StringIO()
     pprint(spec, out)
-    hookenv.log("Container spec (sans secrets) <<EOM\n{}\nEOM".format(out.getvalue()))
+    hookenv.log(
+        "Container environment config (sans secrets) <<EOM\n{}\nEOM".format(
+            out.getvalue()
+        )
+    )
+
+    # if we need credentials (secrets) for our image, add them to the spec after logging
+    if config.get("image_user") and config.get("image_pass"):
+        spec.get("containers")[0].get("imageDetails")["username"] = config["image_user"]
+        spec.get("containers")[0].get("imageDetails")["password"] = config["image_pass"]
 
-    # Add the secrets after logging
     config_with_secrets = full_container_config()
     if config_with_secrets is None:
         return None  # status already set
@@ -134,10 +147,16 @@ def first_install():
         hookenv.log("Wordpress vhost is not yet listening - retrying")
         return False
     elif wordpress_configured() or not config["initial_settings"]:
-        hookenv.log("No initial_setting provided or wordpress already configured. Skipping first install.")
+        hookenv.log(
+            "No initial_setting provided or wordpress already configured. Skipping first install."
+        )
         return True
     hookenv.log("Starting wordpress initial configuration")
-    payload = {"admin_password": host.pwgen(24), "blog_public": "checked", "Submit": "submit"}
+    payload = {
+        "admin_password": host.pwgen(24),
+        "blog_public": "checked",
+        "Submit": "submit",
+    }
     payload.update(safe_load(config["initial_settings"]))
     payload["admin_password2"] = payload["admin_password"]
     if not payload["blog_public"]:
@@ -147,21 +166,37 @@ def first_install():
     if missing:
         hookenv.log("Error: missing wordpress settings: {}".format(missing))
         return False
-    call_wordpress("/wp-admin/install.php?step=2", payload=payload)
-    host.write_file(os.path.join("/root/", "initial.passwd"), payload["admin_password"], perms=0o400)
+    call_wordpress("/wp-admin/install.php?step=2", redirects=True, payload=payload)
+    host.write_file(
+        os.path.join("/root/", "initial.passwd"), payload["admin_password"], perms=0o400
+    )
     return True
 
 
-def call_wordpress(uri, redirects=True, payload={}):
+def call_wordpress(uri, redirects=True, payload={}, _depth=1):
+    max_depth = 10
+    if _depth > max_depth:
+        hookenv.log("Redirect loop detected in call_worpress()")
+        raise RuntimeError("Redirect loop detected in call_worpress()")
     config = hookenv.config()
     service_ip = get_service_ip("website")
     if service_ip:
         headers = {"Host": config["blog_hostname"]}
         url = urlunparse(("http", service_ip, uri, "", "", ""))
         if payload:
-            return requests.post(url, allow_redirects=redirects, headers=headers, data=payload)
+            r = requests.post(
+                url, allow_redirects=False, headers=headers, data=payload, timeout=30
+            )
+        else:
+            r = requests.get(url, allow_redirects=False, headers=headers, timeout=30)
+        if redirects and r.is_redirect:
+            # recurse, but strip the scheme and host first, we need to connect over HTTP by bare IP
+            o = urlparse(r.headers.get("Location"))
+            return call_wordpress(
+                o.path, redirects=redirects, payload=payload, _depth=_depth + 1
+            )
         else:
-            return requests.get(url, allow_redirects=redirects, headers=headers)
+            return r
     else:
         hookenv.log("Error getting service IP")
         return False
@@ -180,7 +215,15 @@ def wordpress_configured():
         r = call_wordpress("/", redirects=False)
     except requests.exceptions.ConnectionError:
         return False
-    if r.status_code == 302 and re.match("^.*/wp-admin/install.php", r.headers.get("location", "")):
+    if r.status_code == 302 and re.match(
+        "^.*/wp-admin/install.php", r.headers.get("location", "")
+    ):
+        return False
+    elif r.status_code == 302 and re.match(
+        "^.*/wp-admin/setup-config.php", r.headers.get("location", "")
+    ):
+        hookenv.log("MySQL database setup failed, we likely have no wp-config.php")
+        status.blocked("MySQL database setup failed, we likely have no wp-config.php")
         return False
     else:
         return True
@@ -192,8 +235,13 @@ def is_vhost_ready():
     try:
         r = call_wordpress("/wp-login.php", redirects=False)
     except requests.exceptions.ConnectionError:
+        hookenv.log("call_wordpress() returned requests.exceptions.ConnectionError")
+        return False
+    if r is None:
+        hookenv.log("call_wordpress() returned None")
         return False
-    if r.status_code in (403, 404):
+    if hasattr(r, "status_code") and r.status_code in (403, 404):
+        hookenv.log("call_wordpress() returned status {}".format(r.status_code))
         return False
     else:
         return True