xiangfu0 commented on code in PR #20121:
URL: https://github.com/apache/pinot/pull/20121#discussion_r4445965162
##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/sink/PinotSink.java:
##########
@@ -83,6 +86,15 @@ public PinotSink(PinotGenericRowConverter<T>
recordConverter, TableConfig tableC
DEFAULT_SEGMENT_FLUSH_MAX_NUM_RECORDS, DEFAULT_EXECUTOR_POOL_SIZE);
}
+ /// Same as [#PinotSink(PinotGenericRowConverter, TableConfig, Schema,
String)], and also adds `authToken` to the
+ /// batch config so segment uploads authenticate with the controller. The
token only lives in the in-memory table
+ /// config; it is never written back to the controller.
+ public PinotSink(PinotGenericRowConverter<T> recordConverter, TableConfig
tableConfig, Schema schema,
+ String controllerBaseUrl, @Nullable String authToken) {
Review Comment:
(minor, security) `prepareTableConfigForSink` mutates the caller's
`TableConfig` in place (L135/L156), so after this constructor the caller's
object holds the token. I checked locally: `callerConfig.toJsonString()` then
includes `"authToken":"Basic secret"`. If the app later logs that config or
sends it back via `updateTableConfig`, the token leaks or gets persisted,
despite the README's "in-memory only" note. Deep-copying first (e.g. a JSON
round-trip, since `new TableConfig(tc)` is shallow) would make the note hold.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
