Merge ~mthaddon/charm-k8s-ingress/+git/charm-k8s-ingress:rate-limiting into charm-k8s-ingress:master

[Tom Haddon]

Add rate-limiting

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

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

[🤖 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)]

PASSED: Continuous integration, rev:173827808d137da0d290e2d55fd861e936520757
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/34/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/71/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/85424/

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

review: Approve (continuous-integration)

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

Change successfully merged at revision 667d31f13320570bb8e324afe9b103fdb5f8376b

diff --git a/config.yaml b/config.yaml
index 5f0c3a4..885c09b 100644
--- a/config.yaml
+++ b/config.yaml
@@ -15,6 +15,20 @@ options:
     default: ""
     description: k8s config. If you're using microk8s, this should be the output of `microk8s config`.
     type: string
+  limit-rps:
+    default: 0
+    description: >
+      Number of requests accepted from a given IP each second. The burst limit
+      is set to this limit multiplied by 5. When clients exceed this limit a
+      503 error will be returned.
+      Setting this to 0 disables rate-limiting.
+    type: int
+  limit-whitelist:
+    default: ""
+    description: >
+      If rate-limiting is set, client IP source ranges to be excluded. The value
+      is a comma-separated list of CIDRs.
+    type: string
   max-body-size:
     default: 0
     description: Max allowed body-size (for file uploads) in megabytes, set to 0 to disable limits.
diff --git a/lib/charms/ingress/v0/ingress.py b/lib/charms/ingress/v0/ingress.py
index acef2c0..e002f18 100644
--- a/lib/charms/ingress/v0/ingress.py
+++ b/lib/charms/ingress/v0/ingress.py
@@ -11,6 +11,8 @@ Import `IngressRequires` in your charm, with two required options:
     - service-hostname (required)
     - service-name (required)
     - service-port (required)
+    - limit-rps
+    - limit-whitelist
     - max_body-size
     - retry-errors
     - service-namespace
@@ -47,7 +49,7 @@ LIBAPI = 0
 
 # Increment this PATCH version before using `charmcraft push-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 8
+LIBPATCH = 9
 
 logger = logging.getLogger(__name__)
 
@@ -58,6 +60,8 @@ REQUIRED_INGRESS_RELATION_FIELDS = {
 }
 
 OPTIONAL_INGRESS_RELATION_FIELDS = {
+    "limit-rps",
+    "limit-whitelist",
     "max-body-size",
     "retry-errors",
     "service-namespace",
diff --git a/src/charm.py b/src/charm.py
index 9f84ece..3531e9d 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -83,6 +83,20 @@ class IngressCharm(CharmBase):
         return "{}-ingress".format(self._get_config_or_relation_data("service-name", ""))
 
     @property
+    def _limit_rps(self):
+        """Return limit-rps value from config or relation."""
+        limit_rps = self._get_config_or_relation_data("limit-rps", 0)
+        if limit_rps:
+            return str(limit_rps)
+        # Don't return "0" which would evaluate to True.
+        return ""
+
+    @property
+    def _limit_whitelist(self):
+        """Return the limit-whitelist value from config or relation."""
+        return self._get_config_or_relation_data("limit-whitelist", "")
+
+    @property
     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)
@@ -206,6 +220,10 @@ class IngressCharm(CharmBase):
         annotations = {
             "nginx.ingress.kubernetes.io/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:
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 9dda0c1..9fa2a77 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -386,6 +386,51 @@ class TestCharm(unittest.TestCase):
             ),
         )
         self.assertEqual(self.harness.charm._get_k8s_ingress(), expected)
+        # Test limit-whitelist on its own makes no change.
+        self.harness.update_config({"limit-whitelist": "10.0.0.0/16"})
+        self.assertEqual(self.harness.charm._get_k8s_ingress(), expected)
+        # And if we set limit-rps we get both. Unset other options to minimize output.
+        self.harness.update_config(
+            {
+                "limit-rps": 5,
+                "ingress-class": "",
+                "max-body-size": 0,
+                "retry-errors": "",
+                "session-cookie-max-age": 0,
+            }
+        )
+        expected = kubernetes.client.NetworkingV1beta1Ingress(
+            api_version="networking.k8s.io/v1beta1",
+            kind="Ingress",
+            metadata=kubernetes.client.V1ObjectMeta(
+                name="gunicorn-ingress",
+                annotations={
+                    "nginx.ingress.kubernetes.io/limit-rps": "5",
+                    "nginx.ingress.kubernetes.io/limit-whitelist": "10.0.0.0/16",
+                    "nginx.ingress.kubernetes.io/rewrite-target": "/",
+                    "nginx.ingress.kubernetes.io/ssl-redirect": "false",
+                },
+            ),
+            spec=kubernetes.client.NetworkingV1beta1IngressSpec(
+                rules=[
+                    kubernetes.client.NetworkingV1beta1IngressRule(
+                        host="foo.internal",
+                        http=kubernetes.client.NetworkingV1beta1HTTPIngressRuleValue(
+                            paths=[
+                                kubernetes.client.NetworkingV1beta1HTTPIngressPath(
+                                    path="/",
+                                    backend=kubernetes.client.NetworkingV1beta1IngressBackend(
+                                        service_port=80,
+                                        service_name="gunicorn-service",
+                                    ),
+                                )
+                            ]
+                        ),
+                    )
+                ]
+            ),
+        )
+        self.assertEqual(self.harness.charm._get_k8s_ingress(), expected)
 
     def test_get_k8s_service(self):
         """Test getting our definition of a k8s service."""