From 07ca0be07bafc8e8d374aa13c637f4c830ff68d3 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 25 May 2021 14:19:26 +1000 Subject: [PATCH 1/2] *: clean up remaining golangci-lint failures Most of these were false positives or cases where we want to ignore the lint, but the change to the BPF generation is actually useful. Signed-off-by: Aleksa Sarai --- .github/workflows/validate.yml | 2 +- .../cgroups/ebpf/devicefilter/devicefilter.go | 15 ++++++--------- libcontainer/cgroups/fs/memory.go | 8 -------- libcontainer/integration/seccomp_test.go | 8 ++++---- .../seccomp/patchbpf/enosys_linux_test.go | 8 -------- libcontainer/system/proc.go | 2 +- 6 files changed, 12 insertions(+), 31 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index e41a7af7950..8f651acb162 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -20,7 +20,7 @@ jobs: - uses: golangci/golangci-lint-action@v2 with: # must be specified without patch version - version: v1.36 + version: v1.40 # Only show new issues for a pull request. only-new-issues: true diff --git a/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go b/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go index 10e575d2640..96cbca3916e 100644 --- a/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go +++ b/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go @@ -110,8 +110,7 @@ func (p *program) appendRule(rule *devices.Rule) error { return errors.New("the program is finalized") } - bpfType := int32(-1) - hasType := true + var bpfType int32 switch rule.Type { case devices.CharDevice: bpfType = int32(unix.BPF_DEVCG_DEV_CHAR) @@ -119,7 +118,7 @@ func (p *program) appendRule(rule *devices.Rule) error { bpfType = int32(unix.BPF_DEVCG_DEV_BLOCK) default: // We do not permit 'a', nor any other types we don't know about. - return errors.Errorf("invalid Type %q", string(rule.Type)) + return errors.Errorf("invalid type %q", string(rule.Type)) } if rule.Major > math.MaxUint32 { return errors.Errorf("invalid major %d", rule.Major) @@ -150,12 +149,10 @@ func (p *program) appendRule(rule *devices.Rule) error { nextBlockSym = "block-" + strconv.Itoa(p.blockID+1) prevBlockLastIdx = len(p.insts) - 1 ) - if hasType { - p.insts = append(p.insts, - // if (R2 != bpfType) goto next - asm.JNE.Imm(asm.R2, bpfType, nextBlockSym), - ) - } + p.insts = append(p.insts, + // if (R2 != bpfType) goto next + asm.JNE.Imm(asm.R2, bpfType, nextBlockSym), + ) if hasAccess { p.insts = append(p.insts, // if (R3 & bpfAccess != R3 /* use R1 as a temp var */) goto next diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index dc27cb9e9c9..18641bf87ff 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -200,14 +200,6 @@ func (s *MemoryGroup) GetStats(path string, stats *cgroups.Stats) error { return nil } -func memoryAssigned(cgroup *configs.Cgroup) bool { - return cgroup.Resources.Memory != 0 || - cgroup.Resources.MemoryReservation != 0 || - cgroup.Resources.MemorySwap > 0 || - cgroup.Resources.OomKillDisable || - (cgroup.Resources.MemorySwappiness != nil && int64(*cgroup.Resources.MemorySwappiness) != -1) -} - func getMemoryData(path, name string) (cgroups.MemoryData, error) { memoryData := cgroups.MemoryData{} diff --git a/libcontainer/integration/seccomp_test.go b/libcontainer/integration/seccomp_test.go index 288dc893c99..244f2752f5c 100644 --- a/libcontainer/integration/seccomp_test.go +++ b/libcontainer/integration/seccomp_test.go @@ -37,7 +37,7 @@ func TestSeccompDenyGetcwdWithErrno(t *testing.T) { container, err := newContainer(t, config) ok(t, err) - defer container.Destroy() + defer container.Destroy() //nolint:errcheck buffers := newStdBuffers() pwd := &libcontainer.Process{ @@ -100,7 +100,7 @@ func TestSeccompDenyGetcwd(t *testing.T) { container, err := newContainer(t, config) ok(t, err) - defer container.Destroy() + defer container.Destroy() //nolint:errcheck buffers := newStdBuffers() pwd := &libcontainer.Process{ @@ -170,7 +170,7 @@ func TestSeccompPermitWriteConditional(t *testing.T) { container, err := newContainer(t, config) ok(t, err) - defer container.Destroy() + defer container.Destroy() //nolint:errcheck buffers := newStdBuffers() dmesg := &libcontainer.Process{ @@ -226,7 +226,7 @@ func TestSeccompDenyWriteConditional(t *testing.T) { container, err := newContainer(t, config) ok(t, err) - defer container.Destroy() + defer container.Destroy() //nolint:errcheck buffers := newStdBuffers() dmesg := &libcontainer.Process{ diff --git a/libcontainer/seccomp/patchbpf/enosys_linux_test.go b/libcontainer/seccomp/patchbpf/enosys_linux_test.go index ee26a6a6108..ce5f02545f8 100644 --- a/libcontainer/seccomp/patchbpf/enosys_linux_test.go +++ b/libcontainer/seccomp/patchbpf/enosys_linux_test.go @@ -106,14 +106,6 @@ var testArches = []string{ "s390x", } -func archStringToNative(arch string) (nativeArch, error) { - scmpArch, err := libseccomp.GetArchFromString(arch) - if err != nil { - return 0, fmt.Errorf("unknown architecture %q: %v", arch, err) - } - return archToNative(scmpArch) -} - func testEnosysStub(t *testing.T, defaultAction configs.Action, arches []string) { explicitSyscalls := []string{ "setns", diff --git a/libcontainer/system/proc.go b/libcontainer/system/proc.go index b73cf70b434..d0407cfe42a 100644 --- a/libcontainer/system/proc.go +++ b/libcontainer/system/proc.go @@ -96,7 +96,7 @@ func parseStat(data string) (stat Stat_t, err error) { // one (PID) and two (Name) in the paren-split. parts = strings.Split(data[i+2:], " ") var state int - fmt.Sscanf(parts[3-3], "%c", &state) + fmt.Sscanf(parts[3-3], "%c", &state) //nolint:staticcheck // "3-3" is more readable in this context. stat.State = State(state) fmt.Sscanf(parts[22-3], "%d", &stat.StartTime) return stat, nil From 37767c0510551afbc1924b124bbeb5563681eac1 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 25 May 2021 14:22:00 +1000 Subject: [PATCH 2/2] ci: lint: show all errors in PRs It seems that golangci-lint didn't warn us about new lint errors that were added after we enabled it, so just run the full thing and give us all the errors on every PR run -- as long as we keep master lint-clean it doesn't matter whether we set this or not. Signed-off-by: Aleksa Sarai --- .github/workflows/validate.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 8f651acb162..f54cc6e987f 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -21,8 +21,6 @@ jobs: with: # must be specified without patch version version: v1.40 - # Only show new issues for a pull request. - only-new-issues: true shfmt: runs-on: ubuntu-20.04