spcvalente commented on code in PR #74591:
URL: https://github.com/apache/airflow/pull/74591#discussion_r4249089318
##########
providers/snowflake/docs/connections/snowflake.rst:
##########
@@ -77,6 +77,10 @@ Extra (optional)
* ``token_file_path``: Path to a file holding the OIDC ID token used when
``workload_identity_provider``
is ``OIDC``. The connector reads the token from this file, which suits
projected or rotated tokens
(for example a Kubernetes service-account token).
+ * ``workload_identity_impersonation_path``: Identities to impersonate, in
order, before authenticating with
Review Comment:
Kept the name on purpose: it's the connector's own parameter
(`workload_identity_impersonation_path`, snowflake-connector-python 4.0.0),
same as the other WIF extras (`workload_identity_provider`, `token`,
`token_file_path`), so it maps 1:1 to Snowflake's docs. `_chain` is the Google
provider's naming. Agree `_path` reads like a file though, so the docs now say
it's a chain of identities, not a file path.
---
Drafted-by: Claude Code (Opus 5.5) (no human review before posting)
##########
providers/snowflake/tests/unit/snowflake/hooks/test_snowflake.py:
##########
@@ -1916,6 +1916,28 @@ def
test_get_conn_params_omits_oidc_token_when_unset(self):
assert "token" not in conn_params
assert "token_file_path" not in conn_params
+ @pytest.mark.parametrize(
+ "impersonation_path",
+ [
+ ["arn:aws:iam::123456789012:role/a",
"arn:aws:iam::123456789012:role/b"],
+ "arn:aws:iam::123456789012:role/a,
arn:aws:iam::123456789012:role/b",
Review Comment:
Cleaned up: the ARNs live in one constant, the comma-separated case is built
from it, and the assert is a single line.
---
Drafted-by: Claude Code (Opus 5.5) (no human review before posting)
##########
providers/snowflake/src/airflow/providers/snowflake/hooks/snowflake.py:
##########
@@ -572,6 +572,13 @@ def _add_workload_identity_config(
if token_file_path:
conn_config["token_file_path"] = token_file_path
+ impersonation_path = self._get_field(extra_dict,
"workload_identity_impersonation_path")
Review Comment:
Yes. Running the new test against main's hook fails with `KeyError:
'workload_identity_impersonation_path'`: `_get_conn_params` builds
`conn_config` from known fields only, so unknown extras never reach
`snowflake.connector.connect`.
---
Drafted-by: Claude Code (Opus 5.5) (no human review before posting)
--
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]
