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

MQTT5 Protocol spec #75

Merged
merged 10 commits into from
Apr 9, 2024

Conversation

devkelley
Copy link
Contributor

This is a draft of how a uTransport would communicate over MQTT:

I would envision that there is a device specific mqtt broker and a cloud based mqtt broker (or more). If a uMessage is being routed to the cloud only the uAuthority information is really needed in the topic. Once the uMessage reaches the correct uDevice, then the header is processed to get the uEntity and uResource from the sink uuri. The uMessage is then passed over the local mqtt broker to the correct component.

Things to discuss:

  • Should microURI be used anywhere here?
  • The uMessage payload is simply the uMessage protobuf object written to bytes. (vec)

@lynn2009
Copy link

lynn2009 commented Mar 6, 2024

"the uAuthority information is really needed in the topic." -- MQTT broker provides multiple ways to distribute messages (MQTT pub/sub, streaming bridge etc), the streaming bridge may also use other uAttributes, for example proriity.

@devkelley
Copy link
Contributor Author

"the uAuthority information is really needed in the topic." -- MQTT broker provides multiple ways to distribute messages (MQTT pub/sub, streaming bridge etc), the streaming bridge may also use other uAttributes, for example proriity.

Do you think other uAttributes should be present in the topic? The current proposal has several uAttributes being passed as custom header props. Would that work for something like the streaming bridge?

up-l1/mqtt.adoc Outdated Show resolved Hide resolved
up-l1/mqtt.adoc Outdated Show resolved Hide resolved
up-l1/mqtt.adoc Outdated Show resolved Hide resolved
up-l1/mqtt.adoc Outdated Show resolved Hide resolved
up-l1/mqtt.adoc Outdated Show resolved Hide resolved
@PLeVasseur
Copy link
Contributor

@devkelley -- I'm not familiar with MQTT5 yet, but in order to support the use-case of use by a uStreamer, could you add into the spec how we'd handle the situation outlined in the Zenoh spec for URI Mapping in the case that a uStreamer wants to subscribe to "everything" from a particular UAuthority?

In particular, talking with @evshary, we came up with this:
image
to allow creation of a Zenoh KeyExpr which would be able to "see" anything going on a certain UAuthority.

And then CY wrote up-client-zenoh-rust in such a way that that special UUri containing only a UAuthority would result in registering and unregistering from everything as appropriate (publish, notification, request, response). For reference you could take a look here.

Hope that all made sense -- if not we can chat here or on Slack 🙂

@devkelley
Copy link
Contributor Author

devkelley commented Mar 19, 2024

@devkelley -- I'm not familiar with MQTT5 yet, but in order to support the use-case of use by a uStreamer, could you add into the spec how we'd handle the situation outlined in the Zenoh spec for URI Mapping in the case that a uStreamer wants to subscribe to "everything" from a particular UAuthority?

In particular, talking with @evshary, we came up with this: image to allow creation of a Zenoh KeyExpr which would be able to "see" anything going on a certain UAuthority.

And then CY wrote up-client-zenoh-rust in such a way that that special UUri containing only a UAuthority would result in registering and unregistering from everything as appropriate (publish, notification, request, response). For reference you could take a look here.

Hope that all made sense -- if not we can chat here or on Slack 🙂

Hi Peter, this is a good question! Mqtt has two wildcard characters that allow for receiving messages from multiple topics. + allows a subscriber to place a wildcard a specific level (ie. d2c/192.168.0.0/3/+/123 would allow a subscription to all versions of a specific entity). # allows for a subscription to all levels of a topic and is placed at the end (ie. d2c/192.168.0.0/# will allow a subscription to all messages from that authority). I think we would want to use the format of d2c/192.168.0.0/+/+/+ as it will constrain the messages to that level of topic. I will add it to the spec in the case a uuri is passed with just uAuthority.

I just added a description for this scenario, take a look when you get a chance @PLeVasseur

up-l1/mqtt.adoc Outdated Show resolved Hide resolved
up-l1/mqtt.adoc Show resolved Hide resolved
up-l1/mqtt.adoc Show resolved Hide resolved
up-l1/mqtt.adoc Outdated Show resolved Hide resolved
@stevenhartley stevenhartley added this to the v1.0.0-alpha.1 milestone Apr 3, 2024
Copy link
Contributor

@stevenhartley stevenhartley left a comment

Choose a reason for hiding this comment

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

@devkelley with the last change I think we are good to merge in the initial proposal and we can adapt afterwards. Lets sync up on the progress of up-client-mqtt5-rust later this week

up-l1/mqtt.adoc Outdated Show resolved Hide resolved
@stevenhartley stevenhartley added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 4, 2024
Put a link to the short URI specifications
Copy link
Contributor

@stevenhartley stevenhartley left a comment

Choose a reason for hiding this comment

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

I'm OK with the changes, I merged the minor comment I had

@devkelley devkelley marked this pull request as ready for review April 9, 2024 15:37
@stevenhartley stevenhartley merged commit ca3e8a3 into eclipse-uprotocol:main Apr 9, 2024
1 check passed

Subscribe to all messages to the current Authority

- `{UUri.UAuthority.number}/+/+/+` where `UAttributes.type != Publish`
Copy link

Choose a reason for hiding this comment

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

Filtering based on UserProperty is not inherently supported by MQTT's native subscribe function. Instead, this type of filtering have to be implemented at the application level.
whether it's the cloud subscribing to messages from a vehicle or a vehicle subscribing to cloud messages, this approach is inefficient. specially on Cloud side.

Copy link

Choose a reason for hiding this comment

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

