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

[FEAT] Disable InfluxDB certificate verification #440

Closed
SaswatPadhi opened this issue Feb 5, 2023 · 3 comments · Fixed by #441
Closed

[FEAT] Disable InfluxDB certificate verification #440

SaswatPadhi opened this issue Feb 5, 2023 · 3 comments · Fixed by #441
Labels
good first issue Good for newcomers help wanted Extra attention is needed waiting for response

Comments

@SaswatPadhi
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I use a self-signed certificate on my InfluxDB server. When trying to use this server with scrutiny, it fails to start with an error about unknown certificate authority. This is expected, since the certificate is self signed.

Describe the solution you'd like

Most applications allow disabling SSL verification (e.g., verify_ssl: false in Home Assistant's influxdb integration). It would be nice to have something like that in scrutiny.

@AnalogJ
Copy link
Owner

AnalogJ commented Feb 5, 2023

Hey @SaswatPadhi
Scrutiny uses the official influxdata/influxdb-client-go sdk under the hood to communicate with the InfluxDB API.

https://github.com/AnalogJ/scrutiny/blob/master/webapp/backend/pkg/database/scrutiny_repository.go#L98

Looks like its possible to customize the TLS config via an option in the Client constructor.

https://github.com/influxdata/influxdb-client-go#options

 client := influxdb2.NewClientWithOptions("http://localhost:8086/", "my-token",
    influxdb2.DefaultOptions().
        SetUseGZip(true).
        SetTLSConfig(&tls.Config{
            InsecureSkipVerify: true,
        }))

Would you be willing to test it out and open a PR?

@AnalogJ AnalogJ added help wanted Extra attention is needed good first issue Good for newcomers waiting for response labels Feb 5, 2023
@SaswatPadhi
Copy link
Contributor Author

Thanks for the pointer, @AnalogJ!

I'd be happy to test the change and open a PR shortly.

@SaswatPadhi
Copy link
Contributor Author

Hi @AnalogJ,

I have tested the changes at my end and just opened a PR for merging it.

Besides the influxdb2 client, there was an http client in InfluxSetupComplete which also required the TLS config tweak.

AnalogJ added a commit that referenced this issue Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed waiting for response
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants