Merge pull request #597 from hashicorp/brandonc/fix_linter

Fix or ignore all outsanding golangci-lint errors

.github/workflows/golangci-lint.yml

@@ -1,23 +1,16 @@
 name: Linter
 on:
   push:
-    branches: [ main ]
+    branches: [main]
   pull_request:
 jobs:
-  golangci:
-    name: lint
+  lint:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
-      - name: Set local Go version
-        run: |
-          VERSION=`cat .go-version| awk '{printf$1}'`
-          echo "go_version=$VERSION" >> $GITHUB_ENV
-      - name: Setup Go Environment
-        uses: actions/setup-go@v3
+      - uses: actions/setup-go@v3
         with:
-          go-version: "${{ env.go_version }}"
-      - name: golangci-lint
-        uses: golangci/golangci-lint-action@v2
+          go-version-file: 'go.mod'
+      - uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.42
+          version: v1.50.1

.golangci.yml

@@ -20,19 +20,27 @@ linters:
     - misspell #https://github.com/client9/misspell
 issues:
   exclude-rules:
+  - path: tfe_integration_test.go
+    linters:
+    - errcheck # Many calls in this test are known to return an error and not checked
+  - linters:
+      - stylecheck
+    text: "Ascii" # Permanently part of the public interface unless we break the API
   - path: _test\.go
     linters:
     - unused
     - deadcode
+    - unparam
 linters-settings:
   errcheck:
     # https://github.com/kisielk/errcheck#excluding-functions
     check-type-assertions: true
-    check-blank: true  
+    check-blank: true
   goconst:
-    min-len: 20 
-    min-occurrences: 5 
+    min-len: 20
+    min-occurrences: 5
     ignore-calls: false
+    ignore-tests: true
   gocritic:
     enabled-tags:
       - diagnostic

admin_setting_cost_estimation_integration_test.go

