From 1a2dfd41ba3907dea944b925f5aec18101bb1725 Mon Sep 17 00:00:00 2001 From: twangboy Date: Wed, 6 May 2026 15:47:16 -0600 Subject: [PATCH 1/3] Fix editable installs for troubleshooting --- tools/pkg/salt_build_backend.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tools/pkg/salt_build_backend.py b/tools/pkg/salt_build_backend.py index 1901df771a43..cd501c5206da 100644 --- a/tools/pkg/salt_build_backend.py +++ b/tools/pkg/salt_build_backend.py @@ -7,6 +7,10 @@ sys.path.insert(0, PROJECT_ROOT) from setuptools import build_meta as _orig +from setuptools.build_meta import build_editable as setuptools_build_editable +from setuptools.build_meta import ( + prepare_metadata_for_build_editable as setuptools_prepare_metadata, +) # PEP 517 hooks prepare_metadata_for_build_wheel = _orig.prepare_metadata_for_build_wheel @@ -16,6 +20,18 @@ get_requires_for_build_sdist = _orig.get_requires_for_build_sdist +def build_editable(wheel_directory, config_settings=None, metadata_directory=None): + # Delegate to the actual setuptools implementation + return setuptools_build_editable( + wheel_directory, config_settings, metadata_directory + ) + + +def prepare_metadata_for_build_editable(metadata_directory, config_settings=None): + # Delegate to setuptools to get the metadata + return setuptools_prepare_metadata(metadata_directory, config_settings) + + def _parse_requirements_file(requirements_file): parsed_requirements = [] if not os.path.exists(requirements_file): From 154524ba47bf132e3e2ce9e99057cb2614601939 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 15 May 2026 21:09:00 -0600 Subject: [PATCH 2/3] Fix minion startup failure on Windows when ipv6: true is set Three IPC socket paths in the TCP transport hardcoded AF_INET / 127.0.0.1 regardless of the ipv6 option. On Windows, binding or connecting an AF_INET6 socket to an IPv4 address (or vice-versa) is rejected by the OS, causing the minion to fail to start with gaierror(11001). - Add _ipc_loopback(opts) helper to salt.transport.base; update ipc_publish_server() and ipc_publish_client() to pass ::1 instead of 127.0.0.1 when ipv6: true is set - Fix TCPPuller.start() to select AF_INET6 when the host contains ':' - Fix _TCPPubServerPublisher._connect() with the same family detection Add unit tests for all three fixes. Update the ipv6 option documentation in master.rst, minion.rst, and the default config files to explain the IPC loopback address behaviour. (fixes #66603) --- changelog/66603.fixed.md | 9 ++ conf/master | 4 + conf/minion | 4 + doc/ref/configuration/master.rst | 7 +- doc/ref/configuration/minion.rst | 5 + pkg/common/conf/master | 4 + salt/transport/base.py | 24 +++-- salt/transport/tcp.py | 5 +- tests/pytests/unit/transport/test_base.py | 122 +++++++++++++++++++++- tests/pytests/unit/transport/test_tcp.py | 86 +++++++++++++++ 10 files changed, 260 insertions(+), 10 deletions(-) create mode 100644 changelog/66603.fixed.md diff --git a/changelog/66603.fixed.md b/changelog/66603.fixed.md new file mode 100644 index 000000000000..a3bd1d948e9f --- /dev/null +++ b/changelog/66603.fixed.md @@ -0,0 +1,9 @@ +Fixed a regression where setting ``ipv6: true`` in the minion configuration +caused the minion to fail to start on Windows. Three IPC socket paths in the +TCP transport hardcoded ``AF_INET`` or ``127.0.0.1`` regardless of the IPv6 +setting: the IPC publish server/client addresses in ``salt.transport.base``, +the ``TCPPuller`` server socket, and the ``_TCPPubServerPublisher`` client +socket. On Windows, mixing an ``AF_INET6`` socket with the IPv4 loopback +address (or vice-versa) is rejected by the OS. All three paths now use +``::1`` with ``AF_INET6`` when ``ipv6: true`` is set, and ``127.0.0.1`` +with ``AF_INET`` otherwise. diff --git a/conf/master b/conf/master index ab37e280ef2b..b72d89ef00d1 100644 --- a/conf/master +++ b/conf/master @@ -16,6 +16,10 @@ # Whether the master should listen for IPv6 connections. If this is set to True, # the interface option must be adjusted, too. (For example: "interface: '::'") +# When enabled, Salt also uses the IPv6 loopback address (::1) for internal +# IPC connections between daemon processes instead of 127.0.0.1. On Windows, +# this is required because Windows does not permit binding an AF_INET6 socket +# to an IPv4 address. #ipv6: False # The tcp port used by the publisher: diff --git a/conf/minion b/conf/minion index 70cbe8934a45..323299a04feb 100644 --- a/conf/minion +++ b/conf/minion @@ -63,6 +63,10 @@ #master_failback_interval: 0 # Set whether the minion should connect to the master via IPv6: +# When set to True, Salt also uses the IPv6 loopback address (::1) for +# internal IPC connections instead of 127.0.0.1. On Windows, this is +# required because Windows does not permit binding an AF_INET6 socket to +# an IPv4 address. #ipv6: False # Set the number of seconds to wait before attempting to resolve diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 06cf14bbf742..c147bdae1449 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -46,7 +46,12 @@ The local interface to bind to, must be an IP address. Default: ``False`` Whether the master should listen for IPv6 connections. If this is set to True, -the interface option must be adjusted too (for example: ``interface: '::'``) +the interface option must be adjusted too (for example: ``interface: '::'``). + +When enabled, Salt also uses the IPv6 loopback address (``::1``) for internal +IPC connections between daemon processes instead of ``127.0.0.1``. On Windows, +this is required because Windows does not permit binding an ``AF_INET6`` socket +to an IPv4 address. .. code-block:: yaml diff --git a/doc/ref/configuration/minion.rst b/doc/ref/configuration/minion.rst index 590320b0b052..8441baa2b662 100644 --- a/doc/ref/configuration/minion.rst +++ b/doc/ref/configuration/minion.rst @@ -123,6 +123,11 @@ Default: ``None`` Whether the master should be connected over IPv6. By default salt minion will try to automatically detect IPv6 connectivity to master. +When set to ``True``, Salt also uses the IPv6 loopback address (``::1``) for +internal IPC connections instead of ``127.0.0.1``. On Windows, this is +required because Windows does not permit binding an ``AF_INET6`` socket to an +IPv4 address. + .. code-block:: yaml ipv6: True diff --git a/pkg/common/conf/master b/pkg/common/conf/master index 4f0fa646d495..9869057fea59 100644 --- a/pkg/common/conf/master +++ b/pkg/common/conf/master @@ -16,6 +16,10 @@ # Whether the master should listen for IPv6 connections. If this is set to True, # the interface option must be adjusted, too. (For example: "interface: '::'") +# When enabled, Salt also uses the IPv6 loopback address (::1) for internal +# IPC connections between daemon processes instead of 127.0.0.1. On Windows, +# this is required because Windows does not permit binding an AF_INET6 socket +# to an IPv4 address. #ipv6: False # The tcp port used by the publisher: diff --git a/salt/transport/base.py b/salt/transport/base.py index 6b9e2636f017..c185d2cb7a3b 100644 --- a/salt/transport/base.py +++ b/salt/transport/base.py @@ -188,18 +188,30 @@ def _minion_hash(hash_type, minion_id): return hasher(salt.utils.stringutils.to_bytes(minion_id)).hexdigest()[:10] +def _ipc_loopback(opts): + """ + Return the loopback address appropriate for the configured IP family. + + IPC connections always bind to loopback. When ``ipv6: true`` is set the + socket layer creates an ``AF_INET6`` socket, so the address must be the + IPv6 loopback ``::1`` rather than the IPv4 ``127.0.0.1``; binding an + ``AF_INET6`` socket to ``127.0.0.1`` fails on Windows. + """ + return "::1" if opts.get("ipv6", False) else "127.0.0.1" + + def ipc_publish_client(node, opts, io_loop): # Default to TCP for now kwargs = {"transport": "tcp", "ssl": None} if opts["ipc_mode"] == "tcp": if node == "master": kwargs.update( - host="127.0.0.1", + host=_ipc_loopback(opts), port=int(opts["tcp_master_pub_port"]), ) else: kwargs.update( - host="127.0.0.1", + host=_ipc_loopback(opts), port=int(opts["tcp_pub_port"]), ) else: @@ -232,16 +244,16 @@ def ipc_publish_server(node, opts): if opts["ipc_mode"] == "tcp": if node == "master": kwargs.update( - pub_host="127.0.0.1", + pub_host=_ipc_loopback(opts), pub_port=int(opts["tcp_master_pub_port"]), - pull_host="127.0.0.1", + pull_host=_ipc_loopback(opts), pull_port=int(opts["tcp_master_pull_port"]), ) else: kwargs.update( - pub_host="127.0.0.1", + pub_host=_ipc_loopback(opts), pub_port=int(opts["tcp_pub_port"]), - pull_host="127.0.0.1", + pull_host=_ipc_loopback(opts), pull_port=int(opts["tcp_pull_port"]), ) else: diff --git a/salt/transport/tcp.py b/salt/transport/tcp.py index d51aa7bbf652..4f0f43b345d1 100644 --- a/salt/transport/tcp.py +++ b/salt/transport/tcp.py @@ -1222,7 +1222,8 @@ def start(self): self.sock = tornado.netutil.bind_unix_socket(self.path, self.mode) else: log.trace("IPCServer: binding to socket: %s:%s", self.host, self.port) - self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + family = socket.AF_INET6 if ":" in (self.host or "") else socket.AF_INET + self.sock = socket.socket(family, socket.SOCK_STREAM) self.sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) self.sock.setblocking(0) self.sock.bind((self.host, self.port)) @@ -1652,7 +1653,7 @@ async def _connect(self, timeout=None): sock_addr = self.path log.debug("Publisher connecting to %s", self.path) else: - sock_type = socket.AF_INET + sock_type = socket.AF_INET6 if ":" in (self.host or "") else socket.AF_INET sock_addr = (self.host, self.port) log.debug("Publisher connecting to %s:%s", self.host, self.port) diff --git a/tests/pytests/unit/transport/test_base.py b/tests/pytests/unit/transport/test_base.py index 5d541073d676..dbd24a105ee5 100644 --- a/tests/pytests/unit/transport/test_base.py +++ b/tests/pytests/unit/transport/test_base.py @@ -7,13 +7,133 @@ import pytest import salt.transport.base -from tests.support.mock import patch +from tests.support.mock import MagicMock, patch pytestmark = [ pytest.mark.core_test, ] +# --------------------------------------------------------------------------- +# _ipc_loopback +# --------------------------------------------------------------------------- + + +def test_ipc_loopback_ipv4(): + assert salt.transport.base._ipc_loopback({}) == "127.0.0.1" + assert salt.transport.base._ipc_loopback({"ipv6": False}) == "127.0.0.1" + + +def test_ipc_loopback_ipv6(): + assert salt.transport.base._ipc_loopback({"ipv6": True}) == "::1" + + +# --------------------------------------------------------------------------- +# ipc_publish_client +# --------------------------------------------------------------------------- + +_IPC_TCP_OPTS = { + "ipc_mode": "tcp", + "ipv6": False, + "tcp_pub_port": 4510, + "tcp_pull_port": 4511, + "tcp_master_pub_port": 4512, + "tcp_master_pull_port": 4513, + "hash_type": "sha256", + "id": "test-minion", + "sock_dir": "/var/run/salt", +} + + +def test_ipc_publish_client_minion_ipv4(): + """ipc_publish_client uses 127.0.0.1 for minion when ipv6 is False.""" + opts = {**_IPC_TCP_OPTS, "ipv6": False} + mock_client = MagicMock() + with patch("salt.transport.base.publish_client", return_value=mock_client) as mock: + salt.transport.base.ipc_publish_client("minion", opts, io_loop=None) + _, kwargs = mock.call_args + assert kwargs["host"] == "127.0.0.1" + + +def test_ipc_publish_client_minion_ipv6(): + """ipc_publish_client uses ::1 for minion when ipv6 is True.""" + opts = {**_IPC_TCP_OPTS, "ipv6": True} + mock_client = MagicMock() + with patch("salt.transport.base.publish_client", return_value=mock_client) as mock: + salt.transport.base.ipc_publish_client("minion", opts, io_loop=None) + _, kwargs = mock.call_args + assert kwargs["host"] == "::1" + + +def test_ipc_publish_client_master_ipv4(): + """ipc_publish_client uses 127.0.0.1 for master when ipv6 is False.""" + opts = {**_IPC_TCP_OPTS, "ipv6": False} + mock_client = MagicMock() + with patch("salt.transport.base.publish_client", return_value=mock_client) as mock: + salt.transport.base.ipc_publish_client("master", opts, io_loop=None) + _, kwargs = mock.call_args + assert kwargs["host"] == "127.0.0.1" + + +def test_ipc_publish_client_master_ipv6(): + """ipc_publish_client uses ::1 for master when ipv6 is True.""" + opts = {**_IPC_TCP_OPTS, "ipv6": True} + mock_client = MagicMock() + with patch("salt.transport.base.publish_client", return_value=mock_client) as mock: + salt.transport.base.ipc_publish_client("master", opts, io_loop=None) + _, kwargs = mock.call_args + assert kwargs["host"] == "::1" + + +# --------------------------------------------------------------------------- +# ipc_publish_server +# --------------------------------------------------------------------------- + + +def test_ipc_publish_server_minion_ipv4(): + """ipc_publish_server uses 127.0.0.1 for minion when ipv6 is False.""" + opts = {**_IPC_TCP_OPTS, "ipv6": False} + mock_server = MagicMock() + with patch("salt.transport.base.publish_server", return_value=mock_server) as mock: + salt.transport.base.ipc_publish_server("minion", opts) + _, kwargs = mock.call_args + assert kwargs["pub_host"] == "127.0.0.1" + assert kwargs["pull_host"] == "127.0.0.1" + + +def test_ipc_publish_server_minion_ipv6(): + """ipc_publish_server uses ::1 for minion when ipv6 is True.""" + opts = {**_IPC_TCP_OPTS, "ipv6": True} + mock_server = MagicMock() + with patch("salt.transport.base.publish_server", return_value=mock_server) as mock: + salt.transport.base.ipc_publish_server("minion", opts) + _, kwargs = mock.call_args + assert kwargs["pub_host"] == "::1" + assert kwargs["pull_host"] == "::1" + + +def test_ipc_publish_server_master_ipv4(): + """ipc_publish_server uses 127.0.0.1 for master when ipv6 is False.""" + opts = {**_IPC_TCP_OPTS, "ipv6": False} + mock_server = MagicMock() + with patch("salt.transport.base.publish_server", return_value=mock_server) as mock: + salt.transport.base.ipc_publish_server("master", opts) + _, kwargs = mock.call_args + assert kwargs["pub_host"] == "127.0.0.1" + assert kwargs["pull_host"] == "127.0.0.1" + + +def test_ipc_publish_server_master_ipv6(): + """ipc_publish_server uses ::1 for master when ipv6 is True.""" + opts = {**_IPC_TCP_OPTS, "ipv6": True} + mock_server = MagicMock() + with patch("salt.transport.base.publish_server", return_value=mock_server) as mock: + salt.transport.base.ipc_publish_server("master", opts) + _, kwargs = mock.call_args + assert kwargs["pub_host"] == "::1" + assert kwargs["pull_host"] == "::1" + + def test_unclosed_warning(): transport = salt.transport.base.Transport() diff --git a/tests/pytests/unit/transport/test_tcp.py b/tests/pytests/unit/transport/test_tcp.py index 68f9165935ff..19a7b29d1869 100644 --- a/tests/pytests/unit/transport/test_tcp.py +++ b/tests/pytests/unit/transport/test_tcp.py @@ -119,6 +119,92 @@ def test_get_bind_addr(): assert res == ("192.168.0.1", 1) +def test_tcppuller_start_ipv4(): + """TCPPuller uses AF_INET when host is an IPv4 address.""" + puller = salt.transport.tcp.TCPPuller(host="127.0.0.1", port=4511) + created_sockets = [] + + def fake_socket(family, *args, **kwargs): + sock = MagicMock() + sock.family = family + created_sockets.append(sock) + return sock + + with patch("salt.transport.tcp.socket.socket", side_effect=fake_socket): + with patch("tornado.netutil.add_accept_handler"): + puller.start() + + assert len(created_sockets) == 1 + assert created_sockets[0].family == socket.AF_INET + + +def test_tcppuller_start_ipv6(): + """TCPPuller uses AF_INET6 when host is an IPv6 address.""" + puller = salt.transport.tcp.TCPPuller(host="::1", port=4511) + created_sockets = [] + + def fake_socket(family, *args, **kwargs): + sock = MagicMock() + sock.family = family + created_sockets.append(sock) + return sock + + with patch("salt.transport.tcp.socket.socket", side_effect=fake_socket): + with patch("tornado.netutil.add_accept_handler"): + puller.start() + + assert len(created_sockets) == 1 + assert created_sockets[0].family == socket.AF_INET6 + + +def test_tcppubserverpublisher_connect_ipv4(): + """_TCPPubServerPublisher uses AF_INET when connecting to an IPv4 address.""" + publisher = salt.transport.tcp._TCPPubServerPublisher( + host="127.0.0.1", port=4511, path=None + ) + created_sockets = [] + + def fake_socket(family, *args, **kwargs): + sock = MagicMock() + sock.family = family + created_sockets.append(sock) + return sock + + async def run(): + publisher._connecting_future = tornado.concurrent.Future() + publisher._connecting_future.set_result(True) + with patch("salt.transport.tcp.socket.socket", side_effect=fake_socket): + with patch.object(tornado.iostream.IOStream, "connect", return_value=None): + await publisher._connect() + + publisher.io_loop.run_sync(run) + assert any(s.family == socket.AF_INET for s in created_sockets) + + +def test_tcppubserverpublisher_connect_ipv6(): + """_TCPPubServerPublisher uses AF_INET6 when connecting to an IPv6 address.""" + publisher = salt.transport.tcp._TCPPubServerPublisher( + host="::1", port=4511, path=None + ) + created_sockets = [] + + def fake_socket(family, *args, **kwargs): + sock = MagicMock() + sock.family = family + created_sockets.append(sock) + return sock + + async def run(): + publisher._connecting_future = tornado.concurrent.Future() + publisher._connecting_future.set_result(True) + with patch("salt.transport.tcp.socket.socket", side_effect=fake_socket): + with patch.object(tornado.iostream.IOStream, "connect", return_value=None): + await publisher._connect() + + publisher.io_loop.run_sync(run) + assert any(s.family == socket.AF_INET6 for s in created_sockets) + + @pytest.mark.usefixtures("_squash_exepected_message_client_warning") async def test_message_client_cleanup_on_close(client_socket, temp_salt_master): """ From 8035c3a11e7153eb7da7682a490177cccb90aa1d Mon Sep 17 00:00:00 2001 From: twangboy Date: Sat, 16 May 2026 00:18:08 -0600 Subject: [PATCH 3/3] Fix test failures on Linux --- tests/pytests/unit/transport/test_tcp.py | 56 ++++++++++++------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/tests/pytests/unit/transport/test_tcp.py b/tests/pytests/unit/transport/test_tcp.py index 19a7b29d1869..f5b2adda3a3e 100644 --- a/tests/pytests/unit/transport/test_tcp.py +++ b/tests/pytests/unit/transport/test_tcp.py @@ -159,50 +159,50 @@ def fake_socket(family, *args, **kwargs): def test_tcppubserverpublisher_connect_ipv4(): """_TCPPubServerPublisher uses AF_INET when connecting to an IPv4 address.""" + io_loop = tornado.ioloop.IOLoop() publisher = salt.transport.tcp._TCPPubServerPublisher( - host="127.0.0.1", port=4511, path=None + host="127.0.0.1", port=4511, path=None, io_loop=io_loop ) - created_sockets = [] + captured_family = [] def fake_socket(family, *args, **kwargs): - sock = MagicMock() - sock.family = family - created_sockets.append(sock) - return sock + captured_family.append(family) + raise OSError("test abort") + + publisher._connecting_future = tornado.concurrent.Future() - async def run(): - publisher._connecting_future = tornado.concurrent.Future() - publisher._connecting_future.set_result(True) - with patch("salt.transport.tcp.socket.socket", side_effect=fake_socket): - with patch.object(tornado.iostream.IOStream, "connect", return_value=None): - await publisher._connect() + with patch("salt.transport.tcp.socket.socket", fake_socket): + try: + io_loop.run_sync(publisher._connect, timeout=3) + except Exception: + pass - publisher.io_loop.run_sync(run) - assert any(s.family == socket.AF_INET for s in created_sockets) + io_loop.close() + assert captured_family == [socket.AF_INET] def test_tcppubserverpublisher_connect_ipv6(): """_TCPPubServerPublisher uses AF_INET6 when connecting to an IPv6 address.""" + io_loop = tornado.ioloop.IOLoop() publisher = salt.transport.tcp._TCPPubServerPublisher( - host="::1", port=4511, path=None + host="::1", port=4511, path=None, io_loop=io_loop ) - created_sockets = [] + captured_family = [] def fake_socket(family, *args, **kwargs): - sock = MagicMock() - sock.family = family - created_sockets.append(sock) - return sock + captured_family.append(family) + raise OSError("test abort") + + publisher._connecting_future = tornado.concurrent.Future() - async def run(): - publisher._connecting_future = tornado.concurrent.Future() - publisher._connecting_future.set_result(True) - with patch("salt.transport.tcp.socket.socket", side_effect=fake_socket): - with patch.object(tornado.iostream.IOStream, "connect", return_value=None): - await publisher._connect() + with patch("salt.transport.tcp.socket.socket", fake_socket): + try: + io_loop.run_sync(publisher._connect, timeout=3) + except Exception: + pass - publisher.io_loop.run_sync(run) - assert any(s.family == socket.AF_INET6 for s in created_sockets) + io_loop.close() + assert captured_family == [socket.AF_INET6] @pytest.mark.usefixtures("_squash_exepected_message_client_warning")