kubeflow / trainer / 37654841788

72%

push

github

fix(runtimes): guard nil SSHAuthMountPath and MPIImplementation dereferences in MPI plugin (#4157)

* fix(runtimes): guard nil SSHAuthMountPath and MPIImplementation dereferences in MPI plugin

EnforceMLPolicy and buildHostFileConfigMap in
pkg/runtime/framework/plugins/mpi/mpi.go unconditionally dereferenced
two pointer fields on MPIMLPolicySource:

- SSHAuthMountPath at line 198
- MPIImplementation at lines 208 and 322

Both fields carry +kubebuilder:default markers, but those defaults are
only applied by the API server. Objects constructed without going through
it (unit tests, or objects built in-process) leave the fields nil,
causing nil pointer dereference panics.

Replace each bare dereference with ptr.Deref using the field's
documented CRD default as the fallback ("/root/.ssh" for
SSHAuthMountPath, OpenMPI for MPIImplementation), consistent with how
PR #4054 fixed the same pattern in the Flux plugin.

Note: the NumProcPerNode sites at lines 131 and 220 were fixed
separately in PR #4049.

Add two regression test cases to TestMPI that exercise each nil path
and confirm no panic.

Signed-off-by: Nikhil Bansiwal <nikhilbansiwal3495@gmail.com>

* fix(runtimes): use constants.MPISSHAuthDefaultMountPath in MPI plugin

Replace the "/root/.ssh" string literal in ptr.Deref with a named
constant defined next to the other MPI SSH constants in pkg/constants.

Suggested by andreyvelich in PR review.

Signed-off-by: Nikhil Bansiwal <nikhilbansiwal3495@gmail.com>

* test(runtimes): merge MPI nil-fallback test cases into one

Merge "sshAuthMountPath is nil in mlPolicy and falls back to /root/.ssh"
and "mpiImplementation is nil in mlPolicy and falls back to OpenMPI"
into a single case where both fields are nil, as suggested by
andreyvelich in the PR review.

Signed-off-by: Nikhil Bansiwal <nikhilbansiwal3495@gmail.com>

---------

Signed-off-by: Nikhil Bansiwal <nikhilbansiwal3495@gmail.com>

3 of 3 new or added lines in 1 file covered. (100.0%)

3339 of 4621 relevant lines covered (72.26%)

0.84 hits per line