From 0300f03907ea7a88463c7c3f74fb56580654c5fb Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 10 May 2023 00:30:25 +0000 Subject: [PATCH 01/35] WIP Test Isolation using linux namespaces --- bazel_ros2_rules/test_isolation/BUILD.bazel | 6 ++ bazel_ros2_rules/test_isolation/unshare.cc | 97 +++++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 bazel_ros2_rules/test_isolation/BUILD.bazel create mode 100644 bazel_ros2_rules/test_isolation/unshare.cc diff --git a/bazel_ros2_rules/test_isolation/BUILD.bazel b/bazel_ros2_rules/test_isolation/BUILD.bazel new file mode 100644 index 00000000..43818352 --- /dev/null +++ b/bazel_ros2_rules/test_isolation/BUILD.bazel @@ -0,0 +1,6 @@ + + +cc_test( + name = "unshare", + srcs = ["unshare.cc"], +) diff --git a/bazel_ros2_rules/test_isolation/unshare.cc b/bazel_ros2_rules/test_isolation/unshare.cc new file mode 100644 index 00000000..6e4122f9 --- /dev/null +++ b/bazel_ros2_rules/test_isolation/unshare.cc @@ -0,0 +1,97 @@ + +#include +#include +#include + +#include +#include +#include +#include + +// #define _GNU_SOURCE /* To get defns of NI_MAXSERV and NI_MAXHOST */ +#include +#include +#include +#include +#include +#include +#include +#include + + +void die(const char * message) { + std::cout << "TEST_ISOLATION: " << message << ".\n"; + exit(-1); +} + + +int main(int argc, char ** argv) { + // Create linux namespaces for the current process + // * A new user namespace to avoid needing CAP_SYS_ADMIN to create + // network and IPC namespaces + // * A new network namespace to prevent cross-talk via the network + // * A new IPC namespaces to prevent cross-talk via shared memrory + int result = unshare(CLONE_NEWUSER | CLONE_NEWNET | CLONE_NEWIPC); + + if (result != 0) { + perror("unshare"); + return result; + } + + + // Assert there is exactly one network interface + struct ifaddrs *ifaddr; + + if (-1 == getifaddrs(&ifaddr)) { + perror("getifaddrs"); + die("could not get network interfaces"); + } + if (nullptr == ifaddr) { + die("there are no network interfaces"); + } + if (nullptr != ifaddr->ifa_next) { + die("there are multiple network interfaces"); + } + + // Need a socket to do ioctl stuff on + int fd = socket(AF_INET, SOCK_DGRAM, 0); + if( fd < 0 ){ + die("could not open a socket"); + } + + struct ifreq ioctl_request; + + // Check what flags are set on the interface + strncpy(ioctl_request.ifr_name, ifaddr->ifa_name, IFNAMSIZ); + int err = ioctl(fd, SIOCGIFFLAGS, &ioctl_request); + if (0 != err) { + perror("ioctl"); + die("failed to get interface flags"); + } + + // Expecting a loopback interface. + if (!(ioctl_request.ifr_flags & IFF_LOOPBACK)) { + die("the only interface is not a loopback interface"); + } + + // Enable multicast if it's not already enabled. + if (!(ioctl_request.ifr_flags & IFF_MULTICAST)) { + std::cout << "Multicast is not enabled on the interface\n"; + + ioctl_request.ifr_flags |= IFF_MULTICAST; + + int err = ioctl(fd, SIOCSIFFLAGS, &ioctl_request); + if (0 != err) { + perror("ioctl"); + die("failed to set interface flags"); + } + } + + freeifaddrs(ifaddr); + + // TODO(sloretz) Exec test process + + + // Unhappy code so I see output from bazel_test without having to look up the bazel CLI flag + return -1; +} From 45dc291b2ce8ad28941fdc8fafa871c76e78d24b Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 17 May 2023 21:36:08 +0000 Subject: [PATCH 02/35] exec from command line Signed-off-by: Shane Loretz --- bazel_ros2_rules/test_isolation/BUILD.bazel | 4 +++- bazel_ros2_rules/test_isolation/unshare.cc | 21 +++++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/bazel_ros2_rules/test_isolation/BUILD.bazel b/bazel_ros2_rules/test_isolation/BUILD.bazel index 43818352..26a06323 100644 --- a/bazel_ros2_rules/test_isolation/BUILD.bazel +++ b/bazel_ros2_rules/test_isolation/BUILD.bazel @@ -1,6 +1,8 @@ -cc_test( +cc_binary( name = "unshare", srcs = ["unshare.cc"], ) + +cc_test diff --git a/bazel_ros2_rules/test_isolation/unshare.cc b/bazel_ros2_rules/test_isolation/unshare.cc index 6e4122f9..9468f95b 100644 --- a/bazel_ros2_rules/test_isolation/unshare.cc +++ b/bazel_ros2_rules/test_isolation/unshare.cc @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -20,12 +21,17 @@ void die(const char * message) { - std::cout << "TEST_ISOLATION: " << message << ".\n"; + std::cerr << "TEST_ISOLATION: " << message << ".\n"; exit(-1); } int main(int argc, char ** argv) { + if (argc < 2) { + die("shim must be given a command to execute"); + } + // TODO die if the first argument is not an absolute path, or doesn't exist. + // Create linux namespaces for the current process // * A new user namespace to avoid needing CAP_SYS_ADMIN to create // network and IPC namespaces @@ -76,7 +82,6 @@ int main(int argc, char ** argv) { // Enable multicast if it's not already enabled. if (!(ioctl_request.ifr_flags & IFF_MULTICAST)) { - std::cout << "Multicast is not enabled on the interface\n"; ioctl_request.ifr_flags |= IFF_MULTICAST; @@ -89,9 +94,17 @@ int main(int argc, char ** argv) { freeifaddrs(ifaddr); - // TODO(sloretz) Exec test process + // Have to copy new a new array that terminates with a null pointer. + std::vector new_argv; + for (int i = 1; i < argc; ++i) { + new_argv.push_back(argv[i]); + } + new_argv.push_back(nullptr); + // Exec a new process - should never return! + execv(new_argv.at(0), &new_argv.at(0)); - // Unhappy code so I see output from bazel_test without having to look up the bazel CLI flag + perror("execv"); + die("Call to execv failed"); return -1; } From 407365aaabc81aee1c28623012705a7fd23ddec3 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 17 May 2023 21:56:23 +0000 Subject: [PATCH 03/35] Bring up loopback interface Signed-off-by: Shane Loretz --- bazel_ros2_rules/test_isolation/unshare.cc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bazel_ros2_rules/test_isolation/unshare.cc b/bazel_ros2_rules/test_isolation/unshare.cc index 9468f95b..8d741873 100644 --- a/bazel_ros2_rules/test_isolation/unshare.cc +++ b/bazel_ros2_rules/test_isolation/unshare.cc @@ -80,16 +80,15 @@ int main(int argc, char ** argv) { die("the only interface is not a loopback interface"); } - // Enable multicast if it's not already enabled. - if (!(ioctl_request.ifr_flags & IFF_MULTICAST)) { + // Enable multicast + ioctl_request.ifr_flags |= IFF_MULTICAST; + // Bring up interface + ioctl_request.ifr_flags |= IFF_UP; - ioctl_request.ifr_flags |= IFF_MULTICAST; - - int err = ioctl(fd, SIOCSIFFLAGS, &ioctl_request); - if (0 != err) { - perror("ioctl"); - die("failed to set interface flags"); - } + err = ioctl(fd, SIOCSIFFLAGS, &ioctl_request); + if (0 != err) { + perror("ioctl"); + die("failed to set interface flags"); } freeifaddrs(ifaddr); From eadaff41402735e4b2a3361617e1b1d77184e596 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 17 May 2023 22:57:11 +0000 Subject: [PATCH 04/35] Move linux namespace creation to separate library Signed-off-by: Shane Loretz --- bazel_ros2_rules/test_isolation/BUILD.bazel | 17 ++- .../test_isolation/create_linux_namespaces.cc | 87 ++++++++++++++ .../test_isolation/create_linux_namespaces.h | 22 ++++ bazel_ros2_rules/test_isolation/isolate.cc | 35 ++++++ bazel_ros2_rules/test_isolation/unshare.cc | 109 ------------------ 5 files changed, 156 insertions(+), 114 deletions(-) create mode 100644 bazel_ros2_rules/test_isolation/create_linux_namespaces.cc create mode 100644 bazel_ros2_rules/test_isolation/create_linux_namespaces.h create mode 100644 bazel_ros2_rules/test_isolation/isolate.cc delete mode 100644 bazel_ros2_rules/test_isolation/unshare.cc diff --git a/bazel_ros2_rules/test_isolation/BUILD.bazel b/bazel_ros2_rules/test_isolation/BUILD.bazel index 26a06323..b610b52b 100644 --- a/bazel_ros2_rules/test_isolation/BUILD.bazel +++ b/bazel_ros2_rules/test_isolation/BUILD.bazel @@ -1,8 +1,15 @@ - +cc_library( + name = "create_linux_namespaces_cc", + srcs = ["create_linux_namespaces.cc"], + hdrs = ["create_linux_namespaces.h"], + visibility = ["//visibility:public"], +) cc_binary( - name = "unshare", - srcs = ["unshare.cc"], + name = "isolate", + srcs = ["isolate.cc"], + deps = [ + ":create_linux_namespaces_cc", + ], + visibility = ["//visibility:public"], ) - -cc_test diff --git a/bazel_ros2_rules/test_isolation/create_linux_namespaces.cc b/bazel_ros2_rules/test_isolation/create_linux_namespaces.cc new file mode 100644 index 00000000..bc256781 --- /dev/null +++ b/bazel_ros2_rules/test_isolation/create_linux_namespaces.cc @@ -0,0 +1,87 @@ +#include "create_linux_namespaces.h" + +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace bazel_ros2_rules { + +void error(const char * message) +{ + std::cerr << "create_linux_namesapces: " + << message << ":" << strerror(errno) << "\n"; +} + +bool create_linux_namespaces() +{ + int result = unshare(CLONE_NEWUSER | CLONE_NEWNET | CLONE_NEWIPC); + + if (result != 0) { + error("failed to call unshare"); + return false; + } + + // Assert there is exactly one network interface + struct ifaddrs *ifaddr; + + if (-1 == getifaddrs(&ifaddr)) { + error("could not get network interfaces"); + return false; + } + if (nullptr == ifaddr) { + error("there are no network interfaces"); + return false; + } + if (nullptr != ifaddr->ifa_next) { + error("there are multiple network interfaces"); + return false; + } + + // Need a socket to do ioctl stuff on + int fd = socket(AF_INET, SOCK_DGRAM, 0); + if( fd < 0 ){ + error("could not open a socket"); + freeifaddrs(ifaddr); + return false; + } + + struct ifreq ioctl_request; + + // Check what flags are set on the interface + strncpy(ioctl_request.ifr_name, ifaddr->ifa_name, IFNAMSIZ); + int err = ioctl(fd, SIOCGIFFLAGS, &ioctl_request); + if (0 != err) { + freeifaddrs(ifaddr); + error("failed to get interface flags"); + return false; + } + + // Expecting a loopback interface. + if (!(ioctl_request.ifr_flags & IFF_LOOPBACK)) { + error("the only interface is not a loopback interface"); + freeifaddrs(ifaddr); + return false; + } + + // Enable multicast + ioctl_request.ifr_flags |= IFF_MULTICAST; + // Bring up interface + ioctl_request.ifr_flags |= IFF_UP; + + err = ioctl(fd, SIOCSIFFLAGS, &ioctl_request); + if (0 != err) { + error("failed to set interface flags"); + freeifaddrs(ifaddr); + return false; + } + + freeifaddrs(ifaddr); + return true; +} +} // namespace bazel_ros2_rules diff --git a/bazel_ros2_rules/test_isolation/create_linux_namespaces.h b/bazel_ros2_rules/test_isolation/create_linux_namespaces.h new file mode 100644 index 00000000..731952aa --- /dev/null +++ b/bazel_ros2_rules/test_isolation/create_linux_namespaces.h @@ -0,0 +1,22 @@ +#pragma once + +namespace bazel_ros2_rules { +/// Creates linux namespaces suitable for isolating ROS 2 traffic. +/// +/// The new namespaces are: +/// * A new user namespace to avoid needing CAP_SYS_ADMIN to create +/// network and IPC namespaces +/// * A new network namespace to prevent cross-talk via the network +/// * A new IPC namespaces to prevent cross-talk via shared memory +/// +/// It also configures network namespace to enable ROS 2 traffic. +/// At the end of a successful call the current process will be in +/// the created namespaces. +/// Depending on what part of the process fails, an unsuccessful +/// call may also leave the current process in new namespaces. +/// There is no way to undo this. +/// +/// \return true iff the namespaces were created successfully. +bool create_linux_namespaces(); + +} // namespace bazel_ros2_rules diff --git a/bazel_ros2_rules/test_isolation/isolate.cc b/bazel_ros2_rules/test_isolation/isolate.cc new file mode 100644 index 00000000..1e2ef6b5 --- /dev/null +++ b/bazel_ros2_rules/test_isolation/isolate.cc @@ -0,0 +1,35 @@ +#include + +#include +#include + +#include "create_linux_namespaces.h" + +void die(const char * message) { + std::cerr << "isolate: " << message << ".\n"; + exit(-1); +} + +int main(int argc, char ** argv) { + if (argc < 2) { + die("shim must be given a command to execute"); + } + + if (not bazel_ros2_rules::create_linux_namespaces()) { + die("Failed to fully create isolated environment"); + } + + // Have to copy new a new array that terminates with a null pointer. + std::vector new_argv; + for (int i = 1; i < argc; ++i) { + new_argv.push_back(argv[i]); + } + new_argv.push_back(nullptr); + + // Exec a new process - should never return! + execv(new_argv.at(0), &new_argv.at(0)); + + perror("execv"); + die("Call to execv failed"); + return -1; +} diff --git a/bazel_ros2_rules/test_isolation/unshare.cc b/bazel_ros2_rules/test_isolation/unshare.cc deleted file mode 100644 index 8d741873..00000000 --- a/bazel_ros2_rules/test_isolation/unshare.cc +++ /dev/null @@ -1,109 +0,0 @@ - -#include -#include -#include -#include - -#include -#include -#include -#include - -// #define _GNU_SOURCE /* To get defns of NI_MAXSERV and NI_MAXHOST */ -#include -#include -#include -#include -#include -#include -#include -#include - - -void die(const char * message) { - std::cerr << "TEST_ISOLATION: " << message << ".\n"; - exit(-1); -} - - -int main(int argc, char ** argv) { - if (argc < 2) { - die("shim must be given a command to execute"); - } - // TODO die if the first argument is not an absolute path, or doesn't exist. - - // Create linux namespaces for the current process - // * A new user namespace to avoid needing CAP_SYS_ADMIN to create - // network and IPC namespaces - // * A new network namespace to prevent cross-talk via the network - // * A new IPC namespaces to prevent cross-talk via shared memrory - int result = unshare(CLONE_NEWUSER | CLONE_NEWNET | CLONE_NEWIPC); - - if (result != 0) { - perror("unshare"); - return result; - } - - - // Assert there is exactly one network interface - struct ifaddrs *ifaddr; - - if (-1 == getifaddrs(&ifaddr)) { - perror("getifaddrs"); - die("could not get network interfaces"); - } - if (nullptr == ifaddr) { - die("there are no network interfaces"); - } - if (nullptr != ifaddr->ifa_next) { - die("there are multiple network interfaces"); - } - - // Need a socket to do ioctl stuff on - int fd = socket(AF_INET, SOCK_DGRAM, 0); - if( fd < 0 ){ - die("could not open a socket"); - } - - struct ifreq ioctl_request; - - // Check what flags are set on the interface - strncpy(ioctl_request.ifr_name, ifaddr->ifa_name, IFNAMSIZ); - int err = ioctl(fd, SIOCGIFFLAGS, &ioctl_request); - if (0 != err) { - perror("ioctl"); - die("failed to get interface flags"); - } - - // Expecting a loopback interface. - if (!(ioctl_request.ifr_flags & IFF_LOOPBACK)) { - die("the only interface is not a loopback interface"); - } - - // Enable multicast - ioctl_request.ifr_flags |= IFF_MULTICAST; - // Bring up interface - ioctl_request.ifr_flags |= IFF_UP; - - err = ioctl(fd, SIOCSIFFLAGS, &ioctl_request); - if (0 != err) { - perror("ioctl"); - die("failed to set interface flags"); - } - - freeifaddrs(ifaddr); - - // Have to copy new a new array that terminates with a null pointer. - std::vector new_argv; - for (int i = 1; i < argc; ++i) { - new_argv.push_back(argv[i]); - } - new_argv.push_back(nullptr); - - // Exec a new process - should never return! - execv(new_argv.at(0), &new_argv.at(0)); - - perror("execv"); - die("Call to execv failed"); - return -1; -} From 01b3f35c96fc4516e3cd976a53fc456522d9c44e Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Thu, 18 May 2023 18:07:39 +0000 Subject: [PATCH 05/35] Rename some things Signed-off-by: Shane Loretz --- .../{test_isolation => network-isolation}/BUILD.bazel | 8 ++++---- .../{test_isolation => network-isolation}/isolate.cc | 4 ++-- .../network_isolation.cc} | 6 +++--- .../network_isolation.h} | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) rename bazel_ros2_rules/{test_isolation => network-isolation}/BUILD.bazel (52%) rename bazel_ros2_rules/{test_isolation => network-isolation}/isolate.cc (88%) rename bazel_ros2_rules/{test_isolation/create_linux_namespaces.cc => network-isolation/network_isolation.cc} (95%) rename bazel_ros2_rules/{test_isolation/create_linux_namespaces.h => network-isolation/network_isolation.h} (92%) diff --git a/bazel_ros2_rules/test_isolation/BUILD.bazel b/bazel_ros2_rules/network-isolation/BUILD.bazel similarity index 52% rename from bazel_ros2_rules/test_isolation/BUILD.bazel rename to bazel_ros2_rules/network-isolation/BUILD.bazel index b610b52b..cda72c83 100644 --- a/bazel_ros2_rules/test_isolation/BUILD.bazel +++ b/bazel_ros2_rules/network-isolation/BUILD.bazel @@ -1,7 +1,7 @@ cc_library( - name = "create_linux_namespaces_cc", - srcs = ["create_linux_namespaces.cc"], - hdrs = ["create_linux_namespaces.h"], + name = "network_isolation_cc", + srcs = ["network_isolation.cc"], + hdrs = ["network_isolation.h"], visibility = ["//visibility:public"], ) @@ -9,7 +9,7 @@ cc_binary( name = "isolate", srcs = ["isolate.cc"], deps = [ - ":create_linux_namespaces_cc", + ":network_isolation_cc", ], visibility = ["//visibility:public"], ) diff --git a/bazel_ros2_rules/test_isolation/isolate.cc b/bazel_ros2_rules/network-isolation/isolate.cc similarity index 88% rename from bazel_ros2_rules/test_isolation/isolate.cc rename to bazel_ros2_rules/network-isolation/isolate.cc index 1e2ef6b5..8187323a 100644 --- a/bazel_ros2_rules/test_isolation/isolate.cc +++ b/bazel_ros2_rules/network-isolation/isolate.cc @@ -3,7 +3,7 @@ #include #include -#include "create_linux_namespaces.h" +#include "network_isolation.h" void die(const char * message) { std::cerr << "isolate: " << message << ".\n"; @@ -15,7 +15,7 @@ int main(int argc, char ** argv) { die("shim must be given a command to execute"); } - if (not bazel_ros2_rules::create_linux_namespaces()) { + if (!network_isolation::create_linux_namespaces()) { die("Failed to fully create isolated environment"); } diff --git a/bazel_ros2_rules/test_isolation/create_linux_namespaces.cc b/bazel_ros2_rules/network-isolation/network_isolation.cc similarity index 95% rename from bazel_ros2_rules/test_isolation/create_linux_namespaces.cc rename to bazel_ros2_rules/network-isolation/network_isolation.cc index bc256781..324a2a8c 100644 --- a/bazel_ros2_rules/test_isolation/create_linux_namespaces.cc +++ b/bazel_ros2_rules/network-isolation/network_isolation.cc @@ -1,4 +1,4 @@ -#include "create_linux_namespaces.h" +#include "network_isolation.h" #include #include @@ -10,7 +10,7 @@ #include -namespace bazel_ros2_rules { +namespace network_isolation { void error(const char * message) { @@ -84,4 +84,4 @@ bool create_linux_namespaces() freeifaddrs(ifaddr); return true; } -} // namespace bazel_ros2_rules +} // namespace network_isolation diff --git a/bazel_ros2_rules/test_isolation/create_linux_namespaces.h b/bazel_ros2_rules/network-isolation/network_isolation.h similarity index 92% rename from bazel_ros2_rules/test_isolation/create_linux_namespaces.h rename to bazel_ros2_rules/network-isolation/network_isolation.h index 731952aa..6c9e7cba 100644 --- a/bazel_ros2_rules/test_isolation/create_linux_namespaces.h +++ b/bazel_ros2_rules/network-isolation/network_isolation.h @@ -1,6 +1,6 @@ #pragma once -namespace bazel_ros2_rules { +namespace network_isolation { /// Creates linux namespaces suitable for isolating ROS 2 traffic. /// /// The new namespaces are: @@ -19,4 +19,4 @@ namespace bazel_ros2_rules { /// \return true iff the namespaces were created successfully. bool create_linux_namespaces(); -} // namespace bazel_ros2_rules +} // namespace network_isolation From 6f1e200efb4553ce687fe3e667bc6a9a6d5adfa0 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Thu, 18 May 2023 18:27:45 +0000 Subject: [PATCH 06/35] grammar Signed-off-by: Shane Loretz --- bazel_ros2_rules/network-isolation/isolate.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel_ros2_rules/network-isolation/isolate.cc b/bazel_ros2_rules/network-isolation/isolate.cc index 8187323a..032da1d3 100644 --- a/bazel_ros2_rules/network-isolation/isolate.cc +++ b/bazel_ros2_rules/network-isolation/isolate.cc @@ -19,7 +19,7 @@ int main(int argc, char ** argv) { die("Failed to fully create isolated environment"); } - // Have to copy new a new array that terminates with a null pointer. + // Copy to a new array that terminates with a null pointer at the end. std::vector new_argv; for (int i = 1; i < argc; ++i) { new_argv.push_back(argv[i]); From 48f2b7960ffe1e5967ac75d08942b1dc7caa2e10 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Fri, 19 May 2023 00:17:14 +0000 Subject: [PATCH 07/35] ros_cc_test isolated by default Signed-off-by: Shane Loretz --- bazel_ros2_rules/ros2/ros_cc.bzl | 4 ++++ bazel_ros2_rules/ros2/tools/dload.bzl | 1 + bazel_ros2_rules/ros2/tools/dload_cc.bzl | 27 ++++++++++++++++++++++-- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/bazel_ros2_rules/ros2/ros_cc.bzl b/bazel_ros2_rules/ros2/ros_cc.bzl index fbc7ffdc..48756d30 100644 --- a/bazel_ros2_rules/ros2/ros_cc.bzl +++ b/bazel_ros2_rules/ros2/ros_cc.bzl @@ -166,6 +166,7 @@ def ros_cc_test( cc_binary_rule = native.cc_binary, cc_library_rule = native.cc_library, cc_test_rule = native.cc_test, + network_isolation = True, **kwargs): """ Builds a C/C++ test and wraps it with a shim that will inject the minimal @@ -211,6 +212,7 @@ def ros_cc_test( name = shim_name, target = ":" + noshim_name, env_changes = shim_env_changes, + network_isolation = network_isolation, **shim_kwargs ) @@ -220,4 +222,6 @@ def ros_cc_test( deps = ["@bazel_ros2_rules//ros2:dload_shim_cc"], tags = ["nolint"] + kwargs.get("tags", []), ) + if network_isolation: + kwargs['deps'].append("@bazel_ros2_rules//network-isolation:network_isolation_cc") cc_test_rule(name = name, **kwargs) diff --git a/bazel_ros2_rules/ros2/tools/dload.bzl b/bazel_ros2_rules/ros2/tools/dload.bzl index b2100485..ebe4961b 100644 --- a/bazel_ros2_rules/ros2/tools/dload.bzl +++ b/bazel_ros2_rules/ros2/tools/dload.bzl @@ -59,6 +59,7 @@ def get_dload_shim_attributes(): cfg = "target", ), "env_changes": attr.string_list_dict(), + "network_isolation": attr.bool(default=False), } def do_dload_shim(ctx, template, to_list): diff --git a/bazel_ros2_rules/ros2/tools/dload_cc.bzl b/bazel_ros2_rules/ros2/tools/dload_cc.bzl index 939cae6b..9ec4d397 100644 --- a/bazel_ros2_rules/ros2/tools/dload_cc.bzl +++ b/bazel_ros2_rules/ros2/tools/dload_cc.bzl @@ -12,10 +12,19 @@ load( "get_dload_shim_attributes", ) +_ISOLATE_IMPORT = '#include "network-isolation/network_isolation.h"' +_ISOLATE_CALL_OR_RETURN = """\ +if (!network_isolation::create_linux_namespaces()) {{ + return -1; +}} +""" + _REEXEC_TEMPLATE = """\ #include "ros2/tools/dload_shim.h" +CC_ISOLATE_IMPORT int main(int argc, const char * argv[]) {{ + CC_ISOLATE_CALL const char * executable_path = "{executable_path}"; std::vector names = {names}; std::vector> actions = {actions}; @@ -24,12 +33,23 @@ int main(int argc, const char * argv[]) {{ }} """ +def _resolve_isolation(template, network_isolation): + isolate_import = "" + isolate_call = "" + if network_isolation: + isolate_import = _ISOLATE_IMPORT + isolate_call = _ISOLATE_CALL_OR_RETURN + template = template.replace("CC_ISOLATE_IMPORT", isolate_import) + template = template.replace("CC_ISOLATE_CALL", isolate_call) + return template + def _to_cc_list(collection): """Turn collection into a C++ aggregate initializer expression.""" return "{" + ", ".join(collection) + "}" def _dload_cc_reexec_impl(ctx): - return do_dload_shim(ctx, _REEXEC_TEMPLATE, _to_cc_list) + template = _resolve_isolation(_REEXEC_TEMPLATE, ctx.attr.network_isolation) + return do_dload_shim(ctx, template, _to_cc_list) dload_cc_reexec = rule( doc = """\ @@ -52,10 +72,12 @@ dload_cc_reexec = rule( _LDWRAP_TEMPLATE = """\ #include "ros2/tools/dload_shim.h" +CC_ISOLATE_IMPORT extern "C" int __real_main(int argc, char** argv); extern "C" int __wrap_main(int argc, char** argv) {{ + CC_ISOLATE_CALL std::vector names = {names}; std::vector> actions = {actions}; bazel_ros2_rules::ApplyEnvironmentActions(argv[0], names, actions); @@ -64,7 +86,8 @@ extern "C" int __wrap_main(int argc, char** argv) {{ """ def _dload_cc_ldwrap_impl(ctx): - return do_dload_shim(ctx, _LDWRAP_TEMPLATE, _to_cc_list) + template = _resolve_isolation(_LDWRAP_TEMPLATE, ctx.attr.network_isolation) + return do_dload_shim(ctx, template, _to_cc_list) dload_cc_ldwrap = rule( doc = """\ From 50934865afe9092bba55277c00944f53da7014b5 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Fri, 19 May 2023 18:23:00 +0000 Subject: [PATCH 08/35] WIP CPython extension for isolation Signed-off-by: Shane Loretz --- bazel_ros2_rules/WORKSPACE | 7 ++ .../network-isolation/BUILD.bazel | 15 +++ .../network-isolation/network_isolation_py.cc | 38 +++++++ bazel_ros2_rules/tools/python_repository.bzl | 106 ++++++++++++++++++ 4 files changed, 166 insertions(+) create mode 100644 bazel_ros2_rules/network-isolation/network_isolation_py.cc create mode 100644 bazel_ros2_rules/tools/python_repository.bzl diff --git a/bazel_ros2_rules/WORKSPACE b/bazel_ros2_rules/WORKSPACE index cb0ca1cd..c054f338 100644 --- a/bazel_ros2_rules/WORKSPACE +++ b/bazel_ros2_rules/WORKSPACE @@ -15,3 +15,10 @@ http_archive( load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace") bazel_skylib_workspace() + + +load("@bazel_ros2_rules//tools:python_repository.bzl", "python_repository") + +python_repository( + name = "python" +) diff --git a/bazel_ros2_rules/network-isolation/BUILD.bazel b/bazel_ros2_rules/network-isolation/BUILD.bazel index cda72c83..7489d78d 100644 --- a/bazel_ros2_rules/network-isolation/BUILD.bazel +++ b/bazel_ros2_rules/network-isolation/BUILD.bazel @@ -13,3 +13,18 @@ cc_binary( ], visibility = ["//visibility:public"], ) + +# Create a CPython extension +cc_binary( + name = "libnetwork_isolation_py.so", + linkshared = True, + linkstatic = False, + srcs = ["network_isolation_py.cc"], + deps = ["@python//:python_cc"], +) + +py_library( + name = "libnetwork_isolation_py", + data = [":libnetwork_isolation_py.so"], + visibility = ["//visibility:public"], +) diff --git a/bazel_ros2_rules/network-isolation/network_isolation_py.cc b/bazel_ros2_rules/network-isolation/network_isolation_py.cc new file mode 100644 index 00000000..5bd95d7c --- /dev/null +++ b/bazel_ros2_rules/network-isolation/network_isolation_py.cc @@ -0,0 +1,38 @@ +#define PY_SSIZE_T_CLEAN +#include + +#include "network-isolation/network_isolation.h" + +static PyObject * +create_linux_namespaces(PyObject *, PyObject *) +{ + if (!network_isolation::create_linux_namespaces()) { + Py_RETURN_FALSE; + } + Py_RETURN_TRUE; +} + +static PyMethodDef methods[] = { + + {"create_linux_namespaces", &create_linux_namespaces, METH_NOARGS, + "Isolate the current process using linux namespaces."}, + {NULL, NULL, 0, NULL} /* sentinel */ +}; + +static PyModuleDef module = { + PyModuleDef_HEAD_INIT, + .m_name = "network_isolation_py", + .m_doc = "Tools to isolate network traffic on linux.", + .m_size = -1, + &methods, +}; + +PyMODINIT_FUNC +PyInit_network_isolation_py(void) +{ + m = PyModule_Create(&module); + if (m == NULL) + return NULL; + + return m; +} diff --git a/bazel_ros2_rules/tools/python_repository.bzl b/bazel_ros2_rules/tools/python_repository.bzl new file mode 100644 index 00000000..da6c4632 --- /dev/null +++ b/bazel_ros2_rules/tools/python_repository.bzl @@ -0,0 +1,106 @@ +""" +Finds local system Python headers and libraries using python-config and +makes them available to be used as a C/C++ dependency. + +This implementation is inspired by the one in Drake, but uses +sysconfig instead. + +Arguments: + name: A unique name for this rule. + python_interpreter: Name or path to the python interpreter. (optional) +""" + +load("@bazel_ros2_rules//tools:execute.bzl", "execute_or_fail") + +def python_info(repo_ctx, python_command): + + interpreter_path = execute_or_fail(repo_ctx, + [python_command, "-c", "import sys; print(sys.executable)"]).stdout + + version = execute_or_fail(repo_ctx, + [python_command, "-c", "import sysconfig; print(sysconfig.get_python_version())"]).stdout + + include = execute_or_fail(repo_ctx, + [python_command, "-c", "import sysconfig; print(sysconfig.get_path('include'))"]).stdout + + platinclude = execute_or_fail(repo_ctx, + [python_command, "-c", "import sysconfig; print(sysconfig.get_path('platinclude'))"]).stdout + + extension_suffix = execute_or_fail(repo_ctx, + [python_command, "-c", "import sysconfig; print(sysconfig.get_config_var('EXT_SUFFIX'))"]).stdout + if "None" == extension_suffix: + fail("Failed to get python extension suffix") + + return struct( + interpreter_path = interpreter_path.strip(), + version = version.strip(), + includes = [include.strip(), platinclude.strip()], + extension_suffix = extension_suffix.strip(), + ) + +def _impl(repo_ctx): + py_info = python_info(repo_ctx, repo_ctx.attr.python_command) + + skylark_content = """ +# DO NOT EDIT: generated by python_repository() +# WARNING: Avoid using this macro in any repository rules which require +# `load()` at the WORKSPACE level. Instead, load these constants through +# `BUILD.bazel` or `package.BUILD.bazel` files. + +PYTHON_INTERPRETER_PATH = "{interpreter_path}" +PYTHON_EXTENSION_SUFFIX = "{extension_suffix}" +PYTHON_VERSION = "{version}" +PYTHON_INCLUDES = {includes} +""".format( + interpreter_path = py_info.interpreter_path, + extension_suffix = py_info.extension_suffix, + version = py_info.version, + includes = py_info.includes + ) + repo_ctx.file( + "pyinfo.bzl", + content = skylark_content, + executable = False, + ) + + build_content = """# DO NOT EDIT: generated by python_repository() + +licenses(["notice"]) # Python-2.0 + +load("@python//:pyinfo.bzl", "PYTHON_INCLUDES") + +cc_library( + name = "python_headers", + includes = PYTHON_INCLUDES, + visibility = ["//visibility:private"], +) + +cc_library( + name = "python_cc", + includes = PYTHON_INCLUDES, + linkopts = [], + visibility = ["//visibility:public"], + deps = [":python_headers"], +) +""" + + repo_ctx.file( + "BUILD.bazel", + content = build_content, + executable = False, + ) + +python_repository = repository_rule( + _impl, + attrs = { + "python_command": attr.string(default="python3"), + }, + local = True, + configure = True, +) + + +# python_repository() creates a bazel repository with generated starlark with info about +# the python installation +# cpython_extension() is a macro that creates a cpython extension, using info from python_repository() +# cpython_extension has to load stuff from generated repository, so it can't live in this file. From 288000bf121fb3b5c7879ec8185efe5da808e668 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 31 May 2023 21:41:17 +0000 Subject: [PATCH 09/35] First commit where Python tests seem to work --- bazel_ros2_rules/WORKSPACE | 7 ------ .../BUILD.bazel | 11 +++++---- .../isolate.cc | 0 .../network_isolation.cc | 0 .../network_isolation.h | 0 .../network_isolation_py.cc | 13 ++++------ bazel_ros2_rules/ros2/ros_cc.bzl | 2 +- bazel_ros2_rules/ros2/ros_py.bzl | 4 ++++ bazel_ros2_rules/ros2/tools/dload_cc.bzl | 2 +- bazel_ros2_rules/ros2/tools/dload_py.bzl | 24 ++++++++++++++++++- bazel_ros2_rules/tools/python_repository.bzl | 4 ++-- 11 files changed, 43 insertions(+), 24 deletions(-) rename bazel_ros2_rules/{network-isolation => network_isolation}/BUILD.bazel (71%) rename bazel_ros2_rules/{network-isolation => network_isolation}/isolate.cc (100%) rename bazel_ros2_rules/{network-isolation => network_isolation}/network_isolation.cc (100%) rename bazel_ros2_rules/{network-isolation => network_isolation}/network_isolation.h (100%) rename bazel_ros2_rules/{network-isolation => network_isolation}/network_isolation_py.cc (81%) diff --git a/bazel_ros2_rules/WORKSPACE b/bazel_ros2_rules/WORKSPACE index c054f338..cb0ca1cd 100644 --- a/bazel_ros2_rules/WORKSPACE +++ b/bazel_ros2_rules/WORKSPACE @@ -15,10 +15,3 @@ http_archive( load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace") bazel_skylib_workspace() - - -load("@bazel_ros2_rules//tools:python_repository.bzl", "python_repository") - -python_repository( - name = "python" -) diff --git a/bazel_ros2_rules/network-isolation/BUILD.bazel b/bazel_ros2_rules/network_isolation/BUILD.bazel similarity index 71% rename from bazel_ros2_rules/network-isolation/BUILD.bazel rename to bazel_ros2_rules/network_isolation/BUILD.bazel index 7489d78d..5da71835 100644 --- a/bazel_ros2_rules/network-isolation/BUILD.bazel +++ b/bazel_ros2_rules/network_isolation/BUILD.bazel @@ -16,15 +16,18 @@ cc_binary( # Create a CPython extension cc_binary( - name = "libnetwork_isolation_py.so", + name = "network_isolation_py.so", linkshared = True, linkstatic = False, srcs = ["network_isolation_py.cc"], - deps = ["@python//:python_cc"], + deps = [ + ":network_isolation_cc", + "@python_dev//:headers", + ], ) py_library( - name = "libnetwork_isolation_py", - data = [":libnetwork_isolation_py.so"], + name = "network_isolation_py", + data = [":network_isolation_py.so"], visibility = ["//visibility:public"], ) diff --git a/bazel_ros2_rules/network-isolation/isolate.cc b/bazel_ros2_rules/network_isolation/isolate.cc similarity index 100% rename from bazel_ros2_rules/network-isolation/isolate.cc rename to bazel_ros2_rules/network_isolation/isolate.cc diff --git a/bazel_ros2_rules/network-isolation/network_isolation.cc b/bazel_ros2_rules/network_isolation/network_isolation.cc similarity index 100% rename from bazel_ros2_rules/network-isolation/network_isolation.cc rename to bazel_ros2_rules/network_isolation/network_isolation.cc diff --git a/bazel_ros2_rules/network-isolation/network_isolation.h b/bazel_ros2_rules/network_isolation/network_isolation.h similarity index 100% rename from bazel_ros2_rules/network-isolation/network_isolation.h rename to bazel_ros2_rules/network_isolation/network_isolation.h diff --git a/bazel_ros2_rules/network-isolation/network_isolation_py.cc b/bazel_ros2_rules/network_isolation/network_isolation_py.cc similarity index 81% rename from bazel_ros2_rules/network-isolation/network_isolation_py.cc rename to bazel_ros2_rules/network_isolation/network_isolation_py.cc index 5bd95d7c..f63255ee 100644 --- a/bazel_ros2_rules/network-isolation/network_isolation_py.cc +++ b/bazel_ros2_rules/network_isolation/network_isolation_py.cc @@ -1,8 +1,9 @@ #define PY_SSIZE_T_CLEAN #include -#include "network-isolation/network_isolation.h" +#include "network_isolation/network_isolation.h" +extern "C" { static PyObject * create_linux_namespaces(PyObject *, PyObject *) { @@ -13,7 +14,6 @@ create_linux_namespaces(PyObject *, PyObject *) } static PyMethodDef methods[] = { - {"create_linux_namespaces", &create_linux_namespaces, METH_NOARGS, "Isolate the current process using linux namespaces."}, {NULL, NULL, 0, NULL} /* sentinel */ @@ -24,15 +24,12 @@ static PyModuleDef module = { .m_name = "network_isolation_py", .m_doc = "Tools to isolate network traffic on linux.", .m_size = -1, - &methods, + methods, }; PyMODINIT_FUNC PyInit_network_isolation_py(void) { - m = PyModule_Create(&module); - if (m == NULL) - return NULL; - - return m; + return PyModule_Create(&module); +} } diff --git a/bazel_ros2_rules/ros2/ros_cc.bzl b/bazel_ros2_rules/ros2/ros_cc.bzl index 48756d30..93a5d19a 100644 --- a/bazel_ros2_rules/ros2/ros_cc.bzl +++ b/bazel_ros2_rules/ros2/ros_cc.bzl @@ -223,5 +223,5 @@ def ros_cc_test( tags = ["nolint"] + kwargs.get("tags", []), ) if network_isolation: - kwargs['deps'].append("@bazel_ros2_rules//network-isolation:network_isolation_cc") + kwargs['deps'].append("@bazel_ros2_rules//network_isolation:network_isolation_cc") cc_test_rule(name = name, **kwargs) diff --git a/bazel_ros2_rules/ros2/ros_py.bzl b/bazel_ros2_rules/ros2/ros_py.bzl index 5184c8f3..ba04ab3f 100644 --- a/bazel_ros2_rules/ros2/ros_py.bzl +++ b/bazel_ros2_rules/ros2/ros_py.bzl @@ -135,6 +135,7 @@ def ros_py_test( rmw_implementation = None, py_binary_rule = native.py_binary, py_test_rule = native.py_test, + network_isolation = True, **kwargs): """ Builds a Python test and wraps it with a shim that will inject the minimal @@ -176,6 +177,7 @@ def ros_py_test( name = shim_name, target = ":" + noshim_name, env_changes = shim_env_changes, + network_isolation = network_isolation, **shim_kwargs ) @@ -186,4 +188,6 @@ def ros_py_test( deps = ["@bazel_ros2_rules//ros2:dload_shim_py"], tags = ["nolint"] + kwargs.get("tags", []), ) + if network_isolation: + kwargs['deps'].append("@bazel_ros2_rules//network_isolation:network_isolation_py") py_test_rule(name = name, **kwargs) diff --git a/bazel_ros2_rules/ros2/tools/dload_cc.bzl b/bazel_ros2_rules/ros2/tools/dload_cc.bzl index 9ec4d397..f98488fd 100644 --- a/bazel_ros2_rules/ros2/tools/dload_cc.bzl +++ b/bazel_ros2_rules/ros2/tools/dload_cc.bzl @@ -12,7 +12,7 @@ load( "get_dload_shim_attributes", ) -_ISOLATE_IMPORT = '#include "network-isolation/network_isolation.h"' +_ISOLATE_IMPORT = '#include "network_isolation/network_isolation.h"' _ISOLATE_CALL_OR_RETURN = """\ if (!network_isolation::create_linux_namespaces()) {{ return -1; diff --git a/bazel_ros2_rules/ros2/tools/dload_py.bzl b/bazel_ros2_rules/ros2/tools/dload_py.bzl index 4aaf2195..2e216629 100644 --- a/bazel_ros2_rules/ros2/tools/dload_py.bzl +++ b/bazel_ros2_rules/ros2/tools/dload_py.bzl @@ -13,23 +13,45 @@ load( "get_dload_shim_attributes", ) +_ISOLATE_IMPORT = """\ +import network_isolation.network_isolation_py as isolation +""" + +_ISOLATE_CALL_OR_RETURN = """\ +if not isolation.create_linux_namespaces(): + sys.exit(-1) +""" + _DLOAD_PY_SHIM_TEMPLATE = """\ assert __name__ == "__main__" from bazel_ros2_rules.ros2.tools.dload_shim import do_dload_shim +PY_ISOLATE_IMPORT +PY_ISOLATE_CALL executable_path = "{executable_path}" names = {names} actions = {actions} do_dload_shim(executable_path, names, actions) """ +def _resolve_isolation(template, network_isolation): + isolate_import = "" + isolate_call = "" + if network_isolation: + isolate_import = _ISOLATE_IMPORT + isolate_call = _ISOLATE_CALL_OR_RETURN + template = template.replace("PY_ISOLATE_IMPORT", isolate_import) + template = template.replace("PY_ISOLATE_CALL", isolate_call) + return template + def _to_py_list(collection): """Turn collection into a Python list expression.""" return "[" + ", ".join(collection) + "]" def _dload_py_shim_impl(ctx): - return do_dload_shim(ctx, _DLOAD_PY_SHIM_TEMPLATE, _to_py_list) + template = _resolve_isolation(_DLOAD_PY_SHIM_TEMPLATE, ctx.attr.network_isolation) + return do_dload_shim(ctx, template, _to_py_list) dload_py_shim = rule( attrs = get_dload_shim_attributes(), diff --git a/bazel_ros2_rules/tools/python_repository.bzl b/bazel_ros2_rules/tools/python_repository.bzl index da6c4632..bc045cd2 100644 --- a/bazel_ros2_rules/tools/python_repository.bzl +++ b/bazel_ros2_rules/tools/python_repository.bzl @@ -1,6 +1,6 @@ """ -Finds local system Python headers and libraries using python-config and -makes them available to be used as a C/C++ dependency. +Find local system Python headers and libraries and +make them available to be used as a C/C++ dependency. This implementation is inspired by the one in Drake, but uses sysconfig instead. From 7892d86fe1178f859ffc782e949ef1e6e6df86cc Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 31 May 2023 21:45:41 +0000 Subject: [PATCH 10/35] Remove unused file --- bazel_ros2_rules/tools/python_repository.bzl | 106 ------------------- 1 file changed, 106 deletions(-) delete mode 100644 bazel_ros2_rules/tools/python_repository.bzl diff --git a/bazel_ros2_rules/tools/python_repository.bzl b/bazel_ros2_rules/tools/python_repository.bzl deleted file mode 100644 index bc045cd2..00000000 --- a/bazel_ros2_rules/tools/python_repository.bzl +++ /dev/null @@ -1,106 +0,0 @@ -""" -Find local system Python headers and libraries and -make them available to be used as a C/C++ dependency. - -This implementation is inspired by the one in Drake, but uses -sysconfig instead. - -Arguments: - name: A unique name for this rule. - python_interpreter: Name or path to the python interpreter. (optional) -""" - -load("@bazel_ros2_rules//tools:execute.bzl", "execute_or_fail") - -def python_info(repo_ctx, python_command): - - interpreter_path = execute_or_fail(repo_ctx, - [python_command, "-c", "import sys; print(sys.executable)"]).stdout - - version = execute_or_fail(repo_ctx, - [python_command, "-c", "import sysconfig; print(sysconfig.get_python_version())"]).stdout - - include = execute_or_fail(repo_ctx, - [python_command, "-c", "import sysconfig; print(sysconfig.get_path('include'))"]).stdout - - platinclude = execute_or_fail(repo_ctx, - [python_command, "-c", "import sysconfig; print(sysconfig.get_path('platinclude'))"]).stdout - - extension_suffix = execute_or_fail(repo_ctx, - [python_command, "-c", "import sysconfig; print(sysconfig.get_config_var('EXT_SUFFIX'))"]).stdout - if "None" == extension_suffix: - fail("Failed to get python extension suffix") - - return struct( - interpreter_path = interpreter_path.strip(), - version = version.strip(), - includes = [include.strip(), platinclude.strip()], - extension_suffix = extension_suffix.strip(), - ) - -def _impl(repo_ctx): - py_info = python_info(repo_ctx, repo_ctx.attr.python_command) - - skylark_content = """ -# DO NOT EDIT: generated by python_repository() -# WARNING: Avoid using this macro in any repository rules which require -# `load()` at the WORKSPACE level. Instead, load these constants through -# `BUILD.bazel` or `package.BUILD.bazel` files. - -PYTHON_INTERPRETER_PATH = "{interpreter_path}" -PYTHON_EXTENSION_SUFFIX = "{extension_suffix}" -PYTHON_VERSION = "{version}" -PYTHON_INCLUDES = {includes} -""".format( - interpreter_path = py_info.interpreter_path, - extension_suffix = py_info.extension_suffix, - version = py_info.version, - includes = py_info.includes - ) - repo_ctx.file( - "pyinfo.bzl", - content = skylark_content, - executable = False, - ) - - build_content = """# DO NOT EDIT: generated by python_repository() - -licenses(["notice"]) # Python-2.0 - -load("@python//:pyinfo.bzl", "PYTHON_INCLUDES") - -cc_library( - name = "python_headers", - includes = PYTHON_INCLUDES, - visibility = ["//visibility:private"], -) - -cc_library( - name = "python_cc", - includes = PYTHON_INCLUDES, - linkopts = [], - visibility = ["//visibility:public"], - deps = [":python_headers"], -) -""" - - repo_ctx.file( - "BUILD.bazel", - content = build_content, - executable = False, - ) - -python_repository = repository_rule( - _impl, - attrs = { - "python_command": attr.string(default="python3"), - }, - local = True, - configure = True, -) - - -# python_repository() creates a bazel repository with generated starlark with info about -# the python installation -# cpython_extension() is a macro that creates a cpython extension, using info from python_repository() -# cpython_extension has to load stuff from generated repository, so it can't live in this file. From 4958e7f44624fa46447224e84de3c2ee545a7e26 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 31 May 2023 21:54:02 +0000 Subject: [PATCH 11/35] linters --- bazel_ros2_rules/network_isolation/BUILD.bazel | 4 ++-- bazel_ros2_rules/ros2/ros_cc.bzl | 4 +++- bazel_ros2_rules/ros2/ros_py.bzl | 4 +++- bazel_ros2_rules/ros2/tools/dload.bzl | 2 +- bazel_ros2_rules/ros2/tools/dload_py.bzl | 3 ++- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/bazel_ros2_rules/network_isolation/BUILD.bazel b/bazel_ros2_rules/network_isolation/BUILD.bazel index 5da71835..2a26491d 100644 --- a/bazel_ros2_rules/network_isolation/BUILD.bazel +++ b/bazel_ros2_rules/network_isolation/BUILD.bazel @@ -8,18 +8,18 @@ cc_library( cc_binary( name = "isolate", srcs = ["isolate.cc"], + visibility = ["//visibility:public"], deps = [ ":network_isolation_cc", ], - visibility = ["//visibility:public"], ) # Create a CPython extension cc_binary( name = "network_isolation_py.so", + srcs = ["network_isolation_py.cc"], linkshared = True, linkstatic = False, - srcs = ["network_isolation_py.cc"], deps = [ ":network_isolation_cc", "@python_dev//:headers", diff --git a/bazel_ros2_rules/ros2/ros_cc.bzl b/bazel_ros2_rules/ros2/ros_cc.bzl index 93a5d19a..29f944c9 100644 --- a/bazel_ros2_rules/ros2/ros_cc.bzl +++ b/bazel_ros2_rules/ros2/ros_cc.bzl @@ -223,5 +223,7 @@ def ros_cc_test( tags = ["nolint"] + kwargs.get("tags", []), ) if network_isolation: - kwargs['deps'].append("@bazel_ros2_rules//network_isolation:network_isolation_cc") + kwargs["deps"].append( + "@bazel_ros2_rules//network_isolation:network_isolation_cc", + ) cc_test_rule(name = name, **kwargs) diff --git a/bazel_ros2_rules/ros2/ros_py.bzl b/bazel_ros2_rules/ros2/ros_py.bzl index ba04ab3f..13c1dfea 100644 --- a/bazel_ros2_rules/ros2/ros_py.bzl +++ b/bazel_ros2_rules/ros2/ros_py.bzl @@ -189,5 +189,7 @@ def ros_py_test( tags = ["nolint"] + kwargs.get("tags", []), ) if network_isolation: - kwargs['deps'].append("@bazel_ros2_rules//network_isolation:network_isolation_py") + kwargs["deps"].append( + "@bazel_ros2_rules//network_isolation:network_isolation_py", + ) py_test_rule(name = name, **kwargs) diff --git a/bazel_ros2_rules/ros2/tools/dload.bzl b/bazel_ros2_rules/ros2/tools/dload.bzl index ebe4961b..ee0bea51 100644 --- a/bazel_ros2_rules/ros2/tools/dload.bzl +++ b/bazel_ros2_rules/ros2/tools/dload.bzl @@ -59,7 +59,7 @@ def get_dload_shim_attributes(): cfg = "target", ), "env_changes": attr.string_list_dict(), - "network_isolation": attr.bool(default=False), + "network_isolation": attr.bool(default = False), } def do_dload_shim(ctx, template, to_list): diff --git a/bazel_ros2_rules/ros2/tools/dload_py.bzl b/bazel_ros2_rules/ros2/tools/dload_py.bzl index 2e216629..46b8b0d5 100644 --- a/bazel_ros2_rules/ros2/tools/dload_py.bzl +++ b/bazel_ros2_rules/ros2/tools/dload_py.bzl @@ -50,7 +50,8 @@ def _to_py_list(collection): return "[" + ", ".join(collection) + "]" def _dload_py_shim_impl(ctx): - template = _resolve_isolation(_DLOAD_PY_SHIM_TEMPLATE, ctx.attr.network_isolation) + template = _resolve_isolation( + _DLOAD_PY_SHIM_TEMPLATE, ctx.attr.network_isolation) return do_dload_shim(ctx, template, _to_py_list) dload_py_shim = rule( From 7499342129d0f7f7c768a43cc52d7e6b1f0b4762 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 31 May 2023 22:03:24 +0000 Subject: [PATCH 12/35] lint --- bazel_ros2_rules/ros2/tools/dload_py.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bazel_ros2_rules/ros2/tools/dload_py.bzl b/bazel_ros2_rules/ros2/tools/dload_py.bzl index 46b8b0d5..c7f1ff32 100644 --- a/bazel_ros2_rules/ros2/tools/dload_py.bzl +++ b/bazel_ros2_rules/ros2/tools/dload_py.bzl @@ -51,7 +51,9 @@ def _to_py_list(collection): def _dload_py_shim_impl(ctx): template = _resolve_isolation( - _DLOAD_PY_SHIM_TEMPLATE, ctx.attr.network_isolation) + _DLOAD_PY_SHIM_TEMPLATE, + ctx.attr.network_isolation, + ) return do_dload_shim(ctx, template, _to_py_list) dload_py_shim = rule( From 2a22e42229795392e2aa032517157378c8cb13a9 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Wed, 31 May 2023 22:52:35 +0000 Subject: [PATCH 13/35] Add python_dev repo --- bazel_ros2_rules/WORKSPACE | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bazel_ros2_rules/WORKSPACE b/bazel_ros2_rules/WORKSPACE index cb0ca1cd..b386eb55 100644 --- a/bazel_ros2_rules/WORKSPACE +++ b/bazel_ros2_rules/WORKSPACE @@ -15,3 +15,7 @@ http_archive( load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace") bazel_skylib_workspace() + +load("//deps:defs.bzl", "add_bazel_ros2_rules_dependencies") + +add_bazel_ros2_rules_dependencies() From 8f015db8c15f64f5f132f220d7874701d77e6a29 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Tue, 20 Jun 2023 23:32:48 +0000 Subject: [PATCH 14/35] Try to fix one CI job by updating apt package lists --- .github/workflows/bazelized_drake_ros.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index b2af0ebe..641b7037 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -24,6 +24,8 @@ jobs: run: du -hs $(readlink -f /home/runner/.cache/ci) - name: Simplify apt upgrades run: .github/simplify_apt_and_upgrades.sh + - name: Get latest apt data + run: sudo apt-get update - name: Configure drake_ros Bazel for CI run: ln -s ../.github/ci.bazelrc ./user.bazelrc working-directory: drake_ros From f279ccfce05a7553d98dc898ed0d22936f150b02 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Tue, 20 Jun 2023 23:44:06 +0000 Subject: [PATCH 15/35] Move where we apt-get update to later --- .github/workflows/bazelized_drake_ros.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index 641b7037..69190806 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -24,8 +24,6 @@ jobs: run: du -hs $(readlink -f /home/runner/.cache/ci) - name: Simplify apt upgrades run: .github/simplify_apt_and_upgrades.sh - - name: Get latest apt data - run: sudo apt-get update - name: Configure drake_ros Bazel for CI run: ln -s ../.github/ci.bazelrc ./user.bazelrc working-directory: drake_ros @@ -39,6 +37,8 @@ jobs: - name: Download Drake run: bazel build @drake//:module_py working-directory: drake_ros + - name: Get latest apt data + run: sudo apt-get update - name: Install Drake's dependencies run: yes | sudo ./bazel-drake_ros/external/drake/setup/ubuntu/install_prereqs.sh working-directory: drake_ros From 9e2ecdf756412ab6631e2cc8599720368e18c9da Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Tue, 20 Jun 2023 23:50:14 +0000 Subject: [PATCH 16/35] Add debugging step to find missing shared library --- .github/workflows/bazelized.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/bazelized.yml b/.github/workflows/bazelized.yml index dd78a73a..beddaf09 100644 --- a/.github/workflows/bazelized.yml +++ b/.github/workflows/bazelized.yml @@ -55,6 +55,9 @@ jobs: - name: Build Bazel workspace (ros2_example_installed) run: bazel build //... working-directory: ros2_example_bazel_installed + - name: XXX Sloretz debug missing shared library + run: find -L . -name "libexternal_Sbazel_Uros2_Urules_Snetwork_Uisolation_Slibnetwork_Uisolation_Ucc.so" + working-directory: ros2_example_bazel_installed - name: Test Bazel workspace (ros2_example_installed) run: bazel test //... @ros2//... working-directory: ros2_example_bazel_installed From f7c2c29eaf78f0b9d404edeaf8a2ea5e99ad1524 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Mon, 17 Jul 2023 00:07:14 +0000 Subject: [PATCH 17/35] fixes rosdep linter Signed-off-by: Aditya Pande --- ros2_example_bazel_installed/setup/prereq-rosdep-keys.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ros2_example_bazel_installed/setup/prereq-rosdep-keys.txt b/ros2_example_bazel_installed/setup/prereq-rosdep-keys.txt index 48ad3dff..50b888d9 100644 --- a/ros2_example_bazel_installed/setup/prereq-rosdep-keys.txt +++ b/ros2_example_bazel_installed/setup/prereq-rosdep-keys.txt @@ -21,6 +21,7 @@ liborocos-kdl-dev libqt5-core libqt5-gui libqt5-opengl +libqt5-svg libqt5-widgets libsqlite3-dev libx11-dev @@ -63,6 +64,7 @@ python3-pytest python3-qt5-bindings python3-rosdistro-modules python3-setuptools +python3-vcstool python3-yaml qtbase5-dev spdlog From 9c5ffce515a196a4a22363c112f3ed65e8c67a95 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Mon, 17 Jul 2023 05:20:23 +0000 Subject: [PATCH 18/35] Fixed CI error Signed-off-by: Aditya Pande --- bazel_ros2_rules/network_isolation/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel_ros2_rules/network_isolation/BUILD.bazel b/bazel_ros2_rules/network_isolation/BUILD.bazel index 2a26491d..f74865e1 100644 --- a/bazel_ros2_rules/network_isolation/BUILD.bazel +++ b/bazel_ros2_rules/network_isolation/BUILD.bazel @@ -19,7 +19,7 @@ cc_binary( name = "network_isolation_py.so", srcs = ["network_isolation_py.cc"], linkshared = True, - linkstatic = False, + linkstatic = True, deps = [ ":network_isolation_cc", "@python_dev//:headers", From 41063615afe85ed9d2d4fa2e26af28d642fa82e2 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Mon, 17 Jul 2023 05:22:13 +0000 Subject: [PATCH 19/35] Cleanup Signed-off-by: Aditya Pande --- .github/workflows/bazelized.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/bazelized.yml b/.github/workflows/bazelized.yml index beddaf09..dd78a73a 100644 --- a/.github/workflows/bazelized.yml +++ b/.github/workflows/bazelized.yml @@ -55,9 +55,6 @@ jobs: - name: Build Bazel workspace (ros2_example_installed) run: bazel build //... working-directory: ros2_example_bazel_installed - - name: XXX Sloretz debug missing shared library - run: find -L . -name "libexternal_Sbazel_Uros2_Urules_Snetwork_Uisolation_Slibnetwork_Uisolation_Ucc.so" - working-directory: ros2_example_bazel_installed - name: Test Bazel workspace (ros2_example_installed) run: bazel test //... @ros2//... working-directory: ros2_example_bazel_installed From e03cfb2f0133ff64b837b42840bc2b6d0ce3c3a0 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Thu, 20 Jul 2023 00:12:53 +0000 Subject: [PATCH 20/35] Removed apt update step in workflow Signed-off-by: Aditya Pande --- .github/workflows/bazelized_drake_ros.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index 69190806..b2af0ebe 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -37,8 +37,6 @@ jobs: - name: Download Drake run: bazel build @drake//:module_py working-directory: drake_ros - - name: Get latest apt data - run: sudo apt-get update - name: Install Drake's dependencies run: yes | sudo ./bazel-drake_ros/external/drake/setup/ubuntu/install_prereqs.sh working-directory: drake_ros From 430fed753b28ec0f140fda39a595ed64e2945936 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Thu, 20 Jul 2023 20:59:44 +0000 Subject: [PATCH 21/35] sandbox_debug flag to CI Signed-off-by: Aditya Pande --- .github/workflows/bazelized_drake_ros.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index b2af0ebe..3f061916 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -60,7 +60,7 @@ jobs: run: ln -s ../.github/ci.bazelrc ./user.bazelrc working-directory: drake_ros_examples - name: Build drake_ros_examples - run: bazel build //... + run: bazel build --sandbox_debug //... working-directory: drake_ros_examples - name: Test drake_ros_examples run: bazel test //... From bab9094a6287e55aae9e66744477fc5d48cdfa86 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Mon, 24 Jul 2023 05:38:28 +0000 Subject: [PATCH 22/35] Update workflow to save space Signed-off-by: Aditya Pande --- .github/workflows/bazelized_drake_ros.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index 3f061916..fb0bf972 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -33,7 +33,7 @@ jobs: run: rosdep update - name: Install bazel_ros2_rules dependencies # TODO(sloretz) make bazel_ros2_rules/setup/install_prereqs.sh - run: sudo apt install python3 python3-toposort + run: sudo apt --no-install-recommends install python3 python3-toposort - name: Download Drake run: bazel build @drake//:module_py working-directory: drake_ros @@ -44,7 +44,7 @@ jobs: # TODO(sloretz) can't use rosdep here because we need the archive downloaded, # but can't download the archive with bazel until we have the dependencies installed # so bazel_ros2_rules can inspect it. - run: sudo apt-get install -y libconsole-bridge-dev + run: sudo apt --no-install-recommends install -y libconsole-bridge-dev # CI for drake_ros. - name: Build drake_ros run: bazel build //... From 31c102b62ddc5aa73b9908fd41456ab73986d816 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Mon, 24 Jul 2023 20:43:01 +0000 Subject: [PATCH 23/35] Bazel clean workspace Signed-off-by: Aditya Pande --- .github/workflows/bazelized_drake_ros.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index fb0bf972..703f55e0 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -52,6 +52,9 @@ jobs: - name: Test drake_ros run: bazel test //... working-directory: drake_ros + - name: Clean up drake_ros + run: bazel clean --expunge_async + working-directory: drake_ros # CI for drake_ros_examples. - name: Install additional ROS dependencies for drake_ros_examples run: yes | sudo ./setup/install_prereqs.sh From 454ef51223ccafc88aa6c8d74099851904d73b65 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Mon, 24 Jul 2023 23:19:41 +0000 Subject: [PATCH 24/35] Removed sandbox debug and added bazel clean without expunge Signed-off-by: Aditya Pande --- .github/workflows/bazelized_drake_ros.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index 703f55e0..09da922d 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -53,7 +53,7 @@ jobs: run: bazel test //... working-directory: drake_ros - name: Clean up drake_ros - run: bazel clean --expunge_async + run: bazel clean working-directory: drake_ros # CI for drake_ros_examples. - name: Install additional ROS dependencies for drake_ros_examples @@ -63,7 +63,7 @@ jobs: run: ln -s ../.github/ci.bazelrc ./user.bazelrc working-directory: drake_ros_examples - name: Build drake_ros_examples - run: bazel build --sandbox_debug //... + run: bazel build //... working-directory: drake_ros_examples - name: Test drake_ros_examples run: bazel test //... From 3432e81b69f8896859c7beff2f5487ea9496fee7 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Wed, 2 Aug 2023 15:49:15 +0000 Subject: [PATCH 25/35] Added a test case for network_isolation Signed-off-by: Aditya Pande --- bazel_ros2_rules/ros2/ros_cc.bzl | 2 +- bazel_ros2_rules/ros2/ros_py.bzl | 2 +- ros2_example_bazel_installed/BUILD.bazel | 22 +++++++ .../test/network_isolation_test.py | 25 ++++++++ .../test/talker_listener.py | 63 +++++++++++++++++++ 5 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 ros2_example_bazel_installed/test/network_isolation_test.py create mode 100644 ros2_example_bazel_installed/test/talker_listener.py diff --git a/bazel_ros2_rules/ros2/ros_cc.bzl b/bazel_ros2_rules/ros2/ros_cc.bzl index 29f944c9..b79febb4 100644 --- a/bazel_ros2_rules/ros2/ros_cc.bzl +++ b/bazel_ros2_rules/ros2/ros_cc.bzl @@ -166,7 +166,7 @@ def ros_cc_test( cc_binary_rule = native.cc_binary, cc_library_rule = native.cc_library, cc_test_rule = native.cc_test, - network_isolation = True, + network_isolation = False, **kwargs): """ Builds a C/C++ test and wraps it with a shim that will inject the minimal diff --git a/bazel_ros2_rules/ros2/ros_py.bzl b/bazel_ros2_rules/ros2/ros_py.bzl index 13c1dfea..edbdadbc 100644 --- a/bazel_ros2_rules/ros2/ros_py.bzl +++ b/bazel_ros2_rules/ros2/ros_py.bzl @@ -135,7 +135,7 @@ def ros_py_test( rmw_implementation = None, py_binary_rule = native.py_binary, py_test_rule = native.py_test, - network_isolation = True, + network_isolation = False, **kwargs): """ Builds a Python test and wraps it with a shim that will inject the minimal diff --git a/ros2_example_bazel_installed/BUILD.bazel b/ros2_example_bazel_installed/BUILD.bazel index 01257a15..599dba3c 100644 --- a/ros2_example_bazel_installed/BUILD.bazel +++ b/ros2_example_bazel_installed/BUILD.bazel @@ -85,3 +85,25 @@ ros_cc_test( srcs = ["test/use_console_bridge.cc"], deps = ["@ros2//:console_bridge_vendor_cc"], ) + +ros_py_binary( + name = "talker_listener_isolated_py", + srcs = ["test/talker_listener.py"], + main = "test/talker_listener.py", + deps = [ + "@ros2//:rclpy_py", + "@ros2//:std_msgs_py", + ], +) + +ros_py_binary( + name = "network_isolation_test", + srcs = ["test/network_isolation_test.py"], + main = "test/network_isolation_test.py", + deps = [ + "//:talker_listener_isolated_py", + ], + data = [ + "@bazel_ros2_rules//network_isolation:isolate", + ], +) diff --git a/ros2_example_bazel_installed/test/network_isolation_test.py b/ros2_example_bazel_installed/test/network_isolation_test.py new file mode 100644 index 00000000..d0fd69e6 --- /dev/null +++ b/ros2_example_bazel_installed/test/network_isolation_test.py @@ -0,0 +1,25 @@ +import argparse +import sys +from subprocess import Popen + +# This script spawns a bunch of talker-listener pairs +# publishing on the same topic. They are isolated using +# the network isolation mechanism, and hence should not +# cross talk. + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--number_of_isolated_pairs", type=int, default=5) + args = parser.parse_args() + + subprocess_list = [] + for i in range(args.number_of_isolated_pairs): + subprocess_list.append(Popen(['external/bazel_ros2_rules/network_isolation/isolate', + sys.executable, 'test/talker_listener.py', + '--id', str(i)])) + + for process in subprocess_list: + process.wait() + +if __name__ == "__main__": + main() diff --git a/ros2_example_bazel_installed/test/talker_listener.py b/ros2_example_bazel_installed/test/talker_listener.py new file mode 100644 index 00000000..e1f7ece4 --- /dev/null +++ b/ros2_example_bazel_installed/test/talker_listener.py @@ -0,0 +1,63 @@ +import argparse + +import rclpy +import rclpy.node +import std_msgs +from std_msgs.msg import Float64 +from rclpy.executors import MultiThreadedExecutor + +class Talker(rclpy.node.Node): + def __init__(self, id): + super().__init__('talker_' + str(id)) + self._publisher = self.create_publisher( + std_msgs.msg.Float64, 'chatter', 10) + self._timer = self.create_timer(0.1, self._timer_callback) + self._id = id + + def _timer_callback(self): + msg = std_msgs.msg.Float64() + msg.data = float(self._id) + self._publisher.publish(msg) + +class Listener(rclpy.node.Node): + def __init__(self, id): + super().__init__('listener_' + str(id)) + self._subscription = self.create_subscription( + std_msgs.msg.Float64, + 'chatter', + self._topic_callback, + 10) + timeout = self.declare_parameter('timeout', 2.0) + self._timer = self.create_timer( + timeout.value, self._timer_callback) + self._expected_messages_received = 0 + self._id = id + + def _topic_callback(self, msg): + assert msg.data == self._id, \ + f"I heard '{msg.data}' yet I was expecting '{self._id}'!" + self._expected_messages_received += 1 + + def _timer_callback(self): + assert self._expected_messages_received > 0, \ + f"I did not hear '{self._id}' even once!" + rclpy.shutdown() + +def main(): + # This script launches a pair of a talker and a listener + # that are bound to some numerical id. The talker publishes + # the id, and the listener expectes that id in the msg. + parser = argparse.ArgumentParser() + parser.add_argument("--id", type=int, default=0) + args = parser.parse_args() + + rclpy.init() + + executor = MultiThreadedExecutor() + executor.add_node(Talker(args.id)) + executor.add_node(Listener(args.id)) + + executor.spin() + +if __name__ == "__main__": + main() From fb9703d7263e24398ba94dc9d5b481eb9118b112 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Wed, 2 Aug 2023 15:50:59 +0000 Subject: [PATCH 26/35] Typo Signed-off-by: Aditya Pande --- ros2_example_bazel_installed/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ros2_example_bazel_installed/BUILD.bazel b/ros2_example_bazel_installed/BUILD.bazel index 599dba3c..76c52db8 100644 --- a/ros2_example_bazel_installed/BUILD.bazel +++ b/ros2_example_bazel_installed/BUILD.bazel @@ -96,7 +96,7 @@ ros_py_binary( ], ) -ros_py_binary( +ros_py_test( name = "network_isolation_test", srcs = ["test/network_isolation_test.py"], main = "test/network_isolation_test.py", From a34408a8d9b89b2489f1c10d70aa8d9338211da6 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Wed, 2 Aug 2023 16:00:09 +0000 Subject: [PATCH 27/35] Linter Signed-off-by: Aditya Pande --- ros2_example_bazel_installed/BUILD.bazel | 32 ++++++++++++------------ 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/ros2_example_bazel_installed/BUILD.bazel b/ros2_example_bazel_installed/BUILD.bazel index 76c52db8..e8446dad 100644 --- a/ros2_example_bazel_installed/BUILD.bazel +++ b/ros2_example_bazel_installed/BUILD.bazel @@ -87,23 +87,23 @@ ros_cc_test( ) ros_py_binary( - name = "talker_listener_isolated_py", - srcs = ["test/talker_listener.py"], - main = "test/talker_listener.py", - deps = [ - "@ros2//:rclpy_py", - "@ros2//:std_msgs_py", - ], + name = "talker_listener_isolated_py", + srcs = ["test/talker_listener.py"], + main = "test/talker_listener.py", + deps = [ + "@ros2//:rclpy_py", + "@ros2//:std_msgs_py", + ], ) ros_py_test( - name = "network_isolation_test", - srcs = ["test/network_isolation_test.py"], - main = "test/network_isolation_test.py", - deps = [ - "//:talker_listener_isolated_py", - ], - data = [ - "@bazel_ros2_rules//network_isolation:isolate", - ], + name = "network_isolation_test", + srcs = ["test/network_isolation_test.py"], + data = [ + "@bazel_ros2_rules//network_isolation:isolate", + ], + main = "test/network_isolation_test.py", + deps = [ + "//:talker_listener_isolated_py", + ], ) From 21fe89b01c537b5e69a958fc4a110028bc5ccef6 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Wed, 27 Sep 2023 17:54:23 +0000 Subject: [PATCH 28/35] Free up disk space Signed-off-by: Aditya Pande --- .github/workflows/bazelized_drake_ros.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index 542cf659..afac67d8 100644 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -32,6 +32,13 @@ jobs: run: du -hs $(readlink -f /home/runner/.cache/bazel_ci) || true # Setup. + - name: Free up disk space + run: | + sudo rm -rf \ + /usr/share/dotnet \ + /opt/ghc \ + /usr/local/share/boost \ + "$AGENT_TOOLSDIRECTORY" || true - name: Simplify apt upgrades run: .github/simplify_apt_and_upgrades.sh - name: Configure drake_ros Bazel for CI From 8ce84560cbf4cad49240978c5f9ec0775cf630dc Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Tue, 10 Oct 2023 22:29:27 +0000 Subject: [PATCH 29/35] Added a readme Signed-off-by: Aditya Pande --- bazel_ros2_rules/network_isolation/README.md | 99 ++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 bazel_ros2_rules/network_isolation/README.md diff --git a/bazel_ros2_rules/network_isolation/README.md b/bazel_ros2_rules/network_isolation/README.md new file mode 100644 index 00000000..943a4dc9 --- /dev/null +++ b/bazel_ros2_rules/network_isolation/README.md @@ -0,0 +1,99 @@ +# Introduction +This directory contains tools and targets to isolate ros2 tests in this repository using linux network namespaces. +At its core, it uses the ``unshare()`` system call, and is meant to isolate ROS2 traffic. It creates a new user namespace, +new network namespace to prevent cross talk via the network, and new IPC namespace to prevent cross talk using shared memory. + +The existing dload_shim is used to pass on the required argument and isolate the tests. + +## Why do we need this ? +When running ROS2 tests in parallel, they might publish on the same topics and there might be cross talk between tests. rmw config or ROS domain id based isolation is possible, +but it does not scale well, due to limited ports and domain id available. Linux namespaces provide a generic and a scalable way to solve this problem. + +## Why not isolate individual targets instead of tests ? +Isolation using the namespace approach requires 3 namespaces, or "credentials" for processes to talk to each other, or be in the same realm : IPC, user and network namespaces. +Tests are more generic than individual targets, as the tests are free to fork() or run any number of processes they want, and they'll live in the same namespace. + +If we do isolate a process 'A', how do we make sure the next process 'B' will live in the same namespace as 'A' (if they're meant to talk to each other) ? There needs to be an API to provide the above mentioned "credentials" to the new process +somehow, which out of scope of this feature for now. + +# Targets +The logic lives in the following targets, which are meant to be used with the bazel rules ``ros_cc_test()`` and ``ros_py_test()`` : +* ``network_isolation_cc`` (cc_library): This is where the core logic lives, and the ``unshare()`` call is run. +* ``network_isolation_py.so``(cpython extension) : Python binding for the network isolation logic. +* ``network_isolation_py`` (py_library) : Importable python module for the network isolation logic. + +Other than these, there is a standalone executable target called ``isolate`` which is meant to be used in a standalone way, and not with the +``ros_*_test()`` rules. It isolates the process in the first argument provided to it. This uses the same ``unshare()`` logic as ``network_isolation_cc``. + +# Example usage +There are 3 ways to use this feature : + +## Using ``ros_cc_test()`` rule : +There is now an extra argument (``network_isolation``) available to the rule, so we can modify a test in ``ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel`` as : + +``` +ros_cc_test( + name = "talker_listener_cc_test", + size = "small", + srcs = ["test/talker_listener.cc"], + rmw_implementation = "rmw_cyclonedds_cpp", + network_isolation = True, + deps = [ + ":listener_cc", + ":talker_cc", + "@ros2//:rclcpp_cc", + "@ros2//:std_msgs_cc", + "@ros2//resources/rmw_isolation:rmw_isolation_cc", + ], +) +``` + +## Using the ``ros_py_test()`` rule : +Similarly for the python test rule, we can add ``network_isolation`` to ``True``. Consider this section in ``drake_ros_examples/examples/iiwa_manipulator/BUILD.bazel`` : + +``` +ros_py_test( + name = "iiwa_manipulator_test", + network_isolation = True, + srcs = ["test/iiwa_manipulator_test.py"], + data = [ + ":iiwa_manipulator", + ":iiwa_manipulator_py", + ], + main = "test/iiwa_manipulator_test.py", + deps = [ + "@ros2//resources/bazel_ros_env:bazel_ros_env_py", + ], +) +``` + + +## Using the ``isolate`` target: +As mentioned before, the ``isolate`` target is meant to be used in a generic way and will isolate the process provided to it, i.e. : +``` +cd bazel_ros2_rules +bazel run //network_isolation:isolate -- /bin/bash -c "echo HELLO_WORLD" +``` + +Here, the bash command for printing "HELLO_WORLD" is isolated, and can be replaced with any ros2 command. For instance, if you have ros2 humble installed +outside of drake-ros, using debs, you could run a publisher and subscriber that would be isolated from each other : + +Lets start the talker from the demo nodes package: +``` +bazel run //network_isolation:isolate -- /bin/bash -c "source /opt/ros/humble/setup.bash && export RMW_IMPLEMENTATION=rmw_cyclonedds_cpp && ros2 run demo_nodes_cpp talker" +``` + +In another terminal, run : +``` +bazel run //network_isolation:isolate -- /bin/bash -c "source /opt/ros/humble/setup.bash && export RMW_IMPLEMENTATION=rmw_cyclonedds_cpp && ros2 run demo_nodes_cpp listener" +``` + +These 2 processes should not be able to talk to each other, when used with ``isolate``. + +# Testing this feature in CI +Among the 3 ways to use this feature, as far as CI is concerned, there is a test for the ``isolate`` target mechanism. + +The test target is called ``network_isolation_test`` and uses ``ros2_example_bazel_installed/test/network_isolation_test.py`` to run 5 (by default) talker-listener pairs which have an id attached to them. +They all publish and listen on the same topic at the same time, and expect no cross talk between the pairs. The number of pairs can be changed using the ``--id`` cmdline argument if needed. + +The other 2 ways to use this feature, using ``ros_*_test()`` rules, run on tests, and not on executable targets, so *writing a test for a test* seems like an anti-pattern. The ``isolate`` target uses the same logic, so should be sufficient for testing. From ea130634b2fe10dd7764b65d87c039edbaa4eace Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Tue, 10 Oct 2023 22:32:29 +0000 Subject: [PATCH 30/35] readme minor edits Signed-off-by: Aditya Pande --- bazel_ros2_rules/network_isolation/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bazel_ros2_rules/network_isolation/README.md b/bazel_ros2_rules/network_isolation/README.md index 943a4dc9..b8b7553a 100644 --- a/bazel_ros2_rules/network_isolation/README.md +++ b/bazel_ros2_rules/network_isolation/README.md @@ -6,8 +6,8 @@ new network namespace to prevent cross talk via the network, and new IPC namespa The existing dload_shim is used to pass on the required argument and isolate the tests. ## Why do we need this ? -When running ROS2 tests in parallel, they might publish on the same topics and there might be cross talk between tests. rmw config or ROS domain id based isolation is possible, -but it does not scale well, due to limited ports and domain id available. Linux namespaces provide a generic and a scalable way to solve this problem. +When running ROS2 tests in parallel, they might publish on the same topics and there might be cross talk between tests. RMW config or ROS domain id based isolation is possible, +but it does not scale well, due to limited ports and domain ids available. Linux namespaces provide a generic and a scalable way to solve this problem. ## Why not isolate individual targets instead of tests ? Isolation using the namespace approach requires 3 namespaces, or "credentials" for processes to talk to each other, or be in the same realm : IPC, user and network namespaces. @@ -25,7 +25,7 @@ The logic lives in the following targets, which are meant to be used with the ba Other than these, there is a standalone executable target called ``isolate`` which is meant to be used in a standalone way, and not with the ``ros_*_test()`` rules. It isolates the process in the first argument provided to it. This uses the same ``unshare()`` logic as ``network_isolation_cc``. -# Example usage +# How do we use this feature ? There are 3 ways to use this feature : ## Using ``ros_cc_test()`` rule : From e47cfbc7cb030cd65b05ef1bc31071a49398f1f6 Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Tue, 10 Oct 2023 22:40:12 +0000 Subject: [PATCH 31/35] readme minor edits Signed-off-by: Aditya Pande --- bazel_ros2_rules/network_isolation/README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bazel_ros2_rules/network_isolation/README.md b/bazel_ros2_rules/network_isolation/README.md index b8b7553a..79cd4fff 100644 --- a/bazel_ros2_rules/network_isolation/README.md +++ b/bazel_ros2_rules/network_isolation/README.md @@ -29,7 +29,7 @@ Other than these, there is a standalone executable target called ``isolate`` whi There are 3 ways to use this feature : ## Using ``ros_cc_test()`` rule : -There is now an extra argument (``network_isolation``) available to the rule, so we can modify a test in ``ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel`` as : +There is now an extra argument (``network_isolation``) available to the rule, so for e.g. we can modify a test in ``ros2_example_bazel_installed/ros2_example_apps/BUILD.bazel`` as : ``` ros_cc_test( @@ -48,6 +48,8 @@ ros_cc_test( ) ``` +Whatever processes are spawned by this test will be contained in a namespace, and will be able to talk to each other, but not to any other ros nodes running outside of this test. + ## Using the ``ros_py_test()`` rule : Similarly for the python test rule, we can add ``network_isolation`` to ``True``. Consider this section in ``drake_ros_examples/examples/iiwa_manipulator/BUILD.bazel`` : @@ -67,9 +69,11 @@ ros_py_test( ) ``` +Similarly, the ros nodes spawned in this test can only talk to each other, and not other nodes running on the system at the same time, even if you have multiple instances of this test running in different terminals. + ## Using the ``isolate`` target: -As mentioned before, the ``isolate`` target is meant to be used in a generic way and will isolate the process provided to it, i.e. : +As mentioned before, the ``isolate`` target is meant to be used in a generic way and will isolate **any** process provided to it provided it is available in the bazel sandbox, for e.g. : ``` cd bazel_ros2_rules bazel run //network_isolation:isolate -- /bin/bash -c "echo HELLO_WORLD" From 8d84a102c45e45279b5e559ef2bf9de34437a28b Mon Sep 17 00:00:00 2001 From: Alejandro Hernandez Cordero Date: Fri, 22 Mar 2024 11:04:26 +0100 Subject: [PATCH 32/35] Added suggestions Signed-off-by: Alejandro Hernandez Cordero --- .../network_isolation/network_isolation.cc | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/bazel_ros2_rules/network_isolation/network_isolation.cc b/bazel_ros2_rules/network_isolation/network_isolation.cc index 324a2a8c..7f9c2563 100644 --- a/bazel_ros2_rules/network_isolation/network_isolation.cc +++ b/bazel_ros2_rules/network_isolation/network_isolation.cc @@ -1,14 +1,15 @@ #include "network_isolation.h" -#include +#include #include -#include +#include +#include +#include +#include #include +#include +#include #include -#include -#include - -#include namespace network_isolation { @@ -81,6 +82,25 @@ bool create_linux_namespaces() return false; } + // For programs that use both LCM and ROS, we need an LCM route ala + // sudo route add -net 224.0.0.0 netmask 240.0.0.0 dev lo + struct rtentry route = {}; + auto* dest = reinterpret_cast(&route.rt_dst); + dest->sin_family = AF_INET; + dest->sin_addr.s_addr = inet_addr("224.0.0.0"); + auto* mask = reinterpret_cast(&route.rt_genmask); + mask->sin_family = AF_INET; + mask->sin_addr.s_addr = inet_addr("240.0.0.0"); + std::string device{"lo"}; + route.rt_dev = device.data(); + route.rt_flags = RTF_UP; + err = ioctl(fd, SIOCADDRT, &route); + if (0 != err) { + error("failed to set route"); + freeifaddrs(ifaddr); + return false; + } + freeifaddrs(ifaddr); return true; } From c2b9823720a495ec2e718810e3b3e73ee3d343a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Hern=C3=A1ndez=20Cordero?= Date: Mon, 25 Mar 2024 18:29:13 +0100 Subject: [PATCH 33/35] suggestions to network isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alejandro Hernández Cordero --- .../network_isolation/network_isolation.cc | 2 ++ .../network_isolation/network_isolation_py.cc | 36 ++++--------------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/bazel_ros2_rules/network_isolation/network_isolation.cc b/bazel_ros2_rules/network_isolation/network_isolation.cc index 7f9c2563..d601bb4b 100644 --- a/bazel_ros2_rules/network_isolation/network_isolation.cc +++ b/bazel_ros2_rules/network_isolation/network_isolation.cc @@ -1,5 +1,7 @@ #include "network_isolation.h" +#include + #include #include #include diff --git a/bazel_ros2_rules/network_isolation/network_isolation_py.cc b/bazel_ros2_rules/network_isolation/network_isolation_py.cc index f63255ee..51ee4240 100644 --- a/bazel_ros2_rules/network_isolation/network_isolation_py.cc +++ b/bazel_ros2_rules/network_isolation/network_isolation_py.cc @@ -1,35 +1,11 @@ -#define PY_SSIZE_T_CLEAN -#include - #include "network_isolation/network_isolation.h" -extern "C" { -static PyObject * -create_linux_namespaces(PyObject *, PyObject *) -{ - if (!network_isolation::create_linux_namespaces()) { - Py_RETURN_FALSE; - } - Py_RETURN_TRUE; -} - -static PyMethodDef methods[] = { - {"create_linux_namespaces", &create_linux_namespaces, METH_NOARGS, - "Isolate the current process using linux namespaces."}, - {NULL, NULL, 0, NULL} /* sentinel */ -}; +#include -static PyModuleDef module = { - PyModuleDef_HEAD_INIT, - .m_name = "network_isolation_py", - .m_doc = "Tools to isolate network traffic on linux.", - .m_size = -1, - methods, -}; +namespace py = pybind11; -PyMODINIT_FUNC -PyInit_network_isolation_py(void) -{ - return PyModule_Create(&module); -} +PYBIND11_MODULE(network_isolation_py, m) { + m.def("create_linux_namespaces", &network_isolation::create_linux_namespaces, R"pbdoc( + Creates linux namespaces suitable for isolating ROS 2 traffic. + )pbdoc"); } From d000c8b193f2e95e2f8e9d2dacdbd249c224b2fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Hern=C3=A1ndez=20Cordero?= Date: Mon, 25 Mar 2024 18:31:05 +0100 Subject: [PATCH 34/35] make linters happy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alejandro Hernández Cordero --- .../network_isolation/network_isolation_py.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/bazel_ros2_rules/network_isolation/network_isolation_py.cc b/bazel_ros2_rules/network_isolation/network_isolation_py.cc index 51ee4240..aa0fbe0a 100644 --- a/bazel_ros2_rules/network_isolation/network_isolation_py.cc +++ b/bazel_ros2_rules/network_isolation/network_isolation_py.cc @@ -1,11 +1,12 @@ -#include "network_isolation/network_isolation.h" - #include +#include "network_isolation/network_isolation.h" + namespace py = pybind11; -PYBIND11_MODULE(network_isolation_py, m) { - m.def("create_linux_namespaces", &network_isolation::create_linux_namespaces, R"pbdoc( +PYBIND11_MODULE(network_isolation_py, m) +{ + m.def("create_linux_namespaces", &network_isolation::create_linux_namespaces, R"pbdoc( Creates linux namespaces suitable for isolating ROS 2 traffic. )pbdoc"); } From ad1131b7a539266d0a59afb2c3da70721389926e Mon Sep 17 00:00:00 2001 From: Aditya Pande Date: Mon, 25 Mar 2024 11:20:28 -0700 Subject: [PATCH 35/35] Update bazelized_drake_ros.yml --- .github/workflows/bazelized_drake_ros.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/bazelized_drake_ros.yml b/.github/workflows/bazelized_drake_ros.yml index 6625d994..f5f46438 100755 --- a/.github/workflows/bazelized_drake_ros.yml +++ b/.github/workflows/bazelized_drake_ros.yml @@ -52,13 +52,9 @@ jobs: run: rosdep update - name: Install bazel_ros2_rules dependencies # TODO(sloretz) make bazel_ros2_rules/setup/install_prereqs.sh -<<<<<<< HEAD - run: sudo apt --no-install-recommends install python3 python3-toposort -======= run: | sudo apt install python3 python3-toposort sudo rm -rf /var/lib/apt/lists/* ->>>>>>> upstream/main - name: Download Drake run: bazel build @drake//:module_py working-directory: drake_ros