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

fix: aws-auth cm deletes managed nodegroups entry #1524

Conversation

joanayma
Copy link

PR o'clock

I think there are open issues related to this but I'm not sure.

Description

The first apply of this module when used with node_groups generates a correct aws-auth file, that it's updated by AWS to add managed node groups access.
The second apply will destroy that entry that leave the managed nodes in DEGRADED state with the :

  # module.cluster.kubernetes_config_map.aws_auth[0] will be updated in-place
  ~ resource "kubernetes_config_map" "aws_auth" {
      ~ data        = {
          ~ "mapRoles"    = <<-EOT
                - groups:
                  - system:bootstrappers
                  - system:nodes
                  rolearn: arn:aws:iam::REDACTED:role/k8s-REDACTED20210811080122700700000009
                  username: system:node:{{EC2PrivateDNSName}}
                - groups:
                  - system:masters
                  rolearn: arn:aws:iam::REDACTED:role/path-to-another-user-role/REDACTED
                  username: admin
                - groups:
                  - system:masters
                  rolearn: arn:aws:iam::REDACTED:role/path-to-admin-role/REDACTED
                  username: admin
                - groups:
                  - system:readonly
                  rolearn: arn:aws:iam::REDACTED:role/path-to-user-role/REDACTED
                  username: admin
              - - groups:
              -   - system:bootstrappers
              -   - system:nodes
              -   rolearn: arn:aws:iam::REDACTED:role/path/to-my/k8s-iam/k8s-REDACTED20210811080122700700000009
              -   username: system:node:{{EC2PrivateDNSName}}
            EOT
            # (2 unchanged elements hidden)
        }
        id          = "kube-system/aws-auth"
        # (1 unchanged attribute hidden)

        # (1 unchanged block hidden)
    }

After applying the changes on this PR, the entry is kept (terraform map entry removed/added):

                  - system:bootstrappers
                  - system:nodes
                  rolearn: arn:aws:iam::REDACTED:role/k8s-REDACTED20210811080122700700000009
                  username: system:node:{{EC2PrivateDNSName}}
                - groups:
              +   - system:bootstrappers
              +   - system:nodes
              +   rolearn: arn:aws:iam::REDACTED:role/path/to-my/k8s-iam/k8s-REDACTED20210811080122700700000009
              +   username: system:node:{{EC2PrivateDNSName}}
              + - groups:
[...]
              - - groups:
              -   - system:bootstrappers
              -   - system:nodes
              -   rolearn: arn:aws:iam::REDACTED:role/path/to-my/k8s-iam/k8s-REDACTED20210811080122700700000009
              -   username: system:node:{{EC2PrivateDNSName}}

Checklist

  • pre-commit hooks passed:
Terraform fmt............................................................Passed
Terraform docs...........................................................Failed
hookid: terraform_docs

Files were modified by this hook.

Terraform validate.......................................................Passed
Terraform validate with tflint...........................................Passed

@stale
Copy link

stale bot commented Sep 14, 2021

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
To track this PR (even if closed), please open a corresponding issue if one does not already exist.

@stale stale bot added the stale label Sep 14, 2021
@stale
Copy link

stale bot commented Sep 21, 2021

This PR has been automatically closed because it has not had recent activity since being marked as stale.
Please reopen when work resumes.

@stale stale bot closed this Sep 21, 2021
@joanayma
Copy link
Author

/reopen

@joanayma joanayma changed the title fix: aws-auth cm deletes managed nodegroups entry fix: aws-auth cm deletes managed nodegroups entry WIP Sep 28, 2021
@joanayma
Copy link
Author

Can anyone reopen and take a look to this please?

@daroga0002 daroga0002 reopened this Sep 29, 2021
@stale stale bot removed the stale label Sep 29, 2021
@daroga0002
Copy link
Contributor

@joanayma could you open a issue with this problem and describing how to replicate it and what we are trying to address by this PR?

Maybe this is related to #1595 ?

@joanayma
Copy link
Author

joanayma commented Oct 1, 2021

Exactly. As far as I tested, the managed worker nodes will be unhealthy if the worker role in aws_auth configmap is not with the full path. In the other hand, the AWS IAM authenticatior requires it without the IAM path. Actually there's an ancient issue open about this kubernetes-sigs/aws-iam-authenticator#268.

Comment on lines +62 to -55
# EKS managed node groups needs full role arn
for role in module.node_groups.aws_auth_roles :
{
rolearn = role["worker_role_arn"]
username = "system:node:{{EC2PrivateDNSName}}"
groups = [
"system:bootstrappers",
"system:nodes",
],
Copy link
Author

Choose a reason for hiding this comment

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

Here I'm repeating the worker IAM role ARN to add it with the full IAM path.

Comment on lines +94 to +99
mapRoles = replace(yamlencode(
distinct(concat(
local.configmap_roles,
var.map_roles,
))
)
), "\"", "")
Copy link
Author

Choose a reason for hiding this comment

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

Here I'm removing the quotes from the formated yaml to match the diff of kubernetes_config_map.aws_auth.

@joanayma joanayma changed the title fix: aws-auth cm deletes managed nodegroups entry WIP fix: aws-auth cm deletes managed nodegroups entry Oct 1, 2021
@joanayma
Copy link
Author

joanayma commented Oct 5, 2021

It is working fine without this changes.

@joanayma joanayma closed this Oct 5, 2021
@github-actions
Copy link

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 github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants