Skip to content
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

cdc: Allow webhook sink to provide client certificates to the remote webhook server #74645

Merged
merged 1 commit into from Jan 14, 2022

Conversation

sherman-grewal
Copy link
Contributor

@sherman-grewal sherman-grewal commented Jan 10, 2022

Summary

Allow the webhook sink to provide client certificates to the remote webhook server.

Resolves #74230

Testing

  1. Install certstrap to help with creating the CA and other required certificates
$ brew install certstrap
  1. Create a CA
$ certstrap init --common-name "ExampleCA"
Enter passphrase (empty for no passphrase):
Enter same passphrase again:
Created out/ExampleCA.key
Created out/ExampleCA.crt
Created out/ExampleCA.crl
  1. Create the certificate for the server
$ certstrap request-cert --domain  "localhost"
Enter passphrase (empty for no passphrase):
Enter same passphrase again:
Created out/localhost.key
Created out/localhost.csr
  1. Create the certificate for the client
$ certstrap request-cert --domain  "client"
Enter passphrase (empty for no passphrase):
Enter same passphrase again:
Created out/client.key
Created out/client.csr
  1. Sign the certificate for the server
$ certstrap sign localhost --CA ExampleCA
Created out/localhost.crt from out/localhost.csr signed by out/ExampleCA.key
  1. Sign the certificate for the client
$ certstrap sign client --CA ExampleCA 
Created out/client.crt from out/client.csr signed by out/ExampleCA.key
  1. Pull the server code in this file, and then run the following in your console
$ go build server.go
$ ./server -host "localhost" -srvcert "<path-to-server-cert>" -cacert "<path-to-ca-cert>" \
-srvkey "<path-to-server-key>" -port 3000 -certopt 4
  1. Follow the steps to creating a webhook sink here, but use the following SQL command to create the changefeed
CREATE CHANGEFEED
    FOR TABLE movr.vehicles
    INTO 'webhook-https://localhost:3000?insecure_tls_skip_verify=true&client_cert=<client-cert>&client_key=<client-key>'
    WITH updated;

Note: The client_cert and client_key must all be passed directly as base64-encoded values

Release notes (enterprise change): Client certificates may now be provided for the webhook changefeed sink.

@sherman-grewal sherman-grewal requested a review from a team as a code owner January 10, 2022 20:02
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@sherman-grewal sherman-grewal marked this pull request as draft January 10, 2022 20:02
Copy link
Contributor

@miretskiy miretskiy left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @HonoreDB and @sherman-grewal)


-- commits, line 5 at r1:
I think you need to amend your change to include some some additional information.
In particular, we should definitely have a release note here.


pkg/ccl/changefeedccl/sink_webhook.go, line 362 at r1 (raw file):

	if err := u.decodeBase64(changefeedbase.SinkParamClientKey, &dialConfig.clientKey); err != nil {
		return nil, err
	}

Can we expand e.g. webhookFeedFactory (in testfeed_test.go) to also support these params?
For example, we could, perhaps add an option to webhookTestWithOptions function to also specify if we want "secure" connection, and have webhookFeedFactory start a secure http server for message ingestion?

@sherman-grewal sherman-grewal marked this pull request as ready for review January 14, 2022 15:12
Copy link
Contributor

@HonoreDB HonoreDB left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @HonoreDB and @sherman-grewal)

remote webhook server

Release note (enterprise change): Client certificates may now be provided
for the webhook changefeed sink.
@sherman-grewal
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jan 14, 2022

Build succeeded:

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.

cdc: Allow webhook-https scheme to send client certificates
4 participants