You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "stephen-bracken (via GitHub)" <gi...@apache.org> on 2026/04/19 14:36:42 UTC
[PR] Add multi team secret support to VaultBackend [airflow]
stephen-bracken opened a new pull request, #65493:
URL: https://github.com/apache/airflow/pull/65493
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull request.
Write a good git commit message following this guide: http://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the documentation.
Feel free to ping (in general) for the review if you do not see reaction for a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
Add support for team scoped secrets to the VaultBackend secrets backend
---
related: #65372
If multi_team is enabled, the VaultBackend will now prefix each variable or connection id with the team name,
i.e. `{base_path}/{team_name}/{key}`.
If no key is found under the team specific secrets then a global path will be searched instead. This global path
will default to using the base path, but can also be prefixed e.g. `{base_path}/global/{key}` or `{base_path}/shared/{key}` etc.
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR, please
change below checkbox to `[X]` followed by the name of the tool, uncomment the "Generated-by".
-->
- [ ] Yes (please specify the tool below)
- [X] No
<!--
Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118898320
##########
providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py:
##########
@@ -68,6 +70,175 @@ def test_get_connection(self, mock_hvac):
connection = test_client.get_connection(conn_id="test_postgres")
assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"
+ @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ def test_get_connection_team_value(self, mock_hvac):
+ mock_client = mock.MagicMock()
+ mock_hvac.Client.return_value = mock_client
+ mock_client.secrets.kv.v2.read_secret_version.return_value = {
+ "request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
+ "lease_id": "",
+ "renewable": False,
+ "lease_duration": 0,
+ "data": {
+ "data": {
+ "conn_type": "postgresql",
+ "login": "airflow",
+ "password": "airflow",
+ "host": "host",
+ "port": "5432",
+ "schema": "airflow",
+ "extra": '{"foo":"bar","baz":"taz"}',
+ },
+ "metadata": {
+ "created_time": "2020-03-16T21:01:43.331126Z",
+ "deletion_time": "",
+ "destroyed": False,
+ "version": 1,
+ },
+ },
+ "wrap_info": None,
+ "warnings": None,
+ "auth": None,
+ }
+
+ kwargs = {
+ "connections_path": "connections",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ }
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ test_client = VaultBackend(**kwargs)
+ connection = test_client.get_connection(conn_id="test_postgres", team_name="foo")
+ mock_client.secrets.kv.v2.read_secret_version.assert_called_with(
+ path=test_client.connections_path + "/foo/test_postgres",
+ mount_point="airflow",
+ version=None,
+ raise_on_deleted_version=True,
+ )
+ assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"
+
+ @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ def test_get_connection_no_team_separation(self, mock_hvac):
+ mock_client = mock.MagicMock()
+ mock_hvac.Client.return_value = mock_client
+ mock_client.secrets.kv.v2.read_secret_version.return_value = {
+ "request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
+ "lease_id": "",
+ "renewable": False,
+ "lease_duration": 0,
+ "data": {
+ "data": {
+ "conn_type": "postgresql",
+ "login": "airflow",
+ "password": "airflow",
+ "host": "host",
+ "port": "5432",
+ "schema": "airflow",
+ "extra": '{"foo":"bar","baz":"taz"}',
+ },
+ "metadata": {
+ "created_time": "2020-03-16T21:01:43.331126Z",
+ "deletion_time": "",
+ "destroyed": False,
+ "version": 1,
+ },
+ },
+ "wrap_info": None,
+ "warnings": None,
+ "auth": None,
+ }
+
+ kwargs = {
+ "connections_path": "connections",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ "use_team_secrets_path": False,
+ }
+
+ with conf_vars({("core", "multi_team"): "True"}):
Review Comment:
Same
##########
providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py:
##########
@@ -68,6 +70,175 @@ def test_get_connection(self, mock_hvac):
connection = test_client.get_connection(conn_id="test_postgres")
assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"
+ @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ def test_get_connection_team_value(self, mock_hvac):
+ mock_client = mock.MagicMock()
+ mock_hvac.Client.return_value = mock_client
+ mock_client.secrets.kv.v2.read_secret_version.return_value = {
+ "request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
+ "lease_id": "",
+ "renewable": False,
+ "lease_duration": 0,
+ "data": {
+ "data": {
+ "conn_type": "postgresql",
+ "login": "airflow",
+ "password": "airflow",
+ "host": "host",
+ "port": "5432",
+ "schema": "airflow",
+ "extra": '{"foo":"bar","baz":"taz"}',
+ },
+ "metadata": {
+ "created_time": "2020-03-16T21:01:43.331126Z",
+ "deletion_time": "",
+ "destroyed": False,
+ "version": 1,
+ },
+ },
+ "wrap_info": None,
+ "warnings": None,
+ "auth": None,
+ }
+
+ kwargs = {
+ "connections_path": "connections",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ }
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ test_client = VaultBackend(**kwargs)
+ connection = test_client.get_connection(conn_id="test_postgres", team_name="foo")
+ mock_client.secrets.kv.v2.read_secret_version.assert_called_with(
+ path=test_client.connections_path + "/foo/test_postgres",
+ mount_point="airflow",
+ version=None,
+ raise_on_deleted_version=True,
+ )
+ assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"
+
+ @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ def test_get_connection_no_team_separation(self, mock_hvac):
Review Comment:
I am not sure this is possible but can you try to factorize these tests into one (one for connection, one for variable, etc) with `@pytest.mark.parametrize(`?
##########
providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py:
##########
@@ -68,6 +70,175 @@ def test_get_connection(self, mock_hvac):
connection = test_client.get_connection(conn_id="test_postgres")
assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"
+ @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ def test_get_connection_team_value(self, mock_hvac):
+ mock_client = mock.MagicMock()
+ mock_hvac.Client.return_value = mock_client
+ mock_client.secrets.kv.v2.read_secret_version.return_value = {
+ "request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
+ "lease_id": "",
+ "renewable": False,
+ "lease_duration": 0,
+ "data": {
+ "data": {
+ "conn_type": "postgresql",
+ "login": "airflow",
+ "password": "airflow",
+ "host": "host",
+ "port": "5432",
+ "schema": "airflow",
+ "extra": '{"foo":"bar","baz":"taz"}',
+ },
+ "metadata": {
+ "created_time": "2020-03-16T21:01:43.331126Z",
+ "deletion_time": "",
+ "destroyed": False,
+ "version": 1,
+ },
+ },
+ "wrap_info": None,
+ "warnings": None,
+ "auth": None,
+ }
+
+ kwargs = {
+ "connections_path": "connections",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ }
+
+ with conf_vars({("core", "multi_team"): "True"}):
Review Comment:
nit: please use decorator `@conf_vars(` instead, I find it easier and clearer to read
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "stephen-bracken (via GitHub)" <gi...@apache.org>.
stephen-bracken commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118830041
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -214,7 +249,10 @@ def get_connection(self, conn_id: str, team_name: str | None = None) -> Connecti
# problems when instantiating the backend during configuration
from airflow.models.connection import Connection
- response = self.get_response(conn_id)
+ if conf.getboolean("core", "multi_team", fallback=False):
+ response = self._get_team_or_global_secret(self.connections_path, team_name, conn_id)
Review Comment:
sure, I'll update this
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118753103
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -214,7 +249,10 @@ def get_connection(self, conn_id: str, team_name: str | None = None) -> Connecti
# problems when instantiating the backend during configuration
from airflow.models.connection import Connection
- response = self.get_response(conn_id)
+ if conf.getboolean("core", "multi_team", fallback=False):
+ response = self._get_team_or_global_secret(self.connections_path, team_name, conn_id)
Review Comment:
Why we cannot do `response = self._get_team_or_global_secret(self.connections_path, team_name, conn_id)` regardless of whether multi team is configured in the environment? The same way we do with variable?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "stephen-bracken (via GitHub)" <gi...@apache.org>.
stephen-bracken commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3117268609
##########
providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py:
##########
@@ -68,6 +70,175 @@ def test_get_connection(self, mock_hvac):
connection = test_client.get_connection(conn_id="test_postgres")
assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"
+ @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ def test_get_connection_team_value(self, mock_hvac):
+ mock_client = mock.MagicMock()
+ mock_hvac.Client.return_value = mock_client
+ mock_client.secrets.kv.v2.read_secret_version.return_value = {
+ "request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
+ "lease_id": "",
+ "renewable": False,
+ "lease_duration": 0,
+ "data": {
+ "data": {
+ "conn_type": "postgresql",
+ "login": "airflow",
+ "password": "airflow",
+ "host": "host",
+ "port": "5432",
+ "schema": "airflow",
+ "extra": '{"foo":"bar","baz":"taz"}',
+ },
+ "metadata": {
+ "created_time": "2020-03-16T21:01:43.331126Z",
+ "deletion_time": "",
+ "destroyed": False,
+ "version": 1,
+ },
+ },
+ "wrap_info": None,
+ "warnings": None,
+ "auth": None,
+ }
+
+ kwargs = {
+ "connections_path": "connections",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ }
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ test_client = VaultBackend(**kwargs)
+ connection = test_client.get_connection(conn_id="test_postgres", team_name="foo")
+ mock_client.secrets.kv.v2.read_secret_version.assert_called_with(
Review Comment:
If multi_team is disabled then team_name should never be passed
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "stephen-bracken (via GitHub)" <gi...@apache.org>.
stephen-bracken commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118715262
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -99,6 +104,8 @@ def __init__(
connections_path: str | None = "connections",
variables_path: str | None = "variables",
config_path: str | None = "config",
+ use_team_secrets_path: bool = True,
Review Comment:
sorry let me clarify, this could be set in the `[secrets] backend_kwargs` setting by _deployment managers_, who could then decide whether or not to use the team structured paths for secrets
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118550687
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -99,6 +104,8 @@ def __init__(
connections_path: str | None = "connections",
variables_path: str | None = "variables",
config_path: str | None = "config",
+ use_team_secrets_path: bool = True,
Review Comment:
Do we need it? In which case this parameter should be `False`?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 merged PR #65493:
URL: https://github.com/apache/airflow/pull/65493
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3115567526
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -189,6 +200,30 @@ def _get_secret_with_base(self, base_path: str | None, key: str) -> dict | None:
secret_path=(mount_point + "/" if mount_point else "") + secret_path
)
+ def _get_team_or_global_secret(self, base_path: str | None, team_name: str | None, key: str):
+ """
+ Get a secret from a team specific path or the global path.
+
+ If multi team is enabled, check {base_path}/{team_name}/{key}, then fallback to {base_path}/{global_path}/{key} or {base_path}/{key}.
+ """
+ if base_path is None:
+ return None
+ if (
+ conf.get("core", "multi_team", fallback=False)
+ and self.use_team_secrets_path
+ and team_name is not None
+ ):
+ response = self._get_secret_with_base(base_path + "/" + team_name, key)
+ if response is not None:
+ return response
+ # Fallback to global secret
+ if conf.get("core", "multi_team", fallback=False) and self.global_secrets_path is not None:
+ path = base_path + "/" + self.global_secrets_path
Review Comment:
When `connections_path`/`variables_path` is configured as an empty string (which the backend already treats specially in `_get_secret_with_base`), building `base_path + "/" + team_name` produces a leading slash (e.g. "/team") and changes the secret path format. Consider using `build_path(base_path, team_name)` (or handling `base_path == ""`) when constructing the team/global base paths to preserve the existing no-leading-slash behavior.
```suggestion
response = self._get_secret_with_base(self.build_path(base_path, team_name), key)
if response is not None:
return response
# Fallback to global secret
if conf.get("core", "multi_team", fallback=False) and self.global_secrets_path is not None:
path = self.build_path(base_path, self.global_secrets_path)
```
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -189,6 +200,30 @@ def _get_secret_with_base(self, base_path: str | None, key: str) -> dict | None:
secret_path=(mount_point + "/" if mount_point else "") + secret_path
)
+ def _get_team_or_global_secret(self, base_path: str | None, team_name: str | None, key: str):
+ """
+ Get a secret from a team specific path or the global path.
+
+ If multi team is enabled, check {base_path}/{team_name}/{key}, then fallback to {base_path}/{global_path}/{key} or {base_path}/{key}.
+ """
+ if base_path is None:
+ return None
+ if (
+ conf.get("core", "multi_team", fallback=False)
+ and self.use_team_secrets_path
+ and team_name is not None
+ ):
+ response = self._get_secret_with_base(base_path + "/" + team_name, key)
+ if response is not None:
+ return response
+ # Fallback to global secret
+ if conf.get("core", "multi_team", fallback=False) and self.global_secrets_path is not None:
Review Comment:
`multi_team` is a boolean config, but this code uses `conf.get(...)` in boolean contexts. If the config value is the string "False" (common when set via env/ini), it will still be truthy and incorrectly enable team/global lookup. Use `conf.getboolean("core", "multi_team", fallback=False)` for these checks (and avoid mixing different fallbacks in different places).
```suggestion
multi_team_enabled = conf.getboolean("core", "multi_team", fallback=False)
if multi_team_enabled and self.use_team_secrets_path and team_name is not None:
response = self._get_secret_with_base(base_path + "/" + team_name, key)
if response is not None:
return response
# Fallback to global secret
if multi_team_enabled and self.global_secrets_path is not None:
```
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -214,7 +249,10 @@ def get_connection(self, conn_id: str, team_name: str | None = None) -> Connecti
# problems when instantiating the backend during configuration
from airflow.models.connection import Connection
- response = self.get_response(conn_id)
+ if conf.get("core", "multi_team", fallback=True):
+ response = self._get_team_or_global_secret(self.connections_path, team_name, conn_id)
+ else:
+ response = self.get_response(conn_id)
Review Comment:
`get_connection()` currently uses `conf.get("core", "multi_team", fallback=True)` which (a) defaults to enabling the multi-team path even when unset, and (b) has the same truthy-string problem as above. This should be `conf.getboolean("core", "multi_team", fallback=False)` to match core/provider usage and ensure team scoping is only active when explicitly enabled.
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -52,6 +53,10 @@ class VaultBackend(BaseSecretsBackend, LoggingMixin):
(default: 'variables'). If set to None (null), requests for variables will not be sent to Vault.
:param config_path: Specifies the path of the secret to read Airflow Configurations
(default: 'config'). If set to None (null), requests for configurations will not be sent to Vault.
+ :param use_team_secrets_paths: Flag to enable team scoped secret retrieval from {base_path}/{team_name}/{key}
Review Comment:
Docstring parameter name looks inconsistent: it documents `use_team_secrets_paths` but the actual init kwarg is `use_team_secrets_path`. Align the docstring with the real argument name so users configuring `backend_kwargs` don’t get misled.
```suggestion
:param use_team_secrets_path: Flag to enable team scoped secret retrieval from {base_path}/{team_name}/{key}
```
##########
providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py:
##########
@@ -68,6 +70,175 @@ def test_get_connection(self, mock_hvac):
connection = test_client.get_connection(conn_id="test_postgres")
assert connection.get_uri() == "postgresql://airflow:airflow@host:5432/airflow?foo=bar&baz=taz"
+ @mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
+ def test_get_connection_team_value(self, mock_hvac):
+ mock_client = mock.MagicMock()
+ mock_hvac.Client.return_value = mock_client
+ mock_client.secrets.kv.v2.read_secret_version.return_value = {
+ "request_id": "94011e25-f8dc-ec29-221b-1f9c1d9ad2ae",
+ "lease_id": "",
+ "renewable": False,
+ "lease_duration": 0,
+ "data": {
+ "data": {
+ "conn_type": "postgresql",
+ "login": "airflow",
+ "password": "airflow",
+ "host": "host",
+ "port": "5432",
+ "schema": "airflow",
+ "extra": '{"foo":"bar","baz":"taz"}',
+ },
+ "metadata": {
+ "created_time": "2020-03-16T21:01:43.331126Z",
+ "deletion_time": "",
+ "destroyed": False,
+ "version": 1,
+ },
+ },
+ "wrap_info": None,
+ "warnings": None,
+ "auth": None,
+ }
+
+ kwargs = {
+ "connections_path": "connections",
+ "mount_point": "airflow",
+ "auth_type": "token",
+ "url": "http://127.0.0.1:8200",
+ "token": "s.7AU0I51yv1Q1lxOIg1F3ZRAS",
+ }
+
+ with conf_vars({("core", "multi_team"): "True"}):
+ test_client = VaultBackend(**kwargs)
+ connection = test_client.get_connection(conn_id="test_postgres", team_name="foo")
+ mock_client.secrets.kv.v2.read_secret_version.assert_called_with(
Review Comment:
The new multi-team tests cover the enabled (`multi_team=True`) cases, but there’s no regression test ensuring that when `core.multi_team` is disabled and a `team_name` is passed, VaultBackend does *not* add the team prefix. Adding that case would prevent future regressions (and would have caught the `conf.get(...)` truthy-string issue).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #65493:
URL: https://github.com/apache/airflow/pull/65493#issuecomment-4291411980
Tests are failing
`FAILED providers/hashicorp/tests/unit/hashicorp/secrets/test_vault.py::TestVaultSecrets::test_get_variable_value_multi_team[global_var_no_team] - AssertionError: assert equals failed
`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "stephen-bracken (via GitHub)" <gi...@apache.org>.
stephen-bracken commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118665449
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -99,6 +104,8 @@ def __init__(
connections_path: str | None = "connections",
variables_path: str | None = "variables",
config_path: str | None = "config",
+ use_team_secrets_path: bool = True,
Review Comment:
`use_team_secrets_path` only gets used if `multi_team` is enabled. The default for multi team should be to use team separated paths imo, but I added this flag to allow users to preserve the old path behaviour if necessary.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #65493:
URL: https://github.com/apache/airflow/pull/65493#issuecomment-4293970256
Awesome work, congrats on your first merged pull request! You are invited to check our [Issue Tracker](https://github.com/apache/airflow/issues) for additional contributions.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118729020
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -99,6 +104,8 @@ def __init__(
connections_path: str | None = "connections",
variables_path: str | None = "variables",
config_path: str | None = "config",
+ use_team_secrets_path: bool = True,
Review Comment:
Oh I see, alright, that makes sense
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add multi team secret support to VaultBackend [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on code in PR #65493:
URL: https://github.com/apache/airflow/pull/65493#discussion_r3118697752
##########
providers/hashicorp/src/airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -99,6 +104,8 @@ def __init__(
connections_path: str | None = "connections",
variables_path: str | None = "variables",
config_path: str | None = "config",
+ use_team_secrets_path: bool = True,
Review Comment:
As a user, how can you set this parameter to `False`?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org