Fix minion startup failure on Windows when ipv6: true is set

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.

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:

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

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
 

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

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:

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:

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)
 

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()

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."""
+    io_loop = tornado.ioloop.IOLoop()
+    publisher = salt.transport.tcp._TCPPubServerPublisher(
+        host="127.0.0.1", port=4511, path=None, io_loop=io_loop
+    )
+    captured_family = []
+
+    def fake_socket(family, *args, **kwargs):
+        captured_family.append(family)
+        raise OSError("test abort")
+
+    publisher._connecting_future = tornado.concurrent.Future()
+
+    with patch("salt.transport.tcp.socket.socket", fake_socket):
+        try:
+            io_loop.run_sync(publisher._connect, timeout=3)
+        except Exception:
+            pass
+
+    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, io_loop=io_loop
+    )
+    captured_family = []
+
+    def fake_socket(family, *args, **kwargs):
+        captured_family.append(family)
+        raise OSError("test abort")
+
+    publisher._connecting_future = tornado.concurrent.Future()
+
+    with patch("salt.transport.tcp.socket.socket", fake_socket):
+        try:
+            io_loop.run_sync(publisher._connect, timeout=3)
+        except Exception:
+            pass
+
+    io_loop.close()
+    assert captured_family == [socket.AF_INET6]
+
+
 @pytest.mark.usefixtures("_squash_exepected_message_client_warning")
 async def test_message_client_cleanup_on_close(client_socket, temp_salt_master):
     """

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):