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

Data Source to query return back a list of EKS Cluster OIDC urls #13719

Closed
dliao-tyro-admin opened this issue Jun 11, 2020 · 10 comments · Fixed by #20883
Closed

Data Source to query return back a list of EKS Cluster OIDC urls #13719

dliao-tyro-admin opened this issue Jun 11, 2020 · 10 comments · Fixed by #20883
Labels
enhancement Requests to existing resources that expand the functionality or scope. new-data-source Introduces a new data source. service/eks Issues and PRs that pertain to the eks service.
Milestone

Comments

@dliao-tyro-admin
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Our company has been looking into ways of implementing IAM roles for service accounts (IRSA) and were hoping we could simplify the discovery process of any newly create/destroyed EKS clusters through the use of tags.

By allowing us to return a list of AWS EKS clusters, we would be able to create the required IAM trust relationship.

For this to operate, it would involve:

  • list all eks clusters
  • describe each cluster
  • filter and return

New or Affected Resource(s)

Potential idea for the new data source:
Add New data source eks_clusters.

Potential Terraform Configuration

data aws_caller_identity "current" {}

data eks_clusters "this" {
  filter {
    name   = "tag:group"
    values = ["foobar"]
  }
}

data "aws_iam_policy_document" "assume_role" {
  dynamic "statement" {
    for_each = data.eks_clusters.this
    content {
      effect  = "Allow"
      actions = ["sts:AssumeRoleWithWebIdentity"]

      principals {
        type        = "Federated"
        identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:oidc-provider/${trimprefix(statement.value.identity.0.oidc.0.issuer, "https://")}"]
      }
    }
  }
}

References

  • #0000
@dliao-tyro-admin dliao-tyro-admin added the enhancement Requests to existing resources that expand the functionality or scope. label Jun 11, 2020
@ghost ghost added the service/iam Issues and PRs that pertain to the iam service. label Jun 11, 2020
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Jun 11, 2020
@ewbankkit ewbankkit added new-data-source Introduces a new data source. service/eks Issues and PRs that pertain to the eks service. and removed needs-triage Waiting for first response or review from a maintainer. service/iam Issues and PRs that pertain to the iam service. labels Jun 11, 2020
@jdheyburn
Copy link
Contributor

Happy to pick this one up.

Thinking about implementation, in the sdk there are two API calls for reading clusters: ListClusters and DescribeCluster. The former retrieves a list of cluster names in the region whereas the latter provides the detail of each cluster.

Unfortunately there's no DescribeClusters API endpoint which, if followed other API convention, would take in some parameters for filtering the result. Therefore have no choice but to follow this:

  1. Call ListClusters
  2. For each cluster name:
    1. call DescribeCluster
    2. if cluster matches the filter then add to response
  3. Return response

This makes a lot of API calls, I'll raise some tickets for the addition of a DescribeClusters endpoint - will post them here once done.

@jdheyburn
Copy link
Contributor

Update: I started working on a PR for this for the workaround specified above (while we are waiting for the API to be implemented - I raised a feature request with AWS for this). Hope to have the PR created soon so that it can be reviewed.

@Vince-Chenal
Copy link
Contributor

Hello @jdheyburn,

I was starting to write the needed datasource and then I found this issue.
Do you have something to submit already ?

Just a little remark:
Instead of retrieving the actual list of clusters as proposed in @dliao-tyro's example, we can make it as simple as it is for aws_instances by only retrieving clusters names and then leverage the existing aws_eks_cluster data source.

EC2 example:

data "aws_instances" "instances" {
  instance_tags = {
    foo = "bar"
  }
}

data "aws_instance" "instances" {
  for_each    = toset(data.aws_instances.instances.ids)
  instance_id = each.value
}

Would give us something like:

data "aws_eks_clusters" "clusters" {
  cluster_tags = {
    foo = "bar"
  }
}

data "aws_eks_cluster" "instances" {
  for_each    = toset(data.aws_eks_clusters.clusters.names)
  name         = each.value
}

@jdheyburn
Copy link
Contributor

@Vince-Chenal I can push a PR for review, some of the tests still need to be verified though.

As for the suggestion, it seems what you're proposing would have to make an additional API call unnecessarily.

data.aws_eks_clusters

  1. List EKS Clusters
  2. Describe each EKS Cluster
  3. Filter each EKS cluster based on what has been input in data.aws_eks_clusters
  4. Return to user

data.aws_eks_cluster
5. Describe EKS cluster again

We can skip the additional call altogether by returning everything that had already been described at the end of step 4.

It seems to me aws_instance was perhaps written at a time when complex objects couldn't be returned? Hence why a small number of attributes are returned in flat lists.

@jdheyburn
Copy link
Contributor

I've added a draft PR @Vince-Chenal, just need to publish the output of acceptance testing. Once they've passed I'll open it ready for review.

@Vince-Chenal
Copy link
Contributor

Hello @jdheyburn,

I still think that it would be cleaner to take advantage of what's already implemented within the eks_cluster datasource (code reuse , easier to test)
It seems like the issue is stuck but I really see the value.

Are you still on it ?

@jdheyburn
Copy link
Contributor

Hey @Vince-Chenal, sorry for the delay on this - I switched jobs inbetween and so lost track of this one. The draft PR I raised had the bulk of the work done, I just couldn't get the acceptance tests to pass. I'll see if I can get some time over the next couple of days to rebase and see if I can get them to pass.

Re using code eks_cluster datasource, that still uses the DescribeCluster endpoint which only returns one cluster. Since the AWS API is lacking DescribeClusters (i.e. returns multiple clusters) it would be better to retrieve all clusters via ListCluster and perform some local filtering. As mentioned, the codebase has changed a bit from when the draft PR was raised so I'll spend some time rebasing and getting it back in order 👍🏻

@Vince-Chenal
Copy link
Contributor

Hello here,

I created this PR to implement the datasource the way I explained here.

I did not implement any kind of filtering because the ListClusters does not allow it for now (https://docs.aws.amazon.com/sdk-for-go/api/service/eks/#ListClustersInput) and I wanted to keep it as simple as possible

@github-actions
Copy link

This functionality has been released in v3.59.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

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 Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. new-data-source Introduces a new data source. service/eks Issues and PRs that pertain to the eks service.
Projects
None yet
5 participants