Merge pull request #594 from atombrella/feature/upgrade_golangci_lint

Update to golangci-lint version 2

Author: tanmay-hc

.github/workflows/go-validate.yml

@@ -49,9 +49,9 @@ jobs:
       - uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6.3.0
         with:
           go-version: ${{ needs.get-go-version.outputs.go-version }}
-      - uses: golangci/golangci-lint-action@82d40c283aeb1f2b6595839195e95c2d6a49081b # v5.0.0
+      - uses: golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20 # v9.2.0
         with:
-          version: v1.64.8
+          version: v2.10.1
           only-new-issues: true
   check-fmt:
     needs:

.golangci.yml

@@ -1,44 +1,37 @@
 # Copyright IBM Corp. 2013, 2025
 # SPDX-License-Identifier: MPL-2.0
 
-issues:
-  # List of regexps of issue texts to exclude, empty list by default.
-  # But independently from this option we use default exclude patterns,
-  # it can be disabled by `exclude-use-default: false`. To list all
-  # excluded by default patterns execute `golangci-lint run --help`
-
-  exclude-rules:
-    # Exclude gosimple bool check
-    - linters:
-      - gosimple
-      text: "S(1002|1008|1021)"
-    # Exclude failing staticchecks for now
-    - linters:
-        - staticcheck
-      text: "SA(1006|1019|4006|4010|4017|5007|6005|9004):"
-    # Exclude lll issues for long lines with go:generate
-    - linters:
-        - lll
-      source: "^//go:generate "
-
-  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
-  max-issues-per-linter: 0
-
-  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
-  max-same-issues: 0
+version: "2"
 
 linters:
-  disable-all: true
   enable:
-    - errcheck
-    - goimports
-    - gosimple
+    - asasalint
+    - asciicheck
+    - bidichk
+    - decorder
+    - dupword
     - govet
     - ineffassign
+    - misspell
+    - nilerr
+    - reassign
     - staticcheck
     - unconvert
     - unused
-  fast: true
+  settings:
+    errcheck:
+      # report about not checking of errors in type assertions: `a := b.(MyStruct)`;
+      # default is false: such cases aren't reported by default.
+      check-type-assertions: false
+
+      # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
+      # default is false: such cases aren't reported by default.
+      check-blank: false
+    staticcheck:
+      # SAxxxx checks in https://staticcheck.dev/docs/configuration/options/#checks
+      # Example (to disable some checks): [ "all", "-SA1000", "-SA1001"]
+      # Run `GL_DEBUG=staticcheck golangci-lint run --enable=staticcheck` to see all available checks and enabled by config checks.
+      checks: ["all", "-SA4006", "-ST1000", "-ST1003", "-ST1005", "-ST1012", "-ST1016", "-ST1020", "-ST1021", "-ST1022", "-QF1001", "-QF1004", "-QF1008", "-QF1012"]
 
 # options for analysis running
 run:
@@ -54,72 +47,15 @@ run:
   # include test files or not, default is true
   tests: true
 
-  # list of build tags, all linters use it. Default is empty list.
-  #build-tags:
-  #  - mytag
-
-  # which dirs to skip: issues from them won't be reported;
-  # can use regexp here: generated.*, regexp is applied on full path;
-  # default value is empty list, but default dirs are skipped independently
-  # from this option's value (see skip-dirs-use-default).
-  #skip-dirs:
-  #  - src/external_libs
-  #  - autogenerated_by_my_lib
-
-  # default is true. Enables skipping of directories:
-  #   vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
-  skip-dirs-use-default: true
-
   # which files to skip: they will be analyzed, but issues from them
   # won't be reported. Default value is empty list, but there is
   # no need to include all autogenerated files, we confidently recognize
   # autogenerated files. If it's not please let us know.
-  skip-files:
-    - ".*\\.hcl2spec\\.go$"
+  # skip-files:
+  #   - ".*\\.hcl2spec\\.go$"
   #  - lib/bad.go
 
