design icon indicating copy to clipboard operation
design copied to clipboard

Design document for Node Interface Definition Language (IDL)

Open artivis opened this issue 5 years ago • 26 comments

Design document for the 'Node Interface Definition Language (IDL)',

a simple and standardized manner to export the complete interface (action/message/parameter/service) of node(s) in a package.

Initially discussed within the Security WG, this proposal is not security-related only. Security features and tooling building on top of this will be proposed in a subsequent document.

FYI @ruffsl @jacobperron @thomas-moulard @kyrofa

artivis avatar Nov 25 '19 16:11 artivis

@jacobperron Thanks for this initial review. I'll address it asap.

artivis avatar Nov 25 '19 19:11 artivis

In addition to my comments below, it would also be nice to add an "alternatives" section where we document other options considered (if any) related to the node IDL.

I'm not sure of what alternatives we are talking about, alternative ideas to the concept presented here (explicitly exported node api) or alternative implementation design of the IDL or something else ?

artivis avatar Nov 25 '19 19:11 artivis

Using IDL for this is imo a really confusing choice. ROS 2 already used the OMG IDL spec for defining message definitions. Therefore I strongly suggest a different abbreviation / terminology for this.

We were expecting this particular concern to be raised as we share it too. However we kept this terminology - 'Node IDL' - as it best describes the proposal. We're open to suggestions tho.

artivis avatar Nov 25 '19 20:11 artivis

Also the "definition language" part of IDL doesn't make sense since the information is described in XML and not like message interfaces in an actual "definition language" (specific syntax).

We did tripped a little over the IDL acronym. Depending on the source the D letter stands for different things and we started this document with Description in mind rather than Definition.

Maybe "Node API definition / descriptor"? (We can figure that out later though to not derail the discussion with a bike shed topic like this.)

Agreed.

artivis avatar Nov 25 '19 20:11 artivis

There is something I haven't seen made explicit yet, but which I think we should. This is about where the definitions are gathered and what they mean, semantically.

The current proposal says that a package can define the interfaces of the nodes it includes. Accepting that this can be done using xincludes to include multiple XML files that define individual nodes, this is what I think is semantically being created by the current proposal:

  • A package is a namespace, defined by a directory and a package-level IDL file
  • Within the package namespace are one or more object types, defined in the package-level IDL file or with individual files collected into that (i.e. the IDL descriptions of nodes)
  • Within the package namespace are instances of those types, defined by source code that implements them (i.e. the node implementations)
  • Within the package namespace are one or more configuration set types, defined in the package-level IDL file or with individual files collected into that (i.e. the IDL descriptions of parameters)
  • Within the package namespace are uses of the configuration set types (i.e. the actual declaration and use of parameters by nodes)
  • A node from one package could declare that it implements a node interface defined in a Node IDL in another package.

I may have missed some parts by I think it gets my point across.

At the source level, the "package namespace" concept does not really exist in current ROS. However it does exist at the tooling level, in tools such as ros2 run and launch.

What I want to be clear on is if we are all happy making the "package namespace" concept much more formal than it currently is and extend it throughout the run-time ROS system. Having run-time checks of "package interfaces" will do that because it makes the package a run-time entity.

Personally I am fine with that, I think it's a natural extension of where things are currently and there are benefits to doing so for identifying specific node implementations, even in cases where two nodes from two packages implement the exact same interface declared in the exact same Node IDL file.

gbiggs avatar Dec 17 '19 02:12 gbiggs

Would the Node IDL make it possible to create some sort of rosbridge between nodes of different ROS 2 distributions?

It could be very liberating in large heterogeneous systems to allow nodes running on different distributions to communicate with each other. Right now if one node is stuck on an old distribution, for one reason or another like being proprietary code, it cannot participate in the system unless all other nodes are held or regressed back to that old distribution. Even if it was less efficient, a bridge could allow most nodes to upgrade at will while still letting old nodes participate in the system.

peterpolidoro avatar Jan 10 '20 16:01 peterpolidoro

