Skip to content

Commit

Permalink
Enable excluded lint checks
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
stmcginnis committed May 17, 2021
1 parent 799da05 commit 6a24e15
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 6 deletions.
11 changes: 11 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/framework/alltypes_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/framework/bootstrap/kind_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion test/framework/deployment_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion test/framework/kubetest/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ import (
"io"
"os"
"path"
"path/filepath"
)

func copyFile(srcFilePath, destFilePath string) error {
err := os.MkdirAll(path.Dir(destFilePath), 0o750)
if err != nil {
return err
}
srcFile, err := os.Open(srcFilePath)
srcFile, err := os.Open(filepath.Clean(srcFilePath))
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion test/framework/namespace_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/infrastructure/docker/docker/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 6a24e15

Please sign in to comment.