Removing Private Key from helm values in CLI Onboarding (Including previous helm v4 changes)

src/connectedk8s/HISTORY.rst

@@ -2,6 +2,15 @@
 
 Release History
 ===============
+1.11.1
++++++++
+* Added ARM64 support for Helm binary installation.
+* Handle removal of '--all' flag in Helm 4 to ensure compatibility.
+* Added Helm overrides support for Application Gateway for Containers (AGC).
+* Updated CSP version.
+* Updated CLIENT_PROXY_VERSION to 1.3.033892.
+* Updated pre-diagnostics version.
+
 1.11.0
 +++++
 * [Breaking Change] Removed deprecated '--app-id' and '--app-secret' RBAC parameters from the extension.

src/connectedk8s/azext_connectedk8s/__init__.py

@@ -10,6 +10,37 @@
 
 from azext_connectedk8s._help import helps
 
+
+def _patch_urllib3_getheaders_compat() -> None:
+    """
+    Restore ``urllib3.response.HTTPResponse.getheaders`` if the urllib3 that
+    az core loads is v2.x and dropped the method. The kubernetes python client
+    (through at least 29.x) still calls ``http_resp.getheaders()`` inside
+    ``ApiException.__init__``, so without this shim any non-2xx Kubernetes API
+    response (e.g. a 404 on ``read_namespace('azure-arc')`` during onboarding)
+    crashes with ``AttributeError: 'HTTPResponse' object has no attribute
+    'getheaders'``.
+
+    We can't fix this by pinning urllib3 in setup.py: az core's site-packages
+    appears earlier on sys.path than the extension's bundled copy, so az's
+    urllib3 always wins. The patch is idempotent and a no-op when getheaders
+    already exists.
+    """
+    try:
+        from urllib3.response import HTTPResponse
+    except Exception:  # pylint: disable=broad-except
+        return
+    if hasattr(HTTPResponse, "getheaders"):
+        return
+
+    def getheaders(self):  # type: ignore[no-untyped-def]
+        return self.headers
+
+    HTTPResponse.getheaders = getheaders  # type: ignore[attr-defined,method-assign]
+
+
+_patch_urllib3_getheaders_compat()
+
 if TYPE_CHECKING:
     from azure.cli.core import AzCli
     from knack.commands import CLICommand

src/connectedk8s/azext_connectedk8s/_constants.py

@@ -98,6 +98,15 @@
 )
 Custom_Token_Env_Var_Sub_Id_Missing_Fault_Type = "Required environment variable 'AZURE_SUBSCRIPTION_ID' is not set, when using Custom Acces Token."
 Release_Install_Namespace = "azure-arc-release"
+Helm_Release_Name = "azure-arc"
+Onboarding_PrivateKey_Secret_Name = "azure-arc-connect-privatekey"
+Onboarding_PrivateKey_Secret_Data_Key = "privateKey"
+Min_Agent_Version_For_Secret_Injection = "1.35.0"
+Min_Agent_Version_For_Secret_Injection_Preview = "1.35.0-preview"
+Stable_Release_Train = "stable"
+Preview_Release_Train = "preview"
+Inject_PrivateKey_Secret_Fault_Type = "inject-private-key-secret-error"
+Strip_Chart_PrivateKey_Secret_Fault_Type = "strip-chart-private-key-secret-error"
 Workload_Identity_Release_Name = "wiextension"
 Workload_Identity_Release_Namespace = "arc-workload-identity"
 Helm_Environment_File_Fault_Type = "helm-environment-file-error"
@@ -476,7 +485,7 @@
 )
 DNS_Check_Result_String = "DNS Result:"
 AZ_CLI_ADAL_TO_MSAL_MIGRATE_VERSION = "2.30.0"
-CLIENT_PROXY_VERSION = "1.3.032281"
+CLIENT_PROXY_VERSION = "1.3.033892"
 CLIENT_PROXY_FOLDER = ".clientproxy"
 API_SERVER_PORT = 47011
 CLIENT_PROXY_PORT = 47010
@@ -491,7 +500,7 @@
 # URL constants
 CLIENT_PROXY_MCR_TARGET = "azureconnectivity/proxy"
 HELM_MCR_URL = "azurearck8s/helm"
-HELM_VERSION = "v3.12.2"
+HELM_VERSION = "v3.20.1"
 Download_And_Install_Kubectl_Fault_Type = "Failed to download and install kubectl"
 Azure_Access_Token_Variable = "AZURE_ACCESS_TOKEN"
 Provisioned_Cluster_Kind = "provisionedcluster"

