Enabled last batch of linters

Signed-off-by: Kashif Khan <kashif.khan@est.tech>

.golangci.yaml

@@ -8,27 +8,37 @@ run:
 linters:
   disable-all: true
   enable:
+  - asasalint
   - asciicheck
+  - bidichk
   - bodyclose
+  # - containedctx
   - dogsled
-  # - errcheck
-  # - errorlint
+  - dupword
+  - durationcheck
+  - errcheck
+  - errchkjson
   - exportloopref
+  - gci
+  - ginkgolinter
   - goconst
   - gocritic
   - godot
   - gofmt
   - goimports
+  - goprintffuncname
+  - gosec
   - gosimple
   - govet
   - importas
-  - gosec
   - ineffassign
+  - loggercheck
   - misspell
-  # - nakedret
-  # - nilerr
-  # - noctx
+  - nakedret
+  - nilerr
+  - noctx
   - nolintlint
+  - nosprintfhostport
   - prealloc
   - predeclared
   - revive
@@ -38,7 +48,9 @@ linters:
   - thelper
   - typecheck
   - unconvert
+  - unparam
   - unused
+  - usestdlibvars
   - whitespace
   # Run with --fast=false for more extensive checks
   fast: true
@@ -114,6 +126,9 @@ issues:
     - unused
     - revive
     - stylecheck
+    - noctx
+    - unparam
+    - gci
   # Specific exclude rules for deprecated fields that are still part of the codebase.
   # These should be removed as the referenced deprecated item is removed from the project.
   - linters:

apis/metal3.io/v1alpha1/baremetalhost_types.go

@@ -132,7 +132,7 @@ const (
 	OperationalStatusOK OperationalStatus = "OK"
 
 	// OperationalStatusDiscovered is the status value for when the
-	// host is only partially configured, such as when when the BMC
+	// host is only partially configured, such as when the BMC
 	// address is known but the login credentials are not.
 	OperationalStatusDiscovered OperationalStatus = "discovered"
 

apis/metal3.io/v1alpha1/baremetalhost_types_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )

apis/metal3.io/v1alpha1/baremetalhost_validation.go

@@ -11,7 +11,6 @@ import (
 	"strings"
 
 	"github.com/google/uuid"
-
 	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
 )
 

apis/metal3.io/v1alpha1/baremetalhost_webhook.go

@@ -15,9 +15,8 @@ package v1alpha1
 import (
 	"fmt"
 
-	kerrors "k8s.io/apimachinery/pkg/util/errors"
-
 	"k8s.io/apimachinery/pkg/runtime"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )

apis/metal3.io/v1alpha1/bmceventsubscription_webhook.go

@@ -15,9 +15,8 @@ package v1alpha1
 import (
 	"fmt"
 
-	kerrors "k8s.io/apimachinery/pkg/util/errors"
-
 	"k8s.io/apimachinery/pkg/runtime"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )

apis/metal3.io/v1alpha1/hostfirmwaresettings_types_test.go

@@ -4,7 +4,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )

cmd/get-hardware-details/main.go

