-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[Feature] Per-Store TLS configuration in Thanos Query #977
Comments
Definitely going to try and throw something together for this. Ideas so far are: a) use JDBC-like URLs to configure individual stores (e.g. or b) use file-based configuration like the object store. @bwplotka If I go for option "b" should I re-use the object-store file config stuff, or are you happy for me to implement a query-store specific config file? |
So I think we talked about it here: #899 I would say for this specifc use case, let's start with option
What? We should never mix object store stuff with discovery stuff. In fact you have ready FileSD logic which is compatible with Prometheus FileSD. It allows arbitrary labels, so we can implement easily this:
I would say this is the way to go. What do you think? (: Thanks for proposing this and sorry for massive delay! |
@bwplotka Apologies, I meant I can look at writing some similar code to what is in place for FileSD to implement |
Yea, I would say no specifc config just literally special labels we document properly in our docs |
IMO people just coud just run envoy/nginx/etc proxy and handle TLS outside of Thanos. As then we don't have to work on cert reloading all the different options, etc. @bwplotka wdyt? |
I attempted to use nginx, but it appears that the grpc call is not sending a host header. Can anyone confirm this is the case? (Or at least anyone with better knowledge of the code and go than myself) |
Why does it need that? I think you should be able to proxy all the traffic to GPRC backend. Something like:
|
In my case I would like to park the grpc on a virtual host, instead of running a special nginx instance just for grpc. |
I don't think we pass host headers nor we have an option to configure them. So right now I don't think it will work |
I'ts not an objection, I just wanted to point out that it won't work right now. Whether we want to add host header or no is a seperate discussion and it will take a bit of time. BTW a lot of people right now run isitio / linkerd, the sidecar per service approach, so I think it's not a wide problem. |
Up to this point nginx ingress has worked for us and I would rather not complicate things any more then I need to. I have been looking at envoy and ambassador, and it may be a good fit down the road. For the moment I am using port based tcp for nginx ingress since most of our use cases are behind vpn or firewall. I was curious if I was missing something or if there were plans to pass header info since grpc is built on top of http(2). |
@daviddyball , I'm not sure if I understand your use case correctly, but does this pr #508 solve your problem? There are flags like --grpc-server-tls-cert/--grpc-client-tls-cert added by this pr in query doc and other components' docs |
@benjaminhuo I don't think it does solve the problem here. The issue is that I'd like |
For now we won't be implementining Per Store TLS configuration. Our general suggestion is run sidecar proxy envoy / nginx / etc. Hopefully we will update docs soon. We will close the ticket once the docs are there. |
I would also like something like this. My idea, which may be wrong, is:
ideally, I would like to define something like:
so, it seems that without setting as far as I understand there is no way of doing this right now. |
@daviddyball I'm considering using ambassador as an edge proxy in each cluster too. But for the central monitoring cluster, is it possible for ambassador to originate tls connection to the remote cluster(egress grpc tls traffic)? Would you share your config? thanks. @bwplotka @caarlos0 , I found a blog post https://improbable.io/blog/thanos-architecture-at-improbable saying
Just wondering where I can find envoy config examples like this for ingress and especially egress tls traffic? Thanks very much |
@benjaminhuo I had |
Yeah, I'm considering using envoy to manage outbound tls. Then you've the same tls client config for all the store specified in thanos query including the one in the same cluster as thanos query? |
We're not validating TLS at the moment @benjaminhuo , so there's no TLS config on |
I see :) |
It looks like there are mix of things in this feature requests. So is it about host header or per store TLS config? |
Per-store TLS config. The ability to specify TLS for some stores and disable it for others. We've since worked around the issue, so if it doesn't fit with the intended design of Thanos, then it can be closed as |
@Namanl2001 by local instance do you mean side car that is in same namespace? and by remote do you mean side car in different cluster altogether? To clarify from my side, I can add multiple sidecars to thanos query IF I use service name in my kubernetes cluster. But the moment I add a sidecar which is behind https and the respective TLS certs, I am unable to add any other side car. |
Hello 👋 Looks like there was no activity on this issue for the last two months. |
Working PR #4389 |
Hello 👋 Looks like there was no activity on this issue for the last two months. |
Closing for now as promised, let us know if you need this to be reopened! 🤗 |
Looks like this is still unresolved |
Hello 👋 Looks like there was no activity on this issue for the last two months. |
What is the status? |
Hi all, is somebody working on this? We also have indications that this issue is not resolved. |
This is a very necessary feature |
New to setting up Thanos and I've just spent a good amount of time trying to debug my ingress and TLS setup only to discover this issue and realising that you can only either have TLS-enabled stores with the |
I also got burned by this. |
This is the extremely painful workaround: we're going to set up an In your cluster, you can create the envoy proxy this way: # the purpose of this manifest is to convert
# https://some-address.example.com with a given Basic auth
# into a service http://some-thanos-sidecar.monitoring.svc.cluster.local with no SSL
# this works around https://github.com/thanos-io/thanos/issues/977
apiVersion: v1
kind: ConfigMap
metadata:
name: some-thanos-sidecar-envoy
namespace: monitoring
data:
envoy.yaml: |
static_resources:
listeners:
- name: listener_0
address:
socket_address: { address: 0.0.0.0, port_value: 8080 }
filter_chains:
- filters:
- name: envoy.filters.network.http_connection_manager
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
stat_prefix: ingress_http
codec_type: AUTO
route_config:
name: local_route
virtual_hosts:
- name: local_service
domains: [ "*" ]
routes:
- match: { prefix: "/" }
route:
cluster: service_cluster
timeout: 0s
host_rewrite_literal: some-address.example.com
request_headers_to_add:
- header:
key: "Authorization"
# use the following to encode the value after Basic
# echo -n "username:_some_password_" | base64
value: "Basic abcdefgxyz=="
http_filters:
- name: envoy.filters.http.router
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
clusters:
- name: service_cluster
http2_protocol_options: {}
connect_timeout: 0.25s
type: LOGICAL_DNS
# Comment out the following line to test on v6 networks
dns_lookup_family: V4_ONLY
lb_policy: ROUND_ROBIN
load_assignment:
cluster_name: service_cluster
endpoints:
- lb_endpoints:
- endpoint:
address:
socket_address:
address: some-address.example.com
port_value: 443
transport_socket:
name: envoy.transport_sockets.tls
typed_config:
"@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
common_tls_context:
alpn_protocols: h2
sni: some-address.example.com
admin:
address:
socket_address: { address: 0.0.0.0, port_value: 8001 }
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: some-thanos-sidecar-envoy-proxy
namespace: monitoring
spec:
replicas: 1
selector:
matchLabels:
app: some-thanos-sidecar-envoy-proxy
template:
metadata:
labels:
app: some-thanos-sidecar-envoy-proxy
spec:
containers:
- name: envoy
image: envoyproxy/envoy:v1.26.2
ports:
- containerPort: 8080
volumeMounts:
- name: envoy-config
mountPath: /etc/envoy
volumes:
- name: envoy-config
configMap:
name: some-thanos-sidecar-envoy
---
apiVersion: v1
kind: Service
metadata:
name: some-thanos-sidecar-envoy-proxy
namespace: monitoring
spec:
selector:
app: some-thanos-sidecar-envoy-proxy
ports:
- protocol: TCP
port: 80
targetPort: 8080
type: ClusterIP Observe an Authorization header is added. This makes sense if you are protecting your thanos sidecar in a remote cluster using authentication. Now, add the following to your query service:
Here's an example from my Flux HelmRelease manifest using the bitnami thanos chart: apiVersion: helm.toolkit.fluxcd.io/v2beta1
kind: HelmRelease
metadata:
name: thanos
namespace: monitoring
spec:
interval: 5m
chart:
spec:
version: "12.6.2"
chart: thanos
sourceRef:
kind: HelmRepository
name: bitnami
interval: 60m
install:
crds: Create
upgrade:
crds: CreateReplace
# https://github.com/prometheus-community/helm-charts/blob/main/charts/kube-prometheus-stack/values.yaml
values:
# this is specific to your cluster
existingObjstoreSecret: thanos-objstore-config
query:
enabled: true
extraFlags:
# works around https://github.com/thanos-io/thanos/issues/977
- --endpoint=some-thanos-sidecar-envoy-proxy.monitoring.svc.cluster.local:80
- --query.timeout=5m
- --query.lookback-delta=15m
dnsDiscovery:
enabled: true
sidecarsService: kube-prometheus-stack-thanos-discovery
sidecarsNamespace: monitoring
queryFrontend:
enabled: true
extraFlags:
- --query-frontend.compress-responses
- --query-range.split-interval=12h
- --labels.split-interval=12h
- --query-range.max-retries-per-request=10
- --labels.max-retries-per-request=10
- --query-frontend.log-queries-longer-than=10s
storegateway:
enabled: true
compactor:
enabled: true To secure your thanos sidecar with basic auth, use an appropriate ingress configuration. |
5 years and still the exact same issue 😢 |
I just wasted the better part of a day on this. My Query component communicates with a collection of in-cluster and out-of-cluster stores, requiring TLS for the external communication, but not the internal ones. If this isn't going to be prioritized as a feature, could it at least be called out as a gotcha somewhere pretty obvious in the Query docs? |
I prepared this by introducing a config file where every endpoint is configured - we can add grpc options there too! By now it should be straight forward I think |
Request
Feature request to add support for per-store tls configuration.
Use Case
I have
store
running alongsidequery
on my main "monitoring" cluster and we don't use TLS for comms between pods on the same cluster. I also want to pull metrics from my remote Prometheus instances which exist on different clusters, but these endpoints are TLS encrypted because they are cross-clusterSuggestion
Change the
--store
argument to use a more definitive style for remote endpoints e.g.:grpc://<ip-address>:<port>
grpc+tls://<ip-address>:<port>
This would allow specifying per-store TLS settings and allow a combination of secure and insecure comms for store endpoints.
Related PR from @adrien-f : #899
The text was updated successfully, but these errors were encountered: