1fanwang opened a new issue, #70875: URL: https://github.com/apache/airflow/issues/70875
### Apache Airflow version 3.2.2 (apache-airflow-task-sdk 1.2.2). Also present on `main` at 79fa2e622218138f1638fe3a225156e254b3e748. ### If "Other Airflow 2/3 version" selected, which one? _No response_ ### What happened? The deprecation shim at `airflow.sdk.execution_time.secrets_masker` tells you to migrate to `airflow.sdk._shared.secrets_masker`. For `mask_secret` its own code deliberately does something else, and the two are not equivalent: `airflow.sdk.log.mask_secret` also forwards the secret to the supervisor, `airflow.sdk._shared.secrets_masker.mask_secret` does not. Following the warning silently drops supervisor masking. The warning says `_shared` ([L33-L37](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/task-sdk/src/airflow/sdk/execution_time/secrets_masker.py#L33-L37)): ```python warnings.warn( "Importing from 'airflow.sdk.execution_time.secrets_masker' is deprecated and will be " "removed in a future version. Please use 'airflow.sdk._shared.secrets_masker' instead.", DeprecatedImportWarning, stacklevel=2, ) ``` The code hands you `airflow.sdk.log` ([L43-L47](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/task-sdk/src/airflow/sdk/execution_time/secrets_masker.py#L43-L47)): ```python def __getattr__(name: str): if name == "mask_secret": from airflow.sdk.log import mask_secret return mask_secret ``` Confirmed by object identity on 3.2.2: ``` shim.mask_secret is airflow.sdk.log.mask_secret -> True shim.mask_secret is airflow.sdk._shared.secrets_masker.mask_secret -> False ``` That special case is correct, and it is load bearing. `airflow.sdk.log.mask_secret` registers with the shared masker **and** notifies the supervisor ([log.py L250-L266](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/task-sdk/src/airflow/sdk/log.py#L250-L266)): ```python def mask_secret(secret: JsonValue, name: str | None = None) -> None: """Mask a secret in both task process and supervisor process. ...""" _secrets_masker().add_mask(secret, name) with suppress(Exception): from airflow.sdk.execution_time import task_runner from airflow.sdk.execution_time.comms import MaskSecret if comms := getattr(task_runner, "SUPERVISOR_COMMS", None): comms.send(MaskSecret(value=secret, name=name)) ``` The `_shared` one the warning recommends stops at the first line ([secrets_masker.py L92-…](https://github.com/apache/airflow/blob/79fa2e622218138f1638fe3a225156e254b3e748/shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py#L92)); there is no `SUPERVISOR_COMMS` reference anywhere in that module. Under AIP-72 the supervisor is a separate process, so a secret registered only through `_shared` can still appear in supervisor log output. The failure is silent. Both modules import, both expose `mask_secret`, both accept the call and return `None`. A test asserting the symbol is importable and callable passes against either one, including the one that misses the supervisor. ### What you think should happen instead? The warning text and the module docstring should name `airflow.sdk.log` for `mask_secret`, matching what `__getattr__` already does. Either carve out the exception in the message, or drop the special case so the message becomes true. The shim came in with https://github.com/apache/airflow/pull/54915, added defensively after https://github.com/apache/airflow/pull/54449 removed the path, so this reads as incidental rather than intended. ### How to reproduce ```python import warnings warnings.simplefilter("ignore") import airflow.sdk.execution_time.secrets_masker as shim import airflow.sdk._shared.secrets_masker as shared import airflow.sdk.log as sdklog print(shim.mask_secret is sdklog.mask_secret) # True, what the shim gives you print(shim.mask_secret is shared.mask_secret) # False, what the warning tells you to use ``` ### Operating System macOS 15 (not OS specific) ### Versions of Apache Airflow Providers _No response_ ### Deployment Other ### Deployment details _No response_ ### Anything else? Two engineers migrating the same codebase independently read the deprecation message, moved to `airflow.sdk._shared.secrets_masker`, and ended up with weaker masking than they started with. Neither noticed, because every wrong answer in this area imports and runs fine. The human readable guidance is the only signal most people act on, and right now it points away from the implementation the shim itself chose. ### Are you willing to submit PR? - [X] Yes I am willing to submit a PR! ### Code of Conduct - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md) -- 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]
