Skip to content

Commit

Permalink
We assume that the default hosts will be always in the same group, th…
Browse files Browse the repository at this point in the history 
…is is not true because they can be part of diffirent ingress groups
  • Loading branch information
Tarik Ghallab committed Dec 14, 2021
1 parent 9d3ae9f commit e496aab
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 39 deletions.
74 changes: 42 additions & 32 deletions fiaas_deploy_daemon/deployer/kubernetes/ingress.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,22 +107,33 @@ def _get_issuer_type(self, host):

return self._tls_issuer_type_default

def _set_ingresses_names(self, ingresses,name):


def _fix_ingresses(self, ingresses, default_ingress_items, name):
current_name = name
new_ingresses = []
#Search in which ingress group we have default hosts
def _has_default_ingress_item(ingress):
for i in ingress.ingress_items:
if i in default_ingress_items:
return True
return False

for item in ingresses:
new_item =item._replace(name=current_name)
new_item = item._replace(name=current_name)
new_item = new_item._replace(default=_has_default_ingress_item(item))
new_ingresses.append(new_item)
current_name = "{}-{}".format(name,ingresses.index(item)+1)
current_name = "{}-{}".format(name, ingresses.index(item)+1)
return new_ingresses

def _group_ingresses(self, app_spec):
''' Group the ingresses so that those with annotations are individual, those that don't need tls are grouped together, and those using non-default TLS-issuers
are separated by TLS-issuer type
'''
explicit_host = _has_explicitly_set_host(app_spec.ingresses)
ingress_items = [item._replace(host=self._apply_host_rewrite_rules(item.host)) for item in app_spec.ingresses if item.host]
ingress_items += self._expand_default_hosts(app_spec)
default_ingress_items = self._expand_default_hosts(app_spec)
ingress_items += default_ingress_items

AnnotatedIngress = namedtuple("AnnotatedIngress", ["name", "ingress_items", "annotations", "explicit_host",
"issuer_type", "default"])
Expand All @@ -136,45 +147,44 @@ def _group_ingresses(self, app_spec):
issuer_type = self._get_issuer_type(ingress_item.host)
if ingress_item.annotations:
annotated_ingress = AnnotatedIngress(name="", ingress_items=[ingress_item],
annotations=ingress_item.annotations,
explicit_host=True, issuer_type=issuer_type,
default=False)
annotations=ingress_item.annotations,
explicit_host=True, issuer_type=issuer_type,
default=False)
annotated_ingresses.append(annotated_ingress)
elif self._ingress_tls._should_disable_ingress_tls([ingress_item.host]) is True:
#Group no tls hosts together
# Group no tls hosts together
notls_ingress = notls_ingresses.setdefault("no_tls",
AnnotatedIngress(name="",
ingress_items=[],
annotations={},
explicit_host=explicit_host,
issuer_type=issuer_type,
default=False))
AnnotatedIngress(name="",
ingress_items=[],
annotations={},
explicit_host=explicit_host,
issuer_type=issuer_type,
default=False))
notls_ingress.ingress_items.append(ingress_item)
elif issuer_type != self._tls_issuer_type_default:
#Group by issuer type
# Group by issuer type
annotated_ingress = override_issuer_ingresses.setdefault(issuer_type,
AnnotatedIngress(name="",
ingress_items=[],
annotations={},
explicit_host=explicit_host,
issuer_type=issuer_type,
default=False))
AnnotatedIngress(name="",
ingress_items=[],
annotations={},
explicit_host=explicit_host,
issuer_type=issuer_type,
default=False))
annotated_ingress.ingress_items.append(ingress_item)
else:
#This change to fix the issue: when we dont have any default ingress item the ingress is added to the ingresses list anyway. Now it will be added only if we have atleast one default ingress item (host)
# This change to fix the issue: when we dont have any default ingress item the ingress is added to the ingresses list anyway. Now it will be added only if we have atleast one default ingress item (host)
default_ingress = default_ingresses.setdefault("default",
AnnotatedIngress(name="", ingress_items=[], annotations={},
explicit_host=explicit_host, issuer_type=self._tls_issuer_type_default,
default=True))
AnnotatedIngress(name="", ingress_items=[], annotations={},
explicit_host=explicit_host, issuer_type=self._tls_issuer_type_default,
default=True))
default_ingress.ingress_items.append(ingress_item)


ingresses.extend(i for i in default_ingresses.values())
ingresses.extend(annotated_ingresses)
ingresses.extend(i for i in override_issuer_ingresses.values())
ingresses.extend(i for i in notls_ingresses.values())

ingresses = self._set_ingresses_names(ingresses,app_spec.name)
#Set the correct name for each group and set default=True for the groups that have atleast one default host
ingresses = self._fix_ingresses(ingresses, default_ingress_items, app_spec.name)

return ingresses

Expand Down Expand Up @@ -276,7 +286,7 @@ def __init__(self, config):
self.enable_deprecated_tls_entry_per_host = config.enable_deprecated_tls_entry_per_host

