From 6a24e15c523d083739e2d605249457c212f92a33 Mon Sep 17 00:00:00 2001 From: Sean McGinnis Date: Fri, 14 May 2021 16:28:13 -0500 Subject: [PATCH] Enable excluded lint checks There are a set of lint checks that are disabled by default with golangci-lint. This enables them by setting exclude-use-default to false. This by itself triggers a set of lint failures due to these additional checks. To by systematic about addressing these potential issues, this adds those checks back as individual exclusions so they can be addressed on by one. This eliminates one of those individual checks by addressing cases of "Potential file inclusion via variable" where passed in variables are used in os.OpenFile calls. Signed-off-by: Sean McGinnis --- .golangci.yml | 11 +++++++++++ test/framework/alltypes_helpers.go | 2 +- test/framework/bootstrap/kind_util.go | 2 +- test/framework/deployment_helpers.go | 2 +- test/framework/kubetest/setup.go | 3 ++- test/framework/namespace_helpers.go | 2 +- test/infrastructure/docker/docker/machine.go | 2 +- 7 files changed, 18 insertions(+), 6 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4a32eddc669e..358d834aaef5 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -38,10 +38,21 @@ linters: issues: max-same-issues: 0 max-issues-per-linter: 0 + # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant + # changes in PRs and avoid nitpicking. + exclude-use-default: false # List of regexps of issue texts to exclude, empty list by default. exclude: - Using the variable on range scope `(tc)|(rt)|(tt)|(test)|(testcase)|(testCase)` in function literal - "G108: Profiling endpoint is automatically exposed on /debug/pprof" + - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked + # The following are being worked on to remove their exclusion. This list should be reduced or go away all together over time. + # If it is decided they will not be addressed they should be moved above this comment. + - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form) + - package comment should be of the form "Package (.+) ..." + - Subprocess launch(ed with variable|ing should be audited) + - (Expect directory permissions to be 0750 or less|Expect file permissions to be 0600 or less) + - (G104|G307) run: timeout: 10m diff --git a/test/framework/alltypes_helpers.go b/test/framework/alltypes_helpers.go index 55d5277e70c2..7832ca928c61 100644 --- a/test/framework/alltypes_helpers.go +++ b/test/framework/alltypes_helpers.go @@ -138,7 +138,7 @@ func dumpObject(resource runtime.Object, logPath string) { namespace := metaObj.GetNamespace() name := metaObj.GetName() - resourceFilePath := path.Join(logPath, namespace, kind, name+".yaml") + resourceFilePath := filepath.Clean(path.Join(logPath, namespace, kind, name+".yaml")) Expect(os.MkdirAll(filepath.Dir(resourceFilePath), 0755)).To(Succeed(), "Failed to create folder %s", filepath.Dir(resourceFilePath)) f, err := os.OpenFile(resourceFilePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) diff --git a/test/framework/bootstrap/kind_util.go b/test/framework/bootstrap/kind_util.go index 698d81f2150f..013cdd71858f 100644 --- a/test/framework/bootstrap/kind_util.go +++ b/test/framework/bootstrap/kind_util.go @@ -152,7 +152,7 @@ func save(ctx context.Context, image, dest string) error { // copied from kind https://github.com/kubernetes-sigs/kind/blob/v0.7.0/pkg/cmd/kind/load/docker-image/docker-image.go#L158 // loads an image tarball onto a node. func load(imageTarName string, node kindnodes.Node) error { - f, err := os.Open(imageTarName) + f, err := os.Open(filepath.Clean(imageTarName)) if err != nil { return errors.Wrap(err, "failed to open image") } diff --git a/test/framework/deployment_helpers.go b/test/framework/deployment_helpers.go index ec09b6e78195..88361a395ade 100644 --- a/test/framework/deployment_helpers.go +++ b/test/framework/deployment_helpers.go @@ -120,7 +120,7 @@ func WatchDeploymentLogs(ctx context.Context, input WatchDeploymentLogsInput) { go func(pod corev1.Pod, container corev1.Container) { defer GinkgoRecover() - logFile := path.Join(input.LogPath, input.Deployment.Name, pod.Name, container.Name+".log") + logFile := filepath.Clean(path.Join(input.LogPath, input.Deployment.Name, pod.Name, container.Name+".log")) Expect(os.MkdirAll(filepath.Dir(logFile), 0755)).To(Succeed()) f, err := os.OpenFile(logFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) diff --git a/test/framework/kubetest/setup.go b/test/framework/kubetest/setup.go index 8302000d1a45..ccd4d88f1ff8 100644 --- a/test/framework/kubetest/setup.go +++ b/test/framework/kubetest/setup.go @@ -20,6 +20,7 @@ import ( "io" "os" "path" + "path/filepath" ) func copyFile(srcFilePath, destFilePath string) error { @@ -27,7 +28,7 @@ func copyFile(srcFilePath, destFilePath string) error { if err != nil { return err } - srcFile, err := os.Open(srcFilePath) + srcFile, err := os.Open(filepath.Clean(srcFilePath)) if err != nil { return err } diff --git a/test/framework/namespace_helpers.go b/test/framework/namespace_helpers.go index 74416099886a..c237f9642803 100644 --- a/test/framework/namespace_helpers.go +++ b/test/framework/namespace_helpers.go @@ -129,7 +129,7 @@ func WatchNamespaceEvents(ctx context.Context, input WatchNamespaceEventsInput) Expect(input.ClientSet).NotTo(BeNil(), "input.ClientSet is required for WatchNamespaceEvents") Expect(input.Name).NotTo(BeEmpty(), "input.Name is required for WatchNamespaceEvents") - logFile := path.Join(input.LogFolder, "resources", input.Name, "events.log") + logFile := filepath.Clean(path.Join(input.LogFolder, "resources", input.Name, "events.log")) Expect(os.MkdirAll(filepath.Dir(logFile), 0755)).To(Succeed()) f, err := os.OpenFile(logFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) diff --git a/test/infrastructure/docker/docker/machine.go b/test/infrastructure/docker/docker/machine.go index 71796c879acc..01370c5498a0 100644 --- a/test/infrastructure/docker/docker/machine.go +++ b/test/infrastructure/docker/docker/machine.go @@ -259,7 +259,7 @@ func (m *Machine) PreloadLoadImages(ctx context.Context, images []string) error defer os.RemoveAll(dir) for i, image := range images { - imageTarPath := filepath.Join(dir, fmt.Sprintf("image-%d.tar", i)) + imageTarPath := filepath.Clean(filepath.Join(dir, fmt.Sprintf("image-%d.tar", i))) err = exec.Command("docker", "save", "-o", imageTarPath, image).Run() if err != nil {