From 13190ff89a6690601d52ce935ddf1c55c264314a Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 31 Jul 2023 14:13:38 -0700 Subject: [PATCH] Use fixtures when already present --- salt/transport/base.py | 3 +- salt/transport/tcp.py | 20 ++--- tests/pytests/unit/conftest.py | 2 +- tests/pytests/unit/test_auth.py | 63 ++++++--------- tests/pytests/unit/test_master.py | 78 ++++++++----------- tests/pytests/unit/transport/test_ipc.py | 14 ++-- .../unit/transport/test_publish_client.py | 3 +- 7 files changed, 72 insertions(+), 111 deletions(-) diff --git a/salt/transport/base.py b/salt/transport/base.py index 7fdd733f938..78d5f11dd32 100644 --- a/salt/transport/base.py +++ b/salt/transport/base.py @@ -48,7 +48,8 @@ def request_client(opts, io_loop): elif ttype == "tcp": import salt.transport.tcp - return salt.transport.tcp.TCPReqClient(opts, io_loop=io_loop) + resolver = salt.transport.tcp.Resolver() + return salt.transport.tcp.TCPReqClient(opts, resolver=resolver, io_loop=io_loop) else: raise Exception("Channels are only defined for tcp, zeromq") diff --git a/salt/transport/tcp.py b/salt/transport/tcp.py index cfee7ba0a1f..12011157fbd 100644 --- a/salt/transport/tcp.py +++ b/salt/transport/tcp.py @@ -1730,19 +1730,13 @@ class TCPReqClient(salt.transport.base.RequestClient): stream = None while stream is None and (not self._closed and not self._closing): try: - if self.host and self.port: - stream = await self._tcp_client.connect( - ip_bracket(self.host, strip=True), - self.port, - ssl_options=self.opts.get("ssl"), - **kwargs, - ) - else: - sock_type = socket.AF_UNIX - stream = tornado.iostream.IOStream( - socket.socket(sock_type, socket.SOCK_STREAM) - ) - await stream.connect(path) + # XXX: Support ipc sockets too + stream = await self._tcp_client.connect( + ip_bracket(self.host, strip=True), + self.port, + ssl_options=self.opts.get("ssl"), + **kwargs, + ) except Exception as exc: # pylint: disable=broad-except log.warning( "TCP Message Client encountered an exception while connecting to" diff --git a/tests/pytests/unit/conftest.py b/tests/pytests/unit/conftest.py index 43deeaa618e..0f8478f51b1 100644 --- a/tests/pytests/unit/conftest.py +++ b/tests/pytests/unit/conftest.py @@ -26,7 +26,7 @@ def master_opts(tmp_path): Default master configuration with relative temporary paths to not require root permissions. """ root_dir = tmp_path / "master" - opts = salt.config.DEFAULT_MASTER_OPTS.copy() + opts = salt.config.master_config(None) opts["__role"] = "master" opts["root_dir"] = str(root_dir) for name in ("cachedir", "pki_dir", "sock_dir", "conf_dir"): diff --git a/tests/pytests/unit/test_auth.py b/tests/pytests/unit/test_auth.py index e37dfdbe87e..cb0aceef0cd 100644 --- a/tests/pytests/unit/test_auth.py +++ b/tests/pytests/unit/test_auth.py @@ -47,15 +47,14 @@ def load_auth(): @pytest.fixture -def master_acl_master_opts(): - opts = salt.config.master_config(None) - opts["publisher_acl"] = {} - opts["publisher_acl_blacklist"] = {} - opts["master_job_cache"] = "" - opts["sign_pub_messages"] = False - opts["con_cache"] = "" - opts["external_auth"] = {} - opts["external_auth"]["pam"] = { +def master_acl_master_opts(master_opts): + master_opts["publisher_acl"] = {} + master_opts["publisher_acl_blacklist"] = {} + master_opts["master_job_cache"] = "" + master_opts["sign_pub_messages"] = False + master_opts["con_cache"] = "" + master_opts["external_auth"] = {} + master_opts["external_auth"]["pam"] = { "test_user": [ {"*": ["test.ping"]}, {"minion_glob*": ["foo.bar"]}, @@ -93,7 +92,7 @@ def master_acl_master_opts(): }, ], } - yield opts + yield master_opts @pytest.fixture @@ -150,19 +149,18 @@ def master_acl_valid_load(): @pytest.fixture -def auth_acl_master_opts(): +def auth_acl_master_opts(master_opts): """ Master options """ - opts = salt.config.master_config(None) - opts["publisher_acl"] = {} - opts["publisher_acl_blacklist"] = {} - opts["master_job_cache"] = "" - opts["sign_pub_messages"] = False - opts["con_cache"] = "" - opts["external_auth"] = {} - opts["external_auth"] = {"pam": {"test_user": [{"alpha_minion": ["test.ping"]}]}} - yield opts + master_opts["publisher_acl"] = {} + master_opts["publisher_acl_blacklist"] = {} + master_opts["master_job_cache"] = "" + master_opts["sign_pub_messages"] = False + master_opts["con_cache"] = "" + master_opts["external_auth"] = {} + master_opts["external_auth"] = {"pam": {"test_user": [{"alpha_minion": ["test.ping"]}]}} + yield master_opts @pytest.fixture @@ -372,8 +370,8 @@ async def test_master_publish_group(master_acl_clear_funcs, master_acl_valid_loa # Request sys.doc master_acl_valid_load["fun"] = "sys.doc" - # XXX: Of course we won't fire an event if publihs isn't called. If - # sys.dock is there wwhen we publish is that a bug? + # XXX: Of course we won't fire an event if publish isn't called. If + # sys.dock is there when we publish is that a bug? # await master_acl_clear_funcs.publish(master_acl_valid_load) @@ -457,19 +455,6 @@ async def test_master_minion_glob(master_acl_clear_funcs, master_acl_valid_load) ), "Did not fire {} for minion glob".format(requested_function) -async def test_master_function_glob(master_acl_clear_funcs, master_acl_valid_load): - """ - Test to ensure that we can allow access to a given - set of functions in an execution module as selected - by a glob. ex: - - my_user: - my_minion: - 'test.*' - """ - # Unimplemented - - @pytest.mark.skip_on_windows(reason="PAM eauth not available on Windows") async def test_args_empty_spec(master_acl_clear_funcs, master_acl_valid_load): """ @@ -636,10 +621,10 @@ async def test_args_kwargs_match(master_acl_clear_funcs, master_acl_valid_load): Test simple kwargs restriction allowed. 'test_user_func': - '*': - - test.echo: - kwargs: - text: 'KWMSG:.*' + '*': + - test.echo: + kwargs: + text: 'KWMSG:.*' """ _check_minions_return = {"minions": ["some_minions"], "missing": []} with patch( diff --git a/tests/pytests/unit/test_master.py b/tests/pytests/unit/test_master.py index 7436474b16c..793ed7d2292 100644 --- a/tests/pytests/unit/test_master.py +++ b/tests/pytests/unit/test_master.py @@ -7,14 +7,6 @@ import salt.utils.platform from tests.support.mock import MagicMock, patch -@pytest.fixture -def master_opts(): - """ - Master options - """ - yield salt.config.master_config(None) - - @pytest.fixture def maintenence_opts(master_opts): """ @@ -204,12 +196,11 @@ def test_when_syndic_return_processes_load_then_correct_values_should_be_returne fake_return.assert_called_with(expected_return) -def test_aes_funcs_white(): +def test_aes_funcs_white(master_opts): """ Validate methods exposed on AESFuncs exist and are callable """ - opts = salt.config.master_config(None) - aes_funcs = salt.master.AESFuncs(opts) + aes_funcs = salt.master.AESFuncs(master_opts) try: for name in aes_funcs.expose_methods: func = getattr(aes_funcs, name, None) @@ -233,12 +224,11 @@ def test_transport_methods(): assert foo.get_method("bang") is None -def test_aes_funcs_black(): +def test_aes_funcs_black(master_opts): """ Validate methods on AESFuncs that should not be called remotely """ - opts = salt.config.master_config(None) - aes_funcs = salt.master.AESFuncs(opts) + aes_funcs = salt.master.AESFuncs(master_opts) # Any callable that should not explicitly be allowed should be added # here. blacklist_methods = [ @@ -284,12 +274,11 @@ def test_aes_funcs_black(): aes_funcs.destroy() -def test_clear_funcs_white(): +def test_clear_funcs_white(master_opts): """ Validate methods exposed on ClearFuncs exist and are callable """ - opts = salt.config.master_config(None) - clear_funcs = salt.master.ClearFuncs(opts, {}) + clear_funcs = salt.master.ClearFuncs(master_opts, {}) try: for name in clear_funcs.expose_methods: func = getattr(clear_funcs, name, None) @@ -298,12 +287,11 @@ def test_clear_funcs_white(): clear_funcs.destroy() -def test_clear_funcs_black(): +def test_clear_funcs_black(master_opts): """ Validate methods on ClearFuncs that should not be called remotely """ - opts = salt.config.master_config(None) - clear_funcs = salt.master.ClearFuncs(opts, {}) + clear_funcs = salt.master.ClearFuncs(master_opts, {}) blacklist_methods = [ "__class__", "__delattr__", @@ -365,7 +353,7 @@ def test_runner_token_not_authenticated(clear_funcs): } } ret = clear_funcs.runner({"token": "asdfasdfasdfasdf"}) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -391,7 +379,7 @@ def test_runner_token_authorization_error(clear_funcs): ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=[])): ret = clear_funcs.runner(clear_load) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -415,7 +403,7 @@ def test_runner_token_salt_invocation_error(clear_funcs): ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=["testing"])): ret = clear_funcs.runner(clear_load) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -432,7 +420,7 @@ def test_runner_eauth_not_authenticated(clear_funcs): } } ret = clear_funcs.runner({"eauth": "foo"}) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -455,7 +443,7 @@ def test_runner_eauth_authorization_error(clear_funcs): ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=[])): ret = clear_funcs.runner(clear_load) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -476,7 +464,7 @@ def test_runner_eauth_salt_invocation_error(clear_funcs): ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=["testing"])): ret = clear_funcs.runner(clear_load) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -491,7 +479,7 @@ def test_runner_user_not_authenticated(clear_funcs): } } ret = clear_funcs.runner({}) - assert mock_ret == ret + assert ret == mock_ret # wheel tests @@ -509,7 +497,7 @@ def test_wheel_token_not_authenticated(clear_funcs): } } ret = clear_funcs.wheel({"token": "asdfasdfasdfasdf"}) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -534,8 +522,7 @@ def test_wheel_token_authorization_error(clear_funcs): "salt.auth.LoadAuth.authenticate_token", MagicMock(return_value=mock_token) ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=[])): ret = clear_funcs.wheel(clear_load) - - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -558,8 +545,7 @@ def test_wheel_token_salt_invocation_error(clear_funcs): "salt.auth.LoadAuth.authenticate_token", MagicMock(return_value=mock_token) ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=["testing"])): ret = clear_funcs.wheel(clear_load) - - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -576,7 +562,7 @@ def test_wheel_eauth_not_authenticated(clear_funcs): } } ret = clear_funcs.wheel({"eauth": "foo"}) - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -598,8 +584,7 @@ def test_wheel_eauth_authorization_error(clear_funcs): "salt.auth.LoadAuth.authenticate_eauth", MagicMock(return_value=True) ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=[])): ret = clear_funcs.wheel(clear_load) - - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -619,8 +604,7 @@ def test_wheel_eauth_salt_invocation_error(clear_funcs): "salt.auth.LoadAuth.authenticate_eauth", MagicMock(return_value=True) ), patch("salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=["testing"])): ret = clear_funcs.wheel(clear_load) - - assert mock_ret == ret + assert ret == mock_ret @pytest.mark.slow_test @@ -635,7 +619,7 @@ def test_wheel_user_not_authenticated(clear_funcs): } } ret = clear_funcs.wheel({}) - assert mock_ret == ret + assert ret == mock_ret # publish tests @@ -655,7 +639,7 @@ async def test_publish_user_is_blacklisted(clear_funcs): with patch( "salt.acl.PublisherACL.user_is_blacklisted", MagicMock(return_value=True) ): - assert mock_ret == await clear_funcs.publish({"user": "foo", "fun": "test.arg"}) + assert await clear_funcs.publish({"user": "foo", "fun": "test.arg"}) == mock_ret @pytest.mark.slow_test @@ -672,7 +656,7 @@ async def test_publish_cmd_blacklisted(clear_funcs): with patch( "salt.acl.PublisherACL.user_is_blacklisted", MagicMock(return_value=False) ), patch("salt.acl.PublisherACL.cmd_is_blacklisted", MagicMock(return_value=True)): - assert mock_ret == await clear_funcs.publish({"user": "foo", "fun": "test.arg"}) + assert await clear_funcs.publish({"user": "foo", "fun": "test.arg"}) == mock_ret @pytest.mark.slow_test @@ -695,7 +679,7 @@ async def test_publish_token_not_authenticated(clear_funcs): with patch( "salt.acl.PublisherACL.user_is_blacklisted", MagicMock(return_value=False) ), patch("salt.acl.PublisherACL.cmd_is_blacklisted", MagicMock(return_value=False)): - assert mock_ret == await clear_funcs.publish(load) + assert await clear_funcs.publish(load) == mock_ret @pytest.mark.slow_test @@ -729,7 +713,7 @@ async def test_publish_token_authorization_error(clear_funcs): ), patch( "salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=[]) ): - assert mock_ret == await clear_funcs.publish(load) + assert await clear_funcs.publish(load) == mock_ret @pytest.mark.slow_test @@ -752,7 +736,7 @@ async def test_publish_eauth_not_authenticated(clear_funcs): with patch( "salt.acl.PublisherACL.user_is_blacklisted", MagicMock(return_value=False) ), patch("salt.acl.PublisherACL.cmd_is_blacklisted", MagicMock(return_value=False)): - assert mock_ret == await clear_funcs.publish(load) + assert await clear_funcs.publish(load) == mock_ret @pytest.mark.slow_test @@ -783,7 +767,7 @@ async def test_publish_eauth_authorization_error(clear_funcs): ), patch( "salt.auth.LoadAuth.get_auth_list", MagicMock(return_value=[]) ): - assert mock_ret == await clear_funcs.publish(load) + assert await clear_funcs.publish(load) == mock_ret @pytest.mark.slow_test @@ -801,7 +785,7 @@ async def test_publish_user_not_authenticated(clear_funcs): with patch( "salt.acl.PublisherACL.user_is_blacklisted", MagicMock(return_value=False) ), patch("salt.acl.PublisherACL.cmd_is_blacklisted", MagicMock(return_value=False)): - assert mock_ret == await clear_funcs.publish(load) + assert await clear_funcs.publish(load) == mock_ret @pytest.mark.slow_test @@ -833,7 +817,7 @@ async def test_publish_user_authenticated_missing_auth_list(clear_funcs): ), patch( "salt.utils.master.get_values_of_matching_keys", MagicMock(return_value=[]) ): - assert mock_ret == await clear_funcs.publish(load) + assert await clear_funcs.publish(load) == mock_ret @pytest.mark.slow_test @@ -868,7 +852,7 @@ async def test_publish_user_authorization_error(clear_funcs): ), patch( "salt.utils.minions.CkMinions.auth_check", MagicMock(return_value=False) ): - assert mock_ret == await clear_funcs.publish(load) + assert await clear_funcs.publish(load) == mock_ret def test_run_func(maintenence): diff --git a/tests/pytests/unit/transport/test_ipc.py b/tests/pytests/unit/transport/test_ipc.py index ea7b35b8003..50761d9ff51 100644 --- a/tests/pytests/unit/transport/test_ipc.py +++ b/tests/pytests/unit/transport/test_ipc.py @@ -23,10 +23,9 @@ def sock_dir(tmp_path): @pytest.fixture -def minion_config(sock_dir): - minion_config = salt.config.minion_config("") - minion_config["sock_dir"] = sock_dir - yield minion_config +def minion_config(sock_dir, minion_opts): + minion_opts["sock_dir"] = sock_dir + yield minion_opts def test_ipc_connect_in_async_methods(): @@ -55,10 +54,9 @@ async def test_ipc_connect_sync_wrapped(io_loop, tmp_path): @pytest.fixture -def master_config(sock_dir): - conf = salt.config.master_config("") - conf["sock_dir"] = sock_dir - yield conf +def master_config(sock_dir, master_opts): + master_opts["sock_dir"] = sock_dir + yield master_opts @pytest.mark.skip_on_windows(reason="Unix socket not available on win32") diff --git a/tests/pytests/unit/transport/test_publish_client.py b/tests/pytests/unit/transport/test_publish_client.py index 18147f1831f..e7f82f578c5 100644 --- a/tests/pytests/unit/transport/test_publish_client.py +++ b/tests/pytests/unit/transport/test_publish_client.py @@ -182,7 +182,6 @@ async def test_publish_client_connect_server_down(transport, io_loop): async def test_publish_client_connect_server_comes_up(transport, io_loop): - print(io_loop) opts = {"master_ip": "127.0.0.1"} host = "127.0.0.1" port = 11122 @@ -194,7 +193,7 @@ async def test_publish_client_connect_server_comes_up(transport, io_loop): ctx = zmq.asyncio.Context() uri = f"tcp://{opts['master_ip']}:{port}" msg = salt.payload.dumps({"meh": 123}) - print(f"send {msg}") + log.debug("TEST - Senging %r", msg) client = salt.transport.zeromq.PublishClient( opts, io_loop, host=host, port=port )