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

[aws-eks] wrong order of cluster.awsAuth.addMastersRole & cluster.addNodegroup breaks awsAuth #7595

Closed
arjanschaaf opened this issue Apr 24, 2020 · 4 comments · Fixed by #8901
Assignees
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. p1

Comments

@arjanschaaf
Copy link
Contributor

If you execute the cluster.awsAuth.addMastersRole & cluster.addNodegroup calls in the wrong order you end up with an awsAuth configmap in your cluster that misses the rolemapping for the node group role. The result of this is that the nodes in the nodegroup can't connect to the apiserver and the cluster is broken.

Reproduction Steps

  1. create an EKS cluster
    const cluster = new eks.Cluster(this, 'eks-cloud-infra', {
      clusterName: 'eks-cloud-infra',
      defaultCapacity: 0,
      version: '1.15',
      vpc: vpc,
    });
  1. add a nodegroup to the new cluster
    cluster.addNodegroup('node-group-cloud-infra', {
      instanceType: new ec2.InstanceType('t3.large'),
      minSize: 1,
      maxSize: 10
    });
  1. add additional master roles (we do this because we want to give 2 groups of users control of the EKS cluster that authenticate using an IAM Role created by Control Tower
    cluster.awsAuth.addMastersRole(iam.Role.fromRoleArn(this, 'administrators', 'arn:aws:iam::12345:role/AWSReservedSSO_AWSAdministratorAccess_234d7d11d88198f7'));
    cluster.awsAuth.addMastersRole(iam.Role.fromRoleArn(this, 'powerusers', 'arn:aws:iam::12345:role/AWSReservedSSO_PowerUserAccess_432fa27f4402ce4e'));

Error Log

expected result from kubectl -n kube-system get configmap aws-auth -o yaml

apiVersion: v1
data:
  mapAccounts: '[]'
  mapRoles: |
    - groups:
      - system:masters
      rolearn: arn:aws:iam::12345:role/AWSReservedSSO_AWSAdministratorAccess_437d7d11d88198f7
      username: arn:aws:iam::12345:role/AWSReservedSSO_AWSAdministratorAccess_437d7d11d88198f7
    - groups:
      - system:masters
      rolearn: arn:aws:iam::12345:role/AWSReservedSSO_PowerUserAccess_ebffa27f4402ce4e
      username: arn:aws:iam::12345:role/AWSReservedSSO_PowerUserAccess_ebffa27f4402ce4e
    - groups:
      - system:bootstrappers
      - system:nodes
      rolearn: arn:aws:iam::12345:role/CloudInfrastructureStack-ekscloudinfraNodegroupnod-GKQNHZRL0EM6
      username: system:node:{{EC2PrivateDNSName}}
  mapUsers: '[]'
kind: ConfigMap

actual result from kubectl -n kube-system get configmap aws-auth -o yaml

  apiVersion: v1
  data:
    mapAccounts: '[]'
    mapRoles: '[{"rolearn":"arn:aws:iam::12345:role/AWSReservedSSO_AWSAdministratorAccess_437d7d11d88198f7","username":"arn:aws:iam::12345:role/AWSReservedSSO_AWSAdministratorAccess_437d7d11d88198f7","groups":["system:masters"]},{"rolearn":"arn:aws:iam::12345:role/AWSReservedSSO_PowerUserAccess_ebffa27f4402ce4e","username":"arn:aws:iam::12345:role/AWSReservedSSO_PowerUserAccess_ebffa27f4402ce4e","groups":["system:masters"]}]'
    mapUsers: '[]'
  kind: ConfigMap

Please note that the formatting looks strange, but more importantly the system:bootstrappers & system:nodes groups are missing for the nodegroup role

Environment

  • CLI Version : aws-cli/2.0.5
  • Framework Version: 1.34.1 (build 7b21aa0)
  • OS : OSX
  • Language : Typescript

Other

Changing the order of executing the cluster.awsAuth.addMastersRole & cluster.addNodegroup methods leads to the desired result:

    cluster.awsAuth.addMastersRole(iam.Role.fromRoleArn(this, 'administrators', 'arn:aws:iam::12345:role/AWSReservedSSO_AWSAdministratorAccess_234d7d11d88198f7'));
    cluster.awsAuth.addMastersRole(iam.Role.fromRoleArn(this, 'powerusers', 'arn:aws:iam::12345:role/AWSReservedSSO_PowerUserAccess_432fa27f4402ce4e'));

    cluster.addNodegroup('node-group-cloud-infra', {
      instanceType: new ec2.InstanceType('t3.large'),
      minSize: 1,
      maxSize: 10
    });

This is 🐛 Bug Report

@arjanschaaf arjanschaaf added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 24, 2020
@SomayaB SomayaB added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Apr 29, 2020
@eladb eladb added the p1 label May 1, 2020
@eladb
Copy link
Contributor

eladb commented May 1, 2020

Thanks for reporting!

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label May 19, 2020
@eladb eladb added this to the EKS Developer Preview milestone Jun 24, 2020
@eladb eladb changed the title wrong order of cluster.awsAuth.addMastersRole & cluster.addNodegroup breaks awsAuth [EKS Bug] wrong order of cluster.awsAuth.addMastersRole & cluster.addNodegroup breaks awsAuth Jun 24, 2020
@eladb eladb removed this from the EKS Developer Preview milestone Jun 24, 2020
@pahud
Copy link
Contributor

pahud commented Jul 4, 2020

@eladb

I believe this is because we don't do the AwsAuth for managed ng and let the eks control plane do it instead. So the state will be inconsistent.

// As managed nodegroup will auto map the instance role to RBAC behind the scene and users don't have to manually
// do it anymore. We don't need to print out the instance role arn now.

Maybe we still should do it for manage nodegroups? Let me try add this logic?

@eladb
Copy link
Contributor

eladb commented Jul 5, 2020

@eladb

I believe this is because we don't do the AwsAuth for managed ng and let the eks control plane do it instead. So the state will be inconsistent.

// As managed nodegroup will auto map the instance role to RBAC behind the scene and users don't have to manually
// do it anymore. We don't need to print out the instance role arn now.

Maybe we still should do it for manage nodegroups? Let me try add this logic?

I see. Yes. I think we must also do it for Managed Node Groups or otherwise we will lose track of the state.

@pahud
Copy link
Contributor

pahud commented Jul 5, 2020

@eladb OK. Let me create the PR.

@mergify mergify bot closed this as completed in #8901 Jul 6, 2020
mergify bot pushed a commit that referenced this issue Jul 6, 2020
…terRole (#8901)

fix(eks): missing nodegroup identity in aws-auth after awsAuth.addMasterRole

This PR adds the state tracking by `awsAuth.addRoleMapping` for the managed nodegroups

Fixed: #7595 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@iliapolo iliapolo added this to the EKS Dev Preview milestone Aug 10, 2020
@iliapolo iliapolo changed the title [EKS Bug] wrong order of cluster.awsAuth.addMastersRole & cluster.addNodegroup breaks awsAuth [aws-eks] wrong order of cluster.awsAuth.addMastersRole & cluster.addNodegroup breaks awsAuth Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service bug This issue is a bug. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants