michaeljmarshall commented on issue #20862: URL: https://github.com/apache/pulsar/issues/20862#issuecomment-1650239074
> > but that is not always the case because the connector framework passes a configuration map of Map<String, Object>. > > I don't quite understand what's wrong with the incoming map. Can you explain in detail? The map is not the problem. In the section you quoted, I am describing the currently available mechanism to access secrets from a sink or a source. The current flow has two options: Option 1: 1. Access all secrets from the context's `getSecret(String)` method. Option 2: 1. Create a class for the configuration of the connector. 2. Annotate fields that are sensitive correctly. (This annotation is a custom Pulsar annotation and is not documented.) 3. Convert the `Map<String, Object>` to the class from step 1 using the `IOConfigUtils` class, and that class will pull secrets using the `getSecret` method described in option 1. The problem with option 2 is that it hasn't been consistently implemented, and it leaves the correct implementation up to the sink/source creator instead of giving users a secure default. For example, the Canal Source connector takes the effort to create the config class and correctly annotate the methods: https://github.com/apache/pulsar/blob/ec102fb024a6ea2b195826778300f20e330dff06/pulsar-io/canal/src/main/java/org/apache/pulsar/io/canal/CanalSourceConfig.java#L39-L50 However, when the connector loads the configuration, it does so without the `IOConfigUtils`, which is not compliant with option 2. https://github.com/apache/pulsar/blob/ec102fb024a6ea2b195826778300f20e330dff06/pulsar-io/canal/src/main/java/org/apache/pulsar/io/canal/CanalSourceConfig.java#L83-L92 As such, a user must put the `username` and the `password` in the source configuration instead of in the secrets configuration. Other connectors that use the annotation with `sensitive=true` but without securely loading the config are `SolrSinkConfig`, `InfluxDBSinkConfig`, `JdbcSinkConfig`, `RabbitMQAbstractConfig`, and `RedisAbstractConfig`. There are also connectors that do not use any annotations and just treat passwords like normal config. The `AerospikeSinkConfig` is one example: https://github.com/apache/pulsar/blob/ec102fb024a6ea2b195826778300f20e330dff06/pulsar-io/aerospike/src/main/java/org/apache/pulsar/io/aerospike/AerospikeSinkConfig.java#L32-L58 The above examples are all in the main apache/pulsar repo. We can expect even more confusion from third party connectors. My core thesis is that the existing framework is not well documented and requires connector maintainers to do an extra step that could easily be handled by the framework itself. > BTW: I think you need to mention a PIP first. Thanks. A similar feature did not need a PIP https://github.com/apache/pulsar/pull/20116. I think this feature is small enough that it does not need a PIP. In your opinion, why should this require a PIP? Thanks. -- 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]
