Skip to content

Commit 27da99e

Browse files
authored
fix(eks): Allow helm pull from non-ECR OCI repositories (#25237)
This fixes an issue that occurs when installing Helm charts from OCI repositories other than ECR (e.g. `oci://ghcr.io/grafana-operator/helm-charts/grafana-operator`). The issue occurs because `subprocess.check_output` is called with `shell=True`, which only invokes the first item of the passed cmnd sequence. So instead of `helm pull $REPO --version $VERSION --untar`, only `helm` is executed, which results in the following output (also observed in the Lambda logs): <details> <summary>Output</summary> ``` The Kubernetes package manager Common actions for Helm: - helm search: search for charts - helm pull: download a chart to your local directory to view - helm install: upload the chart to Kubernetes - helm list: list releases of charts Environment variables: | Name | Description | |------------------------------------|---------------------------------------------------------------------------------------------------| | $HELM_CACHE_HOME | set an alternative location for storing cached files. | | $HELM_CONFIG_HOME | set an alternative location for storing Helm configuration. | | $HELM_DATA_HOME | set an alternative location for storing Helm data. | | $HELM_DEBUG | indicate whether or not Helm is running in Debug mode | | $HELM_DRIVER | set the backend storage driver. Values are: configmap, secret, memory, sql. | | $HELM_DRIVER_SQL_CONNECTION_STRING | set the connection string the SQL storage driver should use. | | $HELM_MAX_HISTORY | set the maximum number of helm release history. | | $HELM_NAMESPACE | set the namespace used for the helm operations. | | $HELM_NO_PLUGINS | disable plugins. Set HELM_NO_PLUGINS=1 to disable plugins. | | $HELM_PLUGINS | set the path to the plugins directory | | $HELM_REGISTRY_CONFIG | set the path to the registry config file. | | $HELM_REPOSITORY_CACHE | set the path to the repository cache directory | | $HELM_REPOSITORY_CONFIG | set the path to the repositories file. | | $KUBECONFIG | set an alternative Kubernetes configuration file (default "~/.kube/config") | | $HELM_KUBEAPISERVER | set the Kubernetes API Server Endpoint for authentication | | $HELM_KUBECAFILE | set the Kubernetes certificate authority file. | | $HELM_KUBEASGROUPS | set the Groups to use for impersonation using a comma-separated list. | | $HELM_KUBEASUSER | set the Username to impersonate for the operation. | | $HELM_KUBECONTEXT | set the name of the kubeconfig context. | | $HELM_KUBETOKEN | set the Bearer KubeToken used for authentication. | | $HELM_KUBEINSECURE_SKIP_TLS_VERIFY | indicate if the Kubernetes API server's certificate validation should be skipped (insecure) | | $HELM_KUBETLS_SERVER_NAME | set the server name used to validate the Kubernetes API server certificate | | $HELM_BURST_LIMIT | set the default burst limit in the case the server contains many CRDs (default 100, -1 to disable)| Helm stores cache, configuration, and data based on the following configuration order: - If a HELM_*_HOME environment variable is set, it will be used - Otherwise, on systems supporting the XDG base directory specification, the XDG variables will be used - When no other location is set a default location will be used based on the operating system By default, the default directories depend on the Operating System. The defaults are listed below: | Operating System | Cache Path | Configuration Path | Data Path | |------------------|---------------------------|--------------------------------|-------------------------| | Linux | $HOME/.cache/helm | $HOME/.config/helm | $HOME/.local/share/helm | | macOS | $HOME/Library/Caches/helm | $HOME/Library/Preferences/helm | $HOME/Library/helm | | Windows | %TEMP%\helm | %APPDATA%\helm | %APPDATA%\helm | Usage: helm [command] Available Commands: completion generate autocompletion scripts for the specified shell create create a new chart with the given name dependency manage a chart's dependencies env helm client environment information get download extended information of a named release help Help about any command history fetch release history install install a chart lint examine a chart for possible issues list list releases package package a chart directory into a chart archive plugin install, list, or uninstall Helm plugins pull download a chart from a repository and (optionally) unpack it in local directory push push a chart to remote registry login to or logout from a registry repo add, list, remove, update, and index chart repositories rollback roll back a release to a previous revision search search for a keyword in charts show show information of a chart status display the status of the named release template locally render templates test run tests for a release uninstall uninstall a release upgrade upgrade a release verify verify that a chart at the given path has been signed and is valid version print the client version information Flags: --burst-limit int client-side default throttling limit (default 100) --debug enable verbose output -h, --help help for helm --kube-apiserver string the address and the port for the Kubernetes API server --kube-as-group stringArray group to impersonate for the operation, this flag can be repeated to specify multiple groups. --kube-as-user string username to impersonate for the operation --kube-ca-file string the certificate authority file for the Kubernetes API server connection --kube-context string name of the kubeconfig context to use --kube-insecure-skip-tls-verify if true, the Kubernetes API server's certificate will not be checked for validity. This will make your HTTPS connections insecure --kube-tls-server-name string server name to use for Kubernetes API server certificate validation. If it is not provided, the hostname used to contact the server is used --kube-token string bearer token used for authentication --kubeconfig string path to the kubeconfig file -n, --namespace string namespace scope for this request --registry-config string path to the registry config file (default "/Users/fabian/Library/Preferences/helm/registry/config.json") --repository-cache string path to the file containing cached repository indexes (default "/Users/fabian/Library/Caches/helm/repository") --repository-config string path to the file containing repository names and URLs (default "/Users/fabian/Library/Preferences/helm/repositories.yaml") Use "helm [command] --help" for more information about a command. ``` </details> From the `subprocess` logs: > On POSIX with shell=True, the shell defaults to /bin/sh. If args is a string, the string specifies the command to execute through the shell. This means that the string must be formatted exactly as it would be when typed at the shell prompt. This includes, for example, quoting or backslash escaping filenames with spaces in them. **If args is a sequence, the first item specifies the command string, and any additional items will be treated as additional arguments to the shell itself.** See https://docs.python.org/3/library/subprocess.html#popen-constructor Merging the `helm` command and its arguments into the first item of the `cmnd` list fixes the issue. To quickly verify the fix, this code can be added to the file: ```python logging.basicConfig(level=logging.DEBUG) tmpdir = tempfile.TemporaryDirectory() chart_dir = get_chart_from_oci(tmpdir.name, "oci://ghcr.io/grafana-operator/helm-charts/grafana-operator", "v5.0.0-rc0") ``` Without the fix, running the file with Python should then result in the behaviour described above. With the fix the chart should be pulled correctly into the temporary directory. ```sh python packages/aws-cdk-lib/aws-eks/lib/kubectl-handler/helm/__init__.py ``` Closes #24710 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent ecd7374 commit 27da99e

File tree

19 files changed

+268
-154
lines changed

19 files changed

+268
-154
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.eks-helm-asset.js.snapshot/asset.1eabd374284db340b74179e3429008132f5b6b0b7b28d472d852807d7f5f9746/outbound.js

-69
This file was deleted.

packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.eks-helm-asset.js.snapshot/asset.45017ac1fb5b50dac36a255c328b0fe125f18a8e6d3689e188eab5e3a1bf8146/outbound.js

+73
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ def get_oci_cmd(repository, version):
127127
cmnd = [f"helm pull {repository} --version {version} --untar"]
128128
else:
129129
logger.error("OCI repository format not recognized, falling back to helm pull")
130-
cmnd = ['helm', 'pull', repository, '--version', version, '--untar']
130+
cmnd = [f"helm pull {repository} --version {version} --untar"]
131131

132132
return cmnd
133133

0 commit comments

Comments
 (0)