[jaeger-operator] Fix certificate condition in deployment file for #375 #376
Conversation
Signed-off-by: AshutoshNirkhe <[email protected]>
AshutoshNirkhe
force-pushed
the
fix-for-certs
branch
from
899d5d6 to
0ee1fbc
Compare
@batazor appreciate if you could have a look too ? |
|
@AshutoshNirkhe Did you tried to deploy it with .certs.certificate.create to |
| @@ -48,10 +48,12 @@ spec: | |||
| - containerPort: 9443 | |||
| name: webhook-server | |||
| protocol: TCP | |||
| {{- if .Values.certs.certificate.create }} | |||
There was a problem hiding this comment.
Check out the https://github.com/jaegertracing/helm-charts/tree/main/charts/jaeger-operator#prerequisites for the operator chart and my comment in conversation. In case you don't want to use cert-manager you should create certificate for webhook service in a secret instead.
There was a problem hiding this comment.
@czomo thanks for the explanation. I have already configured it to use webhooks and self-signed issuer/certs now so am good.
Basically it started with if we can disable both webhooks and hence certificates for webhooks.
Like what if I disable both the webhooks ? (https://github.com/jaegertracing/helm-charts/blob/jaeger-operator-2.32.2/charts/jaeger-operator/values.yaml#L22)
Why should I still care about certificates ? So if webhooks are mandatory, shall we get rid off those flags from values file to enable/disable them or update the Readme ?
There was a problem hiding this comment.
jaeger-operator can be deployed in multiple ways, that's why there is option to disable specific components.
webhooks in general are integrated part of jaeger because they are responsible for validating jeager resources(validating webhook) or in case of mutating webhooks adding jaeger sidecars. However there are some cases when you want do disable them because of conflicts with other resources. Be carefull about that.
One of the purpose of certs is handling secure connection between service and webhooks, more features will be added in the future.
There was a problem hiding this comment.
Ok so I think its wiser to not disable any of that :) We don't this PR anymore in that case.
There was a problem hiding this comment.
We should be able to disable creating the certificates so people can use their own cert-manager issuers and settings. Is this PR being merged?
pavelnikolov
changed the title
|
Is any update for this PR? We're facing the same issue. Many thanks |
What this PR does
Which issue this PR fixes
(optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format,will close that issue when PR gets merged)
Checklist
[jaeger]or[jaeger-operator])