[oc] Auto create TLS cert in collector deployment #914

annanay25 · 2020-02-18T07:50:12Z

For OpenShift platform -

Annotate the collector service with tag service.beta.openshift.io/serving-cert-secret-name
Mount the auto-created secret (with the crt and key)on the pod and use it in the cli for the collector grpc server

Resolves: #599

Signed-off-by: Annanay [email protected]

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Signed-off-by: Annanay <[email protected]>

jpkrohling

Looks really good, thanks for this PR! I have only a couple of comments that would make our lives easier in the future.

The lint step also failed because of tls.TLSConfig. Rename to tls.Config and it's all good :-)

pkg/config/tls/tls.go

pkg/deployment/collector.go

jpkrohling · 2020-02-18T13:02:27Z

cc @kevinearls, @jkandasa could one of you please give this a try? It should be sufficient to just run make run instead of building an assembly via openshift-courier.

Signed-off-by: Annanay <[email protected]>

annanay25 · 2020-02-19T08:09:36Z

Thanks for the review @jpkrohling. I've addressed comments :)

Signed-off-by: Annanay <[email protected]>

jpkrohling · 2020-02-19T09:41:46Z

Looks good! The mentioned PR has been merged, so, this can be rebased against master again.

@kevinearls, @jkandasa, could one of you please test this?

Signed-off-by: Annanay <[email protected]>

pkg/config/tls/tls.go

jpkrohling · 2020-02-20T15:14:35Z

@annanay25 do you need a cluster to test this?

annanay25 · 2020-02-23T14:17:51Z

do you need a cluster to test this?

Yes, that would be great @jpkrohling :) . I need to confirm if service.beta.openshift.io/inject-cabundle=true injects both the cert and the key (I think it should, since it says ca-bundle).

jpkrohling · 2020-02-24T13:35:01Z

Just sent you the details to your OpenShift cluster in private, via Gitter.

Signed-off-by: Annanay <[email protected]>

annanay25 · 2020-02-25T14:23:55Z

There seems to be one warning -

Annanays-Mac:jaeger-operator annanay$ kubectl logs -f pod/simple-prod-collector-649fddfc7c-xxd65
2020/02/25 14:21:03 maxprocs: Leaving GOMAXPROCS=2: CPU quota undefined
{"level":"info","ts":1582640463.727751,"caller":"flags/service.go:115","msg":"Mounting metrics handler on admin server","route":"/metrics"}
{"level":"info","ts":1582640463.728069,"caller":"flags/admin.go:108","msg":"Mounting health check on admin server","route":"/"}
{"level":"info","ts":1582640463.7281487,"caller":"flags/admin.go:114","msg":"Starting admin HTTP server","http-port":14269}
{"level":"info","ts":1582640463.7281687,"caller":"flags/admin.go:100","msg":"Admin server started","http-port":14269,"health-status":"unavailable"}
{"level":"info","ts":1582640463.783358,"caller":"config/config.go:172","msg":"Elasticsearch detected","version":5}
{"level":"info","ts":1582640463.8457026,"caller":"collector/main.go:128","msg":"Starting jaeger-collector TChannel server","port":14267}
{"level":"warn","ts":1582640463.8457642,"caller":"collector/main.go:129","msg":"TChannel has been deprecated and will be removed in a future release"}
{"level":"info","ts":1582640463.9107623,"caller":"grpcserver/grpc_server.go:64","msg":"Starting jaeger-collector gRPC server","grpc-port":"14250"}
{"level":"info","ts":1582640463.9108763,"caller":"collector/main.go:148","msg":"Starting jaeger-collector HTTP server","http-port":14268}
{"level":"info","ts":1582640463.9109273,"caller":"healthcheck/handler.go:128","msg":"Health Check state change","status":"ready"}
{"level":"info","ts":1582640463.936088,"caller":"collector/main.go:240","msg":"Listening for Zipkin HTTP traffic","zipkin.http-port":9411}
WARNING: 2020/02/25 14:21:32 grpc: Server.Serve failed to complete security handshake from "10.129.2.19:60554": tls: first record does not look like a TLS handshake
WARNING: 2020/02/25 14:21:33 grpc: Server.Serve failed to complete security handshake from "10.129.2.19:60560": tls: first record does not look like a TLS handshake

