Support deploying multiple ingresses

This adds support for deploying multiple ingresses with FIAAS, by
separating the ingresses specified in the config by their distinct
requirements for annotations. This is useful as it means for ingress
controllers that use annotations for configuration - this can be
determining which controller is used (public/private network,
or configuration that affects behaviour (whitelisting, HTTP-auth)
per host/path.

fiaas_deploy_daemon/deployer/kubernetes/ingress.py

@@ -22,12 +22,14 @@
 from itertools import chain
 
 from k8s.client import NotFound
+from k8s.base import Equality, Inequality, Exists
 from k8s.models.common import ObjectMeta
 from k8s.models.ingress import Ingress, IngressSpec, IngressRule, HTTPIngressRuleValue, HTTPIngressPath, IngressBackend, \
     IngressTLS
 
 from fiaas_deploy_daemon.retry import retry_on_upsert_conflict
 from fiaas_deploy_daemon.tools import merge_dicts
+from collections import namedtuple
 
 LOG = logging.getLogger(__name__)
 
@@ -43,48 +45,89 @@ def deploy(self, app_spec, labels):
         if self._should_have_ingress(app_spec):
             self._create(app_spec, labels)
         else:
-            self.delete(app_spec)
+            self._delete_unused(app_spec, labels)
 
     def delete(self, app_spec):
-        LOG.info("Deleting ingress for %s", app_spec.name)
+        LOG.info("Deleting ingresses for %s", app_spec.name)
         try:
-            Ingress.delete(app_spec.name, app_spec.namespace)
+            Ingress.delete_list(namespace=app_spec.namespace, labels={"app": Equality(app_spec.name), "fiaas/deployment_id": Exists()})
         except NotFound:
             pass
 
-    @retry_on_upsert_conflict
     def _create(self, app_spec, labels):
-        LOG.info("Creating/updating ingress for %s", app_spec.name)
-        annotations = {
-            u"fiaas/expose": u"true" if _has_explicitly_set_host(app_spec) else u"false"
+        LOG.info("Creating/updating ingresses for %s", app_spec.name)
+        custom_labels = merge_dicts(app_spec.labels.ingress, labels)
+
+        # Group app_spec.ingresses to separate those with annotations
+        AnnotatedIngress = namedtuple("AnnotatedIngress", ["name", "ingress_items", "annotations"])
+        unannotated_ingress = AnnotatedIngress(name=app_spec.name, ingress_items=[], annotations={})
+        ingresses_by_annotations = [unannotated_ingress]
+        for ingress_item in app_spec.ingresses:
+            LOG.info(ingress_item)
+            if ingress_item.annotations:
+                next_name = "{}-{}".format(app_spec.name, len(ingresses_by_annotations))
+                annotated_ingresses = AnnotatedIngress(name=next_name, ingress_items=[ingress_item], annotations=ingress_item.annotations)
+                ingresses_by_annotations.append(annotated_ingresses)
+            else:
+                unannotated_ingress.ingress_items.append(ingress_item)
+
+        LOG.info("Will create %s ingresses", len(ingresses_by_annotations))
+        for annotated_ingress in ingresses_by_annotations:
+            if len(annotated_ingress.ingress_items) == 0:
+                LOG.info("No items, skipping: %s", annotated_ingress)
+                continue
+
+            self._create_ingress(app_spec, annotated_ingress, custom_labels)
+
+        self._delete_unused(app_spec, custom_labels)
+
+    @retry_on_upsert_conflict
+    def _create_ingress(self, app_spec, annotated_ingress, labels):
+        default_annotations = {
+            u"fiaas/expose": u"true" if _has_explicitly_set_host(annotated_ingress.ingress_items) else u"false"
         }
+        annotations = merge_dicts(annotated_ingress.annotations, app_spec.annotations.ingress, default_annotations)
 
-        custom_labels = merge_dicts(app_spec.labels.ingress, labels)
-        custom_annotations = merge_dicts(app_spec.annotations.ingress, annotations)
-        metadata = ObjectMeta(name=app_spec.name, namespace=app_spec.namespace, labels=custom_labels,
-                              annotations=custom_annotations)
+        metadata = ObjectMeta(name=annotated_ingress.name, namespace=app_spec.namespace, labels=labels,
+                              annotations=annotations)
 
         per_host_ingress_rules = [
             IngressRule(host=self._apply_host_rewrite_rules(ingress_item.host),
                         http=self._make_http_ingress_rule_value(app_spec, ingress_item.pathmappings))
-            for ingress_item in app_spec.ingresses
+            for ingress_item in annotated_ingress.ingress_items
             if ingress_item.host is not None
         ]
-        default_host_ingress_rules = self._create_default_host_ingress_rules(app_spec)
+        if annotated_ingress.annotations:
+            use_suffixes = False
+            host_ingress_rules = per_host_ingress_rules
+        else:
+            use_suffixes = True
+            host_ingress_rules = per_host_ingress_rules + self._create_default_host_ingress_rules(app_spec)
 
-        ingress_spec = IngressSpec(rules=per_host_ingress_rules + default_host_ingress_rules)
+        ingress_spec = IngressSpec(rules=host_ingress_rules)
 
         ingress = Ingress.get_or_create(metadata=metadata, spec=ingress_spec)
-        self._ingress_tls.apply(ingress, app_spec, self._get_hosts(app_spec))
+
+        hosts_for_tls = [rule.host for rule in host_ingress_rules]
+        self._ingress_tls.apply(ingress, app_spec, hosts_for_tls, use_suffixes=use_suffixes)
         self._owner_references.apply(ingress, app_spec)
         ingress.save()
 
+    def _delete_unused(self, app_spec, labels):
+        filter_labels = [
+            ("app", Equality(labels["app"])),
+            ("fiaas/deployment_id", Exists()),
+            ("fiaas/deployment_id", Inequality(labels["fiaas/deployment_id"]))
+        ]
+        Ingress.delete_list(namespace=app_spec.namespace, labels=filter_labels)
+
     def _generate_default_hosts(self, name):
         for suffix in self._ingress_suffixes:
             yield u"{}.{}".format(name, suffix)
 
     def _create_default_host_ingress_rules(self, app_spec):
-        all_pathmappings = chain.from_iterable(ingress_item.pathmappings for ingress_item in app_spec.ingresses)
+        all_pathmappings = chain.from_iterable(ingress_item.pathmappings
+                                               for ingress_item in app_spec.ingresses if not ingress_item.annotations)
         http_ingress_rule_value = self._make_http_ingress_rule_value(app_spec, all_pathmappings)
         return [IngressRule(host=host, http=http_ingress_rule_value)
                 for host in self._generate_default_hosts(app_spec.name)]
@@ -99,7 +142,7 @@ def _should_have_ingress(self, app_spec):
         return self._can_generate_host(app_spec) and _has_ingress(app_spec) and _has_http_port(app_spec)
 
     def _can_generate_host(self, app_spec):
-        return len(self._ingress_suffixes) > 0 or _has_explicitly_set_host(app_spec)
+        return len(self._ingress_suffixes) > 0 or _has_explicitly_set_host(app_spec.ingresses)
 
     @staticmethod
     def _make_http_ingress_rule_value(app_spec, pathmappings):
@@ -115,8 +158,8 @@ def _get_hosts(self, app_spec):
                 for ingress_item in app_spec.ingresses if ingress_item.host is not None]
 
 
-def _has_explicitly_set_host(app_spec):
-    return any(ingress.host is not None for ingress in app_spec.ingresses)
+def _has_explicitly_set_host(ingress_items):
+    return any(ingress_item.host is not None for ingress_item in ingress_items)
 
 
 def _has_http_port(app_spec):
@@ -142,7 +185,7 @@ def __init__(self, config):
         self._shortest_suffix = sorted(config.ingress_suffixes, key=len)[0] if config.ingress_suffixes else None
         self.enable_deprecated_tls_entry_per_host = config.enable_deprecated_tls_entry_per_host
 
-    def apply(self, ingress, app_spec, hosts):
+    def apply(self, ingress, app_spec, hosts, use_suffixes=True):
         if self._should_have_ingress_tls(app_spec):
             tls_annotations = {}
             if self._cert_issuer or app_spec.ingress_tls.certificate_issuer:
@@ -162,8 +205,12 @@ def apply(self, ingress, app_spec, hosts):
             else:
                 ingress.spec.tls = []
 
-            collapsed = self._collapse_hosts(app_spec, hosts)
-            ingress.spec.tls.append(IngressTLS(hosts=collapsed, secretName="{}-ingress-tls".format(app_spec.name)))
+            if use_suffixes:
+                # adding app-name to suffixes could result in a host too long to be the common-name of a cert, and
+                # as the user doesn't control it we should generate a host we know will fit
+                hosts = self._collapse_hosts(app_spec, hosts)
+
+            ingress.spec.tls.append(IngressTLS(hosts=hosts, secretName="{}-ingress-tls".format(ingress.metadata.name)))
 
     def _collapse_hosts(self, app_spec, hosts):
         """The first hostname in the list will be used as Common Name in the certificate"""

fiaas_deploy_daemon/specs/models.py

@@ -119,6 +119,7 @@ def version(self):
 IngressItemSpec = namedtuple("IngressItemSpec", [
     "host",
     "pathmappings",
+    "annotations"
 ])
 
 IngressPathMappingSpec = namedtuple("IngressPathMappingSpec", [

fiaas_deploy_daemon/specs/v3/defaults.yml

@@ -25,6 +25,7 @@ ingress: # Generate ingress rules for access from outside cluster. To disable, s
     paths: # List of paths exposed to which application port
     - path: / # Path the application answers on
       port: http # Name of the port path is served on
+    annotations: {}
 healthchecks: # Healthchecks defined for your application. If omitted and a single port is defined, liveness will default to http or tcp depending on port type, and readiness will be a copy of liveness. If no ports or multiple ports are defined, healthchecks are not provided and should be defined explicitly
   liveness:
     # Valid configuration requires exactly one of execute|http|tcp

fiaas_deploy_daemon/specs/v3/factory.py

@@ -207,15 +207,15 @@ def resolve_port_number(port):
             else:
                 raise InvalidConfiguration("{} is not a valid port name or port number".format(port))
 
-        def ingress_item(host, paths):
+        def ingress_item(host, paths, annotations):
             ingress_path_mapping_specs = [
                 IngressPathMappingSpec(path=pathmapping["path"], port=resolve_port_number(pathmapping["port"]))
                 for pathmapping in paths
             ]
-            return IngressItemSpec(host=host, pathmappings=ingress_path_mapping_specs)
+            return IngressItemSpec(host=host, pathmappings=ingress_path_mapping_specs, annotations=annotations)
 
         if len(http_ports.items()) > 0:
-            return [ingress_item(host_path_mapping["host"], host_path_mapping["paths"])
+            return [ingress_item(host_path_mapping["host"], host_path_mapping["paths"], host_path_mapping["annotations"])
                     for host_path_mapping in ingress_lookup]
         else:
             return []

tests/conftest.py

@@ -36,11 +36,11 @@ def prometheus_registry():
 
 
 @pytest.helpers.register
-def assert_any_call(mockk, first, *args):
+def assert_any_call(mockk, first, *args, **kwargs):
     __tracebackhide__ = True
 
     def _assertion():
-        mockk.assert_any_call(first, *args)
+        mockk.assert_called_once_with(first, *args, **kwargs)
 
     _add_useful_error_message(_assertion, mockk, first, args)
 

tests/fiaas_deploy_daemon/conftest.py

@@ -62,7 +62,7 @@ def app_spec():
         deployment_id="test_app_deployment_id",
         labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}),
         annotations=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}),
-        ingresses=[IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)])],
+        ingresses=[IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={})],
         strongbox=StrongboxSpec(enabled=False, iam_role=None, aws_region="eu-west-1", groups=None),
         singleton=False,
         ingress_tls=IngressTlsSpec(enabled=False, certificate_issuer=None),