You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "hussein-awala (via GitHub)" <gi...@apache.org> on 2023/03/03 22:04:55 UTC
[GitHub] [airflow] hussein-awala opened a new pull request, #29908: Fix empty paths in Vault secrets backend
hussein-awala opened a new pull request, #29908:
URL: https://github.com/apache/airflow/pull/29908
Currently when we provide an empty string for `connections_path`, `variables_path` and `config_path`, Vault secrets backend tries to read the key `/secret_key` instead of `secret_key`. This PR handle the case when those paths are empty string.
### Why this is necessary?
#29734 added support for multiple mount_point, and these mount points can have different paths for variables, config and secrets, so instead of supporting multiple paths, providing the full secret path with empty string for `connections_path`, `variables_path` and `config_path` can solve the problem.
--
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
[GitHub] [airflow] hussein-awala commented on a diff in pull request #29908: Fix empty paths in Vault secrets backend
Posted by "hussein-awala (via GitHub)" <gi...@apache.org>.
hussein-awala commented on code in PR #29908:
URL: https://github.com/apache/airflow/pull/29908#discussion_r1125562097
##########
airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -175,8 +175,10 @@ def get_response(self, conn_id: str) -> dict | None:
mount_point, conn_key = self._parse_path(conn_id)
if self.connections_path is None or conn_key is None:
return None
-
- secret_path = self.build_path(self.connections_path, conn_key)
+ if self.connections_path == "":
+ secret_path = conn_key
Review Comment:
If we want to use Vault secrets backend only for connections, we can provide `None` for `variables_path` and `config_path` to return a quick `None`, then Airflow will check in the other backend secrets (Metadata then environment variables).
So I kept `None` here to deactivate some functionalities in the secrets backend.
--
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
[GitHub] [airflow] eladkal commented on a diff in pull request #29908: Fix empty paths in Vault secrets backend
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on code in PR #29908:
URL: https://github.com/apache/airflow/pull/29908#discussion_r1125561432
##########
airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -175,8 +175,10 @@ def get_response(self, conn_id: str) -> dict | None:
mount_point, conn_key = self._parse_path(conn_id)
if self.connections_path is None or conn_key is None:
return None
-
- secret_path = self.build_path(self.connections_path, conn_key)
+ if self.connections_path == "":
+ secret_path = conn_key
Review Comment:
why do we consider `None` and `""` to be dealt differently?
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #29908: Fix empty paths in Vault secrets backend
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #29908:
URL: https://github.com/apache/airflow/pull/29908#discussion_r1125563374
##########
airflow/providers/hashicorp/secrets/vault.py:
##########
@@ -175,8 +175,10 @@ def get_response(self, conn_id: str) -> dict | None:
mount_point, conn_key = self._parse_path(conn_id)
if self.connections_path is None or conn_key is None:
return None
-
- secret_path = self.build_path(self.connections_path, conn_key)
+ if self.connections_path == "":
+ secret_path = conn_key
Review Comment:
I believe this is because None can really only happens if there is no "/" in path and mount_point is not defined. Which is a basic requirement for vault - any time you expect something from vault you need to specify the mount_point and "/" is a way how it can be specified directly in the path.
--
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
[GitHub] [airflow] potiuk merged pull request #29908: Fix empty paths in Vault secrets backend
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #29908:
URL: https://github.com/apache/airflow/pull/29908
--
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