-
Notifications
You must be signed in to change notification settings - Fork 621
Add Hyperdrive bindings #1266
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
Add Hyperdrive bindings #1266
Changes from all commits
896f2fc
6ea42ca
4401e88
cf1191e
d245f99
57ce723
e06e9eb
50c27e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| // Copyright (c) 2023 Cloudflare, Inc. | ||
| // Licensed under the Apache 2.0 license found in the LICENSE file or at: | ||
| // https://opensource.org/licenses/Apache-2.0 | ||
|
|
||
| #include "hyperdrive.h" | ||
| #include <openssl/rand.h> | ||
| #include <cstdint> | ||
| #include <kj/compat/http.h> | ||
| #include <kj/encoding.h> | ||
| #include "sockets.h" | ||
| #include "global-scope.h" | ||
| #include <kj/string.h> | ||
| #include <workerd/util/uuid.h> | ||
|
|
||
| namespace workerd::api { | ||
| Hyperdrive::Hyperdrive(uint clientIndex, kj::String database, | ||
| kj::String user, kj::String password, kj::String scheme) | ||
| : clientIndex(clientIndex), database(kj::mv(database)), | ||
| user(kj::mv(user)), password(kj::mv(password)), scheme(kj::mv(scheme)) { | ||
| kj::byte randomBytes[16]; | ||
| KJ_ASSERT(RAND_bytes(randomBytes, sizeof(randomBytes)) == 1); | ||
| randomHost = kj::str(kj::encodeHex(randomBytes), ".hyperdrive.local"); | ||
| } | ||
|
|
||
| jsg::Ref<Socket> Hyperdrive::connect(jsg::Lock& js) { | ||
| auto connPromise = connectToDb(); | ||
|
|
||
| auto paf = kj::newPromiseAndFulfiller<kj::Maybe<kj::Exception>>(); | ||
| auto conn = kj::newPromisedStream(connPromise.then( | ||
| [&f = *paf.fulfiller](kj::Own<kj::AsyncIoStream> stream) { | ||
| f.fulfill(kj::none); | ||
| return kj::mv(stream); | ||
| }, [&f = *paf.fulfiller](kj::Exception e) { | ||
| KJ_LOG(WARNING, "failed to connect to local database", e); | ||
| f.fulfill(kj::cp(e)); | ||
| return kj::mv(e); | ||
| }).attach(kj::mv(paf.fulfiller))); | ||
|
|
||
| // TODO(someday): Support TLS? It's not at all necessary since we're connecting locally, but | ||
| // some users may want it anyway. | ||
| auto nullTlsStarter = kj::heap<kj::TlsStarterCallback>(); | ||
| auto sock = setupSocket(js, kj::mv(conn), kj::none, kj::mv(nullTlsStarter), | ||
| false, kj::str(this->randomHost), false); | ||
| sock->handleProxyStatus(js, kj::mv(paf.promise)); | ||
| return sock; | ||
| } | ||
|
|
||
| kj::StringPtr Hyperdrive::getDatabase() { | ||
| return this->database; | ||
| } | ||
|
|
||
| kj::StringPtr Hyperdrive::getUser() { | ||
| return this->user; | ||
| } | ||
| kj::StringPtr Hyperdrive::getPassword() { | ||
| return this->password; | ||
| } | ||
|
|
||
| kj::StringPtr Hyperdrive::getScheme() { | ||
| return this->scheme; | ||
| } | ||
|
|
||
| kj::StringPtr Hyperdrive::getHost() { | ||
| if (!registeredConnectOverride) { | ||
| IoContext::current().getCurrentLock().getWorker().setConnectOverride( | ||
| kj::str(this->randomHost, ":", getPort()), KJ_BIND_METHOD(*this, connect)); | ||
| registeredConnectOverride = true; | ||
| } | ||
| return this->randomHost; | ||
| } | ||
|
|
||
| // Always returns the default postgres port | ||
| uint16_t Hyperdrive::getPort() { | ||
| return 5432; | ||
|
jasnell marked this conversation as resolved.
|
||
| } | ||
|
|
||
| kj::String Hyperdrive::getConnectionString() { | ||
| return kj::str(getScheme(), "://", getUser(), ":", getPassword(), "@", getHost(), ":", getPort(), | ||
| "/", getDatabase(), "?sslmode=disable"); | ||
| } | ||
|
Comment on lines
+77
to
+80
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tmthecoder @a-robinson this hardcodes
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If you want to support remote databases that require SSL but also local DBs that don't support it, it sounds like what we want for local mode is It should be possible to vary the default sslmode in workerd compared to on the edge, but I'd suggest bringing it up internally either in chat or as a ticket so we don't lose track of it. And I wouldn't expect Tejas to respond given that his internship ended last year :) |
||
|
|
||
| kj::Promise<kj::Own<kj::AsyncIoStream>> Hyperdrive::connectToDb() { | ||
| auto& context = IoContext::current(); | ||
| auto service = context.getSubrequestChannel(this->clientIndex, | ||
| true, kj::none, "hyperdrive_dev"_kjc); | ||
|
|
||
| kj::HttpHeaderTable headerTable; | ||
| kj::HttpHeaders headers(headerTable); | ||
|
|
||
| auto connectReq = kj::newHttpClient(*service)->connect( | ||
| kj::str(getHost(), ":", getPort()), headers, kj::HttpConnectSettings{}); | ||
|
|
||
| auto status = co_await connectReq.status; | ||
|
|
||
| if (status.statusCode >= 200 && status.statusCode < 300) { | ||
| co_return kj::mv(connectReq.connection); | ||
| } | ||
|
|
||
| KJ_IF_SOME(e, status.errorBody) { | ||
| try { | ||
| auto errorBody = co_await e->readAllText(); | ||
| kj::throwFatalException(KJ_EXCEPTION( | ||
| FAILED, kj::str("unexpected error connecting to database: ", errorBody))); | ||
| } catch (const kj::Exception& e) { | ||
| kj::throwFatalException( | ||
| KJ_EXCEPTION(FAILED, kj::str("unexpected error connecting to database " | ||
| "and couldn't read error details: ", e))); | ||
| } | ||
| } | ||
| else { | ||
| kj::throwFatalException( | ||
| KJ_EXCEPTION(FAILED, kj::str("unexpected error connecting to database: ", | ||
| status.statusText))); | ||
| } | ||
| } | ||
| } // namespace workerd::api::public_beta | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // Copyright (c) 2023 Cloudflare, Inc. | ||
| // Licensed under the Apache 2.0 license found in the LICENSE file or at: | ||
| // https://opensource.org/licenses/Apache-2.0 | ||
|
|
||
| #pragma once | ||
|
|
||
| #include "streams.h" | ||
| #include <capnp/compat/json.h> | ||
| #include <cstdint> | ||
| #include <kj/common.h> | ||
| #include <kj/async-io.h> | ||
| #include <workerd/jsg/jsg.h> | ||
| #include <workerd/util/http-util.h> | ||
|
|
||
| namespace workerd::api { | ||
|
|
||
| // A Hyperdrive resource for development integrations. | ||
| // | ||
| // Provides the same interface as Hyperdrive while sending connection | ||
| // traffic directly to postgres | ||
| class Hyperdrive : public jsg::Object { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sharing the same class name both here and in the internal repo had me worried, but I guess it works judging by the CI on your internal PR ¯\(ツ)/¯ |
||
| public: | ||
| // `clientIndex` is what to pass to IoContext::getHttpClient() to get an HttpClient | ||
| // representing this namespace. | ||
| explicit Hyperdrive(uint clientIndex, kj::String database, | ||
| kj::String user, kj::String password, kj::String scheme); | ||
| jsg::Ref<Socket> connect(jsg::Lock& js); | ||
| kj::StringPtr getDatabase(); | ||
| kj::StringPtr getUser(); | ||
| kj::StringPtr getPassword(); | ||
| kj::StringPtr getScheme(); | ||
|
|
||
| kj::StringPtr getHost(); | ||
| uint16_t getPort(); | ||
|
|
||
| kj::String getConnectionString(); | ||
|
|
||
| JSG_RESOURCE_TYPE(Hyperdrive, CompatibilityFlags::Reader flags) { | ||
| JSG_LAZY_READONLY_INSTANCE_PROPERTY(database, getDatabase); | ||
| JSG_LAZY_READONLY_INSTANCE_PROPERTY(user, getUser); | ||
| JSG_LAZY_READONLY_INSTANCE_PROPERTY(password, getPassword); | ||
| JSG_LAZY_READONLY_INSTANCE_PROPERTY(host, getHost); | ||
| JSG_LAZY_READONLY_INSTANCE_PROPERTY(port, getPort); | ||
| JSG_LAZY_READONLY_INSTANCE_PROPERTY(connectionString, getConnectionString); | ||
|
|
||
| JSG_METHOD(connect); | ||
| } | ||
|
|
||
| private: | ||
| uint clientIndex; | ||
| kj::String randomHost; | ||
| kj::String database; | ||
| kj::String user; | ||
| kj::String password; | ||
| kj::String scheme; | ||
| bool registeredConnectOverride = false; | ||
|
|
||
| kj::Promise<kj::Own<kj::AsyncIoStream>> connectToDb(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style nit, but it'd be nice to put an empty line in between the class's member variables and any private functions just to more cleanly visually separate them. When scanning over this I initially was trying to figure out why we needed a |
||
| }; | ||
| #define EW_HYPERDRIVE_ISOLATE_TYPES \ | ||
| api::Hyperdrive | ||
| } // namespace workerd::api | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1013,6 +1013,10 @@ KJ_TEST("Server: capability bindings") { | |
| ` items.push(await (await env.r2.get("baz")).text()); | ||
| ` await env.queue.send("hello"); | ||
| ` items.push("Hello from Queue\n"); | ||
| ` const connection = await env.hyperdrive.connect(); | ||
| ` const encoded = new TextEncoder().encode("hyperdrive-test"); | ||
| ` await connection.writable.getWriter().write(new Uint8Array(encoded)); | ||
| ` items.push(`Hello from Hyperdrive(${env.hyperdrive.user})\n`); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we have a test that actually calls Have you at least confirmed locally that it's able to successfully connect to a real DB? |
||
| ` return new Response(items.join("")); | ||
| ` } | ||
| `} | ||
|
|
@@ -1030,6 +1034,15 @@ KJ_TEST("Server: capability bindings") { | |
| ), | ||
| ( name = "queue", | ||
| queue = "queue-outbound" | ||
| ), | ||
| ( name = "hyperdrive", | ||
| hyperdrive = ( | ||
| designator = "hyperdrive-outbound", | ||
| database = "test-db", | ||
| user = "test-user", | ||
| password = "test-password", | ||
| scheme = "postgresql" | ||
| ) | ||
| ) | ||
| ] | ||
| ) | ||
|
|
@@ -1038,6 +1051,10 @@ KJ_TEST("Server: capability bindings") { | |
| ( name = "kv-outbound", external = "kv-host" ), | ||
| ( name = "r2-outbound", external = "r2-host" ), | ||
| ( name = "queue-outbound", external = "queue-host" ), | ||
| ( name = "hyperdrive-outbound", external = ( | ||
| address = "hyperdrive-host", | ||
| tcp = () | ||
| )) | ||
| ], | ||
| sockets = [ | ||
| ( name = "main", | ||
|
|
@@ -1119,11 +1136,16 @@ KJ_TEST("Server: capability bindings") { | |
| )"_blockquote); | ||
| } | ||
|
|
||
| { | ||
| auto subreq = test.receiveSubrequest("hyperdrive-host"); | ||
| subreq.recv("hyperdrive-test"); | ||
| } | ||
| conn.recvHttp200(R"( | ||
| Hello from HTTP | ||
| Hello from KV | ||
| Hello from R2 | ||
| Hello from Queue | ||
| Hello from Hyperdrive(test-user) | ||
| )"_blockquote); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how much it really matters, but this file is missing a copyright header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should really just have a github action that checks this, adds the headers, and opens an automated pr adding them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we still need to add a copyright header here