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.

Sign up for GitHub

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

WIP: Add enclave information #681

Draft
wants to merge 6 commits into
base: rolling
Choose a base branch
from
Draft

WIP: Add enclave information #681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
459f6ad
First attempt at including enclave information in info output
danthony06 Dec 14, 2021
8956780
Fixing import
danthony06 Dec 14, 2021
c8deb25
Changing named tuple reference
danthony06 Dec 15, 2021
a585738
Fixing function that was erroneously changed
danthony06 Dec 15, 2021
5b10eae
Simplifying outptu
danthony06 Dec 16, 2021
ba61fc2
Removing duplicated code
danthony06 Dec 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions ros2node/ros2node/api/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
)

NodeName = namedtuple('NodeName', ('name', 'namespace', 'full_name'))
NodeNameEnclave = namedtuple('NodeNameEnclave', ('name', 'enclave'))
TopicInfo = namedtuple('Topic', ('name', 'types'))


Expand Down Expand Up @@ -71,6 +72,23 @@ def get_node_names(*, node, include_hidden_nodes=False):
]


def get_node_names_with_enclaves(*, node, include_hidden_nodes=False):
node_names_and_namespaces_with_enclaves = node.get_node_names_and_namespaces_with_enclaves()
return [
NodeNameEnclave(
name=NodeName(
name=t[0],
namespace=t[1],
full_name=t[1] + ('' if t[1].endswith('/') else '/') + t[0]),
enclave=t[2])
for t in node_names_and_namespaces_with_enclaves
if (
include_hidden_nodes or
(t[0] and not t[0].startswith(HIDDEN_NODE_PREFIX))
)
]


def get_topics(remote_node_name, func, *, include_hidden_topics=False):
node = parse_node_name(remote_node_name)
names_and_types = func(node.name, node.namespace)
Expand Down
16 changes: 13 additions & 3 deletions ros2node/ros2node/verb/info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from ros2cli.node.strategy import NodeStrategy
from ros2node.api import get_action_client_info
from ros2node.api import get_action_server_info
from ros2node.api import get_node_names
from ros2node.api import get_node_names_with_enclaves
from ros2node.api import get_publisher_info
from ros2node.api import get_service_client_info
from ros2node.api import get_service_server_info
Expand All @@ -28,6 +28,10 @@
from ros2node.verb import VerbExtension


def print_enclaves(enclaves):
print(*[2 * ' {}'.format(e) for e in enclaves if e != '/'], sep='\n')


def print_names_and_types(names_and_types):
print(*[2 * ' ' + s.name + ': ' + ', '.join(s.types) for s in names_and_types], sep='\n')

Expand All @@ -47,8 +51,10 @@ def add_arguments(self, parser, cli_name):

def main(self, *, args):
with NodeStrategy(args) as node:
node_names = get_node_names(node=node, include_hidden_nodes=args.include_hidden)
count = [n.full_name for n in node_names].count(args.node_name)
node_names_with_enclaves = get_node_names_with_enclaves(
node=node,
include_hidden_nodes=args.include_hidden)
count = [n.name.full_name for n in node_names_with_enclaves].count(args.node_name)
if count > 1:
print(
INFO_NONUNIQUE_WARNING_TEMPLATE.format(
Expand Down Expand Up @@ -80,5 +86,9 @@ def main(self, *, args):
node=node, remote_node_name=args.node_name, include_hidden=args.include_hidden)
print(' Action Clients:')
print_names_and_types(actions_clients)
print(' Enclaves:')
print_enclaves([n.enclave for n in node_names_with_enclaves
if n.name.full_name == args.node_name])
Comment on lines +89 to +91
Copy link
Collaborator

Choose a reason for hiding this comment

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

i am okay with either way, but we could consider this is one of the option for ros2 node info.

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure which is the best option. My rationale for leaving it in the default output is:

  1. Most people are running unsecured systems, and in this case it does not add a lot to the information output
  2. If someone is running a secured system, this can be important information, and users might want to frequently see this when they are examining a system.

Copy link
Author

Choose a reason for hiding this comment

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

Is there any interest in this, or should I withdraw the PR? Internally, we've found that this information would be very useful for our development, and I'd like to help the community if possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is helpful. can you open this PR as Non-Draft for review?

Most people are running unsecured systems, and in this case it does not add a lot to the information output

IMO, this is why i came up with option such as --include-enclaves.


else:
return "Unable to find node '" + args.node_name + "'"