Skip to content

Commit 22b0d1c

Browse files
nybidarigvisor-bot
authored andcommitted
Fix validateCapabilities to handle nil and empty arrays for spec validation.
validateCapabilities was using reflect.DeepEqual which considers nil and empty slices as different. But for validation of spec, these should be considered as same. Fix this by calling validateArray (for each field within capabilities) which compares the length of the array. PiperOrigin-RevId: 811436172
1 parent 092d474 commit 22b0d1c

File tree

2 files changed

+33
-13
lines changed

2 files changed

+33
-13
lines changed

runsc/container/container_test.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3879,7 +3879,7 @@ func TestSpecValidation(t *testing.T) {
38793879
mutate: func(spec, restoreSpec *specs.Spec, _, _ string) {
38803880
restoreSpec.Process.Capabilities.Bounding = append(restoreSpec.Process.Capabilities.Bounding, "CAP_NET_RAW")
38813881
},
3882-
wantErr: "Capabilities does not match across checkpoint restore",
3882+
wantErr: "Capabilities.Bounding does not match across checkpoint restore",
38833883
},
38843884
{
38853885
name: "Resources",
@@ -4111,6 +4111,23 @@ func TestSpecValidationForArgs(t *testing.T) {
41114111
}
41124112
}
41134113

4114+
func TestSpecValidationForCapabilities(t *testing.T) {
4115+
conf := testutil.TestConfig(t)
4116+
oldSpecs := make(map[string]*specs.Spec)
4117+
spec, _ := sleepSpecConf(t)
4118+
spec.Process.Capabilities.Bounding = nil
4119+
oldSpecs["container1"] = spec
4120+
4121+
newSpecs := make(map[string]*specs.Spec)
4122+
restoreSpec, _ := sleepSpecConf(t)
4123+
restoreSpec.Process.Capabilities.Bounding = []string{}
4124+
newSpecs["container1"] = restoreSpec
4125+
4126+
if err := specutils.RestoreValidateSpec(oldSpecs, newSpecs, conf); err != nil {
4127+
t.Errorf("spec validation failed, got: %v, want: nil", err)
4128+
}
4129+
}
4130+
41144131
func TestCheckpointResume(t *testing.T) {
41154132
for name, conf := range configs(t, true /* noOverlay */) {
41164133
t.Run(name, func(t *testing.T) {

runsc/specutils/restore.go

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -210,25 +210,28 @@ func validateMap[K comparable, V comparable](field, cName string, oldM map[K]V,
210210
return nil
211211
}
212212

213-
func sortCapabilities(o *specs.LinuxCapabilities) {
214-
sort.Strings(o.Bounding)
215-
sort.Strings(o.Effective)
216-
sort.Strings(o.Inheritable)
217-
sort.Strings(o.Permitted)
218-
sort.Strings(o.Ambient)
219-
}
220-
221213
func validateCapabilities(field, cName string, oldCaps, newCaps *specs.LinuxCapabilities) error {
222214
if oldCaps == nil && newCaps == nil {
223215
return nil
224216
}
225217
if oldCaps == nil || newCaps == nil {
226218
return validateError(field, cName, oldCaps, newCaps)
227219
}
228-
sortCapabilities(oldCaps)
229-
sortCapabilities(newCaps)
230-
if !reflect.DeepEqual(oldCaps, newCaps) {
231-
return validateError(field, cName, oldCaps, newCaps)
220+
221+
if err := validateArray(field+".Bounding", cName, oldCaps.Bounding, newCaps.Bounding); err != nil {
222+
return err
223+
}
224+
if err := validateArray(field+".Effective", cName, oldCaps.Effective, newCaps.Effective); err != nil {
225+
return err
226+
}
227+
if err := validateArray(field+".Inheritable", cName, oldCaps.Inheritable, newCaps.Inheritable); err != nil {
228+
return err
229+
}
230+
if err := validateArray(field+".Permitted", cName, oldCaps.Permitted, newCaps.Permitted); err != nil {
231+
return err
232+
}
233+
if err := validateArray(field+".Ambient", cName, oldCaps.Ambient, newCaps.Ambient); err != nil {
234+
return err
232235
}
233236
return nil
234237
}

0 commit comments

Comments
 (0)