Skip to content

Add TaskAction validation before finalizer registration#7006

Merged
AdilFayyaz merged 8 commits intov2from
adil/task-action-validation
Mar 11, 2026
Merged

Add TaskAction validation before finalizer registration#7006
AdilFayyaz merged 8 commits intov2from
adil/task-action-validation

Conversation

@AdilFayyaz
Copy link

@AdilFayyaz AdilFayyaz commented Mar 10, 2026

Tracking issue

Closes #6934

Why are the changes needed?

In PR #6903, it was noted that plugin resolution (the check for whether a registered plugin exists for a given taskType) happened after the finalizer was added to the resource. This meant that if a TaskAction had an unrecognized taskType, the controller would:

  1. Add a finalizer on the first reconcile
  2. Discover the plugin is missing on the second reconcile
  3. Mark the resource terminal

At that point, deleting the resource requires the controller to run handleAbortAndFinalize to remove the finalizer even though no plugin ever ran and there is nothing to clean up. Additionally, validateSpec only checked 4 of the 9 required spec fields, leaving runName, org, project, domain, and actionName unvalidated at the controller level.

What changes were proposed in this pull request?

  • Replaced the separate validateSpec function and inline ResolvePlugin block with a unified validateTaskAction function that validates all required spec fields and resolves the plugin in a single pass
  • This validation now runs before the finalizer is added if validation fails, the resource is marked terminal (Failed condition), not requeued, and no finalizer is registered, making deletion trivial
  • Introduced a pluginResolver interface so validateTaskAction is testable without a real plugin registry
  • Added taskaction_validation_test.go with standard Go unit tests covering all 9 required field checks and the plugin-not-found case
  • Fixed the existing integration test fixture which was missing required fields (taskType, taskTemplate)

How was this patch tested?

  • Unit tests: go test ./pkg/controller/... -run TestValidateTaskAction -v covers all 9 missing-field cases and plugin-not-found
  • Integration test: go test ./pkg/controller/... -v confirms reconcile completes without error and sets conditions

Setup process

Screenshots

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

Docs link

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
@AdilFayyaz AdilFayyaz self-assigned this Mar 10, 2026
@AdilFayyaz AdilFayyaz added the added Merged changes that add new functionality label Mar 10, 2026
@github-actions github-actions bot mentioned this pull request Mar 10, 2026
3 tasks
Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
pingsutw and others added 6 commits March 10, 2026 17:34
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
@AdilFayyaz AdilFayyaz merged commit 0444f0e into v2 Mar 11, 2026
14 checks passed
@AdilFayyaz AdilFayyaz deleted the adil/task-action-validation branch March 11, 2026 15:38
afbrock pushed a commit that referenced this pull request Mar 18, 2026
* add: validation

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>

* fix

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>

* [V2] Add GitHub Actions workflow for Go unit tests (#6999)

Signed-off-by: Kevin Su <pingsutw@apache.org>

* add: validation

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>

* fix

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>

* minor fix

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>

* fix: tests

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>

---------

Signed-off-by: M. Adil Fayyaz <62440954+AdilFayyaz@users.noreply.github.com>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Co-authored-by: Kevin Su <pingsutw@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

added Merged changes that add new functionality flyte2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants