Validate smartstack.yaml in paasta validate

We currently do this in an internal repo (`py-gitolite`), but I don't
see why we don't just do this in `paasta validate` - especially since
moving this check left means that folks are alerted to broken config
earlier rather than when they attempt to integrate.

The smartstack_schema.json in this repo was behind the one in
py-gitolite, so I've gone ahead and made them match.

The majority of the added code (outside the tests, which I had Claude
Code regenerate for me) is more-or-less taken straight from py-gitolite
with minimal changes.

I ran `paasta validate` over all soaconfigs and verified that there are
no new failures when run with this code.

Author: nemacysts

paasta_tools/cli/cmds/validate.py

@@ -34,6 +34,9 @@
 
 import pytz
 from croniter import croniter
+from environment_tools.type_utils import available_location_types
+from environment_tools.type_utils import compare_types
+from environment_tools.type_utils import convert_location_type
 from jsonschema import Draft4Validator
 from jsonschema import exceptions
 from jsonschema import FormatChecker
@@ -120,6 +123,7 @@ class AutoscalingValidationError(SoaValidationError):
     "rollback",  # automatic rollbacks during deployments
     "tron",  # batch workloads
     "eks",  # eks workloads
+    "smartstack",  # mesh configs
     "autotuned_defaults/kubernetes",
     "autotuned_defaults/cassandracluster",
 }
@@ -166,6 +170,26 @@ class AutoscalingValidationError(SoaValidationError):
     METRICS_PROVIDER_PROMQL: {"desired_active_requests_per_replica"},
 }
 
+# Listener names in Envoy cannot exceed 128 characters and use the
+# following format:
+#    service_name.namespace.listener
+# This naming scheme leaves 128 - 10 = 118 characters
+# for the Smartstack service name and namespace.
+# See COREBACK-6303 for more context.
+MAX_ENVOY_NAME_LEN = 118
+
+# Socket names cannot exceed 108 characters, and the longest socket
+# name generated by HAProxy uses the following format:
+#    /var/run/synapse/sockets/service_name.namespace.LONGPID.sock.tmp
+# This naming scheme leaves 108 - 43 = 65 characters combined for the
+# Smartstack service name and namespace. We leave a generous buffer
+# to arrive at a maximum name length of 55, in case e.g. the .sock
+# suffix is renamed to a longer name for certain sockets.
+# See SMTSTK-204 for more context.
+# NOTE: the above is mostly still true - but the path we use is now /var/run/envoy/sockets/...
+# so we may want to adjust this a tad in the future ;)
+MAX_SMARTSTACK_NAME_LEN = 55
+
 
 class ConditionConfig(TypedDict, total=False):
     """
@@ -971,6 +995,190 @@ def validate_cpu_burst(service_path: str) -> bool:
     return returncode
 
 
+def _check_smartstack_name_length_envoy(service: str, namespace: str) -> None:
+    """Ensures that Smartstack service name and namespace does not
+    exceed the limit on the length of Envoy's listener names
+    """
+    if len(service) + len(namespace) > MAX_ENVOY_NAME_LEN:
+        raise ValueError(
+            "Service name and namespace exceeds max listener name length in Envoy. Note that the full listener name "
+            'is "{}.{}.listener". Please rename so that the combined length of the service name and namespace does '
+            "not exceed {} characters".format(service, namespace, MAX_ENVOY_NAME_LEN),
+        )
+
+
+def _check_smartstack_name_length(service: str, namespace: str) -> None:
+    """Ensure that Smartstack name does not
+    exceed limits on HAProxy socket name
+    """
+    if len(service + namespace) > MAX_SMARTSTACK_NAME_LEN:
+        socket_name = "/var/run/synapse/sockets/{}.{}.sock.LONGPID.tmp".format(
+            service,
+            namespace,
+        )
+        raise ValueError(
+            "Name exceeds max socket name length. Note that the full socket name under the HAProxy naming scheme "
+            'is "{}". Please rename so that the combined length of the service name and namespace does not '
+            "exceed {} characters".format(socket_name, MAX_SMARTSTACK_NAME_LEN),
+        )
+
+
+@lru_cache()
+def _get_etc_services() -> list[str]:
+    with open("/etc/services") as f:
+        return f.read().splitlines()
+
+
+@lru_cache()
+def _get_etc_services_entry(port_lookup: int) -> str | None:
+    entries = _get_etc_services()
+    for entry in entries:
+        try:
+            service = entry.split()[0]
+            port = entry.split()[1]
+            if port.startswith("%s/" % str(port_lookup)):
+                return service
+        except IndexError:
+            continue
+    return None
+
+
+def _check_proxy_port_in_use(service: str, namespace: str, port: int) -> bool:
+    if port is None:
+        return False
+
+    # TODO(luisp): this should probably check the distributed /nail/etc/services
+    # smartstack.yamls OR we should more automatically manage /etc/services
+    etc_services_entry = _get_etc_services_entry(port)
+    if etc_services_entry is None:
+        return False
+    elif f"{service}.{namespace}" == etc_services_entry:
+        return False
+    else:
+        raise ValueError(
+            (
+                "port {} is already in use by {} according to /etc/services, it cannot be used by "
+                "{}.{}. Please either pick a different port or update /etc/services via puppet"
+            ).format(port, etc_services_entry, service, namespace),
+        )
+
+
+def _check_advertise_discover(
+    smartstack_data: dict[str, Any]
+) -> None:  # XXX: we should use a TypedDict here
+    """Need to ensure a few properties about smartstack files
+    1) discover is a member of advertise
+    2) discovery and advertise contain valid locations
+    3) extra_advertise contains valid locations
+    4) rhs of extra_advertise are >= discover type
+    """
+
+    def assert_valid_type(location_type: str) -> None:
+        if location_type not in available_location_types():
+            raise ValueError(
+                'Location type "{}" not a valid Yelp location type'.format(
+                    location_type,
+                ),
+            )
+
+    def assert_valid_location(location_string: str) -> None:
+        try:
+            typ, loc = location_string.split(":")
+            assert len(convert_location_type(loc, typ, typ)) == 1
+        except Exception:
+            raise ValueError(
+                'Location string "{}" not a valid Yelp location'.format(
+                    location_string,
+                ),
+            )
+
+    advertise = smartstack_data.get("advertise", ["region"])
+    discover = smartstack_data.get("discover", "region")
+    if discover not in advertise:
+        raise ValueError(
+            'discover key "{}" not a member of advertise "{}"'.format(
+                discover,
+                advertise,
+            ),
+        )
+    for location_type in [discover] + advertise:
+        assert_valid_type(location_type)
+
+    extra_advertisements = smartstack_data.get("extra_advertise", {})
+    for source, destinations in extra_advertisements.items():
+        assert_valid_location(source)
+        for destination in destinations:
+            assert_valid_location(destination)
+            dest_type = destination.split(":")[0]
+            if compare_types(dest_type, discover) > 0:
+                raise ValueError(
+                    'Right hand side "{}" less general than discover type "{}". Your advertisement would potentially '
+                    "result in more hosts seeing your service than intended. Please change the type of your RHS to be "
+                    ">= the discover type".format(destination, discover),
+                )
+
+
+def _check_smartstack_valid_proxy(proxied_through: str) -> None:
+    """Checks whether its parameter is a valid Smartstack namespace. Can be used for proxied_through or clb_proxy."""
+    proxy_service, proxy_namespace = proxied_through.split(".")
+    proxy_smartstack_filename = f"{proxy_service}/smartstack.yaml"
+    try:
+        yaml_data = get_config_file_dict(proxy_smartstack_filename)
+        if proxy_namespace not in yaml_data:
+            raise ValueError(
+                f"{proxied_through} is not a valid Smartstack namespace to proxy through: "
+                f"{proxy_namespace} not found in {proxy_smartstack_filename}.",
+            )
+    except FileNotFoundError:
+        raise ValueError(
+            f"{proxied_through} is not a valid Smartstack namespace to proxy through: "
+            f"{proxy_smartstack_filename} does not exist.",
+        )
+
+
+def _check_smartstack_proxied_through(
+    smartstack_data: dict[str, Any]
+) -> None:  # XXX: we should use a TypedDict here
+    """Checks the proxied_through field of a Smartstack namespace refers to another valid Smartstack namespace"""
+    if "proxied_through" not in smartstack_data:
+        return
+
+    proxied_through = smartstack_data["proxied_through"]
+    _check_smartstack_valid_proxy(proxied_through)
+
+
+def validate_smartstack(service_path: str) -> bool:
+    if not os.path.exists(os.path.join(service_path, "smartstack.yaml")):
+        # not every service is mesh-registered, exit early if this is the case
+        return True
+
+    config = get_config_file_dict(os.path.join(service_path, "smartstack.yaml"))
+    if not config:
+        print(
+            failure(
+                "smartstack.yaml is empty - if this service is not mesh-registed, please remove this file.",
+                "http://paasta.readthedocs.io/en/latest/yelpsoa_configs.html",
+            )
+        )
+        return False
+
+    _, service = path_to_soa_dir_service(service_path)
+    for namespace, namespace_config in config.items():
+        # XXX(luisp): these should all really either return bools or be enforced in the schema
+        # ...i'm mostly leaving these as-is since i'm trying to remove some internal code that
+        # duplicates a bunch of paasta validate checks (i.e., py-gitolite)
+        _check_smartstack_name_length_envoy(service, namespace)
+        if namespace_config["proxy_port"]:
+            _check_smartstack_name_length(service, namespace)
+        proxy_port = namespace_config["proxy_port"]
+        _check_proxy_port_in_use(service, namespace, proxy_port)
+        _check_advertise_discover(namespace_config)
+        _check_smartstack_proxied_through(namespace_config)
+
+    print(success("All SmartStack configs are valid"))
+    return True
+
+
 def paasta_validate_soa_configs(
     service: str, service_path: str, verbose: bool = False
 ) -> bool:
@@ -993,6 +1201,7 @@ def paasta_validate_soa_configs(
         validate_secrets,
         validate_min_max_instances,
         validate_cpu_burst,
+        validate_smartstack,
     ]
 
     # NOTE: we're explicitly passing a list comprehension to all()
@@ -1006,7 +1215,12 @@ def paasta_validate(args):
 
     :param args: argparse.Namespace obj created from sys.args by cli
     """
-    service_path = get_service_path(args.service, args.yelpsoa_config_root)
     service = args.service or guess_service_name()
+    service_path = get_service_path(service, args.yelpsoa_config_root)
+
+    # not much we can do if we have no path to inspect ;)
+    if not service_path:
+        return 1
+
     if not paasta_validate_soa_configs(service, service_path, args.verbose):
         return 1

paasta_tools/cli/schemas/smartstack_schema.json

@@ -61,6 +61,11 @@
                 "minimum": 0,
                 "maximum": 60000
             },
+            "timeout_client_ms": {
+                "type": "integer",
+                "minimum": 0,
+                "maximum": 86400000
+            },
             "timeout_server_ms": {
                 "type": "integer",
                 "minimum": 0,
@@ -116,6 +121,7 @@
             "mode": {
                 "enum": [
                     "http",
+                    "http2",
                     "https",
                     "tcp"
                 ]
@@ -130,6 +136,10 @@
                     "CLUSTER_PROVIDED"
                 ]
             },
+            "choice_count": {
+                "type": "integer",
+                "minimum": 1
+            },
             "chaos": {
                 "type": "object",
                 "additionalProperties": {
@@ -154,6 +164,9 @@
             "proxied_through": {
                 "type": "string"
             },
+            "clb_proxy": {
+                "type": "string"
+            },
             "fixed_delay": {
                 "type": "object",
                 "additionalProperties": {