Mongey / terraform-provider-kafka

Terraform provider for managing Apache Kafka Topics + ACLs
MIT License
520 stars 132 forks source link

fix(fail_on): Added fail_on list to enable failing plan on condition such as decreasing partition count #432

Open dbmurphy opened 4 months ago

dbmurphy commented 4 months ago

When using the provider, we found we wanted to fail plans if the partition count was reduced rather than trigger ForceNew. To support this but let others choose to replace the resource, we added fail_on, a list of string conditions. The code can check for these to override behaviors, which you can pass in when starting the provider to feature flag this change.

dbmurphy commented 4 months ago

@Mongey, Is there anything you need here, or do I need to tag someone as a reviewer?

dbmurphy commented 4 months ago

Not sure whats up with your lint/testacc mine is very different. And obviously I can't fix the lint issue for upstream.

➜  terraform-provider-kafka git:(topic_place_fail_plan) ✗ docker ps | grep kafka
4c1805fdb50d   terraform-provider-kafka-kafka1    "/etc/confluent/dock…"   42 seconds ago   Up 40 seconds   0.0.0.0:9092->9092/tcp, :::9092->9092/tcp                       terraform-provider-kafka-kafka1-1
0d089ddd5b44   terraform-provider-kafka-kafka3    "/etc/confluent/dock…"   42 seconds ago   Up 40 seconds   0.0.0.0:9094->9092/tcp, :::9094->9092/tcp                       terraform-provider-kafka-kafka3-1
ac4059e316e0   terraform-provider-kafka-kafka2    "/etc/confluent/dock…"   42 seconds ago   Up 40 seconds   0.0.0.0:9093->9092/tcp, :::9093->9092/tcp                       terraform-provider-kafka-kafka2-1
a17d96bfc3bb   confluentinc/cp-zookeeper:latest   "/etc/confluent/dock…"   42 seconds ago   Up 41 seconds   2888/tcp, 0.0.0.0:2181->2181/tcp, :::2181->2181/tcp, 3888/tcp   terraform-provider-kafka-zookeeper-1                                             /0.0s
➜  terraform-provider-kafka git:(topic_place_fail_plan) ✗ go mod tidy ; go mod verify; go mod vendor; golangci-lint run; make testacc | tail -n 10
all modules verified
../../../../.goenv/versions/1.21.6/src/net/http/internal/chunked.go:79:14: undefined: max (typecheck)
        cr.excess = max(cr.excess, 0)
                    ^
2024/07/17 11:15:34 [DEBUG] deletetopic done! syslog-e8dfe78b-b5ce-cea0-6e5c-50d4d66d3b68
2024-07-17T11:15:34.256-0500 [DEBUG] sdk.helper_resource: Stopping providers: test_terraform_path=/usr/local/bin/terraform test_working_directory=/var/folders/8d/wmy8f0kj7b3_my72ymy8mf600000gp/T/plugintest3481534103 test_step_number=4 test_name=TestAcc_TopicAlterReplicationFactor
2024-07-17T11:15:34.256-0500 [DEBUG] sdk.helper_resource: Calling TestCase CheckDestroy: test_working_directory=/var/folders/8d/wmy8f0kj7b3_my72ymy8mf600000gp/T/plugintest3481534103 test_step_number=4 test_name=TestAcc_TopicAlterReplicationFactor test_terraform_path=/usr/local/bin/terraform
2024/07/17 11:15:34 [INFO] 👋 reading topic 'syslog-e8dfe78b-b5ce-cea0-6e5c-50d4d66d3b68' from Kafka: true
2024/07/17 11:15:34 [DEBUG] Refreshing metadata for topic 'syslog-e8dfe78b-b5ce-cea0-6e5c-50d4d66d3b68'
2024-07-17T11:15:35.029-0500 [DEBUG] sdk.helper_resource: Called TestCase CheckDestroy: test_step_number=4 test_name=TestAcc_TopicAlterReplicationFactor test_terraform_path=/usr/local/bin/terraform test_working_directory=/var/folders/8d/wmy8f0kj7b3_my72ymy8mf600000gp/T/plugintest3481534103
2024-07-17T11:15:35.031-0500 [DEBUG] sdk.helper_resource: Finished TestCase: test_name=TestAcc_TopicAlterReplicationFactor
--- PASS: TestAcc_TopicAlterReplicationFactor (70.51s)
PASS
ok      github.com/Mongey/terraform-provider-kafka/kafka        72.533s      
dbmurphy commented 4 months ago

I would like some help here. Should I not do an ACC test the way I am doing it to test the provider? I think this sometimes, but not always, throws off the testing, as it gets confused about which provider to use.

Mongey commented 3 months ago

@dbmurphy I'll try locally. I've not really sure what to make of the feature though. Would this not be better as some-other-tool inspecting the plan, and seeing that a topic wasn't going to be recreated, and, using that to fail a CI build etc ?

dbmurphy commented 3 months ago

I don't think so it's more akin to termination protection with AWS resources that would reject plans. however as Kafka lacks support for this the way the aws api does this simulated the same pattern I could move it to the resource level and call its termination protection but I was thinking to make a generic provider level list we could set for future use case if people wanted to prevent user or acl changes in the same termination protection way.