Fix linting issues

Signed-off-by: Jacob Weinstock <[email protected]>

.golangci.yml

@@ -80,6 +80,10 @@ linters-settings:
       - name: var-naming
       - name: unconditional-recursion
       - name: waitgroup-by-value
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag
+      - name: struct-tag
+        arguments:
+          - "json,inline"
   staticcheck:
     go: "1.18"
   unused:

Tools.mk

@@ -32,7 +32,7 @@ KUSTOMIZE     := $(TOOLS_DIR)/kustomize-$(KUSTOMIZE_VER)
 SETUP_ENVTEST_VER := v0.0.0-20220304125252-9ee63fc65a97
 SETUP_ENVTEST     := $(TOOLS_DIR)/setup-envtest-$(SETUP_ENVTEST_VER)
 
-GOLANGCI_LINT_VER := v1.52
+GOLANGCI_LINT_VER := v1.61
 GOLANGCI_LINT     := $(TOOLS_DIR)/golangci-lint-$(GOLANGCI_LINT_VER)
 
 YAMLFMT_VER := v0.6

api/v1alpha1/workflow_methods.go

@@ -60,7 +60,7 @@ func (w *Workflow) getTaskActionInfo() taskInfo {
 	INNER:
 		for ai, action := range task.Actions {
 			// Find the first non-successful action
-			switch action.Status {
+			switch action.Status { //nolint:exhaustive // WorkflowStatePreparing is only used in Workflows not Actions.
 			case WorkflowStateSuccess:
 				actionIndex++
 				continue

api/v1alpha1/workflow_types.go

@@ -89,7 +89,7 @@ type Status struct {
 	// is no information available. A Reason clarifies an HTTP status
 	// code but does not override it.
 	// +optional
-	Reason metav1.StatusReason `json:"reason,omitempty" protobuf:"bytes,4,opt,name=reason,casttype=StatusReason"`
+	Reason metav1.StatusReason `json:"reason,omitempty" protobuf:"bytes,4,opt,name=reason"`
 	// Extended data associated with the reason.  Each reason may define its
 	// own extended details. This field is optional and the data returned
 	// is not guaranteed to conform to any schema except that defined by

cmd/tink-controller-v1alpha2/main.go

@@ -73,14 +73,14 @@ func NewRootCommand() *cobra.Command {
 
 	cmd := &cobra.Command{
 		Use: "tink-controller",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
+		PreRunE: func(cmd *cobra.Command, _ []string) error {
 			viper, err := createViper(logger)
 			if err != nil {
 				return fmt.Errorf("config init: %w", err)
 			}
 			return applyViper(viper, cmd)
 		},
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			logger.Info("Starting controller version " + version)
 
 			ccfg := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
@@ -172,7 +172,7 @@ func applyViper(v *viper.Viper, cmd *cobra.Command) error {
 		for _, err := range errors {
 			errs = append(errs, err.Error())
 		}
-		return fmt.Errorf(strings.Join(errs, ", "))
+		return fmt.Errorf("%s", strings.Join(errs, ", "))
 	}
 
 	return nil

cmd/tink-controller/main.go

@@ -60,14 +60,14 @@ func NewRootCommand() *cobra.Command {
 
 	cmd := &cobra.Command{
 		Use: "tink-controller",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
+		PreRunE: func(cmd *cobra.Command, _ []string) error {
 			viper, err := createViper(logger)
 			if err != nil {
 				return fmt.Errorf("config init: %w", err)
 			}
 			return applyViper(viper, cmd)
 		},
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			logger.Info("Starting controller version " + version)
 
 			ccfg := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(
@@ -146,7 +146,7 @@ func applyViper(v *viper.Viper, cmd *cobra.Command) error {
 		for _, err := range errors {
 			errs = append(errs, err.Error())
 		}
-		return fmt.Errorf(strings.Join(errs, ", "))
+		return fmt.Errorf("%s", strings.Join(errs, ", "))
 	}
 
 	return nil

cmd/tink-server/main.go

@@ -78,14 +78,14 @@ func NewRootCommand() *cobra.Command {
 
 	cmd := &cobra.Command{
 		Use: "tink-server",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
+		PreRunE: func(cmd *cobra.Command, _ []string) error {
 			viper, err := createViper(logger)
 			if err != nil {
 				return err
 			}
 			return applyViper(viper, cmd)
 		},
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			// I am not sure if it is right for this to be here,
 			// but as last step I want to keep compatibility with
 			// what we have for a little bit and I thinik that's
@@ -201,7 +201,7 @@ func applyViper(v *viper.Viper, cmd *cobra.Command) error {
 		for _, err := range errors {
 			errs = append(errs, err.Error())
 		}
-		return fmt.Errorf(strings.Join(errs, ", "))
+		return fmt.Errorf("%s", strings.Join(errs, ", "))
 	}
 
 	return nil

cmd/tink-worker/cmd/root.go

@@ -39,10 +39,10 @@ func NewRootCommand(version string) *cobra.Command {
 		Use:     "tink-worker",
 		Short:   "Tink Worker",
 		Version: version,
-		PreRunE: func(cmd *cobra.Command, args []string) error {
+		PreRunE: func(cmd *cobra.Command, _ []string) error {
 			return initViper(logger, cmd)
 		},
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			retryInterval := viper.GetDuration("retry-interval")
 			retries := viper.GetInt("max-retry")
 			workerID := viper.GetString("id")

cmd/tink-worker/worker/log_capturer_test.go

@@ -94,7 +94,7 @@ func TestLogCapturerContextLogger(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
+		t.Run(tc.name, func(_ *testing.T) {
 			logger := zapr.NewLogger(zap.Must(zap.NewDevelopment()))
 			ctx := context.Background()
 			if tc.logger != nil {

cmd/virtual-worker/cmd/root.go

@@ -34,10 +34,10 @@ func NewRootCommand(version string) *cobra.Command {
 	rootCmd := &cobra.Command{
 		Use:   "virtual-worker",
 		Short: "Virtual Tink Worker",
-		PreRunE: func(cmd *cobra.Command, args []string) error {
+		PreRunE: func(cmd *cobra.Command, _ []string) error {
 			return createViper(logger, cmd)
 		},
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			retryInterval := viper.GetDuration("retry-interval")
 			retries := viper.GetInt("max-retry")
 			workerID := viper.GetString("id")

cmd/virtual-worker/worker/container_manager.go

@@ -30,7 +30,7 @@ type fakeManager struct {
 }
 
 func (m *fakeManager) sleep() {
-	jitter := time.Duration(m.r.Int31n(int32(m.sleepJitter.Milliseconds()))) * time.Millisecond
+	jitter := time.Duration(m.r.Int63n(m.sleepJitter.Milliseconds())) * time.Millisecond
 	time.Sleep(jitter + m.sleepMinimum)
 }
 

internal/agent/agent_test.go

@@ -105,7 +105,7 @@ func TestAgent_ConcurrentWorkflows(t *testing.T) {
 	// Started is used to indicate the runtime has received the workflow.
 	started := make(chan struct{})
 	rntime := agent.ContainerRuntimeMock{
-		RunFunc: func(ctx context.Context, action workflow.Action) error {
+		RunFunc: func(ctx context.Context, _ workflow.Action) error {
 			started <- struct{}{}
 			<-ctx.Done()
 			return nil
@@ -412,7 +412,7 @@ message`,
 			trnport := transport.Noop()
 
 			rntime := agent.ContainerRuntimeMock{
-				RunFunc: func(ctx context.Context, action workflow.Action) error {
+				RunFunc: func(_ context.Context, action workflow.Action) error {
 					if res, ok := tc.Errors[action.ID]; ok {
 						return failure.NewReason(res.Message, res.Reason)
 					}
@@ -424,7 +424,7 @@ message`,
 			// to check for the last expected action.
 			lastEventReceived := make(chan struct{})
 			recorder := event.RecorderMock{
-				RecordEventFunc: func(contextMoqParam context.Context, event event.Event) error {
+				RecordEventFunc: func(_ context.Context, event event.Event) error {
 					if cmp.Equal(event, tc.Events[len(tc.Events)-1]) {
 						lastEventReceived <- struct{}{}
 					}

internal/agent/transport/file_test.go

@@ -46,7 +46,7 @@ func TestFile(t *testing.T) {
 	defer cancel()
 
 	handler := &transport.WorkflowHandlerMock{
-		HandleWorkflowFunc: func(contextMoqParam context.Context, workflow workflow.Workflow, recorder event.Recorder) {
+		HandleWorkflowFunc: func(_ context.Context, workflow workflow.Workflow, _ event.Recorder) {
 			if !cmp.Equal(expect, workflow) {
 				t.Fatalf("Workflow diff:\n%v", cmp.Diff(expect, workflow))
 			}

internal/agent/transport/grpc_test.go

@@ -46,15 +46,15 @@ func TestGRPC(t *testing.T) {
 		ContextFunc: context.Background,
 	}
 	client := &workflowproto.WorkflowServiceClientMock{
-		GetWorkflowsFunc: func(ctx context.Context, in *workflowproto.GetWorkflowsRequest, opts ...grpc.CallOption) (workflowproto.WorkflowService_GetWorkflowsClient, error) {
+		GetWorkflowsFunc: func(_ context.Context, _ *workflowproto.GetWorkflowsRequest, _ ...grpc.CallOption) (workflowproto.WorkflowService_GetWorkflowsClient, error) {
 			return stream, nil
 		},
 	}
 
 	var wg sync.WaitGroup
 	wg.Add(1)
 	handler := &transport.WorkflowHandlerMock{
-		HandleWorkflowFunc: func(contextMoqParam context.Context, workflow workflow.Workflow, recorder event.Recorder) {
+		HandleWorkflowFunc: func(_ context.Context, _ workflow.Workflow, _ event.Recorder) {
 			defer wg.Done()
 			close(responses)
 		},

internal/cli/agent.go

@@ -23,7 +23,7 @@ func NewAgent() *cobra.Command {
 	// TODO(chrisdoherty4) Handle signals
 	cmd := cobra.Command{
 		Use: "tink-agent",
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, _ []string) error {
 			zl, err := zap.NewProduction()
 			if err != nil {
 				return fmt.Errorf("init logger: %w", err)
@@ -35,7 +35,7 @@ func NewAgent() *cobra.Command {
 				return fmt.Errorf("create runtime: %w", err)
 			}
 
-			conn, err := grpc.DialContext(cmd.Context(), opts.TinkServerAddr)
+			conn, err := grpc.NewClient(opts.TinkServerAddr)
 			if err != nil {
 				return fmt.Errorf("dial tink server: %w", err)
 			}

internal/client/client.go

@@ -18,7 +18,7 @@ func NewClientConn(authority string, tlsEnabled bool, tlsInsecure bool) (*grpc.C
 		creds = grpc.WithTransportCredentials(insecure.NewCredentials())
 	}
 
-	conn, err := grpc.Dial(authority, creds, grpc.WithStatsHandler(otelgrpc.NewClientHandler()))
+	conn, err := grpc.NewClient(authority, creds, grpc.WithStatsHandler(otelgrpc.NewClientHandler()))
 	if err != nil {
 		return nil, errors.Wrap(err, "dial tinkerbell server")
 	}

internal/deprecated/workflow/reconciler.go

@@ -10,7 +10,6 @@ import (
 	"github.com/tinkerbell/tink/api/v1alpha1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -111,7 +110,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
 }
 
 func (r *Reconciler) handleHardwareAllowPXE(ctx context.Context, stored *v1alpha1.Workflow, hardware *v1alpha1.Hardware) error {
-
 	// We need to set allowPXE to true before a workflow runs.
 	// We need to set allowPXE to false after a workflow completes successfully.
 
@@ -206,7 +204,7 @@ func (r *Reconciler) processNewWorkflow(ctx context.Context, logger logr.Logger,
 	}
 
 	// netboot the hardware if requested
-	if stored.Spec.BootOpts.OneTimeNetboot {
+	if stored.Spec.BootOpts.OneTimeNetboot { //nolint:nestif // Will work on this complexity.
 		// check if the hardware has a bmcRef
 		if hardware.Spec.BMCRef == nil {
 			return reconcile.Result{}, fmt.Errorf("hardware %s does not have a BMC, cannot perform one time netboot", hardware.Name)
@@ -227,12 +225,10 @@ func (r *Reconciler) processNewWorkflow(ctx context.Context, logger logr.Logger,
 				stored.Status.OneTimeNetboot.DeletionStatus = &v1alpha1.Status{Status: v1alpha1.StatusSuccess, Message: "previous existing one time netboot job deleted"}
 
 				return reconcile.Result{Requeue: true}, nil
+			} else if errors.IsNotFound(err) {
+				stored.Status.OneTimeNetboot.DeletionStatus = &v1alpha1.Status{Status: v1alpha1.StatusSuccess, Message: "no existing one time netboot job found"}
 			} else {
-				if apierrors.IsNotFound(err) {
-					stored.Status.OneTimeNetboot.DeletionStatus = &v1alpha1.Status{Status: v1alpha1.StatusSuccess, Message: "no existing one time netboot job found"}
-				} else {
-					return reconcile.Result{Requeue: true}, err
-				}
+				return reconcile.Result{Requeue: true}, err
 			}
 		}
 
@@ -329,7 +325,7 @@ func toTemplateHardwareData(hardware v1alpha1.Hardware) templateHardwareData {
 	return contract
 }
 
-func (r *Reconciler) processRunningWorkflow(_ context.Context, stored *v1alpha1.Workflow) reconcile.Result {
+func (r *Reconciler) processRunningWorkflow(_ context.Context, stored *v1alpha1.Workflow) reconcile.Result { //nolint:unparam // This is the way controller runtime works.
 	// Check for global timeout expiration
 	if r.nowFunc().After(stored.GetStartTime().Add(time.Duration(stored.Status.GlobalTimeout) * time.Second)) {
 		stored.Status.State = v1alpha1.WorkflowStateTimeout

internal/workflow/internal/reconcile_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/rs/zerolog"
 	tinkv1 "github.com/tinkerbell/tink/api/v1alpha2"
 	"github.com/tinkerbell/tink/internal/ptr"
-	. "github.com/tinkerbell/tink/internal/workflow/internal"
+	. "github.com/tinkerbell/tink/internal/workflow/internal" //nolint:revive // Dot imports should not be used. Problem for another time though.
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"