From feae656a3952145ceee036c9d8511bcfde38cda7 Mon Sep 17 00:00:00 2001 From: David Boreham Date: Wed, 19 Aug 2026 19:49:50 -0600 Subject: [PATCH 1/2] Make k8s label selectors unique --- src/stack/deploy/k8s/cluster_info.py | 9 ++++++++- tests/unit/test_k8s_objects.py | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/stack/deploy/k8s/cluster_info.py b/src/stack/deploy/k8s/cluster_info.py index d763349..9399498 100644 --- a/src/stack/deploy/k8s/cluster_info.py +++ b/src/stack/deploy/k8s/cluster_info.py @@ -617,7 +617,14 @@ def get_deployments(self, image_pull_policy: str = None): spec = client.V1DeploymentSpec( replicas=self.spec.get_replicas(), template=template, - selector={"matchLabels": {"app": self.app_name}}, + # The selector has to name the service as well as the app: with + # "app" alone every Deployment in the deployment nominally selects + # every pod in it, and only the pod-template-hash the controller + # adds to each ReplicaSet keeps them from fighting. What that costs + # in the meantime is legibility -- "kubectl logs deploy/deploy-foo" + # picks an arbitrary pod out of all of them. Immutable once the + # Deployment exists, so this applies to newly created ones. + selector={"matchLabels": {"app": self.app_name, "service": service_name}}, ) deployment = client.V1Deployment( diff --git a/tests/unit/test_k8s_objects.py b/tests/unit/test_k8s_objects.py index 30b8502..de27f14 100644 --- a/tests/unit/test_k8s_objects.py +++ b/tests/unit/test_k8s_objects.py @@ -961,7 +961,7 @@ def test_deployment_shape_and_defaults(tmp_path): assert deployment["kind"] == "Deployment" assert deployment["metadata"]["name"] == "deploy-web" assert deployment["spec"]["replicas"] == 1 - assert deployment["spec"]["selector"] == {"matchLabels": {"app": TEST_CLUSTER_ID}} + assert deployment["spec"]["selector"] == {"matchLabels": {"app": TEST_CLUSTER_ID, "service": "web"}} assert deployment["spec"]["template"]["metadata"]["labels"] == { "app": TEST_CLUSTER_ID, "service": "web", @@ -996,6 +996,12 @@ def test_one_deployment_per_service(tmp_path): deployments = cluster_info.get_deployments() assert [k8s_dict(d)["metadata"]["name"] for d in deployments] == ["deploy-web", "deploy-worker"] + # Each Deployment selects its own pods and nobody else's: with "app" alone in the + # selector every one of them nominally owns every pod in the deployment. + assert [k8s_dict(d)["spec"]["selector"]["matchLabels"] for d in deployments] == [ + {"app": TEST_CLUSTER_ID, "service": "web"}, + {"app": TEST_CLUSTER_ID, "service": "worker"}, + ] # --------------------------------------------------------------------------- From ed4ebb358b002ecc098d3a7b606665aef0969982 Mon Sep 17 00:00:00 2001 From: David Boreham Date: Wed, 19 Aug 2026 20:03:20 -0600 Subject: [PATCH 2/2] Stop users from colliding with our cluster labels --- src/stack/constants.py | 4 +++ src/stack/deploy/deployment_create.py | 21 ++++++++++++++ tests/unit/test_k8s_objects.py | 42 +++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/src/stack/constants.py b/src/stack/constants.py index 21980c8..2e0e5ea 100644 --- a/src/stack/constants.py +++ b/src/stack/constants.py @@ -51,6 +51,10 @@ kube_config_filename = "kubeconfig.yml" kube_config_key = "kube-config" labels_key = "labels" +# The pod labels stack generates for itself. Both the Deployment and the Service +# select their pods on these, so a spec's `labels` may not redefine them -- see +# _check_labels in deployment_create. +reserved_label_keys = ("app", "service") network_key = "network" node_affinities_key = "node-affinities" node_tolerations_key = "node-tolerations" diff --git a/src/stack/deploy/deployment_create.py b/src/stack/deploy/deployment_create.py index d09e9ca..b009819 100644 --- a/src/stack/deploy/deployment_create.py +++ b/src/stack/deploy/deployment_create.py @@ -540,6 +540,26 @@ def _check_volume_definitions(spec): raise Exception(f"Affinity for volume {volume_name} must specify label and value") +def _check_labels(spec): + # A spec's labels go onto the pod template, where two keys are already spoken + # for: the Deployment selects its own pods on app and service, and so does the + # Service. Redefining either leaves a selector that no longer matches the pods + # it names, which k8s rejects outright -- with an error about label selectors + # rather than about the spec that caused it, at the point of creating the + # object rather than here. Every other key is the author's to use. Checked on + # every target and not just k8s: the names are reserved either way, and labels + # that do nothing on compose today would break the deployment on retargeting. + labels = spec.get_labels() + if not isinstance(labels, dict): + raise Exception(f"{constants.labels_key} must be a mapping of label name to value") + reserved = sorted(key for key in labels if key in constants.reserved_label_keys) + if reserved: + raise Exception( + f"{constants.labels_key} cannot redefine {', '.join(reserved)}: " + f"stack sets {' and '.join(constants.reserved_label_keys)} on every pod and selects on them" + ) + + def _check_runtime_class(spec): # A RuntimeClass is a k8s object, so naming one on a compose deployment cannot # mean anything. Rejected rather than ignored, on the same reasoning as the @@ -617,6 +637,7 @@ def create_operation(deployment_command_context, parsed_spec: Spec | MergedSpec, log_debug(f"parsed spec: {parsed_spec}") _check_volume_definitions(parsed_spec) _check_runtime_class(parsed_spec) + _check_labels(parsed_spec) # Validated here as well as at init, since a spec file is edited by hand. stack_secrets.validate_spec_secrets(parsed_spec) diff --git a/tests/unit/test_k8s_objects.py b/tests/unit/test_k8s_objects.py index de27f14..faac18d 100644 --- a/tests/unit/test_k8s_objects.py +++ b/tests/unit/test_k8s_objects.py @@ -27,6 +27,7 @@ import pytest from conftest import TEST_CLUSTER_ID, k8s_dict, make_cluster_info +from stack import constants MINIMAL_POD = """\ @@ -1056,6 +1057,47 @@ def test_labels_substitute_container_name_and_keep_defaults(tmp_path): } +def _check_labels(spec_obj): + from stack.deploy.deployment_create import _check_labels as check + from stack.deploy.spec import Spec + + check(Spec(obj=spec_obj)) + + +def test_reserved_label_keys_are_the_ones_generated(tmp_path): + # The check below knows which keys are stack's own; this is what keeps that + # list honest if the generated set ever changes. + cluster_info = make_cluster_info(tmp_path, MINIMAL_POD, k8s_spec()) + + labels = k8s_dict(cluster_info.get_deployments()[0])["spec"]["template"]["metadata"]["labels"] + assert set(labels) == set(constants.reserved_label_keys) + + +@pytest.mark.parametrize("key", ["app", "service"]) +def test_reserved_label_key_is_rejected(key): + # Redefining either leaves the Deployment's selector naming pods that no longer + # carry the label, which k8s rejects when the object is created -- too late, and + # complaining about label selectors rather than about the spec. + with pytest.raises(Exception, match="cannot redefine"): + _check_labels(k8s_spec(labels={key: "whatever"})) + + +def test_reserved_label_key_is_rejected_on_compose_too(tmp_path): + # Nothing reads labels on compose today, so this one costs the author nothing + # now and saves them the rejection on retargeting to k8s. + with pytest.raises(Exception, match="cannot redefine"): + _check_labels(k8s_spec(labels={"app": "whatever"}, **{"deploy-to": "compose"})) + + +def test_other_label_keys_are_accepted(tmp_path): + _check_labels(k8s_spec(labels={"{name}-tier": "web", "owner": "team"})) + + +def test_scalar_labels_are_rejected(tmp_path): + with pytest.raises(Exception, match="must be a mapping"): + _check_labels(k8s_spec(labels="web")) + + def test_node_affinity_from_spec(tmp_path): spec = k8s_spec(**{"node-affinities": [{"label": "disk", "value": "ssd"}]}) cluster_info = make_cluster_info(tmp_path, MINIMAL_POD, spec)