You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "amoghrajesh (via GitHub)" <gi...@apache.org> on 2025/03/19 08:11:12 UTC
[PR] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
amoghrajesh opened a new pull request, #47946:
URL: https://github.com/apache/airflow/pull/47946
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! 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 committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Pure Draft as of now, just dumping some things here that I want to reorder/make better
Setup:
```
export AIRFLOW__SECRETS__BACKEND="airflow.sdk.execution_time.apiserver_secrets_backend.APIServerSecretsBackend"
export AIRFLOW__WORKERS__BACKEND="airflow.secrets.local_filesystem.LocalFilesystemBackend"
export AIRFLOW__WORKERS__BACKEND_KWARGS='{"connections_file_path": "/files/conn.json"}'
```
Config:
```
root@cf562ecaa129:/opt/airflow# airflow config get-value secrets backend
airflow.sdk.execution_time.apiserver_secrets_backend.APIServerSecretsBackend
root@cf562ecaa129:/opt/airflow# airflow config get-value workers backend
airflow.secrets.local_filesystem.LocalFilesystemBackend
root@cf562ecaa129:/opt/airflow#
```
DAG:
```
from __future__ import annotations
from airflow.models.baseoperator import BaseOperator
from airflow import DAG
from airflow.sdk.definitions.connection import Connection
import attrs
class CustomOperator(BaseOperator):
def execute(self, context):
c = Connection.get_connection_from_secrets(conn_id="CONN_A")
print("The connection is", c)
print("Connection attributes are", vars(c))
with DAG("example_get_connection_from_secrets", schedule=None, catchup=False) as dag:
CustomOperator(task_id="print_conn")
```
Using conn.json for `LocalFilesystemBackend`
```
{
"CONN_A": "mysql://host_a",
"CONN_B": {
"conn_type": "scheme",
"host": "host",
"schema": "schema",
"login": "Login",
"password": "None",
"port": "1234"
}
}
```
<img width="1724" alt="image" src="https://github.com/user-attachments/assets/6c8db36d-66a7-4dc1-9bb6-fc171651215d" />
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003167107
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -87,6 +88,35 @@
return _get_connection(conn_id)
+ @classmethod
+ def get_connection_from_secrets(cls, conn_id: str) -> Connection:
+ """
+ Get connection by conn_id.
+
+ :param conn_id: connection id
+ :return: connection
+ """
+ # TODO: check cache first
+ # enabled only if SecretCache.init() has been called first
+
+ # iterate over configured backends if not in cache (or expired)
+ from airflow.sdk.execution_time.supervisor import SECRETS_BACKEND
+
+ for secrets_backend in SECRETS_BACKEND:
+ print("The backend in the sdk is", secrets_backend)
Review Comment:
Ah debug log forgotten and pushed!
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2005313730
##########
airflow/config_templates/config.yml:
##########
@@ -1416,6 +1416,29 @@ api:
version_added: 2.7.0
example: ~
default: "False"
+workers:
+ description: Configuration related to workers that run Airflow tasks.
+ options:
+ secrets_backend:
+ description: |
+ Full class name of secrets backend to enable for workers (will precede env vars backend)
+ version_added: 3.0.0
+ type: string
+ example: "airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend"
+ default: ""
+ secrets_backend_kwargs:
+ description: |
+ The secrets_backend_kwargs param is loaded into a dictionary and passed to ``__init__``
+ of secrets backend class. See documentation for the secrets backend you are using.
+ JSON is expected.
+
+ Example for AWS Systems Manager ParameterStore:
+ ``{"connections_prefix": "/airflow/connections", "profile_name": "default"}``
+ version_added: 3.0.0
+ type: string
+ sensitive: true
+ example: ~
+ default: ""
Review Comment:
Wdyt?
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2005512691
##########
airflow/config_templates/config.yml:
##########
@@ -1416,6 +1416,29 @@ api:
version_added: 2.7.0
example: ~
default: "False"
+workers:
+ description: Configuration related to workers that run Airflow tasks.
+ options:
+ secrets_backend:
+ description: |
+ Full class name of secrets backend to enable for workers (will precede env vars backend)
+ version_added: 3.0.0
+ type: string
+ example: "airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend"
+ default: ""
+ secrets_backend_kwargs:
+ description: |
+ The secrets_backend_kwargs param is loaded into a dictionary and passed to ``__init__``
+ of secrets backend class. See documentation for the secrets backend you are using.
+ JSON is expected.
+
+ Example for AWS Systems Manager ParameterStore:
+ ``{"connections_prefix": "/airflow/connections", "profile_name": "default"}``
+ version_added: 3.0.0
+ type: string
+ sensitive: true
+ example: ~
+ default: ""
Review Comment:
I have handled in this way and published testing results below!
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003035254
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -24,7 +24,8 @@
import attrs
-from airflow.exceptions import AirflowException
+from airflow.exceptions import AirflowException, AirflowNotFoundException
+from airflow.sdk.api.datamodels._generated import ConnectionResponse
Review Comment:
Since this is a user facing class, let's avoid importing `ConnectionResponse` and defining `_convert_connection_to_response` here if at all possible.
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2005304733
##########
airflow/config_templates/config.yml:
##########
@@ -1416,6 +1416,29 @@ api:
version_added: 2.7.0
example: ~
default: "False"
+workers:
+ description: Configuration related to workers that run Airflow tasks.
+ options:
+ secrets_backend:
+ description: |
+ Full class name of secrets backend to enable for workers (will precede env vars backend)
+ version_added: 3.0.0
+ type: string
+ example: "airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend"
+ default: ""
+ secrets_backend_kwargs:
+ description: |
+ The secrets_backend_kwargs param is loaded into a dictionary and passed to ``__init__``
+ of secrets backend class. See documentation for the secrets backend you are using.
+ JSON is expected.
+
+ Example for AWS Systems Manager ParameterStore:
+ ``{"connections_prefix": "/airflow/connections", "profile_name": "default"}``
+ version_added: 3.0.0
+ type: string
+ sensitive: true
+ example: ~
+ default: ""
Review Comment:
I am not sold on requiring a separate config.
We can re-use the same config on both workers and api-server. As that configuration just tells us how to connect to a secrets backend.
If users want to have separate secrets backend on API-server and Workers, they can have different values set for the same config.
E.g.
Worker has
```env
AIRFLOW__SECRETS__BACKEND=airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend
```
and API-server has:
```env
AIRFLOW__SECRETS__BACKEND=airflow.providers.hashicorp.secrets.vault.VaultBackend
```
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh merged PR #47946:
URL: https://github.com/apache/airflow/pull/47946
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2005350747
##########
airflow/config_templates/config.yml:
##########
@@ -1416,6 +1416,29 @@ api:
version_added: 2.7.0
example: ~
default: "False"
+workers:
+ description: Configuration related to workers that run Airflow tasks.
+ options:
+ secrets_backend:
+ description: |
+ Full class name of secrets backend to enable for workers (will precede env vars backend)
+ version_added: 3.0.0
+ type: string
+ example: "airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend"
+ default: ""
+ secrets_backend_kwargs:
+ description: |
+ The secrets_backend_kwargs param is loaded into a dictionary and passed to ``__init__``
+ of secrets backend class. See documentation for the secrets backend you are using.
+ JSON is expected.
+
+ Example for AWS Systems Manager ParameterStore:
+ ``{"connections_prefix": "/airflow/connections", "profile_name": "default"}``
+ version_added: 3.0.0
+ type: string
+ sensitive: true
+ example: ~
+ default: ""
Review Comment:
I think the runtime behaviour should be something like
```
conf.get("worker", "secrets_backend", fallback=None) or conf.get("secrets", "backend")
```
That way you _can_ have one uniform config on all nodes but control the behaviour on workers vs api servers 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
Re: [PR] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on PR #47946:
URL: https://github.com/apache/airflow/pull/47946#issuecomment-2736175462
Just linked the original issue to this PR too: https://github.com/apache/airflow/issues/45435
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003054484
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -123,6 +126,8 @@
TerminalTIState.SUCCESS,
]
+SECRETS_BACKEND: Any
Review Comment:
Shouldn't be any!
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #47946:
URL: https://github.com/apache/airflow/pull/47946#issuecomment-2737004120
Whats remaining in terms of testing?
1. Priority order of API server
2. Context
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2005071392
##########
tests/core/test_configuration.py:
##########
@@ -1586,12 +1625,6 @@ def test_sensitive_values():
("sentry", "sentry_dsn"),
("database", "sql_alchemy_engine_args"),
("core", "sql_alchemy_conn"),
- ("celery_broker_transport_options", "sentinel_kwargs"),
- ("celery", "broker_url"),
- ("celery", "flower_basic_auth"),
- ("celery", "result_backend"),
- ("opensearch", "username"),
- ("opensearch", "password"),
Review Comment:
Removed by mistake!
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003443243
##########
task-sdk/src/airflow/sdk/definitions/variable.py:
##########
@@ -52,3 +57,38 @@ def get(cls, key: str, default: Any = NOTSET, deserialize_json: bool = False):
if e.error.error == ErrorType.VARIABLE_NOT_FOUND and default is not NOTSET:
return default
raise
+
+ @classmethod
+ def get_variable_from_secrets(cls, key: str):
+ """
+ Get Airflow Variable by iterating over all Secret Backends.
+
+ :param key: Variable Key
+ :return: Variable Value
+ """
+ # TODO: check cache first
+ # enabled only if SecretCache.init() has been called first
+ from airflow.sdk.execution_time.supervisor import SECRETS_BACKEND
+
+ var_val = NOTSET
+ # iterate over backends if not in cache (or expired)
+ for secrets_backend in SECRETS_BACKEND:
+ try:
+ var_val = secrets_backend.get_variable(key=key)
+ if var_val is not NOTSET:
+ return var_val
+ except Exception:
+ log.exception(
+ "Unable to retrieve variable from secrets backend (%s). "
+ "Checking subsequent secrets backend.",
+ type(secrets_backend).__name__,
+ )
+
+ if var_val is not NOTSET:
+ raise AirflowNotFoundException(f"The variable with key `{key}` isn't defined")
+
+ return var_val
+
+ @staticmethod
+ def _convert_variable_to_response(key: str, value: str | None) -> VariableResponse:
+ return VariableResponse(key=key, value=value)
Review Comment:
I am taking this path ahead and proceeding
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #47946:
URL: https://github.com/apache/airflow/pull/47946#issuecomment-2740266522
To try that hybrid scenario:
Do not set workers secrets backend config, but set for secrets backend:
```
root@2480ed297606:/opt/airflow# airflow config get-value workers secrets_backend
root@2480ed297606:/opt/airflow# airflow config get-value secrets backend
airflow.secrets.local_filesystem.LocalFilesystemBackend
root@2480ed297606:/opt/airflow# airflow config get-value secrets backend_kwargs
{"connections_file_path": "/files/conn.json", "variables_file_path": "/files/var.json"}
```
conn.json btw:
```
{
"CONN_API_SERVER_A": "mysql://host_a"
}
```
Now try this dag:
```
from __future__ import annotations
from airflow.models.baseoperator import BaseOperator
from airflow import DAG
from airflow.sdk.definitions.connection import Connection
class CustomOperator(BaseOperator):
def execute(self, context):
c = Connection.get(conn_id="CONN_API_SERVER_A")
print("The connection is", c)
# print("Connection attributes are", vars(c))
with DAG("example_get_connection_from_secrets", schedule=None, catchup=False, tags=["secrets_backend"]) as dag:
CustomOperator(task_id="print_conn")
```
<img width="1715" alt="image" src="https://github.com/user-attachments/assets/9a1e435f-a4d8-41d0-acdb-226680614f50" />
The above test shows that we get the config as fallback from secrets backend of API server and function if not provided for workers.
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003275521
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -24,7 +24,8 @@
import attrs
-from airflow.exceptions import AirflowException
+from airflow.exceptions import AirflowException, AirflowNotFoundException
+from airflow.sdk.api.datamodels._generated import ConnectionResponse
Review Comment:
I moved those utils to supervisor for the time being, LMK what you think
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003200618
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -123,6 +126,8 @@
TerminalTIState.SUCCESS,
]
+SECRETS_BACKEND: Any
Review Comment:
Right yeah, set it to the correct type
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #47946:
URL: https://github.com/apache/airflow/pull/47946#issuecomment-2735777714
Much more readier now, might need to do code cleanup and ofc addition of tests
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003259215
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -24,7 +24,8 @@
import attrs
-from airflow.exceptions import AirflowException
+from airflow.exceptions import AirflowException, AirflowNotFoundException
+from airflow.sdk.api.datamodels._generated import ConnectionResponse
Review Comment:
But that doesn't feel too right too.
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003071677
##########
task-sdk/src/airflow/sdk/definitions/variable.py:
##########
@@ -52,3 +57,38 @@ def get(cls, key: str, default: Any = NOTSET, deserialize_json: bool = False):
if e.error.error == ErrorType.VARIABLE_NOT_FOUND and default is not NOTSET:
return default
raise
+
+ @classmethod
+ def get_variable_from_secrets(cls, key: str):
+ """
+ Get Airflow Variable by iterating over all Secret Backends.
+
+ :param key: Variable Key
+ :return: Variable Value
+ """
+ # TODO: check cache first
+ # enabled only if SecretCache.init() has been called first
+ from airflow.sdk.execution_time.supervisor import SECRETS_BACKEND
+
+ var_val = NOTSET
+ # iterate over backends if not in cache (or expired)
+ for secrets_backend in SECRETS_BACKEND:
+ try:
+ var_val = secrets_backend.get_variable(key=key)
+ if var_val is not NOTSET:
+ return var_val
+ except Exception:
+ log.exception(
+ "Unable to retrieve variable from secrets backend (%s). "
+ "Checking subsequent secrets backend.",
+ type(secrets_backend).__name__,
+ )
+
+ if var_val is not NOTSET:
+ raise AirflowNotFoundException(f"The variable with key `{key}` isn't defined")
+
+ return var_val
+
+ @staticmethod
+ def _convert_variable_to_response(key: str, value: str | None) -> VariableResponse:
+ return VariableResponse(key=key, value=value)
Review Comment:
One option is to move this entire thing including the sequencing in supervisor.py to `airflow.sdk.execution_time.context. _get_variable` function -- so it becomes a single way to get variable internally.
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003035254
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -24,7 +24,8 @@
import attrs
-from airflow.exceptions import AirflowException
+from airflow.exceptions import AirflowException, AirflowNotFoundException
+from airflow.sdk.api.datamodels._generated import ConnectionResponse
Review Comment:
Since this is a user facing class, let's avoid importing `ConnectionResponse` and defining `_convert_connection_to_response` here.
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003029080
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -87,6 +88,35 @@
return _get_connection(conn_id)
+ @classmethod
+ def get_connection_from_secrets(cls, conn_id: str) -> Connection:
+ """
+ Get connection by conn_id.
+
+ :param conn_id: connection id
+ :return: connection
+ """
+ # TODO: check cache first
+ # enabled only if SecretCache.init() has been called first
+
+ # iterate over configured backends if not in cache (or expired)
+ from airflow.sdk.execution_time.supervisor import SECRETS_BACKEND
+
+ for secrets_backend in SECRETS_BACKEND:
+ print("The backend in the sdk is", secrets_backend)
Review Comment:
```suggestion
```
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2002702721
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -87,6 +88,35 @@
return _get_connection(conn_id)
+ @classmethod
+ def get_connection_from_secrets(cls, conn_id: str) -> Connection:
+ """
+ Get connection by conn_id.
+
+ :param conn_id: connection id
+ :return: connection
+ """
+ # TODO: check cache first
+ # enabled only if SecretCache.init() has been called first
+
+ # iterate over configured backends if not in cache (or expired)
+ from airflow.sdk.execution_time.supervisor import SECRETS_BACKEND
+
+ for secrets_backend in SECRETS_BACKEND:
+ print("The backend in the sdk is", secrets_backend)
Review Comment:
## Clear-text logging of sensitive information
This expression logs [sensitive data (secret)](1) as clear text.
This expression logs [sensitive data (secret)](2) as clear text.
[Show more details](https://github.com/apache/airflow/security/code-scanning/439)
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on PR #47946:
URL: https://github.com/apache/airflow/pull/47946#issuecomment-2735686625
Work left to do, but created a PR so that i get feedback on the general direction I am proceeding in
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2005360252
##########
airflow/config_templates/config.yml:
##########
@@ -1416,6 +1416,29 @@ api:
version_added: 2.7.0
example: ~
default: "False"
+workers:
+ description: Configuration related to workers that run Airflow tasks.
+ options:
+ secrets_backend:
+ description: |
+ Full class name of secrets backend to enable for workers (will precede env vars backend)
+ version_added: 3.0.0
+ type: string
+ example: "airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend"
+ default: ""
+ secrets_backend_kwargs:
+ description: |
+ The secrets_backend_kwargs param is loaded into a dictionary and passed to ``__init__``
+ of secrets backend class. See documentation for the secrets backend you are using.
+ JSON is expected.
+
+ Example for AWS Systems Manager ParameterStore:
+ ``{"connections_prefix": "/airflow/connections", "profile_name": "default"}``
+ version_added: 3.0.0
+ type: string
+ sensitive: true
+ example: ~
+ default: ""
Review Comment:
I think a good compromise here would be to allow workers to configure their own backends, but if not set, then we fallback onto the config defined for 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
Re: [PR] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2005360636
##########
airflow/config_templates/config.yml:
##########
@@ -1416,6 +1416,29 @@ api:
version_added: 2.7.0
example: ~
default: "False"
+workers:
+ description: Configuration related to workers that run Airflow tasks.
+ options:
+ secrets_backend:
+ description: |
+ Full class name of secrets backend to enable for workers (will precede env vars backend)
+ version_added: 3.0.0
+ type: string
+ example: "airflow.providers.amazon.aws.secrets.systems_manager.SystemsManagerParameterStoreBackend"
+ default: ""
+ secrets_backend_kwargs:
+ description: |
+ The secrets_backend_kwargs param is loaded into a dictionary and passed to ``__init__``
+ of secrets backend class. See documentation for the secrets backend you are using.
+ JSON is expected.
+
+ Example for AWS Systems Manager ParameterStore:
+ ``{"connections_prefix": "/airflow/connections", "profile_name": "default"}``
+ version_added: 3.0.0
+ type: string
+ sensitive: true
+ example: ~
+ default: ""
Review Comment:
`conf.get("worker", "secrets_backend", fallback=None) or conf.get("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
Re: [PR] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003251664
##########
task-sdk/src/airflow/sdk/definitions/connection.py:
##########
@@ -24,7 +24,8 @@
import attrs
-from airflow.exceptions import AirflowException
+from airflow.exceptions import AirflowException, AirflowNotFoundException
+from airflow.sdk.api.datamodels._generated import ConnectionResponse
Review Comment:
It is possible yes, let me try and move it to supervisor itself im thinking. It doesnt have to be linked to the class too, its a general util method
--
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] Move Secrets Backend for Variables & Connections support to Task SDK [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #47946:
URL: https://github.com/apache/airflow/pull/47946#discussion_r2003587558
##########
task-sdk/src/airflow/sdk/definitions/variable.py:
##########
@@ -52,3 +57,38 @@ def get(cls, key: str, default: Any = NOTSET, deserialize_json: bool = False):
if e.error.error == ErrorType.VARIABLE_NOT_FOUND and default is not NOTSET:
return default
raise
+
+ @classmethod
+ def get_variable_from_secrets(cls, key: str):
+ """
+ Get Airflow Variable by iterating over all Secret Backends.
+
+ :param key: Variable Key
+ :return: Variable Value
+ """
+ # TODO: check cache first
+ # enabled only if SecretCache.init() has been called first
+ from airflow.sdk.execution_time.supervisor import SECRETS_BACKEND
+
+ var_val = NOTSET
+ # iterate over backends if not in cache (or expired)
+ for secrets_backend in SECRETS_BACKEND:
+ try:
+ var_val = secrets_backend.get_variable(key=key)
+ if var_val is not NOTSET:
+ return var_val
+ except Exception:
+ log.exception(
+ "Unable to retrieve variable from secrets backend (%s). "
+ "Checking subsequent secrets backend.",
+ type(secrets_backend).__name__,
+ )
+
+ if var_val is not NOTSET:
+ raise AirflowNotFoundException(f"The variable with key `{key}` isn't defined")
+
+ return var_val
+
+ @staticmethod
+ def _convert_variable_to_response(key: str, value: str | None) -> VariableResponse:
+ return VariableResponse(key=key, value=value)
Review Comment:
I did this in latest commit: b7ad2e6da6cfa0d08d3c0d3209162452e7d4d851
--
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