Update spec validation for args.

Update spec validation for args for another edge case with tests.

PiperOrigin-RevId: 863420702

Author: nybidari

runsc/container/container_test.go

@@ -4260,27 +4260,67 @@ func TestSpecValidationIgnore(t *testing.T) {
 }
 
 func TestSpecValidationForArgs(t *testing.T) {
-	conf := testutil.TestConfig(t)
-	oldSpecs := make(map[string]*specs.Spec)
-	spec, _ := sleepSpecConf(t)
-	spec.Process.Cwd = "/bin"
-	spec.Process.Args[0] = "/bin/sleep"
-	oldSpecs["container1"] = spec
+	tests := []struct {
+		name        string
+		args        []string
+		restoreArgs []string
+		wantErr     bool
+	}{
+		{
+			name:        "base path same",
+			args:        []string{"/bin/sleep", "1000"},
+			restoreArgs: []string{"sleep", "1000"},
+			wantErr:     false,
+		},
+		{
+			name:        "executable path",
+			args:        []string{"/bin/sleep", "1000"},
+			restoreArgs: []string{"./sleep", "1000"},
+			wantErr:     false,
+		},
+		{
+			name:        "different args",
+			args:        []string{"/bin/sleep", "1000", "1"},
+			restoreArgs: []string{"./sleep", "1000", "infinity"},
+			wantErr:     true,
+		},
+		{
+			name:        "no base path",
+			args:        []string{"/bin/sleep/", "1000", "1"},
+			restoreArgs: []string{"sleep", "1000", "infinity"},
+			wantErr:     true,
+		},
+		{
+			name:        "same length with different path",
+			args:        []string{"/bin/sleep", "1000", "1"},
+			restoreArgs: []string{"/bin/sheep", "1000", "1"},
+			wantErr:     true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			conf := testutil.TestConfig(t)
 
-	newSpecs := make(map[string]*specs.Spec)
-	restoreSpec, _ := sleepSpecConf(t)
-	restoreSpec.Process.Cwd = "/bin"
-	restoreSpec.Process.Args[0] = "./sleep"
-	newSpecs["container1"] = restoreSpec
+			oldSpecs := make(map[string]*specs.Spec)
+			spec, _ := sleepSpecConf(t)
+			spec.Process.Cwd = "/bin"
+			spec.Process.Args = test.args
+			oldSpecs["container1"] = spec
 
-	if err := specutils.RestoreValidateSpec(oldSpecs, newSpecs, conf); err != nil {
-		t.Errorf("spec validation failed, got: %v, want: nil", err)
-	}
+			newSpecs := make(map[string]*specs.Spec)
+			restoreSpec, _ := sleepSpecConf(t)
+			restoreSpec.Process.Cwd = "/bin"
+			restoreSpec.Process.Args = test.restoreArgs
+			newSpecs["container1"] = restoreSpec
 
-	spec.Process.Args = append(spec.Process.Args, "1")
-	restoreSpec.Process.Args = append(restoreSpec.Process.Args, "infinity")
-	if err := specutils.RestoreValidateSpec(oldSpecs, newSpecs, conf); err == nil {
-		t.Errorf("spec validation passed when we expected it to fail")
+			err := specutils.RestoreValidateSpec(oldSpecs, newSpecs, conf)
+			if !test.wantErr && err != nil {
+				t.Errorf("spec validation failed, got: %v, want: nil", err)
+			}
+			if test.wantErr && err == nil {
+				t.Errorf("spec validation passed when we expected it to fail")
+			}
+		})
 	}
 }
 

runsc/specutils/restore.go

@@ -17,7 +17,7 @@ package specutils
 import (
 	"encoding/json"
 	"fmt"
-	"path"
+	"path/filepath"
 	"reflect"
 	"slices"
 	"sort"
@@ -309,7 +309,7 @@ func ifNil[T any](v *T) *T {
 // are not resolving any paths in the spec, maybe this is happening at a higher
 // layer. Ideally we should not be resolving any paths, this is a
 // workaround fix.
-func validateArgs(oldArgs, newArgs []string, cwd, cName string) error {
+func validateArgs(oldArgs, newArgs []string, cName string) error {
 	if len(oldArgs) != len(newArgs) {
 		return validateError("Args", cName, oldArgs, newArgs)
 	}
@@ -318,26 +318,16 @@ func validateArgs(oldArgs, newArgs []string, cwd, cName string) error {
 		return nil
 	}
 
-	oldExecPath := oldArgs[0]
-	newExecPath := newArgs[0]
-	hasPrefixOld := strings.HasPrefix(oldExecPath, "./")
-	hasPrefixNew := strings.HasPrefix(newExecPath, "./")
-	if hasPrefixOld == hasPrefixNew {
-		if !slices.Equal(oldArgs, newArgs) {
+	if oldArgs[0] != newArgs[0] {
+		oldExecName := filepath.Base(oldArgs[0])
+		newExecName := filepath.Base(newArgs[0])
+		if oldExecName != newExecName {
 			return validateError("Args", cName, oldArgs, newArgs)
 		}
-		return nil
+		log.Warningf("Validating args with the exec paths, old: %v new: %v", oldArgs[0], newArgs[0])
 	}
 
-	// One of the entrypoints across checkpoint restore is not resolved.
-	// Resolve the entrypoint using cwd to get the absolute path and compare.
-	if hasPrefixOld {
-		oldExecPath = path.Join(cwd, oldExecPath)
-	}
-	if hasPrefixNew {
-		newExecPath = path.Join(cwd, newExecPath)
-	}
-	if oldExecPath != newExecPath || !slices.Equal(oldArgs[1:], newArgs[1:]) {
+	if !slices.Equal(oldArgs[1:], newArgs[1:]) {
 		return validateError("Args", cName, oldArgs, newArgs)
 	}
 	return nil
@@ -390,7 +380,7 @@ func validateSpecForContainer(oSpec, nSpec *specs.Spec, cName string) error {
 		return err
 	}
 	oldProcess.Rlimits, newProcess.Rlimits = nil, nil
-	if err := validateArgs(oldProcess.Args, newProcess.Args, oldProcess.Cwd, cName); err != nil {
+	if err := validateArgs(oldProcess.Args, newProcess.Args, cName); err != nil {
 		return err
 	}
 	oldProcess.Args, newProcess.Args = nil, nil