fix(kubernetes): derive Helm names accepted by Kubernetes

Sergio Gimenezrequested to merge
fix/helm-release-name into refactor/tfsdk-architecture-alignment 
flowchart LR
  IN["deployment unit ref"] --> L["lowercase, runs outside a-z 0-9 - become -,<br/>trim -"] --> Q{"starts with<br/>a letter?"}
  Q -- "no" --> P["prepend r-"] --> C
  Q -- "yes" --> C["cut to 53 chars (Helm limit)"] --> OUT["release name<br/>= external ID for status / update / terminate"]
ref release name
3f9c1e2a-... (UUID) r-3f9c1e2a-...
podinfo_demo podinfo-demo
podinfo podinfo (unchanged) 
flowchart LR
  F["fix/helm-release-name<br/>tf-sdk !201 (this MR)"] --> R["refactor/tfsdk-architecture-alignment<br/>(no parent MR yet)"]
  R -. "release step" .-> PUB["publish openop-tfsdk"] -. "bump pin, now 1.1.1" .-> SRM["SRM lockfile"]

The adapter used the deployment-unit reference directly as its Helm release name. UUIDs can start with digits and CAMARA instance names can contain underscores; charts naming Services after the release then failed Kubernetes validation.

Normalize the name to lowercase letters, digits and hyphens, prepend r- when needed, and retain Helm's 53-character limit. Valid existing names keep their spelling. Activation returns the derived external ID, which status/update/terminate continue to use. Also classify Helm invalid-object errors so the logs identify the actual failure.

Pipeline 28593 is red at its first step, isort, on 10 files this MR doesn't touch. The base branch fails on the same 10 files, so the job stopped before black, flake8 and pytest. Replaying the job in its image (python:3.12-slim) on both branches:

validate-mr step base refactor/tfsdk-architecture-alignment this MR
isort 10 files the same 10 files, none from this diff
black clean clean
flake8 clean clean
pytest common/edgecloud, common/network 3 + 3 passed 3 + 3 passed
pytest contract/ 51 passed 60 passed (9 new)

Fixing those imports belongs on the base branch, not in this hotfix. The VM overlay exercised digit-leading UUIDs and a requested name containing an underscore through ready and deletion.

Target refactor/tfsdk-architecture-alignment: this is one hotfix commit on the new openop_tfsdk package, not the separate legacy adapter MR !199 (closed). No parent MR for the architecture-alignment branch was open when checked; agree its integration path with the SDK maintainers.

Release requirement: publish a new immutable openop-tfsdk version, then update SRM's pinned dependency and lockfile. SRM still pins 1.1.1; merging this source change alone does not ship it in SRM. The branch has no push pipeline; its CI validation job is configured for merge requests.

Edited by Sergio Gimenez

Merge request reports

src/openop_tfsdk/contract/adapters/kubernetes.py

+22 −3
Changes for src/openop_tfsdk/contract/adapters/kubernetes.py: 22 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ What a kubernetes control path can execute. Not topology, so not read from 
the bootstrap YAML.