That's an interesting idea, @peterpolidoro. Off-hand I'd say it could help with it inasmuch as the node in question might change what it calls a given topic across distros, but the IDL doesn't dive into message contents, so it would be fairly limited. Specifically, if the message used by the given topic changed its structure, the IDL can't really help you there.

kyrofa avatar Jan 10 '20 17:01 kyrofa

but the IDL doesn't dive into message contents

Perhaps the message contents are outside the scope of the Node IDL, but that information could be very valuable in a variety of situations. It could allow you to verify, for safety or consistency, that a message is defined as you expect. It might also be used to create some sort of plug-and-play interface that allows you to first ask a node to give you its IDL and use that to generate the messages necessary to communicate with that node so you do not need to access the original package that defined the node messages. It might be nice at times to be able to get everything you need to communicate with a node locally from the node itself, instead of relying on a connection to the outside world and finding the proper interface version for that node.

peterpolidoro avatar Jan 14 '20 19:01 peterpolidoro

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/guidelines-on-how-to-architect-ros-based-systems/12641/17

ros-discourse avatar Feb 12 '20 16:02 ros-discourse

The proposal has been updated to match the implementation in https://github.com/ubuntu-robotics/nodl .

Changes of note:

  • Added "executable" as an attribute at the node level, specifying the executable containing the node (needed for launch integration)
  • Export mechanism specified as using the ament index plugin system
  • Added details of how to export NoDL files from a package using CMakeLists.txt or setup.py
  • Removed QoS related functionality, can be proposed later as an extension/future revision to NoDL
  • Added mention of a formal ".xsd" schema that is provided in the NoDL package
  • "Node IDL" changed to "NoDL" or "Node Definition Language" throughout

Arnatious avatar Jul 13 '20 18:07 Arnatious

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2020-07-16/15468/1

ros-discourse avatar Jul 18 '20 01:07 ros-discourse

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/matching-the-dynamic-nature-of-ros-to-the-static-nature-of-dds-security/15653/1

ros-discourse avatar Jul 28 '20 22:07 ros-discourse

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/announcing-a-ros-package-generator/16317/3

ros-discourse avatar Sep 09 '20 19:09 ros-discourse

@artivis What's the status of this design? I see there has been lots of discussion, though no approvals as of yet. Is there more work that needs to be done here?

clalancette avatar Sep 24 '20 17:09 clalancette

@artivis What's the status of this design? I see there has been lots of discussion, though no approvals as of yet. Is there more work that needs to be done here?

Let me forward this to @Arnatious who has picked up this up.

artivis avatar Sep 24 '20 18:09 artivis

@artivis What's the status of this design? I see there has been lots of discussion, though no approvals as of yet. Is there more work that needs to be done here?

There's been no comment since the last revision of the nodl schema, which added the role attribute. I'm taking that as a good sign.

We have a reference implementation in https://github.com/ubuntu-robotics/nodl, and are nearly done with a simple launch_ros integration pending some changes in SROS2. I think this proposal is ready to move ahead.

Arnatious avatar Sep 24 '20 20:09 Arnatious

We have a reference implementation in https://github.com/ubuntu-robotics/nodl, and are nearly done with a simple launch_ros integration pending some changes in SROS2. I think this proposal is ready to move ahead.

All right, then we need some approvals. Once we have that, I'll go ahead and merge.

clalancette avatar Oct 08 '20 13:10 clalancette

@Arnatious Have you thought of using yaml instead of xml? I just read some PRs and saw that for QoS settings first xml was discussed in #296 and then they decided to go the yaml way like for parameters in #300. Is it because of DDS or security interaction?

budrus avatar Oct 08 '20 19:10 budrus

Have you thought of using yaml instead of xml?

We actually prefer YAML, but DDS and SROS2 use XML, and the package.xml (which the nodl would be alongside) is also XML. Using YAML just didn't seem consistent.

kyrofa avatar Oct 08 '20 21:10 kyrofa

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/eprosima-presents-visual-ros/17632/3