I think its a client trying to connect to the collector without using TLS, but @jpkrohling could you PTAL?

jpkrohling

This goes in the right direction and gets things deployed, but might need a couple of adjustments in order to work.

pkg/config/tls/tls.go

pkg/strategy/production.go

pkg/deployment/agent.go

Signed-off-by: Annanay <[email protected]>

jpkrohling · 2020-02-26T09:19:34Z

Looks like it's still not quite working yet. This is what I see in the Agent logs for the Query:

{"level":"info","ts":1582707258.2944024,"caller":"base/balancer.go:83","msg":"base.baseBalancer: got new ClientConn state: {{[{10.129.2.18:14250 0  <nil>}] <nil>} <nil>}","system":"grpc","grpc_log":true}
{"level":"info","ts":1582707260.305737,"caller":"base/balancer.go:140","msg":"base.baseBalancer: handle SubConn state change: 0xc0002256b0, CONNECTING","system":"grpc","grpc_log":true}
{"level":"info","ts":1582707260.3057969,"caller":"roundrobin/roundrobin.go:50","msg":"roundrobinPicker: newPicker called with readySCs: map[]","system":"grpc","grpc_log":true}
{"level":"info","ts":1582707260.3066912,"caller":"base/balancer.go:140","msg":"base.baseBalancer: handle SubConn state change: 0xc0002256b0, TRANSIENT_FAILURE","system":"grpc","grpc_log":true}
{"level":"info","ts":1582707260.3067415,"caller":"transport/log.go:30","msg":"transport: loopyWriter.run returning. connection error: desc = \"transport is closing\"","system":"grpc","grpc_log":true}

And this is from the collector logs:

$ kubectl logs simple-prod-collector-5d556d5db-dxmpr
2020/02/26 08:50:59 maxprocs: Leaving GOMAXPROCS=2: CPU quota undefined
{"level":"info","ts":1582707059.8038766,"caller":"flags/service.go:115","msg":"Mounting metrics handler on admin server","route":"/metrics"}
{"level":"info","ts":1582707059.804306,"caller":"flags/admin.go:108","msg":"Mounting health check on admin server","route":"/"}
{"level":"info","ts":1582707059.8043773,"caller":"flags/admin.go:114","msg":"Starting admin HTTP server","http-port":14269}
{"level":"info","ts":1582707059.8044014,"caller":"flags/admin.go:100","msg":"Admin server started","http-port":14269,"health-status":"unavailable"}
{"level":"info","ts":1582707059.8288152,"caller":"config/config.go:172","msg":"Elasticsearch detected","version":6}
{"level":"info","ts":1582707059.917191,"caller":"collector/main.go:128","msg":"Starting jaeger-collector TChannel server","port":14267}
{"level":"warn","ts":1582707059.917238,"caller":"collector/main.go:129","msg":"TChannel has been deprecated and will be removed in a future release"}
{"level":"info","ts":1582707059.9429655,"caller":"grpcserver/grpc_server.go:64","msg":"Starting jaeger-collector gRPC server","grpc-port":"14250"}
{"level":"info","ts":1582707059.9430513,"caller":"collector/main.go:148","msg":"Starting jaeger-collector HTTP server","http-port":14268}
{"level":"info","ts":1582707059.9430842,"caller":"healthcheck/handler.go:128","msg":"Health Check state change","status":"ready"}
{"level":"info","ts":1582707059.9643774,"caller":"collector/main.go:240","msg":"Listening for Zipkin HTTP traffic","zipkin.http-port":9411}
WARNING: 2020/02/26 08:51:18 grpc: Server.Serve failed to complete security handshake from "10.129.2.19:38274": tls: first record does not look like a TLS handshake
WARNING: 2020/02/26 08:51:19 grpc: Server.Serve failed to complete security handshake from "10.129.2.19:38278": tls: first record does not look like a TLS handshake
WARNING: 2020/02/26 08:51:20 grpc: Server.Serve failed to complete security handshake from "10.129.2.19:38294": tls: first record does not look like a TLS handshake

