Closed mihau closed 4 years ago
@objectiser thoughts? I think we talked a bit about having the ingress as a child of the Query spec, and decided to have it as a top-level because we have only one ingress at the moment.
If we want to have this ingress at the top-level in the long term, then I'm OK with the approach proposed by @mihau.
@jpkrohling yes I think the ingress at the top level is fine for now - as it is currently defined. I'm also ok with the approach suggested in this issue, the only question is how this would impact the IngressSecurityType
field in the JaegerIngressSpec
which can define oauth-proxy
?
They would be incompatible, as Ingress
isn't used in OpenShift, while oauth-proxy
is only used in OpenShift.
IIRC, the Ingress code is already ignored when --platform=openshift
.
As shown in this example, OpenShift OAuth is controlled via the spec.ingress.security
property.
Right, I meant that the code that creates the actual Kubernetes Ingress
is platform dependent, so, we could just ignore these new properties if the operator is running in OpenShift.
Or did you mean that the TLS configuration is a candidate for another possible value for IngressSecurityType
?
Yes that was one possibility. So as oauth-proxy
is the default when platform is openshift
, what should the default value be when platform is kubernetes
? And then should the use of the hosts
and tls
values only be used if the security
property is set to a particular value?
I think they are not related, actually. One is about auth, the other is about securing the pipe. Perhaps we should consider renaming Security
to Auth
?
All in all, I like @mihau's approach, as it's practical and fits well when the operator runs in Kubernetes. We should just make sure to log somewhere (as I think we do already) that the other options are ignored if the platform is OpenShift.
SGTM, so @mihau looks like you have the green light :)
Hi @mihau, I'm very interested by your proposal and I look forward to seeing your work.
@mihau This would be great!
Given that @mihau posted the proposal originally last November, it is possible that someone else may need to take up the implementation. Is there anyone else that would be interested in contributing this feature?
Will there be an option in JaegerIngressSpec to specify hosts for ingress resource?
@stevie-, we might. The proposal from this issue has been accepted, but so far, nobody sent a PR implementing it.
This is my current workaround example if anyone needs something similar. Basically, turn off the generated ingress and create my own ingress.
jaeger instance to disable the default ingress
apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
name: jaeger
spec:
ingress:
enabled: false
separate ingress using nginx ingress, cert-manager, lets-encrypt with http challenge
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
name: traces
annotations:
kubernetes.io/ingress.class: "nginx"
certmanager.k8s.io/cluster-issuer: "letsencrypt-production"
certmanager.k8s.io/acme-challenge-type: http01
spec:
tls:
- hosts:
- traces.domain.com
secretName: traces-domain-tls
rules:
- host: traces.domain.com
http:
paths:
- path: /
backend:
serviceName: jaeger-query
servicePort: 16686
To access the UI, you would go to https://traces.domain.com.
Our Workaround fir Nginx ingress: We use the help of external-dns and just add an annotation to the ingress spec for Jaeger CRD. This adds a DNS record for the ingress loadbalancer and Nginx ingress does the rest.
Caution: May not work if you have more ingress with host set to '*'.
@stevie- can you give code sample as I didn’t follow completely? I’m a little new to all this.
Snippet of our Helm Template to create the Jaeger resource
apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
name: {{ include "jaeger.fullname" . }}-es-{{ .Values.jaeger.strategy }}
labels:
app.kubernetes.io/name: {{ include "jaeger.name" . }}
helm.sh/chart: {{ include "jaeger.chart" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
spec:
strategy: {{ .Values.jaeger.strategy }}
ingress:
enabled: {{ .Values.operatorIngress.enabled }}
annotations:
external-dns.alpha.kubernetes.io/hostname: {{ .Values.operatorIngress.host }}
so if someone still struggles to get this work here is jaeger cr config for ingress
spec:
ingress:
enabled: true
annotations:
kubernetes.io/ingress.class: nginx
hosts:
- xxxx.com
@prageethw would you mind opening a PR with an example? Here's the right place for it:
https://github.com/jaegertracing/jaeger-operator/tree/master/deploy/examples
@jpkrohling done https://github.com/jaegertracing/jaeger-operator/pull/1231
Is there a support for context root as well? Our project needs something like mydomain.com/jaeger for the ingress
There seems to be no way to specify hostname for query ingress while creating a jaeger resource. Allowing for that as well as TLS settings would make it easy to handle by popular ingress controllers such as nginx.
I would approach it by adding fields to
jaeger
CRD similar to:I could implement it as such if everyone is ok with this approach.