Skip to content

fix: add nil safety checks and immutability validation - #3864

Open
Swaraj-do wants to merge 4 commits into
kubeflow:masterfrom
Swaraj-do:fix/runtime-nil-checks
Open

fix: add nil safety checks and immutability validation#3864
Swaraj-do wants to merge 4 commits into
kubeflow:masterfrom
Swaraj-do:fix/runtime-nil-checks

Conversation

@Swaraj-do

@Swaraj-do Swaraj-do commented Aug 6, 2026

Copy link
Copy Markdown

What this PR does / why we need it

This PR resolves several critical nil-pointer dereference vulnerabilities and enforces immutability validation across the TrainJob controller, admission webhooks, and runtime plugins:

  1. ReplicatedJob & Runtime Nil Safety (pkg/runtime & jobset plugin):

    • Adds nil-pointer guards in ExtractResourcePerNodeFromRuntime and JobSet.Validate to safely handle minimal or malformed ReplicatedJob specifications without triggering controller panics.
    • Prevents crashes when processing empty or partial job templates.
  2. Controller Status Preservation (pkg/controller/trainjob_controller.go):

    • Adds nil runtime guards in setTrainJobStatus.
    • Preserves pre-existing status conditions accumulated during reconciliation iterations rather than overwriting them.
  3. runtimeRef Immutability Validation (pkg/webhooks/trainjob_webhook.go):

    • Enforces validation in the TrainJob admission webhook ensuring spec.runtimeRef cannot be modified during updates after creation.
  4. TorchTune Plugin Guards (pkg/runtime/framework/plugins/torch/torchtune.go):

    • Safely handles nil spec.trainer inside validateTorchTune and getRecipeAndConfig, preventing admission webhook panics when trainer fields are absent.
  5. Comprehensive Unit Test Coverage:

    • Adds dedicated unit tests across trainjob_controller_test.go, jobset_test.go, torchtune_test.go, runtime_test.go, and trainjob_webhook_test.go to prevent regressions.

Checklist

  • Title follows conventional commit format (fix: ...)
  • Code complies with project style and guidelines
  • Unit tests included for all modified components (go test ./pkg/...)
  • DCO Signed-off on all commits

@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jeffwan for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow
google-oss-prow Bot requested a review from kuizhiqing August 6, 2026 14:54
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

🎉 Welcome to the Kubeflow Trainer! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

  • If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards.
  • Our team will review your PR soon! cc @kubeflow/kubeflow-trainer-team

Join the community:

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

@Swaraj-do
Swaraj-do force-pushed the fix/runtime-nil-checks branch from 8f7277f to 1ce7bff Compare August 6, 2026 15:10
Add nil safety checks in ExtractResourcePerNodeFromRuntime and JobSet.Validate to prevent runtime panics when processing minimal or malformed ReplicatedJob specifications.

Signed-off-by: Swaraj-do <swarajmohanty183@gmail.com>
Add nil runtime guard in setTrainJobStatus and preserve pre-existing status conditions set during reconciliation.

Signed-off-by: Swaraj-do <swarajmohanty183@gmail.com>
@Swaraj-do
Swaraj-do force-pushed the fix/runtime-nil-checks branch from 1ce7bff to 384fc5f Compare August 6, 2026 15:18
@Swaraj-do Swaraj-do changed the title fix(runtime): add nil pointer checks for replicated job template specs fix(runtimes): add nil pointer checks for replicated job template specs Aug 6, 2026
Validate that Spec.RuntimeRef cannot be modified during TrainJob updates.

Signed-off-by: Swaraj-do <swarajmohanty183@gmail.com>
@Swaraj-do Swaraj-do changed the title fix(runtimes): add nil pointer checks for replicated job template specs fix: add nil safety checks and immutability validation Aug 6, 2026
Safely handle nil spec.trainer in validateTorchTune and getRecipeAndConfig to prevent admission webhook panics.

Signed-off-by: Swaraj-do <swarajmohanty183@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants