-
Notifications
You must be signed in to change notification settings - Fork 190
Add environment variable to override daemon endpoint #1099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rolling
Are you sure you want to change the base?
Conversation
This is particularly important if '0' is used as a port because the system will dynamically assign an unused port to the socket. There should be no behavioral changes as a result of this modification. Signed-off-by: Scott K Logan <[email protected]>
To better support a mechanism to override the ROS 2 daemon URL, this change inverts the chain of functions used to determine what hostname, port, and URL should be used. There should be no behavioral changes as a result of this modification. Signed-off-by: Scott K Logan <[email protected]>
This change adds a new environment variable 'ROS2_DAEMON_SERVER_URL' which can be used to change the expected endpoint for communicating with the ROS 2 daemon process. Unspecified parts of the URL will be replaced with default values, so it's possible to omit the scheme, host, and port to retain default behavior while still overriding other parts of the URL. Signed-off-by: Scott K Logan <[email protected]>
def __get__(self, instance, owner): | ||
return (get_path(),) | ||
|
||
rpc_paths = _GetRpcPaths() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little more explanation on this snippet:
All of the other "getters" for endpoint-related information (get_address
, get_port
, and get_xmlrpc_server_url
) will change if the environment variables change. If we just set the rpc_paths
class attribute, it will be set to whatever the value is when the import occurs and won't change.
from ros2cli.xmlrpc.local_server import SimpleXMLRPCRequestHandler | ||
|
||
|
||
SERVER_URL_VARIABLE_NAME = 'ROS2_DAEMON_SERVER_URL' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, this is not officially documented anywhere, but currently we can only spawn the daemon process with 127.0.0.1
which is localhost loopback physical network IF (with only offset of port number that comes from ROS_DOMAIN_ID). and this fix can expose the daemon XMLRPC API to any IP address that is owned by this host.
although I believe this is more flexible for user to manage the ros2 daemon process in the LAN (for example, we can set SERVER_URL_VARIABLE_NAME to all ROS 2 machines and use the single ros2 daemon server in the entire ROS 2 network. this is useful for resource constrained devices to avoid starting ros2 daemon process in local), this could generate the security concern when we use the security enclaves.
when this ros2 daemon is bound to IP address in the LAN with security enclaves, that exposes the ROS 2 connectivity information to anyone in the LAN via XLRPC. this did not happen before, and there is a possibility that 3rd party remote machine or application can collect this info that is NOT supposed to be seen if administrator is not careful enough. maybe this is just a corner case, but looks like a minor security concern.
after all i think this enhancement makes sense, but we probably do not advertise this environmental variable for end users? if we do that, i think there needs to be some explanation how this works.
Description
This change adds a new environment variable 'ROS2_DAEMON_SERVER_URL' which can be used to change the expected endpoint for communicating with the ROS 2 daemon process.
Unspecified parts of the URL will be replaced with default values, so it's possible to omit the scheme, host, and port to retain default behavior while still overriding other parts of the URL.
There are two supporting commits in this PR which shouldn't result in any user-facing changes alone, and are separated for review purposes. They may either be squashed upon merging or retained as individuals by a rebase merge.
Is this user-facing behavior change?
Yes, users can now use the
ROS2_DAEMON_SERVER_URL
environment variable to control the endpoint over which daemon communication occurs.Did you use Generative AI?
No
Additional Information
The long-term goal of this work is to support daemon isolation for parallelized testing in other packages.