ashb commented on code in PR #67868:
URL: https://github.com/apache/airflow/pull/67868#discussion_r3395225652
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/trigger.py:
##########
@@ -24,14 +24,29 @@
from airflow.api_fastapi.core_api.base import BaseModel
+def _redact_kwargs(_: object) -> str:
Review Comment:
Nit: While this technical is redaction, Can we call this as
```suggestion
def _remove_kwargs(_: object) -> str:
```
instead? As it doesn't follow ever other use of redaction in Airflow.
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/trigger.py:
##########
@@ -24,14 +24,29 @@
from airflow.api_fastapi.core_api.base import BaseModel
+def _redact_kwargs(_: object) -> str:
+ """
+ Return empty trigger kwargs for API responses.
+
+ Trigger ``kwargs`` may contain sensitive values (for example credentials a
deferred
+ operator hands to its trigger -- an API key, a token), so they are never
exposed through
+ the REST API. The field is kept in the response schema for backwards
compatibility -- so
+ existing API consumers do not break on a missing property -- but it is
always returned
+ empty, as ``"{}"`` (the stringified empty dict, matching the string format
the field has
+ always used). The triggerer still decrypts and uses the real kwargs at
runtime; only the
+ API representation is emptied.
+ """
+ return "{}"
+
+
class TriggerResponse(BaseModel):
"""Trigger serializer for responses."""
model_config = ConfigDict(populate_by_name=True)
id: int
classpath: str
- kwargs: Annotated[str, BeforeValidator(str)]
+ kwargs: Annotated[str, BeforeValidator(_redact_kwargs)]
Review Comment:
The other thing we could do is this:
```suggestion
kwargs: Annotated[str, BeforeValidator(_redact_kwargs)]
@computed_field
@property
@deprecated
def kwargs(self) -> str:
# Trigger ``kwargs`` may contain sensitive values (for example
credentials a deferred
# operator hands to its trigger -- an API key, a token), so they are
never exposed through
# the REST API. The field is kept in the response schema for backwards
compatibility -- so
# existing API consumers do not break on a missing property -- but it is
always returned
# empty, as ``"{}"`` (the stringified empty dict, matching the string
format the field has
# always used). The triggerer still decrypts and uses the real kwargs at
runtime; only the
# API representation is emptied.
return "{}"
```
(comment, not doc string, so that string doesn't appear in the API docs. Up
to you if you think it's useful to include something/the full thing for API
docs.
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/trigger.py:
##########
@@ -24,14 +24,29 @@
from airflow.api_fastapi.core_api.base import BaseModel
+def _redact_kwargs(_: object) -> str:
+ """
+ Return empty trigger kwargs for API responses.
+
+ Trigger ``kwargs`` may contain sensitive values (for example credentials a
deferred
+ operator hands to its trigger -- an API key, a token), so they are never
exposed through
+ the REST API. The field is kept in the response schema for backwards
compatibility -- so
+ existing API consumers do not break on a missing property -- but it is
always returned
+ empty, as ``"{}"`` (the stringified empty dict, matching the string format
the field has
+ always used). The triggerer still decrypts and uses the real kwargs at
runtime; only the
+ API representation is emptied.
+ """
+ return "{}"
+
+
class TriggerResponse(BaseModel):
"""Trigger serializer for responses."""
model_config = ConfigDict(populate_by_name=True)
id: int
classpath: str
- kwargs: Annotated[str, BeforeValidator(str)]
+ kwargs: Annotated[str, BeforeValidator(_redact_kwargs)]
Review Comment:
Lets mark this field as deprecated
https://pydantic.dev/docs/validation/latest/concepts/fields/#deprecated-fields
- and make sure that we don't issue a warning from our own code.
--
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]
