Skip to content

Commit 72f2a95

Browse files
authored
fix(eks): nested OCI repository names for private ECR helmchart deployments are not properly handled (#23378)
Fixes #22340 I've changed the way of extracting components (registry domain & region) from repository to regexp with named capture groups. I've tested it by extending `integ.eks-helm-asset.ts` with second deployment from manually created private ECR. But I didn't provide coverage for this issue, because before deployment private ECR has to be created and filled by an example chart - i'm not aware of any simple way to automate a second part in tests. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-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`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 462a42b commit 72f2a95

File tree

2 files changed

+17
-12
lines changed

2 files changed

+17
-12
lines changed

packages/@aws-cdk/aws-eks/lib/kubectl-handler/helm/__init__.py

+9-11
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import shutil
77
import tempfile
88
import zipfile
9-
from urllib.parse import urlparse, unquote
109

1110
logger = logging.getLogger()
1211
logger.setLevel(logging.INFO)
@@ -95,26 +94,25 @@ def helm_handler(event, context):
9594

9695
def get_oci_cmd(repository, version):
9796
# Generates OCI command based on pattern. Public ECR vs Private ECR are treated differently.
98-
cmnd = []
99-
private_ecr_pattern = '\d+.dkr.ecr.[a-z]+-[a-z]+-\d.amazonaws.com'
100-
public_ecr = 'public.ecr.aws'
97+
private_ecr_pattern = 'oci://(?P<registry>\d+.dkr.ecr.(?P<region>[a-z]+-[a-z]+-\d).amazonaws.com)*'
98+
public_ecr_pattern = 'oci://(?P<registry>public.ecr.aws)*'
10199

102-
registry = repository.rsplit('/', 1)[0].replace('oci://', '')
100+
private_registry = re.match(private_ecr_pattern, repository).groupdict()
101+
public_registry = re.match(public_ecr_pattern, repository).groupdict()
103102

104-
if re.fullmatch(private_ecr_pattern, registry) is not None:
103+
if private_registry['registry'] is not None:
105104
logger.info("Found AWS private repository")
106-
region = registry.replace('.amazonaws.com', '').split('.')[-1]
107105
cmnd = [
108-
f"aws ecr get-login-password --region {region} | " \
109-
f"helm registry login --username AWS --password-stdin {registry}; helm pull {repository} --version {version} --untar"
106+
f"aws ecr get-login-password --region {private_registry['region']} | " \
107+
f"helm registry login --username AWS --password-stdin {private_registry['registry']}; helm pull {repository} --version {version} --untar"
110108
]
111-
elif registry.startswith(public_ecr):
109+
elif public_registry['registry'] is not None:
112110
logger.info("Found AWS public repository, will use default region as deployment")
113111
region = os.environ.get('AWS_REGION', 'us-east-1')
114112

115113
cmnd = [
116114
f"aws ecr-public get-login-password --region {region} | " \
117-
f"helm registry login --username AWS --password-stdin {public_ecr}; helm pull {repository} --version {version} --untar"
115+
f"helm registry login --username AWS --password-stdin {public_registry['registry']}; helm pull {repository} --version {version} --untar"
118116
]
119117
else:
120118
logger.error("OCI repository format not recognized, falling back to helm pull")

packages/@aws-cdk/aws-eks/test/integ.eks-helm-asset.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ class EksClusterStack extends Stack {
6060
createNamespace: true,
6161
});
6262

63+
// there is no opinionated way of testing charts from private ECR, so there is description of manual steps needed to reproduce:
64+
// 1. `export AWS_PROFILE=youraccountprofile; aws ecr create-repository --repository-name helm-charts-test/s3-chart --region YOUR_REGION`
65+
// 2. `helm pull oci://public.ecr.aws/aws-controllers-k8s/s3-chart --version v0.1.0`
66+
// 3. Login to ECR (howto: https://docs.aws.amazon.com/AmazonECR/latest/userguide/push-oci-artifact.html )
67+
// 4. `helm push s3-chart-v0.1.0.tgz oci://YOUR_ACCOUNT_ID.dkr.ecr.YOUR_REGION.amazonaws.com/helm-charts-test/`
68+
// 5. Change `repository` in above test to oci://YOUR_ACCOUNT_ID.dkr.ecr.YOUR_REGION.amazonaws.com/helm-charts-test
69+
// 6. Run integration tests as usual
70+
6371
this.cluster.addHelmChart('test-oci-chart-different-release-name', {
6472
chart: 'lambda-chart',
6573
release: 'lambda-chart-release',
@@ -79,4 +87,3 @@ new integ.IntegTest(app, 'aws-cdk-eks-helm', {
7987
});
8088

8189
app.synth();
82-

0 commit comments

Comments
 (0)