david-streamlio commented on PR #98:
URL: https://github.com/apache/pulsar-connectors/pull/98#issuecomment-4938358601
Two follow-ups on this PR.
**1. Fixed the CI failure.** Apache Rat flagged the new
`instance.properties` as an unapproved license — it was missing the ASF header.
Added it (rat passes locally now) and pushed.
**2. The protobuf pin needs a maintainer decision — flagging rather than
merging as-is.** I dug into the `force("protobuf-java:3.6.1")` and filed the
underlying incompatibility as #99. The key findings:
- The canal client 1.1.7 genuinely can't talk to canal-server against modern
protobuf-java — its generated code calls
`GeneratedMessageV3.makeExtensionsImmutable()`, removed in protobuf 3.21+/4.x.
So this is a real incompatibility, and the test correctly surfaces it.
- **But the fix here is `configurations.all` — module-wide, including the
shipped NAR — pinning to protobuf 3.6.1 (2018, with known CVEs).** The
demonstrated breakage is on the *test* classpath (protobuf 4.31.1 pulled by
pulsar-client). The module's runtimeClasspath resolves no external protobuf at
all, so whether the NAR actually needs this pin depends on what protobuf the
Pulsar runtime provides — likely modern, which would mean the connector is
broken in production too, but that should be confirmed.
- **`canal.client` 1.1.8 exists.** If it's protobuf-4-compatible, upgrading
is cleaner than downgrading protobuf across the NAR.
My recommendation: don't merge the module-wide 3.6.1 pin without deciding
between (a) canal 1.1.8 upgrade, (b) shading protobuf in the NAR, or (c) a
test-scoped pin if production is unaffected. The test itself is sound and
mutation-verified — it's only the bundled dependency workaround that warrants
the call. Details and CVE references in #99.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
