chore: Add make fix and make verify and check go fix in CI (#1645)

Author: lantoli

.githooks/pre-commit

@@ -14,35 +14,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-STAGED_GO_FILES=$(git diff --cached --name-only | grep ".go$")
-
-echo "==> Formatting changed go files... $STAGED_GO_FILES"
-for FILE in ${STAGED_GO_FILES}; do
-	gofmt -w -s "${FILE}"
-	goimports -w "${FILE}"
-	git add "${FILE}"
-done
-
-echo "==> Formatting changed sh files..."
-STAGED_SH_FILES=$(git diff --cached --name-only | grep ".sh$")
-for FILE in ${STAGED_SH_FILES}; do
-	shfmt -l -w "${FILE}"
-	git add "${FILE}"
-done
-
-echo "==> Formatting changed JSON files..."
-STAGED_JSON_FILES=$(git diff --cached --name-only | grep ".json$")
-for FILE in ${STAGED_JSON_FILES}; do
-  prettyFile=$(jq . "${FILE}")
-	echo "${prettyFile}" > "${FILE}"
-	git add "${FILE}"
-done
-
-echo "==> Linting GitHub Actions..."
-STAGED_ACTION_FILES=$(git diff --name-only | grep -E "\.github/workflows/.*(\.yaml|\.yml)$")
-for FILE in ${STAGED_ACTION_FILES}; do
-	actionlint "${FILE}" -color -verbose
-done
-
-echo "==> Done..."
+set -euo pipefail
 
+STAGED_GO_FILES=$(git diff --cached --name-only --diff-filter=ACMR | grep '\.go$' | tr '\n' ' ' || true)
+if [[ -n "${STAGED_GO_FILES}" ]]; then
+	make verify files="${STAGED_GO_FILES}"
+fi

.github/pull_request_template.md

@@ -51,7 +51,7 @@ We're here to help! This is simply a reminder of what we are going to look for b
 - [ ] I have added tests that prove my fix is effective or that my feature works
 - [ ] I have checked that this change does not generate any credentials and that **they are NOT accidentally logged anywhere**.
 - [ ] I have added any necessary documentation (if appropriate)
-- [ ] I have run `make fmt` and formatted my code
+- [ ] I have run `make fix` and verified my code
 - [ ] For CFN Resources: I have released by changes in the private registry and proved by change
   works in Atlas
 

.github/workflows/code-health.yaml

@@ -9,15 +9,12 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
-    - name: Set up Go
-      uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c
-      with:
-        go-version-file: 'cfn-resources/go.mod'
-    - name: Build
-      run: | 
-        cd cfn-resources
-        go build -v ./...
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
+      - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c
+        with:
+          go-version-file: 'cfn-resources/go.mod'
+      - name: Setup and verify
+        run: make tools verify
   mock-generation:
     runs-on: ubuntu-latest
     steps:
@@ -33,32 +30,28 @@ jobs:
         run: |
           FILES=$(git ls-files -o -m --directory --exclude-standard --no-empty-directory)
           LINES=$(echo "$FILES" | awk 'NF' | wc -l)
-          
+
           if [ "$LINES" -ne 0 ]; then
             echo "Detected files that need to be committed:"
             echo "$FILES" | while IFS= read -r line; do echo "  $line"; done
             echo ""
             echo "Mock skeletons are not up-to-date, you may have forgotten to run mockery before committing your changes."
             exit 1
           fi
-  lint:
+  github-linters:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
       - uses: actions/setup-go@4a3601121dd01d1626a1e23e37211e3254c1c06c
         with:
           go-version-file: 'cfn-resources/go.mod'
-          cache: false # see https://github.com/golangci/golangci-lint-action/issues/807
-      - name: golangci-lint
-        uses: golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20
-        with:
-          version: v2.11.4 # Also update GOLANGCI_VERSION variable in Makefile when updating this version
-          working-directory: cfn-resources
       - name: actionlint
-        run: | 
-           make tools 
-           actionlint -verbose -color
+        run: |
+          make tools
+          actionlint -verbose -color
         shell: bash
+      - name: Run ShellCheck
+        uses: bewuethr/shellcheck-action@80bac2daa9fcf95d648200a793d00060857e6dc4
   check-copyright:
     runs-on: ubuntu-latest
     steps:
@@ -79,13 +72,6 @@ jobs:
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
       - name: 'Dependency Review'
         uses: actions/dependency-review-action@2031cfc080254a8a887f58cffee85186f0e49e48
-  shellcheck:
-    name: shellcheck
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
-      - name: Run ShellCheck
-        uses: bewuethr/shellcheck-action@80bac2daa9fcf95d648200a793d00060857e6dc4
   cfn-lint:
     runs-on: ubuntu-latest
     steps:

CONTRIBUTING.md

@@ -92,7 +92,20 @@ make update-atlas-sdk
 ## Prerequisites
 - [AWS CloudFormation CLI](https://github.com/aws-cloudformation/cloudformation-cli)
 - (Optional - only need if building from source) [AWS CloudFormation CLI Go Plugin](https://github.com/aws-cloudformation/cloudformation-cli-go-plugin/) > v1.0
-- (Optional - only need if building from source) [Go](https://golang.org/doc/install) v1.23
+- (Optional - only need if building from source) [Go](https://golang.org/doc/install) (version pinned in [`cfn-resources/go.mod`](cfn-resources/go.mod))
+
+## Development Setup
+
+The Go module lives in `cfn-resources/`, but `make` targets are run from the repository root and `cd` into `cfn-resources/` as needed.
+
+- Run `make tools` once to install the dev tools (`golangci-lint`, `goimports`, `shfmt`, `actionlint`, `mockery`, etc.). Tools install into `$(go env GOPATH)/bin`; the Makefile prepends it to `PATH` for subsequent targets.
+- Run `make link-git-hooks` to install the pre-commit hook. The hook calls `make verify files=$STAGED_GO_FILES` against staged Go files.
+- Run `make fix` to apply formatting (`gofmt`, `goimports`), `golangci-lint --fix`, `go mod tidy`, and `go fix ./...`. This is the default `make` target.
+- Run `make verify` to run the same checks read-only. CI runs this; missing `go fix` rewrites or formatter drift fail the build. Pass `files="path/one.go path/two.go"` to scope the checks.
+- Run `make unit-test` to run the Go unit tests (excludes the e2e packages).
+- Run `make generate-mocks` after changing a mocked interface (output lands in `cfn-resources/testutil/mocksvc`).
+
+The `golangci-lint` version is pinned in the `Makefile` (`GOLANGCI_VERSION`) and is the single source of truth; the CI workflow picks it up via `make tools`.
 
 ## Testing the Provider
 Please see README for each resource for details on unit and integrated AWS testing.

Makefile

@@ -1,5 +1,5 @@
 tags="logging callback metrics scheduler"
-cgo=0 
+cgo=0
 goos=linux
 goarch=amd64
 
@@ -8,7 +8,12 @@ ldXflags=github.com/mongodb/mongodbatlas-cloudformation-resources/util.defaultLo
 ldXflagsD=github.com/mongodb/mongodbatlas-cloudformation-resources/util.defaultLogLevel=debug
 
 MOCKERY_VERSION=v3.5.3
-GOLANGCI_VERSION=v2.11.4 # Also update golangci-lint GH action in code-health.yaml when updating this version
+GOLANGCI_VERSION=v2.11.4
+
+export PATH := $(shell go env GOPATH)/bin:$(PATH)
+export SHELL := env PATH=$(PATH) /bin/bash
+
+default: fix
 
 .PHONY: submit
 submit:
@@ -18,9 +23,33 @@ submit:
 test:
 	(cd cfn-resources && ./cfn-testing-helper.sh $(filter-out $@,$(MAKECMDGOALS)))
 
-.PHONY: fmt
-fmt: ## Format changed go and sh
-	@scripts/fmt.sh
+.PHONY: fix
+fix: ## Format, lint-fix, tidy, and apply go fix
+	(cd cfn-resources && gofmt -s -w .)
+	(cd cfn-resources && goimports -w .)
+	(cd cfn-resources && golangci-lint run --fix --timeout 5m)
+	(cd cfn-resources && go mod tidy)
+	(cd cfn-resources && go fix ./...)
+
+.PHONY: verify
+verify: ## Verify Go code without modifying files. Usage: make verify [files="file1.go file2.go"]
+ifdef files
+	$(eval files_rel := $(patsubst cfn-resources/%,%,$(files)))
+	@bad_fmt=$$(cd cfn-resources && gofmt -l -s $(files_rel)); \
+	if [ -n "$$bad_fmt" ]; then echo "ERROR: gofmt issues:"; echo "$$bad_fmt"; exit 1; fi
+	@bad_imports=$$(cd cfn-resources && goimports -l $(files_rel)); \
+	if [ -n "$$bad_imports" ]; then echo "ERROR: goimports issues:"; echo "$$bad_imports"; exit 1; fi
+	(cd cfn-resources && golangci-lint run --timeout 5m $(addsuffix ...,$(sort $(dir $(files_rel)))))
+	(cd cfn-resources && go fix -diff $(addprefix ./,$(addsuffix ...,$(sort $(dir $(files_rel))))))
+else
+	@bad_fmt=$$(cd cfn-resources && gofmt -l -s .); \
+	if [ -n "$$bad_fmt" ]; then echo "ERROR: gofmt issues:"; echo "$$bad_fmt"; exit 1; fi
+	@bad_imports=$$(cd cfn-resources && goimports -l .); \
+	if [ -n "$$bad_imports" ]; then echo "ERROR: goimports issues:"; echo "$$bad_imports"; exit 1; fi
+	(cd cfn-resources && golangci-lint run --timeout 5m)
+	(cd cfn-resources && go mod tidy -diff)
+	(cd cfn-resources && go fix -diff ./...)
+endif
 
 .PHONY: tools
 tools:  ## Install dev tools
@@ -41,10 +70,6 @@ link-git-hooks: ## Install git hooks
 	find .git/hooks -type l -exec rm {} \;
 	find .githooks -type f -exec ln -sf ../../{} .git/hooks/ \;
 
-.PHONY: lint
-lint: ## Run linter
-	@scripts/lint.sh
-
 .PHONY: unit-test
 unit-test:
 	(cd cfn-resources && go test $$(go list ./... | grep -v /e2e))
@@ -60,7 +85,7 @@ generate-mocks: # uses mockery to generate mocks in folder `cfn-resources/testut
 # resulting file placed in cfn-resources/resource-versions.md
 # aws regions must defined by using AWS_REGIONS env variable, example: `export AWS_REGIONS=af-south-1,ap-east-1`
 .PHONY: generate-resource-versions-markdown
-generate-resource-versions-markdown: 
+generate-resource-versions-markdown:
 	(cd cfn-resources && go run tool/markdown-generator/*.go)
 
 .PHONY: gen-sbom-and-ssdlc-report

cfn-resources/access-list-api-key/cmd/resource/resource.go

@@ -268,7 +268,7 @@ func List(req handler.Request, prevModel *Model, currentModel *Model) (handler.P
 		return handleError(response, LIST, err)
 	}
 
-	accessListModels := make([]interface{}, 0)
+	accessListModels := make([]any, 0)
 	apiResults := accessListResponse.GetResults()
 	for i := range apiResults {
 		l := apiResults[i]

cfn-resources/alert-configuration/cmd/resource/resource.go

@@ -319,7 +319,7 @@ func expandAlertConfigurationNotification(notificationList []NotificationView) (
 			DatadogRegion:            notificationList[ind].DatadogRegion,
 			EmailAddress:             notificationList[ind].EmailAddress,
 			EmailEnabled:             notificationList[ind].EmailEnabled,
-			IntervalMin:              util.Pointer(int(*notificationList[ind].IntervalMin)),
+			IntervalMin:              new(int(*notificationList[ind].IntervalMin)),
 			MicrosoftTeamsWebhookUrl: notificationList[ind].MicrosoftTeamsWebhookUrl,
 			MobileNumber:             notificationList[ind].MobileNumber,
 			OpsGenieApiKey:           notificationList[ind].OpsGenieApiKey,

cfn-resources/api-key/cmd/resource/resource.go

@@ -263,7 +263,7 @@ func List(req handler.Request, prevModel *Model, currentModel *Model) (handler.P
 	}
 
 	apiKeyList := pagedAPIKeysList.GetResults()
-	apiKeys := make([]interface{}, len(apiKeyList))
+	apiKeys := make([]any, len(apiKeyList))
 	for i := range apiKeyList {
 		var model Model
 		model.readAPIKeyDetails(apiKeyList[i])
@@ -334,7 +334,7 @@ func updateOrgKeyProjectRoles(projectAssignment ProjectAssignment, client *util.
 	return assignAPIRequest.Execute()
 }
 
-func unAssignProjectFromOrgKey(projectAssignment ProjectAssignment, client *util.MongoDBClient, orgKeyID *string) (map[string]interface{}, *http.Response, error) {
+func unAssignProjectFromOrgKey(projectAssignment ProjectAssignment, client *util.MongoDBClient, orgKeyID *string) (map[string]any, *http.Response, error) {
 	unAssignAPIRequest := client.Atlas20231115014.ProgrammaticAPIKeysApi.RemoveProjectApiKey(
 		context.Background(),
 		*projectAssignment.ProjectId,
@@ -344,7 +344,7 @@ func unAssignProjectFromOrgKey(projectAssignment ProjectAssignment, client *util
 	return unAssignAPIRequest.Execute()
 }
 
-func updateProjectAssignments(atlasClient *util.MongoDBClient, currentModel *Model, existingModel *Model) (result interface{}, response *http.Response, err error) {
+func updateProjectAssignments(atlasClient *util.MongoDBClient, currentModel *Model, existingModel *Model) (result any, response *http.Response, err error) {
 	// update projectAssignments
 	newAssignments, updateAssignments, removeAssignments := getChangesInProjectAssignments(currentModel.ProjectAssignments, existingModel.ProjectAssignments)
 
@@ -438,7 +438,7 @@ func areStringArraysEqualIgnoreOrder(arr1, arr2 []string) bool {
 	copy(sortedArr2, arr2)
 	sort.Strings(sortedArr2)
 
-	for i := 0; i < len(sortedArr1); i++ {
+	for i := range sortedArr1 {
 		if sortedArr1[i] != sortedArr2[i] {
 			return false
 		}

cfn-resources/autogen/main.go

@@ -19,6 +19,7 @@ import (
 	"flag"
 	"fmt"
 	"os"
+	"strings"
 	"unicode"
 
 	"github.com/dave/jennifer/jen"
@@ -288,11 +289,11 @@ func getLiteralsWithKeys(arr []string) []jen.Code {
 }
 
 func prepareModel(arr []string) string {
-	str := ""
+	var str strings.Builder
 	for _, p := range arr {
-		str += "\t" + p + ":" + "currentModel." + p + ",\n"
+		str.WriteString("\t" + p + ":" + "currentModel." + p + ",\n")
 	}
-	return str
+	return str.String()
 }
 func capitalize(key string) string {
 	r := []rune(key)

cfn-resources/autogen/schema-gen/main.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"maps"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -242,13 +243,13 @@ func main() {
 	<-reqDone
 }
 
-func sortProperties[V any](properties map[string]V) (props map[string]interface{}) {
+func sortProperties[V any](properties map[string]V) (props map[string]any) {
 	var propertyNames []string
 	for name := range properties {
 		propertyNames = append(propertyNames, name)
 	}
 	sort.Strings(propertyNames)
-	props = make(map[string]interface{}, len(properties))
+	props = make(map[string]any, len(properties))
 	for _, name := range propertyNames {
 		props[name] = properties[name]
 	}
@@ -705,9 +706,7 @@ func mergeDefinitionMaps(map1 map[string]Definitions, map2 map[string]Definition
 	if map2 == nil {
 		return map1
 	}
-	for k, v := range map2 {
-		map1[k] = v
-	}
+	maps.Copy(map1, map2)
 	return map1
 }