This also makes it challenging to implement security controls at the MQTT topic level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, what do you think about appending the message type to the topic?

Copy link
Contributor

Choose a reason for hiding this comment

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

Filtering based on UserProperty is not inherently supported by MQTT's native subscribe function. Instead, this type of filtering have to be implemented at the application level. whether it's the cloud subscribing to messages from a vehicle or a vehicle subscribing to cloud messages, this approach is inefficient. specially on Cloud side.

@lynn2009 Right now the purpose of this is only for D2C and C2D and once the message is in the cloud, it will use the clouse native uTransport and uSubscription infrastructure (i.e. Eventhub or something else).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, what do you think about appending the message type to the topic?

Or maybe even put it to the front as the most discriminant field.
Filtering on all Publish messages: pub.v1/+/+/+/+
Filtering on all messages destined to myself: +/{my.authority.id}/+/+/+

Choose a reason for hiding this comment

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

Prefer to have the following patterns:
rpc/{authority}/+/+/+
notf/{authority}/+/+/+
pub/{authority}/+/+/+

This approach enables subscribers to subscribe without needing application-level filtering, making it simpler to implement authorization controls. For instance, the uAuthority would be restricted to subscribing only to topics under rpc/<UUri.uAuthority.number|name>/# and notf/<UUri.uAuthority.number|name>/#, but they would be allowed to publish to topics under pub/<UUri.uAuthority.number|name>/#.

Subscribe to all messages from a different Authority

- `{UUri.UAuthority.number}/+/+/+` where `UAttributes.type == Publish`

Copy link

Choose a reason for hiding this comment

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

See comments as above


Subscribe to all messages from a different Authority

- `{UUri.UAuthority.number}/+/+/+` where `UAttributes.type == Publish`
Copy link

Choose a reason for hiding this comment

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

If the subscribe operation has to use the UUri.uAuthority.number, it is not suitable for cloud to subscribe vehicle messages, given that the VIN is a 17-character string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lynn2009 are you saying that the vin is too long for the topic name? I am unsure what other identifier to use, other than using the micro uri format for the topic name

Copy link
Contributor

Choose a reason for hiding this comment

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

UAuthority.number is either id (VIN) or IP address.

Copy link
Contributor

@sophokles73 sophokles73 Apr 10, 2024

Choose a reason for hiding this comment

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

IMHO @lynn2009 refers to the fact that the cloud component that wants to receive all Publish messages from all vehicles would need to use a separate topic filter per subscribed topic/vehicle (vin1/entity.id/1/resource.id, vin2/entity.id/1/resource.id, etc) as opposed to only one filter (pub.v1/+/+/+/+) for Publish messages originating from any vehicle ...

| UserProperty
| ("11", "{UAttribute.traceParent}") # Optional traceparent
|===

Copy link

Choose a reason for hiding this comment

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

Can we have just "id", "type" instead of numerics in the key?

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommended we use numbers in lieu of strings so that we reduce the number of bytes that we send over the wire and given the numbers are fixed in uattributes.proto anyways.

Copy link
Contributor

Choose a reason for hiding this comment

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

If message size is the main concern, then why don't we simply put the protobuf serialization of UAttributes in a single property like we do in the zenoh transport?

Copy link

Choose a reason for hiding this comment

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

Headers should not have to be deserialized for routing purposes or other purposes.

== Specifications

=== UAttribute Mapping

MQTT 5 supports custom header fields, and we leverage this to map the UAttribute values into the MQTT header.
Copy link
Contributor

Choose a reason for hiding this comment

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

if this specification covers MQTT 5 only, then IMHO the document should be renamed to mqtt5.adoc and this fact should be made explicit in the Overview section already.

| UserProperty
| ("11", "{UAttribute.traceParent}") # Optional traceparent
|===

Copy link
Contributor

Choose a reason for hiding this comment

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

If message size is the main concern, then why don't we simply put the protobuf serialization of UAttributes in a single property like we do in the zenoh transport?


Subscribe to all messages to the current Authority

- `{UUri.UAuthority.number}/+/+/+` where `UAttributes.type != Publish`
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, what do you think about appending the message type to the topic?

Or maybe even put it to the front as the most discriminant field.
Filtering on all Publish messages: pub.v1/+/+/+/+
Filtering on all messages destined to myself: +/{my.authority.id}/+/+/+


Subscribe to all messages from a different Authority

- `{UUri.UAuthority.number}/+/+/+` where `UAttributes.type == Publish`
Copy link
Contributor

@sophokles73 sophokles73 Apr 10, 2024

Choose a reason for hiding this comment

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

IMHO @lynn2009 refers to the fact that the cloud component that wants to receive all Publish messages from all vehicles would need to use a separate topic filter per subscribed topic/vehicle (vin1/entity.id/1/resource.id, vin2/entity.id/1/resource.id, etc) as opposed to only one filter (pub.v1/+/+/+/+) for Publish messages originating from any vehicle ...

@mnemili
Copy link

mnemili commented Apr 10, 2024

@stevenhartley Why cant we do /{vin}/body.access/{v}/rpc.unlock instead of /{vin}/body.access/{v}/rpc.command? We can verify the permission of the rpc without opening the payload..


- `{UUri.UAuthority.number}/+/+/+` where `UAttributes.type == Publish`

When sending a message, the source UUri is used for Publish message types and sink UUri is used for all other message types.
Copy link

Choose a reason for hiding this comment

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

@stevenhartley If we do /{vin}/body.access/{v}/rpc.command, we cant verify permission without opening the payload. Instead if we have /{vin}/body.access/{v}/rpc.unlock, that would help - lot of other MQTT brokers had the same opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants