diff --git a/changelog/58931.added.md b/changelog/58931.added.md new file mode 100644 index 000000000000..2f2810d36863 --- /dev/null +++ b/changelog/58931.added.md @@ -0,0 +1 @@ +Added metalink to mod_repo in yumpkg and documented in pkgrepo state diff --git a/changelog/65788.fixed.md b/changelog/65788.fixed.md new file mode 100644 index 000000000000..1a4f69ff6e4f --- /dev/null +++ b/changelog/65788.fixed.md @@ -0,0 +1 @@ +fix consul.acl_create rule creation diff --git a/salt/modules/consul.py b/salt/modules/consul.py index 205cfb202bd3..fc7ab6523d09 100644 --- a/salt/modules/consul.py +++ b/salt/modules/consul.py @@ -47,6 +47,8 @@ def _query( api_version="v1", data=None, query_params=None, + decode=True, + text=False, ): """ Consul object method function to construct and execute on the API URL. @@ -68,7 +70,7 @@ def _query( token = _get_token() headers = {"X-Consul-Token": token, "Content-Type": "application/json"} - base_url = urllib.parse.urljoin(consul_url, "{}/".format(api_version)) + base_url = urllib.parse.urljoin(consul_url, f"{api_version}/") url = urllib.parse.urljoin(base_url, function, False) if method == "GET": @@ -85,7 +87,8 @@ def _query( method=method, params=query_params, data=data, - decode=True, + decode=decode, + text=text, status=True, header_dict=headers, opts=__opts__, @@ -149,7 +152,7 @@ def list_(consul_url=None, token=None, key=None, **kwargs): query_params["recurse"] = "True" function = "kv/" else: - function = "kv/{}".format(key) + function = f"kv/{key}" query_params["keys"] = "True" query_params["separator"] = "/" @@ -203,7 +206,7 @@ def get(consul_url=None, key=None, token=None, recurse=False, decode=False, raw= raise SaltInvocationError('Required argument "key" is missing.') query_params = {} - function = "kv/{}".format(key) + function = f"kv/{key}" if recurse: query_params["recurse"] = "True" if raw: @@ -286,19 +289,17 @@ def put(consul_url=None, token=None, key=None, value=None, **kwargs): if "cas" in kwargs: if _current["res"]: if kwargs["cas"] == 0: - ret["message"] = "Key {} exists, index must be non-zero.".format(key) + ret["message"] = f"Key {key} exists, index must be non-zero." ret["res"] = False return ret if kwargs["cas"] != _current["data"]["ModifyIndex"]: - ret["message"] = "Key {} exists, but indexes do not match.".format(key) + ret["message"] = f"Key {key} exists, but indexes do not match." ret["res"] = False return ret query_params["cas"] = kwargs["cas"] else: - ret[ - "message" - ] = "Key {} does not exists, CAS argument can not be used.".format(key) + ret["message"] = f"Key {key} does not exists, CAS argument can not be used." ret["res"] = False return ret @@ -316,7 +317,7 @@ def put(consul_url=None, token=None, key=None, value=None, **kwargs): if _current["data"]["Session"] == kwargs["release"]: query_params["release"] = kwargs["release"] else: - ret["message"] = "{} locked by another session.".format(key) + ret["message"] = f"{key} locked by another session." ret["res"] = False return ret @@ -327,7 +328,7 @@ def put(consul_url=None, token=None, key=None, value=None, **kwargs): log.error("Key {0} does not exist. Skipping release.") data = value - function = "kv/{}".format(key) + function = f"kv/{key}" method = "PUT" res = _query( consul_url=consul_url, @@ -340,10 +341,10 @@ def put(consul_url=None, token=None, key=None, value=None, **kwargs): if res["res"]: ret["res"] = True - ret["data"] = "Added key {} with value {}.".format(key, value) + ret["data"] = f"Added key {key} with value {value}." else: ret["res"] = False - ret["data"] = "Unable to add key {} with value {}.".format(key, value) + ret["data"] = f"Unable to add key {key} with value {value}." if "error" in res: ret["error"] = res["error"] return ret @@ -396,7 +397,7 @@ def delete(consul_url=None, token=None, key=None, **kwargs): ret["res"] = False return ret - function = "kv/{}".format(key) + function = f"kv/{key}" res = _query( consul_url=consul_url, token=token, @@ -407,10 +408,10 @@ def delete(consul_url=None, token=None, key=None, **kwargs): if res["res"]: ret["res"] = True - ret["message"] = "Deleted key {}.".format(key) + ret["message"] = f"Deleted key {key}." else: ret["res"] = False - ret["message"] = "Unable to delete key {}.".format(key) + ret["message"] = f"Unable to delete key {key}." if "error" in res: ret["error"] = res["error"] return ret @@ -635,7 +636,7 @@ def agent_join(consul_url=None, token=None, address=None, **kwargs): if "wan" in kwargs: query_params["wan"] = kwargs["wan"] - function = "agent/join/{}".format(address) + function = f"agent/join/{address}" res = _query( consul_url=consul_url, function=function, @@ -680,7 +681,7 @@ def agent_leave(consul_url=None, token=None, node=None): if not node: raise SaltInvocationError('Required argument "node" is missing.') - function = "agent/force-leave/{}".format(node) + function = f"agent/force-leave/{node}" res = _query( consul_url=consul_url, function=function, @@ -690,10 +691,10 @@ def agent_leave(consul_url=None, token=None, node=None): ) if res["res"]: ret["res"] = True - ret["message"] = "Node {} put in leave state.".format(node) + ret["message"] = f"Node {node} put in leave state." else: ret["res"] = False - ret["message"] = "Unable to change state for {}.".format(node) + ret["message"] = f"Unable to change state for {node}." return ret @@ -810,11 +811,11 @@ def agent_check_deregister(consul_url=None, token=None, checkid=None): if not checkid: raise SaltInvocationError('Required argument "checkid" is missing.') - function = "agent/check/deregister/{}".format(checkid) + function = f"agent/check/deregister/{checkid}" res = _query(consul_url=consul_url, function=function, token=token, method="GET") if res["res"]: ret["res"] = True - ret["message"] = "Check {} removed from agent.".format(checkid) + ret["message"] = f"Check {checkid} removed from agent." else: ret["res"] = False ret["message"] = "Unable to remove check from agent." @@ -855,7 +856,7 @@ def agent_check_pass(consul_url=None, token=None, checkid=None, **kwargs): if "note" in kwargs: query_params["note"] = kwargs["note"] - function = "agent/check/pass/{}".format(checkid) + function = f"agent/check/pass/{checkid}" res = _query( consul_url=consul_url, function=function, @@ -865,10 +866,10 @@ def agent_check_pass(consul_url=None, token=None, checkid=None, **kwargs): ) if res["res"]: ret["res"] = True - ret["message"] = "Check {} marked as passing.".format(checkid) + ret["message"] = f"Check {checkid} marked as passing." else: ret["res"] = False - ret["message"] = "Unable to update check {}.".format(checkid) + ret["message"] = f"Unable to update check {checkid}." return ret @@ -906,7 +907,7 @@ def agent_check_warn(consul_url=None, token=None, checkid=None, **kwargs): if "note" in kwargs: query_params["note"] = kwargs["note"] - function = "agent/check/warn/{}".format(checkid) + function = f"agent/check/warn/{checkid}" res = _query( consul_url=consul_url, function=function, @@ -916,10 +917,10 @@ def agent_check_warn(consul_url=None, token=None, checkid=None, **kwargs): ) if res["res"]: ret["res"] = True - ret["message"] = "Check {} marked as warning.".format(checkid) + ret["message"] = f"Check {checkid} marked as warning." else: ret["res"] = False - ret["message"] = "Unable to update check {}.".format(checkid) + ret["message"] = f"Unable to update check {checkid}." return ret @@ -957,7 +958,7 @@ def agent_check_fail(consul_url=None, token=None, checkid=None, **kwargs): if "note" in kwargs: query_params["note"] = kwargs["note"] - function = "agent/check/fail/{}".format(checkid) + function = f"agent/check/fail/{checkid}" res = _query( consul_url=consul_url, function=function, @@ -967,14 +968,16 @@ def agent_check_fail(consul_url=None, token=None, checkid=None, **kwargs): ) if res["res"]: ret["res"] = True - ret["message"] = "Check {} marked as critical.".format(checkid) + ret["message"] = f"Check {checkid} marked as critical." else: ret["res"] = False - ret["message"] = "Unable to update check {}.".format(checkid) + ret["message"] = f"Unable to update check {checkid}." return ret -def agent_service_register(consul_url=None, token=None, **kwargs): +def agent_service_register( + consul_url=None, token=None, decode=False, text=True, **kwargs +): """ The used to add a new service, with an optional health check, to the local agent. @@ -1070,12 +1073,18 @@ def agent_service_register(consul_url=None, token=None, **kwargs): if "Interval" in check_dd: del check_dd["Interval"] # not required, so ignore it - if check_dd: + if len(check_dd.keys()) > 0: data["Check"] = check_dd # if empty, ignore it function = "agent/service/register" res = _query( - consul_url=consul_url, function=function, token=token, method="PUT", data=data + consul_url=consul_url, + function=function, + token=token, + method="PUT", + data=data, + decode=decode, + text=text, ) if res["res"]: ret["res"] = True @@ -1086,7 +1095,9 @@ def agent_service_register(consul_url=None, token=None, **kwargs): return ret -def agent_service_deregister(consul_url=None, token=None, serviceid=None): +def agent_service_deregister( + consul_url=None, token=None, serviceid=None, decode=False, text=True +): """ Used to remove a service. @@ -1114,16 +1125,22 @@ def agent_service_deregister(consul_url=None, token=None, serviceid=None): if not serviceid: raise SaltInvocationError('Required argument "serviceid" is missing.') - function = "agent/service/deregister/{}".format(serviceid) + function = f"agent/service/deregister/{serviceid}" res = _query( - consul_url=consul_url, function=function, token=token, method="PUT", data=data + consul_url=consul_url, + function=function, + token=token, + method="PUT", + data=data, + decode=decode, + text=text, ) if res["res"]: ret["res"] = True - ret["message"] = "Service {} removed from agent.".format(serviceid) + ret["message"] = f"Service {serviceid} removed from agent." else: ret["res"] = False - ret["message"] = "Unable to remove service {}.".format(serviceid) + ret["message"] = f"Unable to remove service {serviceid}." return ret @@ -1168,14 +1185,14 @@ def agent_service_maintenance(consul_url=None, token=None, serviceid=None, **kwa if "reason" in kwargs: query_params["reason"] = kwargs["reason"] - function = "agent/service/maintenance/{}".format(serviceid) + function = f"agent/service/maintenance/{serviceid}" res = _query( consul_url=consul_url, token=token, function=function, query_params=query_params ) if res["res"]: ret["res"] = True - ret["message"] = "Service {} set in maintenance mode.".format(serviceid) + ret["message"] = f"Service {serviceid} set in maintenance mode." else: ret["res"] = False ret["message"] = "Unable to set service {} to maintenance mode.".format( @@ -1255,7 +1272,7 @@ def session_create(consul_url=None, token=None, **kwargs): ret["message"] = ("TTL must be ", "between 0 and 3600.") ret["res"] = False return ret - data["TTL"] = "{}s".format(_ttl) + data["TTL"] = f"{_ttl}s" function = "session/create" res = _query( @@ -1351,7 +1368,7 @@ def session_destroy(consul_url=None, token=None, session=None, **kwargs): if "dc" in kwargs: query_params["dc"] = kwargs["dc"] - function = "session/destroy/{}".format(session) + function = f"session/destroy/{session}" res = _query( consul_url=consul_url, function=function, @@ -1361,10 +1378,10 @@ def session_destroy(consul_url=None, token=None, session=None, **kwargs): ) if res["res"]: ret["res"] = True - ret["message"] = "Destroyed Session {}.".format(session) + ret["message"] = f"Destroyed Session {session}." else: ret["res"] = False - ret["message"] = "Unable to destroy session {}.".format(session) + ret["message"] = f"Unable to destroy session {session}." return ret @@ -1402,7 +1419,7 @@ def session_info(consul_url=None, token=None, session=None, **kwargs): if "dc" in kwargs: query_params["dc"] = kwargs["dc"] - function = "session/info/{}".format(session) + function = f"session/info/{session}" ret = _query( consul_url=consul_url, function=function, token=token, query_params=query_params ) @@ -1770,7 +1787,7 @@ def catalog_service(consul_url=None, token=None, service=None, **kwargs): if "tag" in kwargs: query_params["tag"] = kwargs["tag"] - function = "catalog/service/{}".format(service) + function = f"catalog/service/{service}" ret = _query( consul_url=consul_url, function=function, token=token, query_params=query_params ) @@ -1810,7 +1827,7 @@ def catalog_node(consul_url=None, token=None, node=None, **kwargs): if "dc" in kwargs: query_params["dc"] = kwargs["dc"] - function = "catalog/node/{}".format(node) + function = f"catalog/node/{node}" ret = _query( consul_url=consul_url, function=function, token=token, query_params=query_params ) @@ -1850,7 +1867,7 @@ def health_node(consul_url=None, token=None, node=None, **kwargs): if "dc" in kwargs: query_params["dc"] = kwargs["dc"] - function = "health/node/{}".format(node) + function = f"health/node/{node}" ret = _query( consul_url=consul_url, function=function, token=token, query_params=query_params ) @@ -1890,7 +1907,7 @@ def health_checks(consul_url=None, token=None, service=None, **kwargs): if "dc" in kwargs: query_params["dc"] = kwargs["dc"] - function = "health/checks/{}".format(service) + function = f"health/checks/{service}" ret = _query( consul_url=consul_url, function=function, token=token, query_params=query_params ) @@ -1941,7 +1958,7 @@ def health_service(consul_url=None, token=None, service=None, **kwargs): if "passing" in kwargs: query_params["passing"] = kwargs["passing"] - function = "health/service/{}".format(service) + function = f"health/service/{service}" ret = _query( consul_url=consul_url, function=function, token=token, query_params=query_params ) @@ -1991,7 +2008,7 @@ def health_state(consul_url=None, token=None, state=None, **kwargs): ret["res"] = False return ret - function = "health/state/{}".format(state) + function = f"health/state/{state}" ret = _query( consul_url=consul_url, function=function, token=token, query_params=query_params ) @@ -2061,6 +2078,7 @@ def acl_create(consul_url=None, token=None, **kwargs): :param consul_url: The Consul server URL. :param name: Meaningful indicator of the ACL's purpose. + :param token: Authentication token :param type: Type is either client or management. A management token is comparable to a root user and has the ability to perform any action including creating, @@ -2085,9 +2103,6 @@ def acl_create(consul_url=None, token=None, **kwargs): ret["res"] = False return ret - if "id" in kwargs: - data["id"] = kwargs["id"] - if "name" in kwargs: data["Name"] = kwargs["name"] else: @@ -2097,7 +2112,16 @@ def acl_create(consul_url=None, token=None, **kwargs): data["Type"] = kwargs["type"] if "rules" in kwargs: - data["Rules"] = kwargs["rules"] + rules_str = "" + rules = kwargs["rules"] + for item in rules: + for k, v in item.items(): + if k != "policy": + rules_str += f'{k} "{v}" {{\n' + else: + rules_str += f' {k} = "{v}"\n}}\n' + + data["Rules"] = rules_str function = "acl/create" res = _query( @@ -2218,6 +2242,7 @@ def acl_delete(consul_url=None, token=None, **kwargs): else: ret["res"] = False ret["message"] = "Removing ACL {} failed.".format(kwargs["id"]) + ret["changes"] = res return ret @@ -2378,7 +2403,7 @@ def event_fire(consul_url=None, token=None, name=None, **kwargs): if "tag" in kwargs: query_params = kwargs["tag"] - function = "event/fire/{}".format(name) + function = f"event/fire/{name}" res = _query( consul_url=consul_url, token=token, @@ -2389,7 +2414,7 @@ def event_fire(consul_url=None, token=None, name=None, **kwargs): if res["res"]: ret["res"] = True - ret["message"] = "Event {} fired.".format(name) + ret["message"] = f"Event {name} fired." ret["data"] = res["data"] else: ret["res"] = False diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index 9f3d9d27e6da..5b177531bb40 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -2982,10 +2982,13 @@ def mod_repo(repo, basedir=None, **kwargs): the URL for yum to reference mirrorlist the URL for yum to reference + metalink + the URL for yum to reference + .. versionadded:: 3008.0 Key/Value pairs may also be removed from a repo's configuration by setting - a key to a blank value. Bear in mind that a name cannot be deleted, and a - baseurl can only be deleted if a mirrorlist is specified (or vice versa). + a key to a blank value. Bear in mind that a name cannot be deleted, and one + of baseurl, mirrorlist, or metalink is required. Strict parsing of configuration files is the default, this can be disabled using the ``strict_config`` keyword argument set to False @@ -2996,16 +2999,20 @@ def mod_repo(repo, basedir=None, **kwargs): salt '*' pkg.mod_repo reponame enabled=1 gpgcheck=1 salt '*' pkg.mod_repo reponame basedir=/path/to/dir enabled=1 strict_config=False - salt '*' pkg.mod_repo reponame baseurl= mirrorlist=http://host.com/ + salt '*' pkg.mod_repo reponame basedir= mirrorlist=http://host.com/ + salt '*' pkg.mod_repo reponame basedir= metalink=http://host.com """ + # set link types + link_types = ("baseurl", "mirrorlist", "metalink") + # Filter out '__pub' arguments, as well as saltenv repo_opts = { x: kwargs[x] for x in kwargs if not x.startswith("__") and x not in ("saltenv",) } - if all(x in repo_opts for x in ("mirrorlist", "baseurl")): + if [x in repo_opts for x in link_types].count(True) >= 2: raise SaltInvocationError( - "Only one of 'mirrorlist' and 'baseurl' can be specified" + f"One and only one of {', '.join(link_types)} can be used" ) use_copr = False @@ -3022,12 +3029,11 @@ def mod_repo(repo, basedir=None, **kwargs): del repo_opts[key] todelete.append(key) - # Add baseurl or mirrorlist to the 'todelete' list if the other was - # specified in the repo_opts - if "mirrorlist" in repo_opts: - todelete.append("baseurl") - elif "baseurl" in repo_opts: - todelete.append("mirrorlist") + # Add what ever items in link_types is not in repo_opts to 'todelete' list + linkdict = {x: set(link_types) - {x} for x in link_types} + todelete.extend( + next(iter([y for x, y in linkdict.items() if x in repo_opts.keys()]), []) + ) # Fail if the user tried to delete the name if "name" in todelete: @@ -3090,10 +3096,10 @@ def mod_repo(repo, basedir=None, **kwargs): "was not given" ) - if "baseurl" not in repo_opts and "mirrorlist" not in repo_opts: + if all(x not in repo_opts.keys() for x in link_types): raise SaltInvocationError( - "The repo does not exist and needs to be created, but either " - "a baseurl or a mirrorlist needs to be given" + "The repo does not exist and needs to be created, but none of " + f"{', '.join(link_types)} was given" ) filerepos[repo] = {} else: @@ -3101,16 +3107,15 @@ def mod_repo(repo, basedir=None, **kwargs): repofile = repos[repo]["file"] header, filerepos = _parse_repo_file(repofile, strict_parser) - # Error out if they tried to delete baseurl or mirrorlist improperly - if "baseurl" in todelete: - if "mirrorlist" not in repo_opts and "mirrorlist" not in filerepos[repo]: - raise SaltInvocationError( - "Cannot delete baseurl without specifying mirrorlist" - ) - if "mirrorlist" in todelete: - if "baseurl" not in repo_opts and "baseurl" not in filerepos[repo]: + # Error out if they tried to delete all linktypes + for link_type in link_types: + linklist = set(link_types) - {link_type} + if all( + x not in repo_opts and x not in filerepos[repo] and link_type in todelete + for x in linklist + ): raise SaltInvocationError( - "Cannot delete mirrorlist without specifying baseurl" + f"Cannot delete {link_type} without specifying {' or '.join(linklist)}" ) # Delete anything in the todelete list diff --git a/salt/states/pkgrepo.py b/salt/states/pkgrepo.py index 25959da679c7..e7c3a0fd5718 100644 --- a/salt/states/pkgrepo.py +++ b/salt/states/pkgrepo.py @@ -145,10 +145,10 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs): **YUM/DNF/ZYPPER-BASED SYSTEMS** .. note:: - One of ``baseurl`` or ``mirrorlist`` below is required. Additionally, - note that this state is not presently capable of managing more than one - repo in a single repo file, so each instance of this state will manage - a single repo file containing the configuration for a single repo. + One of ``baseurl``, ``mirrorlist``, or ``metalink`` below is required. + Additionally, note that this state is not presently capable of managing + more than one repo in a single repo file, so each instance of this state + will manage a single repo file containing the configuration for a single repo. name This value will be used in two ways: Firstly, it will be the repo ID, @@ -182,6 +182,11 @@ def managed(name, ppa=None, copr=None, aptkey=True, **kwargs): mirrorlist A URL which points to a file containing a collection of baseurls + metalink + A URL for a curated list of non-stale mirrors only usable with yum/dnf + + .. versionadded:: 3008.0 + comments Sometimes you want to supply additional information, but not as enabled configuration. Anything supplied for this list will be saved diff --git a/tests/integration/modules/test_cmdmod.py b/tests/integration/modules/test_cmdmod.py deleted file mode 100644 index 800111174f08..000000000000 --- a/tests/integration/modules/test_cmdmod.py +++ /dev/null @@ -1,634 +0,0 @@ -import os -import random -import sys -import tempfile -from contextlib import contextmanager - -import pytest - -import salt.utils.path -import salt.utils.platform -import salt.utils.user -from tests.support.case import ModuleCase -from tests.support.helpers import SKIP_INITIAL_PHOTONOS_FAILURES, dedent -from tests.support.runtests import RUNTIME_VARS - -AVAILABLE_PYTHON_EXECUTABLE = salt.utils.path.which_bin( - ["python", "python2", "python2.6", "python2.7"] -) - - -@pytest.mark.windows_whitelisted -class CMDModuleTest(ModuleCase): - """ - Validate the cmd module - """ - - def setUp(self): - self.runas_usr = "nobody" - if salt.utils.platform.is_darwin(): - self.runas_usr = "macsalttest" - - @contextmanager - def _ensure_user_exists(self, name): - if name in self.run_function("user.info", [name]).values(): - # User already exists; don't touch - yield - else: - # Need to create user for test - self.run_function("user.add", [name]) - try: - yield - finally: - self.run_function("user.delete", [name], remove=True) - - @pytest.mark.slow_test - @pytest.mark.skip_on_windows - def test_run(self): - """ - cmd.run - """ - shell = os.environ.get("SHELL") - if shell is None: - # Failed to get the SHELL var, don't run - self.skipTest("Unable to get the SHELL environment variable") - - self.assertTrue(self.run_function("cmd.run", ["echo $SHELL"])) - self.assertEqual( - self.run_function( - "cmd.run", ["echo $SHELL", "shell={}".format(shell)], python_shell=True - ).rstrip(), - shell, - ) - self.assertEqual( - self.run_function("cmd.run", ["ls / | grep etc"], python_shell=True), "etc" - ) - self.assertEqual( - self.run_function( - "cmd.run", - ['echo {{grains.id}} | awk "{print $1}"'], - template="jinja", - python_shell=True, - ), - "minion", - ) - self.assertEqual( - self.run_function( - "cmd.run", ["grep f"], stdin="one\ntwo\nthree\nfour\nfive\n" - ), - "four\nfive", - ) - self.assertEqual( - self.run_function( - "cmd.run", ['echo "a=b" | sed -e s/=/:/g'], python_shell=True - ), - "a:b", - ) - - @pytest.mark.slow_test - def test_stdout(self): - """ - cmd.run_stdout - """ - self.assertEqual( - self.run_function("cmd.run_stdout", ['echo "cheese"']).rstrip(), - "cheese" if not salt.utils.platform.is_windows() else '"cheese"', - ) - - @pytest.mark.slow_test - def test_stderr(self): - """ - cmd.run_stderr - """ - if sys.platform.startswith(("freebsd", "openbsd")): - shell = "/bin/sh" - else: - shell = "/bin/bash" - - self.assertEqual( - self.run_function( - "cmd.run_stderr", - ['echo "cheese" 1>&2', "shell={}".format(shell)], - python_shell=True, - ).rstrip(), - "cheese" if not salt.utils.platform.is_windows() else '"cheese"', - ) - - @pytest.mark.slow_test - def test_run_all(self): - """ - cmd.run_all - """ - if sys.platform.startswith(("freebsd", "openbsd")): - shell = "/bin/sh" - else: - shell = "/bin/bash" - - ret = self.run_function( - "cmd.run_all", - ['echo "cheese" 1>&2', "shell={}".format(shell)], - python_shell=True, - ) - self.assertTrue("pid" in ret) - self.assertTrue("retcode" in ret) - self.assertTrue("stdout" in ret) - self.assertTrue("stderr" in ret) - self.assertTrue(isinstance(ret.get("pid"), int)) - self.assertTrue(isinstance(ret.get("retcode"), int)) - self.assertTrue(isinstance(ret.get("stdout"), str)) - self.assertTrue(isinstance(ret.get("stderr"), str)) - self.assertEqual( - ret.get("stderr").rstrip(), - "cheese" if not salt.utils.platform.is_windows() else '"cheese"', - ) - - @pytest.mark.slow_test - def test_retcode(self): - """ - cmd.retcode - """ - self.assertEqual( - self.run_function("cmd.retcode", ["exit 0"], python_shell=True), 0 - ) - self.assertEqual( - self.run_function("cmd.retcode", ["exit 1"], python_shell=True), 1 - ) - - @pytest.mark.slow_test - def test_run_all_with_success_retcodes(self): - """ - cmd.run with success_retcodes - """ - ret = self.run_function( - "cmd.run_all", ["exit 42"], success_retcodes=[42], python_shell=True - ) - - self.assertTrue("retcode" in ret) - self.assertEqual(ret.get("retcode"), 0) - - @pytest.mark.slow_test - def test_retcode_with_success_retcodes(self): - """ - cmd.run with success_retcodes - """ - ret = self.run_function( - "cmd.retcode", ["exit 42"], success_retcodes=[42], python_shell=True - ) - - self.assertEqual(ret, 0) - - @pytest.mark.slow_test - def test_run_all_with_success_stderr(self): - """ - cmd.run with success_retcodes - """ - random_file = "{}{}{}".format( - RUNTIME_VARS.TMP_ROOT_DIR, os.path.sep, random.random() - ) - - if salt.utils.platform.is_windows(): - func = "type" - expected_stderr = "cannot find the file specified" - else: - func = "cat" - expected_stderr = "No such file or directory" - ret = self.run_function( - "cmd.run_all", - ["{} {}".format(func, random_file)], - success_stderr=[expected_stderr], - python_shell=True, - ) - - self.assertTrue("retcode" in ret) - self.assertEqual(ret.get("retcode"), 0) - - @pytest.mark.slow_test - def test_blacklist_glob(self): - """ - cmd_blacklist_glob - """ - self.assertEqual( - self.run_function("cmd.run", ["bad_command --foo"]).rstrip(), - 'ERROR: The shell command "bad_command --foo" is not permitted', - ) - - @pytest.mark.slow_test - def test_script(self): - """ - cmd.script - """ - args = "saltines crackers biscuits=yes" - script = "salt://script.py" - ret = self.run_function("cmd.script", [script, args], saltenv="base") - self.assertEqual(ret["stdout"], args) - - @pytest.mark.slow_test - def test_script_query_string(self): - """ - cmd.script - """ - args = "saltines crackers biscuits=yes" - script = "salt://script.py?saltenv=base" - ret = self.run_function("cmd.script", [script, args], saltenv="base") - self.assertEqual(ret["stdout"], args) - - @pytest.mark.slow_test - def test_script_retcode(self): - """ - cmd.script_retcode - """ - script = "salt://script.py" - ret = self.run_function("cmd.script_retcode", [script], saltenv="base") - self.assertEqual(ret, 0) - - @pytest.mark.slow_test - def test_script_cwd(self): - """ - cmd.script with cwd - """ - tmp_cwd = tempfile.mkdtemp(dir=RUNTIME_VARS.TMP) - args = "saltines crackers biscuits=yes" - script = "salt://script.py" - ret = self.run_function( - "cmd.script", [script, args], cwd=tmp_cwd, saltenv="base" - ) - self.assertEqual(ret["stdout"], args) - - @pytest.mark.slow_test - def test_script_cwd_with_space(self): - """ - cmd.script with cwd - """ - tmp_cwd = "{}{}test 2".format( - tempfile.mkdtemp(dir=RUNTIME_VARS.TMP), os.path.sep - ) - os.mkdir(tmp_cwd) - - args = "saltines crackers biscuits=yes" - script = "salt://script.py" - ret = self.run_function( - "cmd.script", [script, args], cwd=tmp_cwd, saltenv="base" - ) - self.assertEqual(ret["stdout"], args) - - @pytest.mark.destructive_test - def test_tty(self): - """ - cmd.tty - """ - for tty in ("tty0", "pts3"): - if os.path.exists(os.path.join("/dev", tty)): - ret = self.run_function("cmd.tty", [tty, "apply salt liberally"]) - self.assertTrue("Success" in ret) - - @pytest.mark.skip_on_windows - @pytest.mark.skip_if_binaries_missing("which") - def test_which(self): - """ - cmd.which - """ - cmd_which = self.run_function("cmd.which", ["cat"]) - self.assertIsInstance(cmd_which, str) - cmd_run = self.run_function("cmd.run", ["which cat"]) - self.assertIsInstance(cmd_run, str) - self.assertEqual(cmd_which.rstrip(), cmd_run.rstrip()) - - @pytest.mark.skip_on_windows - @pytest.mark.skip_if_binaries_missing("which") - def test_which_bin(self): - """ - cmd.which_bin - """ - cmds = ["pip3", "pip2", "pip", "pip-python"] - ret = self.run_function("cmd.which_bin", [cmds]) - self.assertTrue(os.path.split(ret)[1] in cmds) - - @pytest.mark.slow_test - def test_has_exec(self): - """ - cmd.has_exec - """ - self.assertTrue( - self.run_function("cmd.has_exec", [AVAILABLE_PYTHON_EXECUTABLE]) - ) - self.assertFalse( - self.run_function("cmd.has_exec", ["alllfsdfnwieulrrh9123857ygf"]) - ) - - @pytest.mark.slow_test - def test_exec_code(self): - """ - cmd.exec_code - """ - code = dedent( - """ - import sys - sys.stdout.write('cheese') - """ - ) - self.assertEqual( - self.run_function( - "cmd.exec_code", [AVAILABLE_PYTHON_EXECUTABLE, code] - ).rstrip(), - "cheese", - ) - - @pytest.mark.slow_test - def test_exec_code_with_single_arg(self): - """ - cmd.exec_code - """ - code = dedent( - """ - import sys - sys.stdout.write(sys.argv[1]) - """ - ) - arg = "cheese" - self.assertEqual( - self.run_function( - "cmd.exec_code", [AVAILABLE_PYTHON_EXECUTABLE, code], args=arg - ).rstrip(), - arg, - ) - - @pytest.mark.slow_test - def test_exec_code_with_multiple_args(self): - """ - cmd.exec_code - """ - code = dedent( - """ - import sys - sys.stdout.write(sys.argv[1]) - """ - ) - arg = "cheese" - self.assertEqual( - self.run_function( - "cmd.exec_code", [AVAILABLE_PYTHON_EXECUTABLE, code], args=[arg, "test"] - ).rstrip(), - arg, - ) - - @pytest.mark.slow_test - def test_quotes(self): - """ - cmd.run with quoted command - """ - cmd = """echo 'SELECT * FROM foo WHERE bar="baz"' """ - expected_result = 'SELECT * FROM foo WHERE bar="baz"' - if salt.utils.platform.is_windows(): - expected_result = "'SELECT * FROM foo WHERE bar=\"baz\"'" - result = self.run_function("cmd.run_stdout", [cmd]).strip() - self.assertEqual(result, expected_result) - - @pytest.mark.skip_if_not_root - @pytest.mark.skip_on_windows(reason="Skip on Windows, requires password") - def test_quotes_runas(self): - """ - cmd.run with quoted command - """ - cmd = """echo 'SELECT * FROM foo WHERE bar="baz"' """ - expected_result = 'SELECT * FROM foo WHERE bar="baz"' - result = self.run_function( - "cmd.run_all", [cmd], runas=RUNTIME_VARS.RUNNING_TESTS_USER - ) - errmsg = "The command returned: {}".format(result) - self.assertEqual(result["retcode"], 0, errmsg) - self.assertEqual(result["stdout"], expected_result, errmsg) - - @pytest.mark.destructive_test - @pytest.mark.skip_if_not_root - @pytest.mark.skip_on_windows(reason="Skip on Windows, uses unix commands") - @pytest.mark.slow_test - def test_avoid_injecting_shell_code_as_root(self): - """ - cmd.run should execute the whole command as the "runas" user, not - running substitutions as root. - """ - cmd = "echo $(id -u)" - - root_id = self.run_function("cmd.run_stdout", [cmd]) - runas_root_id = self.run_function( - "cmd.run_stdout", [cmd], runas=RUNTIME_VARS.RUNNING_TESTS_USER - ) - with self._ensure_user_exists(self.runas_usr): - user_id = self.run_function("cmd.run_stdout", [cmd], runas=self.runas_usr) - - self.assertNotEqual(user_id, root_id) - self.assertNotEqual(user_id, runas_root_id) - self.assertEqual(root_id, runas_root_id) - - @pytest.mark.destructive_test - @pytest.mark.skip_if_not_root - @pytest.mark.skip_on_windows(reason="Skip on Windows, uses unix commands") - @pytest.mark.slow_test - def test_cwd_runas(self): - """ - cmd.run should be able to change working directory correctly, whether - or not runas is in use. - """ - cmd = "pwd" - tmp_cwd = tempfile.mkdtemp(dir=RUNTIME_VARS.TMP) - os.chmod(tmp_cwd, 0o711) - - cwd_normal = self.run_function("cmd.run_stdout", [cmd], cwd=tmp_cwd).rstrip( - "\n" - ) - self.assertEqual(tmp_cwd, cwd_normal) - - with self._ensure_user_exists(self.runas_usr): - cwd_runas = self.run_function( - "cmd.run_stdout", [cmd], cwd=tmp_cwd, runas=self.runas_usr - ).rstrip("\n") - self.assertEqual(tmp_cwd, cwd_runas) - - @pytest.mark.destructive_test - @pytest.mark.skip_if_not_root - @pytest.mark.skip_unless_on_darwin(reason="Applicable to MacOS only") - @pytest.mark.slow_test - def test_runas_env(self): - """ - cmd.run should be able to change working directory correctly, whether - or not runas is in use. - """ - with self._ensure_user_exists(self.runas_usr): - user_path = self.run_function( - "cmd.run_stdout", ['printf %s "$PATH"'], runas=self.runas_usr - ) - # XXX: Not sure of a better way. Environment starts out with - # /bin:/usr/bin and should be populated by path helper and the bash - # profile. - self.assertNotEqual("/bin:/usr/bin", user_path) - - @pytest.mark.destructive_test - @pytest.mark.skip_if_not_root - @pytest.mark.skip_unless_on_darwin(reason="Applicable to MacOS only") - @pytest.mark.slow_test - def test_runas_complex_command_bad_cwd(self): - """ - cmd.run should not accidentally run parts of a complex command when - given a cwd which cannot be used by the user the command is run as. - - Due to the need to use `su -l` to login to another user on MacOS, we - cannot cd into directories that the target user themselves does not - have execute permission for. To an extent, this test is testing that - buggy behaviour, but its purpose is to ensure that the greater bug of - running commands after failing to cd does not occur. - """ - tmp_cwd = tempfile.mkdtemp(dir=RUNTIME_VARS.TMP) - os.chmod(tmp_cwd, 0o700) - - with self._ensure_user_exists(self.runas_usr): - cmd_result = self.run_function( - "cmd.run_all", - ['pwd; pwd; : $(echo "You have failed the test" >&2)'], - cwd=tmp_cwd, - runas=self.runas_usr, - ) - - self.assertEqual("", cmd_result["stdout"]) - self.assertNotIn("You have failed the test", cmd_result["stderr"]) - self.assertNotEqual(0, cmd_result["retcode"]) - - @SKIP_INITIAL_PHOTONOS_FAILURES - @pytest.mark.skip_on_windows - @pytest.mark.skip_if_not_root - @pytest.mark.destructive_test - @pytest.mark.slow_test - def test_runas(self): - """ - Ensure that the env is the runas user's - """ - with self._ensure_user_exists(self.runas_usr): - out = self.run_function( - "cmd.run", ["env"], runas=self.runas_usr - ).splitlines() - self.assertIn("USER={}".format(self.runas_usr), out) - - @pytest.mark.skip_if_binaries_missing("sleep", reason="sleep cmd not installed") - def test_timeout(self): - """ - cmd.run trigger timeout - """ - out = self.run_function( - "cmd.run", ["sleep 2 && echo hello"], f_timeout=1, python_shell=True - ) - self.assertTrue("Timed out" in out) - - @pytest.mark.skip_if_binaries_missing("sleep", reason="sleep cmd not installed") - def test_timeout_success(self): - """ - cmd.run sufficient timeout to succeed - """ - out = self.run_function( - "cmd.run", ["sleep 1 && echo hello"], f_timeout=2, python_shell=True - ) - self.assertEqual(out, "hello") - - @pytest.mark.slow_test - def test_hide_output(self): - """ - Test the hide_output argument - """ - ls_command = ( - ["ls", "/"] if not salt.utils.platform.is_windows() else ["dir", "c:\\"] - ) - - error_command = ["thiscommanddoesnotexist"] - - # cmd.run - out = self.run_function("cmd.run", ls_command, hide_output=True) - self.assertEqual(out, "") - - # cmd.shell - out = self.run_function("cmd.shell", ls_command, hide_output=True) - self.assertEqual(out, "") - - # cmd.run_stdout - out = self.run_function("cmd.run_stdout", ls_command, hide_output=True) - self.assertEqual(out, "") - - # cmd.run_stderr - out = self.run_function("cmd.shell", error_command, hide_output=True) - self.assertEqual(out, "") - - # cmd.run_all (command should have produced stdout) - out = self.run_function("cmd.run_all", ls_command, hide_output=True) - self.assertEqual(out["stdout"], "") - self.assertEqual(out["stderr"], "") - - # cmd.run_all (command should have produced stderr) - out = self.run_function("cmd.run_all", error_command, hide_output=True) - self.assertEqual(out["stdout"], "") - self.assertEqual(out["stderr"], "") - - @pytest.mark.slow_test - def test_cmd_run_whoami(self): - """ - test return of whoami - """ - if not salt.utils.platform.is_windows(): - user = RUNTIME_VARS.RUNTIME_CONFIGS["master"]["user"] - else: - user = salt.utils.user.get_specific_user() - if user.startswith("sudo_"): - user = user.replace("sudo_", "") - cmd = self.run_function("cmd.run", ["whoami"]) - try: - self.assertEqual(user.lower(), cmd.lower()) - except AssertionError as exc: - if not salt.utils.platform.is_windows(): - raise exc from None - if "\\" in user: - user = user.split("\\")[-1] - self.assertEqual(user.lower(), cmd.lower()) - - @pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") - @pytest.mark.slow_test - def test_windows_env_handling(self): - """ - Ensure that nt.environ is used properly with cmd.run* - """ - out = self.run_function( - "cmd.run", ["set"], env={"abc": "123", "ABC": "456"} - ).splitlines() - self.assertIn("abc=123", out) - self.assertIn("ABC=456", out) - - @pytest.mark.slow_test - @pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") - def test_windows_powershell_script_args(self): - """ - Ensure that powershell processes inline script in args - """ - val = "i like cheese" - args = ( - '-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)' - " -ErrorAction Stop".format(val) - ) - script = "salt://issue-56195/test.ps1" - ret = self.run_function( - "cmd.script", [script], args=args, shell="powershell", saltenv="base" - ) - self.assertEqual(ret["stdout"], val) - - @pytest.mark.slow_test - @pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") - @pytest.mark.skip_if_binaries_missing("pwsh") - def test_windows_powershell_script_args_pwsh(self): - """ - Ensure that powershell processes inline script in args with powershell - core - """ - val = "i like cheese" - args = ( - '-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)' - " -ErrorAction Stop".format(val) - ) - script = "salt://issue-56195/test.ps1" - ret = self.run_function( - "cmd.script", [script], args=args, shell="pwsh", saltenv="base" - ) - self.assertEqual(ret["stdout"], val) diff --git a/tests/pytests/functional/modules/test_cmdmod.py b/tests/pytests/functional/modules/test_cmdmod.py new file mode 100644 index 000000000000..03eb579f2c43 --- /dev/null +++ b/tests/pytests/functional/modules/test_cmdmod.py @@ -0,0 +1,565 @@ +import os +import random +import sys + +import pytest + +import salt.config +import salt.utils.path +import salt.utils.platform +import salt.utils.user +from tests.support.helpers import SKIP_INITIAL_PHOTONOS_FAILURES, dedent + +pytestmark = [ + pytest.mark.windows_whitelisted, +] + + +@pytest.fixture(scope="module") +def cmdmod(modules): + return modules.cmd + + +@pytest.fixture(scope="module") +def usermod(modules): + return modules.user + + +@pytest.fixture(scope="module") +def available_python_executable(): + yield salt.utils.path.which_bin( + ["python", "python3", "python3.8", "python3.9", "python3.10"] + ) + + +@pytest.fixture +def runas_usr(): + if salt.utils.platform.is_darwin(): + with pytest.helpers.create_account() as account: + yield account.username + else: + yield "nobody" + + +@pytest.fixture +def running_username(): + """ + Return the username that is running the code. + """ + return salt.utils.user.get_user() + + +@pytest.fixture +def script_contents(state_tree): + _contents = """ + #!/usr/bin/env python3 + import sys + print(" ".join(sys.argv[1:])) + """ + + with pytest.helpers.temp_file("script.py", _contents, state_tree): + yield + + +@pytest.fixture +def issue_56195_test_ps1(state_tree): + _contents = """ + [CmdLetBinding()] + Param( + [SecureString] $SecureString + ) + $Credential = New-Object System.Net.NetworkCredential("DummyId", $SecureString) + $Credential.Password + """ + + with pytest.helpers.temp_file("issue_56195_test.ps1", _contents, state_tree): + yield + + +@pytest.mark.skip_on_windows( + reason="Skip on Windows, Windows enviroment issues for this test - test needs fixing" +) +@pytest.mark.slow_test +def test_run(cmdmod, grains): + """ + cmd.run + """ + shell = os.environ.get("SHELL") + if shell is None: + # Failed to get the SHELL var, don't run + pytest.skip("Unable to get the SHELL environment variable") + + assert cmdmod.run("echo $SHELL") + assert cmdmod.run("echo $SHELL", shell=shell, python_shell=True).rstrip() == shell + assert cmdmod.run("ls / | grep etc", python_shell=True) == "etc" + assert ( + cmdmod.run( + 'echo {{grains.id}} | awk "{print $1}"', + template="jinja", + python_shell=True, + ) + == "func-tests-minion-opts" + ) + assert cmdmod.run("grep f", stdin="one\ntwo\nthree\nfour\nfive\n") == "four\nfive" + assert cmdmod.run('echo "a=b" | sed -e s/=/:/g', python_shell=True) == "a:b" + + +@pytest.mark.slow_test +def test_stdout(cmdmod): + """ + cmd.run_stdout + """ + assert ( + cmdmod.run_stdout('echo "cheese"').rstrip() == "cheese" + if not salt.utils.platform.is_windows() + else '"cheese"' + ) + + +@pytest.mark.slow_test +def test_stderr(cmdmod): + """ + cmd.run_stderr + """ + if sys.platform.startswith(("freebsd", "openbsd")): + shell = "/bin/sh" + else: + shell = "/bin/bash" + + assert ( + cmdmod.run_stderr( + 'echo "cheese" 1>&2', + shell=shell, + python_shell=True, + ).rstrip() + == "cheese" + if not salt.utils.platform.is_windows() + else '"cheese"' + ) + + +@pytest.mark.slow_test +def test_run_all(cmdmod): + """ + cmd.run_all + """ + if sys.platform.startswith(("freebsd", "openbsd")): + shell = "/bin/sh" + else: + shell = "/bin/bash" + + ret = cmdmod.run_all( + 'echo "cheese" 1>&2', + shell=shell, + python_shell=True, + ) + assert "pid" in ret + assert "retcode" in ret + assert "stdout" in ret + assert "stderr" in ret + assert isinstance(ret.get("pid"), int) + assert isinstance(ret.get("retcode"), int) + assert isinstance(ret.get("stdout"), str) + assert isinstance(ret.get("stderr"), str) + assert ( + ret.get("stderr").rstrip() == "cheese" + if not salt.utils.platform.is_windows() + else '"cheese"' + ) + + +@pytest.mark.slow_test +def test_retcode(cmdmod): + """ + cmd.retcode + """ + assert cmdmod.retcode("exit 0", python_shell=True) == 0 + assert cmdmod.retcode("exit 1", python_shell=True) == 1 + + +@pytest.mark.slow_test +def test_run_all_with_success_retcodes(cmdmod): + """ + cmd.run with success_retcodes + """ + ret = cmdmod.run_all("exit 42", success_retcodes=[42], python_shell=True) + + assert "retcode" in ret + assert ret.get("retcode") == 0 + + +@pytest.mark.slow_test +def test_retcode_with_success_retcodes(cmdmod): + """ + cmd.run with success_retcodes + """ + ret = cmdmod.retcode("exit 42", success_retcodes=[42], python_shell=True) + + assert ret == 0 + + +@pytest.mark.slow_test +def test_run_all_with_success_stderr(cmdmod, tmp_path): + """ + cmd.run with success_retcodes + """ + random_file = str(tmp_path / f"{random.random()}") + + if salt.utils.platform.is_windows(): + func = "type" + expected_stderr = "cannot find the file specified" + else: + func = "cat" + expected_stderr = "No such file or directory" + ret = cmdmod.run_all( + f"{func} {random_file}", + success_stderr=[expected_stderr], + python_shell=True, + ) + + assert "retcode" in ret + assert ret.get("retcode") == 0 + + +@pytest.mark.skip_on_windows( + reason="Skip on Windows, Windows enviroment issues for this test - test needs fixing" +) +@pytest.mark.slow_test +def test_script(cmdmod, script_contents): + """ + cmd.script + """ + args = "saltines crackers biscuits=yes" + script = "salt://script.py" + ret = cmdmod.script(script, args, saltenv="base") + assert ret["stdout"] == args + + +@pytest.mark.skip_on_windows( + reason="Skip on Windows, Windows enviroment issues for this test - test needs fixing" +) +@pytest.mark.slow_test +def test_script_query_string(cmdmod, script_contents): + """ + cmd.script + """ + args = "saltines crackers biscuits=yes" + script = "salt://script.py?saltenv=base" + ret = cmdmod.script(script, args, saltenv="base") + assert ret["stdout"] == args + + +@pytest.mark.skip_on_windows( + reason="Skip on Windows, Windows enviroment issues for this test - test needs fixing" +) +@pytest.mark.slow_test +def test_script_retcode(cmdmod, script_contents): + """ + cmd.script_retcode + """ + script = "salt://script.py" + ret = cmdmod.script_retcode(script, saltenv="base") + assert ret == 0 + + +@pytest.mark.skip_on_windows( + reason="Skip on Windows, Windows enviroment issues for this test - test needs fixing" +) +@pytest.mark.slow_test +def test_script_cwd(cmdmod, script_contents, tmp_path): + """ + cmd.script with cwd + """ + tmp_cwd = str(tmp_path) + args = "saltines crackers biscuits=yes" + script = "salt://script.py" + ret = cmdmod.script(script, args, cwd=tmp_cwd, saltenv="base") + assert ret["stdout"] == args + + +@pytest.mark.skip_on_windows( + reason="Skip on Windows, Windows enviroment issues for this test - test needs fixing" +) +@pytest.mark.slow_test +def test_script_cwd_with_space(cmdmod, script_contents, tmp_path): + """ + cmd.script with cwd + """ + tmp_cwd = str(tmp_path / "test 2") + os.mkdir(tmp_cwd) + + args = "saltines crackers biscuits=yes" + script = "salt://script.py" + ret = cmdmod.script(script, args, cwd=tmp_cwd, saltenv="base") + assert ret["stdout"] == args + + +@pytest.mark.destructive_test +def test_tty(cmdmod): + """ + cmd.tty + """ + for tty in ("tty0", "pts3"): + if os.path.exists(os.path.join("/dev", tty)): + ret = cmdmod.tty(tty, "apply salt liberally") + assert "Success" in ret + + +@pytest.mark.skip_on_windows +@pytest.mark.skip_if_binaries_missing("which") +def test_which(cmdmod): + """ + cmd.which + """ + cmd_which = cmdmod.which("cat") + assert isinstance(cmd_which, str) + cmd_run = cmdmod.run("which cat") + assert isinstance(cmd_run, str) + assert cmd_which.rstrip() == cmd_run.rstrip() + + +@pytest.mark.skip_on_windows +@pytest.mark.skip_if_binaries_missing("which") +def test_which_bin(cmdmod): + """ + cmd.which_bin + """ + cmds = ["pip3", "pip2", "pip", "pip-python"] + ret = cmdmod.which_bin(cmds) + assert os.path.split(ret)[1] in cmds + + +@pytest.mark.slow_test +def test_has_exec(cmdmod, available_python_executable): + """ + cmd.has_exec + """ + assert cmdmod.has_exec(available_python_executable) + assert not cmdmod.has_exec("alllfsdfnwieulrrh9123857ygf") + + +@pytest.mark.slow_test +def test_exec_code(cmdmod, available_python_executable): + """ + cmd.exec_code + """ + code = dedent( + """ + import sys + sys.stdout.write('cheese') + """ + ) + assert cmdmod.exec_code(available_python_executable, code).rstrip() == "cheese" + + +@pytest.mark.slow_test +def test_exec_code_with_single_arg(cmdmod, available_python_executable): + """ + cmd.exec_code + """ + code = dedent( + """ + import sys + sys.stdout.write(sys.argv[1]) + """ + ) + arg = "cheese" + assert cmdmod.exec_code(available_python_executable, code, args=arg).rstrip() == arg + + +@pytest.mark.slow_test +def test_exec_code_with_multiple_args(cmdmod, available_python_executable): + """ + cmd.exec_code + """ + code = dedent( + """ + import sys + sys.stdout.write(sys.argv[1]) + """ + ) + arg = "cheese" + assert ( + cmdmod.exec_code(available_python_executable, code, args=[arg, "test"]).rstrip() + == arg + ) + + +@pytest.mark.slow_test +def test_quotes(cmdmod): + """ + cmd.run with quoted command + """ + cmd = """echo 'SELECT * FROM foo WHERE bar="baz"' """ + expected_result = 'SELECT * FROM foo WHERE bar="baz"' + result = cmdmod.run_stdout(cmd).strip() + assert result == expected_result + + +@pytest.mark.skip_if_not_root +@pytest.mark.skip_on_windows(reason="Skip on Windows, requires password") +def test_quotes_runas(cmdmod, running_username): + """ + cmd.run with quoted command + """ + cmd = """echo 'SELECT * FROM foo WHERE bar="baz"' """ + expected_result = 'SELECT * FROM foo WHERE bar="baz"' + result = cmdmod.run_all(cmd, runas=running_username) + errmsg = f"The command returned: {result}" + assert result["retcode"] == 0, errmsg + assert result["stdout"] == expected_result, errmsg + + +@pytest.mark.destructive_test +@pytest.mark.skip_if_not_root +@pytest.mark.skip_on_windows(reason="Skip on Windows, uses unix commands") +@pytest.mark.slow_test +def test_cwd_runas(cmdmod, usermod, runas_usr, tmp_path): + """ + cmd.run should be able to change working directory correctly, whether + or not runas is in use. + """ + cmd = "pwd" + tmp_cwd = str(tmp_path) + os.chmod(tmp_cwd, 0o711) + + cwd_normal = cmdmod.run_stdout(cmd, cwd=tmp_cwd).rstrip("\n") + assert tmp_cwd == cwd_normal + + cwd_runas = cmdmod.run_stdout(cmd, cwd=tmp_cwd, runas=runas_usr).rstrip("\n") + assert tmp_cwd == cwd_runas + + +@pytest.mark.destructive_test +@pytest.mark.skip_if_not_root +@pytest.mark.skip_unless_on_darwin(reason="Applicable to MacOS only") +@pytest.mark.slow_test +def test_runas_env(cmdmod, usermod, runas_usr): + """ + cmd.run should be able to change working directory correctly, whether + or not runas is in use. + """ + user_path = cmdmod.run_stdout('printf %s "$PATH"', runas=runas_usr) + # XXX: Not sure of a better way. Environment starts out with + # /bin:/usr/bin and should be populated by path helper and the bash + # profile. + assert "/bin:/usr/bin" != user_path + + +@pytest.mark.destructive_test +@pytest.mark.skip_if_not_root +@pytest.mark.skip_unless_on_darwin(reason="Applicable to MacOS only") +@pytest.mark.slow_test +def test_runas_complex_command_bad_cwd(cmdmod, usermod, runas_usr, tmp_path): + """ + cmd.run should not accidentally run parts of a complex command when + given a cwd which cannot be used by the user the command is run as. + Due to the need to use `su -l` to login to another user on MacOS, we + cannot cd into directories that the target user themselves does not + have execute permission for. To an extent, this test is testing that + buggy behaviour, but its purpose is to ensure that the greater bug of + running commands after failing to cd does not occur. + """ + tmp_cwd = str(tmp_path) + os.chmod(tmp_cwd, 0o700) + cmd_result = cmdmod.run_all( + 'pwd; pwd; : $(echo "You have failed the test" >&2)', + cwd=tmp_cwd, + runas=runas_usr, + ) + assert "" == cmd_result["stdout"] + assert "You have failed the test" not in cmd_result["stderr"] + assert 0 != cmd_result["retcode"] + + +@SKIP_INITIAL_PHOTONOS_FAILURES +@pytest.mark.skip_on_windows +@pytest.mark.skip_if_not_root +@pytest.mark.destructive_test +@pytest.mark.slow_test +def test_runas(cmdmod, usermod, runas_usr): + """ + Ensure that the env is the runas user's + """ + out = cmdmod.run("env", runas=runas_usr).splitlines() + assert f"USER={runas_usr}" in out + + +@pytest.mark.skip_if_binaries_missing("sleep", reason="sleep cmd not installed") +def test_timeout(cmdmod): + """ + cmd.run trigger timeout + """ + out = cmdmod.run("sleep 2 && echo hello", timeout=1, python_shell=True) + assert "Timed out" in out + + +@pytest.mark.skip_if_binaries_missing("sleep", reason="sleep cmd not installed") +def test_timeout_success(cmdmod): + """ + cmd.run sufficient timeout to succeed + """ + out = cmdmod.run("sleep 1 && echo hello", timeout=2, python_shell=True) + assert out == "hello" + + +@pytest.mark.slow_test +def test_cmd_run_whoami(cmdmod, running_username): + """ + test return of whoami + """ + if not salt.utils.platform.is_windows(): + user = running_username + else: + user = salt.utils.user.get_specific_user() + if user.startswith("sudo_"): + user = user.replace("sudo_", "") + cmd = cmdmod.run("whoami") + assert user.lower() == cmd.lower() + + +@pytest.mark.skip_on_windows( + reason="Skip on Windows, Windows enviroment issues for this test - test needs fixing" +) +@pytest.mark.skip_on_linux +@pytest.mark.slow_test +def test_windows_env_handling(cmdmod): + """ + Ensure that nt.environ is used properly with cmd.run* + """ + out = cmdmod.run("set", env={"abc": "123", "ABC": "456"}).splitlines() + assert "abc=456" in out + + +@pytest.mark.slow_test +@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") +def test_windows_powershell_script_args(cmdmod, issue_56195_test_ps1): + """ + Ensure that powershell processes inline script in args + """ + val = "i like cheese" + args = ( + '-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)' + " -ErrorAction Stop".format(val) + ) + script = "salt://issue_56195_test.ps1" + ret = cmdmod.script(script, args=args, shell="powershell", saltenv="base") + assert ret["stdout"] == val + + +@pytest.mark.slow_test +@pytest.mark.skip_unless_on_windows(reason="Minion is not Windows") +@pytest.mark.skip_if_binaries_missing("pwsh") +def test_windows_powershell_script_args_pwsh(cmdmod, issue_56195_test_ps1): + """ + Ensure that powershell processes inline script in args with powershell + core + """ + val = "i like cheese" + args = ( + '-SecureString (ConvertTo-SecureString -String "{}" -AsPlainText -Force)' + " -ErrorAction Stop".format(val) + ) + script = "salt://issue_56195_test.ps1" + ret = cmdmod.script(script, args=args, shell="pwsh", saltenv="base") + assert ret["stdout"] == val diff --git a/tests/pytests/functional/modules/test_yumpkg.py b/tests/pytests/functional/modules/test_yumpkg.py index 88dcf0ee56b4..f4ef82b8d3b2 100644 --- a/tests/pytests/functional/modules/test_yumpkg.py +++ b/tests/pytests/functional/modules/test_yumpkg.py @@ -1,14 +1,17 @@ +import shutil + import pytest import salt.modules.cmdmod import salt.modules.config import salt.modules.pkg_resource import salt.modules.yumpkg +import salt.utils.files import salt.utils.pkg.rpm +from salt.exceptions import SaltInvocationError pytestmark = [ pytest.mark.skip_if_binaries_missing("rpm", "yum"), - pytest.mark.slow_test, ] @@ -39,6 +42,31 @@ def configure_loader_modules(minion_opts, grains): } +@pytest.fixture +def repo_basedir(tmp_path): + basedir = tmp_path / "test_yum" + basedir.mkdir(exist_ok=True, parents=True) + repo_file = basedir / "salt-test-repo.repo" + file_contents = [ + "[salt-repo]", + "name=Salt repo for RHEL/CentOS 8 PY3", + "baseurl=https://repo.saltproject.io/salt/py3/redhat/8/x86_64/latest", + "skip_if_unavailable=True", + "priority=10", + "enabled_metadata=1", + "gcheck=1", + "gkey=https://repo.saltproject.io/salt/py3/redhat/8/x86_64/latest/SALT-PROJECT-GPG-PUBKEY-2023.pub", + ] + with salt.utils.files.fopen(str(repo_file), "w") as fd: + for line in file_contents: + fd.write(f"{line}\n") + try: + yield basedir + finally: + shutil.rmtree(str(basedir)) + + +@pytest.mark.slow_test def test_yum_list_pkgs(grains): """ compare the output of rpm -qa vs the return of yumpkg.list_pkgs, @@ -58,6 +86,7 @@ def test_yum_list_pkgs(grains): @pytest.mark.destructive_test @pytest.mark.skip_if_not_root +@pytest.mark.slow_test def test_yumpkg_remove_wildcard(): salt.modules.yumpkg.install(pkgs=["httpd-devel", "httpd-tools"]) ret = salt.modules.yumpkg.remove(name="httpd-*") @@ -65,3 +94,167 @@ def test_yumpkg_remove_wildcard(): assert ret["httpd-devel"]["old"] assert not ret["httpd-tools"]["new"] assert ret["httpd-tools"]["old"] + + +def test_yumpkg_mod_repo_fails(repo_basedir): + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="basedir-fail-test", basedir="/fake/directory" + ) + assert ( + str(excinfo.value) + == "The repo does not exist and needs to be created, but none of the following basedir directories exist: ['/fake/directory']" + ) + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="missing-name-fail", basedir=str(repo_basedir) + ) + assert ( + str(excinfo.value) + == "The repo does not exist and needs to be created, but a name was not given" + ) + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="missing-url-fail", basedir=str(repo_basedir), name="missing_url" + ) + assert ( + str(excinfo.value) + == "The repo does not exist and needs to be created, but none of baseurl, mirrorlist, metalink was given" + ) + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="too-many-url-fail", + basedir=str(repo_basedir), + name="toomanyurls", + baseurl="https://example.com", + mirrorlist="https://example.com", + metalink="https://example.com", + ) + assert ( + str(excinfo.value) + == "One and only one of baseurl, mirrorlist, metalink can be used" + ) + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="too-many-url-fail", + basedir=str(repo_basedir), + name="toomanyurls", + baseurl="https://example.com", + metalink="https://example.com", + ) + assert ( + str(excinfo.value) + == "One and only one of baseurl, mirrorlist, metalink can be used" + ) + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="too-many-url-fail", + basedir=str(repo_basedir), + name="toomanyurls", + baseurl="https://example.com", + mirrorlist="https://example.com", + ) + assert ( + str(excinfo.value) + == "One and only one of baseurl, mirrorlist, metalink can be used" + ) + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="too-many-url-fail", + basedir=str(repo_basedir), + name="toomanyurls", + mirrorlist="https://example.com", + metalink="https://example.com", + ) + assert ( + str(excinfo.value) + == "One and only one of baseurl, mirrorlist, metalink can be used" + ) + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="salt-repo", + basedir=str(repo_basedir), + name="", + ) + assert str(excinfo.value) == "The repo name cannot be deleted" + + with pytest.raises(SaltInvocationError) as excinfo: + salt.modules.yumpkg.mod_repo( + repo="salt-repo", + basedir=str(repo_basedir), + name="Salt repo for RHEL/CentOS 8 PY3", + baseurl="", + ) + assert str(excinfo.value).startswith("Cannot delete baseurl without specifying") + assert "metalink" in str(excinfo.value) + assert "mirrorlist" in str(excinfo.value) + + +def test_yumpkg_mod_repo_nochange(repo_basedir): + repo_file = repo_basedir / "salt-test-repo.repo" + test_enable = salt.modules.yumpkg.mod_repo( + repo="salt-repo", + basedir=str(repo_basedir), + name="Salt repo for RHEL/CentOS 8 PY3", + enable=True, + ) + # check return on thing that we included in command + assert test_enable[str(repo_file)]["salt-repo"]["enable"] is True + # check return on thing that we did not include in command + assert ( + test_enable[str(repo_file)]["salt-repo"]["gkey"] + == "https://repo.saltproject.io/salt/py3/redhat/8/x86_64/latest/SALT-PROJECT-GPG-PUBKEY-2023.pub" + ) + + +def test_yumpkg_mod_repo_baseurl_to_metalink(repo_basedir): + repo_file = repo_basedir / "salt-test-repo.repo" + test_metalink = salt.modules.yumpkg.mod_repo( + repo="salt-repo", + basedir=str(repo_basedir), + name="Salt repo for RHEL/CentOS 8 PY3", + metalink="https://example.com", + ) + # new metalink item? + assert ( + test_metalink[str(repo_file)]["salt-repo"]["metalink"] == "https://example.com" + ) + # make sure baseurl is not in the result + assert "baseurl" not in test_metalink[str(repo_file)]["salt-repo"] + + +def test_yumpkg_mod_repo_baseurl_to_mirrorlist(repo_basedir): + repo_file = repo_basedir / "salt-test-repo.repo" + test_mirrorlist = salt.modules.yumpkg.mod_repo( + repo="salt-repo", + basedir=str(repo_basedir), + name="Salt repo for RHEL/CentOS 8 PY3", + mirrorlist="https://example.com", + ) + # new metalink item? + assert ( + test_mirrorlist[str(repo_file)]["salt-repo"]["mirrorlist"] + == "https://example.com" + ) + # make sure baseurl is not in the result + assert "baseurl" not in test_mirrorlist[str(repo_file)]["salt-repo"] + + +def test_yumpkg_mod_repo_create_repo(repo_basedir): + repo_file = repo_basedir / "test.repo" + test_repo = salt.modules.yumpkg.mod_repo( + repo="test", + basedir=str(repo_basedir), + name="test repo", + baseurl="https://example.com", + ) + # new metalink item? + assert test_repo[str(repo_file)]["test"]["baseurl"] == "https://example.com" + assert test_repo[str(repo_file)]["test"]["name"] == "test repo" diff --git a/tests/pytests/integration/modules/test_cmdmod.py b/tests/pytests/integration/modules/test_cmdmod.py index 4e8ce5824ee4..d9c326c3f0a2 100644 --- a/tests/pytests/integration/modules/test_cmdmod.py +++ b/tests/pytests/integration/modules/test_cmdmod.py @@ -1,5 +1,11 @@ +import logging + import pytest +import salt.utils.user + +log = logging.getLogger(__name__) + @pytest.fixture(scope="module") def non_root_account(): @@ -7,6 +13,14 @@ def non_root_account(): yield account +@pytest.fixture +def running_username(): + """ + Return the username that is running the code. + """ + return salt.utils.user.get_user() + + @pytest.mark.skip_if_not_root def test_exec_code_all(salt_call_cli, non_root_account): ret = salt_call_cli.run( @@ -22,3 +36,82 @@ def test_long_stdout(salt_cli, salt_minion): ) assert ret.returncode == 0 assert len(ret.data.strip()) == len(echo_str) + + +@pytest.mark.skip_if_not_root +@pytest.mark.skip_on_windows(reason="Skip on Windows, uses unix commands") +def test_avoid_injecting_shell_code_as_root( + salt_call_cli, non_root_account, running_username +): + """ + cmd.run should execute the whole command as the "runas" user, not + running substitutions as root. + """ + cmd = "echo $(id -u)" + + ret = salt_call_cli.run("cmd.run_stdout", cmd) + root_id = ret.json + ret = salt_call_cli.run("cmd.run_stdout", cmd, runas=running_username) + runas_root_id = ret.json + + ret = salt_call_cli.run("cmd.run_stdout", cmd, runas=non_root_account.username) + user_id = ret.json + + assert user_id != root_id + assert user_id != runas_root_id + assert root_id == runas_root_id + + +@pytest.mark.slow_test +def test_blacklist_glob(salt_call_cli): + """ + cmd_blacklist_glob + """ + cmd = "bad_command --foo" + ret = salt_call_cli.run( + "cmd.run", + cmd, + ) + + assert ( + ret.stderr.rstrip() + == "Error running 'cmd.run': The shell command \"bad_command --foo\" is not permitted" + ) + + +@pytest.mark.slow_test +def test_hide_output(salt_call_cli): + """ + Test the hide_output argument + """ + ls_command = ( + ["ls", "/"] if not salt.utils.platform.is_windows() else ["dir", "c:\\"] + ) + + error_command = ["thiscommanddoesnotexist"] + + # cmd.run + ret = salt_call_cli.run("cmd.run", ls_command, hide_output=True) + assert ret.data == "" + + # cmd.shell + ret = salt_call_cli.run("cmd.shell", ls_command, hide_output=True) + assert ret.data == "" + + # cmd.run_stdout + ret = salt_call_cli.run("cmd.run_stdout", ls_command, hide_output=True) + assert ret.data == "" + + # cmd.run_stderr + ret = salt_call_cli.run("cmd.shell", error_command, hide_output=True) + assert ret.data == "" + + # cmd.run_all (command should have produced stdout) + ret = salt_call_cli.run("cmd.run_all", ls_command, hide_output=True) + assert ret.data["stdout"] == "" + assert ret.data["stderr"] == "" + + # cmd.run_all (command should have produced stderr) + ret = salt_call_cli.run("cmd.run_all", error_command, hide_output=True) + assert ret.data["stdout"] == "" + assert ret.data["stderr"] == "" diff --git a/tests/pytests/unit/modules/test_consul.py b/tests/pytests/unit/modules/test_consul.py index 52f1c8ece2ef..9177aea29bd3 100644 --- a/tests/pytests/unit/modules/test_consul.py +++ b/tests/pytests/unit/modules/test_consul.py @@ -1148,11 +1148,11 @@ def test_catalog_register(): token=token, node=node, address=address, - **nodemeta_kwargs + **nodemeta_kwargs, ) expected = { "data": {"Address": address, "Node": node, "NodeMeta": nodemeta}, - "message": "Catalog registration for {} successful.".format(node), + "message": f"Catalog registration for {node} successful.", "res": True, } @@ -1198,7 +1198,7 @@ def test_catalog_deregister(): checkid=checkid, ) expected = { - "message": "Catalog item {} removed.".format(node), + "message": f"Catalog item {node} removed.", "res": True, } @@ -1569,7 +1569,6 @@ def test_status_peers(): with patch.dict(consul.__salt__, {"config.get": mock_url}): result = consul.status_peers(consul_url=consul_url, token=token) expected = {"data": "test", "res": True} - assert expected == result def test_acl_create(): @@ -1580,6 +1579,13 @@ def test_acl_create(): token = "randomtoken" name = "name1" + rules = [ + { + "key": {"key": "foo/", "policy": "read"}, + "service": {"service": "bar", "policy": "write"}, + } + ] + mock_result = "test" mock_http_result = {"status": 200, "dict": mock_result} mock_http_result_false = {"status": 204, "dict": mock_result} @@ -1604,7 +1610,16 @@ def test_acl_create(): with patch.object(salt.utils.http, "query", return_value=mock_http_result): with patch.dict(consul.__salt__, {"config.get": mock_url}): result = consul.acl_create(consul_url=consul_url, token=token, name=name) - expected = {"message": "ACL {} created.".format(name), "res": True} + expected = {"message": f"ACL {name} created.", "res": True} + assert expected == result + + # test rules + with patch.object(salt.utils.http, "query", return_value=mock_http_result): + with patch.dict(consul.__salt__, {"config.get": mock_url}): + result = consul.acl_create( + consul_url=consul_url, token=token, name=name, rules=rules + ) + expected = {"message": f"ACL {name} created.", "res": True} assert expected == result @@ -1653,7 +1668,7 @@ def test_acl_update(): result = consul.acl_update( consul_url=consul_url, token=token, name=name, id=aclid ) - expected = {"message": "ACL {} created.".format(name), "res": True} + expected = {"message": f"ACL {name} created.", "res": True} assert expected == result @@ -1693,7 +1708,7 @@ def test_acl_delete(): result = consul.acl_delete( consul_url=consul_url, token=token, name=name, id=aclid ) - expected = {"message": "ACL {} deleted.".format(aclid), "res": True} + expected = {"message": f"ACL {aclid} deleted.", "res": True} assert expected == result @@ -1775,7 +1790,7 @@ def test_acl_clone(): ) expected = { "ID": aclid, - "message": "ACL {} cloned.".format(name), + "message": f"ACL {name} cloned.", "res": True, } assert expected == result @@ -1845,7 +1860,7 @@ def test_event_fire(): result = consul.event_fire(consul_url=consul_url, token=token, name=name) expected = { "data": "test", - "message": "Event {} fired.".format(name), + "message": f"Event {name} fired.", "res": True, } assert expected == result