@@ -26,11 +26,11 @@ func TestAdminSettings_CostEstimation_Update(t *testing.T) {
 	client := testClient(t)
 	ctx := context.Background()
 
-	costEstimationSettings, err := client.Admin.Settings.CostEstimation.Read(ctx)
+	_, err := client.Admin.Settings.CostEstimation.Read(ctx)
 	require.NoError(t, err)
 
 	costEnabled := false
-	costEstimationSettings, err = client.Admin.Settings.CostEstimation.Update(ctx, AdminCostEstimationSettingOptions{
+	costEstimationSettings, err := client.Admin.Settings.CostEstimation.Update(ctx, AdminCostEstimationSettingOptions{
 		Enabled: Bool(costEnabled),
 	})
 	require.NoError(t, err)

admin_setting_saml_integration_test.go

@@ -42,13 +42,13 @@ func TestAdminSettings_SAML_Update(t *testing.T) {
 	client := testClient(t)
 	ctx := context.Background()
 
-	samlSettings, err := client.Admin.Settings.SAML.Read(ctx)
+	_, err := client.Admin.Settings.SAML.Read(ctx)
 	require.NoError(t, err)
 
 	enabled := false
 	debug := false
 
-	samlSettings, err = client.Admin.Settings.SAML.Update(ctx, AdminSAMLSettingsUpdateOptions{
+	samlSettings, err := client.Admin.Settings.SAML.Update(ctx, AdminSAMLSettingsUpdateOptions{
 		Enabled: Bool(enabled),
 		Debug:   Bool(debug),
 	})

admin_terraform_version_integration_test.go

@@ -103,7 +103,7 @@ func TestAdminTerraformVersions_CreateDelete(t *testing.T) {
 		opts := AdminTerraformVersionCreateOptions{
 			Version:          String(version),
 			URL:              String("https://www.hashicorp.com"),
-			Sha:              String(genSha(t, "secret", "data")),
+			Sha:              String(genSha(t)),
 			Deprecated:       Bool(true),
 			DeprecatedReason: String("Test Reason"),
 			Official:         Bool(false),
@@ -133,7 +133,7 @@ func TestAdminTerraformVersions_CreateDelete(t *testing.T) {
 		opts := AdminTerraformVersionCreateOptions{
 			Version: String(version),
 			URL:     String("https://www.hashicorp.com"),
-			Sha:     String(genSha(t, "secret", "data")),
+			Sha:     String(genSha(t)),
 		}
 		tfv, err := client.Admin.TerraformVersions.Create(ctx, opts)
 		require.NoError(t, err)
@@ -170,7 +170,7 @@ func TestAdminTerraformVersions_ReadUpdate(t *testing.T) {
 		opts := AdminTerraformVersionCreateOptions{
 			Version:          String(version),
 			URL:              String("https://www.hashicorp.com"),
-			Sha:              String(genSha(t, "secret", "data")),
+			Sha:              String(genSha(t)),
 			Official:         Bool(false),
 			Deprecated:       Bool(true),
 			DeprecatedReason: String("Test Reason"),

admin_user_integration_test.go

@@ -130,7 +130,6 @@ func TestAdminUsers_Delete(t *testing.T) {
 		assert.Empty(t, ul.Items)
 		assert.Equal(t, 1, ul.CurrentPage)
 		assert.Equal(t, 0, ul.TotalCount)
-
 	})
 
 	t.Run("an non-existing user", func(t *testing.T) {

admin_workspace_integration_test.go

@@ -130,15 +130,15 @@ func TestAdminWorkspaces_Read(t *testing.T) {
 		assert.Nil(t, workspace)
 	})
 
-	t.Run("it fails to read a workspace that is non existant", func(t *testing.T) {
+	t.Run("it fails to read a workspace that is non existent", func(t *testing.T) {
 		workspaceID := fmt.Sprintf("non-existent-%s", randomString(t))
 		adminWorkspace, err := client.Admin.Workspaces.Read(ctx, workspaceID)
 		require.Error(t, err)
 		assert.EqualError(t, err, ErrResourceNotFound.Error())
 		assert.Nil(t, adminWorkspace)
 	})
 
-	t.Run("it reads a worksapce successfully", func(t *testing.T) {
+	t.Run("it reads a workspace successfully", func(t *testing.T) {
 		org, orgCleanup := createOrganization(t, client)
 		defer orgCleanup()
 

agent_pool_integration_test.go

@@ -69,7 +69,6 @@ func TestAgentPoolsList(t *testing.T) {
 	})
 
 	t.Run("with query options", func(t *testing.T) {
-
 		pools, err := client.AgentPools.List(ctx, orgTest.Name, &AgentPoolListOptions{
 			Query: agentPool.Name,
 		})

configuration_version.go

@@ -252,13 +252,13 @@ func (s *configurationVersions) ReadWithOptions(ctx context.Context, cvID string
 // Upload packages and uploads Terraform configuration files. It requires the
 // upload URL from a configuration version and the path to the configuration
 // files on disk.
-func (s *configurationVersions) Upload(ctx context.Context, url string, path string) error {
+func (s *configurationVersions) Upload(ctx context.Context, uploadURL, path string) error {
 	body, err := packContents(path)
 	if err != nil {
 		return err
 	}
 
-	req, err := s.client.NewRequest("PUT", url, body)
+	req, err := s.client.NewRequest("PUT", uploadURL, body)
 	if err != nil {
 		return err
 	}

docs/CONTRIBUTING.md

@@ -16,7 +16,9 @@ There are instances where several new resources being added (i.e Workspace Run T
 
 **Note HashiCorp Employees Only:** When submitting a new set of endpoints please ensure that one of your respective team members approves the changes as well before merging.
 
-## Running the Linters Locally
+## Linting
+
+Linting is not necessarily required for a change to be merged, but it helps smooth the review process and catch common mistakes early. If you'd like to run the linters manually, follow these steps:
 
 1. Ensure you have [installed golangci-lint](https://golangci-lint.run/usage/install/#local-installation)
 2. From the CLI, run `make lint`

errors.go

@@ -313,7 +313,7 @@ var (
 
 	ErrRequiredFilename = errors.New("filename is required")
 
-	ErrInvalidAsciiArmor = errors.New("ascii armor is invalid")
+	ErrInvalidAsciiArmor = errors.New("ASCII Armor is invalid")
 
 	ErrRequiredNamespace = errors.New("namespace is required for public registry")
 

helper_test.go

@@ -114,16 +114,16 @@ func fetchTestAccountDetails(t *testing.T, client *Client) *TestAccountDetails {
 	return _testAccountDetails
 }
 
-func downloadFile(filepath string, url string) error {
+func downloadFile(filePath, fileURL string) error {
 	// Get the data
-	resp, err := http.Get(url)
+	resp, err := http.Get(fileURL)
 	if err != nil {
 		return err
 	}
 	defer resp.Body.Close()
 
 	// Create the file
-	out, err := os.Create(filepath)
+	out, err := os.Create(filePath)
 	if err != nil {
 		return err
 	}
@@ -145,11 +145,13 @@ func unzip(src, dest string) error {
 		}
 	}()
 
-	os.MkdirAll(dest, 0755)
+	if err := os.MkdirAll(dest, 0o755); err != nil {
+		return err
+	}
 
 	// Closure to address file descriptors issue with all the deferred .Close() methods
-	extractAndWriteFile := func(f *zip.File) error {
-		rc, err := f.Open()
+	extractAndWriteFile := func(zf *zip.File) error {
+		rc, err := zf.Open()
 		if err != nil {
 			return err
 		}
@@ -159,32 +161,34 @@ func unzip(src, dest string) error {
 			}
 		}()
 
-		path := filepath.Join(dest, f.Name)
+		path := filepath.Join(dest, zf.Name)
 
 		// Check for ZipSlip (Directory traversal)
 		if !strings.HasPrefix(path, filepath.Clean(dest)+string(os.PathSeparator)) {
 			return fmt.Errorf("illegal file path: %s", path)
 		}
 
-		if f.FileInfo().IsDir() {
-			os.MkdirAll(path, f.Mode())
-		} else {
-			os.MkdirAll(filepath.Dir(path), f.Mode())
-			f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
-			if err != nil {
-				return err
+		if zf.FileInfo().IsDir() {
+			return os.MkdirAll(path, zf.Mode())
+		}
+		if err := os.MkdirAll(filepath.Dir(path), zf.Mode()); err != nil {
+			return err
+		}
+		f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, zf.Mode())
+		if err != nil {
+			return err
+		}
+		defer func() {
+			if err := f.Close(); err != nil {
+				panic(err)
 			}
-			defer func() {
-				if err := f.Close(); err != nil {
-					panic(err)
-				}
-			}()
+		}()
 
-			_, err = io.Copy(f, rc)
-			if err != nil {
-				return err
-			}
+		_, err = io.Copy(f, rc)
+		if err != nil {
+			return err
 		}
+
 		return nil
 	}
 
@@ -226,6 +230,7 @@ func createAgent(t *testing.T, client *Client, org *Organization) (*Agent, *Agen
 	var orgCleanup func()
 	var agentPoolTokenCleanup func()
 	var agent *Agent
+	var ok bool
 
 	if org == nil {
 		org, orgCleanup = createOrganization(t, client)
@@ -258,9 +263,11 @@ func createAgent(t *testing.T, client *Client, org *Organization) (*Agent, *Agen
 
 	cmd := exec.Command(agentPath)
 	cmd.Env = os.Environ()
-	cmd.Env = append(cmd.Env, "TFC_AGENT_TOKEN="+agentPoolToken.Token)
-	cmd.Env = append(cmd.Env, "TFC_AGENT_NAME="+"test-agent")
-	cmd.Env = append(cmd.Env, "TFC_ADDRESS="+DefaultConfig().Address)
+	cmd.Env = append(cmd.Env,
+		"TFC_AGENT_TOKEN="+agentPoolToken.Token,
+		"TFC_AGENT_NAME="+"test-agent",
+		"TFC_ADDRESS="+DefaultConfig().Address,
+	)
 
 	go func() {
 		_, err := cmd.CombinedOutput()
@@ -270,11 +277,12 @@ func createAgent(t *testing.T, client *Client, org *Organization) (*Agent, *Agen
 	}()
 
 	t.Cleanup(func() {
-		cmd.Process.Kill()
+		if err := cmd.Process.Kill(); err != nil {
+			t.Error(err)
+		}
 	})
 
 	i, err := retry(func() (interface{}, error) {
-
 		agentList, err := client.Agents.List(ctx, agentPool.ID, nil)
 		if err != nil {
 			return nil, err
@@ -290,7 +298,10 @@ func createAgent(t *testing.T, client *Client, org *Organization) (*Agent, *Agen
 		t.Fatalf("Could not return an agent %s", err)
 	}
 
-	agent = i.(*Agent)
+	agent, ok = i.(*Agent)
+	if !ok {
+		t.Fatalf("Expected type to be *Agent but got %T", agent)
+	}
 
 	return agent, agentPool, cleanup
 }
@@ -926,9 +937,8 @@ func createPolicyCheckedRun(t *testing.T, client *Client, w *Workspace) (*Run, f
 func createPlannedRun(t *testing.T, client *Client, w *Workspace) (*Run, func()) {
 	if paidFeaturesDisabled() {
 		return createRunWaitForStatus(t, client, w, RunPlanned)
-	} else {
-		return createRunWaitForStatus(t, client, w, RunCostEstimated)
 	}
+	return createRunWaitForStatus(t, client, w, RunCostEstimated)
 }
 
 func createCostEstimatedRun(t *testing.T, client *Client, w *Workspace) (*Run, func()) {
@@ -1403,7 +1413,7 @@ func createRegistryProviderPlatform(t *testing.T, client *Client, provider *Regi
 	options := RegistryProviderPlatformCreateOptions{
 		OS:       randomString(t),
 		Arch:     randomString(t),
-		Shasum:   genSha(t, "secret", "data"),
+		Shasum:   genSha(t),
 		Filename: randomString(t),
 	}
 
@@ -1856,7 +1866,7 @@ func createVariableSet(t *testing.T, client *Client, org *Organization, options
 	}
 }
 
-func applyVariableSetToWorkspace(t *testing.T, client *Client, vsID string, wsID string) {
+func applyVariableSetToWorkspace(t *testing.T, client *Client, vsID, wsID string) {
 	if vsID == "" {
 		t.Fatal("variable set ID must not be empty")
 	}
@@ -2077,9 +2087,10 @@ func retry(f retryableFn) (interface{}, error) { //nolint
 	return retryTimes(9, 3, f) // 10 attempts over 30 seconds
 }
 
-func genSha(t *testing.T, secret, data string) string {
-	h := hmac.New(sha256.New, []byte(secret))
-	_, err := h.Write([]byte(data))
+func genSha(t *testing.T) string {
+	t.Helper()
+	h := hmac.New(sha256.New, []byte("secret"))
+	_, err := h.Write([]byte("data"))
 	if err != nil {
 		t.Fatalf("error writing hmac: %s", err)
 	}
@@ -2116,7 +2127,7 @@ func randomStringWithoutSpecialChar(t *testing.T) string {
 	if err != nil {
 		t.Fatal(err)
 	}
-	uuidWithoutHyphens := strings.Replace(v, "-", "", -1)
+	uuidWithoutHyphens := strings.ReplaceAll(v, "-", "")
 	return uuidWithoutHyphens
 }
 
@@ -2130,6 +2141,7 @@ func containsProject(pl []*Project, str string) bool {
 }
 
 func randomSemver(t *testing.T) string {
+	t.Helper()
 	return fmt.Sprintf("%d.%d.%d", rand.Intn(99)+3, rand.Intn(99)+1, rand.Intn(99)+1)
 }
 

ip_ranges.go

@@ -47,7 +47,7 @@ func (i *ipRanges) Read(ctx context.Context, modifiedSince string) (*IPRange, er
 	}
 
 	ir := &IPRange{}
-	err = req.doIpRanges(ctx, ir)
+	err = req.doIPRanges(ctx, ir)
 	if err != nil {
 		return nil, err
 	}