Commit b282d0a2 authored by Sergio Gimenez's avatar Sergio Gimenez
Browse files

fix(kubernetes): give helm releases a name Kubernetes accepts

The adapter used the deployment unit ref as the release name. Charts name
their Services after the release, and a Service name must be a DNS-1035
label: lowercase, starting with a letter. SRM passes instance UUIDs (about
10 in 16 start with a digit) and instance names with '_', so those installs
failed with a Service "... is invalid" that the logs reported only as
helm_command_failed.

release_name() lowercases the ref, turns anything but letters, digits and
'-' into '-', puts "r-" in front when it doesn't start with a letter, and
keeps to helm's 53 characters. A ref that is already valid keeps its name,
and terminate, update and status follow the external id activate returns,
so existing releases are unaffected. helm's "is invalid" errors now log as
invalid_kubernetes_object.

Co-Authored-By: default avatarClaude Opus 5.5 <noreply@anthropic.com>
parent 1c4dcfa2
Loading
Loading
Loading
Loading
Loading
+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",
}


+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:
+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"