Skip to content

Conversation

YakDriver
Copy link
Member

@YakDriver YakDriver commented Sep 4, 2025

Supercedes #455

PCI review checklist

  • I have documented a clear reason for, and description of, the change I am making.

  • If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.

  • If applicable, I've documented the impact of any changes to security controls.

    Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.

Overview

This PR comprehensively modernizes the go-getter codebase by addressing linting warnings, updating CI/CD workflows for better security and performance, and implementing idiomatic Go patterns.

Key Changes

CI/CD Workflow Improvements

  • Updated Go versions: Migrated from Go 1.18/1.19 to modern 1.24/1.25
  • Split test workflows: Separated basic validation (PRs) from integration tests (merges)
    • PRs now run fast unit tests without requiring cloud credentials
    • Integration tests with AWS/GCS credentials only run on main branch pushes
  • Enhanced security: No cloud credentials exposed during PR validation

Code Modernization & Linting Fixes

  • Eliminated deprecated io/ioutil: Replaced all usages with modern os package functions
    • ioutil.ReadFileos.ReadFile
    • ioutil.TempDiros.MkdirTemp
    • ioutil.TempFileos.CreateTemp
  • Modern Go patterns:
    • interface{}any for improved readability
    • slices.Contains() instead of manual loops for better performance
    • Modern range over integers for cleaner iteration

AWS SDK Modernization

  • Updated AWS S3 client configuration: Migrated from deprecated WithEndpointResolverWithOptions to modern BaseEndpoint configuration in NewFromConfig options
  • Ensures compatibility with AWS SDK v2 best practices

Improved Test Architecture

  • Smart credential handling: Tests requiring AWS/GCS credentials automatically skip in short mode
  • Verbose CI output: Added -v flag to see exactly which tests run in validation

Benefits

  • Faster PR validation: Unit tests complete in seconds
  • Enhanced security: No cloud credentials needed for PR checks
  • Better code quality: All linting warnings resolved
  • Go best practices: Modern, idiomatic Go code patterns
  • Comprehensive testing: Full integration coverage maintained on main branch

Testing

  • All unit tests pass with go test -short
  • Integration tests validated with full credentials
  • No linting warnings remain
  • CI/CD workflows tested and functional

This modernization brings the codebase up to current Go standards while significantly improving the developer experience and CI/CD security posture.

@YakDriver YakDriver requested a review from a team as a code owner September 4, 2025 16:01
@YakDriver YakDriver changed the title Fix linter failures Modernize codebase: Fix linting issues, update CI/CD workflow, and improve test categorization Sep 4, 2025
Copy link
Collaborator

@dduzgun-security dduzgun-security left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Small comment about the go version being used which is no longer stable. I believe we would need to bump it in our go.mod file too however it can be addressed in another PR too.

Copy link
Collaborator

@dduzgun-security dduzgun-security left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@dduzgun-security dduzgun-security merged commit d879f88 into hashicorp:main Sep 5, 2025
6 checks passed
@YakDriver YakDriver deleted the f-fix-some-lint branch September 5, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants