Skip to content

Dev 5769/custom cluster admin arn #3201

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Dev 5769/custom cluster admin arn #3201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a3977e
allow custom cluster admin arn to be set
ksoviero-liscio Nov 6, 2024
e7939d6
wrong syntax
ksoviero-liscio Nov 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ locals {
# better controlled by users through Terraform
bootstrap_cluster_creator_admin_permissions = {
cluster_creator = {
principal_arn = data.aws_iam_session_context.current.issuer_arn
principal_arn = var.custom_cluster_creator_admin_arn != "" ? var.custom_cluster_creator_admin_arn : data.aws_iam_session_context.current.issuer_arn
Copy link
Member

@bryantbiggs bryantbiggs Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you shouldn't do this - you should just pass in the intended role via the cluster access entry. this was just a helper utility to make that easier for users to carry over behavior prior to cluster access entries

Copy link
Member

@bryantbiggs bryantbiggs Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just FYI

Copy link
Author

@ksoviero-liscio ksoviero-liscio Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally didn't mean to create this PR, but since I did, @bryantbiggs are you saying to disable enable_cluster_creator_admin_permissions and create an aws_eks_access_entry resource independently of the module, or do you mean something else?

Copy link
Member

@bryantbiggs bryantbiggs Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do that, or you can do it through the module like

terraform-aws-eks/tests/eks-managed-node-group/main.tf

Lines 366 to 381 in d2e6262

access_entries = {
# One access entry with a policy associated
ex-single = {
kubernetes_groups = []
principal_arn = aws_iam_role.this["single"].arn
policy_associations = {
single = {
policy_arn = "arn:aws:eks::aws:cluster-access-policy/AmazonEKSViewPolicy"
access_scope = {
namespaces = ["default"]
type = "namespace"
}
}
}
}

enable_cluster_creator_admin_permissions is just mimicking the behavior of the past - for folks who rely on Terraform going into the cluster to provision additional resources, this makes that easier. but if you aren't using the same Terraform IAM entity to go into the cluster, don't use enable_cluster_creator_admin_permissions and instead just define those entities and what permissions they should have via access_entries

Copy link
Author

@ksoviero-liscio ksoviero-liscio Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see... That's very helpful, thanks for the help!

type = "STANDARD"

policy_associations = {
Expand Down
6 changes: 6 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ variable "enable_cluster_creator_admin_permissions" {
default = false
}

variable "custom_cluster_creator_admin_arn" {
description = "ARN of cluster creator to use instead of current role for access entry administrator"
type = string
default = ""
}

################################################################################
# KMS Key
################################################################################
Expand Down
Loading