From db5b60644d0b33b7a003198d5fb85deb8dbb791a Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Mon, 21 Jul 2025 10:31:52 +0800 Subject: [PATCH 1/9] feat: add additional gha workflows --- .github/workflows/golangci-lint.yml | 27 +++++++++++++++++++++++++++ .github/workflows/labeler.yml | 15 +++++++++++++++ .github/workflows/stale.yml | 27 +++++++++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 .github/workflows/labeler.yml create mode 100644 .github/workflows/stale.yml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 0000000..3fb534b --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,27 @@ +name: golangci-lint +on: + push: + tags: + - v* + branches: + - master + - main + pull_request: + paths-ignore: + - "*.md" +permissions: + contents: read + pull-requests: read +jobs: + golangci: + name: Run golangci-lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - uses: golangci/golangci-lint-action@v7 + with: + version: v2.1 + args: --verbose diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml new file mode 100644 index 0000000..34c455f --- /dev/null +++ b/.github/workflows/labeler.yml @@ -0,0 +1,15 @@ +name: "Issue Labeler" +on: + issues: + types: [opened] + +jobs: + triage: + name: "Triage issues" + runs-on: ubuntu-latest + steps: + - uses: github/issue-labeler@v3.0 + with: + repo-token: "${{ secrets.GITHUB_TOKEN }}" + configuration-path: .github/labeler.yml + enable-versioned-regex: 0 diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml new file mode 100644 index 0000000..e72f1c7 --- /dev/null +++ b/.github/workflows/stale.yml @@ -0,0 +1,27 @@ +name: "Close stale issues and PRs" +on: + schedule: + # Runs at 15:00 UTC every day. + # Actions schedules run at most every 5 minutes. + - cron: "0 15 * * *" +permissions: + issues: write + pull-requests: write +jobs: + stale: + name: Manage stale issues and PRs + runs-on: ubuntu-latest + steps: + - uses: actions/stale@v7 + with: + stale-issue-message: "This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 365 days." + stale-pr-message: "This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 365 days." + close-issue-message: "This issue was closed because it has been stalled for 365 days with no activity." + close-pr-message: "This PR was closed because it has been stalled for 365 days with no activity." + days-before-issue-stale: 30 + days-before-pr-stale: 30 + days-before-issue-close: 365 + days-before-pr-close: 365 + stale-issue-label: stale + stale-pr-label: stale + exempt-issue-labels: accepted From f369296d3b297c285b1276ae38c6506c271a1057 Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Mon, 21 Jul 2025 18:28:10 +0800 Subject: [PATCH 2/9] chore: update golangci-lint config --- .golangci.yml | 183 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 136 insertions(+), 47 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 6b6aca6..e7ece0b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,62 +1,151 @@ +version: "2" run: - skip-dirs-use-default: true - skip-dirs: - - vendor - - ent - - graph - skip-files: - - ".*\\.peg\\.go$" - - ".*ignored_.*.go$" - - "generated.go$" - - model_gen.go - + modules-download-mode: readonly + issues-exit-code: 1 + tests: true linters: - disable-all: true enable: + - asasalint + - asciicheck + - bidichk - bodyclose - - depguard - - dogsled + - containedctx + - contextcheck + - cyclop + - dupl + - durationcheck + - errorlint + - exhaustive - funlen + - gocognit - goconst - gocritic - gocyclo - - gofmt - - goimports - - revive - - gomnd + #- godot + - goheader + - gomoddirectives + - gomodguard + - goprintffuncname - gosec + - makezero - misspell + - mnd + - nakedret + - nestif + - nilerr + - nilnil + - noctx + - nolintlint + - prealloc + - predeclared + - promlinter + - reassign + - rowserrcheck + - sqlclosecheck + - staticcheck + - testableexamples + - thelper - unconvert - - typecheck + - unparam + - usestdlibvars + - wastedassign + - whitespace + settings: + cyclop: + max-complexity: 11 + dupl: + threshold: 100 + errcheck: + check-type-assertions: true + check-blank: false + errorlint: + errorf: true + asserts: true + comparison: true + exhaustive: + default-signifies-exhaustive: false + funlen: + lines: 100 + statements: 50 + gocognit: + min-complexity: 15 + goconst: + min-len: 3 + min-occurrences: 3 + gocritic: + disabled-checks: + - dupImport + - ifElseChain + - octalLiteral + - whyNoLint + - wrapperFunc + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + gocyclo: + min-complexity: 15 + godot: + scope: declarations + exclude: + - "^fixme:" + - "^todo:" + capital: false + misspell: + locale: US + nestif: + min-complexity: 4 + prealloc: + simple: true + range-loops: true + for-loops: false + staticcheck: + checks: + - all + - -ST1000 + - -ST1003 + - -ST1016 + - -ST1020 + - -ST1021 + - -ST1022 + dot-import-whitelist: + - fmt + unparam: + check-exported: false + whitespace: + multi-if: false + multi-func: false + exclusions: + generated: lax - # default - - errcheck - #- govet - - ineffassign - #- staticcheck - - unused + presets: + - comments + - common-false-positives + - legacy + - std-error-handling -linters-settings: - dupl: - threshold: 100 - funlen: - lines: 100 - statements: 60 - depguard: - list-type: blacklist - packages: - - golang.org/x/net/context - - github.com/prometheus/common/log -# gocyclo: -# min-complexity: 15 + rules: [] + warn-unused: true issues: - # Excluding configuration per-path, per-linter, per-text and per-source - exclude-rules: - - path: _test\.go - linters: - - gomnd - # https://github.com/go-critic/go-critic/issues/926 - - linters: - - gocritic - text: "unnecessaryDefer:" + max-issues-per-linter: 0 + max-same-issues: 0 + new: false + fix: false +formatters: + enable: + - gci + - gofmt + - gofumpt + - goimports + settings: + gci: + sections: + - standard + - default + - prefix(github.com/robmorgan/infraspec) + goimports: + local-prefixes: + - github.com/robmorgan/infraspec From 0dee36c4cb79e9507d324201672a17837e826718 Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Mon, 21 Jul 2025 18:28:27 +0800 Subject: [PATCH 3/9] chore: fix warnings --- cmd/init.go | 2 +- cmd/new.go | 6 +- internal/collections/collections.go | 14 +++- internal/collections/collections_test.go | 6 +- internal/config/config.go | 4 +- internal/config/logging.go | 12 +-- internal/generators/generators.go | 50 ++++++----- internal/runner/runner.go | 25 ++---- internal/telemetry/telemetry.go | 11 ++- pkg/assertions/aws/dynamodb.go | 12 ++- pkg/assertions/aws/rds.go | 12 +-- pkg/assertions/aws/s3.go | 12 +-- pkg/awshelpers/auth.go | 2 +- pkg/awshelpers/region.go | 22 ++--- pkg/awshelpers/region_test.go | 1 + pkg/iacprovisioner/cmd.go | 2 +- pkg/iacprovisioner/format.go | 11 ++- pkg/iacprovisioner/options.go | 54 ++++++------ pkg/iacprovisioner/output.go | 20 ++--- pkg/retry/retry.go | 5 +- pkg/retry/retry_test.go | 2 +- pkg/shell/command.go | 55 +++++++----- pkg/shell/output.go | 4 +- pkg/ssh/agent.go | 9 +- pkg/ssh/ssh.go | 43 +++++----- pkg/ssh/ssh_test.go | 16 ++-- pkg/steps/aws/aws.go | 1 - pkg/steps/aws/rds.go | 15 +--- pkg/steps/steps.go | 19 +---- pkg/steps/terraform/set_variable.go | 102 ----------------------- script/build.go | 4 +- test/aws_test.go | 7 +- test/terraform_test.go | 3 +- 33 files changed, 233 insertions(+), 330 deletions(-) delete mode 100644 pkg/steps/terraform/set_variable.go diff --git a/cmd/init.go b/cmd/init.go index bcc557e..9d66d52 100644 --- a/cmd/init.go +++ b/cmd/init.go @@ -29,7 +29,7 @@ func runInit(cmd *cobra.Command, args []string) error { } // Create the features directory - if err := os.MkdirAll(featuresDir, 0o755); err != nil { + if err := os.MkdirAll(featuresDir, 0o755); err != nil { //nolint:mnd return fmt.Errorf("failed to create features directory: %w", err) } diff --git a/cmd/new.go b/cmd/new.go index 9626f77..2250df0 100644 --- a/cmd/new.go +++ b/cmd/new.go @@ -47,7 +47,7 @@ func runNew(cmd *cobra.Command, args []string) error { } // Create features directory if it doesn't exist - if err := os.MkdirAll("features", 0o755); err != nil { + if err := os.MkdirAll("features", 0o755); err != nil { //nolint:mnd return fmt.Errorf("failed to create features directory: %w", err) } @@ -63,7 +63,7 @@ func runNew(cmd *cobra.Command, args []string) error { Then the service should be healthy ` - if err := os.WriteFile(filePath, []byte(template), 0o644); err != nil { + if err := os.WriteFile(filePath, []byte(template), 0o600); err != nil { //nolint:mnd return fmt.Errorf("failed to create feature file: %w", err) } @@ -86,7 +86,7 @@ func getFeatureName(filePath string) string { // Simple title case conversion words := strings.Fields(name) for i, word := range words { - if len(word) > 0 { + if word != "" { words[i] = strings.ToUpper(word[:1]) + strings.ToLower(word[1:]) } } diff --git a/internal/collections/collections.go b/internal/collections/collections.go index 916903a..14e1980 100644 --- a/internal/collections/collections.go +++ b/internal/collections/collections.go @@ -1,7 +1,8 @@ package collections import ( - "math/rand" + "crypto/rand" + "math/big" "slices" ) @@ -53,6 +54,15 @@ func Subtract[T comparable](a, b []T) []T { return result } +// cryptoRandInt generates a cryptographically secure random integer in range [0, limit) +func cryptoRandInt(limit int) int { + n, err := rand.Int(rand.Reader, big.NewInt(int64(limit))) + if err != nil { + panic(err) // TODO - we may wish to handle this more gracefully + } + return int(n.Int64()) +} + // RandomElement returns a random element from the slice, and a boolean indicating whether the slice was empty. func RandomElement[T any](slice []T) (T, bool) { if len(slice) == 0 { @@ -60,6 +70,6 @@ func RandomElement[T any](slice []T) (T, bool) { return zero, false } - index := rand.Intn(len(slice)) + index := cryptoRandInt(len(slice)) return slice[index], true } diff --git a/internal/collections/collections_test.go b/internal/collections/collections_test.go index 17deacf..1fc5fc8 100644 --- a/internal/collections/collections_test.go +++ b/internal/collections/collections_test.go @@ -207,7 +207,7 @@ func TestSubtractWithStrings(t *testing.T) { } } -func TestRandomElement(t *testing.T) { +func TestRandomElement(t *testing.T) { //nolint:gocognit tests := []struct { name string slice []interface{} @@ -244,7 +244,7 @@ func TestRandomElement(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.iterations == 1 { + if tt.iterations == 1 { //nolint:nestif value, ok := RandomElement(tt.slice) if ok != tt.wantOk { t.Errorf("RandomElement() ok = %v, want %v", ok, tt.wantOk) @@ -273,7 +273,7 @@ func TestRandomElement(t *testing.T) { } } -func TestRandomElementTyped(t *testing.T) { +func TestRandomElementTyped(t *testing.T) { //nolint:gocognit tests := []struct { name string slice interface{} diff --git a/internal/config/config.go b/internal/config/config.go index 9f1ec1a..db10c7c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,6 +9,8 @@ import ( yaml "gopkg.in/yaml.v3" ) +var randomStringLength = 6 + // Config represents the main configuration structure type Config struct { Version string `yaml:"version"` @@ -72,7 +74,7 @@ func DefaultConfig() (*Config, error) { Provider: "aws", Functions: Functions{ RandomString: RandomStringConfig{ - Length: 6, + Length: randomStringLength, Charset: "abcdefghijklmnopqrstuvwxyz0123456789", }, }, diff --git a/internal/config/logging.go b/internal/config/logging.go index cfc21f1..080d603 100644 --- a/internal/config/logging.go +++ b/internal/config/logging.go @@ -35,7 +35,6 @@ func init() { encoderCfg.EncodeLevel = zapcore.CapitalColorLevelEncoder encoderCfg.EncodeDuration = nil encoderCfg.EncodeTime = nil - // encoderCfg.TimeKey = "" encoderCfg.EncodeCaller = nil logger = zap.New(zapcore.NewCore( @@ -44,17 +43,10 @@ func init() { Logging.AtomicLogLevel, )) - defer logger.Sync() // flushes buffer, if any + defer logger.Sync() //nolint:errcheck // flushes buffer, if any log = logger.Sugar() Logging.FastLogger = logger Logging.Logger = log - - // cfg.DisableStacktrace = true - // cfg.EncoderConfig.EncodeCaller = nil - // if os.Getenv("INFRASPEC_DEBUG") != "" { - // cfg.DisableStacktrace = false - // cfg.EncoderConfig.EncodeCaller = zapcore.ShortCallerEncoder - // } } func (logging) setLogLevel(lvl zapcore.Level) { @@ -72,7 +64,7 @@ func (logging) SetDevelopmentLogger() { func(zapcore.Core) zapcore.Core { return zapcore.NewCore(zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()), zapcore.AddSync(os.Stderr), Logging.AtomicLogLevel) })) - // zap.ReplaceGlobals(clone) + defer logger.Sync() //nolint:errcheck log = clone.Sugar() diff --git a/internal/generators/generators.go b/internal/generators/generators.go index d695019..66c17aa 100644 --- a/internal/generators/generators.go +++ b/internal/generators/generators.go @@ -1,18 +1,32 @@ package generators import ( - "math/rand" - "time" + "crypto/rand" + "math/big" "github.com/robmorgan/infraspec/internal/config" ) -func init() { - // Seed the random number generator - rand.Seed(time.Now().UnixNano()) +var minPasswordLength = 8 + +// cryptoRandInt generates a cryptographically secure random integer in range [0, limit) +func cryptoRandInt(limit int) int { + n, err := rand.Int(rand.Reader, big.NewInt(int64(limit))) + if err != nil { + panic(err) // In practice, you might want to handle this more gracefully + } + return int(n.Int64()) +} + +// cryptoShuffle performs a cryptographically secure Fisher-Yates shuffle +func cryptoShuffle(slice []byte) { + for i := len(slice) - 1; i > 0; i-- { + j := cryptoRandInt(i + 1) + slice[i], slice[j] = slice[j], slice[i] + } } -// RandomResourceName generates a random resource name with the given prefix +// RandomResourceName generates a random resource name with the given prefix. func RandomResourceName(prefix string, cfg config.RandomStringConfig) string { // Use default values if not specified length := cfg.Length @@ -27,13 +41,13 @@ func RandomResourceName(prefix string, cfg config.RandomStringConfig) string { result := make([]byte, length) for i := range result { - result[i] = charset[rand.Intn(len(charset))] + result[i] = charset[cryptoRandInt(len(charset))] } return prefix + string(result) } -// RandomDNSName generates a DNS-compliant random name +// RandomDNSName generates a DNS-compliant random name. func RandomDNSName(prefix string, cfg config.RandomStringConfig) string { // DNS names can only contain letters, numbers, and hyphens charset := "abcdefghijklmnopqrstuvwxyz0123456789" @@ -44,7 +58,7 @@ func RandomDNSName(prefix string, cfg config.RandomStringConfig) string { }) } -// RandomPassword generates a random password meeting common requirements +// RandomPassword generates a random password meeting common requirements. func RandomPassword(cfg config.RandomStringConfig) string { // Ensure password includes required character types lowercase := "abcdefghijklmnopqrstuvwxyz" @@ -53,29 +67,27 @@ func RandomPassword(cfg config.RandomStringConfig) string { special := "!@#$%^&*" length := cfg.Length - if length < 8 { - length = 8 // minimum safe password length + if length < minPasswordLength { + length = minPasswordLength // minimum safe password length } // Generate password with at least one of each required type password := make([]byte, length) // First four characters are guaranteed to have one of each type - password[0] = lowercase[rand.Intn(len(lowercase))] - password[1] = uppercase[rand.Intn(len(uppercase))] - password[2] = numbers[rand.Intn(len(numbers))] - password[3] = special[rand.Intn(len(special))] + password[0] = lowercase[cryptoRandInt(len(lowercase))] + password[1] = uppercase[cryptoRandInt(len(uppercase))] + password[2] = numbers[cryptoRandInt(len(numbers))] + password[3] = special[cryptoRandInt(len(special))] // Fill the rest randomly allChars := lowercase + uppercase + numbers + special for i := 4; i < length; i++ { - password[i] = allChars[rand.Intn(len(allChars))] + password[i] = allChars[cryptoRandInt(len(allChars))] } // Shuffle the password to avoid predictable patterns - rand.Shuffle(len(password), func(i, j int) { - password[i], password[j] = password[j], password[i] - }) + cryptoShuffle(password) return string(password) } diff --git a/internal/runner/runner.go b/internal/runner/runner.go index a179bda..2357291 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -28,7 +28,7 @@ func New(cfg *config.Config) *Runner { // Run executes the specified feature file func (r *Runner) Run(featurePath string) error { - defer config.Logging.Logger.Sync() + defer config.Logging.Logger.Sync() //nolint:errcheck // flushes buffer, if any // Validate feature file exists if _, err := os.Stat(featurePath); os.IsNotExist(err) { @@ -104,7 +104,10 @@ func (r *Runner) initializeScenario(sc *godog.ScenarioContext) { // If a Terraform configuration was applied, destroy it if contexthelpers.GetTerraformHasApplied(ctx) { config.Logging.Logger.Debug("Terraform has been applied, destroying resources") - terraform.NewTerraformDestroyStep(ctx) + ctx, err = terraform.NewTerraformDestroyStep(ctx) + if err != nil { + config.Logging.Logger.Error("Error destroying Terraform resources", err) + } } return ctx, nil }) @@ -112,7 +115,7 @@ func (r *Runner) initializeScenario(sc *godog.ScenarioContext) { // cleanup performs necessary cleanup after test execution // TODO - this might be necessary if we've invoked tools like Terraform or need to cleanup resources -func (r *Runner) cleanup() error { +func (r *Runner) cleanup() error { //nolint:unparam if !r.cfg.Cleanup.Automatic { config.Logging.Logger.Debug("Automatic cleanup disabled, skipping...") return nil @@ -122,22 +125,6 @@ func (r *Runner) cleanup() error { zap.Int("timeout", r.cfg.Cleanup.Timeout), ) - // done := make(chan error) - // go func() { - // done <- r.context.Cleanup() - // }() - - // select { - // case err := <-done: - // if err != nil { - // return fmt.Errorf("cleanup failed: %w", err) - // } - // r.cfg.Logger.Info("Cleanup completed successfully") - // return nil - // case <-time.After(time.Duration(r.cfg.Cleanup.Timeout) * time.Second): - // return fmt.Errorf("cleanup timed out after %d seconds", r.cfg.Cleanup.Timeout) - // } - config.Logging.Logger.Debug("Cleanup completed successfully") return nil } diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index bac2b25..4ba4fdc 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -10,7 +10,12 @@ import ( ) const ( - AmplitudeAPIKey = "9dc54881885bd60f8ccbb9cef2dfaa7a" + AmplitudeAPIKey = "9dc54881885bd60f8ccbb9cef2dfaa7a" //nolint:gosec +) + +var ( + flushQueueSize = 10 + flushInterval = 10 * time.Second ) type Client struct { @@ -42,8 +47,8 @@ func New(cfg Config) *Client { } config := amplitude.NewConfig(AmplitudeAPIKey) - config.FlushQueueSize = 10 - config.FlushInterval = 10 * time.Second + config.FlushQueueSize = flushQueueSize + config.FlushInterval = flushInterval config.Logger = cfg.Logger client := amplitude.NewClient(config) diff --git a/pkg/assertions/aws/dynamodb.go b/pkg/assertions/aws/dynamodb.go index 2d9bf3e..b920ed2 100644 --- a/pkg/assertions/aws/dynamodb.go +++ b/pkg/assertions/aws/dynamodb.go @@ -33,7 +33,7 @@ func (a *AWSAsserter) AssertTableExists(tableName string) error { input := &dynamodb.ListTablesInput{} result, err := client.ListTables(context.TODO(), input) if err != nil { - return fmt.Errorf("error listing tables: %v", err) + return fmt.Errorf("error listing tables: %w", err) } // Check if the table exists @@ -64,7 +64,7 @@ func (a *AWSAsserter) AssertTableTags(tableName string, expectedTags map[string] result, err := client.ListTagsOfResource(context.TODO(), input) if err != nil { - return fmt.Errorf("error listing tags for table %s: %v", tableName, err) + return fmt.Errorf("error listing tags for table %s: %w", tableName, err) } // Convert the tags to a map @@ -151,7 +151,7 @@ func (a *AWSAsserter) getDynamoDBTable(tableName string) (*types.TableDescriptio result, err := client.DescribeTable(context.TODO(), input) if err != nil { - return nil, fmt.Errorf("error describing DynamoDB table %s: %v", tableName, err) + return nil, fmt.Errorf("error describing DynamoDB table %s: %w", tableName, err) } return result.Table, nil @@ -161,7 +161,7 @@ func (a *AWSAsserter) getDynamoDBTable(tableName string) (*types.TableDescriptio func (a *AWSAsserter) createDynamoDBClient() (*dynamodb.Client, error) { cfg, err := config.LoadDefaultConfig(context.TODO()) if err != nil { - return nil, fmt.Errorf("failed to load AWS config: %v", err) + return nil, fmt.Errorf("failed to load AWS config: %w", err) } return dynamodb.NewFromConfig(cfg), nil @@ -169,6 +169,10 @@ func (a *AWSAsserter) createDynamoDBClient() (*dynamodb.Client, error) { // Helper method to get the billing mode of a DynamoDB table func getDynamoDBBTableBillingMode(tableDesc *types.TableDescription) (types.BillingMode, error) { + if tableDesc == nil { + return "", fmt.Errorf("table description is nil") + } + // Note: as per https://github.com/aws/aws-sdk-go-v2/blob/main/service/dynamodb/types/types.go#L600-L601 // if we don't receive a response that includes the BillingModeSummary, it means the table is in PROVISIONED mode. billingMode := types.BillingModeProvisioned diff --git a/pkg/assertions/aws/rds.go b/pkg/assertions/aws/rds.go index eb2a5b2..de53e85 100644 --- a/pkg/assertions/aws/rds.go +++ b/pkg/assertions/aws/rds.go @@ -41,7 +41,7 @@ func (a *AWSAsserter) AssertRDSServiceAccess() error { _, err = client.DescribeAccountAttributes(context.TODO(), &rds.DescribeAccountAttributesInput{}) if err != nil { - return fmt.Errorf("error accessing the RDS service: %v", err) + return fmt.Errorf("error accessing the RDS service: %w", err) } return nil @@ -58,7 +58,7 @@ func (a *AWSAsserter) AssertRDSDescribeInstances() error { // Describe the DB instances _, err = client.DescribeDBInstances(context.TODO(), &rds.DescribeDBInstancesInput{}) if err != nil { - return fmt.Errorf("error describing DB instances: %v", err) + return fmt.Errorf("error describing DB instances: %w", err) } return nil @@ -78,7 +78,7 @@ func (a *AWSAsserter) AssertDBInstanceExists(dbInstanceID, region string) error result, err := client.DescribeDBInstances(context.TODO(), input) if err != nil { - return fmt.Errorf("error describing DB instance %s: %v", dbInstanceID, err) + return fmt.Errorf("error describing DB instance %s: %w", dbInstanceID, err) } // Check if the DB instance exists @@ -206,7 +206,7 @@ func (a *AWSAsserter) AssertDBInstanceTags(dbInstanceID string, expectedTags map result, err := client.ListTagsForResource(context.TODO(), input) if err != nil { - return fmt.Errorf("error listing tags for DB instance %s: %v", dbInstanceID, err) + return fmt.Errorf("error listing tags for DB instance %s: %w", dbInstanceID, err) } // Convert the tags to a map @@ -230,7 +230,7 @@ func (a *AWSAsserter) AssertDBInstanceTags(dbInstanceID string, expectedTags map } // Helper method to get a DB instance -func (a *AWSAsserter) getDBInstance(dbInstanceID string, region string) (*types.DBInstance, error) { +func (a *AWSAsserter) getDBInstance(dbInstanceID, region string) (*types.DBInstance, error) { client, err := awshelpers.NewRdsClient(region) if err != nil { return nil, err @@ -243,7 +243,7 @@ func (a *AWSAsserter) getDBInstance(dbInstanceID string, region string) (*types. result, err := client.DescribeDBInstances(context.TODO(), input) if err != nil { - return nil, fmt.Errorf("error describing DB instance %s: %v", dbInstanceID, err) + return nil, fmt.Errorf("error describing DB instance %s: %w", dbInstanceID, err) } // Check if the DB instance exists diff --git a/pkg/assertions/aws/s3.go b/pkg/assertions/aws/s3.go index 1e2f1f4..8479b21 100644 --- a/pkg/assertions/aws/s3.go +++ b/pkg/assertions/aws/s3.go @@ -33,7 +33,7 @@ func (a *AWSAsserter) AssertBucketExists(bucketName string) error { Bucket: aws.String(bucketName), }) if err != nil { - return fmt.Errorf("bucket %s does not exist or is not accessible: %v", bucketName, err) + return fmt.Errorf("bucket %s does not exist or is not accessible: %w", bucketName, err) } return nil @@ -51,7 +51,7 @@ func (a *AWSAsserter) AssertBucketVersioning(bucketName string) error { result, err := client.GetBucketVersioning(context.TODO(), input) if err != nil { - return fmt.Errorf("error getting bucket versioning for %s: %v", bucketName, err) + return fmt.Errorf("error getting bucket versioning for %s: %w", bucketName, err) } if result.Status != types.BucketVersioningStatusEnabled { @@ -73,7 +73,7 @@ func (a *AWSAsserter) AssertBucketEncryption(bucketName string) error { result, err := client.GetBucketEncryption(context.TODO(), input) if err != nil { - return fmt.Errorf("error getting bucket encryption for %s: %v", bucketName, err) + return fmt.Errorf("error getting bucket encryption for %s: %w", bucketName, err) } if result.ServerSideEncryptionConfiguration == nil || len(result.ServerSideEncryptionConfiguration.Rules) == 0 { @@ -95,7 +95,7 @@ func (a *AWSAsserter) AssertBucketPublicAccessBlock(bucketName string) error { result, err := client.GetPublicAccessBlock(context.TODO(), input) if err != nil { - return fmt.Errorf("error getting public access block for %s: %v", bucketName, err) + return fmt.Errorf("error getting public access block for %s: %w", bucketName, err) } if result.PublicAccessBlockConfiguration == nil { @@ -117,7 +117,7 @@ func (a *AWSAsserter) AssertBucketServerAccessLogging(bucketName string) error { result, err := client.GetBucketLogging(context.TODO(), input) if err != nil { - return fmt.Errorf("error getting bucket logging for %s: %v", bucketName, err) + return fmt.Errorf("error getting bucket logging for %s: %w", bucketName, err) } if result.LoggingEnabled == nil { @@ -131,7 +131,7 @@ func (a *AWSAsserter) AssertBucketServerAccessLogging(bucketName string) error { func (a *AWSAsserter) createS3Client() (*s3.Client, error) { cfg, err := config.LoadDefaultConfig(context.TODO()) if err != nil { - return nil, fmt.Errorf("failed to load AWS config: %v", err) + return nil, fmt.Errorf("failed to load AWS config: %w", err) } return s3.NewFromConfig(cfg), nil diff --git a/pkg/awshelpers/auth.go b/pkg/awshelpers/auth.go index 6534f17..e20c4a4 100644 --- a/pkg/awshelpers/auth.go +++ b/pkg/awshelpers/auth.go @@ -39,7 +39,7 @@ func NewAuthenticatedSessionFromDefaultCredentials(region string) (*aws.Config, // NewAuthenticatedSessionFromRole returns a new AWS Config after assuming the // role whose ARN is provided in roleARN. If the credentials are not properly // configured in the underlying environment, an error is returned. -func NewAuthenticatedSessionFromRole(region string, roleARN string) (*aws.Config, error) { +func NewAuthenticatedSessionFromRole(region, roleARN string) (*aws.Config, error) { cfg, err := NewAuthenticatedSessionFromDefaultCredentials(region) if err != nil { return nil, err diff --git a/pkg/awshelpers/region.go b/pkg/awshelpers/region.go index 34c1c65..00e7c13 100644 --- a/pkg/awshelpers/region.go +++ b/pkg/awshelpers/region.go @@ -46,7 +46,7 @@ var stableRegions = []string{ // further restrict the stable region list using approvedRegions and forbiddenRegions. We consider stable regions to be // those that have been around for at least 1 year. // Note that regions in the approvedRegions list that are not considered stable are ignored. -func GetRandomStableRegion(approvedRegions []string, forbiddenRegions []string) (string, error) { +func GetRandomStableRegion(approvedRegions, forbiddenRegions []string) (string, error) { regionsToPickFrom := stableRegions if len(approvedRegions) > 0 { regionsToPickFrom = collections.Intersection[string](regionsToPickFrom, approvedRegions) @@ -60,7 +60,7 @@ func GetRandomStableRegion(approvedRegions []string, forbiddenRegions []string) // GetRandomRegion gets a randomly chosen AWS region. If approvedRegions is not empty, this will be a region from the // approvedRegions list; otherwise, this method will fetch the latest list of regions from the AWS APIs and pick one of // those. If forbiddenRegions is not empty, this method will make sure the returned region is not in the forbiddenRegions list. -func GetRandomRegion(approvedRegions []string, forbiddenRegions []string) (string, error) { +func GetRandomRegion(approvedRegions, forbiddenRegions []string) (string, error) { regionFromEnvVar := os.Getenv(regionOverrideEnvVarName) if regionFromEnvVar != "" { config.Logging.Logger.Infof("Using AWS region %s from environment variable %s", regionFromEnvVar, regionOverrideEnvVarName) @@ -100,9 +100,9 @@ func GetAllAwsRegions() ([]string, error) { return nil, err } - var regions []string - for _, region := range out.Regions { - regions = append(regions, aws.ToString(region.RegionName)) + regions := make([]string, len(out.Regions)) + for i := range out.Regions { + regions[i] = aws.ToString(out.Regions[i].RegionName) } return regions, nil @@ -123,9 +123,9 @@ func GetAvailabilityZones(region string) ([]string, error) { return nil, err } - var out []string - for _, availabilityZone := range resp.AvailabilityZones { - out = append(out, aws.ToString(availabilityZone.ZoneName)) + out := make([]string, len(resp.AvailabilityZones)) + for i := range resp.AvailabilityZones { + out[i] = aws.ToString(resp.AvailabilityZones[i].ZoneName) } return out, nil @@ -148,9 +148,9 @@ func GetRegionsForService(serviceName string) ([]string, error) { return nil, err } - var availableRegions []string - for _, p := range resp.Parameters { - availableRegions = append(availableRegions, *p.Value) + availableRegions := make([]string, len(resp.Parameters)) + for i := range resp.Parameters { + availableRegions[i] = *resp.Parameters[i].Value } return availableRegions, nil diff --git a/pkg/awshelpers/region_test.go b/pkg/awshelpers/region_test.go index 9cf2fa4..bc8f0b9 100644 --- a/pkg/awshelpers/region_test.go +++ b/pkg/awshelpers/region_test.go @@ -42,6 +42,7 @@ func TestGetAllAwsRegions(t *testing.T) { } func assertLooksLikeRegionName(t *testing.T, regionName string) { + t.Helper() assert.Regexp(t, "[a-z]{2}-[a-z]+?-[[:digit:]]+", regionName) } diff --git a/pkg/iacprovisioner/cmd.go b/pkg/iacprovisioner/cmd.go index cd13360..c79386c 100644 --- a/pkg/iacprovisioner/cmd.go +++ b/pkg/iacprovisioner/cmd.go @@ -45,7 +45,7 @@ func generateCommand(options *Options, args ...string) shell.Command { } // GetCommonOptions extracts commons terraform options -func GetCommonOptions(options *Options, args ...string) (*Options, []string) { +func GetCommonOptions(options *Options, args ...string) (*Options, []string) { //nolint:gocritic if options.Binary == "" { options.Binary = DefaultExecutable } diff --git a/pkg/iacprovisioner/format.go b/pkg/iacprovisioner/format.go index 2412431..202245b 100644 --- a/pkg/iacprovisioner/format.go +++ b/pkg/iacprovisioner/format.go @@ -51,7 +51,7 @@ func FormatArgs(options *Options, args ...string) []string { planFileSupported := slices.Contains(TerraformCommandsWithPlanFileSupport, commandType) // Include -var and -var-file flags unless we're running 'apply' with a plan file - includeVars := !(commandType == "apply" && len(options.PlanFilePath) > 0) + includeVars := commandType != "apply" || options.PlanFilePath != "" terraformArgs = append(terraformArgs, args...) @@ -91,7 +91,7 @@ func FormatArgs(options *Options, args ...string) []string { // FormatTerraformPlanFileAsArg formats the out variable as a command-line arg for Terraform (e.g. of the format // -out=/some/path/to/plan.out or /some/path/to/plan.out). Only plan supports passing in the plan file as -out; the // other commands expect it as the first positional argument. This returns an empty string if outPath is empty string. -func FormatTerraformPlanFileAsArg(commandType string, outPath string) []string { +func FormatTerraformPlanFileAsArg(commandType, outPath string) []string { if outPath == "" { return nil } @@ -149,7 +149,7 @@ func FormatTerraformBackendConfigAsArgs(vars map[string]interface{}) []string { // useSpaceAsSeparator is false, an equals will separate the prefix and each var (e.g., -backend-config=foo=bar). If // omitNil is false, then nil values will be included, (e.g. -backend-config=foo=null). If // omitNil is true, then nil values will not be included, (e.g. -backend-config=foo). If -func formatTerraformArgs(vars map[string]interface{}, prefix string, useSpaceAsSeparator bool, omitNil bool) []string { +func formatTerraformArgs(vars map[string]interface{}, prefix string, useSpaceAsSeparator, omitNil bool) []string { var args []string for key, value := range vars { @@ -249,7 +249,7 @@ func mapToHclString(m map[string]interface{}) string { keyValuePairs := []string{} for key, value := range m { - keyValuePair := fmt.Sprintf(`"%s" = %s`, key, toHclString(value, true)) + keyValuePair := fmt.Sprintf(`"%q" = %q`, key, toHclString(value, true)) keyValuePairs = append(keyValuePairs, keyValuePair) } @@ -264,7 +264,6 @@ func primitiveToHclString(value interface{}, isNested bool) string { } switch v := value.(type) { - case bool: return strconv.FormatBool(v) @@ -274,7 +273,7 @@ func primitiveToHclString(value interface{}, isNested bool) string { return fmt.Sprintf("\"%v\"", v) } - return fmt.Sprintf("%v", v) + return v default: return fmt.Sprintf("%v", v) diff --git a/pkg/iacprovisioner/options.go b/pkg/iacprovisioner/options.go index c9b2ae5..b79d8de 100644 --- a/pkg/iacprovisioner/options.go +++ b/pkg/iacprovisioner/options.go @@ -8,29 +8,33 @@ import ( "github.com/robmorgan/infraspec/pkg/ssh" ) -var DefaultRetryableTerraformErrors = map[string]string{ - // Helm related terraform calls may fail when too many tests run in parallel. While the exact cause is unknown, - // this is presumably due to all the network contention involved. Usually a retry resolves the issue. - ".*read: connection reset by peer.*": "Failed to reach helm charts repository.", - ".*transport is closing.*": "Failed to reach Kubernetes API.", - - // `terraform init` frequently fails in CI due to network issues accessing plugins. The reason is unknown, but - // eventually these succeed after a few retries. - ".*unable to verify signature.*": "Failed to retrieve plugin due to transient network error.", - ".*unable to verify checksum.*": "Failed to retrieve plugin due to transient network error.", - ".*no provider exists with the given name.*": "Failed to retrieve plugin due to transient network error.", - ".*registry service is unreachable.*": "Failed to retrieve plugin due to transient network error.", - ".*Error installing provider.*": "Failed to retrieve plugin due to transient network error.", - ".*Failed to query available provider packages.*": "Failed to retrieve plugin due to transient network error.", - ".*timeout while waiting for plugin to start.*": "Failed to retrieve plugin due to transient network error.", - ".*timed out waiting for server handshake.*": "Failed to retrieve plugin due to transient network error.", - "could not query provider registry for": "Failed to retrieve plugin due to transient network error.", - - // Provider bugs where the data after apply is not propagated. This is usually an eventual consistency issue, so - // retrying should self resolve it. - // See https://github.com/terraform-providers/terraform-provider-aws/issues/12449 for an example. - ".*Provider produced inconsistent result after apply.*": "Provider eventual consistency error.", -} +var ( + DefaultMaxRetries = 3 + DefaultTimeBetweenRetries = 5 * time.Second + DefaultRetryableTerraformErrors = map[string]string{ + // Helm related terraform calls may fail when too many tests run in parallel. While the exact cause is unknown, + // this is presumably due to all the network contention involved. Usually a retry resolves the issue. + ".*read: connection reset by peer.*": "Failed to reach helm charts repository.", + ".*transport is closing.*": "Failed to reach Kubernetes API.", + + // `terraform init` frequently fails in CI due to network issues accessing plugins. The reason is unknown, but + // eventually these succeed after a few retries. + ".*unable to verify signature.*": "Failed to retrieve plugin due to transient network error.", + ".*unable to verify checksum.*": "Failed to retrieve plugin due to transient network error.", + ".*no provider exists with the given name.*": "Failed to retrieve plugin due to transient network error.", + ".*registry service is unreachable.*": "Failed to retrieve plugin due to transient network error.", + ".*Error installing provider.*": "Failed to retrieve plugin due to transient network error.", + ".*Failed to query available provider packages.*": "Failed to retrieve plugin due to transient network error.", + ".*timeout while waiting for plugin to start.*": "Failed to retrieve plugin due to transient network error.", + ".*timed out waiting for server handshake.*": "Failed to retrieve plugin due to transient network error.", + "could not query provider registry for": "Failed to retrieve plugin due to transient network error.", + + // Provider bugs where the data after apply is not propagated. This is usually an eventual consistency issue, so + // retrying should self resolve it. + // See https://github.com/terraform-providers/terraform-provider-aws/issues/12449 for an example. + ".*Provider produced inconsistent result after apply.*": "Provider eventual consistency error.", + } +) // Options for running IaC Provisioner commands type Options struct { @@ -141,8 +145,8 @@ func WithDefaultRetryableErrors(originalOptions *Options) (*Options, error) { } // These defaults for retry configuration are arbitrary, but have worked well in practice. - newOptions.MaxRetries = 3 - newOptions.TimeBetweenRetries = 5 * time.Second + newOptions.MaxRetries = DefaultMaxRetries + newOptions.TimeBetweenRetries = DefaultTimeBetweenRetries return newOptions, nil } diff --git a/pkg/iacprovisioner/output.go b/pkg/iacprovisioner/output.go index e789abb..8cbbe85 100644 --- a/pkg/iacprovisioner/output.go +++ b/pkg/iacprovisioner/output.go @@ -81,11 +81,11 @@ func parseMap(m map[string]interface{}) (map[string]interface{}, error) { func parseList(items []interface{}) (_ []interface{}, err error) { for i, v := range items { rv := reflect.ValueOf(v) - switch rv.Kind() { + switch rv.Kind() { //nolint:exhaustive // we only care about maps and slices, but we should revisit this soon case reflect.Map: - items[i], err = parseMap(rv.Interface().(map[string]interface{})) + items[i], err = parseMap(rv.Interface().(map[string]interface{})) //nolint:errcheck case reflect.Slice, reflect.Array: - items[i], err = parseList(rv.Interface().([]interface{})) + items[i], err = parseList(rv.Interface().([]interface{})) //nolint:errcheck case reflect.Float64: items[i] = parseFloat(v) } @@ -132,20 +132,18 @@ func OutputListOfObjects(options *Options, key string) ([]map[string]interface{} } var output []map[string]interface{} - if err := json.Unmarshal([]byte(out), &output); err != nil { return nil, err } - var result []map[string]interface{} - - for _, m := range output { + result := make([]map[string]interface{}, len(output)) + for i, m := range output { newMap, err := parseMap(m) if err != nil { return nil, err } - result = append(result, newMap) + result[i] = newMap } return result, nil @@ -165,14 +163,14 @@ func OutputList(options *Options, key string) ([]string, error) { } if outputList, isList := output.([]interface{}); isList { - return parseListOutputTerraform(outputList, key) + return parseListOutputTerraform(outputList) } return nil, UnexpectedOutputType{Key: key, ExpectedType: "map or list", ActualType: reflect.TypeOf(output).String()} } // Parse a list output in the format it is returned by Terraform 0.12 and newer versions -func parseListOutputTerraform(outputList []interface{}, key string) ([]string, error) { +func parseListOutputTerraform(outputList []interface{}) ([]string, error) { list := []string{} for _, item := range outputList { @@ -256,7 +254,7 @@ func OutputForKeys(options *Options, keys []string) (map[string]interface{}, err for _, key := range keys { value, containsValue := outputMap[key]["value"] if !containsValue { - return nil, OutputKeyNotFound(string(key)) + return nil, OutputKeyNotFound(key) } resultMap[key] = value } diff --git a/pkg/retry/retry.go b/pkg/retry/retry.go index 500e1b8..dd86e0b 100644 --- a/pkg/retry/retry.go +++ b/pkg/retry/retry.go @@ -1,6 +1,7 @@ package retry import ( + "errors" "fmt" "regexp" "time" @@ -42,7 +43,7 @@ func DoWithTimeout(actionDescription string, timeout time.Duration, action func( // maxRetries retries. If maxRetries is exceeded, return a MaxRetriesExceeded error. func DoWithRetry(actionDescription string, maxRetries int, sleepBetweenRetries time.Duration, action func() (string, error)) (string, error) { out, err := DoWithRetryInterface(actionDescription, maxRetries, sleepBetweenRetries, func() (interface{}, error) { return action() }) - return out.(string), err + return out.(string), err //nolint:errcheck // we also want the output in an error scenario } // DoWithRetryInterface runs the specified action. If it returns a value, return that value. If it returns a FatalError, return that error @@ -60,7 +61,7 @@ func DoWithRetryInterface(actionDescription string, maxRetries int, sleepBetween return output, nil } - if _, isFatalErr := err.(FatalError); isFatalErr { + if errors.As(err, &FatalError{}) { config.Logging.Logger.Infof("Returning due to fatal error: %v", err) return output, err } diff --git a/pkg/retry/retry_test.go b/pkg/retry/retry_test.go index b0e5acc..8660ca3 100644 --- a/pkg/retry/retry_test.go +++ b/pkg/retry/retry_test.go @@ -132,7 +132,7 @@ func TestDoInBackgroundUntilStopped(t *testing.T) { assert.Equal(t, 3, counter) } -func TestDoWithRetryableErrors(t *testing.T) { +func TestDoWithRetryableErrors(t *testing.T) { //nolint:funlen t.Parallel() expectedOutput := "this is the expected output" diff --git a/pkg/shell/command.go b/pkg/shell/command.go index 9966ebc..4168dfc 100644 --- a/pkg/shell/command.go +++ b/pkg/shell/command.go @@ -30,6 +30,24 @@ func (e *ErrWithCmdOutput) Error() string { return fmt.Sprintf("error while running command: %v; %s", e.Underlying, e.Output.Stderr()) } +// Add this near the top of the file +var allowedCommands = map[string]bool{ + "terraform": true, + "aws": true, + "kubectl": true, + "docker": true, + "git": true, +} + +// validateCommand checks if the command is in the list of allowed commands +func validateCommand(command Command) error { + if !allowedCommands[command.Name] { + return fmt.Errorf("command not allowed: %s", command.Name) + } + + return nil +} + // RunCommandAndGetOutput runs the given command and returns the output as a string or an error if the command fails. func RunCommandAndGetOutput(command Command) (string, error) { output, err := runCommand(command) @@ -44,7 +62,12 @@ func RunCommandAndGetOutput(command Command) (string, error) { func runCommand(command Command) (*output, error) { config.Logging.Logger.Infof("Running command %s with args %s", command.Name, command.Args) - cmd := exec.Command(command.Name, command.Args...) + err := validateCommand(command) + if err != nil { + return nil, err + } + + cmd := exec.Command(command.Name, command.Args...) //nolint:gosec cmd.Dir = command.WorkingDir cmd.Stdin = os.Stdin cmd.Env = formatEnvVars(command) @@ -69,9 +92,6 @@ func runCommand(command Command) (*output, error) { return output, err } - // go streamOutputToLogger(stdout, config.Logging.Logger.Info) - // go streamOutputToLogger(stderr, config.Logging.Logger.Error) - return output, cmd.Wait() } @@ -83,8 +103,9 @@ func readStdoutAndStderr(stdout, stderr io.ReadCloser) (*output, error) { stderrReader := bufio.NewReader(stderr) wg := &sync.WaitGroup{} + wgSize := 2 // stdout and stderr - wg.Add(2) + wg.Add(wgSize) var stdoutErr, stderrErr error go func() { defer wg.Done() @@ -112,22 +133,17 @@ func readData(reader *bufio.Reader, writer io.StringWriter) error { for { line, readErr = reader.ReadString('\n') - // remove newline, our output is in a slice, - // one element per line. + // remove newline, our output is in a slice, one element per line. line = strings.TrimSuffix(line, "\n") - // only return early if the line does not have - // any contents. We could have a line that does - // not not have a newline before io.EOF, we still - // need to add it to the output. - if len(line) == 0 && readErr == io.EOF { + // only return early if the line does not have any contents. We could have a line that does not have a newline + // before io.EOF, we still need to add it to the output. + if line == "" && readErr == io.EOF { break } - // logger.Logger has a Logf method, but not a Log method. - // We have to use the format string indirection to avoid - // interpreting any possible formatting characters in - // the line. + // logger.Logger has a Logf method, but not a Log method. We have to use the format string indirection to avoid + // interpreting any possible formatting characters in the line. // // See https://github.com/gruntwork-io/terratest/issues/982. config.Logging.Logger.Infof("%s", line) @@ -153,10 +169,3 @@ func formatEnvVars(command Command) []string { } return env } - -func streamOutputToLogger(reader io.ReadCloser, logFunc func(args ...interface{})) { - scanner := bufio.NewScanner(reader) - for scanner.Scan() { - logFunc(scanner.Text()) - } -} diff --git a/pkg/shell/output.go b/pkg/shell/output.go index 1114280..ede030f 100644 --- a/pkg/shell/output.go +++ b/pkg/shell/output.go @@ -56,7 +56,7 @@ type outputStream struct { } func (st *outputStream) WriteString(s string) (n int, err error) { - st.Lines = append(st.Lines, string(s)) + st.Lines = append(st.Lines, s) return st.merged.WriteString(s) } @@ -86,7 +86,7 @@ func (m *merged) WriteString(s string) (n int, err error) { m.mut.Lock() defer m.mut.Unlock() - m.Lines = append(m.Lines, string(s)) + m.Lines = append(m.Lines, s) return len(s), nil } diff --git a/pkg/ssh/agent.go b/pkg/ssh/agent.go index bd98518..aefa8d5 100644 --- a/pkg/ssh/agent.go +++ b/pkg/ssh/agent.go @@ -24,7 +24,7 @@ type SshAgent struct { // Create SSH agent, start it in background and returns control back to the main thread // You should stop the agent to cleanup files afterwards by calling `defer s.Stop()` -func NewSshAgent(socketDir string, socketFile string) (*SshAgent, error) { +func NewSshAgent(socketDir, socketFile string) (*SshAgent, error) { var err error s := &SshAgent{make(chan bool), make(chan bool), socketDir, socketFile, agent.NewKeyring(), nil} s.ln, err = net.Listen("unix", s.socketFile) @@ -61,7 +61,7 @@ func (s *SshAgent) run() { continue } } else { - defer c.Close() + defer c.Close() //nolint:gocritic go func(c io.ReadWriter) { err := agent.ServeAgent(s.agent, c) if err != nil { @@ -115,7 +115,10 @@ func SshAgentWithKeyPairs(keyPairs []*KeyPair) (*SshAgent, error) { return nil, err } key := agent.AddedKey{PrivateKey: privateKey} - sshAgent.agent.Add(key) + err = sshAgent.agent.Add(key) + if err != nil { + return nil, err + } } return sshAgent, err diff --git a/pkg/ssh/ssh.go b/pkg/ssh/ssh.go index 24e2924..c241f36 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -38,11 +38,11 @@ type ScpDownloadOptions struct { MaxFileSizeMB int // Don't grab any files > MaxFileSizeMB RemoteDir string // Copy from this directory on the remote machine LocalDir string // Copy RemoteDir to this directory on the local machine - RemoteHost Host // Connection information for the remote machine + RemoteHost *Host // Connection information for the remote machine } // ScpFileTo uploads the contents using SCP to the given host and return an error if the process fails. -func ScpFileTo(host Host, mode os.FileMode, remotePath, contents string) error { +func ScpFileTo(host *Host, mode os.FileMode, remotePath, contents string) error { authMethods, err := createAuthMethodsForHost(host) if err != nil { return err @@ -72,7 +72,7 @@ func ScpFileTo(host Host, mode os.FileMode, remotePath, contents string) error { } // ScpFileFrom downloads the file from remotePath on the given host using SCP and returns an error if the process fails. -func ScpFileFrom(host Host, remotePath string, localDestination *os.File, useSudo bool) error { +func ScpFileFrom(host *Host, remotePath string, localDestination *os.File, useSudo bool) error { authMethods, err := createAuthMethodsForHost(host) if err != nil { return err @@ -135,7 +135,7 @@ func ScpDirFrom(options ScpDownloadOptions, useSudo bool) error { } if errors.Is(err, fs.ErrNotExist) { - err := os.MkdirAll(options.LocalDir, 0o755) + err := os.MkdirAll(options.LocalDir, 0o750) //nolint:mnd if err != nil { return err } @@ -163,14 +163,14 @@ func ScpDirFrom(options ScpDownloadOptions, useSudo bool) error { } // CheckSshConnection checks that you can connect via SSH to the given host and return an error if the connection fails. -func CheckSshConnection(host Host) error { +func CheckSshConnection(host *Host) error { _, err := CheckSshCommand(host, "'exit'") return err } // CheckSshConnectionWithRetry attempts to connect via SSH until max retries has been exceeded and returns an error if // the connection fails -func CheckSshConnectionWithRetry(host Host, retries int, sleepBetweenRetries time.Duration, f ...func(Host) error) error { +func CheckSshConnectionWithRetry(host *Host, retries int, sleepBetweenRetries time.Duration, f ...func(*Host) error) error { handler := CheckSshConnection if f != nil { handler = f[0] @@ -183,7 +183,7 @@ func CheckSshConnectionWithRetry(host Host, retries int, sleepBetweenRetries tim } // CheckSshCommand checks that you can connect via SSH to the given host and run the given command. Returns the stdout/stderr. -func CheckSshCommand(host Host, command string) (string, error) { +func CheckSshCommand(host *Host, command string) (string, error) { authMethods, err := createAuthMethodsForHost(host) if err != nil { return "", err @@ -209,7 +209,7 @@ func CheckSshCommand(host Host, command string) (string, error) { // CheckSshCommandWithRetry checks that you can connect via SSH to the given host and run the given command until max retries has been exceeded. // It return an error if the command fails after max retries has been exceeded. -func CheckSshCommandWithRetry(host Host, command string, retries int, sleepBetweenRetries time.Duration, f ...func(Host, string) (string, error)) (string, error) { +func CheckSshCommandWithRetry(host *Host, command string, retries int, sleepBetweenRetries time.Duration, f ...func(*Host, string) (string, error)) (string, error) { handler := CheckSshCommand if f != nil { handler = f[0] @@ -222,7 +222,7 @@ func CheckSshCommandWithRetry(host Host, command string, retries int, sleepBetwe // CheckPrivateSshConnection attempts to connect to privateHost (which is not addressable from the Internet) via a // separate publicHost (which is addressable from the Internet) and then executes "command" on privateHost and returns // its output. It is useful for checking that it's possible to SSH from a Bastion Host to a private instance. -func CheckPrivateSshConnection(publicHost Host, privateHost Host, command string) (string, error) { +func CheckPrivateSshConnection(publicHost, privateHost *Host, command string) (string, error) { jumpHostAuthMethods, err := createAuthMethodsForHost(publicHost) if err != nil { return "", err @@ -262,7 +262,7 @@ func CheckPrivateSshConnection(publicHost Host, privateHost Host, command string // FetchContentsOfFiles connects to the given host via SSH and fetches the contents of the files at the given filePaths. // If useSudo is true, then the contents will be retrieved using sudo. This method returns a map from file path to // contents. -func FetchContentsOfFiles(host Host, useSudo bool, filePaths ...string) (map[string]string, error) { +func FetchContentsOfFiles(host *Host, useSudo bool, filePaths ...string) (map[string]string, error) { filePathToContents := map[string]string{} for _, filePath := range filePaths { @@ -279,7 +279,7 @@ func FetchContentsOfFiles(host Host, useSudo bool, filePaths ...string) (map[str // FetchContentsOfFile connects to the given host via SSH and fetches the contents of the file at the given filePath. // If useSudo is true, then the contents will be retrieved using sudo. This method returns the contents of that file. -func FetchContentsOfFile(host Host, useSudo bool, filePath string) (string, error) { +func FetchContentsOfFile(host *Host, useSudo bool, filePath string) (string, error) { command := fmt.Sprintf("cat %s", filePath) if useSudo { command = fmt.Sprintf("sudo %s", command) @@ -298,12 +298,10 @@ func listFileInRemoteDir(sshSession *SshSession, options ScpDownloadOptions, use findCommandArgs = append(findCommandArgs, "sudo") } - findCommandArgs = append(findCommandArgs, "find", options.RemoteDir) - findCommandArgs = append(findCommandArgs, "-type", "f") + findCommandArgs = append(findCommandArgs, "find", options.RemoteDir, "-type", "f") filtersLength := len(options.FileNameFilters) if options.FileNameFilters != nil && filtersLength > 0 { - findCommandArgs = append(findCommandArgs, "\\(") for i, curFilter := range options.FileNameFilters { // due to inconsistent bash behavior we need to wrap the @@ -450,6 +448,11 @@ func createSSHClient(options *SshConnectionOptions) (*ssh.Client, error) { return ssh.Dial("tcp", options.ConnectionString(), sshClientConfig) } +var ( + sshPort = 22 + sshConnectionTimeout = 10 * time.Second +) + func createSSHClientConfig(hostOptions *SshConnectionOptions) *ssh.ClientConfig { clientConfig := &ssh.ClientConfig{ User: hostOptions.Username, @@ -457,7 +460,7 @@ func createSSHClientConfig(hostOptions *SshConnectionOptions) *ssh.ClientConfig // Do not do a host key check, as Terratest is only used for testing, not prod HostKeyCallback: NoOpHostKeyCallback, // By default, Go does not impose a timeout, so a SSH connection attempt can hang for a LONG time. - Timeout: 10 * time.Second, + Timeout: sshConnectionTimeout, } clientConfig.SetDefaults() return clientConfig @@ -470,7 +473,7 @@ func NoOpHostKeyCallback(hostname string, remote net.Addr, key ssh.PublicKey) er } // Returns an array of authentication methods -func createAuthMethodsForHost(host Host) ([]ssh.AuthMethod, error) { +func createAuthMethodsForHost(host *Host) ([]ssh.AuthMethod, error) { var methods []ssh.AuthMethod // override local ssh agent with given sshAgent instance @@ -506,11 +509,11 @@ func createAuthMethodsForHost(host Host) ([]ssh.AuthMethod, error) { } // Use given password - if len(host.Password) > 0 { + if host.Password != "" { methods = append(methods, []ssh.AuthMethod{ssh.Password(host.Password)}...) } - // no valid authentication method was provided + // no valid authentication method were provided if len(methods) < 1 { return methods, errors.New("no authentication method defined") } @@ -537,10 +540,10 @@ func sendScpCommandsToCopyFile(mode os.FileMode, fileName, contents string) func } // Gets the port that should be used to communicate with the host -func (h Host) getPort() int { +func (h *Host) getPort() int { // If a CustomPort is not set use standard ssh port if h.CustomPort == 0 { - return 22 + return sshPort } else { return h.CustomPort } diff --git a/pkg/ssh/ssh_test.go b/pkg/ssh/ssh_test.go index 9334971..f28e939 100644 --- a/pkg/ssh/ssh_test.go +++ b/pkg/ssh/ssh_test.go @@ -10,7 +10,7 @@ import ( func TestHostWithDefaultPort(t *testing.T) { t.Parallel() - host := Host{} + host := &Host{} assert.Equal(t, 22, host.getPort(), "host.getPort() did not return the default ssh port of 22") } @@ -19,7 +19,7 @@ func TestHostWithCustomPort(t *testing.T) { t.Parallel() customPort := 2222 - host := Host{CustomPort: customPort} + host := &Host{CustomPort: customPort} assert.Equal(t, customPort, host.getPort(), "host.getPort() did not return the custom port number") } @@ -31,7 +31,7 @@ func TestCheckSshConnectionWithRetry(t *testing.T) { // Reset the global call count timesCalled = 0 - host := Host{Hostname: "Host"} + host := &Host{Hostname: "Host"} retries := 10 assert.Nil(t, CheckSshConnectionWithRetry(host, retries, 3, mockSshConnection)) @@ -41,7 +41,7 @@ func TestCheckSshConnectionWithRetryExceedsMaxRetries(t *testing.T) { // Reset the global call count timesCalled = 0 - host := Host{Hostname: "Host"} + host := &Host{Hostname: "Host"} // Not enough retries retries := 3 @@ -53,7 +53,7 @@ func TestCheckSshCommandWithRetry(t *testing.T) { // Reset the global call count timesCalled = 0 - host := Host{Hostname: "Host"} + host := &Host{Hostname: "Host"} command := "echo -n hello world" retries := 10 @@ -65,7 +65,7 @@ func TestCheckSshCommandWithRetryExceedsRetries(t *testing.T) { // Reset the global call count timesCalled = 0 - host := Host{Hostname: "Host"} + host := &Host{Hostname: "Host"} command := "echo -n hello world" // Not enough retries @@ -75,11 +75,11 @@ func TestCheckSshCommandWithRetryExceedsRetries(t *testing.T) { assert.Error(t, err) } -func mockSshCommand(host Host, command string) (string, error) { +func mockSshCommand(host *Host, command string) (string, error) { return "", mockSshConnection(host) } -func mockSshConnection(host Host) error { +func mockSshConnection(host *Host) error { timesCalled += 1 if timesCalled >= 5 { return nil diff --git a/pkg/steps/aws/aws.go b/pkg/steps/aws/aws.go index f01eaf8..1e63549 100644 --- a/pkg/steps/aws/aws.go +++ b/pkg/steps/aws/aws.go @@ -23,7 +23,6 @@ func RegisterSteps(sc *godog.ScenarioContext) { // Generic AWS steps sc.Step(`^the AWS resource "([^"]*)" should exist$`, newAWSResourceExistsStep) sc.Step(`^the resource "([^"]*)" should have tags$`, newAWSTagsStep) - // sc.Step(`^I wait for resource "([^"]*)" to be "([^"]*)"$`, newAWSWaitForStateStep(ctx)) } // Generic AWS Steps diff --git a/pkg/steps/aws/rds.go b/pkg/steps/aws/rds.go index 3e22903..e583e20 100644 --- a/pkg/steps/aws/rds.go +++ b/pkg/steps/aws/rds.go @@ -49,7 +49,7 @@ func newRDSDescribeInstanceStep(ctx context.Context) error { return nil } -func newRDSInstanceStatusStep(ctx context.Context, dbInstanceID string, status string) error { +func newRDSInstanceStatusStep(ctx context.Context, dbInstanceID, status string) error { rdsAssert, err := getRdsAsserter(ctx) if err != nil { return err @@ -119,16 +119,7 @@ func newRDSInstanceStorageStep(ctx context.Context, dbInstanceID string, allocat return rdsAssert.AssertDBInstanceStorage(dbInstanceID, allocatedStorage, region) } -func newRDSInstanceStorageStepWrapper(ctx context.Context, dbInstanceID string, allocatedStorageStr string) error { - allocatedStorage, err := strconv.ParseInt(allocatedStorageStr, 10, 32) - if err != nil { - return fmt.Errorf("invalid allocated storage value: %s", allocatedStorageStr) - } - - return newRDSInstanceStorageStep(ctx, dbInstanceID, int32(allocatedStorage)) -} - -func newRDSInstanceMultiAZStep(ctx context.Context, dbInstanceID string, multiAZStr string) error { +func newRDSInstanceMultiAZStep(ctx context.Context, dbInstanceID, multiAZStr string) error { rdsAssert, err := getRdsAsserter(ctx) if err != nil { return err @@ -147,7 +138,7 @@ func newRDSInstanceMultiAZStep(ctx context.Context, dbInstanceID string, multiAZ return rdsAssert.AssertDBInstanceMultiAZ(dbInstanceID, multiAZ, region) } -func newRDSInstanceEncryptionStep(ctx context.Context, dbInstanceID string, encryptedStr string) error { +func newRDSInstanceEncryptionStep(ctx context.Context, dbInstanceID, encryptedStr string) error { rdsAssert, err := getRdsAsserter(ctx) if err != nil { return err diff --git a/pkg/steps/steps.go b/pkg/steps/steps.go index 66d4a51..9ce78d0 100644 --- a/pkg/steps/steps.go +++ b/pkg/steps/steps.go @@ -7,28 +7,11 @@ import ( "github.com/robmorgan/infraspec/pkg/steps/terraform" ) -// RegisterSteps registers all step definitions with Godog +// RegisterSteps registers all step definitions. func RegisterSteps(sc *godog.ScenarioContext) { - // Register common steps - registerCommonSteps(sc) - // Register Terraform steps terraform.RegisterSteps(sc) // Register provider-specific steps aws.RegisterSteps(sc) } - -// registerCommonSteps registers common steps that are shared across providers -func registerCommonSteps(sc *godog.ScenarioContext) { - // TODO -} - -// Common step implementations -// func newRandomNameStep(ctx context.Context) func(string) error { -// return func(prefix string) error { -// name := generators.RandomResourceName(prefix, ctx.Config().Functions.RandomString) -// ctx.StoreValue("resource_name", name) -// return nil -// } -// } diff --git a/pkg/steps/terraform/set_variable.go b/pkg/steps/terraform/set_variable.go deleted file mode 100644 index 0ee834c..0000000 --- a/pkg/steps/terraform/set_variable.go +++ /dev/null @@ -1,102 +0,0 @@ -package terraform - -import ( - "fmt" - "os" - "regexp" - "strings" - - "github.com/robmorgan/infraspec/internal/context" -) - -// SetVariableStep handles setting variables in the test context -type SetVariableStep struct { - ctx *context.TestContext -} - -// NewSetVariableStep creates a new SetVariableStep -func NewSetVariableStep(ctx *context.TestContext) *SetVariableStep { - return &SetVariableStep{ - ctx: ctx, - } -} - -// Pattern returns the Gherkin pattern for this step -func (s *SetVariableStep) Pattern() string { - return `^I set variable "([^"]*)" to "([^"]*)"$` -} - -// Execute runs the step implementation -func (s *SetVariableStep) Execute(args ...string) error { - if len(args) != 2 { - return fmt.Errorf("expected 2 arguments, got %d", len(args)) - } - - name := args[0] - value := args[1] - - // Interpolate any variables in the value - interpolatedValue, err := s.interpolateValue(value) - if err != nil { - return fmt.Errorf("failed to interpolate value: %w", err) - } - - // Store in the IaC provisioner options - if s.ctx.GetIacProvisionerOptions() != nil { - s.ctx.GetIacProvisionerOptions().Vars[name] = interpolatedValue - } - - // Also store in context values for future reference - s.ctx.StoreValue(name, interpolatedValue) - - return nil -} - -// interpolateValue replaces variables and environment variables in the value -func (s *SetVariableStep) interpolateValue(value string) (string, error) { - // First, handle stored variables ${variable} - varRegex := regexp.MustCompile(`\${([^}]+)}`) - result := varRegex.ReplaceAllStringFunc(value, func(match string) string { - varName := match[2 : len(match)-1] // Remove ${ and } - if storedValue, exists := s.ctx.GetStoredValues()[varName]; exists { - return storedValue - } - // If not found, leave as is - return match - }) - - // Then, handle environment variables %{ENV_VAR} - envRegex := regexp.MustCompile(`%{([^}]+)}`) - result = envRegex.ReplaceAllStringFunc(result, func(match string) string { - envName := match[2 : len(match)-1] // Remove %{ and } - if envValue, exists := os.LookupEnv(envName); exists { - return envValue - } - // If not found, leave as is - return match - }) - - // Check if there are any unresolved variables - if strings.Contains(result, "${") || strings.Contains(result, "%{") { - // Find all unresolved variables - var unresolvedVars []string - - varMatches := varRegex.FindAllString(result, -1) - for _, match := range varMatches { - varName := match[2 : len(match)-1] - unresolvedVars = append(unresolvedVars, fmt.Sprintf("${%s}", varName)) - } - - envMatches := envRegex.FindAllString(result, -1) - for _, match := range envMatches { - envName := match[2 : len(match)-1] - unresolvedVars = append(unresolvedVars, fmt.Sprintf("%%{%s}", envName)) - } - - if len(unresolvedVars) > 0 { - return "", fmt.Errorf("unresolved variables: %s", strings.Join(unresolvedVars, ", ")) - } - } - - return result, nil -} diff --git a/script/build.go b/script/build.go index 00bc126..f2e96ac 100644 --- a/script/build.go +++ b/script/build.go @@ -71,7 +71,7 @@ func main() { } } - if len(args) < 2 { + if len(args) < 2 { //nolint:mnd if isWindowsTarget() { args = append(args, filepath.Join("bin", "infraspec.exe")) } else { @@ -131,7 +131,7 @@ func date() string { return t.Format("2006-01-02") } -func sourceFilesLaterThan(t time.Time) bool { +func sourceFilesLaterThan(t time.Time) bool { //nolint:gocognit,gocyclo,cyclop foundLater := false err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error { if err != nil { diff --git a/test/aws_test.go b/test/aws_test.go index c38d862..14aadba 100644 --- a/test/aws_test.go +++ b/test/aws_test.go @@ -1,6 +1,7 @@ package test import ( + "os" "path/filepath" "testing" @@ -12,7 +13,7 @@ import ( func TestDynamoDBFeature(t *testing.T) { cfg := testhelpers.SetupAWSTestsAndConfig() - featurePath := filepath.Join("../", "features", "aws", "dynamodb", "dynamodb_table.feature") + featurePath := filepath.Join("..", string(os.PathSeparator), "features", "aws", "dynamodb", "dynamodb_table.feature") err := runner.New(cfg).Run(featurePath) require.NoError(t, err) @@ -20,7 +21,7 @@ func TestDynamoDBFeature(t *testing.T) { func TestS3Feature(t *testing.T) { cfg := testhelpers.SetupAWSTestsAndConfig() - featurePath := filepath.Join("../", "features", "aws", "s3", "s3_bucket.feature") + featurePath := filepath.Join("..", string(os.PathSeparator), "features", "aws", "s3", "s3_bucket.feature") err := runner.New(cfg).Run(featurePath) require.NoError(t, err) @@ -32,7 +33,7 @@ func TestRdsFeature(t *testing.T) { testCases := []struct { featurePath string }{ - {filepath.Join("../", "features", "aws", "rds", "rds_db_instance.feature")}, + {filepath.Join("..", string(os.PathSeparator), "features", "aws", "rds", "rds_db_instance.feature")}, } for _, testCase := range testCases { diff --git a/test/terraform_test.go b/test/terraform_test.go index eb8e387..1e09406 100644 --- a/test/terraform_test.go +++ b/test/terraform_test.go @@ -1,6 +1,7 @@ package test import ( + "os" "path/filepath" "testing" @@ -12,7 +13,7 @@ import ( func TestHelloWorldFeature(t *testing.T) { cfg := testhelpers.SetupAWSTestsAndConfig() - featurePath := filepath.Join("../", "features", "terraform", "hello_world.feature") + featurePath := filepath.Join("..", string(os.PathSeparator), "features", "terraform", "hello_world.feature") err := runner.New(cfg).Run(featurePath) require.NoError(t, err) From 518cd424e0fbbc3c24974761df4fd8ec79eee89d Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Tue, 22 Jul 2025 07:48:21 +0800 Subject: [PATCH 4/9] chore: bump gha actions --- .github/workflows/golangci-lint.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 3fb534b..acc6233 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -21,7 +21,7 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: go.mod - - uses: golangci/golangci-lint-action@v7 + - uses: golangci/golangci-lint-action@v8 with: - version: v2.1 + version: v2.3.0 args: --verbose From b24dcb4fd926203b47d45b958e4d174fdaa5cdc5 Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Tue, 22 Jul 2025 07:48:43 +0800 Subject: [PATCH 5/9] chore: forgot makefile. bump golangci-lint version --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ee5d05d..c91b155 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ deps: ## install dependencies go install mvdan.cc/gofumpt@latest go install github.com/daixiang0/gci@latest go install golang.org/x/tools/cmd/goimports@latest - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.2.1 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.3.0 .PHONY: tidy tidy: ## go mod tidy From cb4fd6548cea3114047d829b325bbb652dacea8c Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Tue, 22 Jul 2025 07:49:07 +0800 Subject: [PATCH 6/9] chore: cleanup test gha --- .github/workflows/test.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a306b99..ae5b3c0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,12 +24,12 @@ jobs: with: terraform_version: "1.0.11" + - uses: actions/checkout@v4 + - name: Set up Go uses: actions/setup-go@v5 with: - go-version: "1.24" - - - uses: actions/checkout@v4 + go-version-file: go.mod - name: Run Tests run: | From bde0027e61156d63f1fa7e20cfb9e9be0b099c2c Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Tue, 22 Jul 2025 07:49:23 +0800 Subject: [PATCH 7/9] chore: add todo --- internal/telemetry/telemetry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 4ba4fdc..cc768b6 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -10,7 +10,7 @@ import ( ) const ( - AmplitudeAPIKey = "9dc54881885bd60f8ccbb9cef2dfaa7a" //nolint:gosec + AmplitudeAPIKey = "9dc54881885bd60f8ccbb9cef2dfaa7a" //nolint:gosec // TODO - rotate and inject via env var ) var ( From 18fee81c46b1b663207595bd7d7fcfd3eebd302e Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Tue, 22 Jul 2025 07:50:01 +0800 Subject: [PATCH 8/9] fix: fix bugs from linter fixes --- pkg/iacprovisioner/format.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/iacprovisioner/format.go b/pkg/iacprovisioner/format.go index 202245b..b3416fa 100644 --- a/pkg/iacprovisioner/format.go +++ b/pkg/iacprovisioner/format.go @@ -51,7 +51,7 @@ func FormatArgs(options *Options, args ...string) []string { planFileSupported := slices.Contains(TerraformCommandsWithPlanFileSupport, commandType) // Include -var and -var-file flags unless we're running 'apply' with a plan file - includeVars := commandType != "apply" || options.PlanFilePath != "" + includeVars := commandType != "apply" || options.PlanFilePath == "" terraformArgs = append(terraformArgs, args...) @@ -249,7 +249,7 @@ func mapToHclString(m map[string]interface{}) string { keyValuePairs := []string{} for key, value := range m { - keyValuePair := fmt.Sprintf(`"%q" = %q`, key, toHclString(value, true)) + keyValuePair := fmt.Sprintf(`%q = %s`, key, toHclString(value, true)) keyValuePairs = append(keyValuePairs, keyValuePair) } From 654ed66742152014ce1d1338ab590ff97849e20a Mon Sep 17 00:00:00 2001 From: Rob Morgan Date: Tue, 22 Jul 2025 08:04:18 +0800 Subject: [PATCH 9/9] chore: downgrade golangci-lint to v2.2.2 --- .github/workflows/golangci-lint.yml | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index acc6233..e5cda2a 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -23,5 +23,5 @@ jobs: go-version-file: go.mod - uses: golangci/golangci-lint-action@v8 with: - version: v2.3.0 + version: v2.2.2 args: --verbose diff --git a/Makefile b/Makefile index c91b155..6689074 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ deps: ## install dependencies go install mvdan.cc/gofumpt@latest go install github.com/daixiang0/gci@latest go install golang.org/x/tools/cmd/goimports@latest - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.3.0 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.2.2 .PHONY: tidy tidy: ## go mod tidy