ockam
ockam copied to clipboard
feat(rust): add support for additional kafka addons
Current behavior
The current support for Apache Kafka is configured via an add-on named confluent.
Proposed changes
I've been testing support for other Apache Kafka compatible brokers and many of them work, but it's a bit weird to configure them by configuring confluent when you're actually using a completely different product. These changes are adding the following subcommands:
ockam project addon configure aiven-kafkaockam project addon configure instaclustrockam project addon configure redpandaockam project addon configure kafka
There's far more code duplication than I'd like in here (see the configure_(kafka|confluent|aiven|instaclustr|redpanda).rs files and the equivalent files in static). Making generics work with clap was beyond my Rust expertise and sending me down a whole rabbit hole that I suspect more experienced people would be able to quickly refactor or educate me.
Thanks to Davide I've managed to make more of this re-usable across the various kafka-compatible options. Would love some 👀 for feedback, improvements, or approval.
I think we could reduce the code by just having one KafkaCommand and an enum for the vendor type:
Just following up on this specific point: the decision was made to make each add-on it's own subcommand so that peers all get similar treatment (i.e., to avoid having influxdb be its own subcommand while all the kafka ones get relegated to a flag on a generic command)
Ok, I think I've taken this one about as far as I can. Also my ~5% time towards dev stuff isn't enough to keep this in sync with a fast moving develop branch, so the sooner I can get some help to get this up to standard + merged the better.
I've tried to make all the various kafka-generic bits reusable and put on vendor-specific branding over the subcommands. There's a couple of linter warnings I don't actually understand how to resolve though.
@adrianbenavides could you take over landing this please, so we can release it with our next release.
One thing I should explicitly call out: in the interests of converging on consisting kafka naming for the generic bits, I also renamed some sql-related bits that have been added since I originally started this PR. I'm not sure if that means we need to consider a migration to rename that table for existing deployments? Or maybe reverting that part of the patch.
I've refactored a little the configure_kafka.rs file to extract the addons subcommnds (confluent, redpanda, ...) into separate files. I've also added a new migration to rename the confluent_config table. It's ready to review now!