ashb commented on code in PR #28239:
URL: https://github.com/apache/airflow/pull/28239#discussion_r1047671128
##########
airflow/utils/log/secrets_masker.py:
##########
@@ -240,17 +241,53 @@ def redact(self, item: Redactable, name: str | None =
None) -> Redacted:
"""
return self._redact(item, name, depth=0)
- def add_mask(self, secret: str | dict | Iterable, name: str | None = None):
- """Add a new secret to be masked to this filter instance."""
+ @cached_property
+ def _mask_adapter(self) -> None | Callable:
+ """Pulls the secret mask adapter from config.
+
+ This lives in a function here to be cached and only hit the config
once.
+ """
from airflow.configuration import conf
- test_mode: bool = conf.getboolean("core", "unit_test_mode")
+ return conf.getimport("logging", "secret_mask_adapter", fallback=None)
+
+ @cached_property
+ def _test_mode(self) -> bool:
+ """Pulls the unit test mode flag from config.
+
+ This lives in a function here to be cached and only hit the config
once.
+ """
+ from airflow.configuration import conf
+
+ return conf.getboolean("core", "unit_test_mode")
+
+ def _add_adaptations(self, secret: str | dict | Iterable, name: str | None
= None):
+ """Adds any adaptations to the secret that should be masked."""
+ if self._mask_adapter:
+ # This can return an iterable of secrets to mask OR a single
secret as a string
+ secret_or_secrets = self._mask_adapter(secret)
+
+ if not isinstance(secret_or_secrets, str):
+ # if its not a string, it must be an iterable
+ for secret in secret_or_secrets:
+ self.add_mask(secret, name, add_adaptations=False)
+ else:
+ self.add_mask(secret_or_secrets, name, add_adaptations=False)
Review Comment:
```suggestion
def _adaptations(self, secret: str):
"""Adds any adaptations to the secret that should be masked."""
if not self._mask_adapter:
return [secret]
# This can return an iterable of secrets to mask OR a single secret
as a string
secret_or_secrets = self._mask_adapter(secret)
if not isinstance(secret_or_secrets, str):
# if its not a string, it must be an iterable
return secret_or_secrets
else:
return [secret]
```
##########
airflow/utils/log/secrets_masker.py:
##########
@@ -240,17 +241,53 @@ def redact(self, item: Redactable, name: str | None =
None) -> Redacted:
"""
return self._redact(item, name, depth=0)
- def add_mask(self, secret: str | dict | Iterable, name: str | None = None):
- """Add a new secret to be masked to this filter instance."""
+ @cached_property
+ def _mask_adapter(self) -> None | Callable:
+ """Pulls the secret mask adapter from config.
+
+ This lives in a function here to be cached and only hit the config
once.
+ """
from airflow.configuration import conf
- test_mode: bool = conf.getboolean("core", "unit_test_mode")
+ return conf.getimport("logging", "secret_mask_adapter", fallback=None)
+
+ @cached_property
+ def _test_mode(self) -> bool:
+ """Pulls the unit test mode flag from config.
+
+ This lives in a function here to be cached and only hit the config
once.
+ """
+ from airflow.configuration import conf
+
+ return conf.getboolean("core", "unit_test_mode")
+
+ def _add_adaptations(self, secret: str | dict | Iterable, name: str | None
= None):
+ """Adds any adaptations to the secret that should be masked."""
+ if self._mask_adapter:
+ # This can return an iterable of secrets to mask OR a single
secret as a string
+ secret_or_secrets = self._mask_adapter(secret)
+
+ if not isinstance(secret_or_secrets, str):
+ # if its not a string, it must be an iterable
+ for secret in secret_or_secrets:
+ self.add_mask(secret, name, add_adaptations=False)
+ else:
+ self.add_mask(secret_or_secrets, name, add_adaptations=False)
+
+ def add_mask(self, secret: str | dict | Iterable, name: str | None = None,
add_adaptations: bool = True):
+ """Add a new secret to be masked to this filter instance.
+
+ If add_adaptations is True, the secret mask adapter will be used to
add adaptations for the secret
+ as well.
+ """
if isinstance(secret, dict):
for k, v in secret.items():
self.add_mask(v, k)
elif isinstance(secret, str):
- if not secret or (test_mode and secret in
SECRETS_TO_SKIP_MASKING_FOR_TESTS):
+ if not secret or (self._test_mode and secret in
SECRETS_TO_SKIP_MASKING_FOR_TESTS):
return
+ if add_adaptations:
+ self._add_adaptations(secret, name)
pattern = re.escape(secret)
if pattern not in self.patterns and (not name or
should_hide_value_for_key(name)):
self.patterns.add(pattern)
Review Comment:
This suggestion won't blindly apply, but we should remove the current
`self.replacer = re.compile("|".join(self.patterns))` line
```suggestion
new_mask = False
for s in self._adaptation(secret):
pattern = re.escape(secret)
if pattern not in self.patterns and (not name or
should_hide_value_for_key(name)):
self.patterns.add(pattern)
new_mask = True
if new_mask:
self.replacer = re.compile("|".join(self.patterns))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
