Fix sync-perm to work correctly when update_fab_perms = False
#14847
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.)
kaxil
approved these changes
Mar 17, 2021
|
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest master or amend the last commit of the PR, and push it with --force-with-lease. |
github-actions
bot
added
the
okay to merge
ashb
added a commit
to astronomer/airflow
that referenced
this pull request
Mar 17, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
ashb
added a commit
to astronomer/airflow
that referenced
this pull request
Mar 17, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
ashb
added a commit
to astronomer/airflow
that referenced
this pull request
Mar 17, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
ashb
added a commit
to astronomer/airflow
that referenced
this pull request
Mar 17, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
ashb
added a commit
to astronomer/airflow
that referenced
this pull request
Mar 17, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
ashb
added a commit
to astronomer/airflow
that referenced
this pull request
Mar 17, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
kaxil
pushed a commit
to astronomer/airflow
that referenced
this pull request
Mar 17, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9) (cherry picked from commit c5ea249)
kaxil
pushed a commit
to astronomer/airflow
that referenced
this pull request
Apr 26, 2021
…he#14847) If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
potiuk
pushed a commit
that referenced
this pull request
May 9, 2021
If Airflow is configured with update_fab_perms config setting to False, then the Op, User and Viewer roles are created _before_ the permissions objects are written to the database, meaning that these roles did not correctly get assigned all the permissions we asked for (the missing permissions are just silently not created.) Because of the "migrate to resource permission" migration this problem is not "disasterous" as all most of the Permissions et al. we use are created by a migration. This changes it so that the permissions are always created/synced before we look at the roles. (Re-running sync-perm wouldn't fix this, as although the second time around the Permissions will exist in the DB, we see that Op role already has permissions and don't make any changes, assuming that the site operators made such changes.) (cherry picked from commit 1cd62b9)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If Airflow is configured with update_fab_perms config setting to False,
then the Op, User and Viewer roles are created before the permissions
objects are written to the database, meaning that these roles did not
correctly get assigned all the permissions we asked for (the missing
permissions are just silently not created.)
Because of the "migrate to resource permission" migration this problem
is not "disasterous" as all most of the Permissions et al. we use are
created by a migration.
This changes it so that the permissions are always created/synced before
we look at the roles.
(Re-running sync-perm wouldn't fix this, as although the second time
around the Permissions will exist in the DB, we see that Op role already
has permissions and don't make any changes, assuming that the site
operators made such changes.)
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.