Fix: Keep compatibility with old FAB versions #41549

joaopamaral · 2024-08-17T03:36:25Z

FAB versions before 1.3.0 still use the old access control format that allows only DAGs resource. This PR fixes the format when airflow is running with FAB versions < 1.3.0.

Tests:

FAB 1.2.2
- Old access_control format: access_control={'Viewer': ["can_read","can_delete"]}

# airflow sync-perm --include-dags 
Updating actions and resources for all existing roles                                                                
Updating permission on all DAG views                                                                                 
[2024-08-19T16:57:58.390+0000] {dagbag.py:567} INFO - Filling up the DagBag from database                            
[2024-08-19T16:57:58.397+0000] {override.py:1105} WARNING - ACCESS CONTROL: {'Viewer': {'can_read', 'can_delete'}}   
[2024-08-19T16:57:58.410+0000] {override.py:1882} INFO - Added Permission can read on DAG:tutorial2 to role Viewer   
[2024-08-19T16:57:58.414+0000] {override.py:1882} INFO - Added Permission can delete on DAG:tutorial2 to role Viewer

New access_control format: access_control={'Viewer': {"DAGs": ["can_read","can_delete"]}}
FAB 1.3.0:
- Mixed formats:

access_control={
        'Viewer': ["can_read","can_delete"]
    }
    
access_control={
        'Viewer': {"DAGs": ["can_read","can_delete"],
                   "DAG Runs": ["can_create"],}
    },

# airflow sync-perm --include-dags                                                                                                                       
Updating actions and resources for all existing roles                                                                                                    
Updating permission on all DAG views                                                                                                                     
[2024-08-19T17:15:21.591+0000] {dagbag.py:567} INFO - Filling up the DagBag from database                                                                
[2024-08-19T17:15:21.601+0000] {override.py:1158} WARNING - ACCESS CONTROL: {'Viewer': {'DAGs': {'can_read', 'can_delete'}}}                             
[2024-08-19T17:15:21.613+0000] {override.py:1158} WARNING - ACCESS CONTROL: {'Viewer': {'DAGs': {'can_read', 'can_delete'}, 'DAG Runs': ['can_create']}}

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
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.

…into fix/compatibility-old-fab # Conflicts: # airflow/models/dag.py

…into fix/compatibility-old-fab

joaopamaral · 2024-08-19T19:30:03Z

There is another breaking change Remove deprecated SubDags that is causing a failure during the sync permissions in old FAB versions: https://github.com/apache/airflow/blame/587015ddc84dfd306e4c2487c3aeb65ae280eab5/airflow/providers/fab/auth_manager/security_manager/override.py#L1076

When I try to run the airflow sync-perm --include-dags with airflow main branch and an old FAB I get the error which is not related to the access control changes:

