From a47904ee8689dff83eb66242c3143eaadc69c2ca Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 14:02:39 +0200 Subject: [PATCH 01/32] Remove site url --- .github/workflows/integration_test.yaml | 2 +- config.yaml | 4 -- .../v0/nginx_route.py | 12 +++--- src/charm.py | 39 +++++-------------- tests/integration/conftest.py | 11 ++++-- tests/integration/test_saml.py | 21 +++------- 6 files changed, 30 insertions(+), 59 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 08012655..dd9d758a 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -1,7 +1,7 @@ name: Integration tests on: - pull_request: + push: jobs: integration-tests: diff --git a/config.yaml b/config.yaml index bdbfb0a3..0bc436c3 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/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index c9a22dd8..61d86445 100644 --- a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -119,7 +119,7 @@ class _NginxRouteCharmEvents(ops.charm.CharmEvents): nginx_route_broken = ops.framework.EventSource(_NginxRouteBrokenEvent) -class _NginxRouteRequirer(ops.framework.Object): +class NginxRouteRequirer(ops.framework.Object): """This class defines the functionality for the 'requires' side of the 'nginx-route' relation. Hook events observed: @@ -148,7 +148,7 @@ def __init__( self._config_reconciliation, ) # Set default values. - self._config: typing.Dict[str, typing.Union[str, int, bool]] = { + self.config: typing.Dict[str, typing.Union[str, int, bool]] = { "service-namespace": self._charm.model.name, **config, } @@ -167,7 +167,7 @@ def _config_reconciliation(self, _event: typing.Any = None) -> None: } for delete_key in delete_keys: del relation_app_data[delete_key] - relation_app_data.update({k: str(v) for k, v in self._config.items()}) + relation_app_data.update({k: str(v) for k, v in self.config.items()}) # C901 is ignored since the method has too many ifs but wouldn't be @@ -195,7 +195,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to session_cookie_max_age: typing.Optional[int] = None, tls_secret_name: typing.Optional[str] = None, nginx_route_relation_name: str = "nginx-route", -) -> None: +) -> NginxRouteRequirer: """Set up nginx-route relation handlers on the requirer side. This function must be invoked in the charm class constructor. @@ -242,6 +242,8 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to nginx_route_relation_name: Specifies the relation name of the relation handled by this requirer class. The relation must have the nginx-route interface. + Returns: + the NginxRouteRequirer. """ config: typing.Dict[str, typing.Union[str, int, bool]] = {} if service_hostname is not None: @@ -281,7 +283,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to if tls_secret_name is not None: config["tls-secret-name"] = tls_secret_name - _NginxRouteRequirer( + return NginxRouteRequirer( charm=charm, config=config, nginx_route_relation_name=nginx_route_relation_name ) diff --git a/src/charm.py b/src/charm.py index 90158966..a057b8d5 100755 --- a/src/charm.py +++ b/src/charm.py @@ -13,14 +13,14 @@ import ops.lib from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider -from charms.nginx_ingress_integrator.v0.nginx_route import require_nginx_route +from charms.nginx_ingress_integrator.v0.nginx_route import NginxRouteRequirer, 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.framework import StoredState 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 @@ -102,7 +102,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, @@ -122,11 +122,11 @@ def __init__(self, *args): ) self._grafana_dashboards = GrafanaDashboardProvider(self) - def _require_nginx_route(self) -> None: + def _require_nginx_route(self) -> NginxRouteRequirer: """Require nginx ingress.""" - require_nginx_route( + return require_nginx_route( charm=self, - service_hostname=self._get_external_hostname(), + service_hostname=None, service_name=self.app.name, service_port=8080, ) @@ -142,28 +142,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 + return self.nginx_route.config["external_hostname"] def _get_external_scheme(self) -> str: """Extract and return schema from site_url. @@ -171,8 +156,7 @@ def _get_external_scheme(self) -> str: Returns: The HTTP schema. """ - site_url = typing.cast(str, self.config["site_url"]) - return urlparse(site_url).scheme if site_url else "http" + return "https" def _get_external_port(self) -> Optional[int]: """Extract and return port from site_url. @@ -180,8 +164,7 @@ def _get_external_port(self) -> Optional[int]: Returns: The port number. """ - site_url = typing.cast(str, self.config["site_url"]) - return urlparse(site_url).port + return 443 def _are_relations_ready(self, _) -> bool: """Check if the needed relations are established. @@ -621,10 +604,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 f70af96e..442530c0 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,7 +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", 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( diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index 92f5a3df..ecbfcd04 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -20,28 +20,19 @@ @pytest.mark.abort_on_fail @pytest.mark.usefixtures("saml_integrator") async def test_saml_auth( # pylint: disable=too-many-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] urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) - host = urlparse(external_url).netloc original_getaddrinfo = socket.getaddrinfo def patched_getaddrinfo(*args): @@ -53,14 +44,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, ) @@ -98,14 +89,14 @@ 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, ) From 31789ba0e8bec2c9b2b85425ce3917b2466fa636 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 15:42:52 +0200 Subject: [PATCH 02/32] Remove site url --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index a057b8d5..87a71439 100755 --- a/src/charm.py +++ b/src/charm.py @@ -126,7 +126,7 @@ def _require_nginx_route(self) -> NginxRouteRequirer: """Require nginx ingress.""" return require_nginx_route( charm=self, - service_hostname=None, + service_hostname=self.app.name, service_name=self.app.name, service_port=8080, ) From 26f6e0562bb77eed2be926cde6db446aef6dd852 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 15:47:56 +0200 Subject: [PATCH 03/32] Remove site url --- src/charm.py | 11 +++++++---- tests/integration/test_saml.py | 3 --- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/charm.py b/src/charm.py index 87a71439..350b1aa3 100755 --- a/src/charm.py +++ b/src/charm.py @@ -141,14 +141,17 @@ def _are_pebble_instances_ready(self) -> bool: self.unit.get_container(container_name).can_connect() for container_name in self.model.unit.containers ) + + def _get_site_url(self) -> str: + return f"https://{self._get_external_hostname}" def _get_external_hostname(self) -> str: - """Extract and return hostname from site_url or default to [application name].local. + """Extract and return hostname from the nginx-route relation data. Returns: - The site URL defined as part of the site_url configuration or a default value. + The hostname configured in the NGINX ingress integrator. """ - return self.nginx_route.config["external_hostname"] + return self.nginx_route.config.get("external_hostname") def _get_external_scheme(self) -> str: """Extract and return schema from site_url. @@ -523,7 +526,7 @@ def _get_indico_env_config(self, container: Container) -> Dict: saml_config: Dict[str, Any] = { "strict": True, "sp": { - "entityId": self.config["site_url"], + "entityId": self._get_site_url(), }, "idp": { "entityId": self.state.saml_config.entity_id, diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index ecbfcd04..954a3472 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -102,6 +102,3 @@ def patched_getaddrinfo(*args): ) 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": ""} - ) From 5b435d289e9efb31f6ba5f7343d9b9dd67bbb94c Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 16:21:12 +0200 Subject: [PATCH 04/32] Remove site url --- lib/charms/nginx_ingress_integrator/v0/nginx_route.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index 61d86445..192bcbb8 100644 --- a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -163,7 +163,7 @@ def _config_reconciliation(self, _event: typing.Any = None) -> None: delete_keys = { relation_field for relation_field in relation_app_data - if relation_field not in self._config + if relation_field not in self.config } for delete_key in delete_keys: del relation_app_data[delete_key] From f3fd209c22f7e1e545ae4848bece799e1604626c Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 17:07:58 +0200 Subject: [PATCH 05/32] Remove site url --- src/charm.py | 7 +++---- tests/integration/test_charm.py | 4 ++-- tests/integration/test_saml.py | 3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/charm.py b/src/charm.py index 350b1aa3..20301e7e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -8,8 +8,7 @@ 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 ops.lib from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider @@ -141,9 +140,9 @@ def _are_pebble_instances_ready(self) -> bool: self.unit.get_container(container_name).can_connect() for container_name in self.model.unit.containers ) - + def _get_site_url(self) -> str: - return f"https://{self._get_external_hostname}" + return f"https://{self._get_external_hostname()}" def _get_external_hostname(self) -> str: """Extract and return hostname from the nginx-route relation data. diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 926a7608..9874b8b0 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -32,7 +32,7 @@ async def test_active(app: Application): @pytest.mark.asyncio @pytest.mark.abort_on_fail -async def test_indico_is_up(ops_test: OpsTest, app: Application): +async def test_indico_is_up(ops_test: OpsTest, app: Application, hostname: str): """Check that the bootstrap page is reachable. Assume that the charm has already been built and is running. @@ -45,7 +45,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): # 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 + f"http://{address}:8080/bootstrap", headers={"Host": hostname}, timeout=10 ) assert response.status_code == 200 diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index 954a3472..ba5a1a69 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -7,7 +7,6 @@ import re import socket from unittest.mock import patch -from urllib.parse import urlparse import pytest import requests @@ -79,7 +78,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], From cc93c49177af85e5de43955319d162c0dfed1ca8 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 17:27:26 +0200 Subject: [PATCH 06/32] Remove site url --- src/charm.py | 2 +- tests/integration/conftest.py | 4 +--- tests/integration/test_charm.py | 4 ++-- tests/integration/test_s3.py | 8 +++++++- tests/integration/test_saml.py | 8 +++++++- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/charm.py b/src/charm.py index 20301e7e..92efda3b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -125,7 +125,7 @@ def _require_nginx_route(self) -> NginxRouteRequirer: """Require nginx ingress.""" return require_nginx_route( charm=self, - service_hostname=self.app.name, + service_hostname=f"{self.app.name}.local", service_name=self.app.name, service_port=8080, ) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 442530c0..3db7cac1 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -81,9 +81,7 @@ 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", series="focal", config={ - "service-hostname": hostname, - }, trust=True + "nginx-ingress-integrator", channel="latest/edge", series="focal", trust=True ), ) await ops_test.model.wait_for_idle( diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 9874b8b0..926a7608 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -32,7 +32,7 @@ async def test_active(app: Application): @pytest.mark.asyncio @pytest.mark.abort_on_fail -async def test_indico_is_up(ops_test: OpsTest, app: Application, hostname: str): +async def test_indico_is_up(ops_test: OpsTest, app: Application): """Check that the bootstrap page is reachable. Assume that the charm has already been built and is running. @@ -45,7 +45,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application, hostname: str): # 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": hostname}, timeout=10 + f"http://{address}:8080/bootstrap", headers={"Host": f"{app.name}.local"}, timeout=10 ) assert response.status_code == 200 diff --git a/tests/integration/test_s3.py b/tests/integration/test_s3.py index 8b130fc4..e55cc0d8 100644 --- a/tests/integration/test_s3.py +++ b/tests/integration/test_s3.py @@ -16,12 +16,18 @@ @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. """ + await ops_test.model.applications["nginx-ingress-integrator"].set_config( + {"external_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 ba5a1a69..deb4f144 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -19,7 +19,7 @@ @pytest.mark.abort_on_fail @pytest.mark.usefixtures("saml_integrator") async def test_saml_auth( # pylint: disable=too-many-arguments - app: Application, + ops_test: OpsTest, saml_email: str, saml_password: str, requests_timeout: float, @@ -30,6 +30,9 @@ async def test_saml_auth( # pylint: disable=too-many-arguments act: configure a SAML target url and fire SAML authentication assert: The SAML authentication process is executed successfully. """ + await ops_test.model.applications["nginx-ingress-integrator"].set_config( + {"external_hostname": hostname} + ) urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) original_getaddrinfo = socket.getaddrinfo @@ -101,3 +104,6 @@ def patched_getaddrinfo(*args): ) assert dashboard_page.status_code == 200 # Revert SAML config for zap to be able to run + await ops_test.model.applications["nginx-ingress-integrator"].set_config( + {"external_hostname": ""} + ) From ecd1a01e17153ba88b5368b7bc094ee5ee6b3715 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 18:03:15 +0200 Subject: [PATCH 07/32] fix some stuff --- src/charm.py | 16 +++++++++--- tests/integration/conftest.py | 1 - tests/integration/test_s3.py | 1 + tests/integration/test_saml.py | 11 +++------ tests/unit/test_core.py | 45 +++++++++++----------------------- 5 files changed, 32 insertions(+), 42 deletions(-) diff --git a/src/charm.py b/src/charm.py index 92efda3b..0a171077 100755 --- a/src/charm.py +++ b/src/charm.py @@ -122,7 +122,12 @@ def __init__(self, *args): self._grafana_dashboards = GrafanaDashboardProvider(self) def _require_nginx_route(self) -> NginxRouteRequirer: - """Require nginx ingress.""" + """Require nginx ingress. + + Returns: + The NginxRouteRequirer. + """ + print("PASOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO") return require_nginx_route( charm=self, service_hostname=f"{self.app.name}.local", @@ -142,6 +147,11 @@ def _are_pebble_instances_ready(self) -> bool: ) def _get_site_url(self) -> str: + """Get the site URL. + + Returns: + The site's URL. + """ return f"https://{self._get_external_hostname()}" def _get_external_hostname(self) -> str: @@ -153,7 +163,7 @@ def _get_external_hostname(self) -> str: return self.nginx_route.config.get("external_hostname") def _get_external_scheme(self) -> str: - """Extract and return schema from site_url. + """Get the HTTP schema. Returns: The HTTP schema. @@ -161,7 +171,7 @@ def _get_external_scheme(self) -> str: return "https" def _get_external_port(self) -> Optional[int]: - """Extract and return port from site_url. + """Get the HTTP port. Returns: The port number. diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 3db7cac1..792bc501 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -60,7 +60,6 @@ def requests_timeout(): async def app_fixture( ops_test: OpsTest, app_name: str, - hostname: str, pytestconfig: Config, ): """Indico charm used for integration testing. diff --git a/tests/integration/test_s3.py b/tests/integration/test_s3.py index e55cc0d8..12aa960a 100644 --- a/tests/integration/test_s3.py +++ b/tests/integration/test_s3.py @@ -22,6 +22,7 @@ async def test_s3(app: Application, s3_integrator: Application, ops_test: OpsTes 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( {"external_hostname": hostname} ) diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index deb4f144..8c542d4b 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -11,7 +11,6 @@ import pytest import requests import urllib3.exceptions -from ops import Application from pytest_operator.plugin import OpsTest @@ -30,9 +29,9 @@ async def test_saml_auth( # pylint: disable=too-many-arguments act: configure a SAML target url and fire SAML authentication assert: The SAML authentication process is executed successfully. """ - await ops_test.model.applications["nginx-ingress-integrator"].set_config( - {"external_hostname": hostname} - ) + assert ops_test.model + nginx_ingress_integrator_app = ops_test.model.applications["nginx-ingress-integrator"] + await nginx_ingress_integrator_app.set_config({"external_hostname": hostname}) urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) original_getaddrinfo = socket.getaddrinfo @@ -104,6 +103,4 @@ def patched_getaddrinfo(*args): ) assert dashboard_page.status_code == 200 # Revert SAML config for zap to be able to run - await ops_test.model.applications["nginx-ingress-integrator"].set_config( - {"external_hostname": ""} - ) + await nginx_ingress_integrator_app.set_config({"external_hostname": ""}) diff --git a/tests/unit/test_core.py b/tests/unit/test_core.py index 7a0d6352..7eeb9c29 100644 --- a/tests/unit/test_core.py +++ b/tests/unit/test_core.py @@ -38,7 +38,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"), @@ -246,20 +246,28 @@ 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", } ) + # 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 + ) + print(nginx_route_relation_data) + self.assertEqual("indico.local", nginx_route_relation_data["service-hostname"]) + 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(443, 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"]) @@ -274,13 +282,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( @@ -318,31 +326,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): """ From 4b185b50bc21b9614ea1f870d1a76f9493183ae3 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 09:35:23 +0200 Subject: [PATCH 08/32] fix unit tests --- src-docs/charm.py.md | 2 +- src/charm.py | 13 ++----------- tests/unit/test_base.py | 4 ++-- tests/unit/test_core.py | 7 +++---- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src-docs/charm.py.md b/src-docs/charm.py.md index 49135725..5490297b 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 0a171077..5886017b 100755 --- a/src/charm.py +++ b/src/charm.py @@ -127,7 +127,6 @@ def _require_nginx_route(self) -> NginxRouteRequirer: Returns: The NginxRouteRequirer. """ - print("PASOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO") return require_nginx_route( charm=self, service_hostname=f"{self.app.name}.local", @@ -160,7 +159,7 @@ def _get_external_hostname(self) -> str: Returns: The hostname configured in the NGINX ingress integrator. """ - return self.nginx_route.config.get("external_hostname") + return self.nginx_route.config.get("service-hostname") def _get_external_scheme(self) -> str: """Get the HTTP schema. @@ -170,14 +169,6 @@ def _get_external_scheme(self) -> str: """ return "https" - def _get_external_port(self) -> Optional[int]: - """Get the HTTP port. - - Returns: - The port number. - """ - return 443 - def _are_relations_ready(self, _) -> bool: """Check if the needed relations are established. @@ -503,7 +494,7 @@ def _get_indico_env_config(self, container: Container) -> Dict: "REDIS_CACHE_URL": self.redis_cache.url, "SECRET_KEY": self._get_indico_secret_key_from_relation(), "SERVICE_HOSTNAME": self._get_external_hostname(), - "SERVICE_PORT": self._get_external_port(), + "SERVICE_PORT": "", "SERVICE_SCHEME": self._get_external_scheme(), "STORAGE_DICT": { "default": "fs:/srv/indico/archive", diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index b5fec98a..9fc767e4 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -44,7 +44,7 @@ def set_up_all_relations(self): "redis-cache", "redis-cache", unit_data={"hostname": "cache-host", "port": "1011"} ) 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]): @@ -58,8 +58,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 7eeb9c29..8d4b11b1 100644 --- a/tests/unit/test_core.py +++ b/tests/unit/test_core.py @@ -126,7 +126,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"]) @@ -178,7 +178,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"]) @@ -255,7 +255,6 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915 nginx_route_relation_data = self.harness.get_relation_data( self.nginx_route_relation_id, self.harness.charm.app ) - print(nginx_route_relation_data) self.assertEqual("indico.local", nginx_route_relation_data["service-hostname"]) updated_plan = self.harness.get_container_pebble_plan("indico").to_dict() @@ -267,7 +266,7 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915 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(443, 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"]) From 687a40865fac0e76052ddb7c4bcf8d47fe2c4058 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 09:57:39 +0200 Subject: [PATCH 09/32] Update docs --- docs/how-to/configure-the-external-hostname.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) 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 From 2d50ea9268d4ebda4b9d5006eec1ab77c28aa5b9 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 09:59:31 +0200 Subject: [PATCH 10/32] fix wfs --- .github/workflows/integration_test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index dd9d758a..08012655 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -1,7 +1,7 @@ name: Integration tests on: - push: + pull_request: jobs: integration-tests: From 1a2aa63fe4f6f10c090e7660333b6813c1170c51 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 10:06:34 +0200 Subject: [PATCH 11/32] undo lib changes --- .../nginx_ingress_integrator/v0/nginx_route.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index 192bcbb8..c9a22dd8 100644 --- a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -119,7 +119,7 @@ class _NginxRouteCharmEvents(ops.charm.CharmEvents): nginx_route_broken = ops.framework.EventSource(_NginxRouteBrokenEvent) -class NginxRouteRequirer(ops.framework.Object): +class _NginxRouteRequirer(ops.framework.Object): """This class defines the functionality for the 'requires' side of the 'nginx-route' relation. Hook events observed: @@ -148,7 +148,7 @@ def __init__( self._config_reconciliation, ) # Set default values. - self.config: typing.Dict[str, typing.Union[str, int, bool]] = { + self._config: typing.Dict[str, typing.Union[str, int, bool]] = { "service-namespace": self._charm.model.name, **config, } @@ -163,11 +163,11 @@ def _config_reconciliation(self, _event: typing.Any = None) -> None: delete_keys = { relation_field for relation_field in relation_app_data - if relation_field not in self.config + if relation_field not in self._config } for delete_key in delete_keys: del relation_app_data[delete_key] - relation_app_data.update({k: str(v) for k, v in self.config.items()}) + relation_app_data.update({k: str(v) for k, v in self._config.items()}) # C901 is ignored since the method has too many ifs but wouldn't be @@ -195,7 +195,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to session_cookie_max_age: typing.Optional[int] = None, tls_secret_name: typing.Optional[str] = None, nginx_route_relation_name: str = "nginx-route", -) -> NginxRouteRequirer: +) -> None: """Set up nginx-route relation handlers on the requirer side. This function must be invoked in the charm class constructor. @@ -242,8 +242,6 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to nginx_route_relation_name: Specifies the relation name of the relation handled by this requirer class. The relation must have the nginx-route interface. - Returns: - the NginxRouteRequirer. """ config: typing.Dict[str, typing.Union[str, int, bool]] = {} if service_hostname is not None: @@ -283,7 +281,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to if tls_secret_name is not None: config["tls-secret-name"] = tls_secret_name - return NginxRouteRequirer( + _NginxRouteRequirer( charm=charm, config=config, nginx_route_relation_name=nginx_route_relation_name ) From 5b60f81c29d9bd2b15b4dee195bc59c1fbf0e187 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 12:29:52 +0200 Subject: [PATCH 12/32] Fix integration tests --- tests/integration/test_charm.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 926a7608..a1ce5ec7 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -45,7 +45,10 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): # 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 + f"https://{address}:8080/bootstrap", + headers={"Host": f"{app.name}.local"}, + timeout=10, + verify=False, ) assert response.status_code == 200 From 7716d3c1f56b899ae6c7e156ef8702cb563bebec Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 12:30:21 +0200 Subject: [PATCH 13/32] update lib --- .../nginx_ingress_integrator/v0/nginx_route.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index c9a22dd8..a2ec38ec 100644 --- a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -86,7 +86,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 6 +LIBPATCH = 7 __all__ = ["require_nginx_route", "provide_nginx_route"] @@ -119,7 +119,7 @@ class _NginxRouteCharmEvents(ops.charm.CharmEvents): nginx_route_broken = ops.framework.EventSource(_NginxRouteBrokenEvent) -class _NginxRouteRequirer(ops.framework.Object): +class NginxRouteRequirer(ops.framework.Object): """This class defines the functionality for the 'requires' side of the 'nginx-route' relation. Hook events observed: @@ -148,7 +148,7 @@ def __init__( self._config_reconciliation, ) # Set default values. - self._config: typing.Dict[str, typing.Union[str, int, bool]] = { + self.config: typing.Dict[str, typing.Union[str, int, bool]] = { "service-namespace": self._charm.model.name, **config, } @@ -163,11 +163,11 @@ def _config_reconciliation(self, _event: typing.Any = None) -> None: delete_keys = { relation_field for relation_field in relation_app_data - if relation_field not in self._config + if relation_field not in self.config } for delete_key in delete_keys: del relation_app_data[delete_key] - relation_app_data.update({k: str(v) for k, v in self._config.items()}) + relation_app_data.update({k: str(v) for k, v in self.config.items()}) # C901 is ignored since the method has too many ifs but wouldn't be @@ -195,7 +195,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to session_cookie_max_age: typing.Optional[int] = None, tls_secret_name: typing.Optional[str] = None, nginx_route_relation_name: str = "nginx-route", -) -> None: +) -> NginxRouteRequirer: """Set up nginx-route relation handlers on the requirer side. This function must be invoked in the charm class constructor. @@ -242,6 +242,9 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to nginx_route_relation_name: Specifies the relation name of the relation handled by this requirer class. The relation must have the nginx-route interface. + + Returns: + the NginxRouteRequirer. """ config: typing.Dict[str, typing.Union[str, int, bool]] = {} if service_hostname is not None: @@ -281,7 +284,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to if tls_secret_name is not None: config["tls-secret-name"] = tls_secret_name - _NginxRouteRequirer( + return NginxRouteRequirer( charm=charm, config=config, nginx_route_relation_name=nginx_route_relation_name ) From 1dad35ba9d25aa2e81f2aa2c6f6be70d943774ef Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 12:59:46 +0200 Subject: [PATCH 14/32] fix tests --- tests/integration/test_charm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index a1ce5ec7..981efae4 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -45,10 +45,10 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): # Send request to bootstrap page and set Host header to app_name (which the application # expects) response = requests.get( - f"https://{address}:8080/bootstrap", - headers={"Host": f"{app.name}.local"}, + f"https://{app.name}.local:8080/bootstrap", timeout=10, verify=False, + proxies = {"https": "http://127.0.0.1:8080"}, ) assert response.status_code == 200 From ccd1e3bf5a55a1f17b7e458c05cf724579e8a069 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 13:25:34 +0200 Subject: [PATCH 15/32] fix tests --- tests/integration/test_s3.py | 2 +- tests/integration/test_saml.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/test_s3.py b/tests/integration/test_s3.py index 12aa960a..05600d2a 100644 --- a/tests/integration/test_s3.py +++ b/tests/integration/test_s3.py @@ -24,7 +24,7 @@ async def test_s3(app: Application, s3_integrator: Application, ops_test: OpsTes """ assert ops_test.model await ops_test.model.applications["nginx-ingress-integrator"].set_config( - {"external_hostname": hostname} + {"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. diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index 8c542d4b..06016d8b 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -31,7 +31,7 @@ async def test_saml_auth( # pylint: disable=too-many-arguments """ assert ops_test.model nginx_ingress_integrator_app = ops_test.model.applications["nginx-ingress-integrator"] - await nginx_ingress_integrator_app.set_config({"external_hostname": hostname}) + await nginx_ingress_integrator_app.set_config({"service-hostname": hostname}) urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) original_getaddrinfo = socket.getaddrinfo @@ -103,4 +103,4 @@ def patched_getaddrinfo(*args): ) assert dashboard_page.status_code == 200 # Revert SAML config for zap to be able to run - await nginx_ingress_integrator_app.set_config({"external_hostname": ""}) + await nginx_ingress_integrator_app.set_config({"service-hostname": ""}) From fe25d94556fe9c570313db2996581a0828c77e92 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 13:55:42 +0200 Subject: [PATCH 16/32] fix tests --- tests/integration/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 981efae4..994ef883 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -48,7 +48,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): f"https://{app.name}.local:8080/bootstrap", timeout=10, verify=False, - proxies = {"https": "http://127.0.0.1:8080"}, + proxies = {"https": f"http://{address}:8080"}, ) assert response.status_code == 200 From 80d9bcddf012eec48d2cbd8640192c2cd4657c37 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 15:45:14 +0200 Subject: [PATCH 17/32] fix tests --- tests/integration/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 994ef883..b04960ac 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -45,7 +45,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): # Send request to bootstrap page and set Host header to app_name (which the application # expects) response = requests.get( - f"https://{app.name}.local:8080/bootstrap", + f"https://{app.name}.local/bootstrap", timeout=10, verify=False, proxies = {"https": f"http://{address}:8080"}, From 8e528d1734f3aabf59970879d242a2a3d0717309 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 16:17:12 +0200 Subject: [PATCH 18/32] fix tests --- tests/integration/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index b04960ac..097f5589 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -48,7 +48,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): f"https://{app.name}.local/bootstrap", timeout=10, verify=False, - proxies = {"https": f"http://{address}:8080"}, + proxies = {"https": f"http://{address}"}, ) assert response.status_code == 200 From 1ae04e3ce35208c29d5bcd8c5d6c019fa2f1ab57 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 16:47:10 +0200 Subject: [PATCH 19/32] fix tests --- tests/integration/test_charm.py | 2 +- tests/unit/test_core.py | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 097f5589..e7ee125a 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -48,7 +48,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): f"https://{app.name}.local/bootstrap", timeout=10, verify=False, - proxies = {"https": f"http://{address}"}, + proxies={"https": f"http://{address}:8080"}, ) assert response.status_code == 200 diff --git a/tests/unit/test_core.py b/tests/unit/test_core.py index 8d4b11b1..344638a5 100644 --- a/tests/unit/test_core.py +++ b/tests/unit/test_core.py @@ -249,14 +249,6 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915 } ) - # 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("indico.local", nginx_route_relation_data["service-hostname"]) - updated_plan = self.harness.get_container_pebble_plan("indico").to_dict() updated_plan_env = updated_plan["services"]["indico"]["environment"] From 8c9a9c6e62849cabdb3fb6ec07ef37553c49e004 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Tue, 7 May 2024 17:11:27 +0200 Subject: [PATCH 20/32] fix tests --- tests/integration/test_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index e7ee125a..6d816a26 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -48,7 +48,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): f"https://{app.name}.local/bootstrap", timeout=10, verify=False, - proxies={"https": f"http://{address}:8080"}, + proxies={"https": f"http://127.0.0.1"}, ) assert response.status_code == 200 From f79db7af056647d4338e7869f2070632797b8f44 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 8 May 2024 10:42:30 +0200 Subject: [PATCH 21/32] fix tests --- src/charm.py | 20 ++------------------ tests/integration/test_charm.py | 2 +- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/charm.py b/src/charm.py index 5886017b..c60eca35 100755 --- a/src/charm.py +++ b/src/charm.py @@ -145,14 +145,6 @@ def _are_pebble_instances_ready(self) -> bool: for container_name in self.model.unit.containers ) - def _get_site_url(self) -> str: - """Get the site URL. - - Returns: - The site's URL. - """ - return f"https://{self._get_external_hostname()}" - def _get_external_hostname(self) -> str: """Extract and return hostname from the nginx-route relation data. @@ -161,14 +153,6 @@ def _get_external_hostname(self) -> str: """ return self.nginx_route.config.get("service-hostname") - def _get_external_scheme(self) -> str: - """Get the HTTP schema. - - Returns: - The HTTP schema. - """ - return "https" - def _are_relations_ready(self, _) -> bool: """Check if the needed relations are established. @@ -495,7 +479,7 @@ def _get_indico_env_config(self, container: Container) -> Dict: "SECRET_KEY": self._get_indico_secret_key_from_relation(), "SERVICE_HOSTNAME": self._get_external_hostname(), "SERVICE_PORT": "", - "SERVICE_SCHEME": self._get_external_scheme(), + "SERVICE_SCHEME": "https", "STORAGE_DICT": { "default": "fs:/srv/indico/archive", }, @@ -526,7 +510,7 @@ def _get_indico_env_config(self, container: Container) -> Dict: saml_config: Dict[str, Any] = { "strict": True, "sp": { - "entityId": self._get_site_url(), + "entityId": f"https://{self._get_external_hostname()}", }, "idp": { "entityId": self.state.saml_config.entity_id, diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 6d816a26..dc0d6a49 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -48,7 +48,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): f"https://{app.name}.local/bootstrap", timeout=10, verify=False, - proxies={"https": f"http://127.0.0.1"}, + proxies={"https": f"https://127.0.0.1"}, ) assert response.status_code == 200 From 29f7a13ce697022bfdcf685685c2a395a4dc9ba6 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 8 May 2024 11:30:44 +0200 Subject: [PATCH 22/32] fix tests --- tests/integration/test_charm.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index dc0d6a49..5d0a13c2 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -45,10 +45,11 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): # Send request to bootstrap page and set Host header to app_name (which the application # expects) response = requests.get( - f"https://{app.name}.local/bootstrap", + f"https://127.0.0.1/bootstrap", + headers={"Host": f"{app.name}.local"}, timeout=10, verify=False, - proxies={"https": f"https://127.0.0.1"}, + # proxies={"https": f"https://127.0.0.1"}, ) assert response.status_code == 200 From 35dc3a30b3bcafb38232e411066a1e3f910ed362 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 8 May 2024 12:01:52 +0200 Subject: [PATCH 23/32] fix zap config --- .github/workflows/integration_test.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 08012655..6c0a3061 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -13,9 +13,10 @@ jobs: modules: '["test_actions.py", "test_charm.py", "test_s3.py", "test_saml.py"]' trivy-fs-enabled: true trivy-image-config: "trivy.yaml" - zap-before-command: "curl -H \"Host: indico.local\" http://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" + zap-before-command: "curl -H \"Host: indico.local\" https://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" zap-enabled: true zap-cmd-options: '-T 60 -z "-addoninstall jython" --hook "/zap/wrk/tests/zap/hook.py"' zap-target: localhost zap-target-port: 80 + zap-target-protocol: "https" zap-rules-file-name: "zap_rules.tsv" From e97adfee158536c3d31c64a2f65f55a9f619e9fd Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 8 May 2024 14:21:21 +0200 Subject: [PATCH 24/32] fix zap config --- .github/workflows/integration_test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 6c0a3061..d3b97c6c 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -13,7 +13,7 @@ jobs: modules: '["test_actions.py", "test_charm.py", "test_s3.py", "test_saml.py"]' trivy-fs-enabled: true trivy-image-config: "trivy.yaml" - zap-before-command: "curl -H \"Host: indico.local\" https://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" + zap-before-command: "curl -k -H \"Host: indico.local\" https://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" zap-enabled: true zap-cmd-options: '-T 60 -z "-addoninstall jython" --hook "/zap/wrk/tests/zap/hook.py"' zap-target: localhost From ad3daa3cd2bce5beb335c0cc2a768c6e572598aa Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 8 May 2024 16:18:22 +0200 Subject: [PATCH 25/32] test stuff --- .github/workflows/integration_test.yaml | 3 +-- src/charm.py | 2 +- tests/integration/test_charm.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index d3b97c6c..514816b7 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -13,10 +13,9 @@ jobs: modules: '["test_actions.py", "test_charm.py", "test_s3.py", "test_saml.py"]' trivy-fs-enabled: true trivy-image-config: "trivy.yaml" - zap-before-command: "curl -k -H \"Host: indico.local\" https://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" + zap-before-command: "curl -k -H \"Host: indico.local\" http://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" zap-enabled: true zap-cmd-options: '-T 60 -z "-addoninstall jython" --hook "/zap/wrk/tests/zap/hook.py"' zap-target: localhost zap-target-port: 80 - zap-target-protocol: "https" zap-rules-file-name: "zap_rules.tsv" diff --git a/src/charm.py b/src/charm.py index c60eca35..e0229f0c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -479,7 +479,7 @@ def _get_indico_env_config(self, container: Container) -> Dict: "SECRET_KEY": self._get_indico_secret_key_from_relation(), "SERVICE_HOSTNAME": self._get_external_hostname(), "SERVICE_PORT": "", - "SERVICE_SCHEME": "https", + "SERVICE_SCHEME": "http", "STORAGE_DICT": { "default": "fs:/srv/indico/archive", }, diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 5d0a13c2..4ca73870 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -45,7 +45,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): # Send request to bootstrap page and set Host header to app_name (which the application # expects) response = requests.get( - f"https://127.0.0.1/bootstrap", + f"http://127.0.0.1/bootstrap", headers={"Host": f"{app.name}.local"}, timeout=10, verify=False, From 3195112ade4e9b7391b6734ebea64b4f05a71e12 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 8 May 2024 18:00:32 +0200 Subject: [PATCH 26/32] test stuff --- tests/integration/test_saml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index 06016d8b..2fba5f38 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -103,4 +103,4 @@ def patched_getaddrinfo(*args): ) assert dashboard_page.status_code == 200 # Revert SAML config for zap to be able to run - await nginx_ingress_integrator_app.set_config({"service-hostname": ""}) + await nginx_ingress_integrator_app.reset_config(["service-hostname"]) From 4ddc7da2ffaee52f2034a08f7fa06c014209f9d7 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Fri, 10 May 2024 12:24:11 +0200 Subject: [PATCH 27/32] fix tests --- tests/integration/test_charm.py | 9 ++------- tests/integration/test_saml.py | 1 + tests/unit/test_core.py | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 4ca73870..59e63512 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -38,18 +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://127.0.0.1/bootstrap", + "http://127.0.0.1/bootstrap", headers={"Host": f"{app.name}.local"}, timeout=10, - verify=False, - # proxies={"https": f"https://127.0.0.1"}, + verify=False, # nosec ) assert response.status_code == 200 diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index 2fba5f38..46396184 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -32,6 +32,7 @@ async def test_saml_auth( # pylint: disable=too-many-arguments 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) original_getaddrinfo = socket.getaddrinfo diff --git a/tests/unit/test_core.py b/tests/unit/test_core.py index 344638a5..7bb68731 100644 --- a/tests/unit/test_core.py +++ b/tests/unit/test_core.py @@ -257,7 +257,7 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915 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("http", updated_plan_env["SERVICE_SCHEME"]) self.assertEqual("", updated_plan_env["SERVICE_PORT"]) self.assertTrue(updated_plan_env["CUSTOMIZATION_DEBUG"]) storage_dict = literal_eval(updated_plan_env["STORAGE_DICT"]) From 3ae55715871358c7e553a13e4fcab67fea74e75c Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 20 May 2024 13:11:00 +0200 Subject: [PATCH 28/32] fix indico config --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index e0229f0c..c60eca35 100755 --- a/src/charm.py +++ b/src/charm.py @@ -479,7 +479,7 @@ def _get_indico_env_config(self, container: Container) -> Dict: "SECRET_KEY": self._get_indico_secret_key_from_relation(), "SERVICE_HOSTNAME": self._get_external_hostname(), "SERVICE_PORT": "", - "SERVICE_SCHEME": "http", + "SERVICE_SCHEME": "https", "STORAGE_DICT": { "default": "fs:/srv/indico/archive", }, From cfa9eee0ea168b3ba03d9db7044b497aecd5f6b7 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 20 May 2024 15:21:48 +0200 Subject: [PATCH 29/32] fix indico config --- tests/integration/test_charm.py | 2 +- tests/unit/test_core.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 59e63512..5af66046 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -41,7 +41,7 @@ async def test_indico_is_up(ops_test: OpsTest, app: Application): # Send request to bootstrap page and set Host header to app_name (which the application # expects) response = requests.get( - "http://127.0.0.1/bootstrap", + "https://127.0.0.1/bootstrap", headers={"Host": f"{app.name}.local"}, timeout=10, verify=False, # nosec diff --git a/tests/unit/test_core.py b/tests/unit/test_core.py index 7bb68731..344638a5 100644 --- a/tests/unit/test_core.py +++ b/tests/unit/test_core.py @@ -257,7 +257,7 @@ def test_config_changed(self, mock_exec): # pylint: disable=R0915 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("http", updated_plan_env["SERVICE_SCHEME"]) + self.assertEqual("https", updated_plan_env["SERVICE_SCHEME"]) self.assertEqual("", updated_plan_env["SERVICE_PORT"]) self.assertTrue(updated_plan_env["CUSTOMIZATION_DEBUG"]) storage_dict = literal_eval(updated_plan_env["STORAGE_DICT"]) From b63e5f55e039a2ae9286366e431144463fd31a93 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Mon, 6 May 2024 14:02:39 +0200 Subject: [PATCH 30/32] Remove site url --- .github/workflows/integration_test.yaml | 2 +- lib/charms/nginx_ingress_integrator/v0/nginx_route.py | 1 - tests/integration/conftest.py | 5 ++++- tests/integration/test_saml.py | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index 514816b7..d6007bb6 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -1,7 +1,7 @@ name: Integration tests on: - pull_request: + push: jobs: integration-tests: diff --git a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index a2ec38ec..3a1ba5c9 100644 --- a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -242,7 +242,6 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to nginx_route_relation_name: Specifies the relation name of the relation handled by this requirer class. The relation must have the nginx-route interface. - Returns: the NginxRouteRequirer. """ diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 792bc501..442530c0 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -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,7 +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", 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( diff --git a/tests/integration/test_saml.py b/tests/integration/test_saml.py index 46396184..9f6ca37d 100644 --- a/tests/integration/test_saml.py +++ b/tests/integration/test_saml.py @@ -104,4 +104,5 @@ def patched_getaddrinfo(*args): ) assert dashboard_page.status_code == 200 # Revert SAML config for zap to be able to run - await nginx_ingress_integrator_app.reset_config(["service-hostname"]) + # await ops_test.model.remove_relation("indico", "saml_integrator") + # await nginx_ingress_integrator_app.reset_config(["service-hostname"]) From 284a69ed0c4bf0f0a9c1a4ce47e75f76897c2c83 Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 22 May 2024 13:16:52 +0200 Subject: [PATCH 31/32] revert some stuff --- .github/workflows/integration_test.yaml | 4 ++-- lib/charms/nginx_ingress_integrator/v0/nginx_route.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integration_test.yaml b/.github/workflows/integration_test.yaml index d6007bb6..08012655 100644 --- a/.github/workflows/integration_test.yaml +++ b/.github/workflows/integration_test.yaml @@ -1,7 +1,7 @@ name: Integration tests on: - push: + pull_request: jobs: integration-tests: @@ -13,7 +13,7 @@ jobs: modules: '["test_actions.py", "test_charm.py", "test_s3.py", "test_saml.py"]' trivy-fs-enabled: true trivy-image-config: "trivy.yaml" - zap-before-command: "curl -k -H \"Host: indico.local\" http://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" + zap-before-command: "curl -H \"Host: indico.local\" http://localhost/bootstrap --data-raw 'csrf_token=00000000-0000-0000-0000-000000000000&first_name=admin&last_name=admin&email=admin%40admin.com&username=admin&password=lunarlobster&confirm_password=lunarlobster&affiliation=Canonical'" zap-enabled: true zap-cmd-options: '-T 60 -z "-addoninstall jython" --hook "/zap/wrk/tests/zap/hook.py"' zap-target: localhost diff --git a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index 3a1ba5c9..a2ec38ec 100644 --- a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -242,6 +242,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to nginx_route_relation_name: Specifies the relation name of the relation handled by this requirer class. The relation must have the nginx-route interface. + Returns: the NginxRouteRequirer. """ From 58c3eed083f3361ab6648797e5245454a6e54dad Mon Sep 17 00:00:00 2001 From: Arturo Seijas Date: Wed, 22 May 2024 14:51:12 +0200 Subject: [PATCH 32/32] wait properly --- tests/integration/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 442530c0..a0a7710f 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -116,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( @@ -124,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