Feat/check multi team enabled when team name provided api

[ahilashsasidharan]

Description

This PR adds pydantic validators to the Pools, Connections and Variables APIs to ensure that multi_team mode is enabled when team_name is provided. This check is missing from these APIs.

Motivation

In the case of the Pools API (which should apply to other APIs) without this check as shown in issue #62251 under Test 3 you are able to create a pool when multi-team is disabled if there is a pre-existing team in the database, and if you try to add a pool with a non-existent team you will get a 500 error. With this change you will now get a 422 value error as shown below with a clearer error.

Tests

• Added tests under airflow-core/tests/unit/api_fastapi/core_api/routes/public for each respective API file to validate the error behaviour when submitting a request with team-name and multi-team mode disabled.
• Fixed failing tests under airflow-core/tests/unit/api_fastapi/core_api/routes/public in Connections and Pools test files due to default behaviour of multi-team mode being disabled in tests.

related: #62251

---

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

• Yes (Google Antigravity)

---

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

[bugraoz93]

Looks good, mine is also nit but I think both are relatively quick changes :)

[ahilashsasidharan]

Thanks for the feedback. Currently debugging an issue with connections with this change. I believe some code is missing in the connections code that causes errors in the CI and when creating connections with this change, so will push code to address the comments what I fix that issue.

[vincbeck]

@ahilashsasidharan are you working on it?

[ahilashsasidharan]

@ahilashsasidharan are you working on it?

@vincbeck yes sorry for the delay on this. I am moving these checks to airflow-core/src/airflow/api_fastapi/core_api/routes/public/ for the respective files as when looking at the CI issues. I noticed an unrelated issues that causes the following error "PydanticSerializationError: Error serializing to JSON: InvalidToken" when creating a connection which I believe is due to having the validator run again in the code and not being able to check the multi_team conf variable.

Edit: Issue I mentioned was an environment issue I resolved.

The CI issue should just be some missing code in the airflow-core/src/airflow/ui/src/queries/useEditConnection.tsx file.

I am working on testing these changes manually to see if there are any other issues and if not will hopefully have something pushed soon.

[pierrejeambrun]

LGTM

[pierrejeambrun]

LGTM beside vincent comment.

Also it appears that CI need some fixing.

[pierrejeambrun]

I have rebase the branch to get the latest CI fixes from main. That could help with unrelated failures.

[o-nikolas]

Seems one test left reliably failing @ahilashsasidharan can you have a look?

[Copilot]

Pull request overview

Adds request-body validation to prevent setting team_name when Airflow core.multi_team is disabled, and updates/extends unit tests to cover the new 422 behavior across Pools, Connections, and Variables APIs.

Changes:

• Added Pydantic model_validator checks in Pools/Connections/Variables request body models.
• Added/updated FastAPI unit tests to assert 422 responses when team_name is provided but multi-team is disabled.
• Adjusted existing tests to explicitly enable multi-team where required for team_name-bearing requests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
airflow-core/src/airflow/api_fastapi/core_api/datamodels/variables.py	Adds multi-team gating validator for team_name in Variable bodies.
airflow-core/src/airflow/api_fastapi/core_api/datamodels/pools.py	Adds multi-team gating validators for pool POST/PATCH bodies.
airflow-core/src/airflow/api_fastapi/core_api/datamodels/connections.py	Adds multi-team gating validator for connection bodies.
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py	Adds tests asserting 422 when team_name is used with multi-team disabled.
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_pools.py	Adds tests for 422 behavior and enables multi-team in existing tests that use team_name.
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py	Adds tests for 422 behavior and enables multi-team in existing tests that use team_name.

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

[ahilashsasidharan]

Question for @o-nikolas and others: I pushed an implementation that moves the check to airflow-core/src/airflow/api_fastapi/core_api/routes/public/ instead of in airflow-core/src/airflow/api_fastapi/core_api/datamodels/. After some initial testing, both implementation work, so I wanted some feedback on which one is preferred.

Looking at the code I feel under routes/public might be more appropriate for this type of check, but it requires more code spread across each file including additional code to handle checking bulk requests. On the other hand the datamodel approach is simpler and reviewers were previously fine with it.

Happy to go with whichever is preferred.

One thing to add for this is that having these checks gives us more control over how errors are sent for Bulk endpoints. Right now the implemenation for errors looks like this:

Error: 400 {"detail":{"message":"team_name cannot be set when multi_team mode is disabled. Please contact your administrator.","invalid_variable_keys":["team_a_api_key","team_a_bucket","team_b_api_key"]}

This would be how an error would like with the check being in datamodels with a model_validator (with more failing actions/entities):

Error: 422 {"detail":[{"type":"value_error","loc":["body","actions",1,"create","entities",0],"msg":"Value error, team_name cannot be set when multi_team mode is disabled","input":{"key":"team_a_api_key","value":"abc123","description":"API key for team A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",1],"msg":"Value error, team_name cannot be set when multi_team mode is disabled","input":{"key":"team_a_bucket","value":"s3://team-a-bucket","description":"S3 bucket for team A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",2],"msg":"Value error, team_name cannot be set when multi_team mode is disabled","input":{"key":"team_b_api_key","value":"xyz789","description":"API key for team B","team_name":"team_b"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",0],"msg":"Value error, team_name cannot be set when multi_team mode is disabled","input":{"key":"team_a_api_key","value":"abc123","description":"API key for team A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",1],"msg":"Value error, team_name cannot be set when multi_team mode is disabled","input":{"key":"team_a_bucket","value":"s3://team-a-bucket","description":"S3 bucket for team A","team_name":"team_a"},"ctx":{"error":{}}},{"type":"value_error","loc":["body","actions",1,"create","entities",2],"msg":"Value error, team_name cannot be set when multi_team mode is disabled","input":{"key":"team_b_api_key","value":"xyz789","description":"API key for team B","team_name":"team_b"},"ctx":{"error":{}}}]}

Both reject the entire request even ones without team_name. That could be changed in public/routes implemenation.

I've left some comments unresolved that are related to the other implemenation and will resolve them once I have some feedback on the preferred implementation.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

[pierrejeambrun]

Why did we move from having this validation done at the pydantic level (which I think was nice, part of the deserializer model) to now have this entangled in the router code?

[ahilashsasidharan]

Why did we move from having this validation done at the pydantic level (which I think was nice, part of the deserializer model) to now have this entangled in the router code?

I was debugging an issue with the pydantic implementation and switched to having it in the router code, but the issue was unrelated to either implementation.

I was just waiting on feedback on the preferred method. If the pydantic method was nice. I will revert back to that as that implementation requires less code and complexity.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

[pierrejeambrun]

Nice thanks for the update.

A UI follow up would be great

[github-actions]

Backport successfully created: v3-2-test

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-2-test