Add ARM64 support for Helm installation in connectedk8s and k8s-extension#34
Add ARM64 support for Helm installation in connectedk8s and k8s-extension#34
Conversation
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
CodeGen Tools Feedback CollectionThank you for using our CodeGen tool. We value your feedback, and we would like to know how we can improve our product. Please take a few minutes to fill our codegen survey |
68f52b5 to
10c0313
Compare
There was a problem hiding this comment.
Pull request overview
Adds ARM64-aware Helm client installation for the connectedk8s and k8s-extension Azure CLI extensions by detecting host architecture, constructing arch-specific Helm artifact paths, and ensuring the versioned download directory exists before saving/extracting the Helm archive.
Changes:
- Detects host CPU architecture (
platform.machine()) and uses it to build Helm archive and install paths. - Adds an ARM64 download flow that pulls Helm from the official Helm distribution when running on ARM64.
- Fixes a latent directory-creation issue by creating
~/.azure/helm/<version>before downloading the archive.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| src/k8s-extension/azext_k8s_extension/custom.py | Adds arch detection, ARM64 Helm download path, and corrects download directory creation for Helm install flow. |
| src/connectedk8s/azext_connectedk8s/custom.py | Mirrors the same ARM64-aware Helm installation updates and directory handling fix for connectedk8s. |
| for i in range(retry_count): | ||
| try: | ||
| from urllib.request import urlretrieve | ||
|
|
||
| urlretrieve(official_helm_url, download_location_file) | ||
| break | ||
| except Exception as e: | ||
| if i == retry_count - 1: | ||
| telemetry.set_exception( | ||
| exception=e, | ||
| fault_type=consts.Download_Helm_Fault_Type, | ||
| summary="Unable to download helm client.", | ||
| ) | ||
| raise CLIInternalError( | ||
| f"Failed to download helm client: {e}", | ||
| recommendation="Please check your internet connection.", | ||
| ) | ||
| time.sleep(retry_delay) |
There was a problem hiding this comment.
urlretrieve is called without an explicit network timeout; in some network failure modes this can hang the CLI indefinitely even with retries. Consider switching to a request method that allows setting a per-attempt timeout (and optionally distinguishing transient vs permanent HTTP errors) so the retry loop behaves as intended.
| if machine_type.lower() in ("aarch64", "arm64"): | ||
| arch = "arm64" | ||
| else: | ||
| arch = "amd64" |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
can we continue keeping the amd64 as default just for playing safe?
| if arch == "arm64": | ||
| # ARM64 Helm binaries are downloaded from the official Helm distribution. | ||
| official_helm_url = f"https://get.helm.sh/{download_file_name}" | ||
| download_location_file = os.path.expanduser( | ||
| os.path.join(download_location, download_file_name) | ||
| ) | ||
| for i in range(retry_count): | ||
| try: | ||
| from urllib.request import urlretrieve | ||
|
|
||
| urlretrieve(official_helm_url, download_location_file) | ||
| break |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| for i in range(retry_count): | ||
| try: | ||
| from urllib.request import urlretrieve | ||
|
|
||
| urlretrieve(official_helm_url, download_location_file) | ||
| break | ||
| except Exception as e: | ||
| if i == retry_count - 1: | ||
| telemetry.set_exception( | ||
| exception=e, | ||
| fault_type=consts.Download_Helm_Fault_Type, | ||
| summary="Unable to download helm client.", | ||
| ) | ||
| raise CLIInternalError( | ||
| f"Failed to download helm client: {e}", | ||
| recommendation="Please check your internet connection.", | ||
| ) | ||
| time.sleep(retry_delay) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if machine_type.lower() in ("aarch64", "arm64"): | ||
| arch = "arm64" | ||
| else: | ||
| arch = "amd64" |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| # ARM64 Helm binaries are downloaded from the official Helm distribution. | ||
| official_helm_url = f"https://get.helm.sh/{download_file_name}" |
There was a problem hiding this comment.
The ARM64 path downloads Helm directly from https://get.helm.sh, which bypasses the cloud-aware MCR routing used for amd64 (including sovereign cloud handling via get_mcr_path). This can break in restricted/sovereign environments where get.helm.sh is blocked. Consider hosting/using an ARM64 artifact in the same registry flow when possible, or make the base download endpoint configurable / cloud-aware so ARM64 follows the same cloud routing guarantees.
| # ARM64 Helm binaries are downloaded from the official Helm distribution. | |
| official_helm_url = f"https://get.helm.sh/{download_file_name}" | |
| # ARM64 Helm binaries are downloaded from a configurable base URL. | |
| # By default, public cloud uses https://get.helm.sh, but restricted/sovereign | |
| # environments should configure AZURE_HELM_ARM64_BASE_URL to a reachable mirror. | |
| base_url = os.getenv("AZURE_HELM_ARM64_BASE_URL") | |
| active_directory_endpoint = getattr( | |
| getattr(cmd.cli_ctx, "cloud", None), | |
| "endpoints", | |
| None, | |
| ) | |
| active_directory_endpoint = ( | |
| getattr(active_directory_endpoint, "active_directory", None) | |
| ) | |
| if not base_url: | |
| # Detect non-public (sovereign/restricted) clouds via the AAD endpoint. | |
| # For such clouds we avoid hardcoding a public Internet endpoint and | |
| # require an explicit, cloud-appropriate mirror to be configured. | |
| if active_directory_endpoint and ( | |
| "login.microsoftonline.com" not in active_directory_endpoint | |
| ): | |
| raise CLIInternalError( | |
| "Downloading the ARM64 helm client from the public Internet " | |
| "is not supported for this cloud.", | |
| recommendation=( | |
| "Mirror the required helm ARM64 archive to a location " | |
| "reachable from this environment and set the " | |
| "AZURE_HELM_ARM64_BASE_URL environment variable to " | |
| "that base URL." | |
| ), | |
| ) | |
| # Public cloud default behaviour: use official Helm distribution. | |
| base_url = "https://get.helm.sh" | |
| official_helm_url = f"{base_url.rstrip('/')}/{download_file_name}" |
| download_location_file = os.path.expanduser( | ||
| os.path.join(download_location, download_file_name) | ||
| ) | ||
| for i in range(retry_count): | ||
| try: | ||
| from urllib.request import urlretrieve | ||
|
|
||
| urlretrieve(official_helm_url, download_location_file) |
There was a problem hiding this comment.
The ARM64 download uses urlretrieve to fetch a Helm archive but does not perform any integrity verification (e.g., SHA256 check) before unpacking and executing it. Since this is an executable supply-chain surface, please validate the download (for example by fetching the corresponding .sha256sum from get.helm.sh and checking it) before extracting/installing.
| download_location_file = os.path.expanduser( | |
| os.path.join(download_location, download_file_name) | |
| ) | |
| for i in range(retry_count): | |
| try: | |
| from urllib.request import urlretrieve | |
| urlretrieve(official_helm_url, download_location_file) | |
| checksum_url = f"{official_helm_url}.sha256sum" | |
| download_location_file = os.path.expanduser( | |
| os.path.join(download_location, download_file_name) | |
| ) | |
| for i in range(retry_count): | |
| try: | |
| from urllib.request import urlopen, urlretrieve | |
| # Download the Helm archive. | |
| urlretrieve(official_helm_url, download_location_file) | |
| # Download and verify the corresponding SHA256 checksum. | |
| with urlopen(checksum_url) as resp: | |
| checksum_content = resp.read().decode("utf-8", errors="replace") | |
| # Expected format: "<sha256> <filename>" | |
| first_line = checksum_content.strip().splitlines()[0] | |
| expected_sha256 = first_line.split()[0] | |
| sha256 = hashlib.sha256() | |
| with open(download_location_file, "rb") as f: | |
| for chunk in iter(lambda: f.read(8192), b""): | |
| sha256.update(chunk) | |
| calculated_sha256 = sha256.hexdigest() | |
| if calculated_sha256 != expected_sha256: | |
| raise CLIInternalError( | |
| "Integrity check failed for downloaded Helm client.", | |
| recommendation="Please retry the operation. If the problem " | |
| "persists, verify network integrity and that get.helm.sh is reachable.", | |
| ) |
| for i in range(retry_count): | ||
| try: | ||
| client.pull( | ||
| target=f"{mcr_url}/{consts.HELM_MCR_URL}:{artifactTag}", |
There was a problem hiding this comment.
what do we need to do, who to reach out, Dalec?, to get the ARM64 version of helm available in mcr?
By directly download from https://get.helm.sh, we will need to run checksum, signature signing etc to ensure the binaries are not tempered.
There was a problem hiding this comment.
Left a comment for this here: https://github.com/AzureArcForKubernetes/connectedk8s/pull/34/changes#r2861545918
There was a problem hiding this comment.
Nice info, @bavneetsingh16. do we need to give different mcr path for arm64, any suggestions?
| recommendation="Please check your internet connection.", | ||
| if arch == "arm64": | ||
| # ARM64 Helm binaries are downloaded from the official Helm distribution. | ||
| official_helm_url = f"https://get.helm.sh/{download_file_name}" |
There was a problem hiding this comment.
We can’t directly reference the Helm url because we need to account for network constraints, such as endpoint allowlists. This endpoint isn’t allowlisted today, which would break customer scenarios.
Instead, the approved and compliant Helm package must be uploaded to ACR and then mapped to MCR. This approach also ensures support for restricted cloud environments which would otherwise break.
| # Fetch system related info | ||
| operating_system = platform.system().lower() | ||
| machine_type = platform.machine() | ||
| if machine_type.lower() in ("aarch64", "arm64"): |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Summary
platform.machine() -> arch) and uses the computedarchin Helm artifact/file/path construction.connectedk8sandk8s-extension.Why
Validation
.whl.Directory handling note
~/.azure/helm/<version>/<archive-file>.~/.azure/helm, not the required version subdirectory~/.azure/helm/<version>.~/.azure/helm/<version>was missing, even if~/.azure/helmalready existed.<version>directory had already been created by a prior run.download_location(~/.azure/helm/<version>) directly before download.