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

aws-eks: OCI HelmChart from public.ecr.aws only works in us-east-1 #23977

Closed
esw-amzn opened this issue Feb 2, 2023 · 4 comments · Fixed by #24104 · 4 remaining pull requests
Closed

aws-eks: OCI HelmChart from public.ecr.aws only works in us-east-1 #23977

esw-amzn opened this issue Feb 2, 2023 · 4 comments · Fixed by #24104 · 4 remaining pull requests
Assignees
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

@esw-amzn
Copy link

esw-amzn commented Feb 2, 2023

Describe the bug

I'm trying to add a HelmChart from the public.ecr.aws OCI repository to a stack deployed in us-west-2 and am getting this error.

Received response status [FAILED] from custom resource. Message returned: Error: b'\nAn error occurred (UnsupportedCommandException) when calling the GetAuthorizati
onToken operation: GetAuthorizationToken command is only supported in us-east-1.\nLogin Succeeded\nError: public.ecr.aws/aws-controllers-k8s/apigatewayv2-chart:0.1.
5: not found\n'  

Expected Behavior

I expect the chart to install without error.

Current Behavior

Failing with the following error.

Received response status [FAILED] from custom resource. Message returned: Error: b'\nAn error occurred (UnsupportedCommandException) when calling the GetAuthorizati
onToken operation: GetAuthorizationToken command is only supported in us-east-1.\nLogin Succeeded\nError: public.ecr.aws/aws-controllers-k8s/apigatewayv2-chart:0.1.
5: not found\n'  

Reproduction Steps

The HelmChart used looks like this. Stack was deployed into us-west-2.

        const chart = new eks.HelmChart(this, 'Resource', {
            cluster: cluster,
            chart: 'apigatewayv2-controller',
            version: '0.1.5',
            repository: 'oci://public.ecr.aws/aws-controllers-k8s/apigatewayv2-chart',
            namespace: NAMESPACE,
            values: {
                cloudProvider: 'aws',
                awsRegion: cluster.env.region,
                autoDiscovery: {
                  clusterName: cluster.clusterName,
                },
                rbac: {
                  serviceAccount: {
                    create: false,
                    name: this._service_account.serviceAccountName,
                  },
                },
                metrics: {
                  enabled: true,
                },
              },
            });

Possible Solution

Change this line of code https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-eks/lib/kubectl-handler/helm/__init__.py#L113

to

region = 'us-east-1'

Additional Information/Context

According to the Amazon ECR public registries documentation the region should always be us-east-1 for the ecr-public get-login-password operation.

https://docs.aws.amazon.com/AmazonECR/latest/public/public-registries.html#public-registry-auth
When authenticating to a public registry, always authenticate to the us-east-1 Region when using the AWS CLI.

CDK CLI Version

2.62.2 (build c164a49)

Framework Version

No response

Node.js Version

v14.21.2

OS

Amazon Linux 2

Language

Typescript

Language Version

4.7.4

Other information

No response

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

pahud commented Feb 2, 2023

related to #23052

@pahud pahud removed the needs-triage This issue or PR still needs to be triaged. label Feb 2, 2023
@pahud pahud closed this as completed Feb 2, 2023
@pahud pahud added the duplicate This issue is a duplicate. label Feb 2, 2023
@github-actions
Copy link

github-actions bot commented Feb 2, 2023

⚠️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.

@pahud pahud reopened this Feb 2, 2023
@pahud
Copy link
Contributor

pahud commented Feb 2, 2023

reopening this issue as #23176 is not resolved yet.

@pahud pahud added p2 effort/small Small work item – less than a day of effort and removed duplicate This issue is a duplicate. labels Feb 2, 2023
@mergify mergify bot closed this as completed in #24104 Feb 23, 2023
mergify bot pushed a commit that referenced this issue Feb 23, 2023
fix helm deploy login for public ECR repositories

I have tested this issue fixed in `us-east-1` and `us-west-2` integ testing for
```
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-east-1
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-west-2
```

Closes #23977.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
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.

Naumel pushed a commit that referenced this issue Feb 24, 2023
fix helm deploy login for public ECR repositories

I have tested this issue fixed in `us-east-1` and `us-west-2` integ testing for
```
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-east-1
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-west-2
```

Closes #23977.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
beck3905 pushed a commit to beck3905/aws-cdk that referenced this issue Feb 28, 2023
fix helm deploy login for public ECR repositories

I have tested this issue fixed in `us-east-1` and `us-west-2` integ testing for
```
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-east-1
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-west-2
```

Closes aws#23977.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 13, 2023
fix helm deploy login for public ECR repositories

I have tested this issue fixed in `us-east-1` and `us-west-2` integ testing for
```
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-east-1
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-west-2
```

Closes aws#23977.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this issue Mar 28, 2023
fix helm deploy login for public ECR repositories

I have tested this issue fixed in `us-east-1` and `us-west-2` integ testing for
```
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-east-1
yarn integ-runner integ.eks-helm-asset.js --force --parallel-regions us-west-2
```

Closes aws#23977.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment