Merge ~mthaddon/charm-k8s-ingress/+git/charm-k8s-ingress:proxy-body-size-default into charm-k8s-ingress:master

[Tom Haddon]

Set default proxy-body-size to 20m but ensure this can be overridden to be unlimited by setting to 0.

Set default proxy-body-size to 20m but ensure this can be overridden to be unlimited by setting to 0.

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

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

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

PASSED: Continuous integration, rev:2f07888a6cc21c53d234b931f2011f588bff9123
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/53/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/130/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/276615/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/53//rebuild

review: Approve (continuous-integration)

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

Change successfully merged at revision a8d4cf034d918b501dfd6ccc0e5f23ebd6c134c8

diff --git a/config.yaml b/config.yaml
index c5d5957..9e95591 100644
--- a/config.yaml
+++ b/config.yaml
@@ -44,7 +44,7 @@ options:
       is a comma-separated list of CIDRs.
     type: string
   max-body-size:
-    default: 0
+    default: 20
     description: Max allowed body-size (for file uploads) in megabytes, set to 0 to disable limits.
     type: int
   path-routes:
diff --git a/src/charm.py b/src/charm.py
index 4926723..287a954 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -118,10 +118,7 @@ class NginxIngressCharm(CharmBase):
     def _max_body_size(self):
         """Return the max-body-size to use for k8s ingress."""
         max_body_size = self._get_config_or_relation_data("max-body-size", 0)
-        if max_body_size:
-            return "{}m".format(max_body_size)
-        # Don't return "0m" which would evaluate to True.
-        return ""
+        return "{}m".format(max_body_size)
 
     @property
     def _rewrite_enabled(self):
@@ -258,15 +255,13 @@ class NginxIngressCharm(CharmBase):
 
         spec = kubernetes.client.NetworkingV1beta1IngressSpec(rules=ingress_rules)
 
-        annotations = {}
+        annotations = {"nginx.ingress.kubernetes.io/proxy-body-size": self._max_body_size}
         if self._rewrite_enabled:
             annotations["nginx.ingress.kubernetes.io/rewrite-target"] = self._rewrite_target
         if self._limit_rps:
             annotations["nginx.ingress.kubernetes.io/limit-rps"] = self._limit_rps
             if self._limit_whitelist:
                 annotations["nginx.ingress.kubernetes.io/limit-whitelist"] = self._limit_whitelist
-        if self._max_body_size:
-            annotations["nginx.ingress.kubernetes.io/proxy-body-size"] = self._max_body_size
         if self._retry_errors:
             annotations["nginx.ingress.kubernetes.io/proxy-next-upstream"] = self._retry_errors
         if self._session_cookie_max_age:
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 949d8f9..86628b1 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -308,6 +308,7 @@ class TestCharm(unittest.TestCase):
         )
         result_dict = self.harness.charm._get_k8s_ingress().to_dict()
         expected = {
+            "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
             "nginx.ingress.kubernetes.io/rewrite-target": "/",
             "nginx.ingress.kubernetes.io/ssl-redirect": "false",
         }
@@ -315,13 +316,17 @@ class TestCharm(unittest.TestCase):
 
         self.harness.update_config({"rewrite-enabled": False})
         result_dict = self.harness.charm._get_k8s_ingress().to_dict()
-        expected = {"nginx.ingress.kubernetes.io/ssl-redirect": "false"}
+        expected = {
+            "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
+            "nginx.ingress.kubernetes.io/ssl-redirect": "false",
+        }
         self.assertEqual(result_dict["metadata"]["annotations"], expected)
 
         self.harness.update_config({"rewrite-target": "/test-target"})
         self.harness.update_config({"rewrite-enabled": True})
 
         expected = {
+            "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
             "nginx.ingress.kubernetes.io/rewrite-target": "/test-target",
             "nginx.ingress.kubernetes.io/ssl-redirect": "false",
         }
@@ -381,6 +386,7 @@ class TestCharm(unittest.TestCase):
             metadata=kubernetes.client.V1ObjectMeta(
                 name="gunicorn-ingress",
                 annotations={
+                    "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
                     "nginx.ingress.kubernetes.io/rewrite-target": "/",
                     "nginx.ingress.kubernetes.io/ssl-redirect": "false",
                 },
@@ -413,6 +419,7 @@ class TestCharm(unittest.TestCase):
             metadata=kubernetes.client.V1ObjectMeta(
                 name="gunicorn-ingress",
                 annotations={
+                    "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
                     "nginx.ingress.kubernetes.io/rewrite-target": "/",
                     "nginx.ingress.kubernetes.io/ssl-redirect": "false",
                 },
@@ -473,6 +480,7 @@ class TestCharm(unittest.TestCase):
             metadata=kubernetes.client.V1ObjectMeta(
                 name="gunicorn-ingress",
                 annotations={
+                    "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
                     "nginx.ingress.kubernetes.io/rewrite-target": "/",
                     "nginx.ingress.kubernetes.io/ssl-redirect": "false",
                 },
@@ -513,6 +521,7 @@ class TestCharm(unittest.TestCase):
             metadata=kubernetes.client.V1ObjectMeta(
                 name="gunicorn-ingress",
                 annotations={
+                    "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
                     "nginx.ingress.kubernetes.io/rewrite-target": "/",
                 },
             ),
@@ -548,7 +557,7 @@ class TestCharm(unittest.TestCase):
         self.harness.update_config(
             {
                 "ingress-class": "nginx",
-                "max-body-size": 20,
+                "max-body-size": 10,
                 "retry-errors": "error,timeout,http_502,http_503",
                 "session-cookie-max-age": 3600,
                 "tls-secret-name": "",
@@ -562,7 +571,7 @@ class TestCharm(unittest.TestCase):
                 annotations={
                     "nginx.ingress.kubernetes.io/affinity": "cookie",
                     "nginx.ingress.kubernetes.io/affinity-mode": "balanced",
-                    "nginx.ingress.kubernetes.io/proxy-body-size": "20m",
+                    "nginx.ingress.kubernetes.io/proxy-body-size": "10m",
                     "nginx.ingress.kubernetes.io/proxy-next-upstream": (
                         "error timeout http_502 http_503"
                     ),
@@ -615,6 +624,7 @@ class TestCharm(unittest.TestCase):
                 annotations={
                     "nginx.ingress.kubernetes.io/limit-rps": "5",
                     "nginx.ingress.kubernetes.io/limit-whitelist": "10.0.0.0/16",
+                    "nginx.ingress.kubernetes.io/proxy-body-size": "0m",
                     "nginx.ingress.kubernetes.io/rewrite-target": "/",
                     "nginx.ingress.kubernetes.io/ssl-redirect": "false",
                 },