Add Static Analysis (#1465)

jfrog · Mar 7, 2022 · 86374a1 · 86374a1
1 parent f4a948f
commit 86374a1
Show file tree

Hide file tree

Showing 34 changed files with 218 additions and 192 deletions.
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -1,4 +1,5 @@
 - [ ] All [tests](https://github.com/jfrog/jfrog-cli#tests) passed. If this feature is not already covered by the tests, I added new tests.
+- [ ] All [static analysis checks](https://github.com/jfrog/jfrog-cli/actions/workflows/analysis.yml) passed.
 - [ ] This pull request is on the dev branch.
 - [ ] I used gofmt for formatting the code before submitting the pull request.
 -----
diff --git a/.github/workflows/analysis.yml b/.github/workflows/analysis.yml
@@ -0,0 +1,29 @@
+name: "Static Analysis"
+on: ["push", "pull_request"]
+jobs:
+  Static-Check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout Source
+        uses: actions/checkout@v2
+      - name: Install Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17.x
+      - name: Static Code Analysis
+        uses: dominikh/staticcheck-action@v1
+
+  Go-Sec:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout Source
+        uses: actions/checkout@v2
+      - name: Install Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.17.x
+      - name: Install gosec
+        run: curl -sfL https://raw.githubusercontent.com/securego/gosec/master/install.sh | sh -s -- -b $(go env GOPATH)/bin
+      - name: Run gosec
+        # Temporary ignoring G301,G302,G306
+        run: gosec -exclude=G204,G301,G302,G304,G306 -exclude-dir=\.*test\.* ./...
diff --git a/README.md b/README.md
@@ -9,12 +9,12 @@
 
 </div>
 
-| Branch |                                                                                    Status                                                                                    |
-| :----: | :--------------------------------------------------------------------------------------------------------------------------------------------------------------------------: |
-|   v2   |   [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=v2)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml)   |
-|  dev   |  [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=dev)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml)   |
-|   v1   |   [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=v1)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml)   |
-| dev-v1 | [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=dev-v1)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml) |
+| Branch |                                                                                                                                                                              Status                                                                                                                                                                              |
+|:------:|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------:|
+|   v2   |     [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=v2)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml) [![Static Analysis](https://github.com/jfrog/jfrog-cli/actions/workflows/analysis.yml/badge.svg?branch=v2)](https://github.com/jfrog/jfrog-cli/actions/workflows/analysis.yml)      |
+|  dev   | [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=dev)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml)        [![Static Analysis](https://github.com/jfrog/jfrog-cli/actions/workflows/analysis.yml/badge.svg?branch=dev)](https://github.com/jfrog/jfrog-cli/actions/workflows/analysis.yml) |
+|   v1   |                                                                                             [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=v1)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml)                                                                                             |
+| dev-v1 |                                                                                           [![JFrog CLI Tests](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml/badge.svg?branch=dev-v1)](https://github.com/jfrog/jfrog-cli/actions/workflows/tests.yml)                                                                                           |
 
 # Table of Contents
 

diff --git a/artifactory/cli.go b/artifactory/cli.go
@@ -23,7 +23,6 @@ import (
 	"github.com/jfrog/jfrog-cli-core/v2/artifactory/utils"
 	containerutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils/container"
 	"github.com/jfrog/jfrog-cli-core/v2/common/commands"
-	coreCommonCommands "github.com/jfrog/jfrog-cli-core/v2/common/commands"
 	"github.com/jfrog/jfrog-cli-core/v2/common/spec"
 	corecommon "github.com/jfrog/jfrog-cli-core/v2/docs/common"
 	coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config"
@@ -1846,7 +1845,7 @@ func curlCmd(c *cli.Context) error {
 }
 
 func newRtCurlCommand(c *cli.Context) (*curl.RtCurlCommand, error) {
-	curlCommand := coreCommonCommands.NewCurlCommand().SetArguments(cliutils.ExtractCommand(c))
+	curlCommand := commands.NewCurlCommand().SetArguments(cliutils.ExtractCommand(c))
 	rtCurlCommand := curl.NewRtCurlCommand(*curlCommand)
 	rtDetails, err := rtCurlCommand.GetServerDetails()
 	if err != nil {
@@ -1870,8 +1869,8 @@ func pipDeprecatedInstallCmd(c *cli.Context) error {
 	// Get pip configuration.
 	pipConfig, err := utils.GetResolutionOnlyConfiguration(utils.Pip)
 	if err != nil {
-		return errors.New(fmt.Sprintf("Error occurred while attempting to read pip-configuration file: %s\n"+
-			"Please run 'jfrog rt pip-config' command prior to running 'jfrog rt %s'.", err.Error(), "pip-install"))
+		return fmt.Errorf("error occurred while attempting to read pip-configuration file: %s\n"+
+			"Please run 'jfrog rt pip-config' command prior to running 'jfrog rt %s'", err.Error(), "pip-install")
 	}
 
 	// Set arg values.

diff --git a/artifactory_test.go b/artifactory_test.go
@@ -1109,6 +1109,7 @@ func TestArtifactoryUploadAsArchive(t *testing.T) {
 	assert.NoError(t, err)
 	runRt(t, "upload", "--spec="+uploadSpecFile)
 	searchFilePath, err := tests.CreateSpec(tests.SearchAllRepo1)
+	assert.NoError(t, err)
 	verifyExistInArtifactory(tests.GetUploadAsArchive(), searchFilePath, t)
 
 	// Verify the properties are valid
@@ -1190,6 +1191,7 @@ func TestArtifactoryUploadAsArchiveWithIncludeDirs(t *testing.T) {
 	assert.NoError(t, fileutils.RemoveTempDir(tests.Out), "Couldn't remove temp dir")
 	assert.NoError(t, os.MkdirAll(tests.Out, 0777))
 	downloadSpecFile, err = tests.CreateSpec(tests.DownloadWithoutExplodeArchives)
+	assert.NoError(t, err)
 	runRt(t, "download", "--spec="+downloadSpecFile)
 	// Change working directory to the zip file's location and unzip it.
 	wd, err := os.Getwd()
@@ -1322,6 +1324,7 @@ func TestArtifactorySelfSignedCert(t *testing.T) {
 	fileSpec := spec.NewBuilder().Pattern(tests.RtRepo1 + "/*.zip").Recursive(true).BuildSpec()
 	assert.NoError(t, err)
 	parsedUrl, err := url.Parse(serverDetails.ArtifactoryUrl)
+	assert.NoError(t, err)
 	serverDetails.ArtifactoryUrl = "https://127.0.0.1:" + cliproxy.GetProxyHttpsPort() + parsedUrl.RequestURI()
 
 	// The server is using self-signed certificates
@@ -1383,6 +1386,7 @@ func TestArtifactoryClientCert(t *testing.T) {
 	fileSpec := spec.NewBuilder().Pattern(tests.RtRepo1 + "/*.zip").Recursive(true).BuildSpec()
 	assert.NoError(t, err)
 	parsedUrl, err := url.Parse(serverDetails.ArtifactoryUrl)
+	assert.NoError(t, err)
 	serverDetails.ArtifactoryUrl = "https://127.0.0.1:" + cliproxy.GetProxyHttpsPort() + parsedUrl.RequestURI()
 	serverDetails.InsecureTls = true
 
@@ -1536,6 +1540,7 @@ func prepareArtifactoryUrlForProxyTest(t *testing.T) string {
 	rtUrl, err := url.Parse(serverDetails.ArtifactoryUrl)
 	assert.NoError(t, err)
 	rtHost, port, err := net.SplitHostPort(rtUrl.Host)
+	assert.NoError(t, err)
 	if rtHost == "localhost" || rtHost == "127.0.0.1" {
 		externalIp, err := getExternalIP()
 		assert.NoError(t, err)
@@ -2183,8 +2188,8 @@ func TestSymlinkInsideSymlinkDirWithRecursionIssueUpload(t *testing.T) {
 
 func validateSymLink(localLinkPath, localFilePath string, t *testing.T) {
 	// In macOS, localFilePath may lead to /var/folders/dn instead of /private/var/folders/dn.
-	// EvalSymlinks in localLinkPath should fix it.
-	localFilePath, err := filepath.EvalSymlinks(localLinkPath)
+	// EvalSymlinks on localFilePath will fix it a head of the comparison at the end of this function.
+	localFilePath, err := filepath.EvalSymlinks(localFilePath)
 	assert.NoError(t, err)
 
 	exists := fileutils.IsPathSymlink(localLinkPath)
@@ -3236,6 +3241,7 @@ func TestArtifactoryDownloadByArchiveEntriesCli(t *testing.T) {
 
 func triggerArchiveIndexing(t *testing.T) {
 	client, err := httpclient.ClientBuilder().Build()
+	assert.NoError(t, err)
 	resp, _, err := client.SendPost(serverDetails.ArtifactoryUrl+"api/archiveIndex/"+tests.RtRepo1, []byte{}, artHttpDetails, "")
 	if err != nil {
 		assert.NoError(t, err, "archive indexing failed")
@@ -4848,7 +4854,7 @@ func TestArtifactoryReplicationCreate(t *testing.T) {
 	runRt(t, "rpldel", tests.RtRepo1)
 
 	// Validate delete replication
-	result, err = servicesManager.GetReplication(tests.RtRepo1)
+	_, err = servicesManager.GetReplication(tests.RtRepo1)
 	assert.Error(t, err)
 	// Cleanup
 	cleanArtifactoryTest()
@@ -5093,6 +5099,7 @@ func simpleUploadWithAntPatternSpec(t *testing.T) {
 	assert.NoError(t, err)
 	verifyExistInArtifactory(tests.GetSimpleAntPatternUploadExpectedRepo1(), searchFilePath, t)
 	searchFilePath, err = tests.CreateSpec(tests.SearchRepo1NonExistFile)
+	assert.NoError(t, err)
 	verifyDoesntExistInArtifactory(searchFilePath, t)
 }
 
@@ -5118,6 +5125,7 @@ func TestUploadWithAntPatternAndExclusionsSpec(t *testing.T) {
 	assert.NoError(t, err)
 	verifyExistInArtifactory(tests.GetAntPatternUploadWithExclusionsExpectedRepo1(), searchFilePath, t)
 	searchFilePath, err = tests.CreateSpec(tests.SearchRepo1NonExistFileAntExclusions)
+	assert.NoError(t, err)
 	verifyDoesntExistInArtifactory(searchFilePath, t)
 	cleanArtifactoryTest()
 }
@@ -5312,11 +5320,12 @@ func prepareTerraformProject(projectName string, t *testing.T, copyDirs bool) st
 	// Copy terraform tests to test environment, so we can change project's config file.
 	assert.NoError(t, fileutils.CopyDir(projectPath, testdataTarget, copyDirs, nil))
 	paths, err := fileutils.ListFilesRecursiveWalkIntoDirSymlink(testdataTarget, false)
+	assert.NoError(t, err)
 	for _, f := range paths {
 		fmt.Println(f)
 	}
 	configFileDir := filepath.Join(filepath.FromSlash(testdataTarget), ".jfrog", "projects")
-	configFileDir, err = tests.ReplaceTemplateVariables(filepath.Join(configFileDir, "terraform.yaml"), configFileDir)
+	_, err = tests.ReplaceTemplateVariables(filepath.Join(configFileDir, "terraform.yaml"), configFileDir)
 	assert.NoError(t, err)
 	return testdataTarget
 }
diff --git a/buildinfo_test.go b/buildinfo_test.go
@@ -916,5 +916,4 @@ type buildAddDepsBuildInfoTestParams struct {
 	expectedDependencies []string
 	buildName            string
 	buildNumber          string
-	validationFunc       func(*testing.T, buildAddDepsBuildInfoTestParams)
 }