terraform-provider-helm icon indicating copy to clipboard operation
terraform-provider-helm copied to clipboard

Add OCI registry block to provider

Open monester opened this issue 2 years ago • 5 comments

Description

Add OCI registry to block to provider definition to have same behavior as Helm binary where you are doing helm registry login only once. As registry is specified in every helm_release username and password are marked as required variables. In documentation it is also marked as option for private registries only.

This would allow to keep passwords to registries away from helm_release resource and ability to use short live token without affecting chart resource.

For compatibility reasons I've kept ability to specify password for OCI in helm_release resource.

Example code for short live token:

data "aws_ecr_authorization_token" "token" {}

resource "aws_ecr_repository" "foo" {
  name = "foo"
}

provider "helm" {
  registry {
    url      = aws_ecr_repository.foo.repository_url
    username = data.aws_ecr_authorization_token.token.user_name
    password = data.aws_ecr_authorization_token.token.password
  }
}

Acceptance tests

  • None

Release Note

Release note for CHANGELOG:

Add "registry" block to helm provider

References

Related issues: #861

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

monester avatar May 08 '22 15:05 monester

@jrhouston can you please review code. I think it is good idea to put oci registries inside helm provider block.

monester avatar May 22 '22 18:05 monester

@jrhouston may I kindly ask for your review? This is a blocker for normal operation of helm charts in OCI registry.

I've found a workaround to perform docker login to registry with dynamic credentials, but it is ugly:

provider "helm" {
  kubernetes {
    host                   = data.aws_eks_cluster.cluster.endpoint
    cluster_ca_certificate = base64decode(data.aws_eks_cluster.cluster.certificate_authority.0.data)
    exec {
      api_version = "client.authentication.k8s.io/v1alpha2"
      command     = "bash"
      args = [
        "-c", <<EOF
        export AWS_REGION=us-west-2
        aws ecr get-login-password | helm registry login  --username AWS --password-stdin 123456.dkr.ecr.us-west-2.amazonaws.com >/dev/null 2>&1;
        aws eks get-token --cluster eks-cluster
EOF
      ]
    }
  }
}

monester avatar Jul 06 '22 07:07 monester

@jrhouston may I kindly ask for your review? This is a blocker for normal operation of helm charts in OCI registry.

I've found a workaround to perform docker login to registry with dynamic credentials,

yes, please review and let’s get this merged.

The posted workaround is basically same as doing helm login with the helm binary before running terraform. This is super finicky, because if your binary version does not match what’s in the provider, you get weird login failures.

For reliably logging in with a token in a CI pipeline, getting this block into the provider is critical.

treksler avatar Jul 16 '22 02:07 treksler

Yep – I agree adding this in makes sense, thanks for opening this @monester. I'll review this as soon as I can to get this into the next release.

jrhouston avatar Jul 19 '22 03:07 jrhouston

@jrhouston This PR is crucial for pulling OCI artefacts from AWS ECR directly from Terraform cloud. (without triggering the redeployment of charts, each time a Terraform plan is updated - as well explained above with short time lived token).

Is there an ETA when such a block might be included into the Helm Terraform provider (and subsequent release)? Many thanks!

pieeri avatar Sep 05 '22 13:09 pieeri

At the risk of piling on: the current state of the helm provider is that it cannot be used with OCI repositories, which the upstream helm project is strongly promoting and signaling is the preferred repo format going forward. Some combination of this and https://github.com/hashicorp/terraform-provider-helm/pull/928 would solve the issue generally, but both PRs have now been open and untouched for months. What gives?

n-oden avatar Oct 24 '22 18:10 n-oden

@n-oden @pieeri This PR is now next on the to-do list for @BBBmau and I to review and get in for the next release of this provider. We're a small team of maintainers so thanks for your patience on this one folks!

jrhouston avatar Oct 31 '22 18:10 jrhouston

@BBBmau any chance to have a release with this fix in november ? or maybe in december if your are busy

DanielCastronovo avatar Nov 16 '22 10:11 DanielCastronovo

Hello! Thank you for opening this PR, I went ahead and added a test for the registry block. @jrhouston could you take a look?

BBBmau avatar Nov 18 '22 15:11 BBBmau

Thank you, thank you, thank you!

n-oden avatar Dec 15 '22 18:12 n-oden

@jrhouston Indeed, thanks a lot! That will help a lot analysing Terraform plans!

pieeri avatar Dec 19 '22 16:12 pieeri

I am struggling to use this. i am trying to use the following pattern:

  data "aws_ecr_authorization_token" "token" {}
  provider "helm" {
    kubernetes {
      config_path    = "eks.kube"
      config_context = "default"
    }
    registry {
      url = "oci://ACCOUNTNUMBER.dkr.ecr.us-east-1.amazonaws.com"
      username = data.aws_ecr_authorization_token.token.user_name
      password = data.aws_ecr_authorization_token.token.password
    }
  }

But as we are using (sub)modules we need to set the providers in the root module. The root module has multiple, aliased, AWS providers. How do I instruct data "aws_ecr_authorization_token" "token" {} to use a specific AWS provider alias/profile?

--- Update ---

In case it helps someone else, apparently it's like this :-) ;

data "aws_ecr_authorization_token" "token" {
  provider = aws.ecr
}

cunningr avatar Dec 20 '22 15:12 cunningr

I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Jan 21 '23 02:01 github-actions[bot]