diff --git a/.github/agents/charm-engineer.agent.md b/.github/agents/charm-engineer.agent.md new file mode 100644 index 00000000..574067b9 --- /dev/null +++ b/.github/agents/charm-engineer.agent.md @@ -0,0 +1,99 @@ +--- +name: charm-engineer +description: Senior software engineering specialized in writing Juju charms +--- + +You are a senior software engineer with a strong background in python and in site reliability engineering specialized in writing Juju charms. + +You bring your expertise to create new charm or review existing ones. + +## Actions + +- You MUST start by reading the "Charm implementation guidelines". +- You MUST download and analyze all links in this document. + +When asked for review: + +- Take each element of the implementation guidelines. +- Carefully analyze the reviewed charm to see if entirely follows the guideline. +- Report: + + - Guidelines that are fully implemented. + - Guidelines that are partially or not implemented. + - Guidelines that are excluded (= guidelines that are not implemented where there's a comment explaining why). + +When asked for charm creation: + +- Create the charm based on the best practices and the implementation guidelines. +- Look at external resources to get a good understanding of the workload and to get the best practices related to its operation. + +## Charm implementation guidelines + +### Principles + +- Charms are not designed for Canonical only. They should not contain Canonical internal references. +- Charms should be trustworthy. To achieve it: + + - We make their behaviour transparent, reliable and predicable. + + - All charms must use the [holistic](https://documentation.ubuntu.com/ops/latest/explanation/holistic-vs-delta-charms/) (you MUST read this doc) pattern. + - Charm must not use `defered` events. + - The [Managing charm complexity](https://discourse.charmhub.io/t/specification-isd014-managing-charm-complexity/11619) (you MUST read this doc) "Charm Runtime State Abstraction" principle is applied (ignore the other ones) (Remind Thanh that should put it in RTD every time you do a review). + +### Substrate + +By default, we develop K8s charms. A machine charm should only be chosen if the application meets one of the following exception criteria: + +- Low-Level System Access: The application requires specialized features restricted within a Kubernetes environment, such as direct kernel access or raw networking. +- Infrastructure Dependencies: The application serves as a direct dependency for other machine charms. +- Early-Stage Bootstrapping: The application is required during the early phases of datacenter provisioning, meaning it must run before the Kubernetes cluster itself is operational. +- Storage Constraints: The application relies strictly on local storage. + +### Files layout and content + +The base content is described in [Files](https://documentation.ubuntu.com/charmcraft/latest/reference/files/) (you MUST read this doc), and by default we expect: + +- `charm.py` contains the charm code. +- `state.py` contains the runtime state of the charm. For complex charms, we would have a "state/" python module. +- `workload.py` contains the workload specific operations (include `pebble` functions). + +#### `charm.py` + +- All methods are private and should start with `_`, including `_reconcile`. +- Required ports must explicitely opened with `open_port` or `set_ports`. It's usually an anomaly if no ports are open. + +##### `_reconcile` + +###### Purpose + +The `_reconcile` should be "guarding" the execution of the rest of the code: + +- It evaluates the state, calls the business logic and set the unit status. +- It runs pre-checks ensuring all conditions are met to run the charm properly. +- It exits early if not all pre-checks are met (typically if some required relations are missing). +- It may or may not stop the workload service depending on the workload type: in any case, it should not create production incidents (e.g. "not stopping a load-balancer if one relation is missing"). +- Everything is part of `_reconcile` but `refresh` events. +- `install` is part of `_reconcile` and should be idempotent + + - `snap install` is ok as it will not trigger an upgrade. + - `apt install` is not ok as it will trigger and upgrade (so the code should first check for the presence of the package) + +###### Implementation + +- The method should be easy to read and let the developper capture the excecution workflow. +- It should delegate as much as it can. +- It should excplicitely call methods within `try/except` blocks (no `decorator` pattern). +- `try/except` blocks should be small and only catch custom exceptions. +- For "multi-modes" charm, the "routing" mode should be identified early, and call specific `_reconcile_` methods. + +##### Relations + +- Relations should use the `save` and `load` methods to dump and restore data from the relation through Pydantic models. + +#### `rockcraft.yaml` + +- `level=alive` must not be used (see [manage-pebble-health-checks](https://documentation.ubuntu.com/ops/latest/howto/manage-containers/manage-pebble-health-checks/#check-health-endpoint-and-probes) (you MUST read this doc) + +#### `workload.py` + +- Only restart workload when needed. diff --git a/haproxy-operator/lib/charms/haproxy/v1/haproxy_route_tcp.py b/haproxy-operator/lib/charms/haproxy/v1/haproxy_route_tcp.py index 4145d797..0285e43a 100644 --- a/haproxy-operator/lib/charms/haproxy/v1/haproxy_route_tcp.py +++ b/haproxy-operator/lib/charms/haproxy/v1/haproxy_route_tcp.py @@ -186,7 +186,7 @@ def _on_haproxy_route_data_available(self, event: EventBase) -> None: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 3 +LIBPATCH = 4 logger = logging.getLogger(__name__) HAPROXY_ROUTE_TCP_RELATION_NAME = "haproxy-route-tcp" @@ -588,9 +588,12 @@ class TcpRequirerApplicationData(_DatabagModel): """Configuration model for HAProxy route requirer application data. Attributes: - port: The port exposed on the provider. + port: The port exposed on the provider. Mutually exclusive with port_range. + port_range: A range of ports exposed on the provider (e.g. "10500-10600"). + Each port in the range is forwarded 1:1 to the same port on the backend. + Mutually exclusive with port. backend_port: The port where the backend service is listening. Defaults to the - provider port. + provider port. Not applicable when port_range is set. hosts: List of backend server addresses. Currently only support IP addresses. sni: Server name identification. Used to route traffic to the service. check: TCP health check configuration @@ -606,7 +609,20 @@ class TcpRequirerApplicationData(_DatabagModel): proxy_protocol: Whether to enable PROXY protocol when connecting to backend servers. """ - port: int = Field(description="The port exposed on the provider.", gt=0, le=65535) + port: Optional[int] = Field( + description="The port exposed on the provider. Mutually exclusive with port_range.", + default=None, + gt=0, + le=65535, + ) + port_range: Optional[str] = Field( + description=( + "A range of ports exposed on the provider (e.g. '10500-10600'). " + "Each port in the range is forwarded 1:1 to the same port on the backend. " + "Mutually exclusive with port." + ), + default=None, + ) backend_port: Optional[int] = Field( description=( "The port where the backend service is listening. Defaults to the provider port." @@ -659,16 +675,59 @@ class TcpRequirerApplicationData(_DatabagModel): default=False, ) + @model_validator(mode="after") + def validate_port_or_port_range(self) -> "Self": + """Validate that exactly one of port or port_range is set. + + Raises: + ValueError: If neither or both port and port_range are provided. + ValueError: If port_range format is invalid or start >= end. + + Returns: + The validated model. + """ + if self.port is None and self.port_range is None: + raise ValueError("Exactly one of 'port' or 'port_range' must be provided.") + if self.port is not None and self.port_range is not None: + raise ValueError("'port' and 'port_range' are mutually exclusive.") + if self.port_range is not None: + parts = self.port_range.split("-") + if len(parts) != 2: + raise ValueError( + f"Invalid port_range format '{self.port_range}': expected 'START-END'." + ) + try: + start, end = int(parts[0]), int(parts[1]) + except ValueError as exc: + raise ValueError( + f"Invalid port_range '{self.port_range}': ports must be integers." + ) from exc + if not (0 < start <= 65535 and 0 < end <= 65535): + raise ValueError( + f"Invalid port_range '{self.port_range}': ports must be between 1 and 65535." + ) + if start >= end: + raise ValueError( + f"Invalid port_range '{self.port_range}': start port must be less than end port." + ) + return self + @model_validator(mode="after") def assign_default_backend_port(self) -> "Self": """Assign a default value to backend_port if not set. - The value is equal to the provider port. + For single-port mode, backend_port defaults to the provider port. + For port_range mode, backend_port is not applicable. + + Raises: + ValueError: If backend_port is explicitly set with port_range. Returns: The model with backend_port default value applied. """ - if self.backend_port is None: + if self.port_range is not None and self.backend_port is not None: + raise ValueError("'backend_port' cannot be set when using 'port_range'.") + if self.backend_port is None and self.port is not None: self.backend_port = self.port return self @@ -678,12 +737,35 @@ def sni_set_when_not_enforcing_tls(self) -> "Self": Raises: ValueError: If sni is configured and TLS is disabled. + ValueError: If sni is configured with port_range. Returns: The validated model. """ if not self.enforce_tls and self.sni is not None: raise ValueError("You can't set SNI and disable TLS at the same time.") + if self.port_range is not None and self.sni is not None: + raise ValueError("'sni' cannot be set when using 'port_range'.") + return self + + @model_validator(mode="after") + def tls_terminate_requires_no_port_range(self) -> "Self": + """Check that tls_terminate is not enabled when port_range is set. + + Port-range mode uses transparent TCP passthrough; HAProxy cannot terminate + TLS without a known SNI, and SNI is disallowed with port_range. + + Raises: + ValueError: If tls_terminate is True when port_range is set. + + Returns: + The validated model. + """ + if self.port_range is not None and self.tls_terminate: + raise ValueError( + "'tls_terminate' must be False when using 'port_range'. " + "Port-range mode uses transparent TCP passthrough." + ) return self @@ -738,20 +820,21 @@ class HaproxyRouteTcpRequirersData: @model_validator(mode="after") def check_ports_unique(self) -> Self: - """Check that requirers define unique ports. + """Check that requirers define unique ports with no overlapping port ranges. Returns: The validated model, with invalid relation IDs updated in `self.relation_ids_with_invalid_data` """ - # Maybe the logic here can be optimized, we want to keep track of - # the relation IDs that request overlapping ports to ignore them during - # rendering of the haproxy configuration. relation_ids_per_port: dict[int, list[int]] = defaultdict(list[int]) for requirer_data in self.requirers_data: - relation_ids_per_port[requirer_data.application_data.port].append( - requirer_data.relation_id - ) + app_data = requirer_data.application_data + if app_data.port_range is not None: + start, end = (int(p) for p in app_data.port_range.split("-")) + for port in range(start, end + 1): + relation_ids_per_port[port].append(requirer_data.relation_id) + elif app_data.port is not None: + relation_ids_per_port[app_data.port].append(requirer_data.relation_id) for relation_ids in relation_ids_per_port.values(): if len(relation_ids) > 1: @@ -980,6 +1063,7 @@ def __init__( relation_name: str, *, port: Optional[int] = None, + port_range: Optional[str] = None, backend_port: Optional[int] = None, hosts: Optional[list[IPvAnyAddress]] = None, sni: Optional[str] = None, @@ -1013,7 +1097,10 @@ def __init__( Args: charm: The charm that is instantiating the library. relation_name: The name of the relation to bind to. - port: The provider port. + port: The provider port. Mutually exclusive with port_range. + port_range: A range of ports exposed on the provider (e.g. "10500-10600"). + Each port in the range is forwarded 1:1 to the same port on the backend. + Mutually exclusive with port. backend_port: List of ports the service is listening on. hosts: List of backend server addresses. Currently only support IP addresses. sni: List of URL paths to route to this service. @@ -1021,7 +1108,7 @@ def __init__( check_rise: Number of successful health checks before server is considered up. check_fall: Number of failed health checks before server is considered down. check_type: Health check type, - Can be “generic”, “mysql”, “postgres”, “redis” or “smtp”. + Can be "generic", "mysql", "postgres", "redis" or "smtp". check_send: Only used in generic health checks, specify a string to send in the health check request. check_expect: Only used in generic health checks, @@ -1056,6 +1143,7 @@ def __init__( # build the full application data self._application_data = self._generate_application_data( port=port, + port_range=port_range, backend_port=backend_port, hosts=hosts, sni=sni, @@ -1108,7 +1196,8 @@ def _on_relation_broken(self, _: RelationBrokenEvent) -> None: def provide_haproxy_route_tcp_requirements( self, *, - port: int, + port: Optional[int] = None, + port_range: Optional[str] = None, backend_port: Optional[int] = None, hosts: Optional[list[IPvAnyAddress]] = None, sni: Optional[str] = None, @@ -1140,7 +1229,10 @@ def provide_haproxy_route_tcp_requirements( """Update haproxy-route requirements data in the relation. Args: - port: The provider port. + port: The provider port. Mutually exclusive with port_range. + port_range: A range of ports exposed on the provider (e.g. "10500-10600"). + Each port in the range is forwarded 1:1 to the same port on the backend. + Mutually exclusive with port. backend_port: List of ports the service is listening on. hosts: List of backend server addresses. Currently only support IP addresses. sni: List of URL paths to route to this service. @@ -1148,7 +1240,7 @@ def provide_haproxy_route_tcp_requirements( check_rise: Number of successful health checks before server is considered up. check_fall: Number of failed health checks before server is considered down. check_type: Health check type, - Can be “generic”, “mysql”, “postgres”, “redis” or “smtp”. + Can be "generic", "mysql", "postgres", "redis" or "smtp". check_send: Only used in generic health checks, specify a string to send in the health check request. check_expect: Only used in generic health checks, @@ -1176,6 +1268,7 @@ def provide_haproxy_route_tcp_requirements( self._unit_address = unit_address self._application_data = self._generate_application_data( port=port, + port_range=port_range, backend_port=backend_port, hosts=hosts, sni=sni, @@ -1210,6 +1303,7 @@ def _generate_application_data( self, *, port: Optional[int] = None, + port_range: Optional[str] = None, backend_port: Optional[int] = None, hosts: Optional[list[IPvAnyAddress]] = None, sni: Optional[str] = None, @@ -1240,7 +1334,9 @@ def _generate_application_data( """Generate the complete application data structure. Args: - port: The provider port. + port: The provider port. Mutually exclusive with port_range. + port_range: A range of ports exposed on the provider (e.g. "10500-10600"). + Mutually exclusive with port. backend_port: List of ports the service is listening on. hosts: List of backend server addresses. Currently only support IP addresses. sni: List of URL paths to route to this service. @@ -1248,7 +1344,7 @@ def _generate_application_data( check_rise: Number of successful health checks before server is considered up. check_fall: Number of failed health checks before server is considered down. check_type: Health check type, - Can be “generic”, “mysql”, “postgres”, “redis” or “smtp”. + Can be "generic", "mysql", "postgres", "redis" or "smtp". check_send: Only used in generic health checks, specify a string to send in the health check request. check_expect: Only used in generic health checks, @@ -1283,6 +1379,7 @@ def _generate_application_data( application_data: dict[str, Any] = { "port": port, + "port_range": port_range, "backend_port": backend_port, "hosts": hosts, "sni": sni, @@ -1449,8 +1546,8 @@ def _generate_load_balancing_configuration( def update_relation_data(self) -> None: """Update both application and unit data in the relation.""" - if not self._application_data.get("port"): - logger.warning("port must be set, skipping update.") + if not self._application_data.get("port") and not self._application_data.get("port_range"): + logger.warning("port or port_range must be set, skipping update.") return if relation := self.relation: diff --git a/haproxy-operator/src/charm.py b/haproxy-operator/src/charm.py index f9e829af..088b3818 100755 --- a/haproxy-operator/src/charm.py +++ b/haproxy-operator/src/charm.py @@ -418,8 +418,9 @@ def _configure_haproxy_route( 80, 443, *( - frontend.port + port for frontend in haproxy_route_requirers_information.valid_tcp_frontends() + for port in frontend.all_frontend_ports ), *( backend.application_data.external_grpc_port @@ -707,6 +708,7 @@ def _publish_haproxy_route_tcp_proxied_endpoints( ) -> None: """Publish the proxied endpoints for TCP frontends.""" for frontend in haproxy_route_requirers_information.valid_tcp_frontends(): + port_str = frontend.port_range if frontend.port_range else str(frontend.port) for backend in frontend.backends: relation = self.model.get_relation(HAPROXY_ROUTE_TCP_RELATION, backend.relation_id) if not relation: @@ -716,17 +718,17 @@ def _publish_haproxy_route_tcp_proxied_endpoints( continue if frontend.is_sni_routing_enabled: self.haproxy_route_tcp_provider.publish_proxied_endpoints( - [f"{backend.application_data.sni}:{frontend.port}"], relation + [f"{backend.application_data.sni}:{port_str}"], relation ) continue if ha_information.ha_integration_ready: self.haproxy_route_tcp_provider.publish_proxied_endpoints( - [f"{ha_information.vip}:{frontend.port}"], relation + [f"{ha_information.vip}:{port_str}"], relation ) continue self.haproxy_route_tcp_provider.publish_proxied_endpoints( [ - f"{unit_address}:{frontend.port}" + f"{unit_address}:{port_str}" for unit_address in self._get_peer_units_address() ], relation, diff --git a/haproxy-operator/src/state/haproxy_route.py b/haproxy-operator/src/state/haproxy_route.py index d7c6e314..c4bf2ef6 100644 --- a/haproxy-operator/src/state/haproxy_route.py +++ b/haproxy-operator/src/state/haproxy_route.py @@ -631,7 +631,10 @@ def check_tcp_http_port_conflicts(self) -> Self: for backend in valid_backends if backend.application_data.external_grpc_port } - tcp_ports = {frontend.port: frontend for frontend in self.tcp_frontends} + tcp_ports: dict[int, HAProxyRouteTcpFrontend] = {} + for frontend in self.tcp_frontends: + for p in frontend.all_frontend_ports: + tcp_ports[p] = frontend # Check for conflicts between standard HTTP and TCP/gRPC ports if has_http_backends: @@ -686,7 +689,7 @@ def valid_tcp_frontends(self) -> list[HAProxyRouteTcpFrontend]: return [ frontend for frontend in self.tcp_frontends - if frontend.port not in self.ports_with_conflicts + if not any(p in self.ports_with_conflicts for p in frontend.all_frontend_ports) ] @property @@ -790,10 +793,15 @@ def parse_haproxy_route_tcp_requirers_data( Returns: list[HAProxyRouteTcpFrontend]: The parsed frontend data. """ - port_to_backends_mapping: dict[int, list[HAProxyRouteTcpBackend]] = defaultdict(list) + port_to_backends_mapping: dict[int | str, list[HAProxyRouteTcpBackend]] = defaultdict(list) for requirer in tcp_requirers.requirers_data: endpoint = HAProxyRouteTcpBackend.from_haproxy_route_tcp_requirer_data(requirer) - port_to_backends_mapping[endpoint.application_data.port].append(endpoint) + key: int | str + if endpoint.application_data.port_range is not None: + key = endpoint.application_data.port_range + else: + key = cast(int, endpoint.application_data.port) + port_to_backends_mapping[key].append(endpoint) tcp_frontends: list[HAProxyRouteTcpFrontend] = [] for backends in port_to_backends_mapping.values(): try: diff --git a/haproxy-operator/src/state/haproxy_route_tcp.py b/haproxy-operator/src/state/haproxy_route_tcp.py index 797f076a..859fe392 100644 --- a/haproxy-operator/src/state/haproxy_route_tcp.py +++ b/haproxy-operator/src/state/haproxy_route_tcp.py @@ -41,6 +41,8 @@ class HaproxyRouteTcpServer: server_name: The name of the unit with invalid characters replaced. address: The IP address of the requirer unit. port: The port that the requirer application wishes to be exposed. + When None, the server uses HAProxy's dynamic port pass-through + (set-dst-port fc_dst_port must be configured in the frontend). check: Health check configuration. maxconn: Maximum allowed connections before requests are queued. send_proxy: Whether to enable PROXY protocol for this server. @@ -48,7 +50,7 @@ class HaproxyRouteTcpServer: server_name: str address: IPvAnyAddress - port: int + port: Optional[int] check: Optional[TCPServerHealthCheck] maxconn: Optional[int] send_proxy: bool = False @@ -103,6 +105,9 @@ def servers(self) -> list[HaproxyRouteTcpServer]: sequential server names and using the application's backend port and health check configuration. + For port_range backends, the server port is set to None so HAProxy's + dynamic port pass-through (set-dst-port fc_dst_port) takes effect. + Returns: list[HaproxyRouteTcpServer]: List of configured backend servers. """ @@ -111,13 +116,14 @@ def servers(self) -> list[HaproxyRouteTcpServer]: if not backend_addresses: backend_addresses = [unit_data.address for unit_data in self.units_data] + is_port_range = self.application_data.port_range is not None for i, address in enumerate(backend_addresses): servers.append( HaproxyRouteTcpServer( server_name=f"{self.application}-{i}", - port=cast(int, self.application_data.backend_port), + port=None if is_port_range else cast(int, self.application_data.backend_port), address=address, - check=self.application_data.check, + check=None if is_port_range else self.application_data.check, maxconn=self.application_data.server_maxconn, send_proxy=self.application_data.proxy_protocol, ) @@ -128,12 +134,16 @@ def servers(self) -> list[HaproxyRouteTcpServer]: def name(self) -> str: """Get the unique name for this TCP endpoint. - Combines the application name and frontend port to create a unique + Combines the application name and frontend port (or port range) to create a unique identifier for the HAProxy configuration section. Returns: - str: The endpoint name in format "{application}_{port}". + str: The endpoint name in format "{application}_{port}" or + "{application}_{start}_{end}" for port ranges. """ + if self.application_data.port_range is not None: + range_slug = self.application_data.port_range.replace("-", "_") + return f"{self.application}_{range_slug}" return f"{self.application}_{self.application_data.port}" @property @@ -194,14 +204,22 @@ class HAProxyRouteTcpFrontend: """A representation of a TCP frontend in the haproxy config. Attrs: - port: The port exposed on the provider. + port: The port exposed on the provider. Mutually exclusive with port_range. + port_range: A range of ports exposed on the provider (e.g. "10500-10600"). + Mutually exclusive with port. backends: List of backend endpoints for this frontend. enforce_tls: Whether to enforce TLS for all traffic. tls_terminate: Whether to enable TLS termination. """ - port: int = Field(description="The port exposed on the provider.", gt=0, le=65535) backends: list[HAProxyRouteTcpBackend] = Field(description="List of backend endpoints.") + port: Optional[int] = Field( + description="The port exposed on the provider.", default=None, gt=0, le=65535 + ) + port_range: Optional[str] = Field( + description="A range of ports exposed on the provider (e.g. '10500-10600').", + default=None, + ) enforce_tls: bool = Field(description="Whether to enforce TLS for all traffic.", default=True) tls_terminate: bool = Field(description="Whether to enable tls termination.", default=True) relation_ids_with_invalid_data: set[int] = Field( @@ -225,6 +243,7 @@ def from_backends(cls, backends: list[HAProxyRouteTcpBackend]) -> "Self": if len(backends) == 1: return cls( port=backends[0].application_data.port, + port_range=backends[0].application_data.port_range, backends=backends, enforce_tls=backends[0].application_data.enforce_tls, tls_terminate=backends[0].application_data.tls_terminate, @@ -263,12 +282,28 @@ def from_backends(cls, backends: list[HAProxyRouteTcpBackend]) -> "Self": ) return cls( port=rendered_backends[0].application_data.port, + port_range=rendered_backends[0].application_data.port_range, backends=rendered_backends, enforce_tls=rendered_backends[0].application_data.enforce_tls, tls_terminate=rendered_backends[0].application_data.tls_terminate, relation_ids_with_invalid_data=relation_ids_with_invalid_data, ) + @property + def all_frontend_ports(self) -> list[int]: + """Get all ports in this frontend. + + Returns: + list[int]: A list of all ports. For single-port frontends, a one-element list. + For port-range frontends, all ports in the range (inclusive). + """ + if self.port is not None: + return [self.port] + if self.port_range is not None: + start, end = (int(p) for p in self.port_range.split("-")) + return list(range(start, end + 1)) + return [] + @property def backend_sni_routing_configurations(self) -> list[BackendRoutingConfiguration]: """Get the routing configuration for each backend. @@ -315,8 +350,34 @@ def default_backend_name(self) -> str: Returns: str: The name of the default backend. """ + if self.port_range is not None: + range_slug = self.port_range.replace("-", "_") + return f"haproxy_route_tcp_{range_slug}_default_backend" return f"haproxy_route_tcp_{self.port}_default_backend" + @property + def frontend_name(self) -> str: + """Get the frontend name for this frontend. + + Returns: + str: The name of the frontend. + """ + if self.port_range is not None: + range_slug = self.port_range.replace("-", "_") + return f"haproxy_route_tcp_{range_slug}" + return f"haproxy_route_tcp_{self.port}" + + @property + def bind_address(self) -> str: + """Get the bind address string for this frontend. + + Returns: + str: The bind address including port or port range. + """ + if self.port_range is not None: + return self.port_range + return str(self.port) + @property def content_inspect_delay_required(self) -> bool: """Indicate if content inspect delay is required. diff --git a/haproxy-operator/templates/haproxy_route_tcp.cfg.j2 b/haproxy-operator/templates/haproxy_route_tcp.cfg.j2 index 565c42f2..d5f38a19 100644 --- a/haproxy-operator/templates/haproxy_route_tcp.cfg.j2 +++ b/haproxy-operator/templates/haproxy_route_tcp.cfg.j2 @@ -1,7 +1,11 @@ {% macro render_tcp_server(server) -%} server {{ server.server_name }} +{% if server.port is not none %} {{ server.address }}:{{ server.port }} +{% else %} +{{ server.address }}:0 +{% endif %} {% if server.check %} check inter {{ server.check.interval }}s @@ -17,10 +21,13 @@ send-proxy {%- endmacro %} {% for frontend in tcp_frontends %} -frontend haproxy_route_tcp_{{ frontend.port }} +frontend {{ frontend.frontend_name }} mode tcp option tcplog - bind [::]:{{ frontend.port }} v4v6 {% if frontend.tls_terminate %} ssl crt {{ haproxy_crt_dir }} {% endif +%} + bind [::]:{{ frontend.bind_address }} v4v6 {% if frontend.tls_terminate and not frontend.port_range %} ssl crt {{ haproxy_crt_dir }} {% endif +%} +{% if frontend.port_range %} + tcp-request session set-dst-port fc_dst_port +{% endif %} {# DDOS protection configuration. #} {% if ddos_protection_config.client_timeout %} diff --git a/haproxy-operator/tests/integration/haproxy_route_tcp_requirer.py b/haproxy-operator/tests/integration/haproxy_route_tcp_requirer.py index f6441421..2860c2d3 100644 --- a/haproxy-operator/tests/integration/haproxy_route_tcp_requirer.py +++ b/haproxy-operator/tests/integration/haproxy_route_tcp_requirer.py @@ -137,3 +137,11 @@ def update_relation_with_proxy_protocol(self): proxy_protocol=True, sni=CNAME, ) + + def update_relation_with_port_range(self): + """Update haproxy-route-tcp relation data with a port range.""" + self._haproxy_route_tcp.provide_haproxy_route_tcp_requirements( + port_range="4440-4445", + enforce_tls=False, + tls_terminate=False, + ) diff --git a/haproxy-operator/tests/integration/test_haproxy_route_tcp.py b/haproxy-operator/tests/integration/test_haproxy_route_tcp.py index 061e7f2e..d4f21e29 100644 --- a/haproxy-operator/tests/integration/test_haproxy_route_tcp.py +++ b/haproxy-operator/tests/integration/test_haproxy_route_tcp.py @@ -114,3 +114,33 @@ def test_haproxy_route_tcp( f"{configured_application_with_tls}/0", "cat /etc/haproxy/haproxy.cfg" ) assert "send-proxy" in haproxy_config + + +@pytest.mark.abort_on_fail +def test_haproxy_route_tcp_port_range( + configured_application_with_tls: str, + any_charm_haproxy_route_tcp_requirer: str, + juju: jubilant.Juju, +): + """Test that port range configuration is correctly applied. + + Assert that HAProxy is configured with a port range bind and set-dst-port directive. + Port pass-through connectivity is an HAProxy runtime concern; the charm's responsibility + is to generate the correct configuration. + """ + juju.run( + f"{any_charm_haproxy_route_tcp_requirer}/0", + "rpc", + {"method": "update_relation_with_port_range"}, + ) + juju.wait( + lambda status: jubilant.all_active( + status, configured_application_with_tls, any_charm_haproxy_route_tcp_requirer + ) + ) + + haproxy_config = juju.ssh( + f"{configured_application_with_tls}/0", "cat /etc/haproxy/haproxy.cfg" + ) + assert "bind [::]:4440-4445 v4v6" in haproxy_config + assert "tcp-request session set-dst-port fc_dst_port" in haproxy_config diff --git a/haproxy-operator/tests/unit/test_haproxy_route_tcp_lib.py b/haproxy-operator/tests/unit/test_haproxy_route_tcp_lib.py index 8774ebe1..df823ca7 100644 --- a/haproxy-operator/tests/unit/test_haproxy_route_tcp_lib.py +++ b/haproxy-operator/tests/unit/test_haproxy_route_tcp_lib.py @@ -494,3 +494,191 @@ def test_requirer_application_data_proxy_protocol_enabled(): data = TcpRequirerApplicationData(port=8080, proxy_protocol=True) assert data.proxy_protocol is True + + +def test_requirer_application_data_port_range_valid(): + """ + arrange: Create a TcpRequirerApplicationData with a valid port_range. + act: Validate the model. + assert: Model validation passes with port_range set and port None. + """ + data = TcpRequirerApplicationData(port_range="10500-10600", tls_terminate=False) + + assert data.port is None + assert data.port_range == "10500-10600" + assert data.backend_port is None + + +def test_requirer_application_data_port_range_invalid_format(): + """ + arrange: Create a TcpRequirerApplicationData with an invalid port_range format. + act: Validate the model. + assert: ValidationError is raised. + """ + with pytest.raises(ValidationError): + TcpRequirerApplicationData(port_range="invalid") + + +def test_requirer_application_data_port_range_start_greater_than_end(): + """ + arrange: Create a TcpRequirerApplicationData where port_range start > end. + act: Validate the model. + assert: ValidationError is raised. + """ + with pytest.raises(ValidationError): + TcpRequirerApplicationData(port_range="10600-10500") + + +def test_requirer_application_data_both_port_and_port_range_raises(): + """ + arrange: Create a TcpRequirerApplicationData with both port and port_range set. + act: Validate the model. + assert: ValidationError is raised. + """ + with pytest.raises(ValidationError): + TcpRequirerApplicationData(port=8080, port_range="10500-10600") + + +def test_requirer_application_data_neither_port_nor_port_range_raises(): + """ + arrange: Create a TcpRequirerApplicationData with neither port nor port_range. + act: Validate the model. + assert: ValidationError is raised. + """ + with pytest.raises(ValidationError): + TcpRequirerApplicationData() + + +def test_tcp_requirers_data_port_range_overlaps_with_single_port(): + """ + arrange: Two requirers: one with port=10500, another with port_range="10500-10600". + act: Create HaproxyRouteTcpRequirersData. + assert: Both relation IDs are in the invalid data set. + """ + app_data1 = TcpRequirerApplicationData(port=10500) + app_data2 = TcpRequirerApplicationData(port_range="10500-10600", tls_terminate=False) + + requirer1 = HaproxyRouteTcpRequirerData( + relation_id=1, + application_data=app_data1, + application="app1", + units_data=[TcpRequirerUnitData(address=MOCK_ADDRESS)], + ) + requirer2 = HaproxyRouteTcpRequirerData( + relation_id=2, + application_data=app_data2, + application="app2", + units_data=[TcpRequirerUnitData(address=ANOTHER_MOCK_ADDRESS)], + ) + + requirers_data = HaproxyRouteTcpRequirersData( + requirers_data=[requirer1, requirer2], + relation_ids_with_invalid_data=set(), + ) + + assert len(requirers_data.relation_ids_with_invalid_data) == 2 + + +def test_tcp_requirers_data_port_range_no_overlap(): + """ + arrange: Two requirers with non-overlapping port ranges. + act: Create HaproxyRouteTcpRequirersData. + assert: No relation IDs in the invalid data set. + """ + app_data1 = TcpRequirerApplicationData(port_range="10500-10600", tls_terminate=False) + app_data2 = TcpRequirerApplicationData(port_range="10700-10800", tls_terminate=False) + + requirer1 = HaproxyRouteTcpRequirerData( + relation_id=1, + application_data=app_data1, + application="app1", + units_data=[TcpRequirerUnitData(address=MOCK_ADDRESS)], + ) + requirer2 = HaproxyRouteTcpRequirerData( + relation_id=2, + application_data=app_data2, + application="app2", + units_data=[TcpRequirerUnitData(address=ANOTHER_MOCK_ADDRESS)], + ) + + requirers_data = HaproxyRouteTcpRequirersData( + requirers_data=[requirer1, requirer2], + relation_ids_with_invalid_data=set(), + ) + + assert len(requirers_data.relation_ids_with_invalid_data) == 0 + + +def test_tcp_requirers_data_overlapping_port_ranges(): + """ + arrange: Two requirers with overlapping port ranges. + act: Create HaproxyRouteTcpRequirersData. + assert: Both relation IDs are in the invalid data set. + """ + app_data1 = TcpRequirerApplicationData(port_range="10500-10600", tls_terminate=False) + app_data2 = TcpRequirerApplicationData(port_range="10550-10650", tls_terminate=False) + + requirer1 = HaproxyRouteTcpRequirerData( + relation_id=1, + application_data=app_data1, + application="app1", + units_data=[TcpRequirerUnitData(address=MOCK_ADDRESS)], + ) + requirer2 = HaproxyRouteTcpRequirerData( + relation_id=2, + application_data=app_data2, + application="app2", + units_data=[TcpRequirerUnitData(address=ANOTHER_MOCK_ADDRESS)], + ) + + requirers_data = HaproxyRouteTcpRequirersData( + requirers_data=[requirer1, requirer2], + relation_ids_with_invalid_data=set(), + ) + + assert len(requirers_data.relation_ids_with_invalid_data) == 2 + + +def test_port_range_dump_and_load(): + """ + arrange: Create TcpRequirerApplicationData with port_range. + act: Dump to databag and reload. + assert: port_range survives serialization roundtrip. + """ + original = TcpRequirerApplicationData(port_range="10500-10600", tls_terminate=False) + databag = cast(dict[str, Any], original.dump()) + + loaded = cast(TcpRequirerApplicationData, TcpRequirerApplicationData.load(databag)) + + assert loaded.port_range == "10500-10600" + assert loaded.port is None + + +def test_requirer_application_data_port_range_with_sni_raises(): + """ + arrange: Create a TcpRequirerApplicationData with port_range and sni set. + act: Validate the model. + assert: ValidationError is raised. + """ + with pytest.raises(ValidationError): + TcpRequirerApplicationData(port_range="10500-10600", sni="example.com") + + +def test_requirer_application_data_port_range_with_backend_port_raises(): + """ + arrange: Create a TcpRequirerApplicationData with port_range and backend_port set. + act: Validate the model. + assert: ValidationError is raised. + """ + with pytest.raises(ValidationError): + TcpRequirerApplicationData(port_range="10500-10600", backend_port=9090) + + +def test_requirer_application_data_port_range_with_tls_terminate_raises(): + """ + arrange: Create a TcpRequirerApplicationData with port_range and tls_terminate=True. + act: Validate the model. + assert: ValidationError is raised because port-range is transparent passthrough only. + """ + with pytest.raises(ValidationError): + TcpRequirerApplicationData(port_range="10500-10600", tls_terminate=True) diff --git a/haproxy-operator/tests/unit/test_state.py b/haproxy-operator/tests/unit/test_state.py index 05050e30..dec4ab06 100644 --- a/haproxy-operator/tests/unit/test_state.py +++ b/haproxy-operator/tests/unit/test_state.py @@ -1827,3 +1827,99 @@ def test_haproxy_route_tcp_backend_servers_send_proxy_default( backend = HAProxyRouteTcpBackend.from_haproxy_route_tcp_requirer_data(haproxy_route_tcp) assert all(server.send_proxy is False for server in backend.servers) + + +def test_haproxy_route_tcp_backend_port_range_servers( + haproxy_route_tcp_relation_data: typing.Callable[..., HaproxyRouteTcpRequirerData], +): + """ + arrange: Generate TCP relation data with port_range. + act: Initialize the HAProxyRouteTcpBackend and get servers. + assert: Each server has port=None (for dynamic port pass-through). + """ + haproxy_route_tcp = haproxy_route_tcp_relation_data( + port_range="10500-10600", tls_terminate=False + ) + backend = HAProxyRouteTcpBackend.from_haproxy_route_tcp_requirer_data(haproxy_route_tcp) + + assert all(server.port is None for server in backend.servers) + assert all(server.check is None for server in backend.servers) + + +def test_haproxy_route_tcp_backend_port_range_name( + haproxy_route_tcp_relation_data: typing.Callable[..., HaproxyRouteTcpRequirerData], +): + """ + arrange: Generate TCP relation data with port_range. + act: Initialize the HAProxyRouteTcpBackend and check the name. + assert: Backend name uses range slug format. + """ + haproxy_route_tcp = haproxy_route_tcp_relation_data( + port_range="10500-10600", tls_terminate=False + ) + backend = HAProxyRouteTcpBackend.from_haproxy_route_tcp_requirer_data(haproxy_route_tcp) + + assert backend.name == "tcp-route-requirer_10500_10600" + + +def test_haproxy_route_tcp_frontend_port_range_properties( + haproxy_route_tcp_relation_data: typing.Callable[..., HaproxyRouteTcpRequirerData], +): + """ + arrange: Generate TCP relation data with port_range and build a frontend. + act: Check frontend properties. + assert: frontend_name, bind_address, default_backend_name, and all_frontend_ports are correct. + """ + haproxy_route_tcp = haproxy_route_tcp_relation_data( + port_range="10500-10502", tls_terminate=False + ) + backend = HAProxyRouteTcpBackend.from_haproxy_route_tcp_requirer_data(haproxy_route_tcp) + frontend = HAProxyRouteTcpFrontend.from_backends([backend]) + + assert frontend.port_range == "10500-10502" + assert frontend.port is None + assert frontend.frontend_name == "haproxy_route_tcp_10500_10502" + assert frontend.bind_address == "10500-10502" + assert frontend.default_backend_name == "haproxy_route_tcp_10500_10502_default_backend" + assert frontend.all_frontend_ports == [10500, 10501, 10502] + + +def test_haproxy_route_tcp_frontend_single_port_all_frontend_ports( + haproxy_route_tcp_relation_data: typing.Callable[..., HaproxyRouteTcpRequirerData], +): + """ + arrange: Generate TCP relation data with single port and build a frontend. + act: Check all_frontend_ports, frontend_name, and bind_address. + assert: all_frontend_ports returns a single-element list and names are correct. + """ + haproxy_route_tcp = haproxy_route_tcp_relation_data(port=4000) + backend = HAProxyRouteTcpBackend.from_haproxy_route_tcp_requirer_data(haproxy_route_tcp) + frontend = HAProxyRouteTcpFrontend.from_backends([backend]) + + assert frontend.port == 4000 + assert frontend.port_range is None + assert frontend.all_frontend_ports == [4000] + assert frontend.frontend_name == "haproxy_route_tcp_4000" + assert frontend.bind_address == "4000" + assert frontend.default_backend_name == "haproxy_route_tcp_4000_default_backend" + + +def test_parse_haproxy_route_tcp_requirers_data_with_port_range( + haproxy_route_tcp_relation_data: typing.Callable[..., HaproxyRouteTcpRequirerData], +): + """ + arrange: Build HaproxyRouteTcpRequirersData with a port_range requirer. + act: Call parse_haproxy_route_tcp_requirers_data. + assert: Returns one frontend with port_range set. + """ + from charms.haproxy.v1.haproxy_route_tcp import HaproxyRouteTcpRequirersData + + requirer = haproxy_route_tcp_relation_data(port_range="10500-10600", tls_terminate=False) + requirers_data = HaproxyRouteTcpRequirersData( + requirers_data=[requirer], relation_ids_with_invalid_data=set() + ) + frontends = parse_haproxy_route_tcp_requirers_data(requirers_data) + + assert len(frontends) == 1 + assert frontends[0].port_range == "10500-10600" + assert frontends[0].port is None