I just got all the certs locally, and I was able to get a working setup. Looks like you are just missing the Agent parameter specifying the service-ca bundle, which makes the agent trust the certs generated by OpenShift's CA. Here's how to test it locally:

$ kubectl get secrets simple-prod-collector-headless-tls -o=go-template='{{index .data "tls.crt"}}' | base64 -d > /tmp/cert.crt

$ kubectl get secrets simple-prod-collector-headless-tls -o=go-template='{{index .data "tls.key"}}' | base64 -d > /tmp/cert.key

$ kubectl apply -f deploy/examples/business-application-injected-sidecar.yaml 

$ kubectl exec -c myapp myapp-7cb8b69d-nvvkv -- cat /var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt > /tmp/service-ca.crt

$ SPAN_STORAGE_TYPE=memory go run -tags ui ./cmd/collector --collector.grpc.tls.enabled=true --collector.grpc.tls.cert=/tmp/cert.crt --collector.grpc.tls.key=/tmp/cert.key 

$ sudo vi /etc/hosts # add a line with the cert's hostname: 127.0.0.1 simple-prod-collector-headless.default.svc

$ SPAN_STORAGE_TYPE=memory go run -tags ui ./cmd/agent --reporter.grpc.host-port=simple-prod-collector-headless.default.svc:14250 --reporter.grpc.tls.enabled=true --reporter.grpc.tls.ca=/tmp/service-ca.crt

In the last command, you should see this in the logs:

{"level":"info","ts":1582708364.6002507,"caller":"base/balancer.go:181","msg":"base.baseBalancer: handle SubConn state change: 0xc0002882b0, READY","system":"grpc","grpc_log":true}
{"level":"info","ts":1582708364.6003134,"caller":"roundrobin/roundrobin.go:48","msg":"roundrobinPicker: newPicker called with info: {map[0xc0002882b0:{{simple-prod-collector-headless.default.svc:14250  <nil> 0 <nil>}}]}","system":"grpc","grpc_log":true}

TL;DR: it should be sufficient to add the following arg to the agent: --reporter.grpc.tls.ca=/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt

Signed-off-by: Annanay <[email protected]>

annanay25 · 2020-02-26T11:24:32Z

Thanks @jpkrohling for the analysis. I've updated the PR.

Also, thanks for provisioning the cluster, which can be taken down once this PR is merged :)

Signed-off-by: Annanay <[email protected]>

annanay25 · 2020-03-03T13:48:52Z

Just added https://github.com/jaegertracing/jaeger-operator/pull/914/files#diff-acd8a9fe2d2997867f151eb11780379bR138-R146 in sidecar.go which was missing previously @jpkrohling

jpkrohling

It's very close!

pkg/deployment/agent.go

pkg/inject/sidecar.go

Signed-off-by: Annanay <[email protected]>

annanay25 · 2020-03-03T15:20:53Z

Done :)

jpkrohling

Looks great now, and I confirm that it works.

Could you add a couple of simple unit tests to cover the new lines? Especially about the issues we faced during these last tests.

When the platform is set to openshift:

the agent should have the TLS options, and the "server-name" should contain ${namespace}.svc.cluster.local
same for the sidecar
the collector should have the new TLS options

pkg/strategy/production.go

Signed-off-by: Annanay <[email protected]>

pkg/deployment/agent_test.go

pkg/deployment/collector_test.go

pkg/inject/sidecar_test.go

pkg/deployment/agent_test.go

Signed-off-by: Annanay <[email protected]>

jpkrohling

