Validate plugins in /plugins API and skip invalid ones with warnings by Brunda10 · Pull Request #55673 · apache/airflow

Brunda10 · 2025-09-15T12:39:10Z

This PR fixes an issue where an invalid plugin definition (e.g., an unsupported destination in external_views) caused the entire /plugins API endpoint to fail with a 500.

Closes : #55571

With this change:

Added validation when constructing PluginResponse objects.

Only valid plugins are returned in the /api/v2/plugins API response.

Invalid plugins are skipped gracefully.

A warning is logged in the terminal with the plugin name and the error details.

This ensures the UI remains functional even if some plugins are misconfigured.

Impact:

Prevents the entire /plugins API from failing due to a single invalid plugin.
Users will continue to see all valid plugins in the UI, while still being informed about invalid ones through logs.

Attached screenshot for reference

pierrejeambrun

Just one comment.

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py

jason810496

Same comment as Pierre, it would be nice to add the tests. Thanks!

I have one small question for logging part.
After Switch all airflow logging to structlog #52651, do we need to switch to structlog for Core API as well?

cc @ashb

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py

ashb · 2025-09-16T08:23:43Z

After #52651, do we need to switch to structlog for Core API as well?

@jason810496 We can switch things over to log = structlog.get_logger(), but any uses of stdlib logging will automatically get captured/redirect to send via structlog too, so no, we don't have to change anything but we can.

ashb

Handling this at the API layer is not really the right place, it should be handled and validated earlier on when plugins are first loaded I think

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py

pierrejeambrun

Just to answer @ashb concerns, we have multiple layers of validation.

Basically the plugin manager will 'load' (just import) plugins. There we allow almost 'any' type of dict object, because it's how it was done in AF2 to allow custom plugin structures.

We just do some very basic validation there, and plugins are considered 'loaded', only really hurtful mistakes that could crash the API server or cause bad behavior are ignored with an error message there.

Then for the API to actually be able to return those responses it has to be valid in regards to API layer pydantic validation. (or the route will crash). There we allow 'extra' keys similarly to AF2, but for existing keys / enums, they have to comply. If they do not match the serializer validation, they will be ignored from the response. (what we try to implement there, instead of erroring the plugin endpoint)

I didn't want to bring 'pydantic' API level validation to the plugin manager because it would introduce coupling from the plugin manager over some 'api layer' level validator, which didn't sound good at the time. I would like to achieve something similar at some point. Basically don't load and ignore anything that is not correct directly at plugin manager load time but:

That could be considered breaking, plugin manager will stop loading stuff and other components (not the API) could still rely on those 'loadable plugins' but not serializable ones.
I didn't find yet time and a good approach to achieve that (without the coupling mentioned above etc...)

So I think we should, remove plugins that the API cannot serialize, and have a count 'in sync' with that. So API client (UI basically), will just see valid plugins from the API standpoint. The PluginManager is more tolerant, and will try load more things as it used too in AF2.

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py

airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_plugins.py

Brunda10 · 2025-09-18T07:34:59Z

Thanks for the feedback @pierrejeambrun ,

I’ve updated the implementation to:

First validate all plugins and build the valid_plugins list.
Then apply pagination and total_entries only on the valid set.
Added a test case to validate pagination when invalid plugins are present, ensuring page size and offsets behave correctly.

pierrejeambrun

Nice thanks

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py

pierrejeambrun · 2025-09-18T11:49:26Z

CI is failing, I'll rebase the PR to trigger it again.

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

boring-cyborg · 2025-09-18T13:59:55Z

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

bbovenzi · 2025-09-18T15:48:02Z

Adding to 3.1 milestone. This feels like a useful bug fix

…55673) * plugin module from breaking on invalid external_view destination * Added Test Case - For invalid plugin * Suggested Change Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com> * Suggested Change Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com> * Suggested Change Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com> * Suggested Change Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com> * suggested change as per feedback * Modified the total plugin count in testcases * validation done before pagination --------- Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com> (cherry picked from commit ba00da4)

Brunda10 requested review from bugraoz93, ephraimbuddy, jason810496, pierrejeambrun, rawwar and shubhamraj-git as code owners September 15, 2025 12:39

boring-cyborg bot added the area:API Airflow's REST/HTTP API label Sep 15, 2025

Brunda10 mentioned this pull request Sep 15, 2025

Whole plugin module breaks if user gives wrong destination for external_view plugin #55571

Closed

2 tasks

pierrejeambrun reviewed Sep 15, 2025

View reviewed changes

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py Outdated Show resolved Hide resolved

jason810496 reviewed Sep 15, 2025

View reviewed changes

Brunda10 commented Sep 16, 2025

View reviewed changes

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py Outdated Show resolved Hide resolved

ashb reviewed Sep 16, 2025

View reviewed changes

pierrejeambrun reviewed Sep 16, 2025

View reviewed changes

pierrejeambrun reviewed Sep 17, 2025

View reviewed changes

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py Outdated Show resolved Hide resolved

airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_plugins.py Show resolved Hide resolved

pierrejeambrun approved these changes Sep 18, 2025

View reviewed changes

airflow-core/src/airflow/api_fastapi/core_api/routes/public/plugins.py Show resolved Hide resolved

Brunda10 and others added 9 commits September 18, 2025 13:49

plugin module from breaking on invalid external_view destination

c1bf29f

Added Test Case - For invalid plugin

605fb8a

Suggested Change

7796b11

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

Suggested Change

7866baa

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

Suggested Change

2b4a937

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

Suggested Change

8ab8972

Co-authored-by: Ash Berlin-Taylor <ash_github@firemirror.com>

suggested change as per feedback

879f16e

Modified the total plugin count in testcases

ccd8ffc

validation done before pagination

2ad2327

pierrejeambrun force-pushed the plugin_validation_fix branch from 208f740 to 2ad2327 Compare September 18, 2025 11:49

pierrejeambrun merged commit ba00da4 into apache:main Sep 18, 2025
109 checks passed

bbovenzi added this to the Airflow 3.1.0 milestone Sep 18, 2025

potiuk mentioned this pull request Nov 8, 2025

Add missing translations for PL for 3.1.3 #58076

Merged

Conversation

Brunda10 commented Sep 15, 2025

Uh oh!

pierrejeambrun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jason810496 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashb commented Sep 16, 2025

Uh oh!

ashb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pierrejeambrun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Brunda10 commented Sep 18, 2025

Uh oh!

pierrejeambrun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pierrejeambrun commented Sep 18, 2025

Uh oh!

Uh oh!

boring-cyborg bot commented Sep 18, 2025

Uh oh!

bbovenzi commented Sep 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jason810496 left a comment •

edited

Loading

pierrejeambrun left a comment •

edited

Loading