amoghrajesh commented on code in PR #55259:
URL: https://github.com/apache/airflow/pull/55259#discussion_r2322517195


##########
shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py:
##########
@@ -779,6 +820,10 @@ def test_mixed_structured_unstructured_data(self):
 class TestSecretsMaskerMerge:
     """Test the merge functionality for restoring original values from 
redacted data."""
 
+    def setup_method(self):
+        self.masker = SecretsMasker()
+        configure_secrets_masker_for_test(self.masker)

Review Comment:
   `merge` doesnt really need separate instance of masker per test as it 
doesn't change state of it



##########
shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py:
##########
@@ -70,38 +70,13 @@ def to_dict(self) -> dict[str, Any]: ...
 """Common terms that should be excluded from masking in both production and 
tests"""
 
 
-@cache
-def get_min_secret_length() -> int:
-    """Get minimum length for a secret to be considered for masking from 
airflow.cfg."""
-    from airflow.configuration import conf
-
-    return conf.getint("logging", "min_length_masked_secret", fallback=5)
-
-
-@cache
-def get_sensitive_variables_fields():
-    """Get comma-separated sensitive Variable Fields from airflow.cfg."""
-    from airflow.configuration import conf
-
-    sensitive_fields = DEFAULT_SENSITIVE_FIELDS.copy()
-    sensitive_variable_fields = conf.get("core", "sensitive_var_conn_names")
-    if sensitive_variable_fields:
-        sensitive_fields |= frozenset({field.strip() for field in 
sensitive_variable_fields.split(",")})
-    return sensitive_fields
-
-
 def should_hide_value_for_key(name):
     """
     Return if the value for this given name should be hidden.
 
     Name might be a Variable name, or key in conn.extra_dejson, for example.
     """
-    from airflow import settings
-
-    if isinstance(name, str) and settings.HIDE_SENSITIVE_VAR_CONN_FIELDS:
-        name = name.strip().lower()
-        return any(s in name for s in get_sensitive_variables_fields())
-    return False
+    return _secrets_masker().should_hide_value_for_key(name)

Review Comment:
   Also kept the top level function, in case its being used in the wild. 
Undecided whether to keep or chop tbh



-- 
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]

Reply via email to