Replan when ingress ready (#131)

.github/workflows/integration_test.yaml

@@ -11,7 +11,7 @@ jobs:
       channel: 1.28-strict/stable
       extra-arguments: |
         --kube-config ${GITHUB_WORKSPACE}/kube-config
-      modules: '["test_auth_proxy.py", "test_cos.py", "test_ingress.py", "test_jenkins.py", "test_k8s_agent.py", "test_machine_agent.py", "test_plugins.py", "test_proxy.py", "test_upgrade.py"]'
+      modules: '["test_auth_proxy.py", "test_cos.py", "test_ingress.py", "test_jenkins.py", "test_k8s_agent.py", "test_machine_agent.py", "test_plugins.py", "test_proxy.py", "test_upgrade.py", "test_external_agent.py"]'
       pre-run-script: |
         -c "sudo microk8s config > ${GITHUB_WORKSPACE}/kube-config
         chmod +x tests/integration/pre_run_script.sh

.licenserc.yaml

@@ -35,4 +35,5 @@ header:
     - 'zap_rules.tsv'
     - 'lib/**'
     - tests/integration/files/dex.yaml
+    - tests/integration/files/identity-bundle-edge-patched.yaml
   comment: on-failure

src/agent.py

@@ -104,7 +104,8 @@ def agent_discovery_url(self) -> str:
             unit_ip = str(binding.network.bind_address)
             try:
                 ipaddress.ip_address(unit_ip)
-                return f"http://{unit_ip}:{jenkins.WEB_PORT}"
+                env_dict = typing.cast(typing.Dict[str, str], self.jenkins.environment)
+                return f"http://{unit_ip}:{jenkins.WEB_PORT}{env_dict['JENKINS_PREFIX']}"
             except ValueError as exc:
                 logger.error(
                     "IP from juju-info is not valid: %s, we can still fall back to using fqdn", exc

src/auth_proxy.py

@@ -8,7 +8,11 @@
 
 import ops
 from charms.oathkeeper.v0.auth_proxy import AuthProxyConfig, AuthProxyRequirer
-from charms.traefik_k8s.v2.ingress import IngressPerAppRequirer
+from charms.traefik_k8s.v2.ingress import (
+    IngressPerAppReadyEvent,
+    IngressPerAppRequirer,
+    IngressPerAppRevokedEvent,
+)
 
 import jenkins
 import pebble
@@ -56,6 +60,16 @@ def __init__(
             self._auth_proxy_relation_departed,
         )
 
+        # Event hooks for agent-discovery-ingress
+        charm.framework.observe(
+            self.ingress.on.ready,
+            self._ingress_on_ready,
+        )
+        charm.framework.observe(
+            self.ingress.on.revoked,
+            self._ingress_on_revoked,
+        )
+
     def _on_auth_proxy_relation_joined(self, event: ops.RelationCreatedEvent) -> None:
         """Configure the auth proxy.
 
@@ -68,24 +82,62 @@ def _on_auth_proxy_relation_joined(self, event: ops.RelationCreatedEvent) -> Non
             event.defer()
             return
 
+        self._update_auth_proxy_config()
+        self._replan_jenkins(event)
+
+    # pylint: disable=duplicate-code
+    def _replan_jenkins(self, event: ops.EventBase, disable_security: bool = False) -> None:
+        """Replan the jenkins service to account for prefix changes.
+
+        Args:
+            event: the event fired.
+            disable_security: Whether or not to replan with security disabled.
+        """
+        container = self.charm.unit.get_container(JENKINS_SERVICE_NAME)
+        if not jenkins.is_storage_ready(container):
+            logger.warning("Service not yet ready. Deferring.")
+            event.defer()
+            return
+        pebble.replan_jenkins(container, self.jenkins, self.state, disable_security)
+
+    def _update_auth_proxy_config(self) -> None:
+        """Update auth_proxy configuration with the correct jenkins url."""
         auth_proxy_config = AuthProxyConfig(
             protected_urls=[self.ingress.url],
             allowed_endpoints=AUTH_PROXY_ALLOWED_ENDPOINTS,
             headers=AUTH_PROXY_HEADERS,
         )
         self.auth_proxy.update_auth_proxy_config(auth_proxy_config=auth_proxy_config)
-        pebble.replan_jenkins(container, self.jenkins, self.state)
 
-    # pylint: disable=duplicate-code
     def _auth_proxy_relation_departed(self, event: ops.RelationDepartedEvent) -> None:
         """Unconfigure the auth proxy.
 
         Args:
-            event: the event triggering the handler.
+            event: the event fired.
         """
-        container = self.charm.unit.get_container(JENKINS_SERVICE_NAME)
-        if not jenkins.is_storage_ready(container):
-            logger.warning("Service not yet ready. Deferring.")
-            event.defer()
-            return
-        pebble.replan_jenkins(container, self.jenkins, self.state)
+        # The charm still sees the relation when this hook is fired
+        # We then force pebble to replan with security
+        self._replan_jenkins(event, False)
+
+    def _ingress_on_ready(self, event: IngressPerAppReadyEvent) -> None:
+        """Handle ready event.
+
+        Args:
+            event: The event fired.
+        """
+        if self.state.auth_proxy_integrated:
+            self._update_auth_proxy_config()
+        self._replan_jenkins(event, self.state.auth_proxy_integrated)
+
+    def _ingress_on_revoked(self, event: IngressPerAppRevokedEvent) -> None:
+        """Handle revoked event.
+
+        Args:
+            event: The event fired.
+        """
+        # call to update_prefix is needed here since the charm is not aware
+        # That the prefix has changed during charm-init
+        self.jenkins.update_prefix("")
+        if self.state.auth_proxy_integrated:
+            self._update_auth_proxy_config()
+        self._replan_jenkins(event, self.state.auth_proxy_integrated)

src/jenkins.py

@@ -3,6 +3,8 @@
 
 """Functions to operate Jenkins."""
 
+# pylint: disable=too-many-lines
+
 import dataclasses
 import functools
 import itertools
@@ -18,6 +20,7 @@
 
 import jenkinsapi.custom_exceptions
 import jenkinsapi.jenkins
+import jenkinsapi.utils.requester
 import ops
 import requests
 from jenkinsapi.node import Node
@@ -198,6 +201,14 @@ def version(self) -> str:
             logger.error("Failed to get Jenkins version, %s", exc)
             raise JenkinsError("Failed to get Jenkins version.") from exc
 
+    def update_prefix(self, prefix: str) -> None:
+        """Update jenkins prefix.
+
+        Args:
+            prefix: the new prefix.
+        """
+        self.environment.update({"JENKINS_PREFIX": prefix})
+
     def _is_ready(self) -> bool:
         """Check if Jenkins webserver is ready.
 
@@ -285,6 +296,35 @@ def _setup_user_token(self, container: ops.Container) -> None:
             container.push(API_TOKEN_PATH, token, user=USER, group=GROUP)
         except ops.pebble.PathError as exc:
             raise JenkinsBootstrapError("Failed to setup user token.") from exc
+        except jenkinsapi.utils.requester.JenkinsAPIException as e:
+            # Jenkins api exception when generating user token
+            # We check if security is disabled
+            logger.info("Generate token failed, checking if security is disabled")
+            response = client.requester.get_url(
+                f"{client.base_server_url()}/manage/api/json?tree=useSecurity"
+            )
+            try:
+                if response.status_code == 200 and not response.json()["useSecurity"]:
+                    # !! Write a random string to the api token as a temporary workaround,
+                    # Prefix it to signify that it's a placeholder token
+                    # Follow-up changes will be needed to rework this.
+                    container.push(
+                        API_TOKEN_PATH,
+                        f"placeholder-{secrets.token_hex(16)}",
+                        user=USER,
+                        group=GROUP,
+                    )
+                    return
+            # Not in the case where security is disabled, reraise the exception
+            except (requests.exceptions.JSONDecodeError, KeyError):
+                logger.error(
+                    "Failed parsing jenkins's security config in response, will raise initial error"
+                )
+            logger.error(
+                "Generate token failed but security is not disabled, API response: HTTP %s",
+                response.status_code,
+            )
+            raise JenkinsBootstrapError("Failed to setup user token") from e
 
     def _configure_proxy(
         self, container: ops.Container, proxy_config: state.ProxyConfig | None = None

src/pebble.py

@@ -18,14 +18,18 @@
 
 
 def replan_jenkins(
-    container: ops.Container, jenkins_instance: jenkins.Jenkins, state: State
+    container: ops.Container,
+    jenkins_instance: jenkins.Jenkins,
+    state: State,
+    disable_security: bool = False,
 ) -> None:
     """Replan the jenkins services.
 
     Args:
         container: the container for with to replan the services.
         jenkins_instance: the Jenkins instance.
         state: the charm state.
+        disable_security: whether to replan with security disabled.
 
     Raises:
         JenkinsBootstrapError: if an error occurs while bootstrapping Jenkins.
@@ -35,7 +39,7 @@ def replan_jenkins(
     try:
         jenkins_instance.wait_ready()
         # Tested in integration
-        if state.auth_proxy_integrated:  # pragma: no cover
+        if disable_security:  # pragma: no cover
             jenkins_instance.bootstrap(
                 container, jenkins.AUTH_PROXY_JENKINS_CONFIG, state.proxy_config
             )

tests/integration/conftest.py

@@ -7,7 +7,7 @@
 import random
 import secrets
 import string
-from typing import Any, AsyncGenerator, Callable, Coroutine, Generator, Iterable, Optional
+from typing import AsyncGenerator, Generator, Iterable, Optional
 
 import jenkinsapi.jenkins
 import kubernetes.config
@@ -22,9 +22,6 @@
 from keycloak import KeycloakAdmin, KeycloakOpenIDConnection
 from lightkube import Client, KubeConfig
 from lightkube.core.exceptions import ApiError
-from playwright.async_api import async_playwright
-from playwright.async_api._generated import Browser, BrowserContext, BrowserType, Page
-from playwright.async_api._generated import Playwright as AsyncPlaywright
 from pytest import FixtureRequest
 from pytest_operator.plugin import OpsTest
 
@@ -42,6 +39,13 @@
 from .types_ import KeycloakOIDCMetadata, LDAPSettings, ModelAppUnit, UnitWebClient
 
 KUBECONFIG = os.environ.get("TESTING_KUBECONFIG", "~/.kube/config")
+IDENTITY_PLATFORM_APPS = [
+    "traefik-admin",
+    "traefik-public",
+    "hydra",
+    "kratos",
+    "kratos-external-idp-integrator",
+]
 
 
 @pytest.fixture(scope="module", name="model")
@@ -781,12 +785,15 @@ async def traefik_application_fixture(model: Model):
 
 @pytest_asyncio.fixture(scope="module", name="oathkeeper_related")
 async def oathkeeper_application_related_fixture(
-    application: Application, client: Client, ext_idp_service: str
+    ops_test: OpsTest, application: Application, client: Client, ext_idp_service: str
 ):
     """The application related to Jenkins via auth_proxy v0 relation."""
     oathkeeper = await application.model.deploy("oathkeeper", channel="edge", trust=True)
-    identity_platform = await application.model.deploy(
-        "identity-platform", channel="edge", trust=True
+    # Using a patched local bundle from identity team here as a temporary workaround for
+    # https://github.com/canonical/traefik-k8s-operator/issues/322 and
+    # https://github.com/juju/python-libjuju/issues/1042
+    await ops_test.run(
+        "juju", "deploy", "./tests/integration/files/identity-bundle-edge-patched.yaml", "--trust"
     )
     await application.model.applications["kratos-external-idp-integrator"].set_config(
         {
@@ -802,7 +809,7 @@ async def oathkeeper_application_related_fixture(
     # See https://github.com/canonical/kratos-operator/issues/182
     await application.model.wait_for_idle(
         status="active",
-        apps=[application.name, oathkeeper.name] + [app.name for app in identity_platform],
+        apps=[application.name, oathkeeper.name] + IDENTITY_PLATFORM_APPS,
         raise_on_error=False,
         timeout=30 * 60,
         idle_period=5,
@@ -828,7 +835,7 @@ async def oathkeeper_application_related_fixture(
 
     await application.model.wait_for_idle(
         status="active",
-        apps=[application.name, oathkeeper.name] + [app.name for app in identity_platform],
+        apps=[application.name, oathkeeper.name] + IDENTITY_PLATFORM_APPS,
         raise_on_error=False,
         timeout=30 * 60,
         idle_period=5,
@@ -884,96 +891,3 @@ def external_user_email() -> str:
 def external_user_password() -> str:
     """Password for testing proxy authentication."""
     return "password"
-
-
-# The playwright fixtures are taken from:
-# https://github.com/microsoft/playwright-python/blob/main/tests/async/conftest.py
-@pytest_asyncio.fixture(scope="module", name="playwright")
-async def playwright_fixture() -> AsyncGenerator[AsyncPlaywright, None]:
-    """Playwright object."""
-    async with async_playwright() as playwright_object:
-        yield playwright_object
-
-
-@pytest_asyncio.fixture(scope="module", name="browser_type")
-async def browser_type_fixture(playwright: AsyncPlaywright) -> AsyncGenerator[BrowserType, None]:
-    """Browser type for playwright."""
-    yield playwright.firefox
-
-
-@pytest_asyncio.fixture(scope="module", name="browser_factory")
-async def browser_factory_fixture(
-    browser_type: BrowserType,
-) -> AsyncGenerator[Callable[..., Coroutine[Any, Any, Browser]], None]:
-    """Browser factory."""
-    browsers = []
-
-    async def launch(**kwargs: Any) -> Browser:
-        """Launch browser.
-
-        Args:
-            kwargs: kwargs.
-
-        Returns:
-            a browser instance.
-        """
-        browser = await browser_type.launch(**kwargs)
-        browsers.append(browser)
-        return browser
-
-    yield launch
-    for browser in browsers:
-        await browser.close()
-
-
-@pytest_asyncio.fixture(scope="module", name="browser")
-async def browser_fixture(
-    browser_factory: Callable[..., Coroutine[Any, Any, Browser]]
-) -> AsyncGenerator[Browser, None]:
-    """Browser."""
-    browser = await browser_factory()
-    yield browser
-    await browser.close()
-
-
-@pytest_asyncio.fixture(name="context_factory")
-async def context_factory_fixture(
-    browser: Browser,
-) -> AsyncGenerator[Callable[..., Coroutine[Any, Any, BrowserContext]], None]:
-    """Playwright context factory."""
-    contexts = []
-
-    async def launch(**kwargs: Any) -> BrowserContext:
-        """Launch browser.
-
-        Args:
-            kwargs: kwargs.
-
-        Returns:
-            the browser context.
-        """
-        context = await browser.new_context(**kwargs)
-        contexts.append(context)
-        return context
-
-    yield launch
-    for context in contexts:
-        await context.close()
-
-
-@pytest_asyncio.fixture(name="context")
-async def context_fixture(
-    context_factory: Callable[..., Coroutine[Any, Any, BrowserContext]]
-) -> AsyncGenerator[BrowserContext, None]:
-    """Playwright context."""
-    context = await context_factory(ignore_https_errors=True)
-    yield context
-    await context.close()
-
-
-@pytest_asyncio.fixture
-async def page(context: BrowserContext) -> AsyncGenerator[Page, None]:
-    """Playwright page."""
-    new_page = await context.new_page()
-    yield new_page
-    await new_page.close()