kaxil opened a new pull request, #68302:
URL: https://github.com/apache/airflow/pull/68302

   Airflow 3.2 (AIP-67 multi-team) added a `team_name` keyword to the secrets 
backend API. `BaseSecretsBackend.get_connection()` forwarded `team_name` to 
`get_conn_value()` unconditionally, and 
`Connection.get_connection_from_secrets()` / 
`Variable.get_variable_from_secrets()` passed it straight to the backend's 
`get_connection()` / `get_variable()`.
   
   Any secrets backend whose override predates the `team_name` keyword (the old 
`(self, conn_id)` / `(self, key)` signature) raises, on every connection and 
variable lookup:
   
   ```
   TypeError: get_conn_value() got an unexpected keyword argument 'team_name'
   ```
   
   The backend-iteration loop catches this in its broad `except Exception` and 
treats it as "secret not found", so the real cause never surfaces. Users 
instead see `AirflowNotFoundException: The conn_id '<id>' isn't defined`, or a 
downstream auth failure when a connection silently isn't applied (for example 
boto3 falling back to an instance profile and getting `403`). It presents as 
"all my connections and variables disappeared after upgrading to 3.2."
   
   This breaks two groups:
   
   - Bundled provider backends pinned below the version that added `team_name`: 
Amazon `SecretsManagerBackend` / `SystemsManagerParameterStoreBackend`, Google 
`CloudSecretManagerBackend`, Azure `AzureKeyVaultBackend`, cncf-kubernetes 
`KubernetesSecretsBackend`, Hashicorp `VaultBackend`, Yandex 
`LockboxSecretBackend`.
   - Custom and third-party secrets backends with the pre-3.2 signature, which 
cannot be fixed by a provider bump.
   
   Introduced in 3.2.0 by the multi-team work (apache/airflow#59476 for 
connections, apache/airflow#58905 for variables), which updated the bundled 
providers in lockstep. Providers that version independently, and custom 
backends, were not (and cannot be) updated in lockstep.
   
   ## Fix
   
   `BaseSecretsBackend` now introspects whether an override accepts `team_name` 
(cached per backend method) and forwards it only when the override accepts it, 
or has a `**kwargs` catch-all; otherwise it calls the legacy signature. This is 
applied in:
   
   - [`get_connection`'s forward to 
`get_conn_value`](https://github.com/apache/airflow/blob/main/shared/secrets_backend/src/airflow_shared/secrets_backend/base.py)
 -- covers the common case across every call path (core models, worker, 
supervisor), all of which reach the override through the base method.
   - The core call sites 
[`Connection.get_connection_from_secrets`](https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/connection.py)
 and 
[`Variable.get_variable_from_secrets`](https://github.com/apache/airflow/blob/main/airflow-core/src/airflow/models/variable.py),
 which pass `team_name` explicitly. This covers backends that override 
`get_connection` / `get_variable` directly (for example Vault), and the 
variable path generally.
   
   ## Design rationale
   
   - **Signature introspection over a blanket `try/except TypeError` retry.** 
Retrying without `team_name` on any `TypeError` would also mask a genuine 
`TypeError` raised inside a backend (a real bug), re-running it as if it were a 
signature mismatch. Introspecting the signature targets exactly the 
legacy-signature case.
   - **Cached per backend method.** `inspect.signature` runs once per backend 
method; the cache key space is the small, fixed set of secrets-backend classes 
loaded in a process.
   - **`**kwargs` counts as accepting `team_name`.** Backends that forward 
`**kwargs` already handle the extra keyword.
   
   Backward compatibility for custom secrets backends is the constraint: a core 
minor upgrade should not require every third-party backend to add a parameter 
in lockstep.
   
   ## Gotchas
   
   - Backends that override `get_connection` / `get_variable` directly (rather 
than `get_conn_value`) are handled at the model call sites, not in the base 
class, because such overrides bypass the base method entirely. Both override 
styles are covered.
   


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