diff --git a/config.yaml b/config.yaml
index 57d79621..ed3c48ee 100644
--- a/config.yaml
+++ b/config.yaml
@@ -30,7 +30,3 @@ options:
type: string
description: Email address of the technical manager of the Indico instance.
default: 'support-tech@mydomain.local'
- site_url:
- type: string
- description: URL through which Indico is accessed by users.
- default: ''
diff --git a/docs/how-to/configure-the-external-hostname.md b/docs/how-to/configure-the-external-hostname.md
index 79f43e02..55dbcc18 100644
--- a/docs/how-to/configure-the-external-hostname.md
+++ b/docs/how-to/configure-the-external-hostname.md
@@ -1,17 +1,13 @@
# How to configure the external hostname
-This charm exposes the `site_url` configuration option to specify the external hostname of the application.
-
-To expose the application it is recommended to set that configuration option and deploy and integrate with the [Nginx Ingress Integrator Operator](https://charmhub.io/nginx-ingress-integrator), that will be automatically configured with the values provided by the charm.
+To expose the application, deploy and integrate with the [Nginx Ingress Integrator Operator](https://charmhub.io/nginx-ingress-integrator). The charm will be automatically exposed at `[application name].local`, being `[application name]` the charm name. To provide a different hostname, set the [service-hostname](https://charmhub.io/nginx-ingress-integrator/configuration#service-hostname) configuration for the Nginx Ingress Integrator Operator.
Assuming Indico is already up and running as `indico`, you'll need to run the following commands:
```
-# Configure the external hostname
-juju config indico site_url=indico.local
# Deploy and integrate with the Nginx Ingress Integrator charm
juju deploy nginx-ingress-integrator
juju trust nginx-ingress-integrator --scope cluster # if RBAC is enabled
juju integrate nginx-ingress-integrator indico
+# Configure the external hostname
+juju config nginx-ingress-integrator service-hostname=indico.example
```
-
-For more details on the configuration options and their default values see the [configuration reference](https://charmhub.io/indico/configure).
\ No newline at end of file
diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md
index 37da405c..b0989aa3 100644
--- a/src-docs/charm.py.md
+++ b/src-docs/charm.py.md
@@ -26,7 +26,7 @@ Charm for Indico on kubernetes.
Attrs: on: Redis relation charm events.
-
+
### function `__init__`
diff --git a/src/charm.py b/src/charm.py
index 8876ab59..ba6b56da 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -8,20 +8,19 @@
import os
import typing
from re import findall
-from typing import Any, Dict, Iterator, List, Optional, Tuple
-from urllib.parse import urlparse
+from typing import Any, Dict, Iterator, List, Optional
import charms.loki_k8s.v0.loki_push_api
import ops
from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider
+from charms.nginx_ingress_integrator.v0.nginx_route import NginxRouteRequirer, require_nginx_route
from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer
-from charms.nginx_ingress_integrator.v0.nginx_route import require_nginx_route
from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider
from charms.redis_k8s.v0.redis import RedisRelationCharmEvents, RedisRequires
from ops.charm import ActionEvent, CharmBase, HookEvent, PebbleReadyEvent, RelationDepartedEvent
from ops.jujuversion import JujuVersion
from ops.main import main
-from ops.model import ActiveStatus, BlockedStatus, Container, MaintenanceStatus, WaitingStatus
+from ops.model import ActiveStatus, Container, MaintenanceStatus, WaitingStatus
from ops.pebble import ExecError
from database_observer import DatabaseObserver
@@ -100,7 +99,7 @@ def __init__(self, *args):
self.framework.observe(
self.on["indico-peers"].relation_departed, self._on_peer_relation_departed
)
- self._require_nginx_route()
+ self.nginx_route = self._require_nginx_route()
self._metrics_endpoint = MetricsEndpointProvider(
self,
@@ -128,11 +127,15 @@ def __init__(self, *args):
container_name="indico",
)
- def _require_nginx_route(self) -> None:
- """Require nginx ingress."""
- require_nginx_route(
+ def _require_nginx_route(self) -> NginxRouteRequirer:
+ """Require nginx ingress.
+
+ Returns:
+ The NginxRouteRequirer.
+ """
+ return require_nginx_route(
charm=self,
- service_hostname=self._get_external_hostname(),
+ service_hostname=f"{self.app.name}.local",
service_name=self.app.name,
service_port=8080,
)
@@ -148,46 +151,13 @@ def _are_pebble_instances_ready(self) -> bool:
for container_name in self.model.unit.containers
)
- def _is_configuration_valid(self) -> Tuple[bool, str]:
- """Validate charm configuration.
-
- Returns:
- Tuple containing as first element whether the configuration is valid.
- and a string with the error, if any, as second element.
- """
- site_url = typing.cast(str, self.config["site_url"])
- if site_url and not urlparse(site_url).hostname:
- return False, "Configuration option site_url is not valid"
- return True, ""
-
def _get_external_hostname(self) -> str:
- """Extract and return hostname from site_url or default to [application name].local.
-
- Returns:
- The site URL defined as part of the site_url configuration or a default value.
- """
- site_url = typing.cast(str, self.config["site_url"])
- if not site_url or not (hostname := urlparse(site_url).hostname):
- return f"{self.app.name}.local"
- return hostname
-
- def _get_external_scheme(self) -> str:
- """Extract and return schema from site_url.
-
- Returns:
- The HTTP schema.
- """
- site_url = typing.cast(str, self.config["site_url"])
- return urlparse(site_url).scheme if site_url else "http"
-
- def _get_external_port(self) -> Optional[int]:
- """Extract and return port from site_url.
+ """Extract and return hostname from the nginx-route relation data.
Returns:
- The port number.
+ The hostname configured in the NGINX ingress integrator.
"""
- site_url = typing.cast(str, self.config["site_url"])
- return urlparse(site_url).port
+ return self.nginx_route.config.get("service-hostname")
def _are_relations_ready(self, _) -> bool:
"""Check if the needed relations are established.
@@ -572,8 +542,8 @@ def _get_indico_env_config(self, container: Container) -> Dict:
"REDIS_CACHE_URL": self._get_redis_url("redis-cache"),
"SECRET_KEY": self._get_indico_secret_key_from_relation(),
"SERVICE_HOSTNAME": self._get_external_hostname(),
- "SERVICE_PORT": self._get_external_port(),
- "SERVICE_SCHEME": self._get_external_scheme(),
+ "SERVICE_PORT": "",
+ "SERVICE_SCHEME": "https",
"STORAGE_DICT": {
"default": "fs:/srv/indico/archive",
},
@@ -604,7 +574,7 @@ def _get_indico_env_config(self, container: Container) -> Dict:
saml_config: Dict[str, Any] = {
"strict": True,
"sp": {
- "entityId": self.config["site_url"],
+ "entityId": f"https://{self._get_external_hostname()}",
},
"idp": {
"entityId": self.state.saml_config.entity_id,
@@ -685,10 +655,6 @@ def _on_config_changed(self, event: HookEvent) -> None:
self.unit.status = WaitingStatus("Waiting for pebble")
return
self.model.unit.status = MaintenanceStatus("Configuring pod")
- is_valid, error = self._is_configuration_valid()
- if not is_valid:
- self.model.unit.status = BlockedStatus(error)
- return
for container_name in self.model.unit.containers:
self._config_pebble(self.unit.get_container(container_name))
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
index 3b584f27..026bd689 100644
--- a/tests/integration/conftest.py
+++ b/tests/integration/conftest.py
@@ -14,10 +14,10 @@
from pytest_operator.plugin import OpsTest
-@fixture(scope="module", name="external_url")
-def external_url_fixture():
+@fixture(scope="module", name="hostname")
+def hostname_fixture():
"""Provides the external URL for Indico."""
- return "https://events.staging.canonical.com"
+ return "events.staging.canonical.com"
@fixture(scope="module")
@@ -60,6 +60,7 @@ def requests_timeout():
async def app_fixture(
ops_test: OpsTest,
app_name: str,
+ hostname: str,
pytestconfig: Config,
):
"""Indico charm used for integration testing.
@@ -80,11 +81,9 @@ async def app_fixture(
ops_test.model.deploy("redis-k8s", "redis-broker", channel="latest/edge"),
ops_test.model.deploy("redis-k8s", "redis-cache", channel="latest/edge"),
ops_test.model.deploy(
- "nginx-ingress-integrator",
- channel="latest/edge",
- revision=133,
- series="focal",
- trust=True,
+ "nginx-ingress-integrator", channel="latest/edge", series="focal", config={
+ "service-hostname": hostname,
+ }, trust=True
),
)
await ops_test.model.wait_for_idle(
@@ -117,7 +116,6 @@ async def app_fixture(
ops_test.model.add_relation(f"{app_name}:redis-cache", "redis-cache"),
ops_test.model.add_relation(app_name, "nginx-ingress-integrator"),
)
- await ops_test.model.wait_for_idle(status="active", raise_on_error=False)
# Install saml_groups plugin
# Disabling line-too-long lint since we are installing the plugin via gh release
await application.set_config(
@@ -125,6 +123,7 @@ async def app_fixture(
"external_plugins": "https://github.com/canonical/flask-multipass-saml-groups/releases/download/1.2.1/flask_multipass_saml_groups-1.2.1-py3-none-any.whl" # noqa: E501 pylint: disable=line-too-long
}
)
+ await ops_test.model.wait_for_idle(status="active", raise_on_error=False)
yield application
diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py
index 25241225..44f1ceaf 100644
--- a/tests/integration/test_charm.py
+++ b/tests/integration/test_charm.py
@@ -38,14 +38,13 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application):
Assume that the charm has already been built and is running.
"""
assert ops_test.model
- # Read the IP address of indico
- status = await ops_test.model.get_status()
- unit = list(status.applications[app.name].units)[0]
- address = status["applications"][app.name]["units"][unit]["address"]
# Send request to bootstrap page and set Host header to app_name (which the application
# expects)
response = requests.get(
- f"http://{address}:8080/bootstrap", headers={"Host": f"{app.name}.local"}, timeout=10
+ "https://127.0.0.1/bootstrap",
+ headers={"Host": f"{app.name}.local"},
+ timeout=10,
+ verify=False, # nosec
)
assert response.status_code == 200
diff --git a/tests/integration/test_s3.py b/tests/integration/test_s3.py
index b9097049..951d2739 100644
--- a/tests/integration/test_s3.py
+++ b/tests/integration/test_s3.py
@@ -16,12 +16,19 @@
@pytest.mark.asyncio
@pytest.mark.abort_on_fail
@pytest.mark.usefixtures("s3_integrator")
-async def test_s3(app: Application, s3_integrator: Application, ops_test: OpsTest):
+async def test_s3(app: Application, s3_integrator: Application, ops_test: OpsTest, hostname: str):
"""
arrange: given charm integrated with S3.
act: do nothing.
assert: the pebble plan matches the S3 values as configured by the integrator.
"""
+ assert ops_test.model
+ await ops_test.model.applications["nginx-ingress-integrator"].set_config(
+ {"service-hostname": hostname}
+ )
+ # The linter does not recognize wait_for_idle as a method,
+ # since ops_test has a model as Optional, so this error must be ignored.
+ await ops_test.model.wait_for_idle(status="active") # type: ignore[union-attr]
# Application actually does have units
return_code, stdout, _ = await ops_test.juju(
"ssh", "--container", app.name, app.units[0].name, "pebble", "plan" # type: ignore
diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py
index 98bec4b4..06c0c8a2 100644
--- a/tests/integration/test_saml.py
+++ b/tests/integration/test_saml.py
@@ -7,12 +7,10 @@
import re
import socket
from unittest.mock import patch
-from urllib.parse import urlparse
import pytest
import requests
import urllib3.exceptions
-from ops import Application
from pytest_operator.plugin import OpsTest
@@ -21,27 +19,22 @@
@pytest.mark.usefixtures("saml_integrator")
async def test_saml_auth( # pylint: disable=too-many-arguments, too-many-positional-arguments
ops_test: OpsTest,
- app: Application,
saml_email: str,
saml_password: str,
requests_timeout: float,
- external_url: str,
+ hostname: str,
):
"""
arrange: given charm in its initial state
act: configure a SAML target url and fire SAML authentication
assert: The SAML authentication process is executed successfully.
"""
- # The linter does not recognize set_config as a method, so this errors must be ignored.
- await app.set_config( # type: ignore[attr-defined] # pylint: disable=W0106
- {"site_url": external_url}
- )
- # The linter does not recognize wait_for_idle as a method,
- # since ops_test has a model as Optional, so this error must be ignored.
- await ops_test.model.wait_for_idle(status="active") # type: ignore[union-attr]
+ assert ops_test.model
+ nginx_ingress_integrator_app = ops_test.model.applications["nginx-ingress-integrator"]
+ await nginx_ingress_integrator_app.set_config({"service-hostname": hostname})
+ await ops_test.model.wait_for_idle(status="active")
urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
- host = urlparse(external_url).netloc
original_getaddrinfo = socket.getaddrinfo
def patched_getaddrinfo(*args):
@@ -53,14 +46,14 @@ def patched_getaddrinfo(*args):
Returns:
Address information with localhost as the patched IP.
"""
- if args[0] == host:
+ if args[0] == hostname:
return original_getaddrinfo("127.0.0.1", *args[1:])
return original_getaddrinfo(*args)
with patch.multiple(socket, getaddrinfo=patched_getaddrinfo), requests.session() as session:
- session.get(f"https://{host}", verify=False)
+ session.get(f"https://{hostname}", verify=False)
login_page = session.get(
- f"https://{host}/login",
+ f"https://{hostname}/login",
verify=False,
timeout=requests_timeout,
)
@@ -88,7 +81,7 @@ def patched_getaddrinfo(*args):
)
assert len(saml_response_matches), saml_callback.text
session.post(
- f"https://{host}/multipass/saml/ubuntu/acs",
+ f"https://{hostname}/multipass/saml/ubuntu/acs",
data={
"RelayState": "None",
"SAMLResponse": saml_response_matches[0],
@@ -98,19 +91,18 @@ def patched_getaddrinfo(*args):
timeout=requests_timeout,
)
session.post(
- f"https://{host}/multipass/saml/ubuntu/acs",
+ f"https://{hostname}/multipass/saml/ubuntu/acs",
data={"SAMLResponse": saml_response_matches[0], "SameSite": "1"},
verify=False,
timeout=requests_timeout,
)
dashboard_page = session.get(
- f"https://{host}/register/ubuntu",
+ f"https://{hostname}/register/ubuntu",
verify=False,
timeout=requests_timeout,
)
assert dashboard_page.status_code == 200
# Revert SAML config for zap to be able to run
- await app.set_config( # type: ignore[attr-defined] # pylint: disable=W0106
- {"site_url": ""}
- )
+ # await ops_test.model.remove_relation("indico", "saml_integrator")
+ # await nginx_ingress_integrator_app.reset_config(["service-hostname"])
diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py
index 7a0af3f7..d6e3a59c 100644
--- a/tests/unit/test_base.py
+++ b/tests/unit/test_base.py
@@ -63,7 +63,7 @@ def set_up_all_relations(self):
)
self.nginx_route_relation_id = self.harness.add_relation( # pylint: disable=W0201
- "nginx-route", "ingress"
+ "nginx-route", "ingress", app_data={"service-hostname": "example.local"}
)
def is_ready(self, apps: List[str]):
@@ -77,8 +77,8 @@ def is_ready(self, apps: List[str]):
def set_relations_and_leader(self):
"""Set Indico relations, the leader and check container readiness."""
- self.set_up_all_relations()
self.harness.set_leader(True)
+ self.set_up_all_relations()
self.is_ready(
[
"indico",
diff --git a/tests/unit/test_core.py b/tests/unit/test_core.py
index 841594d2..3910a16f 100644
--- a/tests/unit/test_core.py
+++ b/tests/unit/test_core.py
@@ -126,7 +126,7 @@ def test_missing_relations(self):
act: trigger a configuration update
assert: the charm is in waiting status until all relations have been set
"""
- self.harness.update_config({"site_url": "foo"})
+ self.harness.update_config({"customization_debug": True})
self.assertEqual(
self.harness.model.unit.status,
ops.WaitingStatus("Waiting for redis-broker availability"),
@@ -214,7 +214,7 @@ def test_indico_pebble_ready_when_secrets_not_enabled(self, mock_exec, mock_juju
updated_plan_env["SECRET_KEY"],
)
self.assertEqual("indico.local", updated_plan_env["SERVICE_HOSTNAME"])
- self.assertIsNone(updated_plan_env["SERVICE_PORT"])
+ self.assertEqual("", updated_plan_env["SERVICE_PORT"])
self.assertEqual("redis://cache-host:1011", updated_plan_env["REDIS_CACHE_URL"])
self.assertFalse(updated_plan_env["ENABLE_ROOMBOOKING"])
self.assertEqual("support-tech@mydomain.local", updated_plan_env["INDICO_SUPPORT_EMAIL"])
@@ -266,7 +266,7 @@ def test_indico_pebble_ready_when_secrets_enabled(self, mock_exec, mock_juju_env
secret_value = secret.get_content().get("secret-key")
self.assertEqual(secret_value, updated_plan_env["SECRET_KEY"])
self.assertEqual("indico.local", updated_plan_env["SERVICE_HOSTNAME"])
- self.assertIsNone(updated_plan_env["SERVICE_PORT"])
+ self.assertEqual("", updated_plan_env["SERVICE_PORT"])
self.assertEqual("redis://cache-host:1011", updated_plan_env["REDIS_CACHE_URL"])
self.assertFalse(updated_plan_env["ENABLE_ROOMBOOKING"])
self.assertEqual("support-tech@mydomain.local", updated_plan_env["INDICO_SUPPORT_EMAIL"])
@@ -334,20 +334,19 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915
"indico_support_email": "example@email.local",
"indico_public_support_email": "public@email.local",
"indico_no_reply_email": "noreply@email.local",
- "site_url": "https://example.local:8080",
}
)
updated_plan = self.harness.get_container_pebble_plan("indico").to_dict()
updated_plan_env = updated_plan["services"]["indico"]["environment"]
- self.assertEqual("example.local", updated_plan_env["SERVICE_HOSTNAME"])
+ self.assertEqual("indico.local", updated_plan_env["SERVICE_HOSTNAME"])
self.assertTrue(updated_plan_env["ENABLE_ROOMBOOKING"])
self.assertEqual("example@email.local", updated_plan_env["INDICO_SUPPORT_EMAIL"])
self.assertEqual("public@email.local", updated_plan_env["INDICO_PUBLIC_SUPPORT_EMAIL"])
self.assertEqual("noreply@email.local", updated_plan_env["INDICO_NO_REPLY_EMAIL"])
self.assertEqual("https", updated_plan_env["SERVICE_SCHEME"])
- self.assertEqual(8080, updated_plan_env["SERVICE_PORT"])
+ self.assertEqual("", updated_plan_env["SERVICE_PORT"])
self.assertTrue(updated_plan_env["CUSTOMIZATION_DEBUG"])
storage_dict = literal_eval(updated_plan_env["STORAGE_DICT"])
self.assertEqual("s3", updated_plan_env["ATTACHMENT_STORAGE"])
@@ -362,13 +361,13 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915
auth_providers = literal_eval(updated_plan_env["INDICO_AUTH_PROVIDERS"])
self.assertEqual("saml", auth_providers["ubuntu"]["type"])
self.assertEqual(
- "https://example.local:8080",
+ "https://indico.local",
auth_providers["ubuntu"]["saml_config"]["sp"]["entityId"],
)
auth_providers = literal_eval(updated_plan_env["INDICO_AUTH_PROVIDERS"])
self.assertEqual("saml", auth_providers["ubuntu"]["type"])
applied_saml_config = auth_providers["ubuntu"]["saml_config"]
- self.assertEqual("https://example.local:8080", applied_saml_config["sp"]["entityId"])
+ self.assertEqual("https://indico.local", applied_saml_config["sp"]["entityId"])
self.assertEqual(saml_config.entity_id, applied_saml_config["idp"]["entityId"])
self.assertEqual(saml_config.certificates[0], applied_saml_config["idp"]["x509cert"])
self.assertEqual(
@@ -406,31 +405,6 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915
environment={},
)
- self.harness.update_config({"site_url": "https://example.local"})
- # ops testing harness doesn't rerun the charm's __init__
- # manually rerun the _require_nginx_route function
- self.harness.charm._require_nginx_route()
- nginx_route_relation_data = self.harness.get_relation_data(
- self.nginx_route_relation_id, self.harness.charm.app
- )
- self.assertEqual("example.local", nginx_route_relation_data["service-hostname"])
-
- @patch.object(ops.Container, "exec")
- def test_config_changed_when_config_invalid(self, mock_exec):
- """
- arrange: charm created and relations established
- act: trigger an invalid site URL configuration change for the charm
- assert: the unit reaches blocked status
- """
- mock_exec.return_value = MagicMock(wait_output=MagicMock(return_value=("", None)))
-
- self.set_relations_and_leader()
- self.harness.update_config({"site_url": "example.local"})
- self.assertEqual(
- self.harness.model.unit.status,
- ops.BlockedStatus("Configuration option site_url is not valid"),
- )
-
@patch.object(ops.Container, "exec")
def test_config_changed_with_external_resources(self, mock_exec):
"""