src/connectedk8s/azext_connectedk8s/_utils.py

@@ -104,11 +104,11 @@ def validate_connect_rp_location(cmd: CLICommand, location: str) -> None:
             "Failed to fetch resource provider details",
         )
 
-    for resourceTypes in providerDetails.resource_types:  # type: ignore[attr-defined]
+    for resourceTypes in providerDetails.resource_types:  # type: ignore[union-attr]
         if resourceTypes.resource_type == "connectedClusters":
             rp_locations = [
                 location.replace(" ", "").lower()
-                for location in resourceTypes.locations
+                for location in resourceTypes.locations  # type: ignore[union-attr]
             ]
             if location.lower() not in rp_locations:
                 telemetry.set_exception(
@@ -1266,6 +1266,196 @@ def cleanup_release_install_namespace_if_exists() -> None:
         )
 
 
+def should_use_secret_injection_flow(
+    release_train: str | None, agent_version: str | None
+) -> bool:
+    """
+    Determine whether to use the secure onboarding flow that pre-creates the
+    onboarding private key as a Kubernetes Secret (instead of passing it through
+    helm values).
+
+    Older agents whose helm chart unconditionally renders the privatekey secret
+    from ``global.onboardingPrivateKey`` must keep using the legacy flow,
+    otherwise the helm release would re-render the secret with an empty
+    ``privateKey`` and leave the cluster stuck in a disconnected state. The
+    chart change that gates the privatekey secret on the helm value being set
+    ships in:
+
+    * ``stable``  release train: agent version ``1.35.0`` and above.
+    * ``preview`` release train: agent version ``1.35.0-preview`` and above.
+    * any other release train (e.g. ``dev``): always use the secure flow.
+    * any agent version ending in ``-dev`` (e.g. ``0.2.5738-dev``) is treated
+      as a dev build and always uses the secure flow, regardless of the train
+      DP attributed it to. This handles the case where a developer overrides
+      ``HELMREGISTRY`` to a dev chart while DP still reports the original
+      ``stable``/``preview`` train.
+
+    From those versions onward the chart only renders the privatekey secret
+    when ``global.onboardingPrivateKey`` is provided, so simply omitting the
+    helm value is sufficient to hand secret ownership to the kubectl-injected
+    resource.
+    """
+    # Dev-suffixed agent versions always use the secure flow regardless of
+    if agent_version and agent_version.lower().endswith("-dev"):
+        return True
+
+    effective_train = (release_train or consts.Stable_Release_Train).lower()
+    if effective_train == consts.Stable_Release_Train:
+        cutoff = consts.Min_Agent_Version_For_Secret_Injection
+    elif effective_train == consts.Preview_Release_Train:
+        cutoff = consts.Min_Agent_Version_For_Secret_Injection_Preview
+    else:
+        # If not dev, or stable/preview train use the legacy flow for safety
+        return False
+
+    if not agent_version:
+        # Cannot determine version on a gated train. Be safe and use the legacy
+        # flow so that older agents aren't broken.
+        return False
+    try:
+        return version.parse(agent_version) >= version.parse(cutoff)
+    except Exception:  # pylint: disable=broad-except
+        # If we can't parse the version, fall back to the legacy flow.
+        return False
+
+
+def ensure_arc_namespace_with_helm_metadata() -> None:
+    """
+    Ensure the ``azure-arc`` namespace exists and is annotated/labeled so that
+    the subsequent ``helm install`` can adopt it without erroring out with
+    "exists and cannot be imported into the current release".
+    """
+    api_instance = kube_client.CoreV1Api()
+    helm_labels = {"app.kubernetes.io/managed-by": "Helm"}
+    helm_annotations = {
+        "meta.helm.sh/release-name": consts.Helm_Release_Name,
+        "meta.helm.sh/release-namespace": consts.Release_Install_Namespace,
+    }
+
+    try:
+        existing_ns = api_instance.read_namespace(consts.Arc_Namespace)
+    except ApiException as ex:
+        if ex.status != 404:
+            kubernetes_exception_handler(
+                ex,
+                consts.Get_Kubernetes_Namespace_Fault_Type,
+                error_message=f"Unable to fetch namespace '{consts.Arc_Namespace}'",
+                summary=f"Unable to fetch namespace '{consts.Arc_Namespace}'",
+            )
+            return
+        # Namespace does not exist, create it with the required metadata.
+        ns_body = kube_client.V1Namespace(
+            metadata=kube_client.V1ObjectMeta(
+                name=consts.Arc_Namespace,
+                labels=helm_labels,
+                annotations=helm_annotations,
+            )
+        )
+        try:
+            api_instance.create_namespace(ns_body)
+        except ApiException as create_ex:
+            kubernetes_exception_handler(
+                create_ex,
+                consts.Inject_PrivateKey_Secret_Fault_Type,
+                error_message=f"Unable to create namespace '{consts.Arc_Namespace}'",
+                summary=f"Unable to create namespace '{consts.Arc_Namespace}'",
+            )
+        return
+
+    # Namespace exists; merge in the Helm adoption metadata so helm can manage it.
+    metadata = existing_ns.metadata or kube_client.V1ObjectMeta()
+    labels = dict(metadata.labels or {})
+    annotations = dict(metadata.annotations or {})
+    labels.update(helm_labels)
+    annotations.update(helm_annotations)
+    patch_body = {"metadata": {"labels": labels, "annotations": annotations}}
+    try:
+        api_instance.patch_namespace(consts.Arc_Namespace, patch_body)
+    except ApiException as patch_ex:
+        kubernetes_exception_handler(
+            patch_ex,
+            consts.Inject_PrivateKey_Secret_Fault_Type,
+            error_message=(
+                f"Unable to patch namespace '{consts.Arc_Namespace}' with Helm "
+                "ownership metadata"
+            ),
+            summary=(
+                f"Unable to patch namespace '{consts.Arc_Namespace}' with Helm "
+                "ownership metadata"
+            ),
+        )
+
+
+def inject_onboarding_private_key_secret(private_key_pem: str) -> None:
+    """
+    Pre-create the onboarding private key as a Kubernetes Secret so the agents
+    can consume it without ever exposing it through helm values. The namespace
+    and secret are annotated/labeled for Helm adoption so the chart can manage
+    them on subsequent upgrades.
+
+    This MUST be called before ``helm install`` so that the cluster never sits
+    in a state where the private key is missing (which would leave it stuck in
+    a disconnected state since the Cluster Identity Operator depends on this
+    secret to fetch identity certificates from HIS).
+    """
+    print(
+        f"Step: {get_utctimestring()}: Pre-creating onboarding private key "
+        f"secret '{consts.Onboarding_PrivateKey_Secret_Name}' in namespace "
+        f"'{consts.Arc_Namespace}'."
+    )
+    ensure_arc_namespace_with_helm_metadata()
+
+    api_instance = kube_client.CoreV1Api()
+    secret_body = kube_client.V1Secret(
+        metadata=kube_client.V1ObjectMeta(
+            name=consts.Onboarding_PrivateKey_Secret_Name,
+            namespace=consts.Arc_Namespace,
+            labels={"app.kubernetes.io/managed-by": "Helm"},
+            annotations={
+                "meta.helm.sh/release-name": consts.Helm_Release_Name,
+                "meta.helm.sh/release-namespace": consts.Release_Install_Namespace,
+            },
+        ),
+        type="Opaque",
+        string_data={consts.Onboarding_PrivateKey_Secret_Data_Key: private_key_pem},
+    )
+
+    try:
+        api_instance.create_namespaced_secret(consts.Arc_Namespace, secret_body)
+    except ApiException as ex:
+        if ex.status != 409:
+            kubernetes_exception_handler(
+                ex,
+                consts.Inject_PrivateKey_Secret_Fault_Type,
+                error_message=(
+                    "Unable to create onboarding private key secret "
+                    f"'{consts.Onboarding_PrivateKey_Secret_Name}' in namespace "
+                    f"'{consts.Arc_Namespace}'"
+                ),
+                summary="Unable to create onboarding private key secret",
+            )
+            return
+        # Secret already exists - replace its contents
+        # so the cluster always uses a private key matching the public key in ARM
+        try:
+            api_instance.replace_namespaced_secret(
+                consts.Onboarding_PrivateKey_Secret_Name,
+                consts.Arc_Namespace,
+                secret_body,
+            )
+        except ApiException as replace_ex:
+            kubernetes_exception_handler(
+                replace_ex,
+                consts.Inject_PrivateKey_Secret_Fault_Type,
+                error_message=(
+                    "Unable to update existing onboarding private key secret "
+                    f"'{consts.Onboarding_PrivateKey_Secret_Name}' in namespace "
+                    f"'{consts.Arc_Namespace}'"
+                ),
+                summary="Unable to update onboarding private key secret",
+            )
+
+
 # DO NOT use this method for re-put scenarios. This method involves new NS creation for helm release. For re-put scenarios, brownfield scenario needs to be handled where helm release still stays in default NS
 def helm_install_release(
     resource_manager: str,
@@ -1288,6 +1478,7 @@ def helm_install_release(
     registry_path: str,
     aad_identity_principal_id: str | None,
     onboarding_timeout: str = consts.DEFAULT_MAX_ONBOARDING_TIMEOUT_HELMVALUE_SECONDS,
+    inject_private_key_via_helm: bool = True,
 ) -> None:
     cmd_helm_install = [
         helm_client_location,
@@ -1299,23 +1490,32 @@ def helm_install_release(
         f"global.kubernetesDistro={kubernetes_distro}",
         "--set",
         f"global.kubernetesInfra={kubernetes_infra}",
-        "--set",
-        f"global.onboardingPrivateKey={private_key_pem}",
-        "--set",
-        "systemDefaultValues.spnOnboarding=false",
-        "--set",
-        f"global.azureEnvironment={cloud_name}",
-        "--set",
-        "systemDefaultValues.clusterconnect-agent.enabled=true",
-        "--namespace",
-        f"{consts.Release_Install_Namespace}",
-        "--create-namespace",
-        "--output",
-        "json",
     ]
+    # Pass the onboarding private key through helm values only for older
+    # (stable < 1.35.0) agents. Newer agents have already received the key via
+    # a pre-created Kubernetes Secret so it never appears in helm values.
+    if inject_private_key_via_helm:
+        cmd_helm_install.extend(
+            ["--set", f"global.onboardingPrivateKey={private_key_pem}"]
+        )
+    cmd_helm_install.extend(
+        [
+            "--set",
+            "systemDefaultValues.spnOnboarding=false",
+            "--set",
+            f"global.azureEnvironment={cloud_name}",
+            "--set",
+            "systemDefaultValues.clusterconnect-agent.enabled=true",
+            "--namespace",
+            f"{consts.Release_Install_Namespace}",
+            "--create-namespace",
+            "--output",
+            "json",
+        ]
+    )
 
     # Special configurations from 2022-09-01 ARM metadata.
-    # "dataplaneEndpoints" property does not appear in arm_metadata structure for public and AGC clouds.
+    # "dataplaneEndpoints" does not appear in arm_metadata for public and AGC
     if "dataplaneEndpoints" in arm_metadata:
         if "arcConfigEndpoint" in arm_metadata["dataplaneEndpoints"]:
             notification_endpoint = arm_metadata["dataplaneEndpoints"][
@@ -1484,6 +1684,25 @@ def redact_sensitive_fields_from_string(input_text: str) -> str:
     return input_text
 
 
+def get_helm_major_version(helm_client_location: str) -> int:
+    """Returns the major version of the helm client (e.g. 3 or 4)."""
+    try:
+        result = Popen(
+            [helm_client_location, "version", "--short"],
+            stdout=PIPE,
+            stderr=PIPE,
+        )
+        out, _ = result.communicate()
+        version_str = out.decode("ascii").strip()
+        # version_str is like "v3.17.0+gabcdef" or "v4.1.3+gabcdef"
+        match = re.match(r"v(\d+)\.", version_str)
+        if match:
+            return int(match.group(1))
+    except (OSError, ValueError):
+        pass
+    return 3  # assume Helm 3 if we cannot determine version
+
+
 def get_release_namespace(
     kube_config: str | None,
     kube_context: str | None,
@@ -1494,11 +1713,14 @@ def get_release_namespace(
     cmd_helm_release = [
         helm_client_location,
         "list",
-        "-a",
         "--all-namespaces",
         "--output",
         "json",
     ]
+    # Helm 4 removed the --all flag (all releases are shown by default).
+    # Helm 3 requires --all to include non-deployed releases.
+    if get_helm_major_version(helm_client_location) < 4:
+        cmd_helm_release.insert(2, "--all")
     if kube_config:
         cmd_helm_release.extend(["--kubeconfig", kube_config])
     if kube_context: