Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix or ignore all outsanding golangci-lint errors #597

Merged
merged 3 commits into from Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 6 additions & 13 deletions .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
14 changes: 11 additions & 3 deletions .golangci.yml
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions admin_setting_cost_estimation_integration_test.go
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions admin_setting_saml_integration_test.go
Expand Up @@ -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),
})
Expand Down
6 changes: 3 additions & 3 deletions admin_terraform_version_integration_test.go
Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"),
Expand Down
1 change: 0 additions & 1 deletion admin_user_integration_test.go
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions admin_workspace_integration_test.go
Expand Up @@ -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()

Expand Down
1 change: 0 additions & 1 deletion agent_pool_integration_test.go
Expand Up @@ -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,
})
Expand Down
4 changes: 2 additions & 2 deletions configuration_version.go
Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion docs/CONTRIBUTING.md
Expand Up @@ -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`
Expand Down
2 changes: 1 addition & 1 deletion errors.go
Expand Up @@ -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")

Expand Down
86 changes: 49 additions & 37 deletions helper_test.go
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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),
}

Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand All @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion ip_ranges.go
Expand Up @@ -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
}
Expand Down