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

ServerGroup TLS settings not configured for remote_read client #373

Closed
iplay88keys opened this issue Jan 13, 2021 · 4 comments · Fixed by #377
Closed

ServerGroup TLS settings not configured for remote_read client #373

iplay88keys opened this issue Jan 13, 2021 · 4 comments · Fixed by #377

Comments

@iplay88keys
Copy link
Contributor

We have Nginx running in front of each of our Prometheus instances which is terminating mutual TLS connections from Promxy before proxying the requests to the Prometheus instance running alongside them (which is listening on localhost).

The issue we are having is that when Promxy is configured with remote_read: true in the ServerGroup settings, the TLS configuration from the Promxy configuration is not being passed to the http client that is created for the PromAPIRemoteRead client.

As a result, we are getting errors with regards to the ServerName not matching the DNS names we have set as the ServerGroup targets.

We are working around this for now by removing the remote_read: true line in our Promxy configuration files. This results in the API client being used instead which supports the TLS configuration. It seems likely that it would be useful to others to have TLS settings be passed in to the PromAPIRemoteRead client.

We tested adding the HTTPClientConfig to the remote client configuration struct which results in successful connections:

cfg := &remote.ClientConfig{
  URL: &config_util.URL{u},
  HTTPClientConfig: s.Cfg.HTTPConfig.HTTPConfig,
  Timeout: model.Duration(time.Minute * 2),
}
@iplay88keys iplay88keys changed the title TLS ServerGroup settings not configured for remote_read client ServerGroup TLS settings not configured for remote_read client Jan 13, 2021
@iplay88keys
Copy link
Contributor Author

There is also a possibility that I just don't fully understand the usage of remote_read and it doesn't need TLS settings.

@jacksontj
Copy link
Owner

@iplay88keys This definitely sounds like a miss, thanks for the report! To clarify a bit, remote_read is an alternative way of fetching the data from the downstream prometheus APIs. Most people don't bother with using remote_read as all the query splitting still uses the API client; but you should definitely be able to configure it this way (just keep in mind aggregations and other splittable queries will still use the "regular" API client). the diff you have above seems like the correct change, if you'd like you can submit a PR and I can merge it in :) If you don't want to I can easily do the same, I just figure you might want the kudo points on github :)

@iplay88keys
Copy link
Contributor Author

@jacksontj Thanks for the quick response! I'd be happy to submit a PR early next week! Thanks for clarifying the use case for remote_read as well!

@iplay88keys
Copy link
Contributor Author

@jacksontj We are working on the PR. We are not seeing any tests around the servergroup.go file, so we will submit without any additional tests for now. We would be happy to add tests, however, if you can point us to where we should add them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants