Skip to content

Make pause DAG its own role separate from edit DAG#30581

Closed
herlambang wants to merge 14 commits intoapache:mainfrom
herlambang:dag-pause-role
Closed

Make pause DAG its own role separate from edit DAG#30581
herlambang wants to merge 14 commits intoapache:mainfrom
herlambang:dag-pause-role

Conversation

@herlambang
Copy link
Contributor

@herlambang herlambang commented Apr 11, 2023

Add ability to pause DAGs as separate role from edit.

Use cases

Giving users ability to running DAGs without having the ability to pause it. Also to prevent users to easily/accidentally suspend DAGs.

Scopes

  • Admin UI
  • API

Changes

  • add ACTION_CAN_PAUSE ("can_pause") in security permissions
  • set can_pause as part of accessible dag in security manager
  • add can_pause dag as part of default USER_PERMISSIONS in security manager
  • add function to check if user eligible to pause all/specific dags in security manager
  • disable toggle Pause/Unpause DAG in home page if user is not eligible for can_pause dag
  • disable toggle Pause/Unpause DAG in detail dag page if user is not eligible for can_pause dag
  • disable toggle Unpause DAG when triggered in trigger form if user is not eligible for can_pause dag
  • change access permission to /paused from ACTION_CAN_EDIT to ACTION_CAN_PAUSE on main view
  • change access permission on patch /dags/<dag-id> and /dags from ACTION_CAN_EDIT to ACTION_CAN_PAUSE on API end point
  • add can_pause on DAG Level Role access control documentation
  • add migration, append can_pause permission to roles that have can_edit dag(s) permission
  • add tests

closes: #22317


^ 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.

@boring-cyborg boring-cyborg bot added area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Apr 11, 2023
@herlambang herlambang marked this pull request as ready for review April 11, 2023 16:20
@herlambang herlambang changed the title Introduce DAG pause role Make pause DAG its own role separate from edit DAG Apr 12, 2023
@ashb
Copy link
Member

ashb commented Apr 12, 2023

What is edit dag role used for if not pause?

@herlambang
Copy link
Contributor Author

To trigger Dag Run, combine with dag_run can_create

@potiuk
Copy link
Member

potiuk commented Apr 22, 2023

I think it looks good in principle, but if I understand it correctly, it is heavily breaking change and it should have support to migrate existing permission that will work in general case.

If I understand it - someone who currently has "CAN_EDIT" role will suddenly not be able to pause dags.

In other words people who could pause dags, will suddenly loose this capability without a warning?

If so, then this is a breaking change and we need to make sure that backwards compatibility is maintained. At the minimum level every role that currently has CAN_EDIT role should have CAN_PAUSE assigned during the migration.

We have to remember, that our users could have their roles assigned in arbitrary way so making sure that ADMIN user has all the permissions is not enough, because our users could have different roles defined on their own.

@herlambang
Copy link
Contributor Author

Hi @potiuk , thanks for the feedback.
I added a migration and adjust the security manager to keep the ability of pausing DAGs with current can_edit permission.
Documentation also added and tests also adjusted.
Looking forward for further review, thanks.

(Sorry for my bad on pushing dirty commit after rebase, added several people as reviewers 🙏 )

@potiuk
Copy link
Member

potiuk commented Apr 28, 2023

I'd need others to take a look if that change makes sense.

@potiuk
Copy link
Member

potiuk commented May 6, 2023

Anyone else wants to chime in ? (and @herlambang -> you will need ot rebase and fix the conflict).

@herlambang
Copy link
Contributor Author

(and @herlambang -> you will need ot rebase and fix the conflict).

on it!

Changes added in description, probably to make it more appealing to read

@potiuk
Copy link
Member

potiuk commented May 9, 2023

Anyone here who is more into permissions? @jhtimmins @ephraimbuddy maybe ? :)

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jun 24, 2023
@aru-trackunit
Copy link
Contributor

@herlambang I love the work you've done so far. Looking forward to test it!

@github-actions github-actions bot removed the stale Stale PRs per the .github/workflows/stale.yml policy file label Jun 28, 2023
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Aug 13, 2023
@github-actions github-actions bot closed this Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues stale Stale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make pause DAG its own role separate from edit DAG

5 participants