Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't fail to log if we can't redact something #16118

Merged
merged 1 commit into from Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 35 additions & 18 deletions airflow/utils/log/secrets_masker.py
Expand Up @@ -16,6 +16,7 @@
# under the License.
"""Mask sensitive information from logs"""
import collections
import io
import logging
import re
from typing import TYPE_CHECKING, Iterable, Optional, Set, TypeVar, Union
Expand All @@ -27,6 +28,10 @@

RedactableItem = TypeVar('RedactableItem')


log = logging.getLogger(__name__)


DEFAULT_SENSITIVE_FIELDS = frozenset(
{
'password',
Expand Down Expand Up @@ -173,24 +178,36 @@ def redact(self, item: "RedactableItem", name: str = None) -> "RedactableItem":
is redacted.

"""
if name and should_hide_value_for_key(name):
return self._redact_all(item)

if isinstance(item, dict):
return {dict_key: self.redact(subval, dict_key) for dict_key, subval in item.items()}
elif isinstance(item, str):
if self.replacer:
# We can't replace specific values, but the key-based redacting
# can still happen, so we can't short-circuit, we need to walk
# the structure.
return self.replacer.sub('***', item)
return item
elif isinstance(item, (tuple, set)):
# Turn set in to tuple!
return tuple(self.redact(subval) for subval in item)
elif isinstance(item, Iterable):
return list(self.redact(subval) for subval in item)
else:
try:
if name and should_hide_value_for_key(name):
return self._redact_all(item)

if isinstance(item, dict):
return {dict_key: self.redact(subval, dict_key) for dict_key, subval in item.items()}
elif isinstance(item, str):
if self.replacer:
# We can't replace specific values, but the key-based redacting
# can still happen, so we can't short-circuit, we need to walk
# the structure.
return self.replacer.sub('***', item)
return item
elif isinstance(item, (tuple, set)):
# Turn set in to tuple!
return tuple(self.redact(subval) for subval in item)
elif isinstance(item, io.IOBase):
return item
elif isinstance(item, Iterable):
return list(self.redact(subval) for subval in item)
else:
return item
except Exception as e: # pylint: disable=broad-except
log.warning(
"Unable to redact %r, please report this via <https://github.com/apache/airflow/issues>. "
"Error was: %s: %s",
item,
type(e).__name__,
str(e),
)
return item

# pylint: enable=too-many-return-statements
Expand Down
24 changes: 24 additions & 0 deletions tests/utils/log/test_secrets_masker.py
Expand Up @@ -72,6 +72,22 @@ def test_args(self, logger, caplog):

assert caplog.text == "INFO Cannot connect to user:***\n"

def test_non_redactable(self, logger, caplog):
class NonReactable:
def __iter__(self):
raise RuntimeError("force fail")

def __repr__(self):
return "<NonRedactable>"

logger.info("Logging %s", NonReactable())

assert caplog.messages == [
"Unable to redact <NonRedactable>, please report this via "
+ "<https://github.com/apache/airflow/issues>. Error was: RuntimeError: force fail",
"Logging <NonRedactable>",
]

def test_extra(self, logger, caplog):
logger.handlers[0].formatter = ShortExcFormatter("%(levelname)s %(message)s %(conn)s")
logger.info("Cannot connect", extra={'conn': "user:password"})
Expand Down Expand Up @@ -202,6 +218,14 @@ def test_redact(self, patterns, name, value, expected):

assert filt.redact(value, name) == expected

def test_redact_filehandles(self, caplog):
filt = SecretsMasker()
with open("/dev/null", "w") as handle:
assert filt.redact(handle, None) == handle

# We shouldn't have logged a warning here
assert caplog.messages == []


class TestShouldHideValueForKey:
@pytest.mark.parametrize(
Expand Down