LGTM, just update the deprecated flag and it's ready to be merged (provided that the CI tests pass).

pkg/config/tls/tls.go

Signed-off-by: Annanay <[email protected]>

annanay25 · 2020-03-05T06:52:17Z

@jpkrohling - Could you please re-run CI on this? I'm not sure if the operator deployment is failing because of this PR

jpkrohling · 2020-03-05T09:36:17Z

Merging, as the tests are passing locally:

Running Smoke end-to-end tests...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	317.085s
Running Cassandra end-to-end tests...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	245.803s
Running Elasticsearch end-to-end tests...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	407.445s
Running Self provisioned Elasticsearch end-to-end tests...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	0.798s
Running Streaming end-to-end tests...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	286.869s
Running Example end-to-end tests part 1...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	123.368s
Running Example end-to-end tests part 2...
ok  	github.com/jaegertracing/jaeger-operator/test/e2e	164.915s

Note that the last one had to be executed twice, because of #945.

annanay25 · 2020-03-05T10:02:45Z

Thanks! 🎉

jpkrohling and others added 4 commits February 14, 2020 12:17

Limit some properties to use at most 63 chars

b445b16

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Restricted the length the ES name may have

8d3ba2a

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Warn if a valid route won't be available due to name length limitation

dfb1d74

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

[oc] Auto create TLS cert in collector deployment

634dc20

Signed-off-by: Annanay <[email protected]>

jpkrohling reviewed Feb 18, 2020

View reviewed changes

pkg/config/tls/tls.go Outdated Show resolved Hide resolved

pkg/config/tls/tls.go Outdated Show resolved Hide resolved

pkg/deployment/collector.go Outdated Show resolved Hide resolved

annanay25 added 2 commits February 19, 2020 13:02

Merge branch 'jpkrohling-truncate-labels' into auto-create-certs

77107ba

Signed-off-by: Annanay <[email protected]>

Address comments, add tls flag to agent reporter, add tests

352e586

Signed-off-by: Annanay <[email protected]>

annanay25 added 2 commits February 19, 2020 14:06

Fix tests

3cee265

Signed-off-by: Annanay <[email protected]>

Fix tests

6d3f182

Signed-off-by: Annanay <[email protected]>

Merge branch 'master' into auto-create-certs

feab557

Signed-off-by: Annanay <[email protected]>

jpkrohling reviewed Feb 20, 2020

View reviewed changes

pkg/config/tls/tls.go Outdated Show resolved Hide resolved

pkg/config/tls/tls.go Outdated Show resolved Hide resolved

Mount tls secret with crt key pair

d6f7b73

Signed-off-by: Annanay <[email protected]>

jpkrohling reviewed Feb 25, 2020

View reviewed changes

Addressed comments

ebb1da9

Signed-off-by: Annanay <[email protected]>

jpkrohling mentioned this pull request Feb 26, 2020

Operation cannot be fulfilled on deployments.apps... #927

Closed

Add crt to agent grpc client to ensure it trusts OpenShift's CA

a05c3fb

Signed-off-by: Annanay <[email protected]>

annanay25 added 3 commits March 2, 2020 15:01

Explicitly add TLS server name in agent connection

c1bb2b3

Signed-off-by: Annanay <[email protected]>

Fix case where grpc reporter type was already added

a900dc8

Signed-off-by: Annanay <[email protected]>

Merge branch 'master' into auto-create-certs

d7cf820

Signed-off-by: Annanay <[email protected]>

jpkrohling reviewed Mar 3, 2020

View reviewed changes

pkg/deployment/agent.go Outdated Show resolved Hide resolved

pkg/inject/sidecar.go Outdated Show resolved Hide resolved

Fix DNS endpoint in reporter.grpc.tls.server-name

ac70b08

Signed-off-by: Annanay <[email protected]>

jpkrohling reviewed Mar 3, 2020

View reviewed changes

pkg/strategy/production.go Show resolved Hide resolved

[openshift] Add tests for grpc tls cli flags

