Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avro parser plugin can not connect to the schema registry over "https". #13334

Closed
BelousovAntonV opened this issue May 25, 2023 · 9 comments · Fixed by #13903
Closed

Avro parser plugin can not connect to the schema registry over "https". #13334

BelousovAntonV opened this issue May 25, 2023 · 9 comments · Fixed by #13903
Labels
feature request Requests for new plugin and for new features to existing plugins help wanted Request for community participation, code, contribution size/m 2-4 day effort

Comments

@BelousovAntonV
Copy link
Contributor

BelousovAntonV commented May 25, 2023

Use Case

We are loading data from Kafka to InfluxDB on on several local machine. During loading data to local Kafka we use cloud schema registry, which we communicate over "https" with SSL context and Authorization Basic header.

Expected behavior

Avro plugin could connect to a schema registry over "https" using security parameters specified in the configuration file.

Actual behavior

At the current moment there is no configuration options to specify security info in the configuration file and plugin returns an error:
E! [inputs.kafka_consumer] Error in plugin: Get "https://schema-registry.cloud.com:443/schemas/1": tls: failed to verify certificate: x509: certificate signed by unknown authority

Additional info

With a local Schema Registry without authentication it works perfect.

@BelousovAntonV BelousovAntonV added the feature request Requests for new plugin and for new features to existing plugins label May 25, 2023
@powersj
Copy link
Contributor

powersj commented May 26, 2023

The avro schema registry will need to grow additional options, like the other HTTP related calls, to be able to use a custom cert and/or ignore invalid TLS. It may be worth having the parser use the httpconfig.HTTPClientConfig in common.

next steps: extend the parser with the http settings to handle TLS settings

@powersj powersj added size/m 2-4 day effort help wanted Request for community participation, code, contribution labels May 26, 2023
@srebhan
Copy link
Member

srebhan commented Jun 9, 2023

@powersj how about only allowing system TLS settings for the registry without customization? This can easily be done based on the URL without additional params... If that's OK for you I can work on it...

@powersj
Copy link
Contributor

powersj commented Jun 9, 2023

how about only allowing system TLS settings for the registry without customization?

Not sure I'm following - the original reporter seems to indicate that they need to add a certificate to validate. Are you suggesting that we say you need your system's cert store to have the necessary items to work?

@srebhan
Copy link
Member

srebhan commented Jun 12, 2023

Oh sorry I misread the original issue... Nevermind.

@BelousovAntonV
Copy link
Contributor Author

I've implemented this functionality to our local telegraf build. Could share my solution. Should I do it and how it should be done in case of "yes"?

@powersj
Copy link
Contributor

powersj commented Jun 13, 2023

@BelousovAntonV always happy to see a PR put up!

@srebhan
Copy link
Member

srebhan commented Jul 12, 2023

@BelousovAntonV please share your implementation by creating a pull-request on Github. To do so you need to fork the telegraf repository and then commit your changes to your fork. The follow the "create a pull-request" link above. If you need help, please let me know!

@BelousovAntonV
Copy link
Contributor Author

BelousovAntonV commented Aug 23, 2023

Hi.
I've created a PR, but I do not know what tests I can write. The schema registry with https protocol is needed for tests. I've deployed changed version of Telegraf to our test env and checked that all is working correct. Could PR be approved without adding the tests?

@srebhan
Copy link
Member

srebhan commented Sep 1, 2023

@BelousovAntonV yeah I think the tests would be nice but not mandatory. Please check the PR as I've added some review comments...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requests for new plugin and for new features to existing plugins help wanted Request for community participation, code, contribution size/m 2-4 day effort
Projects
None yet
3 participants