Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EKS: Unable to log in ECR on AWS China partition when adding helm chart #28460

Closed
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@yubingjiaocn
Copy link
Contributor

Describe the bug

When adding helm chart stored on ECR in AWS China partition, CDK can't generate correct login command to log in to ECR.

The correct repository address is 123456789012.dkr.ecr.cn-north-1.amazonaws.com.cn, while regex in the following code only matches 123456789012.dkr.ecr.cn-north-1.amazonaws.com, cause incomplete repository address.

private_ecr_pattern = 'oci://(?P<registry>\d+\.dkr\.ecr\.(?P<region>[a-z0-9\-]+)\.amazonaws\.com)*'

Expected Behavior

Login to correct repository address and helm chart should be installed successfully.

Current Behavior

Generated login command from log:
aws ecr get-login-password --region cn-north-1 | helm registry login --username AWS --password-stdin <Redacted>.dkr.ecr.cn-north-1.amazonaws.com; helm pull oci://<Redacted>.dkr.ecr.cn-north-1.amazonaws.com.cn/charts/<redacted> --version 0.1.0 --untar

Error log:
[ERROR] Exception: b'time="2023-12-21T22:46:15z” level=info msg="Error logging in to endpoint, trying next endpoint" error=Get \\"https://<Redacted>.dkr.ecr.cn-north-1.amazonaws.com/v2/\\": dial tcp: lookup <Redacted>.dkr.ecr.cn-north-1.amazonaws.com on 169.254.78.1:53: no such host"\nError: Get "https://<Redacted>.dkr.ecr.cn-north-1.amazonaws.com/v2/": dial tcp: lookup <Redacted>.dkr.ecr.cn-north-1.amazonaws.com on 169.254.78.1:53: no such host\n Error: unexpected status from HEAD request to https://<Redacted>.dkr.ecr.cn-north-1.amazonaws.com.cn/v2/sd-on-eks/charts/sd-on-eks/manifest/0.1.0: 401 Unauthorized

Reproduction Steps

  1. Push a OCI formatted helm chart to ECR in AWS China partition
  2. Add helm chart to EKS cluster with the following code. Replace 123456789012 to your account ID, and cn-northwest-1 to your region.
eks.HelmChart(self, "MyOCIChart",
    cluster=cluster,
    chart="some-chart",
    repository="oci://123456789012.dkr.ecr.cn-northwest-1.amazonaws.com.cn/${REPO_NAME}",
    namespace="oci",
    version="0.0.1"
)

Possible Solution

Change regex to match AWS China partition suffix

Additional Information/Context

No response

CDK CLI Version

2.99.1

Framework Version

No response

Node.js Version

v18.17.1

OS

Linux (Ubuntu 22.04.1)

Language

TypeScript

Language Version

5.1.6

Other information

No response

@yubingjiaocn yubingjiaocn added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 22, 2023
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Dec 22, 2023
@pahud pahud self-assigned this Dec 22, 2023
@pahud
Copy link
Contributor

pahud commented Dec 22, 2023

Yes we will need to update our private ecr pattern to match that.

@pahud pahud added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Dec 22, 2023
@pahud pahud removed their assignment Dec 22, 2023
@zhaojiew10
Copy link

We meet the same issue.

Cdk version 2.131. It seems not fix in latest CDK version.

Is there any exact fix time? Thanks!

@pahud
Copy link
Contributor

pahud commented Apr 9, 2024

@zhaojiew10

We'll need a contributor with China region access to submit a PR to move this forward.

@yubingjiaocn
Copy link
Contributor Author

I have submitted a PR with the change. It's a change on regex to identify if a repo is from ECR, and I added a .cn suffix to the regex.

@mergify mergify bot closed this as completed in #29778 Apr 17, 2024
mergify bot pushed a commit that referenced this issue Apr 17, 2024
…29778)

### Issue # (if applicable)

Closes #28460.

### Reason for this change

Current implementation will not identity helm charts stored in ECR on AWS CN regions, and will treat ECR as simple, unauthorized repository. 

### Description of changes

This change add support of ECR on AWS CN region by adding a optional suffix of .cn to the regex.

### Description of how you validated changes

Run the affected regex against helm repo in ECR China (123456789012.dkr.ecr.cn-northwest-1.amazonaws.com.cn)
```
import re
repository = 'oci://123456789012.dkr.ecr.cn-northwest-1.amazonaws.com.cn'
private_ecr_pattern = 'oci://(?P<registry>\d+\.dkr\.ecr\.(?P<region>[a-z0-9\-]+)\.amazonaws\.com(\.cn)?)*'
private_registry = re.match(private_ecr_pattern, repository).groupdict()
print(private_registry['registry'])
```

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment