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

Wire Protocol Definition #1

Merged
merged 6 commits into from Aug 24, 2022
Merged

Wire Protocol Definition #1

merged 6 commits into from Aug 24, 2022

Conversation

bbengfort
Copy link
Contributor

@bbengfort bbengfort commented Aug 23, 2022

This PR specifies the initial protocol buffers for the Ensign protocol including the event definition and the primary RPC definitions. This is meant to be a starting place so that we can iterate on what data is actually needed, developing enums and other data structures as necessary. The question for this PR is whether we're happy with the design framework and where Ensign might go from here.

Fixes SC-8495

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #8495: Wire Protocol: Implement Protobufs.

@bbengfort bbengfort marked this pull request as ready for review August 23, 2022 17:05
@@ -0,0 +1,3 @@
package api

//go:generate protoc -I=$GOPATH/src/github.com/rotationalio/ensign/proto --go_out=. --go_opt=module=github.com/rotationalio/ensign/pkg/api --go-grpc_out=. --go-grpc_opt=module=github.com/rotationalio/ensign/pkg/api ensign/v1beta1/event.proto ensign/v1beta1/topic.proto ensign/v1beta1/ensign.proto
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This generate.go file should handle all versions of ensign (which is why it is on the outer level above the v1beta1 folder).

Copy link
Member

@rebeccabilbro rebeccabilbro left a comment

Choose a reason for hiding this comment

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

Exciting to see the vision coming together @bbengfort!

Comment on lines +89 to +92
message OpenStream {
string topic = 1;
string group = 2;
}
Copy link
Member

Choose a reason for hiding this comment

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

Is there any sense of "permission" that needs to be encoded into the OpenStream message? For instance, might this need to include information that the server can use to validate whether a would-be subscriber should be allowed access to the data in the stream, e.g. based on their region or their privacy settings? Or is permission defined only on the level of an Event (i.e. with the Region and Encryption fields). I know full geographic awareness isn't an MVP feature, so it's also fine if this is a future discussion to come back to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an excellent question and one that merits some discussion.

My current plan is to have authentication and authorization (at a broad level) as a token in the header of the requests. The user would have to supply a client ID or client ID and secret via the SDK for this to work. The token could have claims in it for fine grained permissions if we thought that would be useful.

What I hadn't considered is whether or not we want some sort of fine grained permissions system on events; e.g. read/write permissions for different event types/topic combinations? Maybe something similar to Linux file permissions where we have "owners", "group", and "other" permissions?

In terms of the region that the event originates from, there are three methods of retrieving this information:

  1. Use the users IP address and MaxMind to geolocate the sender.
  2. Use the geoping RRData method and use the location of the ensign node the user connects to
  3. Allow the user to specify what location the data is coming from (e.g. from the device's GPS)

The first and second options cannot be spoofed, so we would probably want to ensure that subscribers use one of these methods so we don't get a subscriber in Iowa saying they're in Tokyo. However, for publishers -- we may want to allow them to specify the region of the data for compliance purposes? E.g. if the publisher is in Ohio they may want to specify certain types of data is Canadian?

Clearly a lot to discuss here; it would be good if we could have a whiteboarding session on this topic once we get to it.

@bbengfort bbengfort merged commit 8f0d7e4 into main Aug 24, 2022
@bbengfort bbengfort deleted the sc-8495/wire-protocol branch August 24, 2022 16:32
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.

None yet

2 participants