-  # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules":
-  # If invoked with -mod=readonly, the go command is disallowed from the implicit
-  # automatic updating of go.mod described above. Instead, it fails when any changes
-  # to go.mod are needed. This setting is most useful to check that go.mod does
-  # not need updates, such as in a continuous integration and testing system.
-  # If invoked with -mod=vendor, the go command assumes that the vendor
-  # directory holds the correct copies of dependencies and ignores
-  # the dependency descriptions in go.mod.
-  # modules-download-mode: vendor
-
-
-# output configuration options
-output:
-  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
-  format: colored-line-number
-
-  # print lines of code with issue, default is true
-  print-issued-lines: true
-
-  # print linter name in the end of issue text, default is true
-  print-linter-name: true
-
-  # make issues output unique by line, default is true
-  uniq-by-line: true
-
-
-# all available settings of specific linters
-linters-settings:
- errcheck:
-    # report about not checking of errors in type assertions: `a := b.(MyStruct)`;
-    # default is false: such cases aren't reported by default.
-    check-type-assertions: false
-
-    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
-    # default is false: such cases aren't reported by default.
-    check-blank: false
-
-    # [deprecated] comma-separated list of pairs of the form pkg:regex
-    # the regex is used to ignore names within pkg. (default "fmt:.*").
-    # see https://github.com/kisielk/errcheck#the-deprecated-method for details
-    ignore: fmt:.*,io/ioutil:^Read.*,io:Close
-
-    # path to a file containing a list of functions to exclude from checking
-    # see https://github.com/kisielk/errcheck#excluding-functions for details
-    #exclude: /path/to/file.txt
+formatters:
+  enable:
+    - gofmt
+    - goimports

builder/azure/arm/azure_error_response.go

