jscheffl commented on code in PR #36492:
URL: https://github.com/apache/airflow/pull/36492#discussion_r1446674118
##########
tests/models/test_trigger.py:
##########
@@ -337,3 +341,47 @@ def
test_get_sorted_triggers_different_priority_weights(session, create_task_ins
trigger_ids_query = Trigger.get_sorted_triggers(capacity=100,
alive_triggerer_ids=[], session=session)
assert trigger_ids_query == [(2,), (1,)]
+
+
+class SensitiveKwargsTrigger(BaseTrigger):
+ """
+ A trigger that has sensitive kwargs.
+ """
+
+ def __init__(self, param1: str, param2: str):
+ super().__init__()
+ self.param1 = param1
+ self.param2 = param2
+
+ def serialize(self) -> tuple[str, dict[str, Any]]:
+ return (
+ "tests.models.test_trigger.SensitiveKwargsTrigger",
+ {
+ "param1": self.param1,
+ "encrypted__param2": self.param2,
+ },
+ )
+
+ async def run(self) -> AsyncIterator[TriggerEvent]:
+ yield TriggerEvent({})
+
+
+@conf_vars({("core", "fernet_key"): Fernet.generate_key().decode()})
+def test_serialize_sensitive_kwargs():
+ """
+ Tests that sensitive kwargs are encrypted.
+ """
+ trigger_instance = SensitiveKwargsTrigger(param1="value1", param2="value2")
+ trigger_row: Trigger = Trigger.from_object(trigger_instance)
+
+ assert trigger_row.kwargs["param1"] == "value1"
+ assert (
+
get_fernet().decrypt(trigger_row.kwargs["encrypted__param2"].encode("utf-8")).decode("utf-8")
+ == "value2"
+ )
Review Comment:
I'd propose to "abstract" the test:
1) ensure that if any dummy string is encrypted that something comes out
which smells like a string and is not empty
2) Take another dummy string, call encryption from the unit test, call
decryption again and check if the same like the input. Important is to NOT
re-implement the same logic but call encrypt+decrypt as "black box" in the unit
test. Such you test the round-trip function but you don't re-implement the same
logic
3) Maybe add a negative test, same like (2) but modify a char in the
encrypted intermediate or rotate the fernet key and check that NOT the same
dummy is returned or decrypt fails with expected error.
This way you don't re-implement the logic and keep the test abstract you you
test (logically) that the code still works.
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