# airflow sync-perm --include-dags                                                                                                                                            │  ____________       _____________
/usr/local/lib/python3.8/site-packages/flask_limiter/extension.py:333 UserWarning: Using the in-memory storage for tracking rate limits as no storage was explicitly specified. This is not recommended for │ ____    |__( )_________  __/__  /________      __
production use. See: https://flask-limiter.readthedocs.io#configuring-a-storage-backend for documentation about configuring the storage backend.                                                            │____  /| |_  /__  ___/_  /_ __  /_  __ \_ | /| / /
Updating actions and resources for all existing roles                                                                                                                                                       │___  ___ |  / _  /   _  __/ _  / / /_/ /_ |/ |/ /
Updating permission on all DAG views                                                                                                                                                                        │ _/_/  |_/_/  /_/    /_/    /_/  \____/____/|__/
[2024-08-19T19:26:23.125+0000] {dagbag.py:567} INFO - Filling up the DagBag from database                                                                                                                   │[2024-08-19 19:24:20 +0000] [140] [INFO] Starting gunicorn 23.0.0
Traceback (most recent call last):                                                                                                                                                                          │[2024-08-19 19:24:20 +0000] [140] [INFO] Listening at: http://[::]:8794 (140)
  File "/usr/local/bin/airflow", line 8, in <module>                                                                                                                                                        │[2024-08-19 19:24:20 +0000] [140] [INFO] Using worker: sync
    sys.exit(main())                                                                                                                                                                                        │[2024-08-19 19:24:20 +0000] [141] [INFO] Booting worker with pid: 141
  File "/opt/airflow/airflow/__main__.py", line 62, in main                                                                                                                                                 │[2024-08-19 19:24:20 +0000] [142] [INFO] Booting worker with pid: 142
    args.func(args)                                                                                                                                                                                         │[2024-08-19T19:24:20.894+0000] {triggerer_job_runner.py:181} INFO - Setting up TriggererHandlerWrapper with handler <FileTaskHandler (NOTSET)>
  File "/opt/airflow/airflow/cli/cli_config.py", line 49, in command                                                                                                                                        │[2024-08-19T19:24:20.894+0000] {triggerer_job_runner.py:237} INFO - Setting up logging queue listener with handlers [<RedirectStdHandler <stdout> (NOTSET)>, <TriggererHandlerWrapper (NOTSET)>]
    return func(*args, **kwargs)                                                                                                                                                                            │[2024-08-19T19:24:20.900+0000] {triggerer_job_runner.py:338} INFO - Starting the triggerer
  File "/opt/airflow/airflow/utils/cli.py", line 115, in wrapper                                                                                                                                            │[2024-08-19T19:25:21.099+0000] {triggerer_job_runner.py:510} INFO - 0 triggers currently running
    return f(*args, **kwargs)                                                                                                                                                                               │[2024-08-19T19:26:21.264+0000] {triggerer_job_runner.py:510} INFO - 0 triggers currently running
  File "/opt/airflow/airflow/utils/providers_configuration_loader.py", line 55, in wrapped_function                                                                                                         │
    return func(*args, **kwargs)                                                                                                                                                                            │
  File "/opt/airflow/airflow/providers/fab/auth_manager/cli_commands/sync_perm_command.py", line 39, in sync_perm                                                                                           │
    appbuilder.sm.create_dag_specific_permissions()                                                                                                                                                         │
  File "/opt/airflow/airflow/providers/fab/auth_manager/security_manager/override.py", line 1067, in create_dag_specific_permissions                                                                        │
    root_dag_id = dag.parent_dag.dag_id if dag.parent_dag else dag.dag_id                                                                                                                                   │
AttributeError: 'SerializedDAG' object has no attribute 'parent_dag'

To run the final tests with this PR change I need to apply the this line change.

@kaxil, do you know if it's related to this PR, is it expected to keep the compatibility with old FAB versions?

kaxil · 2024-08-20T00:11:28Z

There is another breaking change Remove deprecated SubDags that is causing a failure during the sync permissions in old FAB versions: https://github.com/apache/airflow/blame/587015ddc84dfd306e4c2487c3aeb65ae280eab5/airflow/providers/fab/auth_manager/security_manager/override.py#L1076

When I try to run the airflow sync-perm --include-dags with airflow 2.10 and an old FAB I get the error which is not related to the access control changes:

