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

could not login to OCI registry (ecr) #844

Closed
ashtonian opened this issue Mar 30, 2022 · 10 comments · Fixed by #846
Closed

could not login to OCI registry (ecr) #844

ashtonian opened this issue Mar 30, 2022 · 10 comments · Fixed by #846
Labels

Comments

@ashtonian
Copy link

ashtonian commented Mar 30, 2022

Terraform, Provider, Kubernetes and Helm Versions

Terraform version: v1.1.7
Provider version: 2.5.0
Kubernetes version: EKS 1.21

Affected Resource(s)

  • helm_release

Terraform Configuration Files

data "aws_ecr_authorization_token" "token" {}

resource "helm_release" "platform" {
  chart = "oci://${trimprefix(data.aws_ecr_authorization_token.token.proxy_endpoint, "https://")}/my-chart"
  repository_username = data.aws_ecr_authorization_token.token.user_name
  repository_password = data.aws_ecr_authorization_token.token.password
}

Steps to Reproduce

  1. terraform apply

Expected Behavior

Helm provider should be able to login to ecr.

Actual Behavior

could not login to OCI registry "myaccount.dkr.ecr.us-east-1.amazonaws.com": login attempt to https://myaccount.dkr.ecr.us-east-1.amazonaws.com/v2/ failed with status: 403 Forbidden

It was working yesterday after the new release, but now seems to not be working. Maybe its not refreshing the creds properly as ecr credentials require being constantly refreshed?

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@ashtonian ashtonian added the bug label Mar 30, 2022
@ashtonian
Copy link
Author

ashtonian commented Mar 30, 2022

I think this is actually caused by tf refresh which is concerning, I'm not sure how to get past this without deleting the release from state.

Edit: Reverting to 2.4.1 works without modifying state

data "aws_ecr_authorization_token" "token" {}

resource "null_resource" "helm_login" {
  triggers = {
    always_run = timestamp()
  }

  provisioner "local-exec" {
    command = <<-EOT
      HELM_EXPERIMENTAL_OCI=1 \
      helm registry login \
        --username "${data.aws_ecr_authorization_token.token.user_name}" \
        --password "${data.aws_ecr_authorization_token.token.password}" \
        ${data.aws_ecr_authorization_token.token.proxy_endpoint}
    EOT
  }
}

@rpf3
Copy link

rpf3 commented Mar 30, 2022

I'm noticing the same thing from my CI server. Reverting the provider back to 2.4.1 and the CLI to 3.7.1 fixed things again but I had to also revert the code to my solution described here using a null resource.

#666 (comment)

@junaid-ali
Copy link

@ashtonian have you tried time_rotating resource to force re-read of the token data source?

@rpf3
Copy link

rpf3 commented Mar 31, 2022

@junaid-ali maybe I'm missing something but how would you hook that up to the ecr_authorization_token data source?

@ashtonian
Copy link
Author

further investigating - the state file has the correct key in hex format which is what matches aws ecr get-login-password so I think its not using it to login.

@junaid-ali
Copy link

@rpf3 I thought the data source isn't refreshing so was wondering if a time_rotating resource could help there. It's definitely not needed in this case: https://www.terraform.io/language/data-sources#data-resource-behavior

I checked the debug log for plan and seeing this:

[WARN] Provider "registry.terraform.io/hashicorp/aws" produced an unexpected new value for data.aws_ecr_authorization_token.this.
      - .authorization_token: inconsistent values for sensitive attribute
      - .expires_at: was cty.StringVal("2022-04-02T00:08:04Z"), but now cty.StringVal("2022-04-03T03:17:12Z")
      - .password: inconsistent values for sensitive attribute

That could be causing the issue for helm_release to not use the updated datasource values.

@mKeRix
Copy link
Contributor

mKeRix commented Apr 6, 2022

The issue seems to be that during the refresh stage, Terraform only has access to the values present in the state file. However, logging in during the refresh is actually unnecessary, as the chart is never pulled. I submitted a PR at #846 that fixes this issue by dropping the login during refresh. During the create & update it will always use the latest configured credentials. I tested it successfully locally using Amazon ECR as described in the original post.

@junaid-ali
Copy link

@mKeRix for the terraform helm diff to work, wouldn't the login be necessary?

@mKeRix
Copy link
Contributor

mKeRix commented Apr 6, 2022

The refresh doesn't look at the chart from what I saw - just the resource diff function, which seems to be run with the updated credentials though. I pinged the original author of the code in the PR to make sure I didn't overlook something.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants