You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "raboof (via GitHub)" <gi...@apache.org> on 2026/03/26 08:24:44 UTC
[PR] docs: clearer comment on multi-tenancy in the execution API [airflow]
raboof opened a new pull request, #64246:
URL: https://github.com/apache/airflow/pull/64246
Without this the invalid security reports will just keep coming.
<!-- 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
-->
---
##### 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)
<!--
Generated-by: [Tool Name] following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number.
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #64246:
URL: https://github.com/apache/airflow/pull/64246#discussion_r3025328990
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py:
##########
@@ -57,6 +55,13 @@ async def has_xcom_access(
token.id,
xcom_key,
)
+
+ # The current version of Airflow does not support true
+ # multi-tenancy yet (this is well-documented at
+ # https://airflow.apache.org/docs/apache-airflow/stable/security/security_model.html#future-multi-tenancy-isolation),
+ # so for now we always return 'True' here.
+ # When we introduce true multi-tenancy in the future
+ # this would be the place to do add a check.
Review Comment:
Grammar/typo: "this would be the place to do add a check" should be rephrased (e.g., remove the extra "do") to avoid distracting from the intended clarification.
```suggestion
# this would be the place to add a check.
```
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/connections.py:
##########
@@ -33,13 +33,18 @@ async def has_connection_access(
token=CurrentTIToken,
) -> bool:
"""Check if the task has access to the connection."""
- # TODO: Placeholder for actual implementation
-
log.debug(
"Checking access for task instance with key '%s' to connection '%s'",
token.id,
connection_id,
)
+
+ # The current version of Airflow does not support true
+ # multi-tenancy yet (this is well-documented at
+ # https://airflow.apache.org/docs/apache-airflow/stable/security/security_model.html#future-multi-tenancy-isolation),
+ # so for now we always return 'True' here.
+ # When we introduce true multi-tenancy in the future
+ # this would be the place to do add a check.
Review Comment:
Grammar/typo: "this would be the place to do add a check" should be rephrased (e.g., remove the extra "do") to keep the comment clear.
```suggestion
# this would be the place to add a check.
```
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/variables.py:
##########
@@ -37,13 +37,20 @@ async def has_variable_access(
):
"""Check if the task has access to the variable."""
write = request.method not in {"GET", "HEAD", "OPTIONS"}
- # TODO: Placeholder for actual implementation
+
log.debug(
"Checking %s access for task instance with key '%s' to variable '%s'",
"write" if write else "read",
token.id,
variable_key,
)
+
+ # The current version of Airflow does not support true
+ # multi-tenancy yet (this is well-documented at
+ # https://airflow.apache.org/docs/apache-airflow/stable/security/security_model.html#future-multi-tenancy-isolation),
+ # so for now we always return 'True' here.
+ # When we introduce true multi-tenancy in the future
+ # this would be the place to do add a check.
Review Comment:
Grammar/typo: "this would be the place to do add a check" should be rephrased (e.g., remove the extra "do") to keep the comment clear.
```suggestion
# this would be the place to add a check.
```
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #64246:
URL: https://github.com/apache/airflow/pull/64246#issuecomment-4259335679
Great, now green!
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64246:
URL: https://github.com/apache/airflow/pull/64246#issuecomment-4235793374
> (AFAICT the CI failures are unrelated so this PR is good to go, LMK if I misinterpreted that)
unrelated - yes. And suggestion is to always rebase - we sometimes (too often even - and we are working on it) have main broken and we continuously work on getting it greener on average. It's a bit of uphilll battle, and sometimes we are winning it more, sometimes we are loosing it more.
But ABR (Always Be Rebased) is generally best idea.
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #64246:
URL: https://github.com/apache/airflow/pull/64246#issuecomment-4260391951
done!
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #64246:
URL: https://github.com/apache/airflow/pull/64246#discussion_r2993223798
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/connections.py:
##########
@@ -33,13 +33,19 @@ async def has_connection_access(
token=CurrentTIToken,
) -> bool:
"""Check if the task has access to the connection."""
- # TODO: Placeholder for actual implementation
log.debug(
"Checking access for task instance with key '%s' to connection '%s'",
token.id,
connection_id,
)
+
+ # The current version of Airflow does not support true
+ # multi-tenancy yet (this is well-documented at
Review Comment:
```suggestion
# The current version of Airflow does not support
# multi-tenancy yet (this is well-documented at
```
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck merged PR #64246:
URL: https://github.com/apache/airflow/pull/64246
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #64246:
URL: https://github.com/apache/airflow/pull/64246#discussion_r3092534487
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/connections.py:
##########
@@ -33,13 +33,19 @@ async def has_connection_access(
token=CurrentTIToken,
) -> bool:
"""Check if the task has access to the connection."""
- # TODO: Placeholder for actual implementation
log.debug(
"Checking access for task instance with key '%s' to connection '%s'",
token.id,
connection_id,
)
+
+ # The current version of Airflow does not support true
+ # multi-tenancy yet (this is well-documented at
Review Comment:
I think "true multi-tenancy" is OK here? Airflow has _some_ multi-tenancy features as long as the tenants trust each other, but no "true" multi-tenancy for tenants that don't?
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #64246:
URL: https://github.com/apache/airflow/pull/64246#issuecomment-4235795454
(rebased it now)
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #64246:
URL: https://github.com/apache/airflow/pull/64246#issuecomment-4260375947
You have many open discussions, please resolve them
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #64246:
URL: https://github.com/apache/airflow/pull/64246#issuecomment-4235783274
updated to link to https://airflow.apache.org/docs/apache-airflow/stable/security/security_model.html#limiting-dag-author-access-to-subset-of-dags
--
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] docs: clearer comment on multi-tenancy in the execution API [airflow]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on PR #64246:
URL: https://github.com/apache/airflow/pull/64246#issuecomment-4178690413
(AFAICT the CI failures are unrelated so this PR is good to go)
--
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