Repository navigation
Conversation
Pepper gives SALTAPI_* environment variables precedence over the config file, and the test suite never cleared them, despite the pepper_cli fixture claiming to run "inside an empty env". A developer with SALTAPI_URL, SALTAPI_USER or SALTAPI_EAUTH exported would have every CLI test log in to that salt-api with the test credentials instead of the salt-api started by the suite, and unit tests such as test_interactive_logins fail. Add an autouse fixture that removes SALTAPI_*, PEPPER* and MASTER_CONFIG from the environment for every test.
The imp module was removed in Python 3.12, so collecting tests/unit/test_init.py fails there. Load the fresh copy of pepper/__init__.py with importlib.util instead. The version is read from the installed package metadata rather than setup.py, so stop renaming setup.py during the test (which left the checkout broken if the test was interrupted), compare against importlib.metadata instead of setuptools_scm, and cover the not-installed fallback explicitly.
- importlib-metadata<5.0.0 cannot be co-installed with current Salt releases and pepper uses the standard library importlib.metadata. - The cryptography and pyzmq pins conflict with what Salt itself requires (cryptography>=48 for 3006.30 on Python 3.10+); let Salt choose them. - mock and setuptools_scm are no longer used by the test suite. - Bump pytest-salt-factories to 1.0.5.
[os.environ.get("X")] or [...] is always a one element list, so without
API_BACKEND and PYTHON_VERSIONS set the tests session ran with
python=None and --salt-api-backend=rest_None. Split the environment
values and fall back to the full lists when they are unset, moving the
default Python versions to 3.10-3.13.
Also make pip quiet outside CI as the comment intends (CI_RUN is a
string or None, never False) and replace the coverage 5.5 pin, which
predates Python 3.12 support.
Test Python 3.10 through 3.13 against the current Salt releases (3006.30, 3007.17, 3008.5) and master, dropping Python 3.8 and 3.9, which are end of life. Pin actions/checkout and actions/setup-python to the same commit SHAs the Salt repository uses and restrict the workflow token to read-only contents access.
CI runs on GitHub Actions. The Travis and Azure Pipelines definitions target Python 2.7 to 3.8 and Salt 2018.3/2019.2, the Makefile drives the removed tox setup, and nothing uploads coverage to Codecov any more. azure-pipelines.yml also contained a Codecov upload token. Replace the Travis and Codecov badges in the README with the GitHub Actions status badge.
The test minion never completes authentication with the master for Salt 3006 on Python 3.12 and 3.13, or Salt 3007 on Python 3.13, so every test that needs a minion errors during setup. Salt 3007 on Python 3.12 usually passes, but in 7 of 50 recent CI jobs the minion failed to start, stopped answering, or took about a minute per call. Five of those jobs failed and the slowest ran for 81 minutes, against 2 to 7 minutes normally, so exclude it as well. Salt 3006 and 3007 declare support up to Python 3.10; both pass on 3.11, so keep those.
On Salt 3008 the netapi ssh client authorizes the request itself and looks the token up in salt.cache, which does not find the token minted through /login or /token, so salt-api answers 401 for both the session and /run paths. Every other client still passes on 3008. Mark the test xfail (non-strict) until the cause is understood instead of letting it hold the rest of the matrix red.
A job normally finishes in 2 to 7 minutes. When the test minion hangs, the job otherwise runs until GitHub's 6 hour limit and holds a runner the whole time.
This was referenced Nov 5, 2026
test_token wrote a real ~/.peppercache, and a developer's ~/.pepperrc could leak into the tests. Point HOME (and USERPROFILE) at an empty directory for every test.
Use the same hook versions and arguments as Salt's master branch: pyupgrade --py310-plus, isort and black at Salt's settings instead of reorder-python-imports and black -l 100, bandit, yamllint and pylint with saltpylint using a copy of Salt's .pylintrc. The saltpylint blacklist plugin is not loaded. It requires Salt's own helpers (salt.utils.files.fopen, tests.support.mock), and pepper does not depend on salt. pylint replaces flake8, so the flake8 workflow and nox session are removed and the pre-commit job in the test workflow is the only lint step.
No functional changes. This is the output of the pre-commit formatting hooks added in the previous commit.
Drop the urllib2, ConfigParser, raw_input and logging.NullHandler fallbacks. The JSONDecodeError fallback looked up json.decode, which does not exist, so it always fell back to ValueError; import json.JSONDecodeError instead.
- Remove unreachable return statements after raise in req_get and req_stream. - Format the 'Cannot open file' message instead of passing the file name as a second exception argument, which printed a tuple. - Set the umask before the try block in login() so the finally clause cannot reference an unset variable. - Open text files as UTF-8 instead of the locale encoding. - Mark the intentional catch-all handlers and the optparse import. The broad handlers around the token cache and the global SSL context are left in place; later changes replace that code.
Use the same towncrier configuration and template as Salt, with fragments named changelog/<issue>.<type>.md. Document the pre-commit hooks, changelog fragments and the develop branch in the README; the master branch it mentioned no longer exists.
Replace setup.py, setup_requires and MANIFEST.in with PEP 621 metadata built by setuptools and setuptools-scm. Declare requires-python >= 3.10, drop the Python 2 and Cython classifiers, and add requests as an extra for req_get(), req_stream() and kerberos logins. Install pepper as a console_scripts entry point instead of shipping scripts/pepper and scripts/pepper.cmd, so pip creates pepper.exe on Windows, and add python -m pepper. black reads the Python 3.10 floor from requires-python and now uses parenthesized context managers in two tests.
Replace the Python 2.5+ claim and the archived salt-api link, describe the extras and the pepper command, and note that cmd.exe passes single quotes through literally.
Build the sdist and wheel when a GitHub release is published, check that the version setuptools-scm derived matches the release tag, and upload with PyPI trusted publishing from the release environment, so no API token is stored in the repository. Document the release steps in the README. Based on the workflow proposed in saltstack#606. Co-authored-by: Paul Topley <paul.topley@imperva.com>
libpepper replaced ssl._create_default_https_context with the unverified stdlib context at import time. That turned off certificate checks for every HTTPS request in the process, including those made by other libraries, and made --ignore-ssl-errors a no-op. Build the SSL context per request instead, and only skip verification when ignore_ssl_errors is set. Use the opener directly rather than installing it globally, which also keeps --debug-http working together with --ignore-ssl-errors, and report a failed verification with a hint about --ignore-ssl-errors.
get_login_details() and parse_url() each parsed the config file and applied environment overrides. Move that into get_config() so new settings do not need a third copy.
Add --ca-bundle, --client-cert and --client-cert-key, also settable as SALTAPI_CA_BUNDLE, SALTAPI_CLIENT_CERT and SALTAPI_CLIENT_CERT_KEY in .pepperrc or the environment, and the matching Pepper() arguments. They apply to both the urllib and the requests code paths. The client certificate is presented even when --ignore-ssl-errors is set. The SSL context is now built once per client and only for https URLs, so an encrypted client key is unlocked once, and a CA bundle or client certificate that cannot be loaded is reported with its path. The verification error mentions --ca-bundle. Based on saltstack#233. Co-authored-by: Skyler Hawthorne <skyler@dead10ck.dev>
The SSL context was only passed when the salt-api URL was https, so a redirect from http to https verified against the system CAs and sent no client certificate. Build the context on the first HTTPS connection instead; a plain http URL still never loads the TLS files. Refuse a redirect from https to http, and drop X-Auth-Token and Authorization when a redirect goes to another host or port, as requests does for Authorization. With TLS 1.3 a server that wants a client certificate rejects it after the handshake, so the alert surfaced as an uncaught ssl.SSLError when the response was read. Report TLS errors as a PepperException that says a client certificate is needed. Pepper() now raises when client_cert_key is given without client_cert, as the command line already did. The changelog points at --ca-bundle for private CAs before --ignore-ssl-errors.
The --make-token cache held a single token, so profiles pointing at different salt-api servers overwrote each other's token and sent it to the wrong server (saltstack#205). A token that salt-api had already forgotten, for example after a restart, was used until it expired, failing every command with 'Authentication denied' (saltstack#105). Store one token for each URL, user and eauth, and drop expired entries when the file is written. A username that is prompted for is asked before the lookup so it is part of the key. Write the file through a 0600 temporary file that is renamed into place, instead of reopening the existing file without truncating it. A cache in the old single-token format is ignored and replaced. Keep the salt-api version with the token. It normally comes with the login response, and without it a cached token made pepper drop --timeout and split runner and wheel arguments itself. salt-api answering 401 now raises PepperAuthException, which becomes a subclass of PepperException. When the rejected token came from the cache, log in again and retry the command once. salt-api also answers 401 when the user may not run the command, so the cached token is only replaced by a successful login, and the original error is raised when there are no credentials to log in with. The unit tests use a temporary cache file instead of ~/.peppercache.
The cache key did not say whether the token came from /login or from /token for --run-uri, so -T -r replaced the -T token for the same URL, user and eauth, and the next -T run failed with it. Add the token type to the key. When salt-api rejected a cached token and pepper could not log in again, the rejected token stayed in the file and was offered on every run. Remove it. A cached or returned expire that is not a number crashed the cache write. Treat it as expired.
Pepper.local(), local_async() and local_batch() sent expr_form, which Salt removed in 2019.2, so salt-api ignored it and targeted the expression as a glob. Send tgt_type instead and keep expr_form as a deprecated keyword alias that warns. The argument stays in the same position, so positional callers get the targeting they asked for. Add a full_return argument to local() so library users can get minion retcodes the way the CLI does. It is not added to local_async(), which only returns the job ID, or local_batch(), whose results salt-api returns without retcodes. Build the low data for the three local clients in one place. Rename the CLI option destination from expr_form to tgt_type to match. Based on saltstack#186 and saltstack#175. Co-authored-by: Maximilien Cuony <maximilien@theglu.org> Co-authored-by: Ryan Frantz <ryanleefrantz@gmail.com>
Without salt, the result passed to PepperRetcode.validate() was still the JSON text, so none of the retcodes were found: --fail-any always exited 0 and --fail-any-none always exited 255. Parse and unwrap the result once, before choosing how to display it. Based on saltstack#201. Co-authored-by: Leo Davis <leo.davis@sling.com>
Without a --fail-* option pepper always exited 0. Follow the salt and salt-run commands instead: exit with the retcode of a runner or wheel job that failed, found the way salt-run finds it, otherwise exit 1 and print salt's 'Minions returned with non-zero exit code' message when any minion returned a non-zero retcode. The --fail-* validators only looked at the minions in the first return, and read the job data of a runner return as if it were a minion. They now collect retcodes from every return, including runner and wheel jobs, so --fail-any reports a failed orchestration. This supersedes saltstack#196, which searched the return data recursively for retcode keys and so also picked up keys from inside command output.
…heel call, as failures
salt-api 3007 and 3008 answer a local or local_async command whose target matches no minions with an empty return, which pepper printed and exited 0 for. Print salt's "No minions matched the target. No command was sent, no jid was assigned." to stderr and exit with 2, like the salt command, when no --fail-* option is given. A minion that matched but did not respond still has an entry (a bare False on 3007, a no_return entry on 3008), so it is not taken for no match.
req() turned 401 and 500 responses into fixed 'Authentication denied' and 'Server error.' messages and dropped the body, and re-raised other HTTP and connection errors as raw urllib exceptions, which the CLI reported as an uncaught error. req_get() and req_stream() printed the error and returned None. Turn every HTTP error into a PepperException, or PepperAuthException for 401, with the message from salt-api's JSON or HTML error body, and report connection failures and invalid JSON the same way, for the requests code paths too. req_get() and req_stream() now raise instead of returning None, and share their request code. Print uncaught errors to stderr. Log pepper's messages through one handler on the pepper logger instead of the root logger: importing salt installs a root handler of its own, which printed every error a second time. Other libraries' debug messages are no longer shown with -vvv.
--debug-http used http.client's debug output, which printed the login password, the X-Auth-Token header and the session cookie to stdout, and logged the login response with its token. Print the exchange to stderr, as the option's help says, with those values replaced by [redacted], and stop logging the Kerberos request parameters.
Without salt's outputters each poll result was printed as its own JSON document, so the output could not be parsed as JSON (saltstack#104). Collect the results and print them as one document. With salt's outputters they are still shown as each minion returns. The poller always sent the first command with local_async, whatever --client, --batch or --json asked for (saltstack#194). Refuse anything but a single local command instead. The help text named the jobs.list_jobs runner, but the poller calls jobs.lookup_jid, which is the function users need permission for (saltstack#95). The list of minions that did not respond is now sorted, and the poller always ends with its exit code. jobs.lookup_jid returns no retcodes, so that exit code only says whether every minion returned, and the README says so.
Without salt's outputters the poller's returns are printed as one JSON document at the end, so an error from a later poll, such as salt-api going away, threw away the returns that had already arrived. Print them before reporting the error, as before. When no minions matched, local_async returns no jid and the poller failed with "Uncaught Pepper error: 'jid'". Report that no minions matched instead. jobs.lookup_jid returns no retcodes, so the --fail-* options could never fail a poll. Refuse them with --fail-if-incomplete instead of ignoring them.
Like the local client, report a target that matches no minions with salt's "No minions matched the target" message and exit code 2 instead of a Pepper error and 1.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Depends on #632; only the last 3 commits are new here.
local_asyncwhatever--client,--batchor--jsonsaid (--fail-if-incomplete hardcodes local_async client #194).jobs.lookup_jid, notjobs.list_jobs, which is the runner permission users need (Permission denied #95).No minions matched the targetmessage and exit 2, instead of failing withUncaught Pepper error: 'jid'.--fail-*options with--fail-if-incomplete:jobs.lookup_jidreturns no retcodes, so they could never fail a poll.jobs.lookup_jidreturns no retcodes, so the poller's exit code only says whether every minion returned; the README says so.What issues does this PR fix or reference?
Fixes #104
Fixes #194
Fixes #95
Previous Behavior
Output could not be parsed as JSON, other clients were silently replaced, and the help named the wrong runner.
New Behavior
See above.
Fork CI: https://github.com/ggiesen/pepper/actions/runs/42174289765
Merge requirements satisfied?
Commits signed with GPG?
No