@@ -12,10 +12,9 @@ import (
 	"strings"
 
 	"github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes"
-	"k8s.io/klog/v2"
-
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/clients"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner/ironic/hardwaredetails"
+	"k8s.io/klog/v2"
 )
 
 type options struct {

cmd/make-virt-host/main.go

@@ -156,7 +156,14 @@ func main() {
 	}
 
 	domainResult := Domain{}
-	xml.Unmarshal(virshOut, &domainResult)
+	err = xml.Unmarshal(virshOut, &domainResult)
+	if err != nil {
+		fmt.Fprintf(os.Stderr,
+			"ERROR: Could not unmarshal details of domain %s: %s\n",
+			virshDomain, err)
+		os.Exit(1)
+	}
+
 	if *verbose {
 		fmt.Printf("%v\n", domainResult)
 	}
@@ -192,7 +199,12 @@ func main() {
 	}
 
 	var vbmcResult []VBMC
-	json.Unmarshal(vbmcOut, &vbmcResult)
+	_ = json.Unmarshal(vbmcOut, &vbmcResult)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "ERROR: Could not unmarshal details of vbmc: %s\n", err)
+		os.Exit(1)
+	}
+
 	nameToPort := make(map[string]int)
 	for _, vbmc := range vbmcResult {
 		if *verbose {

controllers/metal3.io/action_result.go

@@ -6,10 +6,9 @@ import (
 	"math/rand"
 	"time"
 
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-
 	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
 // This is an upper limit for the ErrorCount, so that the max backoff

controllers/metal3.io/baremetalhost_controller.go

@@ -24,9 +24,15 @@ import (
 	"time"
 
 	"github.com/go-logr/logr"
+	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+	"github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1/profile"
+	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
+	"github.com/metal3-io/baremetal-operator/pkg/imageprovider"
+	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
+	"github.com/metal3-io/baremetal-operator/pkg/secretutils"
+	"github.com/metal3-io/baremetal-operator/pkg/utils"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
-
 	corev1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -39,14 +45,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
-
-	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
-	"github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1/profile"
-	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
-	"github.com/metal3-io/baremetal-operator/pkg/imageprovider"
-	"github.com/metal3-io/baremetal-operator/pkg/provisioner"
-	"github.com/metal3-io/baremetal-operator/pkg/secretutils"
-	"github.com/metal3-io/baremetal-operator/pkg/utils"
 )
 
 const (
@@ -224,7 +222,7 @@ func (r *BareMetalHostReconciler) Reconcile(ctx context.Context, request ctrl.Re
 
 	if err != nil {
 		err = errors.Wrap(err, fmt.Sprintf("action %q failed", initialState))
-		return
+		return result, err
 	}
 
 	// Only save status when we're told to, otherwise we
@@ -253,7 +251,7 @@ func (r *BareMetalHostReconciler) Reconcile(ctx context.Context, request ctrl.Re
 
 	logResult(info, result)
 
-	return
+	return result, nil
 }
 
 // Consume inspect.metal3.io/hardwaredetails when either
@@ -717,7 +715,11 @@ func (r *BareMetalHostReconciler) getPreprovImage(info *reconcileInfo, formats [
 			},
 			Spec: expectedSpec,
 		}
-		controllerutil.SetControllerReference(info.host, &preprovImage, r.Scheme())
+		err = controllerutil.SetControllerReference(info.host, &preprovImage, r.Scheme())
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to set controller reference for PreprovisioningImage")
+		}
+
 		err = r.Create(info.ctx, &preprovImage)
 		return nil, err
 	}
@@ -1459,7 +1461,7 @@ func saveHostProvisioningSettings(host *metal3api.BareMetalHost, info *reconcile
 	// Root device hints may change as a result of RAID
 	dirty, err = updateRootDeviceHints(host, info)
 	if err != nil {
-		return
+		return dirty, err
 	}
 
 	// Copy RAID settings
@@ -1494,7 +1496,7 @@ func saveHostProvisioningSettings(host *metal3api.BareMetalHost, info *reconcile
 		dirty = true
 	}
 
-	return
+	return dirty, nil
 }
 
 func (r *BareMetalHostReconciler) createHostFirmwareSettings(info *reconcileInfo) error {

controllers/metal3.io/baremetalhost_controller_test.go

@@ -8,29 +8,25 @@ import (
 	"fmt"
 	"testing"
 
+	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
+	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
+	"github.com/metal3-io/baremetal-operator/pkg/provisioner/fixture"
+	"github.com/metal3-io/baremetal-operator/pkg/secretutils"
+	"github.com/metal3-io/baremetal-operator/pkg/utils"
+	promutil "github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
-
 	corev1 "k8s.io/api/core/v1"
-
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
-
-	promutil "github.com/prometheus/client_golang/prometheus/testutil"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-
-	metal3api "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
-	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
-	"github.com/metal3-io/baremetal-operator/pkg/provisioner/fixture"
-	"github.com/metal3-io/baremetal-operator/pkg/secretutils"
-	"github.com/metal3-io/baremetal-operator/pkg/utils"
 )
 
 const (
@@ -142,7 +138,7 @@ func newTestReconcilerWithFixture(fix *fixture.Fixture, initObjs ...runtime.Obje
 	c := clientBuilder.Build()
 	// Add a default secret that can be used by most hosts.
 	bmcSecret := newBMCCredsSecret(defaultSecretName, "User", "Pass")
-	c.Create(context.TODO(), bmcSecret)
+	_ = c.Create(context.TODO(), bmcSecret)
 
 	return &BareMetalHostReconciler{
 		Client:             c,
@@ -551,15 +547,17 @@ func TestDoNotAddSecretFinalizersDuringDelete(t *testing.T) {
 	// The fake client will immediately remove the host
 	// from its cache, so let's keep the latest updated
 	// host
-	r.Get(context.TODO(), request.NamespacedName, host)
+	err = r.Get(context.TODO(), request.NamespacedName, host)
+	assert.NoError(t, err)
 	_, err = r.Reconcile(context.Background(), request)
 	assert.NoError(t, err)
 
 	// To simulate an immediate reconciliation loop due the
 	// secret update (and a slow host deletion), let's push
 	// back the host in the client cache.
 	host.ResourceVersion = ""
-	r.Client.Create(context.TODO(), host)
+	err = r.Client.Create(context.TODO(), host)
+	assert.NoError(t, err)
 	previousSecret := getHostSecret(t, r, host)
 	_, err = r.Reconcile(context.Background(), request)
 	assert.NoError(t, err)
@@ -776,7 +774,8 @@ func TestRebootWithSuffixedAnnotation(t *testing.T) {
 	)
 
 	delete(host.Annotations, annotation)
-	r.Update(context.TODO(), host)
+	err := r.Update(context.TODO(), host)
+	assert.NoError(t, err)
 
 	tryReconcile(t, r, host,
 		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
@@ -2683,7 +2682,8 @@ func TestHostFirmwareSettings(t *testing.T) {
 			i.request = newRequest(host)
 
 			hfs := newHostFirmwareSettings(host, tc.Conditions)
-			r.Create(context.TODO(), hfs)
+			err := r.Create(context.TODO(), hfs)
+			assert.NoError(t, err)
 
 			dirty, _, err := r.getHostFirmwareSettings(i)
 			if err != nil {
@@ -2751,7 +2751,8 @@ func TestHFSTransitionToPreparing(t *testing.T) {
 		},
 	}
 
-	r.Update(context.TODO(), hfs)
+	err := r.Update(context.TODO(), hfs)
+	assert.NoError(t, err)
 
 	waitForProvisioningState(t, r, host, metal3api.StatePreparing)
 }
@@ -2783,7 +2784,8 @@ func TestHFSEmptyStatusSettings(t *testing.T) {
 		},
 	}
 
-	r.Update(context.TODO(), hfs)
+	err := r.Update(context.TODO(), hfs)
+	assert.NoError(t, err)
 
 	tryReconcile(t, r, host,
 		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
@@ -2799,7 +2801,8 @@ func TestHFSEmptyStatusSettings(t *testing.T) {
 		},
 	}
 
-	r.Update(context.TODO(), hfs)
+	err = r.Update(context.TODO(), hfs)
+	assert.NoError(t, err)
 	tryReconcile(t, r, host,
 		func(host *metal3api.BareMetalHost, result reconcile.Result) bool {
 			return host.Status.Provisioning.State == metal3api.StateAvailable