8a2deca

Signed-off-by: Annanay <[email protected]>

jpkrohling reviewed Mar 4, 2020

View reviewed changes

Addressed comments

25cf328

Signed-off-by: Annanay <[email protected]>

jpkrohling reviewed Mar 4, 2020

View reviewed changes

pkg/config/tls/tls.go Outdated Show resolved Hide resolved

annanay25 added 3 commits March 4, 2020 20:04

Fix deprecated flag, collector.grpc.tls -> collector.grpc.tls.enabled

ebce8fd

Signed-off-by: Annanay <[email protected]>

Fix deprecated flag, reporter.grpc.tls -> reporter.grpc.tls.enabled

8f4601d

Signed-off-by: Annanay <[email protected]>

Nit, fix tls test

61ba3c3

Signed-off-by: Annanay <[email protected]>

jpkrohling mentioned this pull request Mar 5, 2020

Flaky e2e-tests-examples2/simple-streaming test #945

Closed

jpkrohling approved these changes Mar 5, 2020

View reviewed changes

jpkrohling merged commit deec90d into jaegertracing:master Mar 5, 2020

This was referenced Mar 11, 2020

TLS configuration for collector missing from all-in-one #959

Closed

Update documentation to include TLS configuration for agent -> collector communication jaegertracing/documentation#384

Closed

objectiser mentioned this pull request May 1, 2020

collector-tls-config-volume Not Created #1029

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[oc] Auto create TLS cert in collector deployment #914

[oc] Auto create TLS cert in collector deployment #914

annanay25 commented Feb 18, 2020 •

edited

Loading

jpkrohling left a comment

jpkrohling commented Feb 18, 2020 •

edited

Loading

annanay25 commented Feb 19, 2020

jpkrohling commented Feb 19, 2020

jpkrohling commented Feb 20, 2020

annanay25 commented Feb 23, 2020

jpkrohling commented Feb 24, 2020

annanay25 commented Feb 25, 2020 •

edited

Loading

jpkrohling left a comment

jpkrohling commented Feb 26, 2020

annanay25 commented Feb 26, 2020 •

edited

Loading

annanay25 commented Mar 3, 2020 •

edited

Loading

jpkrohling left a comment

annanay25 commented Mar 3, 2020

jpkrohling left a comment

jpkrohling left a comment

annanay25 commented Mar 5, 2020

jpkrohling commented Mar 5, 2020

annanay25 commented Mar 5, 2020

[oc] Auto create TLS cert in collector deployment #914

[oc] Auto create TLS cert in collector deployment #914

Conversation

annanay25 commented Feb 18, 2020 • edited Loading

jpkrohling left a comment

Choose a reason for hiding this comment

jpkrohling commented Feb 18, 2020 • edited Loading

annanay25 commented Feb 19, 2020

jpkrohling commented Feb 19, 2020

jpkrohling commented Feb 20, 2020

annanay25 commented Feb 23, 2020

jpkrohling commented Feb 24, 2020

annanay25 commented Feb 25, 2020 • edited Loading

jpkrohling left a comment

Choose a reason for hiding this comment

jpkrohling commented Feb 26, 2020

annanay25 commented Feb 26, 2020 • edited Loading

annanay25 commented Mar 3, 2020 • edited Loading

jpkrohling left a comment

Choose a reason for hiding this comment

annanay25 commented Mar 3, 2020

jpkrohling left a comment

Choose a reason for hiding this comment

jpkrohling left a comment

Choose a reason for hiding this comment

annanay25 commented Mar 5, 2020

jpkrohling commented Mar 5, 2020

annanay25 commented Mar 5, 2020

annanay25 commented Feb 18, 2020 •

edited

Loading

jpkrohling commented Feb 18, 2020 •

edited

Loading

annanay25 commented Feb 25, 2020 •

edited

Loading

annanay25 commented Feb 26, 2020 •

edited

Loading

annanay25 commented Mar 3, 2020 •

edited

Loading