# airflow sync-perm --include-dags                                                                                                                                            │  ____________       _____________
/usr/local/lib/python3.8/site-packages/flask_limiter/extension.py:333 UserWarning: Using the in-memory storage for tracking rate limits as no storage was explicitly specified. This is not recommended for │ ____    |__( )_________  __/__  /________      __
production use. See: https://flask-limiter.readthedocs.io#configuring-a-storage-backend for documentation about configuring the storage backend.                                                            │____  /| |_  /__  ___/_  /_ __  /_  __ \_ | /| / /
Updating actions and resources for all existing roles                                                                                                                                                       │___  ___ |  / _  /   _  __/ _  / / /_/ /_ |/ |/ /
Updating permission on all DAG views                                                                                                                                                                        │ _/_/  |_/_/  /_/    /_/    /_/  \____/____/|__/
[2024-08-19T19:26:23.125+0000] {dagbag.py:567} INFO - Filling up the DagBag from database                                                                                                                   │[2024-08-19 19:24:20 +0000] [140] [INFO] Starting gunicorn 23.0.0
Traceback (most recent call last):                                                                                                                                                                          │[2024-08-19 19:24:20 +0000] [140] [INFO] Listening at: http://[::]:8794 (140)
  File "/usr/local/bin/airflow", line 8, in <module>                                                                                                                                                        │[2024-08-19 19:24:20 +0000] [140] [INFO] Using worker: sync
    sys.exit(main())                                                                                                                                                                                        │[2024-08-19 19:24:20 +0000] [141] [INFO] Booting worker with pid: 141
  File "/opt/airflow/airflow/__main__.py", line 62, in main                                                                                                                                                 │[2024-08-19 19:24:20 +0000] [142] [INFO] Booting worker with pid: 142
    args.func(args)                                                                                                                                                                                         │[2024-08-19T19:24:20.894+0000] {triggerer_job_runner.py:181} INFO - Setting up TriggererHandlerWrapper with handler <FileTaskHandler (NOTSET)>
  File "/opt/airflow/airflow/cli/cli_config.py", line 49, in command                                                                                                                                        │[2024-08-19T19:24:20.894+0000] {triggerer_job_runner.py:237} INFO - Setting up logging queue listener with handlers [<RedirectStdHandler <stdout> (NOTSET)>, <TriggererHandlerWrapper (NOTSET)>]
    return func(*args, **kwargs)                                                                                                                                                                            │[2024-08-19T19:24:20.900+0000] {triggerer_job_runner.py:338} INFO - Starting the triggerer
  File "/opt/airflow/airflow/utils/cli.py", line 115, in wrapper                                                                                                                                            │[2024-08-19T19:25:21.099+0000] {triggerer_job_runner.py:510} INFO - 0 triggers currently running
    return f(*args, **kwargs)                                                                                                                                                                               │[2024-08-19T19:26:21.264+0000] {triggerer_job_runner.py:510} INFO - 0 triggers currently running
  File "/opt/airflow/airflow/utils/providers_configuration_loader.py", line 55, in wrapped_function                                                                                                         │
    return func(*args, **kwargs)                                                                                                                                                                            │
  File "/opt/airflow/airflow/providers/fab/auth_manager/cli_commands/sync_perm_command.py", line 39, in sync_perm                                                                                           │
    appbuilder.sm.create_dag_specific_permissions()                                                                                                                                                         │
  File "/opt/airflow/airflow/providers/fab/auth_manager/security_manager/override.py", line 1067, in create_dag_specific_permissions                                                                        │
    root_dag_id = dag.parent_dag.dag_id if dag.parent_dag else dag.dag_id                                                                                                                                   │
AttributeError: 'SerializedDAG' object has no attribute 'parent_dag'

To run the final tests with this PR change I need to apply the this line change.

@kaxil, do you know if it's related to this PR, is it expected to keep the compatibility with old FAB versions?

Older FAB version won't work with Airflow 3. You will need to use the changes from the PR or use apache-airflow-providers-fab==1.3.0rc1

joaopamaral · 2024-08-20T01:35:01Z

Older FAB version won't work with Airflow 3. You will need to use the changes from https://github.com/apache/airflow/pull/41390or use apache-airflow-providers-fab==1.3.0rc1

Thanks for explaining that @kaxil! I've tested with tag 2.10.0 and it's working.

potiuk · 2024-08-20T20:08:08Z

According to the new rules- we should also back-port it to v2-10-test by PR

potiuk · 2024-08-20T20:09:19Z

(we means merging committer shoudl decide if they want to do it themselves or ask the author to do so).

vincbeck · 2024-08-20T20:12:51Z

Good call, actually (and I should have thought about it before merging it), do we need it in main at all? Should it be only in 2.10 branch? Because as Kaxil mentioned:

Older FAB version won't work with Airflow 3. You will need to use the changes from https://github.com/apache/airflow/pull/41390or use apache-airflow-providers-fab==1.3.0rc1

Therefore, this compatibility code should only be for Airflow 2.x?

potiuk · 2024-08-20T20:34:37Z

Possibly - but we have not seen this kind of need before - we assumed we backport everything from main and we do not have "2.10 only" fixes - but yes, this one looks like a legitimate case (@ephraimbuddy @utkarsharma2 @kaxil ?)

airflow/models/dag.py

vincbeck · 2024-08-26T14:32:52Z

@ephraimbuddy @utkarsharma2 @kaxil?

(cherry picked from commit d7d944e)

potiuk · 2024-08-27T13:40:06Z

Regardless from decision #41792 is the backport as it needs to go there.

potiuk · 2024-08-27T14:36:58Z

@joaopamaral -> I just cherry-picked the change to v2-10-test, but tests are failing, would you mind back-porting it and fixing the tests - see #41792

