Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix revoke Dag stale permission on airflow < 2.10 #42844

Open
wants to merge 354 commits into
base: main
Choose a base branch
from

Conversation

joaopamaral
Copy link
Contributor

closes: #42743

This reported issue was happening on airflow < 2.10 using the old access_control format dict[str, set] and it was captured by the test tests/providers/fab/auth_manager/test_security.py when run the airflow 2.9.3 with fab 1.4.0:

FAILED tests/providers/fab/auth_manager/test_security.py::test_access_control_stale_perms_are_revoked - AttributeError: 'set' object has no attribute 'get'

After this fix the same test pass:

tests/providers/fab/auth_manager/test_security.py::test_access_control_stale_perms_are_revoked PASSED

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

@joaopamaral
Copy link
Contributor Author

Tested with airflow 2.9.3:

  • Added a dag with the following access control:
    access_control={
        "Viewer": {'can_read', 'can_edit', 'can_delete'}
    }
  • The UI shows all the new permissions:
image
  • Removed the can_delete permission:
    access_control={
        "Viewer": {'can_read', 'can_edit'}
    }
  • The permission is updated without any error:
image

@joaopamaral joaopamaral marked this pull request as ready for review October 8, 2024 23:03
@joaopamaral joaopamaral changed the title Fix revoke stale on airflow < 2.10 Fix revoke Dag stale permission on airflow < 2.10 Oct 8, 2024
Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static checks are failing but otherwise, LGTM

@potiuk
Copy link
Member

potiuk commented Oct 12, 2024

Yeah. And it needs to be rebased after providers have been moved to top level.

@potiuk
Copy link
Member

potiuk commented Oct 12, 2024

Would also be nice if there is a unit test for it.

moiseenkov and others added 21 commits October 21, 2024 19:49
- Label deprecated items with deprecated decorator.
- Refactor vertex_ai AutoMLHook.create_auto_ml_text_training_job, to
  avoid calling get_auto_ml_text_training_job, to not trigger mypy
errors after applying @deprecate on hook method.
- Update docs.
- Delete irrelevant system tests.

Co-authored-by: Oleg Kachur <[email protected]>
Ensure that the EMR cluster stays running for long enough for all the
tasks in the DAG to complete their operations against the cluster.
* Switch Ui codegen to fastAPI spec

* Rebase

* read access control from config

---------

Co-authored-by: pierrejeambrun <[email protected]>
* Add documentation for FAB DB commands

We recently separated FAB migration from Airflow's migration and added some commands
for handling DB upgrade and downgrade in FAB. This PR adds user-facing documentation
on how to upgrade and downgrade FAB and information about the different commands.

I also added documentation on how contributors can hook their application's migration
into Airflow's migration.

The update_migration_references pre-commit was also updated to include FAB migrations.

* fixup! Add documentation for FAB DB commands

* fixup! fixup! Add documentation for FAB DB commands
This makes it so that we can alembic commands from the root repo dir.

E.g.

```
alembic -c airflow/alembic.ini revision --autogenerate -m "Do something"
```

Sometimes when running from airflow subfolder we get errors.
This index is not needed because there's another index on the table that leads with dag_id.
This is just to ensure that we get each hook on a single line when running in split pane on macbook pro 14".
* Fix pre-commit for auto update of fab migration versions

This fixes the pre-commit for updating the version in FAB migration and
also, updated the actual version as 1.4 since that's going to be the next version

At the same time, added Daniel's fix for airflow alembic.ini file

* fixup! Fix pre-commit for auto update of fab migration versions

* fixup! fixup! Fix pre-commit for auto update of fab migration versions
* Prepare docs for Sep 1st wave of providers

* fix spelling

* update fab
pierrejeambrun and others added 30 commits October 21, 2024 19:49
Bumps [micromatch](https://github.com/micromatch/micromatch) from 4.0.7 to 4.0.8.
- [Release notes](https://github.com/micromatch/micromatch/releases)
- [Changelog](https://github.com/micromatch/micromatch/blob/master/CHANGELOG.md)
- [Commits](micromatch/micromatch@4.0.7...4.0.8)

---
updated-dependencies:
- dependency-name: micromatch
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [rollup](https://github.com/rollup/rollup) from 4.21.0 to 4.24.0.
- [Release notes](https://github.com/rollup/rollup/releases)
- [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md)
- [Commits](rollup/rollup@v4.21.0...v4.24.0)

---
updated-dependencies:
- dependency-name: rollup
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [vite](https://github.com/vitejs/vite/tree/HEAD/packages/vite) from 5.4.4 to 5.4.6.
- [Release notes](https://github.com/vitejs/vite/releases)
- [Changelog](https://github.com/vitejs/vite/blob/v5.4.6/packages/vite/CHANGELOG.md)
- [Commits](https://github.com/vitejs/vite/commits/v5.4.6/packages/vite)

---
updated-dependencies:
- dependency-name: vite
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…solved into new assets (apache#42735)

* fix(assets/managers): fix error handling file loc when asset alias resolved into new assets

* test(asset/manager): add test case test_register_asset_change_with_alias

* refactor(assets/manager): simplify for loop
* Drop Python 3.8 support in core

Add newsfragment

* Drop Python 3.8 support in provider packages
…pache#42711)

* Add ability to switch between table and card views for listing items

* add test for DagCard and loading states

* Add tests for loading states

* PR feedback

* use semantic tokens for reused colors

* Remove eslint-ignore lost in rebase
Previously we were getting 'PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.'  This sets it to the new default, and makes the warning go away.
* feat: allow customizing podManagementPolicy

* fix: adjust schema and identation

* ci: adjust lint

---------

Co-authored-by: Arakaki, Everton <[email protected]>
Previously it worked in general, but did not in certain test scenarios where we did not initialize the database.  This is one solution.
related to apache#41816

Adds a warning log to indicate failure if length of task_key>100.
This is a no-op change right now, but as part of the provider re-org in apache#42505
this sets us up to be able to load the providers code in the tests

The reason this change is done separately is that changes to breeze code form
forks doesn't take effect, and this small change makes it easier to land on
main without having to re-create that large PR.
This PR moves Scarf entry in Release notes at top of 2.10.0 entries and also
adds URL/IP-address info that is covered by [Scarf's privacy policy](https://about.scarf.sh/privacy-policy).
* Add ability to provide proxy for dbt Cloud connection

* Running pre-commit checks

* Update current tests and add new test with proxy
I haven't been that active coding or reviewing. Although I am now returning back to focus on Task SDK (AIP-72) so narrowing down my entries in CODEOWNERS.
…42832)

The [existing documentation](https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/secrets-backends/aws-secrets-manager.html#example-of-storing-google-secrets-in-aws-secrets-manager) on how to set up Google secrets in AWS Secrets Manager is out of date. It led me on a merry chase for hours.

I hereby submit this PR to update the doc. I solemnly swear that the content has been verified using DAG code similar to this

```python
gsheet = GSheetsHook(gcp_conn_id=gcp_conn_id)
values = gsheet.get_values(
    spreadsheet_id=spreadsheet_id,
    range_=f"{sheet_name}!B1:B2",
)
`
…ale-permission-on-version-2.9.x' into fix/fab-access-control-revoke-stale-permission-on-version-2.9.x
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revoking stale permissions breaks DAG import