Revert airflowctl dependency from airflow-core

[bugraoz93]

branch-from: #68726, so it should be merged after that. Include those changes. I have created those separately to align.
reverts: #68175 and #64943

---

Was generative AI tooling used to co-author this PR?

• Yes (please specify the tool below)

---

• Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
• For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
• When adding dependency, check compliance with the ASF 3rd Party License Policy.
• For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

[jedcunningham]

I was actually just reverting #68175. I'm curious why you decided to though @bugraoz93?

[bugraoz93]

I was actually just reverting #68175. I'm curious why you decided to though @bugraoz93?

@jedcunningham I have added that replacement as part of core cli towards airflowctl migration/deprecation. Ash raised a concern earlier regarding one of the migration PRs for AIP 94. I added the AIP-94 direction to the last dev meeting to get clarity on core deprecation cycle for 3.3. We discussed the direction and resulted in change in the meeting due to uncertainty about airflowctl adoption. The decision was to keep the warnings only for developers/maintainers to stop developing tagged commands, rather than moving effort to airflowctl the secure way. Closer to v4, we can again discuss layering to user. AIP 94 will be mainly maintainer deprecation and migration tooling.

After the meeting notes are released, it will be clearer what was discussed in details. Why were going to revert it? I tested with local breeze hope it wasn't breaking :)

[bugraoz93]

I will resolve the conflicts in the coming hours

[jedcunningham]

Cool. Just for extra context, this is why I was going to revert. We can't just "pick the first" user to attribute calls to - that makes the tracked user in the audit log basically useless.

I also have concerns around requiring explicit tokens to be set for non-simple/fab auth manager users. But I never fully formed my argument on that one.

Just keep in mind, this change has already been released in FAB. So any redo etc will need to take into account that that helper is in the wild now and really can't be used...

[bugraoz93]

Cool. Just for extra context, this is why I was going to revert. We can't just "pick the first" user to attribute calls to - that makes the tracked user in the audit log basically useless.

I also have concerns around requiring explicit tokens to be set for non-simple/fab auth manager users. But I never fully formed my argument on that one.

Just keep in mind, this change has already been released in FAB. So any redo etc will need to take into account that that helper is in the wild now and really can't be used...

I should have overlook that part. In my mind we should have create a dummy user and use that with some admin cli tracing. Totally agree, that shouldn't be the case 🤦My bad, thanks Jed for checking in and was going to revert!
That concern is also valid but without a valid user crafted API communication not really possible. There could be better way for sure. That won't be a concern anymore as direction changed.
I was wondering the reason to follow up better. I will make this revert ready in an hour or so. Not on pc unfortunately. Maybe we can yank only fab as others are just useless rather wrong logic in fab? After reverting I can help releasing providers again with accelerated vote. Or even two fab and keycloak providers with auth managers

[bugraoz93]

cc: @shahar1 @potiuk @vincbeck
I totally overlooked this and should have warn this before release. Is it possible to yank keycloak and fab as it is not too late only day went? What do you think?
I would be happy to help with a follow up release for just two providers. For keycloak case it will be more of a useless case but for fab the logic is wrong :(

Edit: Checked and see that we don't even need to create a separate release for Keycloak, as this was the only change for this release. Wanted to get consensus before taking any action

[shahar1]

cc: @shahar1 @potiuk @vincbeck I totally overlooked this and should have warn this before release. Is it possible to yank keycloak and fab as it is not too late only day went? What do you think? I would be happy to help with a follow up release for just two providers. For keycloak case it will be more of a useless case but for fab the logic is wrong :(

Edit: Checked and see that we don't even need to create a separate release for Keycloak, as this was the only change for this release. Wanted to get consensus before taking any action

Yanking is reserved mostly for catastrophic changes (i.e., provider isn't usable at all, critical security breaches, etc.) -
so I'd rather not to do it if the provider is still usable except for the broken feature.
In this case the reverted changes are additive, so I'm not sure that it's justified - please fix me if I'm wrong here.
I'll be happy to help with an ad-hoc release (+accelerated vote if needed).

[bugraoz93]

cc: @shahar1 @potiuk @vincbeck I totally overlooked this and should have warn this before release. Is it possible to yank keycloak and fab as it is not too late only day went? What do you think? I would be happy to help with a follow up release for just two providers. For keycloak case it will be more of a useless case but for fab the logic is wrong :(

Edit: Checked and see that we don't even need to create a separate release for Keycloak, as this was the only change for this release. Wanted to get consensus before taking any action

Yanking is reserved mostly for catastrophic changes (i.e., provider isn't usable at all, critical security breaches, etc.) -
so I'd rather not to do it if the provider is still usable except for the broken feature.
In this case the reverted changes are additive, so I'm not sure that it's justified - please fix me if I'm wrong here.
I'll be happy to help with an ad-hoc release (+accelerated vote if needed).

Thank a lot Shahar for the comment and help offer! I want to collect your thinking around what we should do about it and raise awareness. These methods arent called from any piece of code neither while only aimed to be used from core 3.3.0. I would be okay with agreed approach. It is not exactly a catastrophic thing for say

[shahar1]

cc: @shahar1 @potiuk @vincbeck I totally overlooked this and should have warn this before release. Is it possible to yank keycloak and fab as it is not too late only day went? What do you think? I would be happy to help with a follow up release for just two providers. For keycloak case it will be more of a useless case but for fab the logic is wrong :(
Edit: Checked and see that we don't even need to create a separate release for Keycloak, as this was the only change for this release. Wanted to get consensus before taking any action

Yanking is reserved mostly for catastrophic changes (i.e., provider isn't usable at all, critical security breaches, etc.) -
so I'd rather not to do it if the provider is still usable except for the broken feature.
In this case the reverted changes are additive, so I'm not sure that it's justified - please fix me if I'm wrong here.
I'll be happy to help with an ad-hoc release (+accelerated vote if needed).

Thank a lot Shahar for the comment and help offer! I want to collect your thinking around what we should do about it and raise awareness. These methods arent called from any piece of code neither while only aimed to be used from core 3.3.0. I would be okay with agreed approach. It is not exactly a catastrophic thing for say

Best way IMO - comment directly on the announcement I sent on users@ and dev@, I think that it should be enough at this point.
Once the PR is merged, ping me - and I'll create the ad-hoc releases for both + add a comment on the incompatible versions.

[jedcunningham]

My 2c, just don't use get_cli_user again and you avoid the problem completely 🤷

[bugraoz93]

My 2c, just don't use get_cli_user again and you avoid the problem completely 🤷

Many thanks Jed! It will be impossible to make this usable after this in main and, after backporting, in 3-3-test. Unless someone pulls that exact version, overrides their own auth manager calls to use it somehow, and redeploys Airflow with those changes. It won't be usable as an auth manager initialised from the config at deployment time, while calling them outside the running airflow environment doesn't make any sense either

[github-actions]

Backport failed to create: v3-3-test. View the failure log Run details

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status	Branch	Result
❌	v3-3-test

You can attempt to backport this manually by running:

cherry_picker 6877aea v3-3-test

This should apply the commit to the v3-3-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.