terraform-aws-modules / terraform-aws-msk-kafka-cluster

Terraform module to create AWS MSK (Managed Streaming for Kafka) resources πŸ‡ΊπŸ‡¦
https://registry.terraform.io/modules/terraform-aws-modules/msk-kafka-cluster/aws
Apache License 2.0
55 stars 53 forks source link

feat: Add support for `client_authentication.unauthenticated` and `ebs_storage_info.provisioned_throughput` #4

Closed aidanmelen closed 1 year ago

aidanmelen commented 2 years ago

Description

Add support for the MSK storage_info block. This includes the new provisioned volume throughput feature.

Using the new storage_info block did require setting the minimum version for the aws provider to be >= 4.15.0. Here are the release notes for the aws provider.


Add support for unauthenticated TLS connections. Added new variable client_unauthenticated_access_enabled with a default value set to false for backwards comparability.

Motivation and Context

This PR fixes this issue: #3

How Has This Been Tested?

The basic example runs without any modifications. The broker_node_ebs_volume_size variable now gets passed to the broker_node_ebs_volume_size.storage_info.ebs_storage_info.volume_size resource block rather than the deprecated broker_node_ebs_volume_size.ebs_volume_size argument.

I also update the complete example to have volume throughput configured. Running a plan showed that the module variables were correctly passed the storage_info the MSK resource e.g.

     + broker_node_group_info {
          + az_distribution = "DEFAULT"
          + client_subnets  = (known after apply)
          + ebs_volume_size = (known after apply)
          + instance_type   = "kafka.m5.4xlarge"
          + security_groups = (known after apply)

          + connectivity_info {
              + public_access {
                  + type = (known after apply)
                }
            }

          + storage_info {
              + ebs_storage_info {
                  + volume_size = 1000

                  + provisioned_throughput {
                      + enabled           = true
                      + volume_throughput = 250
                    }
                }
            }
        }

Screenshots (if appropriate):

aidanmelen commented 2 years ago

What's more this change is NONE BREAKING for existing deployments that are using the deprecated broker_node_ebs_volume_size.ebs_volume_size.

The resulting plan after switching to the new broker_node_ebs_volume_size.ebs_volume_size results in the expected addition of the and setting provisioned_throughput.enable = false e.g.

~ storage_info {
    ~ ebs_strage_info {
          # (1 unchanged attribute hidden)

      + provisioned_throughput {
          + enabled = false
         }
     }
     # (1 unchanged block hidden)
}   
aidanmelen commented 2 years ago

@bryantbiggs is there anything else you would like me to test?

frank-bee commented 1 year ago

@bryantbiggs any plans to merge this PR?

tanvp112 commented 1 year ago

Hi @bryantbiggs, client_unauthenticated_access_enabled is a handy feature for sandbox cluster. Can this PR be merged? Happy to give a hand if needed.

vishwa-trulioo commented 1 year ago

Any chance this will get merged?

bryantbiggs commented 1 year ago

yes, apologies - I will carve out some time to review and make some additional changes and get this moved over to https://github.com/terraform-aws-modules

bryantbiggs commented 1 year ago

apologies for the delay ... life and stuff πŸ€·πŸ½β€β™‚οΈ ... but we're pushing the updates now and then moving it over to terraform-aws-modules where it will live out the rest of its days. The current versions on the registry under clowdhaus will stay indefinitely so no need to upgrade until you are ready

bryantbiggs commented 1 year ago

:tada: This issue has been resolved in version 2.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

github-actions[bot] commented 1 year ago

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.