@@ -45,18 +45,18 @@ func (e *azureErrorResponse) Error() string {
 //
 // Errors may contain nested errors, which are JSON documents that have been
 // serialized and escaped.  Keep following this nesting all the way down...
-func formatAzureErrorResponse(error azureErrorDetails, buf *bytes.Buffer, indent string) {
-	if error.isEmpty() {
+func formatAzureErrorResponse(err azureErrorDetails, buf *bytes.Buffer, indent string) {
+	if err.isEmpty() {
 		return
 	}
 
-	buf.WriteString(fmt.Sprintf("ERROR: %s-> %s : %s\n", indent, error.Code, error.Message))
-	for _, x := range error.Details {
+	buf.WriteString(fmt.Sprintf("ERROR: %s-> %s : %s\n", indent, err.Code, err.Message))
+	for _, x := range err.Details {
 		newIndent := fmt.Sprintf("%s  ", indent)
 
 		var aer azureErrorResponse
-		err := json.Unmarshal([]byte(x.Message), &aer)
-		if err == nil {
+		errJson := json.Unmarshal([]byte(x.Message), &aer)
+		if errJson == nil {
 			buf.WriteString(fmt.Sprintf("ERROR: %s-> %s\n", newIndent, x.Code))
 			formatAzureErrorResponse(aer.ErrorDetails, buf, newIndent)
 		} else {

builder/azure/arm/builder.go

@@ -226,7 +226,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 		buildGroupId := commonids.NewResourceGroupID(b.config.ClientConfig.SubscriptionID, b.config.BuildResourceGroupName)
 		group, err := azureClient.ResourceGroupsClient.Get(builderPollingContext, buildGroupId)
 		if err != nil {
-			return nil, fmt.Errorf("Cannot locate the existing build resource resource group %s.", b.config.BuildResourceGroupName)
+			return nil, fmt.Errorf("Cannot locate the existing build resource group %s.", b.config.BuildResourceGroupName)
 		}
 
 		b.config.Location = group.Model.Location
@@ -336,7 +336,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 				normalizedRegions = append(normalizedRegions, TargetRegion{Name: region, ReplicaCount: b.config.SharedGalleryImageVersionReplicaCount})
 			}
 			// SIG requires that replication regions include the region in which the created image version resides
-			if foundMandatoryReplicationRegion == false {
+			if !foundMandatoryReplicationRegion {
 				normalizedRegions = append(normalizedRegions, TargetRegion{
 					Name:                buildLocation,
 					DiskEncryptionSetId: b.config.DiskEncryptionSetId,
@@ -393,7 +393,8 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 	}
 	generatedData := &packerbuilderdata.GeneratedData{State: b.stateBag}
 	var steps []multistep.Step
-	if b.config.OSType == constants.Target_Linux {
+	switch b.config.OSType {
+	case constants.Target_Linux:
 		steps = []multistep.Step{
 			NewStepGetSourceImageName(azureClient, ui, &b.config, generatedData),
 			NewStepCreateResourceGroup(azureClient, ui),
@@ -415,7 +416,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 			NewStepSnapshotOSDisk(azureClient, ui, &b.config),
 			NewStepSnapshotDataDisks(azureClient, ui, &b.config),
 		}
-	} else if b.config.OSType == constants.Target_Windows {
+	case constants.Target_Windows:
 		steps = []multistep.Step{
 			NewStepGetSourceImageName(azureClient, ui, &b.config, generatedData),
 			NewStepCreateResourceGroup(azureClient, ui),
@@ -495,7 +496,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 			NewStepSnapshotOSDisk(azureClient, ui, &b.config),
 			NewStepSnapshotDataDisks(azureClient, ui, &b.config),
 		)
-	} else {
+	default:
 		return nil, fmt.Errorf("Builder does not support the os_type '%s'", b.config.OSType)
 	}
 
@@ -574,6 +575,7 @@ func (b *Builder) writeSSHPrivateKey(ui packersdk.Ui, debugKeyPath string) {
 	if err != nil {
 		ui.Say(fmt.Sprintf("Error saving debug key: %s", err))
 	}
+	//nolint:errcheck
 	defer f.Close()
 
 	// Write the key out
@@ -706,7 +708,7 @@ func (b *Builder) setImageParameters(stateBag multistep.StateBag) {
 }
 
 func normalizeAzureRegion(name string) string {
-	return strings.ToLower(strings.Replace(name, " ", "", -1))
+	return strings.ToLower(strings.ReplaceAll(name, " ", ""))
 }
 
 func (b *Builder) enhanceStateData(stateData map[string]interface{}) {

builder/azure/arm/builder_acc_test.go

@@ -257,6 +257,7 @@ func TestBuilderUserData_Linux(t *testing.T) {
 		t.Fatalf("failed creating tempfile: %s", err)
 	}
 
+	//nolint:errcheck
 	defer os.Remove(tmpfile.Name())
 
 	if _, err := tmpfile.WriteString(testBuilderCustomDataLinux); err != nil {

builder/azure/arm/config.go

@@ -22,7 +22,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer-plugin-sdk/random"
 
 	"github.com/hashicorp/go-azure-sdk/resource-manager/compute/2022-03-01/images"
@@ -975,7 +974,7 @@ func setWinRMCertificate(c *Config) error {
 	cert, err := c.createCertificate()
 
 	// Hide the generated certificate from logs
-	packer.LogSecretFilter.Set(cert)
+	packersdk.LogSecretFilter.Set(cert)
 
 	c.winrmCertificate = cert
 
@@ -1171,7 +1170,7 @@ func assertRequiredParametersSet(c *Config, errs *packersdk.MultiError) {
 			if err != nil {
 				err := fmt.Errorf("Error parsing resource ID from `user_assigned_managed_identities`; please make sure"+
 					" that this value follows the full resource id format: "+
-					"/subscriptions/<SUBSCRIPTON_ID>/resourcegroups/<RESOURCE_GROUP>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/<USER_ASSIGNED_IDENTITY_NAME>.\n"+
+					"/subscriptions/<SUBSCRIPTION_ID>/resourcegroups/<RESOURCE_GROUP>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/<USER_ASSIGNED_IDENTITY_NAME>.\n"+
 					" Original error: %s", err)
 				errs = packersdk.MultiErrorAppend(errs, err)
 			} else {
@@ -1318,7 +1317,7 @@ func assertRequiredParametersSet(c *Config, errs *packersdk.MultiError) {
 	}
 
 	if c.SkipCreateBuildKeyVault && !strings.EqualFold(c.Comm.Type, "winrm") {
-		errs = packer.MultiErrorAppend(errs, fmt.Errorf("Communicator type must be winrm when skip_create_build_key_vault is set"))
+		errs = packersdk.MultiErrorAppend(errs, fmt.Errorf("Communicator type must be winrm when skip_create_build_key_vault is set"))
 	}
 
 	/////////////////////////////////////////////

builder/azure/arm/config_test.go

@@ -2683,6 +2683,9 @@ func TestConfigShouldRejectUserDataAndUserDataFile(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed creating tempfile: %s", err)
 	}
+	//nolint:errcheck
+	defer os.Remove(tmpfile.Name())
+
 	config := map[string]interface{}{
 		"capture_container_name": "ignore",
 		"capture_name_prefix":    "ignore",
@@ -2703,7 +2706,6 @@ func TestConfigShouldRejectUserDataAndUserDataFile(t *testing.T) {
 	var c Config
 	_, err = c.Prepare(config, getPackerConfiguration())
 
-	defer os.Remove(tmpfile.Name())
 	if err == nil {
 		t.Fatal("expected config to reject the use of both user_data and user_data_file")
 	}
@@ -2714,6 +2716,9 @@ func TestConfigShouldRejectCustomDataAndCustomDataFile(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed creating tempfile: %s", err)
 	}
+	//nolint:errcheck
+	defer os.Remove(tmpfile.Name())
+
 	config := map[string]interface{}{
 		"capture_container_name": "ignore",
 		"capture_name_prefix":    "ignore",
@@ -2734,7 +2739,6 @@ func TestConfigShouldRejectCustomDataAndCustomDataFile(t *testing.T) {
 	var c Config
 	_, err = c.Prepare(config, getPackerConfiguration())
 
-	defer os.Remove(tmpfile.Name())
 	if err == nil {
 		t.Fatal("expected config to reject the use of both custom_data and custom_data_file")
 	}

builder/azure/arm/template_factory.go

@@ -336,7 +336,7 @@ func GetVirtualMachineTemplateBuilder(config *Config) (*template.TemplateBuilder
 	}
 
 	// In Azure Network Security Groups define the access controls for the network
-	// They are required when specifiying which inbound IPs are allowed to connect to the network
+	// They are required when specifying which inbound IPs are allowed to connect to the network
 	// They are also required when creating a standard sku public IP Address regardless of inbound IPs allowed.
 	// If a standard IP is set with no inbound addresses, we default to allowing all IP addresses
 	if (config.PublicIpSKU != "Basic") || (len(config.AllowedInboundIpAddresses) >= 1) {

builder/azure/chroot/builder.go

@@ -551,7 +551,7 @@ func buildsteps(
 					}),
 				)
 			} else {
-				panic("Couldn't parse platfrom image urn: " + config.Source + " err: " + err.Error())
+				panic("Couldn't parse platform image urn: " + config.Source + " err: " + err.Error())
 			}
 
 		case sourceDisk:

builder/azure/chroot/shared_image_gallery_destination.go

@@ -65,7 +65,7 @@ func (sigd *SharedImageGalleryDestination) Validate(prefix string) (errs []error
 		warns = append(warns,
 			fmt.Sprintf("%s.target_regions is empty; image will only be available in the region of the gallery", prefix))
 	}
-	if sigd.ExcludeFromLatestTypo == true && sigd.ExcludeFromLatest == false {
+	if sigd.ExcludeFromLatestTypo && !sigd.ExcludeFromLatest {
 		warns = append(warns,
 			fmt.Sprintf("%s.exlude_from_latest is being deprecated, please use exclude_from_latest", prefix))
 		sigd.ExcludeFromLatest = sigd.ExcludeFromLatestTypo