remove empty [] to mapRoles object in aws-auth

[shanxops]

Simply having ${yamlencode(var.map_roles)} in mapRoles for aws-auth creates a empty [] at the end after adding the default roles.
Changing it to be added only when its not empty

PR o'clock

Description

Please explain the changes you made here and link to any relevant issues.

Checklist

• Change added to CHANGELOG.md. All changes must be added and breaking changes and highlighted
• CI tests are passing
• README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

[max-rocket-internet]

Are you really sure having [] is the problem? It's valid YAML.

Changing it to be added only when its not empty

It doesn't look like that's what happening here as @dpiddockcmp mentioned.

Checklist

👀👀👀👀

[shanxops]

@max-rocket-internet

Are you really sure having [] is the problem? It's valid YAML.

Yes it is, by itself. The issue here is mapRoles object is created as below in the template

${join("", distinct(concat(data.template_file.launch_template_worker_role_arns.*.rendered, data.template_file.worker_role_arns.*.rendered)))}
${yamlencode(var.map_roles)}

which actually gets created as below, breaking the aws-auth when var.map_roles = []

mapRoles: |
    - groups:
      - system:bootstrappers
      - system:nodes
      rolearn: arn:aws:iam::accountid:role/rolename
      username: system:node:{{EC2PrivateDNSName}}
      []

[max-rocket-internet]

Thanks @shanmugakarna 💜

[max-rocket-internet]

You haven't update changelog! Very important! But in the interesting of getting stuff done, I'll merge now and update it myself

[shanxops]

@max-rocket-internet @dpiddockcmp
In fact, my earlier commit was correct its == rather than !=. Seems no logic right, let me explain
A small terraform code, to test this

variable "empty_list_with_simple_type" {
  default = []
}

output "empty_list_with_simple_type" {
  value = <<EOF

Original
======
${yamlencode(var.empty_list_with_simple_type)}

Condition: ==
==========
%{if var.empty_list_with_simple_type == []}${yamlencode(var.empty_list_with_simple_type)}%{endif}

Condition: !=
=========
%{if var.empty_list_with_simple_type != []}${yamlencode(var.empty_list_with_simple_type)}%{endif}

EOF
}

variable "empty_list_with_complex_type" {
  type = list(object({
    rolearn  = string
    username = string
    groups   = list(string)
  }))
  default = []
}

output "empty_list_with_complex_type" {
  value = <<EOF

Original
======
${yamlencode(var.empty_list_with_complex_type)}

Condition: ==
==========
%{if var.empty_list_with_complex_type == []}${yamlencode(var.empty_list_with_complex_type)}%{endif}

Condition: !=
=========
%{if var.empty_list_with_complex_type != []}${yamlencode(var.empty_list_with_complex_type)}%{endif}

EOF
}

Output

empty_list_with_complex_type =
Original
======
[]

Condition: ==
==========

Condition: !=
=========
[]

empty_list_with_simple_type =
Original
======
[]

Condition: ==
==========
[]

Condition: !=
=========

See the behaviour here, a complex data type behaves differently inside and outside an if block, while a simple data type behaves as expected
So, we still need == until terraform handles this properly

I think, we need to revert to ==, please review

[dpiddockcmp]

You need to test the other use case of having something in the map:

variable "empty_list_with_complex_type" {
  type = list(object({
    rolearn  = string
    username = string
    groups   = list(string)
  }))
  default = [{
    rolearn  = "arn:blah:blah"
    username = "user"
    groups   = ["group"]
  }]
}

output "empty_list_with_complex_type" {
  value = <<EOF

Original
======
${yamlencode(var.empty_list_with_complex_type)}

Condition: ==
==========
%{if var.empty_list_with_complex_type == []}${yamlencode(var.empty_list_with_complex_type)}%{endif}

Condition: !=
=========
%{if var.empty_list_with_complex_type != []}${yamlencode(var.empty_list_with_complex_type)}%{endif}

EOF
}

Outputs:

empty_list_with_complex_type = 
Original
======
- "groups":
  - "group"
  "rolearn": "arn:blah:blah"
  "username": "user"


Condition: ==
==========


Condition: !=
=========
- "groups":
  - "group"
  "rolearn": "arn:blah:blah"
  "username": "user"

So Terraform is broken on this if type and complex Objects. Depending on comparison used you either always match or never match.

Your usecase cannot currently be solved with this if block. And #611 will wipe out auth maps.

[TBeijen]

Confirmed. This breaks map_roles and attempts to remove existing entries from aws-auth.

Couldn't this work instead? Make the default value for map_roles null, and explictly check for 'not null'?

Or, render the yaml fragment in a local, and only include the output if it's not []?

[github-actions]

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.