"""

import json

import re

import subprocess

from pathlib import Path

from typing import Optional
@@ -82,6 +83,24 @@ def _chart_args(unit) -> list: 
    ]





# helm's own limit; charts name their objects after the release, often with a suffix

_RELEASE_NAME_MAX = 53





def release_name(deployment_unit_ref: str) -> str:

    """A helm release name that charts can build Kubernetes object names from.



    Charts name Services after the release, and a Service name is a DNS-1035 label:

    lowercase letters, digits and '-', starting with a letter. A deployment unit ref is

    often a UUID or an instance name with '_', so it is made into one. A ref that already is

    one is used as it is.

    """

    name = re.sub(r"[^a-z0-9-]+", "-", deployment_unit_ref.lower()).strip("-")

    if not name[:1].isalpha():

        name = f"r-{name}" if name else "r"

    return name[:_RELEASE_NAME_MAX].rstrip("-")





class KubernetesCapabilityAdapter:

    """Declares the kubernetes execution surface and executes it through helm."""
@@ -100,13 +119,13 @@ class KubernetesCapabilityAdapter: 
        unit = request.deployment_unit

        if unit is None or not unit.artifact_ref:

            raise ValidationError("activate needs a deployment_unit with an artifact_ref")

        release_name = unit.deployment_unit_ref

        release = release_name(unit.deployment_unit_ref)

        namespace = self._infrastructure.get("namespace", "default")



        result = _helm(

            [

                "install",

                release_name,

                release,

                *_chart_args(unit),

                "--namespace",

                namespace,
@@ -128,7 +147,7 @@ class KubernetesCapabilityAdapter: 
            operation_id=request.operation_id,

            capability_kind=request.capability_kind,

            state=LifecycleState.ACTIVE,

            external_id=release_name,

            external_id=release,

        )



    def read_runtime_state(self, request: CapabilityRuntimeStateRequest) -> CapabilityRuntimeState:

src/openop_tfsdk/_diagnostics.py

+1 −0
Changes for src/openop_tfsdk/_diagnostics.py: 1 added line, 0 removed lines.
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ _HELM_REASONS = { 
    "forbidden": "authorization_denied",

    "release: not found": "release_not_found",

    "cannot re-use a name": "release_name_in_use",

    "is invalid": "invalid_kubernetes_object",

}

tests/contract/test_kubernetes_release_name.py

0 → 100644
+73 −0
Changes for tests/contract/test_kubernetes_release_name.py: 73 added lines, 0 removed lines.
Original line number Diff line number Diff line 
"""Helm release names must work as Kubernetes object names, whatever the unit ref is."""



import re

import subprocess

from types import SimpleNamespace



import pytest



from openop_tfsdk._diagnostics import helm_diagnostics

from openop_tfsdk.contract.adapters.kubernetes import (

    KubernetesCapabilityAdapter,

    release_name,

)



DNS_1035 = re.compile(r"^[a-z]([-a-z0-9]*[a-z0-9])?$")





@pytest.mark.parametrize(

    ("ref", "expected"),

    [

        ("podinfobcn01", "podinfobcn01"),

        ("podinfo-bcn-01", "podinfo-bcn-01"),

        ("261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f", "r-261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f"),

        ("podinfo_bcn_01", "podinfo-bcn-01"),

        ("Video_Athens_01", "video-athens-01"),

        ("__", "r"),

    ],

)

def test_release_name_is_a_dns_1035_label(ref, expected):

    name = release_name(ref)



    assert name == expected

    assert DNS_1035.match(name)





def test_release_name_fits_helm_limit():

    name = release_name("a" * 40 + "_" + "b" * 40)



    assert len(name) <= 53

    assert DNS_1035.match(name)





def test_activate_installs_under_the_safe_name_and_reports_it(monkeypatch):

    calls = []



    def run(args, **kwargs):

        calls.append(args)

        return subprocess.CompletedProcess(args, 0, stdout="", stderr="")



    monkeypatch.setattr(subprocess, "run", run)

    unit = SimpleNamespace(

        deployment_unit_ref="261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f",

        runtime_type="helm",

        artifact_ref="https://charts.example/podinfo-6.7.1.tgz",

    )

    request = SimpleNamespace(operation_id="op-1", capability_kind="workload", deployment_unit=unit)



    result = KubernetesCapabilityAdapter({"namespace": "oop-apps"}).activate(request)



    assert calls[0][:3] == ["helm", "install", "r-261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f"]

    # terminate, update and status use the external id, so they follow the same name

    assert result.external_id == "r-261fd4d0-fc8a-43f5-b0b9-39b4ba9dbf7f"





def test_an_invalid_object_name_is_named_in_the_diagnostics():

    stderr = (

        'Error: INSTALLATION FAILED: 1 error occurred:\n\t* Service "1abc-podinfo" is invalid: '

        "metadata.name: Invalid value"

    )



    diagnostics = helm_diagnostics(subprocess.CompletedProcess([], 1, stderr=stderr))



    assert diagnostics["reason"] == "invalid_kubernetes_object"