pierrejeambrun commented on code in PR #53943:
URL: https://github.com/apache/airflow/pull/53943#discussion_r2244731085
##########
task-sdk/tests/task_sdk/definitions/test_secrets_masker.py:
##########
@@ -729,3 +731,356 @@ def test_mixed_structured_unstructured_data(self):
assert "***" in redacted_data["description"]
assert redacted_data["nested"]["token"] == "***"
assert redacted_data["nested"]["info"] == "No secrets here"
+
+
+class TestSecretsMaskerMerge:
+ """Test the merge functionality for restoring original values from
redacted data."""
+
+ @pytest.mark.parametrize(
+ ("new_value", "old_value", "name", "expected"),
+ [
+ ("***", "original_secret", "password", "original_secret"),
+ ("new_secret", "original_secret", "password", "new_secret"),
+ ("***", "original_value", "normal_field", "***"),
+ ("new_value", "original_value", "normal_field", "new_value"),
+ ("***", "original_value", None, "***"),
+ ("new_value", "original_value", None, "new_value"),
+ ],
+ )
+ def test_merge_simple_strings(self, new_value, old_value, name, expected):
+ secrets_masker = SecretsMasker()
+ with
patch("airflow.sdk.execution_time.secrets_masker._secrets_masker",
return_value=secrets_masker):
+ result = merge(new_value, old_value, name)
+ assert result == expected
+
+ def test_merge_dictionaries(self):
+ secrets_masker = SecretsMasker()
+
+ old_data = {
+ "password": "original_password",
+ "api_key": "original_api_key",
+ "normal_field": "original_normal",
+ "token": "original_token",
+ }
+
+ new_data = {
+ "password": "***",
+ "api_key": "new_api_key",
+ "normal_field": "new_normal",
+ "token": "***",
+ }
+
+ expected = {
+ "password": "original_password",
+ "api_key": "new_api_key",
+ "normal_field": "new_normal",
+ "token": "original_token",
+ }
+
+ with
patch("airflow.sdk.execution_time.secrets_masker._secrets_masker",
return_value=secrets_masker):
+ result = merge(new_data, old_data)
+ assert result == expected
+
+ def test_merge_nested_dictionaries(self):
+ secrets_masker = SecretsMasker()
+
+ old_data = {
+ "config": {"password": "original_password", "host":
"original_host"},
+ "credentials": {"api_key": "original_api_key", "username":
"original_user"},
+ }
+
+ new_data = {
+ "config": {
+ "password": "***",
+ "host": "new_host",
+ },
+ "credentials": {
+ "api_key": "new_api_key",
+ "username": "new_user",
+ },
+ }
+
+ expected = {
+ "config": {
+ "password": "original_password",
+ "host": "new_host",
+ },
+ "credentials": {
+ "api_key": "new_api_key",
+ "username": "new_user",
+ },
+ }
+
+ with
patch("airflow.sdk.execution_time.secrets_masker._secrets_masker",
return_value=secrets_masker):
+ result = merge(new_data, old_data)
+ assert result == expected
+
+ @pytest.mark.parametrize(
+ ("old_data", "new_data", "name", "expected"),
+ [
+ # Lists
+ (
+ ["original_item1", "original_item2", "original_item3"],
+ ["new_item1", "new_item2"],
+ None,
+ ["new_item1", "new_item2"],
+ ),
+ (
+ ["original_item1", "original_item2"],
+ ["new_item1", "new_item2", "new_item3", "new_item4"],
+ None,
+ ["new_item1", "new_item2", "new_item3", "new_item4"],
+ ),
+ (
+ ["secret1", "secret2", "secret3"],
+ ["***", "new_secret2", "***"],
+ "password",
+ ["secret1", "new_secret2", "secret3"],
+ ),
+ (
+ ["value1", "value2", "value3"],
+ ["***", "new_value2", "***"],
+ "normal_list",
+ ["***", "new_value2", "***"],
+ ),
+ # Tuples
+ (
+ ("original_item1", "original_item2", "original_item3"),
+ ("new_item1", "new_item2"),
+ None,
+ ("new_item1", "new_item2"),
+ ),
+ (
+ ("original_item1", "original_item2"),
+ ("new_item1", "new_item2", "new_item3", "new_item4"),
+ None,
+ ("new_item1", "new_item2", "new_item3", "new_item4"),
+ ),
+ (
+ ("secret1", "secret2", "secret3"),
+ ("***", "new_secret2", "***"),
+ "password",
+ ("secret1", "new_secret2", "secret3"),
+ ),
+ (
+ ("value1", "value2", "value3"),
+ ("***", "new_value2", "***"),
+ "normal_tuple",
+ ("***", "new_value2", "***"),
+ ),
+ # Sets
+ (
+ {"original_item1", "original_item2", "original_item3"},
+ {"new_item1", "new_item2"},
+ None,
+ {"new_item1", "new_item2"},
+ ),
+ (
+ {"original_item1", "original_item2"},
+ {"new_item1", "new_item2", "new_item3", "new_item4"},
+ None,
+ {"new_item1", "new_item2", "new_item3", "new_item4"},
+ ),
+ (
+ {"secret1", "secret2", "secret3"},
+ {"***", "new_secret2", "***"},
+ "password",
+ {"***", "new_secret2", "***"},
+ ),
+ (
+ {"value1", "value2", "value3"},
+ {"***", "new_value2", "***"},
+ "normal_tuple",
+ {"***", "new_value2", "***"},
+ ),
+ # Mixed collections
+ (
+ ["original_item1", "original_item2", "original_item3"],
+ ("new_item1", "new_item2"),
+ None,
+ ("new_item1", "new_item2"),
+ ),
+ (
+ ("original_item1", "original_item2"),
+ ["new_item1", "new_item2", "new_item3", "new_item4"],
+ None,
+ ["new_item1", "new_item2", "new_item3", "new_item4"],
+ ),
+ (
+ ["secret1", "secret2", "secret3"],
+ ("***", "new_secret2", "***"),
+ "password",
+ ("secret1", "new_secret2", "secret3"),
Review Comment:
Yes this was mimicked on the `redact` function that also has a special case
for mixture of `set/tuple` and since we do not handle `set` the mixture end up
here on `list/tuple`.
Any other type mismatch behaves as you describe.
I will remove that special mixture type handling, will make things easier
and we don't need it at the moment.
--
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]
