Repository navigation
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey. There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
Contributor
Author
Replay PR saltstack#65004 onto current upstream/3006.x and apply the same StrictUndefined handling to the standalone tojson filter for parity with | json.
Production renders SLS with SandboxedEnvironment; assert the yaml filter still raises UndefinedError for missing keys in that setup.
…thTokenW `advapi32` is loaded with `use_last_error=True`, so the error code after a failed ctypes call must be read with `ctypes.get_last_error()`, not `win32api.GetLastError()`. The Windows live slot is reset to 0 by Python internals before the old code could read it, producing the misleading "The operation completed successfully" exception that prevented the child process from ever being resumed via ResumeThread. (saltstack#57848)
Contributor
|
@Dkhil Please add a test and change log. |
When multiple ADMX files (e.g. TerminalServer.admx and TerminalServer-Server.admx on Windows Server) define the same policy display name resolving to an identical full path, _lookup_admin_template now deduplicates the matches and returns success rather than a spurious "multiple policies" error. Closes saltstack#62732
When the eauth token backend raised any exception, the previous code
either propagated it or treated it as "remove the token". For Redis
or other network-backed token stores, a transient backend error
(connection drop, NFS hang, full disk) would therefore log every
authenticated user out -- ``rm_token`` deletes valid tokens that
just briefly could not be read.
Split the ``except`` into two cases:
* ``SaltDeserializationError`` -- the on-disk blob is corrupt and
cannot be parsed. The token is permanently unusable, so removing
it is correct: leaving it around would make every subsequent
``get_tok`` for the same id keep failing.
* ``OSError`` (covers ``IOError``) -- transient backend error. The
token itself is fine; do NOT delete it. Return ``{}`` so the
caller treats this request as not-authenticated, and the next
request retries against the backend.
Three behavioural tests guard each branch and the existing expired
path. The IOError test fails on the previous implementation, which
is the regression this change exists to prevent.
Refs: saltstack#69073
CI saltpylint flagged `salt/auth/__init__.py:253` with `W1404(implicit-str-concat)`. The two adjacent string literals on that line are the result of black collapsing what was originally a two-line string onto a single line, which saltpylint reads as a missing comma. Merge them into a single string literal; the message remains under 88 columns so black leaves it alone. Refs: saltstack#69073
Address review feedback from @bdrx312 on PR saltstack#69074: the original `log.warning` calls in the SaltDeserializationError and OSError handlers in `LoadAuth.get_tok` did not capture the exception at all, making troubleshooting harder. Two combined changes: 1. Add the exception class+message inline via `%r` so each occurrence is one greppable line that names which subclass (e.g. `ConnectionResetError`) and its message produced the warning -- enough to triage without a full traceback. 2. Add a companion `log.debug(..., exc_info=True)` per @bdrx312's suggestion. Operators who need the full traceback for a specific intermittent failure can raise the level to DEBUG and see it. Costs nothing at the default WARNING level because logging skips exc_info formatting when the level is disabled. This avoids GBs/hour of stack frames during a flapping Redis or NFS outage (the original `exc_info=True` approach considered earlier) while keeping full diagnostic depth one log-level change away. Refs: saltstack#69073
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
5 participants
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?
fix authification for etcd module when using user/password based auth
Revolution1/etcd3-py#41
What issues does this PR fix or reference?
Fixes
auth when using user/passsword based authentification with etcdv3
#69108