ISD-1561 Add config option for remoting external url and enable webso…

…cket. (#107)

* Add external url and enable websocket config option

* Refactor config parsing, refactor state.from_charm for complexity, add unit tests

---------

Co-authored-by: arturo-seijas <[email protected]>

config.yaml

@@ -8,13 +8,27 @@ options:
       Preferred UTC time range in 24 hour format for restarting Jenkins. If empty, restart will
       take place whenever Jenkins needs to restart. Jenkins will need to restart on the following
       occasion. Plugins that are not part of `allowed-plugins` configuration option are detected.
-      For example, 03-05 will allow Jenkins restart to take place from 3AM UTC to 5AM UTC. 
+      For example, 03-05 will allow Jenkins restart to take place from 3AM UTC to 5AM UTC.
       Awaits for running job completion for 5 minutes.
     default: ""
   allowed-plugins:
     type: string
     description: >
       Comma-separated list of allowed plugin short names. If empty, any plugin can be installed.
       Plugins installed by the user and their dependencies will be removed automatically if not on
-      the list. Included plugins are not automatically installed. 
+      the list. Included plugins are not automatically installed.
     default: "bazaar,blueocean,dependency-check-jenkins-plugin,docker-build-publish,git,kubernetes,ldap,matrix-combinations-parameter,oic-auth,openid,pipeline-groovy-lib,postbuildscript,rebuild,reverse-proxy-auth-plugin,ssh-agent,thinBackup"
+  remoting-external-url:
+    type: string
+    description: >
+      Configure the charm to use this URL when establishing relations with agent charms.
+      This is useful when connecting agents from outside of the charm's Kubernetes cluster.
+      It is assumed that this url is reachable to the agents. A schema (http:// or https://)
+      is required
+    default: ""
+  remoting-enable-websocket:
+    type: boolean
+    description: >
+      Configure inbound agents to use Websocket and skip TCP port 50000.
+      This is useful when the charm is deployed behind a reverse-proxy or behind a firewall.
+    default: false

src/agent.py

@@ -80,21 +80,29 @@ def _on_deprecated_agent_relation_joined(self, event: ops.RelationJoinedEvent) -
             event.defer()
             return
 
+        enable_websocket = bool(self.state.remoting_config.enable_websocket)
         self.charm.unit.status = ops.MaintenanceStatus("Adding agent node.")
         try:
             jenkins.add_agent_node(
                 agent_meta=agent_meta,
                 container=container,
                 # mypy doesn't understand that host can no longer be None.
                 host=host,
+                enable_websocket=enable_websocket,
             )
             secret = jenkins.get_node_secret(container=container, node_name=agent_meta.name)
         except jenkins.JenkinsError as exc:
             self.charm.unit.status = ops.BlockedStatus(f"Jenkins API exception. {exc=!r}")
             return
 
+        configured_remoting_external_url = self.state.remoting_config.external_url
+        jenkins_url = (
+            f"http://{host}:{jenkins.WEB_PORT}"
+            if not configured_remoting_external_url
+            else str(configured_remoting_external_url)
+        )
         event.relation.data[self.model.unit].update(
-            AgentRelationData(url=f"http://{host}:{jenkins.WEB_PORT}", secret=secret)
+            AgentRelationData(url=jenkins_url, secret=secret)
         )
         self.charm.unit.status = ops.ActiveStatus()
 
@@ -124,17 +132,29 @@ def _on_agent_relation_joined(self, event: ops.RelationJoinedEvent) -> None:
             event.defer()
             return
 
+        enable_websocket = bool(self.state.remoting_config.enable_websocket)
         self.charm.unit.status = ops.MaintenanceStatus("Adding agent node.")
         try:
-            jenkins.add_agent_node(agent_meta=agent_meta, container=container, host=host)
+            jenkins.add_agent_node(
+                agent_meta=agent_meta,
+                container=container,
+                host=host,
+                enable_websocket=enable_websocket,
+            )
             secret = jenkins.get_node_secret(container=container, node_name=agent_meta.name)
         except jenkins.JenkinsError as exc:
             self.charm.unit.status = ops.BlockedStatus(f"Jenkins API exception. {exc=!r}")
             return
 
+        configured_remoting_external_url = self.state.remoting_config.external_url
+        jenkins_url = (
+            f"http://{host}:{jenkins.WEB_PORT}"
+            if not configured_remoting_external_url
+            else str(configured_remoting_external_url)
+        )
         event.relation.data[self.model.unit].update(
             {
-                "url": f"http://{host}:{jenkins.WEB_PORT}",
+                "url": jenkins_url,
                 f"{agent_meta.name}_secret": secret,
             }
         )

src/jenkins.py

@@ -514,13 +514,15 @@ def _get_node_config(
     agent_meta: state.AgentMeta,
     container: ops.Container,
     host: typing.Union[IPv4Address, IPv6Address, str],
+    enable_websocket: bool,
 ) -> dict[str, typing.Any]:
     """Get agent node configuration dictionary values.
 
     Args:
         agent_meta: The Jenkins agent metadata to create the node from.
         container: The Jenkins workload container.
         host: The Jenkins server ip address for direct agent tunnel connection.
+        enable_websocket: Whether to use websocket for inbound agent connections.
 
     Returns:
         A dictionary mapping of agent configuration values.
@@ -540,8 +542,12 @@ def _get_node_config(
     )
     attribs = node.get_node_attributes()
     meta = json.loads(attribs["json"])
-    # the field can either take "HOST:PORT", ":PORT", or "HOST:"
-    meta["launcher"]["tunnel"] = f"{host}:"
+    # Websocket is mutually exclusive with tunnel connect through
+    if enable_websocket:
+        meta["launcher"]["webSocket"] = enable_websocket
+    else:
+        # the field can either take "HOST:PORT", ":PORT", or "HOST:"
+        meta["launcher"]["tunnel"] = f"{host}:"
     attribs["json"] = json.dumps(meta)
     return attribs
 
@@ -550,20 +556,27 @@ def add_agent_node(
     agent_meta: state.AgentMeta,
     container: ops.Container,
     host: typing.Union[IPv4Address, IPv6Address, str],
+    enable_websocket: bool,
 ) -> None:
     """Add a Jenkins agent node.
 
     Args:
         agent_meta: The Jenkins agent metadata to create the node from.
         container: The Jenkins workload container.
         host: The Jenkins server ip address for direct agent tunnel connection.
+        enable_websocket: Whether to use websocket for inbound agent connections.
 
     Raises:
         JenkinsError: if an error occurred running groovy script creating the node.
     """
     client = _get_client(_get_api_credentials(container))
     try:
-        config = _get_node_config(agent_meta=agent_meta, container=container, host=host)
+        config = _get_node_config(
+            agent_meta=agent_meta,
+            container=container,
+            host=host,
+            enable_websocket=enable_websocket,
+        )
         client.create_node_with_config(name=agent_meta.name, config=config)
     except jenkinsapi.custom_exceptions.AlreadyExists:
         pass

src/state.py

@@ -10,7 +10,16 @@
 from pathlib import Path
 
 import ops
-from pydantic import BaseModel, Field, HttpUrl, ValidationError, validator
+from pydantic import (
+    AnyHttpUrl,
+    BaseModel,
+    Field,
+    HttpUrl,
+    StrictBool,
+    ValidationError,
+    tools,
+    validator,
+)
 
 from timerange import InvalidTimeRangeError, Range
 
@@ -218,6 +227,40 @@ def from_env(cls) -> typing.Optional["ProxyConfig"]:
         )
 
 
+class RemotingConfig(BaseModel):
+    """Configuration for inbound agent connections.
+
+    Attributes:
+        external_url: External URL for inbound agent connections.
+        enable_websocket: Use websocket for inbound agent connections.
+    """
+
+    external_url: typing.Optional[AnyHttpUrl]
+    enable_websocket: StrictBool
+
+    @classmethod
+    def from_config(cls, config: ops.ConfigData) -> "RemotingConfig":
+        """Instantiate RemotingConfig from juju charm config data.
+
+        Args:
+            config: the charm's config data
+
+        Returns:
+            RemotingConfig with validated attributes.
+        """
+        config_remoting_external_url = config.get("remoting-external-url")
+        external_url = (
+            tools.parse_obj_as(AnyHttpUrl, config_remoting_external_url)
+            if config_remoting_external_url
+            else None
+        )
+        enable_websocket = bool(config.get("remoting-enable-websocket"))
+        return cls(
+            external_url=external_url,
+            enable_websocket=enable_websocket,
+        )
+
+
 @dataclasses.dataclass(frozen=True)
 class State:
     """The Jenkins k8s operator charm state.
@@ -229,6 +272,8 @@ class State:
             deprecated agent relation.
         proxy_config: Proxy configuration to access Jenkins upstream through.
         plugins: The list of allowed plugins to install.
+        remoting_config: Configuration for inbound agents.
+
     """
 
     restart_time_range: typing.Optional[Range]
@@ -238,6 +283,7 @@ class State:
     ]
     proxy_config: typing.Optional[ProxyConfig]
     plugins: typing.Optional[typing.Iterable[str]]
+    remoting_config: RemotingConfig
 
     @classmethod
     def from_charm(cls, charm: ops.CharmBase) -> "State":
@@ -254,17 +300,19 @@ def from_charm(cls, charm: ops.CharmBase) -> "State":
             CharmRelationDataInvalidError: if invalid relation data was received.
             CharmIllegalNumUnitsError: if more than 1 unit of Jenkins charm is deployed.
         """
-        time_range_str = charm.config.get("restart-time-range")
-        if time_range_str:
-            try:
+        try:
+            remoting_config = RemotingConfig.from_config(config=charm.config)
+            time_range_str = charm.config.get("restart-time-range")
+            if time_range_str:
                 restart_time_range = Range.from_str(time_range_str)
-            except InvalidTimeRangeError as exc:
-                logger.error("Invalid config value for restart-time-range, %s", exc)
-                raise CharmConfigInvalidError(
-                    "Invalid config value for restart-time-range."
-                ) from exc
-        else:
-            restart_time_range = None
+            else:
+                restart_time_range = None
+        except InvalidTimeRangeError as exc:
+            logger.error("Invalid config value for restart-time-range, %s", exc)
+            raise CharmConfigInvalidError("Invalid config value for restart-time range.") from exc
+        except ValidationError as exc:
+            logger.error("Invalid charm configuration, %s", exc)
+            raise CharmConfigInvalidError("Invalid charm configuration.") from exc
 
         try:
             agent_relation_meta_map = _get_agent_meta_map_from_relation(
@@ -299,4 +347,5 @@ def from_charm(cls, charm: ops.CharmBase) -> "State":
             deprecated_agent_relation_meta=deprecated_agent_meta_map,
             plugins=plugins,
             proxy_config=proxy_config,
+            remoting_config=remoting_config,
         )

tests/unit/test_jenkins.py

@@ -602,6 +602,7 @@ def test_add_agent_node_fail(
             state.AgentMeta(executors="3", labels="x86_64", name="agent_node_0"),
             container,
             host=mock_ip_addr,
+            enable_websocket=False,
         )
 
 
@@ -620,6 +621,7 @@ def test_add_agent_node_already_exists(
         state.AgentMeta(executors="3", labels="x86_64", name="agent_node_0"),
         container,
         host=mock_ip_addr,
+        enable_websocket=False,
     )
 
 
@@ -638,6 +640,26 @@ def test_add_agent_node(
         state.AgentMeta(executors="3", labels="x86_64", name="agent_node_0"),
         container,
         host=mock_ip_addr,
+        enable_websocket=False,
+    )
+
+
+@pytest.mark.usefixtures("patch_jenkins_node")
+def test_add_agent_node_websocket(
+    container: ops.Container, mock_client: MagicMock, mock_ip_addr: IPv4Address
+):
+    """
+    arrange: given a mocked jenkins client.
+    act: when add_agent is called.
+    assert: no exception is raised.
+    """
+    mock_client.create_node_with_config.return_value = MagicMock(spec=jenkins.Node)
+
+    jenkins.add_agent_node(
+        state.AgentMeta(executors="3", labels="x86_64", name="agent_node_0"),
+        container,
+        host=mock_ip_addr,
+        enable_websocket=True,
     )
 
 

tests/unit/test_state.py

@@ -169,3 +169,18 @@ def test_invalid_num_units(mock_charm: MagicMock):
 
     with pytest.raises(state.CharmIllegalNumUnitsError):
         state.State.from_charm(mock_charm)
+
+
+def test_remotingconfig_invalid(mock_charm: MagicMock):
+    """
+    arrange: given a mock charm with invalid remoting configuration.
+    act: when charm state is initialized.
+    assert: CharmConfigInvalidError is raised.
+    """
+    mock_charm.config = {
+        "remoting-external-url": "invalid",
+        "remoting-enable-websocket": "invalid",
+    }
+
+    with pytest.raises(state.CharmConfigInvalidError):
+        state.State.from_charm(mock_charm)