potiuk commented on code in PR #33632:
URL: https://github.com/apache/airflow/pull/33632#discussion_r1303817691
##########
docs/apache-airflow/security/access-control.rst:
##########
@@ -229,3 +229,39 @@ List Plugins Plugins.can_read
List Task Reschedules Task Reschedules.can_read
Admin
List Triggers Triggers.can_read
Admin
======================================
=======================================================================
============
+
+These DAG-level controls can be set directly through the UI / CLI, or encoded
in the dags themselves through the access_control arg.
+
+Order of precedence for DAG-level permissions
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Since DAG-level access control can be configured in multiple places, conflicts
are inevitable and a clear resolution strategy is required.
+
+As a result, Airflow considers the `access_control` argument supplied on a DAG
itself to be completely authoritative if present, which has a few effects:
+
+ - `access_control` will overwrite any previously existing DAG-level
permissions if it is any value other than `None`.
+
+.. exampleinclude::
/../../airflow/example_dags/example_dag_level_access_control.py
+ :language: python
+ :start-after: [START howto_fine_grained_access_control]
+ :end-before: [END howto_fine_grained_access_control]
+
+ - Setting `access_control={}` will wipe any existing DAG-level permissions
for a given DAG from the DB.
+
+.. exampleinclude::
/../../airflow/example_dags/example_dag_level_access_control.py
+ :language: python
+ :start-after: [START howto_no_fine_grained_access_control]
+ :end-before: [END howto_no_fine_grained_access_control]
+
+ - Removing the access_control block from a DAG altogether (or setting it to
`None`) can leave dangling permissions.
Review Comment:
Yep. This is a side effect that we can accept.
One small comment. Maybe a good idea would be to also accompany it with a
new CLI command `airflow dags clean permissions` and a new API call
(eventually) to do it. Those can be separate PRs, but having them explicitly
spelled out as way to clean permissions on a DAG that previously had
permissions is a way to workaround the problem with dangling permissions.
I see that as an edge case, with this change there is already a way to clean
permissions by setting access_control to `{}` - but I can see a situation that
somoene would like to recover from the historical changes.
--
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]
