-
Notifications
You must be signed in to change notification settings - Fork 314
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
Implement connection props. #3193
Conversation
I'm not entirely happy that every |
20274fa
to
bb2b4f6
Compare
bb2b4f6
to
a03109c
Compare
OK this is ready for review! It was auto-assigned to @harrishancock but he already owes me other reviews so I removed him and added @jp4a50 instead. |
b9c6609
to
dc08c4f
Compare
Last push was fixup: b9c6609 |
|
||
void visitForGc(jsg::GcVisitor& visitor) { | ||
visitor.visit(props); | ||
} |
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.
Non-blocking nit: this should likely implement visitForMemoryInfo(...)
also
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.
LGTM
src/workerd/io/frankenvalue.capnp
Outdated
# be an object. Each property in this list must have a `name`. They will be added as properties | ||
# of the object. | ||
# | ||
# If a property in the list concflicts with a property that already exists in the base value, |
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.
[typo] concflicts
JSG_RESOURCE_TYPE(ExecutionContext, CompatibilityFlags::Reader flags) { | ||
JSG_METHOD(waitUntil); | ||
JSG_METHOD(passThroughOnException); | ||
JSG_LAZY_INSTANCE_PROPERTY(props, getProps); |
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.
What's the rationale for this being read-write instead of read-only?
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.
Performance: A lazy instance property becomes a regular JS property after the first access, which can then be optimized better if it is accessed repeatedly. A regular property would have to call back to C++ for every access.
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 think the question was more: Why JSG_LAZY_INSTANCE_PROPERTY
and not JSG_LAZY_READYONLY_INSTANCE_PROPERTY
... you'd get the same lazy replacement either way. Tho, marking the field read only does have an ever so slight performance impact.
7f6d4cf
to
1744eac
Compare
Currently, `ServiceDesignator` can only resolve to one of a fixed set of objects that already exist, but we want to make it a little bit more dynamic in later commits.
This will allow us to extend it to support connection metadata, which might be different for each binding pointing to the same entrypoint.
See comments for description.
This adds `ctx.props` to the `ctx` object given to `WorkerEntrypoint`s. The property receives metadata about the particular service binding over which the entrypoint was invoked. ``` class MyEntrypoint extends WorkerEntrypoint { foo() { console.log("called by: " + this.ctx.props.caller); } } ``` Service binding declarations in the workerd config may specify what metadata to pass: ``` bindings = [ ( name = "FOO", service = ( name = "my-service", entrypoint = "MyEntrypoint", props = ( json = `{"caller": "my-calling-service"} ) ) ) ] ``` Note that "caller" is just an example. The props can contain anything. Use cases include: * Authentication of the caller's identity. * Authorization / permissions (independent of caller identity). * Specifying a particular resource. For example, if the `WorkerEntrypoint` represents a chat room, `props.roomId` could be the ID of the specific chat room to access. This allows service bindings to implement a deeper capability-based security model, where bindings point to specific resources with specific permissions, instead of general APIs. On Cloudflare, only users who have permission to modify your worker will have permission to create a binding containing arbitrary metadata. Meanwhile we will be creating a mechanism by which you can grant a service binding to your worker to someone, but where you specify the metadata. Thus, you can use the metadata to authenticate requests, without the need for any cryptography.
1744eac
to
c2cce70
Compare
// | ||
// This is called `set` because the new property will override any existing property with the | ||
// same name, but note that this strictly appends content. The replacement happens only when the | ||
// Frankenvalue is finally converted to JS. |
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.
Non-blocking nit: It's likely worth documenting that toJs(...)
will throw if there are properties added but the realized JS value is not actually an object.
struct Json { | ||
kj::String json; | ||
}; | ||
struct V8Serialized { | ||
kj::Array<byte> data; | ||
}; |
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.
Non-blocking nit: Since these are just single valued, structs this might be a bit cleaner:
using Json = kj::String;
using V8Serialized = kj::Array<kj::byte>;
This adds
ctx.props
to thectx
object given toWorkerEntrypoint
s. The property receives metadata about the particular service binding over which the entrypoint was invoked.Service binding declarations in the workerd config may specify what metadata to pass:
Note that "caller" is just an example. The props can contain anything. Use cases include:
WorkerEntrypoint
represents a chat room,props.roomId
could be the ID of the specific chat room to access.This allows service bindings to implement a deeper capability-based security model, where bindings point to specific resources with specific permissions, instead of general APIs.
On Cloudflare, only users who have permission to modify your worker will have permission to create a binding containing arbitrary metadata. Meanwhile we will be creating a mechanism by which you can grant a service binding to your worker to someone, but where you specify the metadata. Thus, you can use the metadata to authenticate requests, without the need for any cryptography.