…1809) * Fix: Keep compatibility with old FAB versions (#41549) * Fix: Tests after #41549 (Keep compatibility with old FAB versions) * Fix test_dag and test_dagbag

joaopamaral added 10 commits August 17, 2024 00:34

Keep compatibility with old FAB versions

ee59a72

Revert

0023963

Use old access control format in FAB versions < 1.3.0

105c7bd

Keep compatibility with old FAB versions

1b7b052

Revert

b18fb58

Use old access control format in FAB versions < 1.3.0

5764da6

add tests

f628934

Merge remote-tracking branch 'joaopamaral/fix/compatibility-old-fab' …

236ba6b

…into fix/compatibility-old-fab # Conflicts: # airflow/models/dag.py

Merge branch 'main' into fix/compatibility-old-fab

1c50059

Merge remote-tracking branch 'joaopamaral/fix/compatibility-old-fab' …

271722e

…into fix/compatibility-old-fab

potiuk added this to the Airflow 2.10.1 milestone Aug 19, 2024

potiuk approved these changes Aug 19, 2024

View reviewed changes

Merge branch 'main' into fix/compatibility-old-fab

f8714c9

joaopamaral marked this pull request as ready for review August 19, 2024 19:30

joaopamaral requested review from kaxil, XD-DENG and ashb as code owners August 19, 2024 19:30

vincbeck approved these changes Aug 20, 2024

View reviewed changes

vincbeck merged commit d7d944e into apache:main Aug 20, 2024
50 checks passed

zachliu reviewed Aug 22, 2024

View reviewed changes

airflow/models/dag.py Show resolved Hide resolved

joaopamaral mentioned this pull request Aug 22, 2024

Airflow DAG access control permissions are not working #41684

Closed

2 tasks

potiuk pushed a commit to potiuk/airflow that referenced this pull request Aug 27, 2024

Fix: Keep compatibility with old FAB versions (apache#41549)

8c37105

(cherry picked from commit d7d944e)

potiuk mentioned this pull request Aug 27, 2024

Fix: Keep compatibility with old FAB versions (#41549) #41792

Closed

joaopamaral added a commit to joaopamaral/airflow that referenced this pull request Aug 27, 2024

Fix: Keep compatibility with old FAB versions (apache#41549)

8bc594a

joaopamaral added a commit to joaopamaral/airflow that referenced this pull request Aug 27, 2024

Fix: Tests after apache#41549 (Keep compatibility with old FAB versions)

55e6a4d

joaopamaral added a commit to joaopamaral/airflow that referenced this pull request Aug 27, 2024

Fix: Tests after apache#41549 (Keep compatibility with old FAB versions)

62709aa

potiuk mentioned this pull request Aug 28, 2024

Keep FAB compatibility for versions before 1.3.0 in 2.10 (#41549) #41809

Merged

utkarsharma2 added the type:bug-fix Changelog: Bug Fixes label Aug 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Keep compatibility with old FAB versions #41549

Fix: Keep compatibility with old FAB versions #41549

joaopamaral commented Aug 17, 2024 •

edited

Loading

joaopamaral commented Aug 19, 2024 •

edited

Loading

kaxil commented Aug 20, 2024

joaopamaral commented Aug 20, 2024

potiuk commented Aug 20, 2024

potiuk commented Aug 20, 2024

vincbeck commented Aug 20, 2024

potiuk commented Aug 20, 2024

vincbeck commented Aug 26, 2024

potiuk commented Aug 27, 2024

potiuk commented Aug 27, 2024

Fix: Keep compatibility with old FAB versions #41549

Fix: Keep compatibility with old FAB versions #41549

Conversation

joaopamaral commented Aug 17, 2024 • edited Loading

joaopamaral commented Aug 19, 2024 • edited Loading

kaxil commented Aug 20, 2024

joaopamaral commented Aug 20, 2024

potiuk commented Aug 20, 2024

potiuk commented Aug 20, 2024

vincbeck commented Aug 20, 2024

potiuk commented Aug 20, 2024

vincbeck commented Aug 26, 2024

potiuk commented Aug 27, 2024

potiuk commented Aug 27, 2024

joaopamaral commented Aug 17, 2024 •

edited

Loading

joaopamaral commented Aug 19, 2024 •

edited

Loading