amoghrajesh commented on code in PR #45931:
URL: https://github.com/apache/airflow/pull/45931#discussion_r2275843984
##########
airflow-core/src/airflow/configuration.py:
##########
@@ -2194,28 +2192,73 @@ def get_custom_secret_backend(worker_mode: bool =
False) -> BaseSecretsBackend |
return secrets_backend_cls(**backend_kwargs)
+def get_importable_secret_backend(class_name: str | None) ->
BaseSecretsBackend | None:
+ """Get secret backend defined in the given class name."""
+ if class_name is not None:
+ secrets_backend_cls = import_string(class_name)
+ return secrets_backend_cls()
+ return None
+
+
def initialize_secrets_backends(
- default_backends: list[str] = DEFAULT_SECRETS_SEARCH_PATH,
+ default_backends: list[str] | None = None,
) -> list[BaseSecretsBackend]:
"""
Initialize secrets backend.
* import secrets backend classes
* instantiate them and return them in a list
"""
- backend_list = []
worker_mode = False
- if default_backends != DEFAULT_SECRETS_SEARCH_PATH:
+ environment_variable_args: str | None = (
+ "airflow.secrets.environment_variables.EnvironmentVariablesBackend"
+ )
+ metastore_args: str | None = "airflow.secrets.metastore.MetastoreBackend"
+ if default_backends is not None:
worker_mode = True
+ environment_variable_args = (
+ environment_variable_args if environment_variable_args in
default_backends else None
+ )
+ metastore_args = metastore_args if metastore_args in default_backends
else None
+ backends_map: dict[str, dict[str, Any]] = {
+ "environment_variable": {
+ "callback": get_importable_secret_backend,
+ "args": (environment_variable_args,),
+ },
+ "metastore": {
+ "callback": get_importable_secret_backend,
+ "args": (metastore_args,),
+ },
+ "custom": {
+ "callback": get_custom_secret_backend,
+ "args": (worker_mode,),
+ },
+ }
- custom_secret_backend = get_custom_secret_backend(worker_mode)
+ backends_order = conf.getlist("secrets", "backends_order", delimiter=",")
- if custom_secret_backend is not None:
- backend_list.append(custom_secret_backend)
+ required_backends = ["metastore", "environment_variable"]
+ if missing_backends := [b for b in required_backends if b not in
backends_order]:
+ raise AirflowConfigException(
+ "The configuration option [secrets]backends_order is
misconfigured. "
+ "The following backend types are missing: %s",
+ missing_backends,
+ )
Review Comment:
This check would behave badly for workers?
Example:
```
required_backends = ["metastore", "environment_variable"]
backends_order = ["custom", "environment_variable"]
```
`required_backends` need not have `metastore` for workers.
##########
airflow-core/src/airflow/configuration.py:
##########
@@ -2138,7 +2137,7 @@ def make_group_other_inaccessible(file_path: str):
def ensure_secrets_loaded(
- default_backends: list[str] = DEFAULT_SECRETS_SEARCH_PATH,
+ default_backends: list[str] | None = None,
Review Comment:
The convention here now is:
- If None, its called from someplace other than worker
- If not None, its called from worker (for `[workers][secrets_backend]`
config)
Its not very intuitive, could we go back with the previous behaviour?
##########
airflow-core/docs/security/secrets/secrets-backend/index.rst:
##########
@@ -64,12 +66,21 @@ The ``[secrets]`` section has the following options:
[secrets]
backend =
backend_kwargs =
+ backends_order =
Set ``backend`` to the fully qualified class name of the backend you want to
enable.
You can provide ``backend_kwargs`` with json and it will be passed as kwargs
to the ``__init__`` method of
your secrets backend.
+``backends_order`` comma-separated list of secret backends. These backends
will be used in the order they are specified.
+Please note that the ``environment_variable`` and ``metastore`` are required
values and cannot be removed
+from the list. Supported values are:
+
+* ``custom``: Custom secret backend specified in the ``secrets[backend]``
configuration option.
+* ``environment_variable``: Standard environment variable backend
``airflow.secrets.environment_variables.EnvironmentVariablesBackend``.
+* ``metastore``: Standard metastore backend
``airflow.secrets.metastore.MetastoreBackend``.
Review Comment:
I also request you to have some user guidelines in the docs to configure it
properly.
For example:
Lets say DB lookup has higher precedence than that of say ENV backend.
This would be shooting ourselves in the foot by compromising the performance
here? DB lookup will be more expensive than DB.
##########
airflow-core/src/airflow/configuration.py:
##########
@@ -2194,28 +2192,73 @@ def get_custom_secret_backend(worker_mode: bool =
False) -> BaseSecretsBackend |
return secrets_backend_cls(**backend_kwargs)
+def get_importable_secret_backend(class_name: str | None) ->
BaseSecretsBackend | None:
+ """Get secret backend defined in the given class name."""
+ if class_name is not None:
+ secrets_backend_cls = import_string(class_name)
+ return secrets_backend_cls()
+ return None
+
+
def initialize_secrets_backends(
- default_backends: list[str] = DEFAULT_SECRETS_SEARCH_PATH,
+ default_backends: list[str] | None = None,
) -> list[BaseSecretsBackend]:
"""
Initialize secrets backend.
* import secrets backend classes
* instantiate them and return them in a list
"""
- backend_list = []
worker_mode = False
- if default_backends != DEFAULT_SECRETS_SEARCH_PATH:
+ environment_variable_args: str | None = (
+ "airflow.secrets.environment_variables.EnvironmentVariablesBackend"
+ )
+ metastore_args: str | None = "airflow.secrets.metastore.MetastoreBackend"
+ if default_backends is not None:
worker_mode = True
+ environment_variable_args = (
+ environment_variable_args if environment_variable_args in
default_backends else None
+ )
+ metastore_args = metastore_args if metastore_args in default_backends
else None
+ backends_map: dict[str, dict[str, Any]] = {
+ "environment_variable": {
+ "callback": get_importable_secret_backend,
+ "args": (environment_variable_args,),
+ },
+ "metastore": {
+ "callback": get_importable_secret_backend,
+ "args": (metastore_args,),
+ },
+ "custom": {
+ "callback": get_custom_secret_backend,
+ "args": (worker_mode,),
+ },
+ }
- custom_secret_backend = get_custom_secret_backend(worker_mode)
+ backends_order = conf.getlist("secrets", "backends_order", delimiter=",")
- if custom_secret_backend is not None:
- backend_list.append(custom_secret_backend)
+ required_backends = ["metastore", "environment_variable"]
+ if missing_backends := [b for b in required_backends if b not in
backends_order]:
Review Comment:
You could use a set here:
```
>>> req = {"a", "b"}
>>> actual = {"c", "a"}
>>> req.intersection(actual)
{'a'}
```
##########
airflow-core/docs/security/secrets/secrets-backend/index.rst:
##########
@@ -64,12 +66,21 @@ The ``[secrets]`` section has the following options:
[secrets]
backend =
backend_kwargs =
+ backends_order =
Set ``backend`` to the fully qualified class name of the backend you want to
enable.
You can provide ``backend_kwargs`` with json and it will be passed as kwargs
to the ``__init__`` method of
your secrets backend.
+``backends_order`` comma-separated list of secret backends. These backends
will be used in the order they are specified.
+Please note that the ``environment_variable`` and ``metastore`` are required
values and cannot be removed
Review Comment:
If not specified, do we plan to auto add them? And in which order?
--
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]
