Skip to content

Commit

Permalink
fix(eks): revert shell=True and allow public ecr to work (aws#20724)
Browse files Browse the repository at this point in the history
This fixes the change made by the following PR. aws#19778

`shell=True` caused regression observed in the following issue: [20402](aws#20402)

The code should now allow Public and Private AWS ECR repositories to work with oci prefix.

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? No

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
harshadbhatia authored and daschaa committed Jul 9, 2022
1 parent 33c2da1 commit 60e5ff6
Show file tree
Hide file tree
Showing 3 changed files with 1,325 additions and 1,324 deletions.
31 changes: 16 additions & 15 deletions packages/@aws-cdk/aws-eks/lib/kubectl-handler/helm/__init__.py
Expand Up @@ -94,20 +94,30 @@ def helm_handler(event, context):


def get_oci_cmd(repository, version):

# Generates OCI command based on pattern. Public ECR vs Private ECR are treated differently.
cmnd = []
pattern = '\d+.dkr.ecr.[a-z]+-[a-z]+-\d.amazonaws.com'
private_ecr_pattern = '\d+.dkr.ecr.[a-z]+-[a-z]+-\d.amazonaws.com'
public_ecr = 'public.ecr.aws'

registry = repository.rsplit('/', 1)[0].replace('oci://', '')

if re.fullmatch(pattern, registry) is not None:
if re.fullmatch(private_ecr_pattern, registry) is not None:
logger.info("Found AWS private repository")
region = registry.replace('.amazonaws.com', '').split('.')[-1]
cmnd = [
f"aws ecr get-login-password --region {region} | " \
f"helm registry login --username AWS --password-stdin {registry}; helm pull {repository} --version {version} --untar"
]
elif registry.startswith(public_ecr):
logger.info("Found AWS public repository, will use default region as deployment")
region = os.environ.get('AWS_REGION', 'us-east-1')

cmnd = [
f"aws ecr-public get-login-password --region {region} | " \
f"helm registry login --username AWS --password-stdin {public_ecr}; helm pull {repository} --version {version} --untar"
]
else:
logger.info("Non AWS OCI repository found")
logger.error("OCI repository format not recognized, falling back to helm pull")
cmnd = ['helm', 'pull', repository, '--version', version, '--untar']

return cmnd
Expand All @@ -122,8 +132,7 @@ def get_chart_from_oci(tmpdir, release, repository = None, version = None):
while retry > 0:
try:
logger.info(cmnd)
env = get_env_with_oci_flag()
output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=tmpdir, env=env)
output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=tmpdir, shell=True)
logger.info(output)

return os.path.join(tmpdir, release)
Expand All @@ -137,13 +146,6 @@ def get_chart_from_oci(tmpdir, release, repository = None, version = None):
raise Exception(f'Operation failed after {maxAttempts} attempts: {output}')


def get_env_with_oci_flag():
env = os.environ.copy()
env['HELM_EXPERIMENTAL_OCI'] = '1'

return env


def helm(verb, release, chart = None, repo = None, file = None, namespace = None, version = None, wait = False, timeout = None, create_namespace = None):
import subprocess

Expand Down Expand Up @@ -172,8 +174,7 @@ def helm(verb, release, chart = None, repo = None, file = None, namespace = None
retry = maxAttempts
while retry > 0:
try:
env = get_env_with_oci_flag()
output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=outdir, env=env)
output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=outdir)
logger.info(output)
return
except subprocess.CalledProcessError as exc:
Expand Down

0 comments on commit 60e5ff6

Please sign in to comment.