jason810496 commented on code in PR #59873:
URL: https://github.com/apache/airflow/pull/59873#discussion_r2653086221


##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/config.py:
##########
@@ -25,6 +31,22 @@ class ConfigOption(StrictBaseModel):
     key: str
     value: str | tuple[str, str]
 
+    @model_validator(mode="after")
+    def redact_value(self) -> Self:
+        if self.value is None:
+            return self
+        if isinstance(self.value, tuple):
+            return self
+        try:
+            value_dict = json.loads(self.value)
+            redacted_dict = redact(value_dict, max_depth=1)
+            self.value = json.dumps(redacted_dict)
+            return self
+        except json.JSONDecodeError:
+            # value is not a serialized string representation of a dict.
+            self.value = str(redact(self.value, self.key))
+            return self

Review Comment:
   It seems we don't need to handle redact as Variable does.
   Since for value of config, it will be str, int, or bool: 
https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html 
instead of dict (json).
   
   Additionally, `redact` can handle `str` and `tuple` well.
   
https://github.com/apache/airflow/blob/cf80ae19840f1d03e16323e7bca819550633db97/shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py#L46
   
   So maybe we could take `redact_password` as example for this case.
   
   
https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py#L46-L51



##########
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_connections.py:
##########
@@ -37,6 +45,41 @@ def test_hook_meta_data(self, test_client):
             if hook_data["connection_type"] == "fs":
                 assert hook_data["hook_name"] == "File (path)"
 
+    @pytest.mark.parametrize(
+        ("extra_fields", "expected_response"),
+        [
+            ({"secret_key": "test-secret_key"}, {"secret_key": "***"}),
+            ({"extra_fields": "test-extra_fields"}, {"extra_fields": 
"test-extra_fields"}),

Review Comment:
   Does it mean that redact for `extra_fields` is not working?



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