ros-discourse avatar Dec 01 '20 23:12 ros-discourse

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/eprosima-presents-visual-ros/17632/4

ros-discourse avatar Dec 02 '20 19:12 ros-discourse

Hi @Arnatious, over the last few weeks - with the help of this design doc and the ubuntu-robotics/nodl API - some preliminary work has been done in writing a nodl_to_policy tool, that converts NoDL description files to a corresponding ROS 2 access control policy file. In doing so, we've come across some ambiguities with the NoDL description that I wanted to raise for discussion here:

  • The NoDL description seems to be lacking the concept of namespace. Every <profile> tag in an access control policy needs a node namespace, and insofar, the only option for namespace is to assume that all nodes are in the default "/" namespace, which is an invalid assumption for many reasons. Having said that, are there/should there be plans to incorporate the namespace concept into NoDL? Possibly as an additional (optional) attribute of the <node> tag?
  • Security enclaves are seen as collections of profiles, and a valid access control policy requires their fully qualified paths. Since the NoDL description does not specify enclave paths, an alternative is to qualify an enclave path using the namespace and node name, for example: "/[ns]/[node]". Having said that, should namespacing get formalized as a concept in the NoDL description, enclave paths are probably a non-issue. Else, we should consider adding the concept of a node's enclave path to the NoDL description too.
  • Thoughts on including default parameter services (describe_parameters, get_parameters, etc.), default topics (rosout, parameter_events), and default actions in a NoDL description? I'm assuming these are implicit with every node, and hence considered redundant from the description's POV. However, when converting from a NoDL description to an access control policy, certain assumptions have to be made about the presence/absence of said default services/topics/actions. This is not ideal from a security POV, even if this simply grants "empty" permissions. A fallout from this discussion is that a node's "type" (is it a lifecycle node?) is currently not expressed in the NoDL description. The "type" of node changes the set of "default" permission tags to consider for it.
  • Should there be a field to specify a node's "type"? I'm thinking of a regular node versus a lifecycle node. An alternate could be to capture the "extra" services/topics of lifecycle nodes with the regular service/topic tags, but that might go against omitting said implicit permission types (previous point).

FYI @iche033 @clalancette @wjwwood

aprotyas avatar Jun 22 '21 22:06 aprotyas

I was wondering if this is still under active development and will get merged in?

danthony06 avatar Nov 29 '21 23:11 danthony06

Hi @danthony06, We do plan on picking up this work soon. Note that altho this PR is note yet merged, NoDL-related packages are already part of REP 2005 - ROS 2 Common Packages.

artivis avatar Dec 02 '21 16:12 artivis

+1 to this. Merging it would give it more visibility and we need to encourage participation and contributors.

El El jue, 2 dic 2021 a las 17:22, Jeremie Deray @.***> escribió:

Hi @danthony06 https://github.com/danthony06, We do plan on picking up this work soon. Note that altho this PR is note yet merged, NoDL-related packages are already part of REP 2005 - ROS 2 Common Packages https://ros.org/reps/rep-2005.html.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ros2/design/pull/266#issuecomment-984783442, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKPYDUB7CUHIOUFK6ZHBGLUO6MMDANCNFSM4JRLER2Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

vmayoral avatar Dec 02 '21 17:12 vmayoral

I'm a little late to this discussion, but I am looking at this from the perspective of generating package documentation with a tool like rosdoc2. When we document interfaces in Python or C++, classes and parameters typically have a short description. It would not be difficult to extend rosdoc2 to read this NoDL file and extract information for documentation purposes - but nodes and parameters would need descriptions. Could you add a description attribute (type string) to the node and parameter elements for use by documentation utilities?

[update] I see that rosindex has some recommendations for node documentation in the package.xml file. Coming from a documentation perspective, their examples show a lot of text for nodes descriptions.

What is the relationship of this PR to the rosindex proposal? I don't think you really want to ask package maintainers to document their packages in two places. If this PR becomes the definitive source, then you'll need the same info (that is a description attribute or element) that rosindex wanted.

rkent avatar Dec 18 '21 16:12 rkent