Open cobbr2 opened 8 months ago
Voting for Prioritization
Volunteering to Work on This Issue
The documentation has already been updated in respect to data "aws_msk_bootstrap_brokers" "example" {
.
The fixed documentation is available here
The relevant commit is this one
Concerning
All of the attribute descriptions that say "One or more DNS Names..." are false. I don't know what they return, but those attributes must allow for the fact that any of those lists can be empty; perhaps they return NULL in those cases, but that should be documented. Those lists should only be populated if the cluster actually supports that particular authentication method.
there is additional documentation on the resource, for example
One or more DNS names (or IP addresses) and SASL IAM port pairs. [...] This attribute will have a value if encryption_info.0.encryption_in_transit.0.client_broker is set to TLS_PLAINTEXT or TLS and client_authentication.0.sasl.0.iam is set to true.
Would it be already helpful to add the "This attribute will..." information, so that it is clear that not all fields will be populated?
Documentation Link
https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/msk_bootstrap_brokers
Description
The example should read
NOT:
data "aws_msk_cluster" "example" {
All of the attribute descriptions that say "One or more DNS Names..." are false. I don't know what they return, but those attributes must allow for the fact that any of those lists can be empty; perhaps they return NULL in those cases, but that should be documented. Those lists should only be populated if the cluster actually supports that particular authentication method.
For serverless clusters, the only attributes that could have one or more entries would be bootstrap_brokers_sasl_iam , since that's the only type supported.
For provisioned clusters, any of them could be empty.
Furthermore:
bootstrap_brokers
attribute implies that you will always get a list, without you having to specify what authentication system you want to use. This is definitely false for serverless clusters. At present, you get an empty string for serverless.bootstrap_brokers_sasl_iam
attribute says it will return a list. In the case of MSK serverless, it returns a string. My guess is that all of these entries return a string with commas separating the values; that's not a "list" in terraform / HCL terms AFAIK. For example, we can't callsort(...)
directly on it.And calling
sort
is required for the provisioned case -- the API will return the values in a randomized order which changes on every call. That means thedata
value is constantly changing, so will trigger spurious updates unless you sort the values. I think that's a minor bug in the data source code; I'd have it sort (and return real lists), though that will now have compatibility problems.References
No response
Would you like to implement a fix?
None