Skip to content

improvement: Generate aws-auth ConfigMap's roles from Object. No more string concat.#790

Merged
barryib merged 2 commits into
terraform-aws-modules:masterfrom
dpiddock:aws-auth
Mar 18, 2020
Merged

improvement: Generate aws-auth ConfigMap's roles from Object. No more string concat.#790
barryib merged 2 commits into
terraform-aws-modules:masterfrom
dpiddock:aws-auth

Conversation

@dpiddockcmp

@dpiddockcmp dpiddockcmp commented Mar 13, 2020

Copy link
Copy Markdown
Contributor

PR o'clock

Description

The current module generates the roles item of the aws-auth ConfigMap using string concatenation. Building a data structure from concat is often fraught with troubles. We can see that with the edge case of having no roles defined generating invalid YAML.

This change uses Terraform primitive data types and then a final yamlencode() to create the roles.

Fixes #689

This shouldn't be a breaking change although it will cause an update to the aws-auth ConfigMap to remove unnecessary blank lines.

Checklist

Changes tested

No roles

module "eks" {
  source       = "../.."
  cluster_name = local.cluster_name
  subnets      = module.vpc.private_subnets
  vpc_id       = module.vpc.vpc_id
}

Previous:

apiVersion: v1
data:
  mapAccounts: |
    []
  mapRoles: |2+


  mapUsers: |
    []
kind: ConfigMap
metadata:
  name: aws-auth
  namespace: kube-system

New:

apiVersion: v1
data:
  mapAccounts: |
    []
  mapRoles: |
    []
  mapUsers: |
    []
kind: ConfigMap
metadata:
  name: aws-auth
  namespace: kube-system

examples/basic

apiVersion: v1
data:
  mapAccounts: |
    - "777777777777"
    - "888888888888"
  mapRoles: |
    - "groups":
      - "system:bootstrappers"
      - "system:nodes"
      "rolearn": "arn:aws:iam::XXXXXXXXXXXX:role/test-eks-yIaQqH4020200313151442206300000008"
      "username": "system:node:{{EC2PrivateDNSName}}"
    - "groups":
      - "system:masters"
      "rolearn": "arn:aws:iam::66666666666:role/role1"
      "username": "role1"
  mapUsers: |
    - "groups":
      - "system:masters"
      "userarn": "arn:aws:iam::66666666666:user/user1"
      "username": "user1"
    - "groups":
      - "system:masters"
      "userarn": "arn:aws:iam::66666666666:user/user2"
      "username": "user2"
kind: ConfigMap
metadata:
  name: aws-auth
  namespace: kube-system

Windows works too:

apiVersion: v1
data:
  mapAccounts: |
    []
  mapRoles: |
    - "groups":
      - "system:bootstrappers"
      - "system:nodes"
      - "eks:kube-proxy-windows"
      "rolearn": "arn:aws:iam::XXXXXXXXXXXX:role/test-eks-yIaQqH4020200313151442206300000008"
      "username": "system:node:{{EC2PrivateDNSName}}"
  mapUsers: |
    []
kind: ConfigMap
metadata:
  name: aws-auth
  namespace: kube-system

@barryib barryib changed the title Generate aws-auth roles from object improvement: Generate aws-auth ConfigMap's roles from Object. No more string concat. Mar 17, 2020
Comment thread CHANGELOG.md Outdated
@barryib

barryib commented Mar 17, 2020

Copy link
Copy Markdown
Member

Awesome as usual @dpiddockcmp. Thanks a lot for this improvement.

Do not use string concat to generate a YAML data structure
@barryib barryib merged commit 3957a7c into terraform-aws-modules:master Mar 18, 2020
@dpiddockcmp dpiddockcmp deleted the aws-auth branch March 18, 2020 09:28
@github-actions

Copy link
Copy Markdown

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 19, 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.

aws-auth syntax error when no worker_groups defined

3 participants