Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws_msk_cluster : default value for client_broker : "TLS" #10673

Closed
kunsalvi opened this issue Oct 30, 2019 · 6 comments · Fixed by #14132
Closed

aws_msk_cluster : default value for client_broker : "TLS" #10673

kunsalvi opened this issue Oct 30, 2019 · 6 comments · Fixed by #14132
Assignees
Labels
service/kafka Issues and PRs that pertain to the kafka service. upstream Addresses functionality related to the cloud provider.
Milestone

Comments

@kunsalvi
Copy link

Hi

I am creating MSK cluster and if i do not keep any encryption_in_transit. It picks up "TLS" value for client_broker and cluster is created. But if I do terrform apply (without making any changes.). It forces below change :
encryption_info.0.encryption_in_transit.0.client_broker: "TLS" => "TLS_PLAINTEXT" (forces new resource)

Moreover if I provide value as :
encryption_in_transit {
client_broker = "TLS"
in_cluster = true
}

Cluster creation fails with :
Error: Error refreshing state: 1 error occurred:
* aws_msk_cluster.atropos_prod: 1 error occurred:
* aws_msk_cluster.atropos_prod: aws_msk_cluster.XXXX: failed requesting bootstrap broker info for "arn:aws:kafka:ap-south-1:286817589435:cluster/XXXX/684bd0d9-eecf-454d-b209-306ead63dda6-4" : BadRequestException: You can't get bootstrap broker nodes for a cluster in FAILED state.
status code: 400, request id: 81a984a9-2646-46e7-86af-236c0a88b329

Terrform version : Terraform v0.11.14

@ghost ghost added the service/kafka Issues and PRs that pertain to the kafka service. label Oct 30, 2019
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Oct 30, 2019
@bflad bflad added upstream Addresses functionality related to the cloud provider. and removed needs-triage Waiting for first response or review from a maintainer. labels Oct 30, 2019
@bflad bflad added this to the v3.0.0 milestone Oct 30, 2019
@bflad
Copy link
Contributor

bflad commented Oct 30, 2019

Thanks for submitting this, @kunsalvi.

A few weeks after general availability launch, the API default for encryption switched from TLS_PLAINTEXT to TLS. We unfortunately cannot update the Terraform default to match the updated API default until a major version update of the Terraform AWS Provider as it would represent a potentially breaking change for existing Terraform configurations. I have marked this in the 3.0.0 milestone for now to ensure this change is included in upcoming planning for that major version.

Regarding the BadRequestException you're seeing, it sounds like you may want to check in with AWS Support on why the cluster is entering a FAILED state -- the maintainers here most likely would not be able to help directly with that unless the MSK team can pinpoint that the Terraform resource is doing something incorrectly.

@kunsalvi
Copy link
Author

kunsalvi commented Nov 4, 2019

@bflad

Thank you for the response. For now, I am explicitly specifying "client_broker" and its working for me.

And "BadRequestException" is due to my certificate issue.

@wvidana
Copy link

wvidana commented Apr 28, 2020

This is even worst now.

If not specified it forces a new resource since now it wants to be null

          - encryption_in_transit {
              - client_broker = "TLS" -> null # forces replacement
              - in_cluster    = true -> null
            }

@bflad bflad self-assigned this Jul 9, 2020
bflad added a commit that referenced this issue Jul 10, 2020
…t.client_broker default to match API default

Reference: #10673

Output from acceptance testing:

```
--- PASS: TestAccAWSMskCluster_basic (1595.73s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (1760.04s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1603.40s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1603.85s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1598.02s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1603.10s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (1806.72s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (2036.98s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (1722.68s)
--- PASS: TestAccAWSMskCluster_Tags (1605.63s)

--- PASS: TestAccAWSMskClusterDataSource_Name (1608.30s)
```
bflad added a commit that referenced this issue Jul 14, 2020
…t.client_broker default to match API default

Reference: #10673

Output from acceptance testing:

```
--- PASS: TestAccAWSMskCluster_basic (1595.73s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (1760.04s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1603.40s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1603.85s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1598.02s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1603.10s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (1806.72s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (2036.98s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (1722.68s)
--- PASS: TestAccAWSMskCluster_Tags (1605.63s)

--- PASS: TestAccAWSMskClusterDataSource_Name (1608.30s)
```
bflad added a commit that referenced this issue Jul 14, 2020
…t.client_broker default to match API default (#14132)

* resource/aws_msk_cluster: Update encryption_info.encryption_in_transit.client_broker default to match API default

Reference: #10673

Output from acceptance testing:

```
--- PASS: TestAccAWSMskCluster_basic (1595.73s)
--- PASS: TestAccAWSMskCluster_BrokerNodeGroupInfo_EbsVolumeSize (1760.04s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionAtRestKmsKeyArn (1603.40s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_ClientBroker (1603.85s)
--- PASS: TestAccAWSMskCluster_EncryptionInfo_EncryptionInTransit_InCluster (1598.02s)
--- PASS: TestAccAWSMskCluster_EnhancedMonitoring (1603.10s)
--- PASS: TestAccAWSMskCluster_LoggingInfo (1806.72s)
--- PASS: TestAccAWSMskCluster_NumberOfBrokerNodes (2036.98s)
--- PASS: TestAccAWSMskCluster_OpenMonitoring (1722.68s)
--- PASS: TestAccAWSMskCluster_Tags (1605.63s)

--- PASS: TestAccAWSMskClusterDataSource_Name (1608.30s)
```

* tests/data-source/aws_msk_cluster: Further clarify why bootstrap_brokers_tls is not using TestCheckResourceAttrPair()
@bflad
Copy link
Contributor

bflad commented Jul 14, 2020

The attribute default update has been merged and will release with version 3.0.0 of the Terraform AWS Provider, likely in the next two weeks. 👍

@ghost
Copy link

ghost commented Jul 31, 2020

This has been released in version 3.0.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Aug 14, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Aug 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
service/kafka Issues and PRs that pertain to the kafka service. upstream Addresses functionality related to the cloud provider.
Projects
None yet
3 participants