Skip to content

Commit

Permalink
chore: Adding bugs Lint Preset (#3334)
Browse files Browse the repository at this point in the history
* chore: Hiding mocks tests

* fix: Removing terratest log parser

* fix: Moving mocks generation

* fix: Fixing mock tests again

* fix: Still need mockery for unit tests, it seems

* chore: Bumping `golangci-lint` to `v1.59.1`

* chore: Addressing lint errors

* fix: Fixing lint configs

* fix: Fixing magic numbers for file permissions

* fix: Resolving remaining lints

* chore: Adding `bugs` preset to lints

* feat: Addressing `bugs` preset lint findings
  • Loading branch information
yhakbar committed Aug 12, 2024
1 parent beabb5e commit cb78603
Show file tree
Hide file tree
Showing 95 changed files with 1,789 additions and 1,689 deletions.
3 changes: 2 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ linters:
fast: false
mnd:
ignored-functions: strconv.Format*,os.*,strconv.Parse*,strings.SplitN,bytes.SplitN

presets:
- bugs
issues:
exclude-dirs:
- docs
Expand Down
3 changes: 2 additions & 1 deletion aws_helper/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ type tokenFetcher string
func (f tokenFetcher) FetchToken(ctx credentials.Context) ([]byte, error) {
// Check if token is a raw value
if _, err := os.Stat(string(f)); err != nil {
return []byte(f), nil
// TODO: See if this lint error should be ignored
return []byte(f), nil //nolint: nilerr
}
token, err := os.ReadFile(string(f))
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion aws_helper/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import (
"github.com/aws/aws-sdk-go/service/sts"
"github.com/gruntwork-io/terragrunt/options"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestTerragruntIsAddedInUserAgent(t *testing.T) {
t.Parallel()

sess, err := CreateAwsSession(nil, options.NewTerragruntOptions())
assert.NoError(t, err)
require.NoError(t, err)

op := &request.Operation{
Name: "",
Expand Down
17 changes: 9 additions & 8 deletions aws_helper/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const simplePolicy = `
Expand Down Expand Up @@ -53,9 +54,9 @@ func TestUnmarshalStringActionResource(t *testing.T) {
t.Parallel()

bucketPolicy, err := UnmarshalPolicy(simplePolicy)
assert.NoError(t, err)
require.NoError(t, err)
assert.NotNil(t, bucketPolicy)
assert.Equal(t, 1, len(bucketPolicy.Statement))
assert.Len(t, bucketPolicy.Statement, 1)
assert.NotNil(t, bucketPolicy.Statement[0].Action)
assert.NotNil(t, bucketPolicy.Statement[0].Resource)

Expand All @@ -74,36 +75,36 @@ func TestUnmarshalStringActionResource(t *testing.T) {
}

out, err := MarshalPolicy(bucketPolicy)
assert.NoError(t, err)
require.NoError(t, err)
assert.NotContains(t, string(out), "null")
}

func TestUnmarshalActionResourceList(t *testing.T) {
t.Parallel()
bucketPolicy, err := UnmarshalPolicy(arraysPolicy)
assert.NoError(t, err)
require.NoError(t, err)
assert.NotNil(t, bucketPolicy)
assert.Equal(t, 1, len(bucketPolicy.Statement))
require.Len(t, bucketPolicy.Statement, 1)
assert.NotNil(t, bucketPolicy.Statement[0].Action)
assert.NotNil(t, bucketPolicy.Statement[0].Resource)

switch actions := bucketPolicy.Statement[0].Action.(type) {
case []interface{}:
assert.Equal(t, 11, len(actions))
require.Len(t, actions, 11)
assert.Contains(t, actions, "s3:ListJobs")
default:
assert.Fail(t, "Expected []string type for Action")
}

switch resource := bucketPolicy.Statement[0].Resource.(type) {
case []interface{}:
assert.Equal(t, 2, len(resource))
require.Len(t, resource, 2)
assert.Contains(t, resource, "arn:aws:s3:*:666:job/*")
default:
assert.Fail(t, "Expected []string type for Resource")
}

out, err := MarshalPolicy(bucketPolicy)
assert.NoError(t, err)
require.NoError(t, err)
assert.NotContains(t, string(out), "null")
}
5 changes: 3 additions & 2 deletions cli/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,13 @@ func wrapWithTelemetry(opts *options.TerragruntOptions) func(ctx *cli.Context, a
return err
}

return runAction(ctx, opts, action)
// TODO: See if this lint should be ignored
return runAction(ctx, opts, action) //nolint:contextcheck
})
}
}

func beforeAction(opts *options.TerragruntOptions) cli.ActionFunc {
func beforeAction(_ *options.TerragruntOptions) cli.ActionFunc {
return func(ctx *cli.Context) error {
// setting current context to the options
// show help if the args are not specified.
Expand Down
6 changes: 3 additions & 3 deletions cli/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestParseTerragruntOptionsFromArgs(t *testing.T) {
if testCase.expectedErr != nil {
assert.EqualError(t, actualErr, testCase.expectedErr.Error())
} else {
assert.Nil(t, actualErr, "Unexpected error: %v", actualErr)
require.NoError(t, actualErr)
assertOptionsEqual(t, *testCase.expectedOptions, *actualOptions, "For args %v", testCase.args)
}
}
Expand Down Expand Up @@ -322,7 +322,7 @@ func TestParseMultiStringArg(t *testing.T) {
if testCase.expectedErr != nil {
assert.EqualError(t, actualErr, testCase.expectedErr.Error())
} else {
assert.Nil(t, actualErr, "Unexpected error: %q", actualErr)
require.NoError(t, actualErr)
assert.Equal(t, testCase.expectedVals, actualOptions.ModulesThatInclude, "For args %q", testCase.args)
}
}
Expand Down Expand Up @@ -355,7 +355,7 @@ func TestParseMutliStringKeyValueArg(t *testing.T) {
if testCase.expectedErr != nil {
assert.ErrorContains(t, actualErr, testCase.expectedErr.Error())
} else {
assert.Nil(t, actualErr, "Unexpected error: %v", actualErr)
require.NoError(t, actualErr)
assert.Equal(t, testCase.expectedVals, actualOptions.AwsProviderPatchOverrides, "For args %v", testCase.args)
}
}
Expand Down
8 changes: 4 additions & 4 deletions cli/commands/aws-provider-patch/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ func runAwsProviderPatch(ctx context.Context, opts *options.TerragruntOptions, c

// The format we expect in the .terraform/modules/modules.json file
type TerraformModulesJson struct {
Modules []TerraformModule
Modules []TerraformModule `json:"Modules"`
}

type TerraformModule struct {
Key string
Source string
Dir string
Key string `json:"Key"`
Source string `json:"Source"`
Dir string `json:"Dir"`
}

// findAllTerraformFiles returns all Terraform source files within the modules being used by this Terragrunt
Expand Down
3 changes: 2 additions & 1 deletion cli/commands/aws-provider-patch/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const terraformCodeExampleOutputOnly = `
Expand Down Expand Up @@ -318,7 +319,7 @@ func TestPatchAwsProviderInTerraformCodeHappyPath(t *testing.T) {
t.Run(testCase.testName, func(t *testing.T) {
t.Parallel()
actualTerraformCode, actualCodeWasUpdated, err := patchAwsProviderInTerraformCode(testCase.originalTerraformCode, "test.tf", testCase.attributesToOverride)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, testCase.expectedCodeWasUpdated, actualCodeWasUpdated)

// We check an array of possible expected code here due to possible ordering differences. That is, the
Expand Down
3 changes: 2 additions & 1 deletion cli/commands/catalog/module/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestFindModules(t *testing.T) {
Expand Down Expand Up @@ -61,7 +62,7 @@ func TestFindModules(t *testing.T) {
ctx := context.Background()

repo, err := NewRepo(ctx, testCase.repoPath, "")
assert.NoError(t, err)
require.NoError(t, err)

modules, err := repo.FindModules(ctx)
assert.Equal(t, testCase.expectedErr, err)
Expand Down
3 changes: 2 additions & 1 deletion cli/commands/catalog/tui/tui.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tui

import (
"context"
"errors"

tea "github.com/charmbracelet/bubbletea"

Expand All @@ -11,7 +12,7 @@ import (

func Run(ctx context.Context, modules module.Modules, opts *options.TerragruntOptions) error {
if _, err := tea.NewProgram(newModel(modules, opts), tea.WithAltScreen(), tea.WithContext(ctx)).Run(); err != nil {
if err := context.Cause(ctx); err == context.Canceled {
if err := context.Cause(ctx); errors.Is(err, context.Canceled) {
return nil
} else if err != nil {
return err
Expand Down
5 changes: 4 additions & 1 deletion cli/commands/flags.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package commands

import (
"errors"

"github.com/gruntwork-io/terragrunt/options"
"github.com/gruntwork-io/terragrunt/pkg/cli"
"github.com/gruntwork-io/terragrunt/shell"
Expand Down Expand Up @@ -362,7 +364,8 @@ func NewHelpFlag(opts *options.TerragruntOptions) cli.Flag {
err := cli.ShowCommandHelp(ctx, cmdName)

// If the command name is not found, it is most likely a terraform command, show Terraform help.
if _, ok := err.(cli.InvalidCommandNameError); ok {
var invalidCommandNameError cli.InvalidCommandNameError
if ok := errors.As(err, &invalidCommandNameError); ok {
terraformHelpCmd := append([]string{cmdName, "-help"}, ctx.Args().Tail()...)
return shell.RunTerraformCommand(ctx, opts, terraformHelpCmd...)
}
Expand Down
3 changes: 2 additions & 1 deletion cli/commands/run-all/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ func TestMissingRunAllArguments(t *testing.T) {
err = Run(context.Background(), tgOptions)
require.Error(t, err)

_, ok := errors.Unwrap(err).(MissingCommand)
var missingCommand MissingCommand
ok := errors.As(err, &missingCommand)
fmt.Println(err, errors.Unwrap(err))
assert.True(t, ok)
}
2 changes: 1 addition & 1 deletion cli/commands/scaffold/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestDefaultTemplateVariables(t *testing.T) {
cfg, err := config.ReadTerragruntConfig(context.Background(), opts, config.DefaultParserOptions(opts))
require.NoError(t, err)
require.NotEmpty(t, cfg.Inputs)
require.Equal(t, 1, len(cfg.Inputs))
require.Len(t, cfg.Inputs, 1)
_, found := cfg.Inputs["required_var_1"]
require.True(t, found)
require.Equal(t, "git::https://github.com/gruntwork-io/terragrunt.git//test/fixture-inputs?ref=v0.53.8", *cfg.Terraform.Source)
Expand Down
47 changes: 25 additions & 22 deletions cli/commands/terraform/download_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package terraform

import (
"context"
"errors"
"fmt"
"io"
"net/url"
Expand All @@ -12,7 +13,6 @@ import (
"testing"

"github.com/gruntwork-io/go-commons/env"
"github.com/gruntwork-io/go-commons/errors"
"github.com/gruntwork-io/terragrunt/terraform"
"github.com/sirupsen/logrus"

Expand Down Expand Up @@ -250,12 +250,13 @@ func TestDownloadTerraformSourceIfNecessaryInvalidTerraformSource(t *testing.T)

terraformSource, terragruntOptions, terragruntConfig, err := createConfig(t, canonicalUrl, downloadDir, false)

assert.NoError(t, err)
require.NoError(t, err)

err = downloadTerraformSourceIfNecessary(context.Background(), terraformSource, terragruntOptions, terragruntConfig)
assert.Error(t, err)
_, ok := errors.Unwrap(err).(DownloadingTerraformSourceErr)
assert.True(t, ok)
require.Error(t, err)
var downloadingTerraformSourceErr DownloadingTerraformSourceErr
ok := errors.As(err, &downloadingTerraformSourceErr)
require.True(t, ok)
}

func TestInvalidModulePath(t *testing.T) {
Expand All @@ -268,13 +269,14 @@ func TestInvalidModulePath(t *testing.T) {
copyFolder(t, "../../../test/fixture-download-source/hello-world-version-remote", downloadDir)

terraformSource, _, _, err := createConfig(t, canonicalUrl, downloadDir, false)
assert.NoError(t, err)
require.NoError(t, err)
terraformSource.WorkingDir += "/not-existing-path"

err = validateWorkingDir(terraformSource)
assert.Error(t, err)
_, ok := errors.Unwrap(err).(WorkingDirNotFound)
assert.True(t, ok)
require.Error(t, err)
var workingDirNotFound WorkingDirNotFound
ok := errors.As(err, &workingDirNotFound)
require.True(t, ok)
}

func TestDownloadInvalidPathToFilePath(t *testing.T) {
Expand All @@ -287,13 +289,14 @@ func TestDownloadInvalidPathToFilePath(t *testing.T) {
copyFolder(t, "../../../test/fixture-download-source/hello-world-version-remote", downloadDir)

terraformSource, _, _, err := createConfig(t, canonicalUrl, downloadDir, false)
assert.NoError(t, err)
require.NoError(t, err)
terraformSource.WorkingDir += "/main.tf"

err = validateWorkingDir(terraformSource)
assert.Error(t, err)
_, ok := errors.Unwrap(err).(WorkingDirNotDir)
assert.True(t, ok)
require.Error(t, err)
var workingDirNotDir WorkingDirNotDir
ok := errors.As(err, &workingDirNotDir)
require.True(t, ok)
}

func TestDownloadTerraformSourceFromLocalFolderWithManifest(t *testing.T) {
Expand Down Expand Up @@ -363,20 +366,20 @@ func TestDownloadTerraformSourceFromLocalFolderWithManifest(t *testing.T) {
func testDownloadTerraformSourceIfNecessary(t *testing.T, canonicalUrl string, downloadDir string, sourceUpdate bool, expectedFileContents string, requireInitFile bool) {
terraformSource, terragruntOptions, terragruntConfig, err := createConfig(t, canonicalUrl, downloadDir, sourceUpdate)

assert.NoError(t, err)
require.NoError(t, err)

err = downloadTerraformSourceIfNecessary(context.Background(), terraformSource, terragruntOptions, terragruntConfig)
require.NoError(t, err, "For terraform source %v: %v", terraformSource, err)

expectedFilePath := util.JoinPath(downloadDir, "main.tf")
if assert.True(t, util.FileExists(expectedFilePath), "For terraform source %v", terraformSource) {
actualFileContents := readFile(t, expectedFilePath)
assert.Equal(t, expectedFileContents, actualFileContents, "For terraform source %v", terraformSource)
require.Equal(t, expectedFileContents, actualFileContents, "For terraform source %v", terraformSource)
}

if requireInitFile {
existsInitFile := util.FileExists(util.JoinPath(terraformSource.WorkingDir, moduleInitRequiredFile))
assert.True(t, existsInitFile)
require.True(t, existsInitFile)
}
}

Expand All @@ -392,7 +395,7 @@ func createConfig(t *testing.T, canonicalUrl string, downloadDir string, sourceU
}

terragruntOptions, err := options.NewTerragruntOptionsForTest("./should-not-be-used")
assert.Nil(t, err, "Unexpected error creating NewTerragruntOptionsForTest: %v", err)
require.NoError(t, err)

terragruntOptions.SourceUpdate = sourceUpdate
terragruntOptions.Env = env.Parse(os.Environ())
Expand All @@ -405,7 +408,7 @@ func createConfig(t *testing.T, canonicalUrl string, downloadDir string, sourceU
}

err = PopulateTerraformVersion(context.Background(), terragruntOptions)
assert.Nil(t, err, "For terraform source %v: %v", terraformSource, err)
require.NoError(t, err)
return terraformSource, terragruntOptions, terragruntConfig, err
}

Expand All @@ -421,11 +424,11 @@ func testAlreadyHaveLatestCode(t *testing.T, canonicalUrl string, downloadDir st
}

opts, err := options.NewTerragruntOptionsForTest("./should-not-be-used")
assert.Nil(t, err, "Unexpected error creating NewTerragruntOptionsForTest: %v", err)
require.NoError(t, err)

actual, err := alreadyHaveLatestCode(terraformSource, opts)
assert.Nil(t, err, "Unexpected error for terraform source %v: %v", terraformSource, err)
assert.Equal(t, expected, actual, "For terraform source %v", terraformSource)
require.NoError(t, err)
require.Equal(t, expected, actual, "For terraform source %v", terraformSource)
}

func tmpDir(t *testing.T) string {
Expand Down Expand Up @@ -465,5 +468,5 @@ func readFile(t *testing.T, path string) string {

func copyFolder(t *testing.T, src string, dest string) {
err := util.CopyFolderContents(filepath.FromSlash(src), filepath.FromSlash(dest), ".terragrunt-test", nil)
require.Nil(t, err)
require.NoError(t, err)
}
7 changes: 4 additions & 3 deletions cli/commands/terraform/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ func processErrorHooks(ctx context.Context, hooks []config.ErrorHook, terragrunt
// https://github.com/gruntwork-io/terragrunt/issues/2045
originalError := errors.Unwrap(e)
if originalError != nil {
processError, cast := originalError.(util.ProcessExecutionError)
if cast {
errorMessage = fmt.Sprintf("%s\n%s", processError.StdOut, processError.Stderr)
var processExecutionError util.ProcessExecutionError
ok := errors.As(originalError, &processExecutionError)
if ok {
errorMessage = fmt.Sprintf("%s\n%s", processExecutionError.StdOut, processExecutionError.Stderr)
}
}
result = fmt.Sprintf("%s\n%s", result, errorMessage)
Expand Down
Loading

0 comments on commit cb78603

Please sign in to comment.