feat(plugins): add Kafka driver plugin - #2437
Merged
Merged
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a Kafka driver plugin: topics in the sidebar, messages in the data grid, consumer group
lag, and producing a message. Registry-only, pure Swift, no new static library.
Fixes #2419.
What it does
A topic is a table and a message is a row, with columns
partition,offset,timestamp,key,value,headers,key_size,value_size. Opening a topic shows its most recenthundred messages, which is what a Kafka debugging session usually wants and what every other
Kafka UI defaults to.
The query editor takes a small command language:
DESCRIBE GROUPis the lag report: committed offset, end offset and the gap, per partition.MCP needed no work. Every tool in
TablePro/Core/MCP/goes through the generic driverabstraction (a grep for
DatabaseType.there returns nothing), solist_tables,browse_table,describe_tableandexecute_queryall reach Kafka as soon as the driverexists. That was the other half of the request.
Why pure Swift rather than librdkafka
Both vendoring routes were checked rather than assumed:
swift-kafka-clientdeclaresplatforms: [.macOS(.v15)]and TablePro targets macOS 14, soit cannot be linked at all. It also needs a
.systemLibraryOpenSSL andlinkedLibrary("sasl2"),which is not shippable in a notarized universal
.tableplugin.librdkafka.ainLibs/needsscripts/publish-libs.sh, a gated operation.Kafka's protocol is length-prefixed binary over TCP and needs no C library. Transport is
SwiftNIO + NIOSSL, already in the dependency graph through
Packages/TableProOracle, andNIOSSL reads the PEM paths
SSLConfigurationalready carries. zstd comes fromfacebook/zstd's ownPackage.swift(BSD, macOS 10.10+, builds from source), because macOSsupplies no zstd at any layer.
Verified against a real broker, not against the spec
The wire format was measured against Apache Kafka 4.3.1 in Docker before any of it was
written down. Four encoding decisions are not derivable from the surrounding format, and each
fails the same way when wrong: the broker consumes what it can and closes the socket with no
error response and nothing in any log.
apiKey == 18). Parsing one shifted a real broker's reply from error 0 to error 76.0x00, not the legacy0xff.0xffsets the varint continuation bit and the broker eats the bytes that follow.include_cluster_authorized_operations, so v12 has two trailing booleans, not three.scripts/check-kafka-protocol.shis committed so a Kafka version bump re-checks all of thisinstead of trusting a transcription, the same shape as
scripts/check-redis-command-routing.sh.It runs 16 checks against a live broker, including producing a batch whose CRC-32C the broker
accepts, which is what proves the hand-rolled Castagnoli table.
Compression was measured too: the broker returns exactly what the producer stored and never
transcodes. Magic bytes confirmed gzip
1f8b, xerial-framed snappy\x82SNAPPY, LZ4frame
04224d18, and zstd28b52ffd, so all four are decoded client-side.What the grid deliberately will not do
Kafka has no update primitive, no per-message delete and no server-side sort, so those are
disabled rather than faked. Topics are typed
external table, whoseallowsRowEditingisfalse, so the grid refuses a cell edit up front with a real message instead of accepting oneand failing at save.
generateStatementsalso returns nil, because returning nil alone makesSave silently do nothing.
Paging is anchored:
buildBrowseQuerybakes the resolved per-partition offsets into thestatement, so page two continues page one instead of re-deriving "newest" against a tail that
has moved. Fetches use
read_committedand drop control batches, or a transactional topicwould show commit markers as rows.
Behavioural change outside the plugin
DatabaseManager+Tunnel.swiftgains a Kafka arm. Kafka's Metadata reply names brokers bytheir advertised address and clients are expected to dial those directly, but behind a
tunnel there is one forwarded port, so
broker-2.internal:9092is unreachable and the fetchhangs. This is the same pin MongoDB (
directConnection) and Redis (standalone) already getthere, for the same reason. It was found by hitting it against a real broker in Docker.
DatabaseCategorygains astreamingcase. Kafka is an event streaming platform and thenearest existing category was
coordination, which it is not.What review caught
Two independent reviews ran over this (a security-focused pass and a general one). Both found
real defects I had written, and both are fixed here rather than deferred.
Authentication bypass. SCRAM's server-signature check was skipped entirely when the
server-final message was not valid UTF-8: that branch
returned, and a normal return meant"authenticated". A hostile broker could answer any challenge with a single
0xFFbyte and bereported as connected, having never known the password. It now throws like every other path.
Attacker-chosen key-derivation cost. The SCRAM iteration count was accepted down to 1, so
a broker chose how cheaply an intercepted proof could be cracked offline. RFC 7677's floor of
4096 is now enforced.
Passwords containing
,or=could never authenticate. RFC 5802'ssaslnameescapingwas being applied to the password as well as the username. It belongs to the username alone;
the password is a raw PBKDF2 input. A Confluent-style API secret ending in
=would havefailed with correct credentials.
A hang on the default browse path.
boundsissued its twoListOffsetscalls withasync let, and both landed on the same connection. Actors are re-entrant, so the secondrequest overwrote the first's continuation: the first never resumed and the tab spun forever.
The calls are sequential now, and
expectrefuses a second registration instead of droppingone, so an overlap is a named error rather than a hang.
Paging drifted.
buildBrowseQuerynever emitted theANCHORclause the design calledfor, so page two re-derived
NEWESTagainst a moved tail and repeated rows from page one.The resolved anchor is now recorded and named explicitly on every later page, which is what
the
ANCHORgrammar existed for.A crash from the query editor.
SKIPwas an unboundedInt:SKIP -1trapped indropFirstand a huge value overflowed theskip + limitaddition. Both ends are bounded now.Also fixed:
ListOffsetsread uncommitted whileFetchread committed, so an opentransaction made every page escalate its byte budget through four round trips and report a
false truncation; aborted-transaction filtering keyed on offset alone, so one producer's
commit marker released another producer's aborted run and leaked rolled-back rows; the batch
decoder re-sliced the whole remaining blob per batch, making it quadratic in the fetch size;
reserveCapacitytrusted a wire-supplied record count; the zstd path trusted a declared sizefor both its
Intconversion and its allocation; LZ4 sized its destination at 255x the blockinstead of the frame's own declared maximum, zero-filling 16 MB per 64 KB block; and the
event loop group was created in
initbut released only bydisconnect(), which a failedconnect never reaches, so every rejected credential leaked a thread.
Security
Security Protocolnow decides whether the connection is encrypted and the SSL mode decideshow strictly the certificate is checked. Without that, choosing
SASL_SSLwhile the SSL modesat at its default of Disabled opened a plaintext socket and sent the SASL password over it.
A TLS protocol with no mode chosen now verifies fully, and a non-TLS protocol never quietly
encrypts because a stale mode was left behind.
The TLS mode mapping matches
OracleTLSMapperexactly, and.preferred/.requiredmappingto no verification is the convention every other driver here follows. The driver no longer
retains the
DriverConnectionConfigafter init, because it carries the password and nothingreads it again.
Testing
Unit tests across 8 suites: golden-byte codec tests for every framing decision above, record
batch decoding (multi-batch blobs, partial trailing batches, control batches, per-producer
aborted runs, null vs empty payloads, an encode/decode round trip with a verified CRC), all
four compression codecs, the KafkaQL parser including the bounded
SKIPand the quote roundtrip, the flattener's column typing and its arithmetic timestamp formatter cross-checked
against Foundation, merge ordering, the SSL/SASL resolution, and SCRAM against RFC 7677's
published vectors.
One of those tests caught a real bug before either reviewer saw it: the gzip loop treated
"input exhausted" as success, so a truncated stream returned partial output that looked whole.
Fixed in the source, not the test.
No UI automation. The plugin adds no new UI: it renders through the existing grid, sidebar and
query editor, and the flows that would be worth driving all need a live broker, which CI has
no way to provide.
Checks
verify.sh buildverify.sh test(8 Kafka suites)verify.sh docsswiftlint --strict(plugin, app and test files)KafkaDriverPluginschemecheck-plugin-manifest.pycheck-kafka-protocol.shagainst Kafka 4.3.1verify.sh plugins(AllPlugins)The
AllPluginsfailure is the Oracle plugin's@TaskLocalmacro on this local toolchain(
unknown attribute 'usableFromInlinenonisolated'), which reproduces without this branch.There are zero Kafka errors in that log and the
KafkaDriverPluginscheme builds on its own.One thing worth flagging for review rather than fixing here: with
SASL_PLAINTEXTselected,the SSL pane still shows whatever mode was last chosen while the connection is deliberately
unencrypted. The behaviour is right (Kafka's listener protocol is the authority) but the pane
is misleading, and
ConnectionSSLViewalready has the warning-label pattern for it.Not in this change
Schema Registry (Avro and Protobuf decoding), ACL management, Kafka Connect, ksqlDB, topic
creation and deletion, topic config editing, consumer group offset reset,
DeleteRecords,tombstone delete, live tail, AWS MSK IAM, and OAUTHBEARER.
Schema Registry is the one worth calling out: on a cluster whose values are Avro, the value
column shows raw bytes including the five-byte Confluent prefix. The design is small (a
registry URL on the connection, a codec keyed off that prefix) and it is the obvious
fast-follow rather than a peer of the others on that list.