Skip to content

changefeedccl: allow base64-encoded client certificate#39832

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
rolandcrosby:cdc-tls-cert
Sep 3, 2019
Merged

changefeedccl: allow base64-encoded client certificate#39832
craig[bot] merged 1 commit intocockroachdb:masterfrom
rolandcrosby:cdc-tls-cert

Conversation

@rolandcrosby
Copy link
Copy Markdown

Adds client_cert and client_key options to the kafka://
changefeed URI scheme. Works like the existing ca_cert option:
the user base64-encodes the contents of a PEM certificate and
private key, and passes those base64 values as parameters in the
Kafka URI.

Fixes #39817.

Release note (enterprise change): Client certificates are now
supported for Kafka changefeed authentication.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@mberhault
Copy link
Copy Markdown
Contributor

Nice. Does it work against a broker with TLS enabled and cert-based auth?

Copy link
Copy Markdown
Contributor

@danhhz danhhz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! :lgtm: assuming you've tested this locally and seen it work end-to-end

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @danhhz and @mberhault)

@rolandcrosby
Copy link
Copy Markdown
Author

Yep, I tested this against a broker with the ssl.client.auth=required option enabled, and it worked correctly. Also verified that the server did not allow me to authenticate if I didn't specify the client certificate, and used openssl s_server to verify that CockroachDB was passing the expected certificate.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Aug 22, 2019

Build failed

Adds `client_cert` and `client_key` options to the `kafka://`
changefeed URI scheme. Works like the existing `ca_cert` option:
the user base64-encodes the contents of a PEM certificate and
private key, and passes those base64 values as parameters in the
Kafka URI.

Fixes #39817.

Release note (enterprise change): Client certificates are now
supported for Kafka changefeed authentication.
@rolandcrosby
Copy link
Copy Markdown
Author

bors r+

craig bot pushed a commit that referenced this pull request Sep 3, 2019
39832: changefeedccl: allow base64-encoded client certificate r=rolandcrosby a=rolandcrosby

Adds `client_cert` and `client_key` options to the `kafka://`
changefeed URI scheme. Works like the existing `ca_cert` option:
the user base64-encodes the contents of a PEM certificate and
private key, and passes those base64 values as parameters in the
Kafka URI.

Fixes #39817.

Release note (enterprise change): Client certificates are now
supported for Kafka changefeed authentication.

39838: storage: fix stress flake r=ajwerner a=ajwerner

While stressing storage for #39643 I encountered
TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic
failing under stress. It also complained about the parent testing.T
having been failed when then using a child so fixed that too though
in a perhaps messy way.

```
--- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic (2.29s)
    --- PASS: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/initial_run (0.14s)
    client_test.go:1280: [NotLeaseHolderError] r1: replica (n1,s1):1 not lease holder; current lease is repl=(n3,s3):3 seq=4 start=1566532897.322355813,1 exp=1566532898.378802707,0 pro=1566532897.478824123,0
    --- FAIL: TestDefaultConnectionDisruptionDoesNotInterfereWithSystemTraffic/after_restart (1.02s)
        testing.go:820: test executed panic(nil) or runtime.Goexit: subtest may have called FailNow on a parent test
```

Release note: None

Co-authored-by: Roland Crosby <roland@cockroachlabs.com>
Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 3, 2019

Build succeeded

@craig craig bot merged commit 719105d into cockroachdb:master Sep 3, 2019
@rolandcrosby rolandcrosby deleted the cdc-tls-cert branch September 3, 2019 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

changefeedccl: support client certificates for kafka sink

4 participants