Fix Broker Hostname Records

[ghost]

what

• Replaced the way the broker hostname records are created.
• Also fixed a typo in the bootstrap_brokers_tls output.

why

Because right now the bootstrap brokers are used to build the hostnames. AWS doesn't give a complete list of brokers in the bootstrap brokers list (see references). So if you have 4 brokers but 3 reported bootstrap brokers then the hostnames for the 4 DNS records are wrong. Also the bootstrap brokers change between refreshes, so the hostnames change with every apply.

references

• https://docs.aws.amazon.com/msk/1.0/apireference/clusters-clusterarn-bootstrap-brokers.html
• https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/msk_cluster#bootstrap_brokers

[mergify]

This pull request is now in conflict. Could you fix it @stephanECD? 🙏

[nitrocode]

@stephanECD thanks for the contribution! I'll run the tests

[nitrocode]

/test all

[nitrocode]

/test all

[nitrocode]

/test all

[nitrocode]

/test all

[nitrocode]

@stephanECD seems like it cannot retrieve the endpoints immediately after creating the cluster. A delay may be needed after cluster creation and before the data source pulls the broker information. This is most likely due to the AWS api not returning consistent results.

were you tested this locally to see if this works as expected?

[ghost]

@nitrocode thanks for looking into this. I don't think the AWS API is the root cause. I was able to fix this error:

│   on .../terraform-aws-msk-apache-kafka-cluster/main.tf line 5, in locals:
│    5:   brokers        = local.enabled ? flatten(local.node_info_list.*.endpoints) : []
│ 
│ This value does not have any attributes.

by simply going back to one line in the code for the brokers. So I would guess your local.node_info_list doesn't look as expected at the time this code runs, maybe there's a race condition when the local.node_info_list is filled, as opposed to when its content is needed for the dns records. Have a look at my newest commit, I think that should solve it. We don't need a splash for data.aws_msk_broker_nodes, since you only use the count to enable or disable it.

[nitrocode]

@stephanECD cool. Were you able to test out the change ?

[nitrocode]

/test all

[ghost]

Reverted the hostname count, seems that was a bad idea.

[nitrocode]

/test all

[nitrocode]

Thanks @stephanECD for the contribution!