def apply(self, ingress, app_spec, hosts, issuer_type, use_suffixes=True):
if self._should_have_ingress_tls(app_spec,hosts):
if self._should_have_ingress_tls(app_spec, hosts):
tls_annotations = {}
if self._cert_issuer or app_spec.ingress_tls.certificate_issuer:
issuer = app_spec.ingress_tls.certificate_issuer if app_spec.ingress_tls.certificate_issuer else self._cert_issuer
Expand Down Expand Up @@ -318,11 +328,11 @@ def _should_have_ingress_tls(self, app_spec, hosts):
return True
elif self._should_disable_ingress_tls(hosts) is True:
return False
else:
else:
return self._use_ingress_tls == 'default_on'

def _should_disable_ingress_tls(self, hosts):
#Check if any ingress host is part of atleast one domain suffix that shoudln't have tls
# Check if any ingress host is part of atleast one domain suffix that shoudln't have tls
for suffix in self._tls_certificate_issuer_disable_for_domain_suffixes:
for host in hosts:
if host == suffix or host.endswith("." + suffix):
Expand Down
25 changes: 18 additions & 7 deletions tests/fiaas_deploy_daemon/deployer/kubernetes/test_ingress_deploy.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -703,14 +703,16 @@ def deployer_disable_tls_for_domain_suffixes(self, config, ingress_tls_disable_t
#Ingress Deployer
#list of ingresses
# config
@pytest.mark.parametrize("ingress_tls_disable_tls_for_domain_suffixes, ingresses_spec, expected_host_groups", [
@pytest.mark.parametrize("ingress_tls_disable_tls_for_domain_suffixes, ingresses_spec, expected_host_groups, hosts,use_suffixes", [
#Disable tls for one default host
({"tls_certificate_issuer_disable_for_domain_suffixes": ["xip.io"]},
[],
[
["testapp.127.0.0.1.xip.io"], #disable tls
["testapp.svc.test.example.com"], #enable tls
]),
],
["testapp.127.0.0.1.xip.io"],
True),
#Disable tls for a host with annotations
({"tls_certificate_issuer_disable_for_domain_suffixes": ["foo.example.com"]},
[
Expand All @@ -725,7 +727,9 @@ def deployer_disable_tls_for_domain_suffixes(self, config, ingress_tls_disable_t
["ann.foo.example.com"], #annotation disable tls
["ann.sub.example.com"], #annotation enable tls
["testapp.127.0.0.1.xip.io","testapp.svc.test.example.com"], #enable tls
]),
],
["ann.sub.example.com"],
False),
#Disable tls for a host with tls issue override
({"tls_certificate_issuer_disable_for_domain_suffixes": ["bar.other.cloud.com"]},
[
Expand All @@ -738,7 +742,9 @@ def deployer_disable_tls_for_domain_suffixes(self, config, ingress_tls_disable_t
["bar.other.cloud.com"], #disable tls with issuer override
["foo.other.cloud.com"], #enable tls with issuer override
["testapp.127.0.0.1.xip.io","testapp.svc.test.example.com"], #enable tls
]),
],
["bar.other.cloud.com"],
False),
#All use cases
({"tls_certificate_issuer_disable_for_domain_suffixes": ["foo.example.com","xip.io"]},
[
Expand All @@ -764,13 +770,14 @@ def deployer_disable_tls_for_domain_suffixes(self, config, ingress_tls_disable_t
["bar.example.com", "foo.bar.example.com","testapp.svc.test.example.com"], #tls enabled
["foo.example.com","sub.foo.example.com","testapp.127.0.0.1.xip.io"], #tls disabled
["other.cloud.com"] #has tls issuer override
]
)
],
["foo.example.com","sub.foo.example.com","testapp.127.0.0.1.xip.io"],
True)
],indirect=['ingress_tls_disable_tls_for_domain_suffixes']
)

@pytest.mark.usefixtures("delete")
def test_applies_tls_certificate_issuer_disable_for_domain_suffixes(self, deployer_disable_tls_for_domain_suffixes, ingress_tls_disable_tls_for_domain_suffixes, app_spec,ingresses_spec,expected_host_groups):
def test_applies_tls_certificate_issuer_disable_for_domain_suffixes(self, deployer_disable_tls_for_domain_suffixes, ingress_tls_disable_tls_for_domain_suffixes, app_spec,ingresses_spec,expected_host_groups,hosts,use_suffixes):
with mock.patch("k8s.models.ingress.Ingress.get_or_create") as get_or_create:
get_or_create.return_value = mock.create_autospec(Ingress, spec_set=True)
if len(ingresses_spec):
Expand All @@ -779,6 +786,10 @@ def test_applies_tls_certificate_issuer_disable_for_domain_suffixes(self, deploy
with mock.patch.object(ingress_tls_disable_tls_for_domain_suffixes, "apply",spec_set=True):
deployer_disable_tls_for_domain_suffixes.deploy(app_spec, LABELS)
host_groups = [sorted(call.args[2]) for call in ingress_tls_disable_tls_for_domain_suffixes.apply.call_args_list]
tls_issuer=DEFAULT_TLS_ISSUER
if hosts[0] == "bar.other.cloud.com":
tls_issuer = "certmanager.k8s.io/issuer"
ingress_tls_disable_tls_for_domain_suffixes.apply.assert_called_with(TypeMatcher(Ingress), app_spec, hosts,tls_issuer, use_suffixes=use_suffixes)
assert ingress_tls_disable_tls_for_domain_suffixes.apply.call_count == len(expected_host_groups)
assert expected_host_groups == sorted(host_groups)

Expand Down

0 comments on commit e496aab

Please sign in to comment.