diff --git a/pkg/cri/sbserver/container_create_linux_test.go b/pkg/cri/sbserver/container_create_linux_test.go index 01b7f5611..e2c62d62e 100644 --- a/pkg/cri/sbserver/container_create_linux_test.go +++ b/pkg/cri/sbserver/container_create_linux_test.go @@ -198,12 +198,14 @@ func TestContainerCapabilities(t *testing.T) { testContainerName := "container-name" testPid := uint32(1234) allCaps := cap.Known() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string capability *runtime.Capability includes []string excludes []string }{ - "should be able to add/drop capabilities": { + { + desc: "should be able to add/drop capabilities", capability: &runtime.Capability{ AddCapabilities: []string{"SYS_ADMIN"}, DropCapabilities: []string{"CHOWN"}, @@ -211,19 +213,22 @@ func TestContainerCapabilities(t *testing.T) { includes: []string{"CAP_SYS_ADMIN"}, excludes: []string{"CAP_CHOWN"}, }, - "should be able to add all capabilities": { + { + desc: "should be able to add all capabilities", capability: &runtime.Capability{ AddCapabilities: []string{"ALL"}, }, includes: allCaps, }, - "should be able to drop all capabilities": { + { + desc: "should be able to drop all capabilities", capability: &runtime.Capability{ DropCapabilities: []string{"ALL"}, }, excludes: allCaps, }, - "should be able to drop capabilities with add all": { + { + desc: "should be able to drop capabilities with add all", capability: &runtime.Capability{ AddCapabilities: []string{"ALL"}, DropCapabilities: []string{"CHOWN"}, @@ -231,7 +236,8 @@ func TestContainerCapabilities(t *testing.T) { includes: util.SubtractStringSlice(allCaps, "CAP_CHOWN"), excludes: []string{"CAP_CHOWN"}, }, - "should be able to add capabilities with drop all": { + { + desc: "should be able to add capabilities with drop all", capability: &runtime.Capability{ AddCapabilities: []string{"SYS_ADMIN"}, DropCapabilities: []string{"ALL"}, @@ -240,7 +246,8 @@ func TestContainerCapabilities(t *testing.T) { excludes: util.SubtractStringSlice(allCaps, "CAP_SYS_ADMIN"), }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() @@ -390,33 +397,39 @@ func TestContainerAndSandboxPrivileged(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string containerPrivileged bool sandboxPrivileged bool expectError bool }{ - "privileged container in non-privileged sandbox should fail": { + { + desc: "privileged container in non-privileged sandbox should fail", containerPrivileged: true, sandboxPrivileged: false, expectError: true, }, - "privileged container in privileged sandbox should be fine": { + { + desc: "privileged container in privileged sandbox should be fine", containerPrivileged: true, sandboxPrivileged: true, expectError: false, }, - "non-privileged container in privileged sandbox should be fine": { + { + desc: "non-privileged container in privileged sandbox should be fine", containerPrivileged: false, sandboxPrivileged: true, expectError: false, }, - "non-privileged container in non-privileged sandbox should be fine": { + { + desc: "non-privileged container in non-privileged sandbox should be fine", containerPrivileged: false, sandboxPrivileged: false, expectError: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.Privileged = test.containerPrivileged sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ Privileged: test.sandboxPrivileged, @@ -439,22 +452,26 @@ func TestPrivilegedBindMount(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string privileged bool expectedSysFSRO bool expectedCgroupFSRO bool }{ - "sysfs and cgroupfs should mount as 'ro' by default": { + { + desc: "sysfs and cgroupfs should mount as 'ro' by default", expectedSysFSRO: true, expectedCgroupFSRO: true, }, - "sysfs and cgroupfs should not mount as 'ro' if privileged": { + { + desc: "sysfs and cgroupfs should not mount as 'ro' if privileged", privileged: true, expectedSysFSRO: false, expectedCgroupFSRO: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.Privileged = test.privileged sandboxConfig.Linux.SecurityContext.Privileged = test.privileged @@ -498,13 +515,15 @@ func TestMountPropagation(t *testing.T) { }, nil } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criMount *runtime.Mount fakeLookupMountFn func(string) (mount.Info, error) optionsCheck []string expectErr bool }{ - "HostPath should mount as 'rprivate' if propagation is MountPropagation_PROPAGATION_PRIVATE": { + { + desc: "HostPath should mount as 'rprivate' if propagation is MountPropagation_PROPAGATION_PRIVATE", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -514,7 +533,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rprivate"}, expectErr: false, }, - "HostPath should mount as 'rslave' if propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER": { + { + desc: "HostPath should mount as 'rslave' if propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -524,7 +544,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rslave"}, expectErr: false, }, - "HostPath should mount as 'rshared' if propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL": { + { + desc: "HostPath should mount as 'rshared' if propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -534,7 +555,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rshared"}, expectErr: false, }, - "HostPath should mount as 'rprivate' if propagation is illegal": { + { + desc: "HostPath should mount as 'rprivate' if propagation is illegal", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -544,7 +566,8 @@ func TestMountPropagation(t *testing.T) { optionsCheck: []string{"rbind", "rprivate"}, expectErr: false, }, - "Expect an error if HostPath isn't shared and mount propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL": { + { + desc: "Expect an error if HostPath isn't shared and mount propagation is MountPropagation_PROPAGATION_BIDIRECTIONAL", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -553,7 +576,8 @@ func TestMountPropagation(t *testing.T) { fakeLookupMountFn: slaveLookupMountFn, expectErr: true, }, - "Expect an error if HostPath isn't slave or shared and mount propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER": { + { + desc: "Expect an error if HostPath isn't slave or shared and mount propagation is MountPropagation_PROPAGATION_HOST_TO_CONTAINER", criMount: &runtime.Mount{ ContainerPath: "container-path", HostPath: "host-path", @@ -563,7 +587,8 @@ func TestMountPropagation(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.os.(*ostesting.FakeOS).LookupMountFn = test.fakeLookupMountFn config, _, _, _ := getCreateContainerTestData() @@ -590,24 +615,28 @@ func TestPidNamespace(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string pidNS runtime.NamespaceMode expected runtimespec.LinuxNamespace }{ - "node namespace mode": { + { + desc: "node namespace mode", pidNS: runtime.NamespaceMode_NODE, expected: runtimespec.LinuxNamespace{ Type: runtimespec.PIDNamespace, Path: opts.GetPIDNamespace(testPid), }, }, - "container namespace mode": { + { + desc: "container namespace mode", pidNS: runtime.NamespaceMode_CONTAINER, expected: runtimespec.LinuxNamespace{ Type: runtimespec.PIDNamespace, }, }, - "pod namespace mode": { + { + desc: "pod namespace mode", pidNS: runtime.NamespaceMode_POD, expected: runtimespec.LinuxNamespace{ Type: runtimespec.PIDNamespace, @@ -615,7 +644,8 @@ func TestPidNamespace(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{Pid: test.pidNS} spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime) require.NoError(t, err) @@ -647,7 +677,9 @@ func TestUserNamespace(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + + for _, test := range []struct { + desc string userNS *runtime.UserNamespace sandboxUserNS *runtime.UserNamespace expNS *runtimespec.LinuxNamespace @@ -656,7 +688,8 @@ func TestUserNamespace(t *testing.T) { expGIDMapping []runtimespec.LinuxIDMapping err bool }{ - "node namespace mode": { + { + desc: "node namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode_NODE}, // Expect userns to NOT be present. expNotNS: &runtimespec.LinuxNamespace{ @@ -664,7 +697,8 @@ func TestUserNamespace(t *testing.T) { Path: opts.GetUserNamespace(testPid), }, }, - "node namespace mode with mappings": { + { + desc: "node namespace mode with mappings", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_NODE, Uids: []*runtime.IDMapping{&idMap}, @@ -672,19 +706,23 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "container namespace mode": { + { + desc: "container namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode_CONTAINER}, err: true, }, - "target namespace mode": { + { + desc: "target namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode_TARGET}, err: true, }, - "unknown namespace mode": { + { + desc: "unknown namespace mode", userNS: &runtime.UserNamespace{Mode: runtime.NamespaceMode(100)}, err: true, }, - "pod namespace mode": { + { + desc: "pod namespace mode", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -697,7 +735,8 @@ func TestUserNamespace(t *testing.T) { expUIDMapping: []runtimespec.LinuxIDMapping{expIDMap}, expGIDMapping: []runtimespec.LinuxIDMapping{expIDMap}, }, - "pod namespace mode with inconsistent sandbox config (different GIDs)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different GIDs)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -710,7 +749,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with inconsistent sandbox config (different UIDs)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different UIDs)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -723,7 +763,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with inconsistent sandbox config (different len)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different len)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -736,7 +777,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with inconsistent sandbox config (different mode)": { + { + desc: "pod namespace mode with inconsistent sandbox config (different mode)", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap}, @@ -749,7 +791,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with several mappings": { + { + desc: "pod namespace mode with several mappings", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap, &idMap}, @@ -757,7 +800,8 @@ func TestUserNamespace(t *testing.T) { }, err: true, }, - "pod namespace mode with uneven mappings": { + { + desc: "pod namespace mode with uneven mappings", userNS: &runtime.UserNamespace{ Mode: runtime.NamespaceMode_POD, Uids: []*runtime.IDMapping{&idMap, &idMap}, @@ -766,9 +810,8 @@ func TestUserNamespace(t *testing.T) { err: true, }, } { - desc := desc test := test - t.Run(desc, func(t *testing.T) { + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{UsernsOptions: test.userNS} // By default, set sandbox and container config to the same (this is // required by containerSpec). However, if the test wants to test for what @@ -817,7 +860,8 @@ func TestNoDefaultRunMount(t *testing.T) { } func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string profile string privileged bool disable bool @@ -826,98 +870,119 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { defaultProfile string sp *runtime.SecurityProfile }{ - "should return error if seccomp is specified when seccomp is not supported": { + { + desc: "should return error if seccomp is specified when seccomp is not supported", profile: runtimeDefault, disable: true, expectErr: true, }, - "should not return error if seccomp is not specified when seccomp is not supported": { + { + desc: "should not return error if seccomp is not specified when seccomp is not supported", profile: "", disable: true, }, - "should not return error if seccomp is unconfined when seccomp is not supported": { + { + desc: "should not return error if seccomp is unconfined when seccomp is not supported", profile: unconfinedProfile, disable: true, }, - "should not set seccomp when privileged is true": { + { + desc: "should not set seccomp when privileged is true", profile: seccompDefaultProfile, privileged: true, }, - "should not set seccomp when seccomp is unconfined": { + { + desc: "should not set seccomp when seccomp is unconfined", profile: unconfinedProfile, }, - "should not set seccomp when seccomp is not specified": { + { + desc: "should not set seccomp when seccomp is not specified", profile: "", }, - "should set default seccomp when seccomp is runtime/default": { + { + desc: "should set default seccomp when seccomp is runtime/default", profile: runtimeDefault, specOpts: seccomp.WithDefaultProfile(), }, - "should set default seccomp when seccomp is docker/default": { + { + desc: "should set default seccomp when seccomp is docker/default", profile: dockerDefault, specOpts: seccomp.WithDefaultProfile(), }, - "should set specified profile when local profile is specified": { + { + desc: "should set specified profile when local profile is specified", profile: profileNamePrefix + "test-profile", specOpts: seccomp.WithProfile("test-profile"), }, - "should use default profile when seccomp is empty": { + { + desc: "should use default profile when seccomp is empty", defaultProfile: profileNamePrefix + "test-profile", specOpts: seccomp.WithProfile("test-profile"), }, - "should fallback to docker/default when seccomp is empty and default is runtime/default": { + { + desc: "should fallback to docker/default when seccomp is empty and default is runtime/default", defaultProfile: runtimeDefault, specOpts: seccomp.WithDefaultProfile(), }, //----------------------------------------------- // now buckets for the SecurityProfile variants //----------------------------------------------- - "sp should return error if seccomp is specified when seccomp is not supported": { + { + desc: "sp should return error if seccomp is specified when seccomp is not supported", disable: true, expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not return error if seccomp is unconfined when seccomp is not supported": { + { + desc: "sp should not return error if seccomp is unconfined when seccomp is not supported", disable: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not set seccomp when privileged is true": { + { + desc: "sp should not set seccomp when privileged is true", privileged: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not set seccomp when seccomp is unconfined": { + { + desc: "sp should not set seccomp when seccomp is unconfined", sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not set seccomp when seccomp is not specified": {}, - "sp should set default seccomp when seccomp is runtime/default": { + { + desc: "sp should not set seccomp when seccomp is not specified", + }, + { + desc: "sp should set default seccomp when seccomp is runtime/default", specOpts: seccomp.WithDefaultProfile(), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should set specified profile when local profile is specified": { + { + desc: "sp should set specified profile when local profile is specified", specOpts: seccomp.WithProfile("test-profile"), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Localhost, LocalhostRef: profileNamePrefix + "test-profile", }, }, - "sp should set specified profile when local profile is specified even without prefix": { + { + desc: "sp should set specified profile when local profile is specified even without prefix", specOpts: seccomp.WithProfile("test-profile"), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Localhost, LocalhostRef: "test-profile", }, }, - "sp should return error if specified profile is invalid": { + { + desc: "sp should return error if specified profile is invalid", expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, @@ -925,7 +990,8 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { cri := &criService{} cri.config.UnsetSeccompProfile = test.defaultProfile ssp := test.sp @@ -957,7 +1023,8 @@ func TestGenerateSeccompSecurityProfileSpecOpts(t *testing.T) { } func TestGenerateApparmorSpecOpts(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string profile string privileged bool disable bool @@ -965,103 +1032,123 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { expectErr bool sp *runtime.SecurityProfile }{ - "should return error if apparmor is specified when apparmor is not supported": { + { + desc: "should return error if apparmor is specified when apparmor is not supported", profile: runtimeDefault, disable: true, expectErr: true, }, - "should not return error if apparmor is not specified when apparmor is not supported": { + { + desc: "should not return error if apparmor is not specified when apparmor is not supported", profile: "", disable: true, }, - "should set default apparmor when apparmor is not specified": { + { + desc: "should set default apparmor when apparmor is not specified", profile: "", specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName), }, - "should not apparmor when apparmor is not specified and privileged is true": { + { + desc: "should not apparmor when apparmor is not specified and privileged is true", profile: "", privileged: true, }, - "should not return error if apparmor is unconfined when apparmor is not supported": { + { + desc: "should not return error if apparmor is unconfined when apparmor is not supported", profile: unconfinedProfile, disable: true, }, - "should not apparmor when apparmor is unconfined": { + { + desc: "should not apparmor when apparmor is unconfined", profile: unconfinedProfile, }, - "should not apparmor when apparmor is unconfined and privileged is true": { + { + desc: "should not apparmor when apparmor is unconfined and privileged is true", profile: unconfinedProfile, privileged: true, }, - "should set default apparmor when apparmor is runtime/default": { + { + desc: "should set default apparmor when apparmor is runtime/default", profile: runtimeDefault, specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName), }, - "should not apparmor when apparmor is default and privileged is true": { + { + desc: "should not apparmor when apparmor is default and privileged is true", profile: runtimeDefault, privileged: true, }, // TODO (mikebrow) add success with existing defined profile tests - "should return error when undefined local profile is specified": { + { + desc: "should return error when undefined local profile is specified", profile: profileNamePrefix + "test-profile", expectErr: true, }, - "should return error when undefined local profile is specified and privileged is true": { + { + desc: "should return error when undefined local profile is specified and privileged is true", profile: profileNamePrefix + "test-profile", privileged: true, expectErr: true, }, - "should return error if specified profile is invalid": { + { + desc: "should return error if specified profile is invalid", profile: "test-profile", expectErr: true, }, //-------------------------------------- // buckets for SecurityProfile struct //-------------------------------------- - "sp should return error if apparmor is specified when apparmor is not supported": { + { + desc: "sp should return error if apparmor is specified when apparmor is not supported", disable: true, expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not return error if apparmor is unconfined when apparmor is not supported": { + { + desc: "sp should not return error if apparmor is unconfined when apparmor is not supported", disable: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not apparmor when apparmor is unconfined": { + { + desc: "sp should not apparmor when apparmor is unconfined", sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should not apparmor when apparmor is unconfined and privileged is true": { + { + desc: "sp should not apparmor when apparmor is unconfined and privileged is true", privileged: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Unconfined, }, }, - "sp should set default apparmor when apparmor is runtime/default": { + { + desc: "sp should set default apparmor when apparmor is runtime/default", specOpts: apparmor.WithDefaultProfile(appArmorDefaultProfileName), sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should not apparmor when apparmor is default and privileged is true": { + { + desc: "sp should not apparmor when apparmor is default and privileged is true", privileged: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_RuntimeDefault, }, }, - "sp should return error when undefined local profile is specified": { + { + desc: "sp should return error when undefined local profile is specified", expectErr: true, sp: &runtime.SecurityProfile{ ProfileType: runtime.SecurityProfile_Localhost, LocalhostRef: profileNamePrefix + "test-profile", }, }, - "sp should return error when undefined local profile is specified even without prefix": { + { + desc: "sp should return error when undefined local profile is specified even without prefix", profile: profileNamePrefix + "test-profile", expectErr: true, sp: &runtime.SecurityProfile{ @@ -1069,7 +1156,8 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { LocalhostRef: "test-profile", }, }, - "sp should return error when undefined local profile is specified and privileged is true": { + { + desc: "sp should return error when undefined local profile is specified and privileged is true", privileged: true, expectErr: true, sp: &runtime.SecurityProfile{ @@ -1078,7 +1166,8 @@ func TestGenerateApparmorSpecOpts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { asp := test.sp csp, err := generateApparmorSecurityProfile(test.profile) if err != nil { @@ -1117,7 +1206,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { defaultSpec, err := oci.GenerateSpec(ctrdutil.NamespacedContext(), nil, &containers.Container{ID: testID}) require.NoError(t, err) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string disableProcMount bool masked []string readonly []string @@ -1125,7 +1215,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { expectedReadonly []string privileged bool }{ - "should apply default if not specified when disable_proc_mount = true": { + { + desc: "should apply default if not specified when disable_proc_mount = true", disableProcMount: true, masked: nil, readonly: nil, @@ -1133,7 +1224,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { expectedReadonly: defaultSpec.Linux.ReadonlyPaths, privileged: false, }, - "should apply default if not specified when disable_proc_mount = false": { + { + desc: "should apply default if not specified when disable_proc_mount = false", disableProcMount: false, masked: nil, readonly: nil, @@ -1141,33 +1233,38 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { expectedReadonly: []string{}, privileged: false, }, - "should be able to specify empty paths": { + { + desc: "should be able to specify empty paths", masked: []string{}, readonly: []string{}, expectedMasked: []string{}, expectedReadonly: []string{}, privileged: false, }, - "should apply CRI specified paths": { + { + desc: "should apply CRI specified paths", masked: []string{"/proc"}, readonly: []string{"/sys"}, expectedMasked: []string{"/proc"}, expectedReadonly: []string{"/sys"}, privileged: false, }, - "default should be nil for privileged": { + { + desc: "default should be nil for privileged", expectedMasked: nil, expectedReadonly: nil, privileged: true, }, - "should be able to specify empty paths, esp. if privileged": { + { + desc: "should be able to specify empty paths, esp. if privileged", masked: []string{}, readonly: []string{}, expectedMasked: nil, expectedReadonly: nil, privileged: true, }, - "should not apply CRI specified paths if privileged": { + { + desc: "should not apply CRI specified paths if privileged", masked: []string{"/proc"}, readonly: []string{"/sys"}, expectedMasked: nil, @@ -1175,7 +1272,8 @@ func TestMaskedAndReadonlyPaths(t *testing.T) { privileged: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c.config.DisableProcMount = test.disableProcMount containerConfig.Linux.SecurityContext.MaskedPaths = test.masked containerConfig.Linux.SecurityContext.ReadonlyPaths = test.readonly @@ -1205,28 +1303,33 @@ func TestHostname(t *testing.T) { c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) { return "real-hostname", nil } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string hostname string networkNs runtime.NamespaceMode expectedEnv string }{ - "should add HOSTNAME=sandbox.Hostname for pod network namespace": { + { + desc: "should add HOSTNAME=sandbox.Hostname for pod network namespace", hostname: "test-hostname", networkNs: runtime.NamespaceMode_POD, expectedEnv: "HOSTNAME=test-hostname", }, - "should add HOSTNAME=sandbox.Hostname for host network namespace": { + { + desc: "should add HOSTNAME=sandbox.Hostname for host network namespace", hostname: "test-hostname", networkNs: runtime.NamespaceMode_NODE, expectedEnv: "HOSTNAME=test-hostname", }, - "should add HOSTNAME=os.Hostname for host network namespace if sandbox.Hostname is not set": { + { + desc: "should add HOSTNAME=os.Hostname for host network namespace if sandbox.Hostname is not set", hostname: "", networkNs: runtime.NamespaceMode_NODE, expectedEnv: "HOSTNAME=real-hostname", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { sandboxConfig.Hostname = test.hostname sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{Network: test.networkNs}, @@ -1357,12 +1460,14 @@ additional-group-for-root:x:22222:root os.WriteFile(filepath.Join(tempRootDir, "etc", "group"), []byte(etcGroup), 0644), ) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string imageConfigUser string securityContext *runtime.LinuxContainerSecurityContext expected runtimespec.User }{ - "Only SecurityContext was set, SecurityContext defines User": { + { + desc: "Only SecurityContext was set, SecurityContext defines User", securityContext: &runtime.LinuxContainerSecurityContext{ RunAsUser: &runtime.Int64Value{Value: 1000}, RunAsGroup: &runtime.Int64Value{Value: 2000}, @@ -1370,12 +1475,14 @@ additional-group-for-root:x:22222:root }, expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}}, }, - "Only imageConfig.User was set, imageConfig.User defines User": { + { + desc: "Only imageConfig.User was set, imageConfig.User defines User", imageConfigUser: "1000", securityContext: nil, expected: runtimespec.User{UID: 1000, GID: 1000, AdditionalGids: []uint32{1000, 11111}}, }, - "Both SecurityContext and ImageConfig.User was set, SecurityContext defines User": { + { + desc: "Both SecurityContext and ImageConfig.User was set, SecurityContext defines User", imageConfigUser: "0", securityContext: &runtime.LinuxContainerSecurityContext{ RunAsUser: &runtime.Int64Value{Value: 1000}, @@ -1384,12 +1491,13 @@ additional-group-for-root:x:22222:root }, expected: runtimespec.User{UID: 1000, GID: 2000, AdditionalGids: []uint32{2000, 3333, 11111}}, }, - "No SecurityContext nor ImageConfig.User were set, runtime default defines User": { + { + desc: "No SecurityContext nor ImageConfig.User were set, runtime default defines User", expected: runtimespec.User{UID: 0, GID: 0, AdditionalGids: []uint32{0, 22222}}, }, } { - desc := desc - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() containerConfig.Linux.SecurityContext = test.securityContext imageConfig.User = test.imageConfigUser @@ -1419,32 +1527,37 @@ func TestNonRootUserAndDevices(t *testing.T) { testDevice := hostDevicesRaw[0] - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string uid, gid *runtime.Int64Value deviceOwnershipFromSecurityContext bool expectedDeviceUID uint32 expectedDeviceGID uint32 }{ - "expect non-root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled": { + { + desc: "expect non-root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled", uid: &runtime.Int64Value{Value: 1}, gid: &runtime.Int64Value{Value: 10}, expectedDeviceUID: *testDevice.UID, expectedDeviceGID: *testDevice.GID, }, - "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled": { + { + desc: "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is disabled", uid: &runtime.Int64Value{Value: 0}, gid: &runtime.Int64Value{Value: 0}, expectedDeviceUID: *testDevice.UID, expectedDeviceGID: *testDevice.GID, }, - "expect non-root container's Devices Uid/Gid to be the same as RunAsUser/RunAsGroup when deviceOwnershipFromSecurityContext is enabled": { + { + desc: "expect non-root container's Devices Uid/Gid to be the same as RunAsUser/RunAsGroup when deviceOwnershipFromSecurityContext is enabled", uid: &runtime.Int64Value{Value: 1}, gid: &runtime.Int64Value{Value: 10}, deviceOwnershipFromSecurityContext: true, expectedDeviceUID: 1, expectedDeviceGID: 10, }, - "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is enabled": { + { + desc: "expect root container's Devices Uid/Gid to be the same as the device Uid/Gid on the host when deviceOwnershipFromSecurityContext is enabled", uid: &runtime.Int64Value{Value: 0}, gid: &runtime.Int64Value{Value: 0}, deviceOwnershipFromSecurityContext: true, @@ -1452,7 +1565,8 @@ func TestNonRootUserAndDevices(t *testing.T) { expectedDeviceGID: *testDevice.GID, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c.config.DeviceOwnershipFromSecurityContext = test.deviceOwnershipFromSecurityContext containerConfig.Linux.SecurityContext.RunAsUser = test.uid containerConfig.Linux.SecurityContext.RunAsGroup = test.gid @@ -1480,42 +1594,48 @@ func TestPrivilegedDevices(t *testing.T) { testContainerName := "container-name" containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string privileged bool privilegedWithoutHostDevices bool privilegedWithoutHostDevicesAllDevicesAllowed bool expectHostDevices bool expectAllDevicesAllowed bool }{ - "expect no host devices when privileged is false": { + { + desc: "expect no host devices when privileged is false", privileged: false, privilegedWithoutHostDevices: false, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: false, expectAllDevicesAllowed: false, }, - "expect no host devices when privileged is false and privilegedWithoutHostDevices is true": { + { + desc: "expect no host devices when privileged is false and privilegedWithoutHostDevices is true", privileged: false, privilegedWithoutHostDevices: true, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: false, expectAllDevicesAllowed: false, }, - "expect host devices and all device allowlist when privileged is true": { + { + desc: "expect host devices and all device allowlist when privileged is true", privileged: true, privilegedWithoutHostDevices: false, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: true, expectAllDevicesAllowed: true, }, - "expect no host devices when privileged is true and privilegedWithoutHostDevices is true": { + { + desc: "expect no host devices when privileged is true and privilegedWithoutHostDevices is true", privileged: true, privilegedWithoutHostDevices: true, privilegedWithoutHostDevicesAllDevicesAllowed: false, expectHostDevices: false, expectAllDevicesAllowed: false, }, - "expect host devices and all devices allowlist when privileged is true and privilegedWithoutHostDevices is true and privilegedWithoutHostDevicesAllDevicesAllowed is true": { + { + desc: "expect host devices and all devices allowlist when privileged is true and privilegedWithoutHostDevices is true and privilegedWithoutHostDevicesAllDevicesAllowed is true", privileged: true, privilegedWithoutHostDevices: true, privilegedWithoutHostDevicesAllDevicesAllowed: true, @@ -1523,7 +1643,8 @@ func TestPrivilegedDevices(t *testing.T) { expectAllDevicesAllowed: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Linux.SecurityContext.Privileged = test.privileged sandboxConfig.Linux.SecurityContext.Privileged = test.privileged diff --git a/pkg/cri/sbserver/container_create_test.go b/pkg/cri/sbserver/container_create_test.go index 778d40b47..5c7d47075 100644 --- a/pkg/cri/sbserver/container_create_test.go +++ b/pkg/cri/sbserver/container_create_test.go @@ -88,18 +88,21 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { testContainerName := "container-name" testPid := uint32(1234) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string podAnnotations []string configChange func(*runtime.PodSandboxConfig) specCheck func(*testing.T, *runtimespec.Spec) }{ - "a passthrough annotation should be passed as an OCI annotation": { + { + desc: "a passthrough annotation should be passed as an OCI annotation", podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") }, }, - "a non-passthrough annotation should not be passed as an OCI annotation": { + { + desc: "a non-passthrough annotation should not be passed as an OCI annotation", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["d"] = "e" }, @@ -110,7 +113,8 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "passthrough annotations should support wildcard match": { + { + desc: "passthrough annotations should support wildcard match", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["t.f"] = "j" c.Annotations["z.g"] = "o" @@ -131,7 +135,8 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() if test.configChange != nil { @@ -154,7 +159,8 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) { } func TestContainerSpecCommand(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criEntrypoint []string criArgs []string imageEntrypoint []string @@ -162,42 +168,49 @@ func TestContainerSpecCommand(t *testing.T) { expected []string expectErr bool }{ - "should use cri entrypoint if it's specified": { + { + desc: "should use cri entrypoint if it's specified", criEntrypoint: []string{"a", "b"}, imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"a", "b"}, }, - "should use cri entrypoint if it's specified even if it's empty": { + { + desc: "should use cri entrypoint if it's specified even if it's empty", criEntrypoint: []string{}, criArgs: []string{"a", "b"}, imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"a", "b"}, }, - "should use cri entrypoint and args if they are specified": { + { + desc: "should use cri entrypoint and args if they are specified", criEntrypoint: []string{"a", "b"}, criArgs: []string{"c", "d"}, imageEntrypoint: []string{"e", "f"}, imageArgs: []string{"g", "h"}, expected: []string{"a", "b", "c", "d"}, }, - "should use image entrypoint if cri entrypoint is not specified": { + { + desc: "should use image entrypoint if cri entrypoint is not specified", criArgs: []string{"a", "b"}, imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"c", "d", "a", "b"}, }, - "should use image args if both cri entrypoint and args are not specified": { + { + desc: "should use image args if both cri entrypoint and args are not specified", imageEntrypoint: []string{"c", "d"}, imageArgs: []string{"e", "f"}, expected: []string{"c", "d", "e", "f"}, }, - "should return error if both entrypoint and args are empty": { + { + desc: "should return error if both entrypoint and args are empty", expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config, _, imageConfig, _ := getCreateContainerTestData() config.Command = test.criEntrypoint config.Args = test.criArgs @@ -211,19 +224,21 @@ func TestContainerSpecCommand(t *testing.T) { return } assert.NoError(t, err) - assert.Equal(t, test.expected, spec.Process.Args, desc) + assert.Equal(t, test.expected, spec.Process.Args, test.desc) }) } } func TestVolumeMounts(t *testing.T) { testContainerRootDir := "test-container-root" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criMounts []*runtime.Mount imageVolumes map[string]struct{} expectedMountDest []string }{ - "should setup rw mount for image volumes": { + { + desc: "should setup rw mount for image volumes", imageVolumes: map[string]struct{}{ "/test-volume-1": {}, "/test-volume-2": {}, @@ -233,7 +248,8 @@ func TestVolumeMounts(t *testing.T) { "/test-volume-2", }, }, - "should skip image volumes if already mounted by CRI": { + { + desc: "should skip image volumes if already mounted by CRI", criMounts: []*runtime.Mount{ { ContainerPath: "/test-volume-1", @@ -248,7 +264,8 @@ func TestVolumeMounts(t *testing.T) { "/test-volume-2", }, }, - "should compare and return cleanpath": { + { + desc: "should compare and return cleanpath", criMounts: []*runtime.Mount{ { ContainerPath: "/test-volume-1", @@ -264,7 +281,8 @@ func TestVolumeMounts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config := &imagespec.ImageConfig{ Volumes: test.imageVolumes, } @@ -301,14 +319,16 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { testContainerName := "container-name" testPid := uint32(1234) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string podAnnotations []string containerAnnotations []string podConfigChange func(*runtime.PodSandboxConfig) configChange func(*runtime.ContainerConfig) specCheck func(*testing.T, *runtimespec.Spec) }{ - "passthrough annotations from pod and container should be passed as an OCI annotation": { + { + desc: "passthrough annotations from pod and container should be passed as an OCI annotation", podConfigChange: func(p *runtime.PodSandboxConfig) { p.Annotations["pod.annotation.1"] = "1" p.Annotations["pod.annotation.2"] = "2" @@ -334,7 +354,8 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "passthrough annotations from pod and container should support wildcard": { + { + desc: "passthrough annotations from pod and container should support wildcard", podConfigChange: func(p *runtime.PodSandboxConfig) { p.Annotations["pod.annotation.1"] = "1" p.Annotations["pod.annotation.2"] = "2" @@ -356,7 +377,8 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { assert.Equal(t, "3", spec.Annotations["pod.annotation.3"]) }, }, - "annotations should not pass through if no passthrough annotations are configured": { + { + desc: "annotations should not pass through if no passthrough annotations are configured", podConfigChange: func(p *runtime.PodSandboxConfig) { p.Annotations["pod.annotation.1"] = "1" p.Annotations["pod.annotation.2"] = "2" @@ -385,7 +407,8 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData() if test.configChange != nil { @@ -441,13 +464,15 @@ func TestBaseRuntimeSpec(t *testing.T) { func TestLinuxContainerMounts(t *testing.T) { const testSandboxID = "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string statFn func(string) (os.FileInfo, error) criMounts []*runtime.Mount securityContext *runtime.LinuxContainerSecurityContext expectedMounts []*runtime.Mount }{ - "should setup ro mount when rootfs is read-only": { + { + desc: "should setup ro mount when rootfs is read-only", securityContext: &runtime.LinuxContainerSecurityContext{ ReadonlyRootfs: true, }, @@ -478,7 +503,8 @@ func TestLinuxContainerMounts(t *testing.T) { }, }, }, - "should setup rw mount when rootfs is read-write": { + { + desc: "should setup rw mount when rootfs is read-write", securityContext: &runtime.LinuxContainerSecurityContext{}, expectedMounts: []*runtime.Mount{ { @@ -507,7 +533,8 @@ func TestLinuxContainerMounts(t *testing.T) { }, }, }, - "should use host /dev/shm when host ipc is set": { + { + desc: "should use host /dev/shm when host ipc is set", securityContext: &runtime.LinuxContainerSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{Ipc: runtime.NamespaceMode_NODE}, }, @@ -537,7 +564,8 @@ func TestLinuxContainerMounts(t *testing.T) { }, }, }, - "should skip container mounts if already mounted by CRI": { + { + desc: "should skip container mounts if already mounted by CRI", criMounts: []*runtime.Mount{ { ContainerPath: "/etc/hostname", @@ -559,7 +587,8 @@ func TestLinuxContainerMounts(t *testing.T) { securityContext: &runtime.LinuxContainerSecurityContext{}, expectedMounts: nil, }, - "should skip hostname mount if the old sandbox doesn't have hostname file": { + { + desc: "should skip hostname mount if the old sandbox doesn't have hostname file", statFn: func(path string) (os.FileInfo, error) { assert.Equal(t, filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"), path) return nil, errors.New("random error") @@ -587,7 +616,8 @@ func TestLinuxContainerMounts(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config := &runtime.ContainerConfig{ Metadata: &runtime.ContainerMetadata{ Name: "test-name", @@ -601,7 +631,7 @@ func TestLinuxContainerMounts(t *testing.T) { c := newTestCRIService() c.os.(*ostesting.FakeOS).StatFn = test.statFn mounts := c.linuxContainerMounts(testSandboxID, config) - assert.Equal(t, test.expectedMounts, mounts, desc) + assert.Equal(t, test.expectedMounts, mounts, test.desc) }) } } diff --git a/pkg/cri/sbserver/container_create_windows_test.go b/pkg/cri/sbserver/container_create_windows_test.go index 2c950eb09..cb20823a4 100644 --- a/pkg/cri/sbserver/container_create_windows_test.go +++ b/pkg/cri/sbserver/container_create_windows_test.go @@ -208,33 +208,39 @@ func TestHostProcessRequirements(t *testing.T) { containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData() ociRuntime := config.Runtime{} c := newTestCRIService() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string containerHostProcess bool sandboxHostProcess bool expectError bool }{ - "hostprocess container in non-hostprocess sandbox should fail": { + { + desc: "hostprocess container in non-hostprocess sandbox should fail", containerHostProcess: true, sandboxHostProcess: false, expectError: true, }, - "hostprocess container in hostprocess sandbox should be fine": { + { + desc: "hostprocess container in hostprocess sandbox should be fine", containerHostProcess: true, sandboxHostProcess: true, expectError: false, }, - "non-hostprocess container in hostprocess sandbox should fail": { + { + desc: "non-hostprocess container in hostprocess sandbox should fail", containerHostProcess: false, sandboxHostProcess: true, expectError: true, }, - "non-hostprocess container in non-hostprocess sandbox should be fine": { + { + desc: "non-hostprocess container in non-hostprocess sandbox should be fine", containerHostProcess: false, sandboxHostProcess: false, expectError: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { containerConfig.Windows.SecurityContext.HostProcess = test.containerHostProcess sandboxConfig.Windows.SecurityContext = &runtime.WindowsSandboxSecurityContext{ HostProcess: test.sandboxHostProcess, diff --git a/pkg/cri/sbserver/container_list_test.go b/pkg/cri/sbserver/container_list_test.go index 58b26fb2d..ce4f59bd1 100644 --- a/pkg/cri/sbserver/container_list_test.go +++ b/pkg/cri/sbserver/container_list_test.go @@ -101,18 +101,22 @@ func TestFilterContainers(t *testing.T) { Labels: map[string]string{"c": "d"}, }, } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string filter *runtime.ContainerFilter expect []*runtime.Container }{ - "no filter": { + { + desc: "no filter", expect: testContainers, }, - "id filter": { + { + desc: "id filter", filter: &runtime.ContainerFilter{Id: "2"}, expect: []*runtime.Container{testContainers[1]}, }, - "state filter": { + { + desc: "state filter", filter: &runtime.ContainerFilter{ State: &runtime.ContainerStateValue{ State: runtime.ContainerState_CONTAINER_EXITED, @@ -120,17 +124,20 @@ func TestFilterContainers(t *testing.T) { }, expect: []*runtime.Container{testContainers[1]}, }, - "label filter": { + { + desc: "label filter", filter: &runtime.ContainerFilter{ LabelSelector: map[string]string{"a": "b"}, }, expect: []*runtime.Container{testContainers[1]}, }, - "sandbox id filter": { + { + desc: "sandbox id filter", filter: &runtime.ContainerFilter{PodSandboxId: "s-2"}, expect: []*runtime.Container{testContainers[1], testContainers[2]}, }, - "mixed filter not matched": { + { + desc: "mixed filter not matched", filter: &runtime.ContainerFilter{ Id: "1", PodSandboxId: "s-2", @@ -138,7 +145,8 @@ func TestFilterContainers(t *testing.T) { }, expect: []*runtime.Container{}, }, - "mixed filter matched": { + { + desc: "mixed filter matched", filter: &runtime.ContainerFilter{ PodSandboxId: "s-2", State: &runtime.ContainerStateValue{ @@ -149,9 +157,10 @@ func TestFilterContainers(t *testing.T) { expect: []*runtime.Container{testContainers[2]}, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { filtered := c.filterCRIContainers(testContainers, test.filter) - assert.Equal(t, test.expect, filtered, desc) + assert.Equal(t, test.expect, filtered, test.desc) }) } } @@ -287,46 +296,54 @@ func TestListContainers(t *testing.T) { assert.NoError(t, c.containerStore.Add(container)) } - for testdesc, testdata := range map[string]struct { + for _, testdata := range []struct { + desc string filter *runtime.ContainerFilter expect []*runtime.Container }{ - "test without filter": { + { + desc: "test without filter", filter: &runtime.ContainerFilter{}, expect: expectedContainers, }, - "test filter by sandboxid": { + { + desc: "test filter by sandboxid", filter: &runtime.ContainerFilter{ PodSandboxId: "s-1abcdef1234", }, expect: expectedContainers[:3], }, - "test filter by truncated sandboxid": { + { + desc: "test filter by truncated sandboxid", filter: &runtime.ContainerFilter{ PodSandboxId: "s-1", }, expect: expectedContainers[:3], }, - "test filter by containerid": { + { + desc: "test filter by containerid", filter: &runtime.ContainerFilter{ Id: "c-1container", }, expect: expectedContainers[:1], }, - "test filter by truncated containerid": { + { + desc: "test filter by truncated containerid", filter: &runtime.ContainerFilter{ Id: "c-1", }, expect: expectedContainers[:1], }, - "test filter by containerid and sandboxid": { + { + desc: "test filter by containerid and sandboxid", filter: &runtime.ContainerFilter{ Id: "c-1container", PodSandboxId: "s-1abcdef1234", }, expect: expectedContainers[:1], }, - "test filter by truncated containerid and truncated sandboxid": { + { + desc: "test filter by truncated containerid and truncated sandboxid", filter: &runtime.ContainerFilter{ Id: "c-1", PodSandboxId: "s-1", @@ -334,7 +351,8 @@ func TestListContainers(t *testing.T) { expect: expectedContainers[:1], }, } { - t.Run(testdesc, func(t *testing.T) { + testdata := testdata + t.Run(testdata.desc, func(t *testing.T) { resp, err := c.ListContainers(context.Background(), &runtime.ListContainersRequest{Filter: testdata.filter}) assert.NoError(t, err) require.NotNil(t, resp) diff --git a/pkg/cri/sbserver/container_remove_test.go b/pkg/cri/sbserver/container_remove_test.go index c3b049299..e00278b82 100644 --- a/pkg/cri/sbserver/container_remove_test.go +++ b/pkg/cri/sbserver/container_remove_test.go @@ -29,25 +29,29 @@ import ( // state correctly. func TestSetContainerRemoving(t *testing.T) { testID := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string status containerstore.Status expectErr bool }{ - "should return error when container is in running state": { + { + desc: "should return error when container is in running state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, expectErr: true, }, - "should return error when container is in starting state": { + { + desc: "should return error when container is in starting state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), Starting: true, }, expectErr: true, }, - "should return error when container is in removing state": { + { + desc: "should return error when container is in removing state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -56,7 +60,8 @@ func TestSetContainerRemoving(t *testing.T) { }, expectErr: true, }, - "should not return error when container is not running and removing": { + { + desc: "should not return error when container is not running and removing", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -65,7 +70,8 @@ func TestSetContainerRemoving(t *testing.T) { expectErr: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: testID}, containerstore.WithFakeStatus(test.status), diff --git a/pkg/cri/sbserver/container_start_test.go b/pkg/cri/sbserver/container_start_test.go index 91184cd74..79340e921 100644 --- a/pkg/cri/sbserver/container_start_test.go +++ b/pkg/cri/sbserver/container_start_test.go @@ -29,25 +29,28 @@ import ( // state correctly. func TestSetContainerStarting(t *testing.T) { testID := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string status containerstore.Status expectErr bool }{ - - "should not return error when container is in created state": { + { + desc: "should not return error when container is in created state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), }, expectErr: false, }, - "should return error when container is in running state": { + { + desc: "should return error when container is in running state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), }, expectErr: true, }, - "should return error when container is in exited state": { + { + desc: "should return error when container is in exited state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -55,7 +58,8 @@ func TestSetContainerStarting(t *testing.T) { }, expectErr: true, }, - "should return error when container is in unknown state": { + { + desc: "should return error when container is in unknown state", status: containerstore.Status{ CreatedAt: 0, StartedAt: 0, @@ -63,14 +67,16 @@ func TestSetContainerStarting(t *testing.T) { }, expectErr: true, }, - "should return error when container is in starting state": { + { + desc: "should return error when container is in starting state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), Starting: true, }, expectErr: true, }, - "should return error when container is in removing state": { + { + desc: "should return error when container is in removing state", status: containerstore.Status{ CreatedAt: time.Now().UnixNano(), Removing: true, @@ -78,7 +84,8 @@ func TestSetContainerStarting(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: testID}, containerstore.WithFakeStatus(test.status), diff --git a/pkg/cri/sbserver/container_stats_list_linux_test.go b/pkg/cri/sbserver/container_stats_list_linux_test.go index 32ff508a6..2b124cf81 100644 --- a/pkg/cri/sbserver/container_stats_list_linux_test.go +++ b/pkg/cri/sbserver/container_stats_list_linux_test.go @@ -28,22 +28,26 @@ import ( ) func TestGetWorkingSet(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v1.MemoryStat expected uint64 }{ - "nil memory usage": { + { + desc: "nil memory usage", memory: &v1.MemoryStat{}, expected: 0, }, - "memory usage higher than inactive_total_file": { + { + desc: "memory usage higher than inactive_total_file", memory: &v1.MemoryStat{ TotalInactiveFile: 1000, Usage: &v1.MemoryEntry{Usage: 2000}, }, expected: 1000, }, - "memory usage lower than inactive_total_file": { + { + desc: "memory usage lower than inactive_total_file", memory: &v1.MemoryStat{ TotalInactiveFile: 2000, Usage: &v1.MemoryEntry{Usage: 1000}, @@ -51,7 +55,8 @@ func TestGetWorkingSet(t *testing.T) { expected: 0, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getWorkingSet(test.memory) assert.Equal(t, test.expected, got) }) @@ -59,22 +64,26 @@ func TestGetWorkingSet(t *testing.T) { } func TestGetWorkingSetV2(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v2.MemoryStat expected uint64 }{ - "nil memory usage": { + { + desc: "nil memory usage", memory: &v2.MemoryStat{}, expected: 0, }, - "memory usage higher than inactive_total_file": { + { + desc: "memory usage higher than inactive_total_file", memory: &v2.MemoryStat{ InactiveFile: 1000, Usage: 2000, }, expected: 1000, }, - "memory usage lower than inactive_total_file": { + { + desc: "memory usage lower than inactive_total_file", memory: &v2.MemoryStat{ InactiveFile: 2000, Usage: 1000, @@ -82,7 +91,8 @@ func TestGetWorkingSetV2(t *testing.T) { expected: 0, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getWorkingSetV2(test.memory) assert.Equal(t, test.expected, got) }) @@ -90,13 +100,14 @@ func TestGetWorkingSetV2(t *testing.T) { } func TestGetAvailableBytes(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v1.MemoryStat workingSetBytes uint64 expected uint64 }{ - - "no limit": { + { + desc: "no limit", memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ Limit: math.MaxUint64, // no limit @@ -106,7 +117,8 @@ func TestGetAvailableBytes(t *testing.T) { workingSetBytes: 500, expected: 0, }, - "with limit": { + { + desc: "with limit", memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ Limit: 5000, @@ -117,7 +129,8 @@ func TestGetAvailableBytes(t *testing.T) { expected: 5000 - 500, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getAvailableBytes(test.memory, test.workingSetBytes) assert.Equal(t, test.expected, got) }) @@ -125,13 +138,14 @@ func TestGetAvailableBytes(t *testing.T) { } func TestGetAvailableBytesV2(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string memory *v2.MemoryStat workingSetBytes uint64 expected uint64 }{ - - "no limit": { + { + desc: "no limit", memory: &v2.MemoryStat{ UsageLimit: math.MaxUint64, // no limit Usage: 1000, @@ -139,7 +153,8 @@ func TestGetAvailableBytesV2(t *testing.T) { workingSetBytes: 500, expected: 0, }, - "with limit": { + { + desc: "with limit", memory: &v2.MemoryStat{ UsageLimit: 5000, Usage: 1000, @@ -148,7 +163,8 @@ func TestGetAvailableBytesV2(t *testing.T) { expected: 5000 - 500, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getAvailableBytesV2(test.memory, test.workingSetBytes) assert.Equal(t, test.expected, got) }) @@ -159,11 +175,13 @@ func TestContainerMetricsMemory(t *testing.T) { c := newTestCRIService() timestamp := time.Now() - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string metrics interface{} expected *runtime.MemoryUsage }{ - "v1 metrics - no memory limit": { + { + desc: "v1 metrics - no memory limit", metrics: &v1.Metrics{ Memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ @@ -186,7 +204,8 @@ func TestContainerMetricsMemory(t *testing.T) { MajorPageFaults: &runtime.UInt64Value{Value: 12}, }, }, - "v1 metrics - memory limit": { + { + desc: "v1 metrics - memory limit", metrics: &v1.Metrics{ Memory: &v1.MemoryStat{ Usage: &v1.MemoryEntry{ @@ -209,7 +228,8 @@ func TestContainerMetricsMemory(t *testing.T) { MajorPageFaults: &runtime.UInt64Value{Value: 12}, }, }, - "v2 metrics - memory limit": { + { + desc: "v2 metrics - memory limit", metrics: &v2.Metrics{ Memory: &v2.MemoryStat{ Usage: 1000, @@ -229,7 +249,8 @@ func TestContainerMetricsMemory(t *testing.T) { MajorPageFaults: &runtime.UInt64Value{Value: 12}, }, }, - "v2 metrics - no memory limit": { + { + desc: "v2 metrics - no memory limit", metrics: &v2.Metrics{ Memory: &v2.MemoryStat{ Usage: 1000, @@ -250,7 +271,8 @@ func TestContainerMetricsMemory(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got, err := c.memoryContainerStats("ID", test.metrics, timestamp) assert.NoError(t, err) assert.Equal(t, test.expected, got) diff --git a/pkg/cri/sbserver/container_stats_list_test.go b/pkg/cri/sbserver/container_stats_list_test.go index 92e4e960a..0ff2e7b06 100644 --- a/pkg/cri/sbserver/container_stats_list_test.go +++ b/pkg/cri/sbserver/container_stats_list_test.go @@ -30,20 +30,23 @@ func TestContainerMetricsCPUNanoCoreUsage(t *testing.T) { secondAfterTimeStamp := timestamp.Add(time.Second) ID := "ID" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string firstCPUValue uint64 secondCPUValue uint64 expectedNanoCoreUsageFirst uint64 expectedNanoCoreUsageSecond uint64 }{ - "metrics": { + { + desc: "metrics", firstCPUValue: 50, secondCPUValue: 500, expectedNanoCoreUsageFirst: 0, expectedNanoCoreUsageSecond: 450, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: ID}, ) diff --git a/pkg/cri/sbserver/container_status_test.go b/pkg/cri/sbserver/container_status_test.go index ec67d27f9..88e6c02b9 100644 --- a/pkg/cri/sbserver/container_status_test.go +++ b/pkg/cri/sbserver/container_status_test.go @@ -88,7 +88,8 @@ func getContainerStatusTestData() (*containerstore.Metadata, *containerstore.Sta } func TestToCRIContainerStatus(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string startedAt int64 finishedAt int64 exitCode int32 @@ -97,14 +98,17 @@ func TestToCRIContainerStatus(t *testing.T) { expectedState runtime.ContainerState expectedReason string }{ - "container created": { + { + desc: "container created", expectedState: runtime.ContainerState_CONTAINER_CREATED, }, - "container running": { + { + desc: "container running", startedAt: time.Now().UnixNano(), expectedState: runtime.ContainerState_CONTAINER_RUNNING, }, - "container exited with reason": { + { + desc: "container exited with reason", startedAt: time.Now().UnixNano(), finishedAt: time.Now().UnixNano(), exitCode: 1, @@ -113,7 +117,8 @@ func TestToCRIContainerStatus(t *testing.T) { expectedState: runtime.ContainerState_CONTAINER_EXITED, expectedReason: "test-reason", }, - "container exited with exit code 0 without reason": { + { + desc: "container exited with exit code 0 without reason", startedAt: time.Now().UnixNano(), finishedAt: time.Now().UnixNano(), exitCode: 0, @@ -121,7 +126,8 @@ func TestToCRIContainerStatus(t *testing.T) { expectedState: runtime.ContainerState_CONTAINER_EXITED, expectedReason: completeExitReason, }, - "container exited with non-zero exit code without reason": { + { + desc: "container exited with non-zero exit code without reason", startedAt: time.Now().UnixNano(), finishedAt: time.Now().UnixNano(), exitCode: 1, @@ -130,7 +136,8 @@ func TestToCRIContainerStatus(t *testing.T) { expectedReason: errorExitReason, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { metadata, status, _, expected := getContainerStatusTestData() // Update status with test case. @@ -154,7 +161,7 @@ func TestToCRIContainerStatus(t *testing.T) { containerStatus := toCRIContainerStatus(container, expected.Image, expected.ImageRef) - assert.Equal(t, expected, containerStatus, desc) + assert.Equal(t, expected, containerStatus, test.desc) }) } } @@ -176,7 +183,8 @@ func TestToCRIContainerInfo(t *testing.T) { } func TestContainerStatus(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string exist bool imageExist bool startedAt int64 @@ -185,18 +193,21 @@ func TestContainerStatus(t *testing.T) { expectedState runtime.ContainerState expectErr bool }{ - "container created": { + { + desc: "container created", exist: true, imageExist: true, expectedState: runtime.ContainerState_CONTAINER_CREATED, }, - "container running": { + { + desc: "container running", exist: true, imageExist: true, startedAt: time.Now().UnixNano(), expectedState: runtime.ContainerState_CONTAINER_RUNNING, }, - "container exited": { + { + desc: "container exited", exist: true, imageExist: true, startedAt: time.Now().UnixNano(), @@ -204,18 +215,21 @@ func TestContainerStatus(t *testing.T) { reason: "test-reason", expectedState: runtime.ContainerState_CONTAINER_EXITED, }, - "container not exist": { + { + desc: "container not exist", exist: false, imageExist: true, expectErr: true, }, - "image not exist": { + { + desc: "image not exist", exist: false, imageExist: false, expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() metadata, status, image, expected := getContainerStatusTestData() // Update status with test case. diff --git a/pkg/cri/sbserver/container_stop_test.go b/pkg/cri/sbserver/container_stop_test.go index 70b1460ac..8dbee0d37 100644 --- a/pkg/cri/sbserver/container_stop_test.go +++ b/pkg/cri/sbserver/container_stop_test.go @@ -28,13 +28,15 @@ import ( func TestWaitContainerStop(t *testing.T) { id := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string status *containerstore.Status cancel bool timeout time.Duration expectErr bool }{ - "should return error if timeout exceeds": { + { + desc: "should return error if timeout exceeds", status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -42,7 +44,8 @@ func TestWaitContainerStop(t *testing.T) { timeout: 200 * time.Millisecond, expectErr: true, }, - "should return error if context is cancelled": { + { + desc: "should return error if context is cancelled", status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -51,7 +54,8 @@ func TestWaitContainerStop(t *testing.T) { cancel: true, expectErr: true, }, - "should not return error if container is stopped before timeout": { + { + desc: "should not return error if container is stopped before timeout", status: &containerstore.Status{ CreatedAt: time.Now().UnixNano(), StartedAt: time.Now().UnixNano(), @@ -61,7 +65,8 @@ func TestWaitContainerStop(t *testing.T) { expectErr: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() container, err := containerstore.NewContainer( containerstore.Metadata{ID: id}, @@ -81,7 +86,7 @@ func TestWaitContainerStop(t *testing.T) { ctx = timeoutCtx } err = c.waitContainerStop(ctx, container) - assert.Equal(t, test.expectErr, err != nil, desc) + assert.Equal(t, test.expectErr, err != nil, test.desc) }) } } diff --git a/pkg/cri/sbserver/container_update_resources_linux_test.go b/pkg/cri/sbserver/container_update_resources_linux_test.go index 6d209db6d..6cff864e8 100644 --- a/pkg/cri/sbserver/container_update_resources_linux_test.go +++ b/pkg/cri/sbserver/container_update_resources_linux_test.go @@ -38,13 +38,16 @@ func TestUpdateOCILinuxResource(t *testing.T) { } return nil } - for desc, test := range map[string]struct { + + for _, test := range []struct { + desc string spec *runtimespec.Spec request *runtime.UpdateContainerResourcesRequest expected *runtimespec.Spec expectErr bool }{ - "should be able to update each resource": { + { + desc: "should be able to update each resource", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -93,7 +96,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, }, - "should skip empty fields": { + { + desc: "should skip empty fields", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -139,7 +143,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, }, - "should be able to fill empty fields": { + { + desc: "should be able to fill empty fields", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -180,7 +185,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, }, - "should be able to patch the unified map": { + { + desc: "should be able to patch the unified map", spec: &runtimespec.Spec{ Process: &runtimespec.Process{OOMScoreAdj: oomscoreadj}, Linux: &runtimespec.Linux{ @@ -230,7 +236,8 @@ func TestUpdateOCILinuxResource(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { config := criconfig.Config{ PluginConfig: criconfig.PluginConfig{ TolerateMissingHugetlbController: true, diff --git a/pkg/cri/sbserver/helpers_test.go b/pkg/cri/sbserver/helpers_test.go index e5b69091a..429dc040f 100644 --- a/pkg/cri/sbserver/helpers_test.go +++ b/pkg/cri/sbserver/helpers_test.go @@ -44,36 +44,44 @@ import ( // TestGetUserFromImage tests the logic of getting image uid or user name of image user. func TestGetUserFromImage(t *testing.T) { newI64 := func(i int64) *int64 { return &i } - for c, test := range map[string]struct { + for _, test := range []struct { + desc string user string uid *int64 name string }{ - "no gid": { + { + desc: "no gid", user: "0", uid: newI64(0), }, - "uid/gid": { + { + desc: "uid/gid", user: "0:1", uid: newI64(0), }, - "empty user": { + { + desc: "empty user", user: "", }, - "multiple separators": { + { + desc: "multiple separators", user: "1:2:3", uid: newI64(1), }, - "root username": { + { + desc: "root username", user: "root:root", name: "root", }, - "username": { + { + desc: "username", user: "test:test", name: "test", }, } { - t.Run(c, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { actualUID, actualName := getUserFromImage(test.user) assert.Equal(t, test.uid, actualUID) assert.Equal(t, test.name, actualName) @@ -145,19 +153,22 @@ systemd_cgroup = true require.NoError(t, err) require.Len(t, nonNilOptsConfig.Runtimes, 3) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string r criconfig.Runtime c criconfig.Config expectedOptions interface{} }{ - "when options is nil, should return nil option for io.containerd.runc.v2": { + { + desc: "when options is nil, should return nil option for io.containerd.runc.v2", r: nilOptsConfig.Runtimes["runcv2"], c: nilOptsConfig, expectedOptions: nil, }, - "when options is not nil, should be able to decode for io.containerd.runc.v2": { - r: nonNilOptsConfig.Runtimes["runcv2"], - c: nonNilOptsConfig, + { + desc: "when options is not nil, should be able to decode for io.containerd.runc.v2", + r: nonNilOptsConfig.Runtimes["runcv2"], + c: nonNilOptsConfig, expectedOptions: &runcoptions.Options{ BinaryName: "runc", Root: "/runcv2", @@ -165,7 +176,8 @@ systemd_cgroup = true }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { opts, err := generateRuntimeOptions(test.r, test.c) assert.NoError(t, err) assert.Equal(t, test.expectedOptions, opts) @@ -174,18 +186,21 @@ systemd_cgroup = true } func TestEnvDeduplication(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string existing []string kv [][2]string expected []string }{ - "single env": { + { + desc: "single env", kv: [][2]string{ {"a", "b"}, }, expected: []string{"a=b"}, }, - "multiple envs": { + { + desc: "multiple envs", kv: [][2]string{ {"a", "b"}, {"c", "d"}, @@ -197,7 +212,8 @@ func TestEnvDeduplication(t *testing.T) { "e=f", }, }, - "env override": { + { + desc: "env override", kv: [][2]string{ {"k1", "v1"}, {"k2", "v2"}, @@ -213,7 +229,8 @@ func TestEnvDeduplication(t *testing.T) { "k4=v6", }, }, - "existing env": { + { + desc: "existing env", existing: []string{ "k1=v1", "k2=v2", @@ -232,7 +249,8 @@ func TestEnvDeduplication(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { var spec runtimespec.Spec if len(test.existing) > 0 { spec.Process = &runtimespec.Process{ @@ -248,17 +266,20 @@ func TestEnvDeduplication(t *testing.T) { } func TestPassThroughAnnotationsFilter(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string podAnnotations map[string]string runtimePodAnnotations []string passthroughAnnotations map[string]string }{ - "should support direct match": { + { + desc: "should support direct match", podAnnotations: map[string]string{"c": "d", "d": "e"}, runtimePodAnnotations: []string{"c"}, passthroughAnnotations: map[string]string{"c": "d"}, }, - "should support wildcard match": { + { + desc: "should support wildcard match", podAnnotations: map[string]string{ "t.f": "j", "z.g": "o", @@ -273,7 +294,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { "y.ca": "b", }, }, - "should support wildcard match all": { + { + desc: "should support wildcard match all", podAnnotations: map[string]string{ "t.f": "j", "z.g": "o", @@ -290,7 +312,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { "y": "b", }, }, - "should support match including path separator": { + { + desc: "should support match including path separator", podAnnotations: map[string]string{ "matchend.com/end": "1", "matchend.com/end1": "2", @@ -349,7 +372,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) }) @@ -437,28 +461,34 @@ func TestValidateTargetContainer(t *testing.T) { err = addContainer(c, testOtherContainerID, testOtherContainerSandboxID, testOtherContainerPID, createdAt, startedAt, 0) require.NoError(t, err, "error creating test container in other pod") - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string targetContainerID string expectError bool }{ - "target container in pod": { + { + desc: "target container in pod", targetContainerID: testTargetContainerID, expectError: false, }, - "target stopped container in pod": { + { + desc: "target stopped container in pod", targetContainerID: testStoppedContainerID, expectError: true, }, - "target container does not exist": { + { + desc: "target container does not exist", targetContainerID: "no-container-with-this-id", expectError: true, }, - "target container in other pod": { + { + desc: "target container in other pod", targetContainerID: testOtherContainerID, expectError: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { targetContainer, err := c.validateTargetContainer(testSandboxID, test.targetContainerID) if test.expectError { require.Error(t, err, "target should have been invalid but no error") @@ -520,6 +550,8 @@ func TestHostNetwork(t *testing.T) { if goruntime.GOOS != "linux" { t.Skip() } + + tt := tt t.Run(tt.name, func(t *testing.T) { if hostNetwork(tt.c) != tt.expected { t.Errorf("failed hostNetwork got %t expected %t", hostNetwork(tt.c), tt.expected) diff --git a/pkg/cri/sbserver/images/image_pull_test.go b/pkg/cri/sbserver/images/image_pull_test.go index f19ef2f7f..0cc004522 100644 --- a/pkg/cri/sbserver/images/image_pull_test.go +++ b/pkg/cri/sbserver/images/image_pull_test.go @@ -39,23 +39,29 @@ func TestParseAuth(t *testing.T) { base64.StdEncoding.Encode(testAuth, []byte(testUser+":"+testPasswd)) invalidAuth := make([]byte, testAuthLen) base64.StdEncoding.Encode(invalidAuth, []byte(testUser+"@"+testPasswd)) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string auth *runtime.AuthConfig host string expectedUser string expectedSecret string expectErr bool }{ - "should not return error if auth config is nil": {}, - "should not return error if empty auth is provided for access to anonymous registry": { + { + desc: "should not return error if auth config is nil", + }, + { + desc: "should not return error if empty auth is provided for access to anonymous registry", auth: &runtime.AuthConfig{}, expectErr: false, }, - "should support identity token": { + { + desc: "should support identity token", auth: &runtime.AuthConfig{IdentityToken: "abcd"}, expectedSecret: "abcd", }, - "should support username and password": { + { + desc: "should support username and password", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -63,16 +69,19 @@ func TestParseAuth(t *testing.T) { expectedUser: testUser, expectedSecret: testPasswd, }, - "should support auth": { + { + desc: "should support auth", auth: &runtime.AuthConfig{Auth: string(testAuth)}, expectedUser: testUser, expectedSecret: testPasswd, }, - "should return error for invalid auth": { + { + desc: "should return error for invalid auth", auth: &runtime.AuthConfig{Auth: string(invalidAuth)}, expectErr: true, }, - "should return empty auth if server address doesn't match": { + { + desc: "should return empty auth if server address doesn't match", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -82,7 +91,8 @@ func TestParseAuth(t *testing.T) { expectedUser: "", expectedSecret: "", }, - "should return auth if server address matches": { + { + desc: "should return auth if server address matches", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -92,7 +102,8 @@ func TestParseAuth(t *testing.T) { expectedUser: testUser, expectedSecret: testPasswd, }, - "should return auth if server address is not specified": { + { + desc: "should return auth if server address is not specified", auth: &runtime.AuthConfig{ Username: testUser, Password: testPasswd, @@ -102,7 +113,8 @@ func TestParseAuth(t *testing.T) { expectedSecret: testPasswd, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { u, s, err := ParseAuth(test.auth, test.host) assert.Equal(t, test.expectErr, err != nil) assert.Equal(t, test.expectedUser, u) @@ -112,12 +124,14 @@ func TestParseAuth(t *testing.T) { } func TestRegistryEndpoints(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string mirrors map[string]criconfig.Mirror host string expected []string }{ - "no mirror configured": { + { + desc: "no mirror configured", mirrors: map[string]criconfig.Mirror{ "registry-1.io": { Endpoints: []string{ @@ -131,7 +145,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "mirror configured": { + { + desc: "mirror configured", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -147,7 +162,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "wildcard mirror configured": { + { + desc: "wildcard mirror configured", mirrors: map[string]criconfig.Mirror{ "*": { Endpoints: []string{ @@ -163,7 +179,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "host should take precedence if both host and wildcard mirrors are configured": { + { + desc: "host should take precedence if both host and wildcard mirrors are configured", mirrors: map[string]criconfig.Mirror{ "*": { Endpoints: []string{ @@ -182,7 +199,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "default endpoint in list with http": { + { + desc: "default endpoint in list with http", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -199,7 +217,8 @@ func TestRegistryEndpoints(t *testing.T) { "http://registry-3.io", }, }, - "default endpoint in list with https": { + { + desc: "default endpoint in list with https", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -216,7 +235,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io", }, }, - "default endpoint in list with path": { + { + desc: "default endpoint in list with path", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -233,7 +253,8 @@ func TestRegistryEndpoints(t *testing.T) { "https://registry-3.io/path", }, }, - "miss scheme endpoint in list with path": { + { + desc: "miss scheme endpoint in list with path", mirrors: map[string]criconfig.Mirror{ "registry-3.io": { Endpoints: []string{ @@ -251,7 +272,8 @@ func TestRegistryEndpoints(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.config.Registry.Mirrors = test.mirrors got, err := c.registryEndpoints(test.host) @@ -262,52 +284,64 @@ func TestRegistryEndpoints(t *testing.T) { } func TestDefaultScheme(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string host string expected string }{ - "should use http by default for localhost": { + { + desc: "should use http by default for localhost", host: "localhost", expected: "http", }, - "should use http by default for localhost with port": { + { + desc: "should use http by default for localhost with port", host: "localhost:8080", expected: "http", }, - "should use http by default for 127.0.0.1": { + { + desc: "should use http by default for 127.0.0.1", host: "127.0.0.1", expected: "http", }, - "should use http by default for 127.0.0.1 with port": { + { + desc: "should use http by default for 127.0.0.1 with port", host: "127.0.0.1:8080", expected: "http", }, - "should use http by default for ::1": { + { + desc: "should use http by default for ::1", host: "::1", expected: "http", }, - "should use http by default for ::1 with port": { + { + desc: "should use http by default for ::1 with port", host: "[::1]:8080", expected: "http", }, - "should use https by default for remote host": { + { + desc: "should use https by default for remote host", host: "remote", expected: "https", }, - "should use https by default for remote host with port": { + { + desc: "should use https by default for remote host with port", host: "remote:8080", expected: "https", }, - "should use https by default for remote ip": { + { + desc: "should use https by default for remote ip", host: "8.8.8.8", expected: "https", }, - "should use https by default for remote ip with port": { + { + desc: "should use https by default for remote ip with port", host: "8.8.8.8:8080", expected: "https", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := defaultScheme(test.host) assert.Equal(t, test.expected, got) }) @@ -315,20 +349,24 @@ func TestDefaultScheme(t *testing.T) { } func TestEncryptedImagePullOpts(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string keyModel string expectedOpts int }{ - "node key model should return one unpack opt": { + { + desc: "node key model should return one unpack opt", keyModel: criconfig.KeyModelNode, expectedOpts: 1, }, - "no key model selected should default to node key model": { + { + desc: "no key model selected should default to node key model", keyModel: "", expectedOpts: 0, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.config.ImageDecryption.KeyModel = test.keyModel got := len(c.encryptedImagesPullOpts()) @@ -400,35 +438,41 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) { func TestGetRepoDigestAndTag(t *testing.T) { digest := digest.Digest("sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582") - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string ref string schema1 bool expectedRepoDigest string expectedRepoTag string }{ - "repo tag should be empty if original ref has no tag": { + { + desc: "repo tag should be empty if original ref has no tag", ref: "gcr.io/library/busybox@" + digest.String(), expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), }, - "repo tag should not be empty if original ref has tag": { + { + desc: "repo tag should not be empty if original ref has tag", ref: "gcr.io/library/busybox:latest", expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), expectedRepoTag: "gcr.io/library/busybox:latest", }, - "repo digest should be empty if original ref is schema1 and has no digest": { + { + desc: "repo digest should be empty if original ref is schema1 and has no digest", ref: "gcr.io/library/busybox:latest", schema1: true, expectedRepoDigest: "", expectedRepoTag: "gcr.io/library/busybox:latest", }, - "repo digest should not be empty if original ref is schema1 but has digest": { + { + desc: "repo digest should not be empty if original ref is schema1 but has digest", ref: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", schema1: true, expectedRepoDigest: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", expectedRepoTag: "", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { named, err := docker.ParseDockerRef(test.ref) assert.NoError(t, err) repoDigest, repoTag := getRepoDigestAndTag(named, digest, test.schema1) diff --git a/pkg/cri/sbserver/images/image_status_test.go b/pkg/cri/sbserver/images/image_status_test.go index 489b8a68f..d4ea0b382 100644 --- a/pkg/cri/sbserver/images/image_status_test.go +++ b/pkg/cri/sbserver/images/image_status_test.go @@ -92,36 +92,44 @@ func TestParseImageReferences(t *testing.T) { // TestGetUserFromImage tests the logic of getting image uid or user name of image user. func TestGetUserFromImage(t *testing.T) { newI64 := func(i int64) *int64 { return &i } - for c, test := range map[string]struct { + for _, test := range []struct { + desc string user string uid *int64 name string }{ - "no gid": { + { + desc: "no gid", user: "0", uid: newI64(0), }, - "uid/gid": { + { + desc: "uid/gid", user: "0:1", uid: newI64(0), }, - "empty user": { + { + desc: "empty user", user: "", }, - "multiple separators": { + { + desc: "multiple separators", user: "1:2:3", uid: newI64(1), }, - "root username": { + { + desc: "root username", user: "root:root", name: "root", }, - "username": { + { + desc: "username", user: "test:test", name: "test", }, } { - t.Run(c, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { actualUID, actualName := getUserFromImage(test.user) assert.Equal(t, test.uid, actualUID) assert.Equal(t, test.name, actualName) diff --git a/pkg/cri/sbserver/images/service_test.go b/pkg/cri/sbserver/images/service_test.go index a16020e88..6420e45f5 100644 --- a/pkg/cri/sbserver/images/service_test.go +++ b/pkg/cri/sbserver/images/service_test.go @@ -104,20 +104,24 @@ func TestRuntimeSnapshotter(t *testing.T) { Snapshotter: "devmapper", } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string runtime criconfig.Runtime expectSnapshotter string }{ - "should return default snapshotter when runtime.Snapshotter is not set": { + { + desc: "should return default snapshotter when runtime.Snapshotter is not set", runtime: defaultRuntime, expectSnapshotter: criconfig.DefaultConfig().Snapshotter, }, - "should return overridden snapshotter when runtime.Snapshotter is set": { + { + desc: "should return overridden snapshotter when runtime.Snapshotter is set", runtime: fooRuntime, expectSnapshotter: "devmapper", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { cri := newTestCRIService() cri.config = criconfig.Config{ PluginConfig: criconfig.DefaultConfig(), diff --git a/pkg/cri/sbserver/podsandbox/helpers_linux_test.go b/pkg/cri/sbserver/podsandbox/helpers_linux_test.go index c61383636..1527c29c1 100644 --- a/pkg/cri/sbserver/podsandbox/helpers_linux_test.go +++ b/pkg/cri/sbserver/podsandbox/helpers_linux_test.go @@ -29,32 +29,39 @@ import ( func TestGetCgroupsPath(t *testing.T) { testID := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string cgroupsParent string expected string }{ - "should support regular cgroup path": { + { + desc: "should support regular cgroup path", cgroupsParent: "/a/b", expected: "/a/b/test-id", }, - "should support systemd cgroup path": { + { + desc: "should support systemd cgroup path", cgroupsParent: "/a.slice/b.slice", expected: "b.slice:cri-containerd:test-id", }, - "should support tailing slash for regular cgroup path": { + { + desc: "should support tailing slash for regular cgroup path", cgroupsParent: "/a/b/", expected: "/a/b/test-id", }, - "should support tailing slash for systemd cgroup path": { + { + desc: "should support tailing slash for systemd cgroup path", cgroupsParent: "/a.slice/b.slice/", expected: "b.slice:cri-containerd:test-id", }, - "should treat root cgroup as regular cgroup path": { + { + desc: "should treat root cgroup as regular cgroup path", cgroupsParent: "/", expected: "/test-id", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { got := getCgroupsPath(test.cgroupsParent, testID) assert.Equal(t, test.expected, got) }) diff --git a/pkg/cri/sbserver/podsandbox/helpers_selinux_linux_test.go b/pkg/cri/sbserver/podsandbox/helpers_selinux_linux_test.go index 8b38bde25..28e8ba933 100644 --- a/pkg/cri/sbserver/podsandbox/helpers_selinux_linux_test.go +++ b/pkg/cri/sbserver/podsandbox/helpers_selinux_linux_test.go @@ -29,18 +29,21 @@ func TestInitSelinuxOpts(t *testing.T) { t.Skip("selinux is not enabled") } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string selinuxOpt *runtime.SELinuxOption processLabel string mountLabel string expectErr bool }{ - "Should return empty strings for processLabel and mountLabel when selinuxOpt is nil": { + { + desc: "Should return empty strings for processLabel and mountLabel when selinuxOpt is nil", selinuxOpt: nil, processLabel: ".*:c[0-9]{1,3},c[0-9]{1,3}", mountLabel: ".*:c[0-9]{1,3},c[0-9]{1,3}", }, - "Should overlay fields on processLabel when selinuxOpt has been initialized partially": { + { + desc: "Should overlay fields on processLabel when selinuxOpt has been initialized partially", selinuxOpt: &runtime.SELinuxOption{ User: "", Role: "user_r", @@ -50,7 +53,8 @@ func TestInitSelinuxOpts(t *testing.T) { processLabel: "system_u:user_r:(container_file_t|svirt_lxc_net_t):s0:c1,c2", mountLabel: "system_u:object_r:(container_file_t|svirt_sandbox_file_t):s0:c1,c2", }, - "Should be resolved correctly when selinuxOpt has been initialized completely": { + { + desc: "Should be resolved correctly when selinuxOpt has been initialized completely", selinuxOpt: &runtime.SELinuxOption{ User: "user_u", Role: "user_r", @@ -60,7 +64,8 @@ func TestInitSelinuxOpts(t *testing.T) { processLabel: "user_u:user_r:user_t:s0:c1,c2", mountLabel: "user_u:object_r:(container_file_t|svirt_sandbox_file_t):s0:c1,c2", }, - "Should be resolved correctly when selinuxOpt has been initialized with level=''": { + { + desc: "Should be resolved correctly when selinuxOpt has been initialized with level=''", selinuxOpt: &runtime.SELinuxOption{ User: "user_u", Role: "user_r", @@ -70,7 +75,8 @@ func TestInitSelinuxOpts(t *testing.T) { processLabel: "user_u:user_r:user_t:s0:c[0-9]{1,3},c[0-9]{1,3}", mountLabel: "user_u:object_r:(container_file_t|svirt_sandbox_file_t):s0", }, - "Should return error when the format of 'level' is not correct": { + { + desc: "Should return error when the format of 'level' is not correct", selinuxOpt: &runtime.SELinuxOption{ User: "user_u", Role: "user_r", @@ -80,7 +86,8 @@ func TestInitSelinuxOpts(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { processLabel, mountLabel, err := initLabelsFromOpt(test.selinuxOpt) if test.expectErr { assert.Error(t, err) @@ -93,59 +100,75 @@ func TestInitSelinuxOpts(t *testing.T) { } func TestCheckSelinuxLevel(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string level string expectNoMatch bool }{ - "s0": { + { + desc: "s0", level: "s0", }, - "s0-s0": { + { + desc: "s0-s0", level: "s0-s0", }, - "s0:c0": { + { + desc: "s0:c0", level: "s0:c0", }, - "s0:c0.c3": { + { + desc: "s0:c0.c3", level: "s0:c0.c3", }, - "s0:c0,c3": { + { + desc: "s0:c0,c3", level: "s0:c0,c3", }, - "s0-s0:c0,c3": { + { + desc: "s0-s0:c0,c3", level: "s0-s0:c0,c3", }, - "s0-s0:c0,c3.c6": { + { + desc: "s0-s0:c0,c3.c6", level: "s0-s0:c0,c3.c6", }, - "s0-s0:c0,c3.c6,c8.c10": { + { + desc: "s0-s0:c0,c3.c6,c8.c10", level: "s0-s0:c0,c3.c6,c8.c10", }, - "s0-s0:c0,c3.c6,c8,c10": { + { + desc: "s0-s0:c0,c3.c6,c8,c10", level: "s0-s0:c0,c3.c6", }, - "s0,c0,c3": { + { + desc: "s0,c0,c3", level: "s0,c0,c3", expectNoMatch: true, }, - "s0:c0.c3.c6": { + { + desc: "s0:c0.c3.c6", level: "s0:c0.c3.c6", expectNoMatch: true, }, - "s0-s0,c0,c3": { + { + desc: "s0-s0,c0,c3", level: "s0-s0,c0,c3", expectNoMatch: true, }, - "s0-s0:c0.c3.c6": { + { + desc: "s0-s0:c0.c3.c6", level: "s0-s0:c0.c3.c6", expectNoMatch: true, }, - "s0-s0:c0,c3.c6.c8": { + { + desc: "s0-s0:c0,c3.c6.c8", level: "s0-s0:c0,c3.c6.c8", expectNoMatch: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { err := checkSelinuxLevel(test.level) if test.expectNoMatch { assert.Error(t, err) diff --git a/pkg/cri/sbserver/podsandbox/helpers_test.go b/pkg/cri/sbserver/podsandbox/helpers_test.go index 494395cbf..b2c0ba52b 100644 --- a/pkg/cri/sbserver/podsandbox/helpers_test.go +++ b/pkg/cri/sbserver/podsandbox/helpers_test.go @@ -31,35 +31,41 @@ import ( func TestGetRepoDigestAndTag(t *testing.T) { digest := imagedigest.Digest("sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59582") - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string ref string schema1 bool expectedRepoDigest string expectedRepoTag string }{ - "repo tag should be empty if original ref has no tag": { + { + desc: "repo tag should be empty if original ref has no tag", ref: "gcr.io/library/busybox@" + digest.String(), expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), }, - "repo tag should not be empty if original ref has tag": { + { + desc: "repo tag should not be empty if original ref has tag", ref: "gcr.io/library/busybox:latest", expectedRepoDigest: "gcr.io/library/busybox@" + digest.String(), expectedRepoTag: "gcr.io/library/busybox:latest", }, - "repo digest should be empty if original ref is schema1 and has no digest": { + { + desc: "repo digest should be empty if original ref is schema1 and has no digest", ref: "gcr.io/library/busybox:latest", schema1: true, expectedRepoDigest: "", expectedRepoTag: "gcr.io/library/busybox:latest", }, - "repo digest should not be empty if original ref is schema1 but has digest": { + { + desc: "repo digest should not be empty if original ref is schema1 but has digest", ref: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", schema1: true, expectedRepoDigest: "gcr.io/library/busybox@sha256:e6693c20186f837fc393390135d8a598a96a833917917789d63766cab6c59594", expectedRepoTag: "", }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { named, err := docker.ParseDockerRef(test.ref) assert.NoError(t, err) repoDigest, repoTag := getRepoDigestAndTag(named, digest, test.schema1) @@ -109,18 +115,21 @@ func TestParseImageReferences(t *testing.T) { } func TestEnvDeduplication(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string existing []string kv [][2]string expected []string }{ - "single env": { + { + desc: "single env", kv: [][2]string{ {"a", "b"}, }, expected: []string{"a=b"}, }, - "multiple envs": { + { + desc: "multiple envs", kv: [][2]string{ {"a", "b"}, {"c", "d"}, @@ -132,7 +141,8 @@ func TestEnvDeduplication(t *testing.T) { "e=f", }, }, - "env override": { + { + desc: "env override", kv: [][2]string{ {"k1", "v1"}, {"k2", "v2"}, @@ -148,7 +158,8 @@ func TestEnvDeduplication(t *testing.T) { "k4=v6", }, }, - "existing env": { + { + desc: "existing env", existing: []string{ "k1=v1", "k2=v2", @@ -167,7 +178,8 @@ func TestEnvDeduplication(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { var spec runtimespec.Spec if len(test.existing) > 0 { spec.Process = &runtimespec.Process{ @@ -183,17 +195,20 @@ func TestEnvDeduplication(t *testing.T) { } func TestPassThroughAnnotationsFilter(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string podAnnotations map[string]string runtimePodAnnotations []string passthroughAnnotations map[string]string }{ - "should support direct match": { + { + desc: "should support direct match", podAnnotations: map[string]string{"c": "d", "d": "e"}, runtimePodAnnotations: []string{"c"}, passthroughAnnotations: map[string]string{"c": "d"}, }, - "should support wildcard match": { + { + desc: "should support wildcard match", podAnnotations: map[string]string{ "t.f": "j", "z.g": "o", @@ -208,7 +223,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { "y.ca": "b", }, }, - "should support wildcard match all": { + { + desc: "should support wildcard match all", podAnnotations: map[string]string{ "t.f": "j", "z.g": "o", @@ -225,7 +241,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { "y": "b", }, }, - "should support match including path separator": { + { + desc: "should support match including path separator", podAnnotations: map[string]string{ "matchend.com/end": "1", "matchend.com/end1": "2", @@ -284,7 +301,8 @@ func TestPassThroughAnnotationsFilter(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { passthroughAnnotations := getPassthroughAnnotations(test.podAnnotations, test.runtimePodAnnotations) assert.Equal(t, test.passthroughAnnotations, passthroughAnnotations) }) diff --git a/pkg/cri/sbserver/podsandbox/sandbox_run_linux_test.go b/pkg/cri/sbserver/podsandbox/sandbox_run_linux_test.go index 4f6340778..82a914917 100644 --- a/pkg/cri/sbserver/podsandbox/sandbox_run_linux_test.go +++ b/pkg/cri/sbserver/podsandbox/sandbox_run_linux_test.go @@ -106,12 +106,14 @@ func getRunPodSandboxTestData() (*runtime.PodSandboxConfig, *imagespec.ImageConf func TestLinuxSandboxContainerSpec(t *testing.T) { testID := "test-id" nsPath := "test-cni" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string configChange func(*runtime.PodSandboxConfig) specCheck func(*testing.T, *runtimespec.Spec) expectErr bool }{ - "spec should reflect original config": { + { + desc: "spec should reflect original config", specCheck: func(t *testing.T, spec *runtimespec.Spec) { // runtime spec should have expected namespaces enabled by default. require.NotNil(t, spec.Linux) @@ -132,7 +134,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.Contains(t, spec.Linux.Sysctl["net.ipv4.ping_group_range"], "0 2147483647") }, }, - "host namespace": { + { + desc: "host namespace", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ NamespaceOptions: &runtime.NamespaceOption{ @@ -161,7 +164,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.NotContains(t, spec.Linux.Sysctl["net.ipv4.ping_group_range"], "0 2147483647") }, }, - "should set supplemental groups correctly": { + { + desc: "should set supplemental groups correctly", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{ SupplementalGroups: []int64{1111, 2222}, @@ -173,7 +177,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.Contains(t, spec.Process.User.AdditionalGids, uint32(2222)) }, }, - "should overwrite default sysctls": { + { + desc: "should overwrite default sysctls", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.Sysctls = map[string]string{ "net.ipv4.ip_unprivileged_port_start": "500", @@ -186,7 +191,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.Contains(t, spec.Linux.Sysctl["net.ipv4.ping_group_range"], "1 1000") }, }, - "sandbox sizing annotations should be set if LinuxContainerResources were provided": { + { + desc: "sandbox sizing annotations should be set if LinuxContainerResources were provided", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.Resources = &v1.LinuxContainerResources{ CpuPeriod: 100, @@ -214,7 +220,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.EqualValues(t, "1024", value) }, }, - "sandbox sizing annotations should not be set if LinuxContainerResources were not provided": { + { + desc: "sandbox sizing annotations should not be set if LinuxContainerResources were not provided", specCheck: func(t *testing.T, spec *runtimespec.Spec) { _, ok := spec.Annotations[annotations.SandboxCPUPeriod] assert.False(t, ok) @@ -226,7 +233,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "sandbox sizing annotations are zero if the resources are set to 0": { + { + desc: "sandbox sizing annotations are zero if the resources are set to 0", configChange: func(c *runtime.PodSandboxConfig) { c.Linux.Resources = &v1.LinuxContainerResources{} }, @@ -246,7 +254,8 @@ func TestLinuxSandboxContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newControllerService() c.config.EnableUnprivilegedICMP = true c.config.EnableUnprivilegedPorts = true @@ -275,13 +284,15 @@ func TestSetupSandboxFiles(t *testing.T) { testID = "test-id" realhostname = "test-real-hostname" ) - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string dnsConfig *runtime.DNSConfig hostname string ipcMode runtime.NamespaceMode expectedCalls []ostesting.CalledDetail }{ - "should check host /dev/shm existence when ipc mode is NODE": { + { + desc: "should check host /dev/shm existence when ipc mode is NODE", ipcMode: runtime.NamespaceMode_NODE, expectedCalls: []ostesting.CalledDetail{ { @@ -317,7 +328,8 @@ func TestSetupSandboxFiles(t *testing.T) { }, }, }, - "should create new /etc/resolv.conf if DNSOptions is set": { + { + desc: "should create new /etc/resolv.conf if DNSOptions is set", dnsConfig: &runtime.DNSConfig{ Servers: []string{"8.8.8.8"}, Searches: []string{"114.114.114.114"}, @@ -360,7 +372,8 @@ options timeout:1 }, }, }, - "should create sandbox shm when ipc namespace mode is not NODE": { + { + desc: "should create sandbox shm when ipc namespace mode is not NODE", ipcMode: runtime.NamespaceMode_POD, expectedCalls: []ostesting.CalledDetail{ { @@ -403,7 +416,8 @@ options timeout:1 }, }, }, - "should create /etc/hostname when hostname is set": { + { + desc: "should create /etc/hostname when hostname is set", hostname: "test-hostname", ipcMode: runtime.NamespaceMode_NODE, expectedCalls: []ostesting.CalledDetail{ @@ -438,7 +452,8 @@ options timeout:1 }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newControllerService() c.os.(*ostesting.FakeOS).HostnameFn = func() (string, error) { return realhostname, nil @@ -469,15 +484,19 @@ options timeout:1 } func TestParseDNSOption(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string servers []string searches []string options []string expectedContent string expectErr bool }{ - "empty dns options should return empty content": {}, - "non-empty dns options should return correct content": { + { + desc: "empty dns options should return empty content", + }, + { + desc: "non-empty dns options should return correct content", servers: []string{"8.8.8.8", "server.google.com"}, searches: []string{"114.114.114.114"}, options: []string{"timeout:1"}, @@ -487,7 +506,8 @@ nameserver server.google.com options timeout:1 `, }, - "expanded dns config should return correct content on modern libc (e.g. glibc 2.26 and above)": { + { + desc: "expanded dns config should return correct content on modern libc (e.g. glibc 2.26 and above)", servers: []string{"8.8.8.8", "server.google.com"}, searches: []string{ "server0.google.com", @@ -506,7 +526,8 @@ options timeout:1 `, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { resolvContent, err := parseDNSOptions(test.servers, test.searches, test.options) if test.expectErr { assert.Error(t, err) diff --git a/pkg/cri/sbserver/podsandbox/sandbox_run_test.go b/pkg/cri/sbserver/podsandbox/sandbox_run_test.go index a3a43fd27..96fa7b59b 100644 --- a/pkg/cri/sbserver/podsandbox/sandbox_run_test.go +++ b/pkg/cri/sbserver/podsandbox/sandbox_run_test.go @@ -38,27 +38,31 @@ func TestSandboxContainerSpec(t *testing.T) { } testID := "test-id" nsPath := "test-cni" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string configChange func(*runtime.PodSandboxConfig) podAnnotations []string imageConfigChange func(*imagespec.ImageConfig) specCheck func(*testing.T, *runtimespec.Spec) expectErr bool }{ - "should return error when entrypoint and cmd are empty": { + { + desc: "should return error when entrypoint and cmd are empty", imageConfigChange: func(c *imagespec.ImageConfig) { c.Entrypoint = nil c.Cmd = nil }, expectErr: true, }, - "a passthrough annotation should be passed as an OCI annotation": { + { + desc: "a passthrough annotation should be passed as an OCI annotation", podAnnotations: []string{"c"}, specCheck: func(t *testing.T, spec *runtimespec.Spec) { assert.Equal(t, spec.Annotations["c"], "d") }, }, - "a non-passthrough annotation should not be passed as an OCI annotation": { + { + desc: "a non-passthrough annotation should not be passed as an OCI annotation", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["d"] = "e" }, @@ -69,7 +73,8 @@ func TestSandboxContainerSpec(t *testing.T) { assert.False(t, ok) }, }, - "passthrough annotations should support wildcard match": { + { + desc: "passthrough annotations should support wildcard match", configChange: func(c *runtime.PodSandboxConfig) { c.Annotations["t.f"] = "j" c.Annotations["z.g"] = "o" @@ -89,7 +94,8 @@ func TestSandboxContainerSpec(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newControllerService() config, imageConfig, specCheck := getRunPodSandboxTestData() if test.configChange != nil { @@ -117,11 +123,15 @@ func TestSandboxContainerSpec(t *testing.T) { } func TestTypeurlMarshalUnmarshalSandboxMeta(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string configChange func(*runtime.PodSandboxConfig) }{ - "should marshal original config": {}, - "should marshal Linux": { + { + desc: "should marshal original config", + }, + { + desc: "should marshal Linux", configChange: func(c *runtime.PodSandboxConfig) { if c.Linux == nil { c.Linux = &runtime.LinuxPodSandboxConfig{} @@ -137,7 +147,8 @@ func TestTypeurlMarshalUnmarshalSandboxMeta(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { meta := &sandboxstore.Metadata{ ID: "1", Name: "sandbox_1", @@ -198,6 +209,7 @@ func TestHostAccessingSandbox(t *testing.T) { {"Security Context namespace host access", hostNamespace, true}, } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { if got := hostAccessingSandbox(tt.config); got != tt.want { t.Errorf("hostAccessingSandbox() = %v, want %v", got, tt.want) diff --git a/pkg/cri/sbserver/sandbox_list_test.go b/pkg/cri/sbserver/sandbox_list_test.go index f120664bc..0bed867fd 100644 --- a/pkg/cri/sbserver/sandbox_list_test.go +++ b/pkg/cri/sbserver/sandbox_list_test.go @@ -53,31 +53,36 @@ func TestToCRISandbox(t *testing.T) { Annotations: config.GetAnnotations(), RuntimeHandler: "test-runtime-handler", } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string state sandboxstore.State expectedState runtime.PodSandboxState }{ - "sandbox state ready": { + { + desc: "sandbox state ready", state: sandboxstore.StateReady, expectedState: runtime.PodSandboxState_SANDBOX_READY, }, - "sandbox state not ready": { + { + desc: "sandbox state not ready", state: sandboxstore.StateNotReady, expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, - "sandbox state unknown": { + { + desc: "sandbox state unknown", state: sandboxstore.StateUnknown, expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { status := sandboxstore.Status{ CreatedAt: createdAt, State: test.state, } expect.State = test.expectedState s := toCRISandbox(meta, status) - assert.Equal(t, expect, s, desc) + assert.Equal(t, expect, s, test.desc) }) } } @@ -157,22 +162,27 @@ func TestFilterSandboxes(t *testing.T) { assert.NoError(t, c.sandboxStore.Add(sb)) } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string filter *runtime.PodSandboxFilter expect []*runtime.PodSandbox }{ - "no filter": { + { + desc: "no filter", expect: testSandboxes, }, - "id filter": { + { + desc: "id filter", filter: &runtime.PodSandboxFilter{Id: "2abcdef"}, expect: []*runtime.PodSandbox{testSandboxes[1]}, }, - "truncid filter": { + { + desc: "truncid filter", filter: &runtime.PodSandboxFilter{Id: "2"}, expect: []*runtime.PodSandbox{testSandboxes[1]}, }, - "state filter": { + { + desc: "state filter", filter: &runtime.PodSandboxFilter{ State: &runtime.PodSandboxStateValue{ State: runtime.PodSandboxState_SANDBOX_READY, @@ -180,20 +190,23 @@ func TestFilterSandboxes(t *testing.T) { }, expect: []*runtime.PodSandbox{testSandboxes[0], testSandboxes[2]}, }, - "label filter": { + { + desc: "label filter", filter: &runtime.PodSandboxFilter{ LabelSelector: map[string]string{"a": "b"}, }, expect: []*runtime.PodSandbox{testSandboxes[1]}, }, - "mixed filter not matched": { + { + desc: "mixed filter not matched", filter: &runtime.PodSandboxFilter{ Id: "1", LabelSelector: map[string]string{"a": "b"}, }, expect: []*runtime.PodSandbox{}, }, - "mixed filter matched": { + { + desc: "mixed filter matched", filter: &runtime.PodSandboxFilter{ State: &runtime.PodSandboxStateValue{ State: runtime.PodSandboxState_SANDBOX_READY, @@ -203,9 +216,10 @@ func TestFilterSandboxes(t *testing.T) { expect: []*runtime.PodSandbox{testSandboxes[2]}, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { filtered := c.filterCRISandboxes(testSandboxes, test.filter) - assert.Equal(t, test.expect, filtered, desc) + assert.Equal(t, test.expect, filtered, test.desc) }) } } diff --git a/pkg/cri/sbserver/sandbox_run_test.go b/pkg/cri/sbserver/sandbox_run_test.go index a80f13d09..a51810925 100644 --- a/pkg/cri/sbserver/sandbox_run_test.go +++ b/pkg/cri/sbserver/sandbox_run_test.go @@ -27,12 +27,16 @@ import ( ) func TestToCNIPortMappings(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string criPortMappings []*runtime.PortMapping cniPortMappings []cni.PortMapping }{ - "empty CRI port mapping should map to empty CNI port mapping": {}, - "CRI port mapping should be converted to CNI port mapping properly": { + { + desc: "empty CRI port mapping should map to empty CNI port mapping", + }, + { + desc: "CRI port mapping should be converted to CNI port mapping properly", criPortMappings: []*runtime.PortMapping{ { Protocol: runtime.Protocol_UDP, @@ -74,7 +78,8 @@ func TestToCNIPortMappings(t *testing.T) { }, }, }, - "CRI port mapping without host port should be skipped": { + { + desc: "CRI port mapping without host port should be skipped", criPortMappings: []*runtime.PortMapping{ { Protocol: runtime.Protocol_UDP, @@ -97,7 +102,8 @@ func TestToCNIPortMappings(t *testing.T) { }, }, }, - "CRI port mapping with unsupported protocol should be skipped": { + { + desc: "CRI port mapping with unsupported protocol should be skipped", criPortMappings: []*runtime.PortMapping{ { Protocol: runtime.Protocol_TCP, @@ -116,54 +122,62 @@ func TestToCNIPortMappings(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { assert.Equal(t, test.cniPortMappings, toCNIPortMappings(test.criPortMappings)) }) } } func TestSelectPodIP(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string ips []string expectedIP string expectedAdditionalIPs []string pref string }{ - "ipv4 should be picked even if ipv6 comes first": { + { + desc: "ipv4 should be picked even if ipv6 comes first", ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"}, expectedIP: "192.168.17.43", expectedAdditionalIPs: []string{"2001:db8:85a3::8a2e:370:7334"}, }, - "ipv6 should be picked even if ipv4 comes first": { + { + desc: "ipv6 should be picked even if ipv4 comes first", ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"}, expectedIP: "2001:db8:85a3::8a2e:370:7334", expectedAdditionalIPs: []string{"192.168.17.43"}, pref: "ipv6", }, - "order should reflect ip selection": { + { + desc: "order should reflect ip selection", ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43"}, expectedIP: "2001:db8:85a3::8a2e:370:7334", expectedAdditionalIPs: []string{"192.168.17.43"}, pref: "cni", }, - - "ipv4 should be picked when there is only ipv4": { + { + desc: "ipv4 should be picked when there is only ipv4", ips: []string{"192.168.17.43"}, expectedIP: "192.168.17.43", expectedAdditionalIPs: nil, }, - "ipv6 should be picked when there is no ipv4": { + { + desc: "ipv6 should be picked when there is no ipv4", ips: []string{"2001:db8:85a3::8a2e:370:7334"}, expectedIP: "2001:db8:85a3::8a2e:370:7334", expectedAdditionalIPs: nil, }, - "the first ipv4 should be picked when there are multiple ipv4": { // unlikely to happen + { + desc: "the first ipv4 should be picked when there are multiple ipv4", // unlikely to happen ips: []string{"2001:db8:85a3::8a2e:370:7334", "192.168.17.43", "2001:db8:85a3::8a2e:370:7335", "192.168.17.45"}, expectedIP: "192.168.17.43", expectedAdditionalIPs: []string{"2001:db8:85a3::8a2e:370:7334", "2001:db8:85a3::8a2e:370:7335", "192.168.17.45"}, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { var ipConfigs []*cni.IPConfig for _, ip := range test.ips { ipConfigs = append(ipConfigs, &cni.IPConfig{ diff --git a/pkg/cri/sbserver/sandbox_stats_windows_test.go b/pkg/cri/sbserver/sandbox_stats_windows_test.go index ac4580e79..ca88e0c6a 100644 --- a/pkg/cri/sbserver/sandbox_stats_windows_test.go +++ b/pkg/cri/sbserver/sandbox_stats_windows_test.go @@ -33,20 +33,23 @@ func TestGetUsageNanoCores(t *testing.T) { secondAfterTimeStamp := timestamp.Add(time.Second) ID := "ID" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string firstCPUValue uint64 secondCPUValue uint64 expectedNanoCoreUsageFirst uint64 expectedNanoCoreUsageSecond uint64 }{ - "metrics": { + { + desc: "metrics", firstCPUValue: 50, secondCPUValue: 500, expectedNanoCoreUsageFirst: 0, expectedNanoCoreUsageSecond: 450, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { container, err := containerstore.NewContainer( containerstore.Metadata{ID: ID}, ) @@ -85,7 +88,8 @@ func Test_criService_podSandboxStats(t *testing.T) { UsageNanoCores uint64 WorkingSetBytes uint64 } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string metrics map[string]*wstats.Statistics sandbox sandboxstore.Sandbox containers []containerstore.Container @@ -93,7 +97,8 @@ func Test_criService_podSandboxStats(t *testing.T) { expectedContainerStats []expectedStats expectError bool }{ - "no metrics found should return error": { + { + desc: "no metrics found should return error", metrics: map[string]*wstats.Statistics{}, sandbox: sandboxstore.Sandbox{}, containers: []containerstore.Container{}, @@ -101,7 +106,8 @@ func Test_criService_podSandboxStats(t *testing.T) { expectedContainerStats: []expectedStats{}, expectError: true, }, - "pod stats will include the container stats": { + { + desc: "pod stats will include the container stats", metrics: map[string]*wstats.Statistics{ "c1": { Container: windowsStat(currentStatsTimestamp, 200, 20), @@ -128,7 +134,8 @@ func Test_criService_podSandboxStats(t *testing.T) { }, expectError: false, }, - "pod with existing stats will have usagenanocores totalled across pods and containers": { + { + desc: "pod with existing stats will have usagenanocores totalled across pods and containers", metrics: map[string]*wstats.Statistics{ "c1": { Container: windowsStat(currentStatsTimestamp, 400, 20), @@ -161,7 +168,8 @@ func Test_criService_podSandboxStats(t *testing.T) { }, expectError: false, }, - "pod sandbox with nil stats still works (hostprocess container scenario)": { + { + desc: "pod sandbox with nil stats still works (hostprocess container scenario)", metrics: map[string]*wstats.Statistics{ "c1": { Container: windowsStat(currentStatsTimestamp, 400, 20), @@ -193,7 +201,8 @@ func Test_criService_podSandboxStats(t *testing.T) { expectError: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { actualPodStats, actualContainerStats, err := c.toPodSandboxStats(test.sandbox, test.metrics, test.containers, currentStatsTimestamp) if test.expectError { assert.NotNil(t, err) @@ -240,25 +249,29 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { timestamp := time.Now() containerID := "c1" sandboxID := "s1" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string sandboxStats *runtime.PodSandboxStats expectError bool expectedSandboxvalue *stats.ContainerStats expectedContainervalue *stats.ContainerStats }{ - "if sandboxstats is nil then skip ": { + { + desc: "if sandboxstats is nil then skip ", sandboxStats: nil, expectError: false, expectedSandboxvalue: nil, }, - "if sandboxstats.windows is nil then skip": { + { + desc: "if sandboxstats.windows is nil then skip", sandboxStats: &runtime.PodSandboxStats{ Windows: nil, }, expectError: false, expectedSandboxvalue: nil, }, - "if sandboxstats.windows.cpu is nil then skip": { + { + desc: "if sandboxstats.windows.cpu is nil then skip", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: nil, @@ -267,7 +280,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { expectError: false, expectedSandboxvalue: nil, }, - "if sandboxstats.windows.cpu.UsageCoreNanoSeconds is nil then skip": { + { + desc: "if sandboxstats.windows.cpu.UsageCoreNanoSeconds is nil then skip", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -278,7 +292,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { expectError: false, expectedSandboxvalue: nil, }, - "Stats for containers that have cpu nil are skipped": { + { + desc: "Stats for containers that have cpu nil are skipped", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -300,7 +315,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { }, expectedContainervalue: nil, }, - "Stats for containers that have UsageCoreNanoSeconds nil are skipped": { + { + desc: "Stats for containers that have UsageCoreNanoSeconds nil are skipped", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -324,7 +340,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { }, expectedContainervalue: nil, }, - "Stats are updated for sandbox and containers": { + { + desc: "Stats are updated for sandbox and containers", sandboxStats: &runtime.PodSandboxStats{ Windows: &runtime.WindowsPodSandboxStats{ Cpu: &runtime.WindowsCpuUsage{ @@ -353,7 +370,8 @@ func Test_criService_saveSandBoxMetrics(t *testing.T) { }, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() c.sandboxStore.Add(sandboxstore.Sandbox{ Metadata: sandboxstore.Metadata{ID: sandboxID}, diff --git a/pkg/cri/sbserver/sandbox_status_test.go b/pkg/cri/sbserver/sandbox_status_test.go index 8bf999b13..5f942b79b 100644 --- a/pkg/cri/sbserver/sandbox_status_test.go +++ b/pkg/cri/sbserver/sandbox_status_test.go @@ -87,24 +87,29 @@ func TestPodSandboxStatus(t *testing.T) { Annotations: config.GetAnnotations(), RuntimeHandler: "test-runtime-handler", } - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string state string expectedState runtime.PodSandboxState }{ - "sandbox state ready": { + { + desc: "sandbox state ready", state: sandboxstore.StateReady.String(), expectedState: runtime.PodSandboxState_SANDBOX_READY, }, - "sandbox state not ready": { + { + desc: "sandbox state not ready", state: sandboxstore.StateNotReady.String(), expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, - "sandbox state unknown": { + { + desc: "sandbox state unknown", state: sandboxstore.StateUnknown.String(), expectedState: runtime.PodSandboxState_SANDBOX_NOTREADY, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { expected.State = test.expectedState got := toCRISandboxStatus(metadata, test.state, createdAt, ip, additionalIPs) assert.Equal(t, expected, got) diff --git a/pkg/cri/sbserver/sandbox_stop_test.go b/pkg/cri/sbserver/sandbox_stop_test.go index b575a4da7..09235768e 100644 --- a/pkg/cri/sbserver/sandbox_stop_test.go +++ b/pkg/cri/sbserver/sandbox_stop_test.go @@ -28,30 +28,35 @@ import ( func TestWaitSandboxStop(t *testing.T) { id := "test-id" - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string state sandboxstore.State cancel bool timeout time.Duration expectErr bool }{ - "should return error if timeout exceeds": { + { + desc: "should return error if timeout exceeds", state: sandboxstore.StateReady, timeout: 200 * time.Millisecond, expectErr: true, }, - "should return error if context is cancelled": { + { + desc: "should return error if context is cancelled", state: sandboxstore.StateReady, timeout: time.Hour, cancel: true, expectErr: true, }, - "should not return error if sandbox is stopped before timeout": { + { + desc: "should not return error if sandbox is stopped before timeout", state: sandboxstore.StateNotReady, timeout: time.Hour, expectErr: false, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { c := newTestCRIService() sandbox := sandboxstore.NewSandbox( sandboxstore.Metadata{ID: id}, @@ -69,7 +74,7 @@ func TestWaitSandboxStop(t *testing.T) { ctx = timeoutCtx } err := c.waitSandboxStop(ctx, sandbox) - assert.Equal(t, test.expectErr, err != nil, desc) + assert.Equal(t, test.expectErr, err != nil, test.desc) }) } } diff --git a/pkg/cri/sbserver/streaming_test.go b/pkg/cri/sbserver/streaming_test.go index 7d43cdb18..9796b3876 100644 --- a/pkg/cri/sbserver/streaming_test.go +++ b/pkg/cri/sbserver/streaming_test.go @@ -24,12 +24,14 @@ import ( ) func TestValidateStreamServer(t *testing.T) { - for desc, test := range map[string]struct { + for _, test := range []struct { + desc string *criService tlsMode streamListenerMode expectErr bool }{ - "should pass with default withoutTLS": { + { + desc: "should pass with default withoutTLS", criService: &criService{ config: config.Config{ PluginConfig: config.DefaultConfig(), @@ -38,7 +40,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: withoutTLS, expectErr: false, }, - "should pass with x509KeyPairTLS": { + { + desc: "should pass with x509KeyPairTLS", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -53,7 +56,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: x509KeyPairTLS, expectErr: false, }, - "should pass with selfSign": { + { + desc: "should pass with selfSign", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -64,7 +68,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: selfSignTLS, expectErr: false, }, - "should return error with X509 keypair but not EnableTLSStreaming": { + { + desc: "should return error with X509 keypair but not EnableTLSStreaming", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -79,7 +84,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error with X509 TLSCertFile empty": { + { + desc: "should return error with X509 TLSCertFile empty", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -94,7 +100,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error with X509 TLSKeyFile empty": { + { + desc: "should return error with X509 TLSKeyFile empty", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -109,7 +116,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error without EnableTLSStreaming and only TLSCertFile set": { + { + desc: "should return error without EnableTLSStreaming and only TLSCertFile set", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -124,7 +132,8 @@ func TestValidateStreamServer(t *testing.T) { tlsMode: -1, expectErr: true, }, - "should return error without EnableTLSStreaming and only TLSKeyFile set": { + { + desc: "should return error without EnableTLSStreaming and only TLSKeyFile set", criService: &criService{ config: config.Config{ PluginConfig: config.PluginConfig{ @@ -140,7 +149,8 @@ func TestValidateStreamServer(t *testing.T) { expectErr: true, }, } { - t.Run(desc, func(t *testing.T) { + test := test + t.Run(test.desc, func(t *testing.T) { tlsMode, err := getStreamListenerMode(test.criService) if test.expectErr { assert.Error(t, err) diff --git a/pkg/cri/sbserver/update_runtime_config_test.go b/pkg/cri/sbserver/update_runtime_config_test.go index 6bbb64433..e416a7112 100644 --- a/pkg/cri/sbserver/update_runtime_config_test.go +++ b/pkg/cri/sbserver/update_runtime_config_test.go @@ -70,29 +70,35 @@ func TestUpdateRuntimeConfig(t *testing.T) { }` ) - for name, test := range map[string]struct { + for _, test := range []struct { + name string noTemplate bool emptyCIDR bool networkReady bool expectCNIConfig bool }{ - "should not generate cni config if cidr is empty": { + { + name: "should not generate cni config if cidr is empty", emptyCIDR: true, expectCNIConfig: false, }, - "should not generate cni config if template file is not specified": { + { + name: "should not generate cni config if template file is not specified", noTemplate: true, expectCNIConfig: false, }, - "should not generate cni config if network is ready": { + { + name: "should not generate cni config if network is ready", networkReady: true, expectCNIConfig: false, }, - "should generate cni config if template is specified and cidr is provided": { + { + name: "should generate cni config if template is specified and cidr is provided", expectCNIConfig: true, }, } { - t.Run(name, func(t *testing.T) { + test := test + t.Run(test.name, func(t *testing.T) { testDir := t.TempDir() templateName := filepath.Join(testDir, "template") err := os.WriteFile(templateName, []byte(testTemplate), 0666)