SamWheating commented on issue #32839: URL: https://github.com/apache/airflow/issues/32839#issuecomment-1688935875
Just catching up on this issue and figured I'd write out some thoughts here - We have two places to define permissions (manually via the UI/CLI and in code via `DAG.access_control`) and there's no stance on which one is authoritative which leads to all sorts of unexpected behaviour (mostly around [not cleaning up roles enough](https://github.com/apache/airflow/issues/25149), or [cleaning up roles too aggressively](https://github.com/apache/airflow/issues/32839)) I would propose making the access control for a given DAG authoritative as long as it is explicitly set. This way, if a DAG was updated, removing `access_control` from a DAG would not trigger a cleanup, but setting `access_control={}` would. So we have three cases. A DAG defined like this: ```python with DAG( dag_id="test", schedule="0 0 * * *", start_date=pendulum.datetime(2021, 1, 1, tz="UTC"), access_control={'role1': {'can_read'}, 'role2': {'can_read', 'can_edit', 'can_delete'}} ) as dag: ``` Would overwrite any existing dag-level permissions for this DAG, ensuring that the roles listed here are the only ones with permissions specific to this DAG. While a DAG with explicitly empty access_contol, like this: ```python with DAG( dag_id="test", schedule="0 0 * * *", start_date=pendulum.datetime(2021, 1, 1, tz="UTC"), access_control={} ) as dag: ``` Would explicitly clear out all DAG-level permissions for this role. While a DAG without any access_control defined: ```python with DAG( dag_id="test", schedule="0 0 * * *", start_date=pendulum.datetime(2021, 1, 1, tz="UTC"), ) as dag: ``` Would just defer to the existing DAG-level roles in the database. This presents its own hazards around dangling and stray permissions, but I think that any approach here is going to have some sort of downside. The important part is that the approach to DAG-level access is clearly-defined and well documented (neither of which is true at the moment). Thoughts? I likely missed some consideration here, but I am working on an implementation PR which I will share shortly. -- 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]
