Lint gocritic

.golangci.yml

@@ -5,6 +5,7 @@ linters:
   - dupl
   - errcheck
   - goconst
+  - gocritic
   - gocyclo
   - gofmt
   - goimports

cmd/manager/main.go

@@ -201,7 +201,7 @@ func webhookPath(prefix string, obj runtime.Object, mgr manager.Manager) (string
 
 	// if the strategy to generate this path changes we should update init code and webhook setup
 	// right now this is in sync how controller-runtime generates these paths
-	return prefix + "-" + strings.Replace(gvk.Group, ".", "-", -1) + "-" +
+	return prefix + "-" + strings.ReplaceAll(gvk.Group, ".", "-") + "-" +
 		gvk.Version + "-" + strings.ToLower(gvk.Kind), nil
 }
 

config/crds/kudo.dev_instances.yaml

@@ -122,7 +122,7 @@ spec:
               type: array
             planStatus:
               additionalProperties:
-                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                       | Never executed |                               |                               v |    Error    |<------>|    Pending     |       ^                       |       |                       v       |               +-------+--------+       |               +-------+--------+       |                       |       v                       v | Fatal error |        |    Complete    |"
+                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                        | Never executed |                                |                                v |    Error    |<------>|    Pending     |        ^                       |        |                       v        |               +-------+--------+        |               +-------+--------+        |                       |        v                       v | Fatal error |        |    Complete    |"
                 properties:
                   lastUpdatedTimestamp:
                     format: date-time

pkg/apis/kudo/v1beta1/instance_types.go

@@ -62,24 +62,24 @@ type InstanceStatus struct {
 //
 // These are valid states and transitions
 //
-//                       +----------------+
-//                       | Never executed |
-//                       +-------+--------+
-//                               |
-//                               v
-//+-------------+        +-------+--------+
-//|    Error    |<------>|    Pending     |
-//+------+------+        +-------+--------+
-//       ^                       |
-//       |                       v
-//       |               +-------+--------+
-//       +-------------->|  In progress   |
-//       |               +-------+--------+
-//       |                       |
-//       v                       v
-//+------+------+        +-------+--------+
-//| Fatal error |        |    Complete    |
-//+-------------+        +----------------+
+//                        +----------------+
+//                        | Never executed |
+//                        +-------+--------+
+//                                |
+//                                v
+// +-------------+        +-------+--------+
+// |    Error    |<------>|    Pending     |
+// +------+------+        +-------+--------+
+//        ^                       |
+//        |                       v
+//        |               +-------+--------+
+//        +-------------->|  In progress   |
+//        |               +-------+--------+
+//        |                       |
+//        v                       v
+// +------+------+        +-------+--------+
+// | Fatal error |        |    Complete    |
+// +-------------+        +----------------+
 //
 type PlanStatus struct {
 	Name    string          `json:"name,omitempty"`

pkg/engine/task/podexec/pod_exec.go

@@ -195,7 +195,7 @@ func untarFile(fs afero.Fs, r io.Reader, fileName string) error {
 				return err
 			}
 
-			written = written + w
+			written += w
 			if written > writtenLimit {
 				return errors.New("untar aborted because archive exceeds 4GiB")
 			}

pkg/engine/task/task.go

@@ -152,7 +152,7 @@ func newToggle(task *kudoapi.Task) (Tasker, error) {
 }
 
 var (
-	pipeFileKeyRe = regexp.MustCompile(`^[a-zA-Z0-9_\-]+$`) //a-z, A-Z, 0-9, _ and - are allowed
+	pipeFileKeyRe = regexp.MustCompile(`^[a-zA-Z0-9_\-]+$`) // a-z, A-Z, 0-9, _ and - are allowed
 )
 
 func validPipeFile(pf PipeFile) error {

pkg/engine/workflow/engine.go

@@ -90,12 +90,13 @@ func Execute(pl *ActivePlan, em *engine.Metadata, c client.Client, di discovery.
 		}
 
 		// Check current phase status: skip if finished, proceed if in progress, break out if a fatal error has occurred
-		if phaseStatus.Status.IsFinished() {
-			phasesLeft = phasesLeft - 1
+		switch {
+		case phaseStatus.Status.IsFinished():
+			phasesLeft--
 			continue
-		} else if phaseStatus.Status.IsRunning() {
+		case phaseStatus.Status.IsRunning():
 			phaseStatus.Set(kudoapi.ExecutionInProgress)
-		} else {
+		default:
 			break
 		}
 
@@ -115,13 +116,13 @@ func Execute(pl *ActivePlan, em *engine.Metadata, c client.Client, di discovery.
 			}
 
 			// Check current phase status: skip if finished, proceed if in progress, break out if a fatal error has occurred
-			if stepStatus.Status.IsFinished() {
+			switch {
+			case stepStatus.Status.IsFinished():
 				delete(stepsLeft, stepStatus.Name)
 				continue
-			} else if stepStatus.Status.IsRunning() {
+			case stepStatus.Status.IsRunning():
 				stepStatus.Set(kudoapi.ExecutionInProgress)
-			} else {
-				// we are not in progress and not finished. An unexpected error occurred so that we can not proceed to the next phase
+			default:
 				break
 			}
 
@@ -221,7 +222,7 @@ func Execute(pl *ActivePlan, em *engine.Metadata, c client.Client, di discovery.
 			}
 		} else {
 			phaseStatus.Set(kudoapi.ExecutionComplete)
-			phasesLeft = phasesLeft - 1
+			phasesLeft--
 		}
 	}
 

pkg/kubernetes/status/health.go

@@ -67,8 +67,7 @@ func IsTerminallyFailed(obj runtime.Object) (terminal bool, msg string, err erro
 		return true, "", nil
 	}
 
-	switch obj := obj.(type) {
-	case *batchv1.Job:
+	if obj, ok := obj.(*batchv1.Job); ok {
 		return isJobTerminallyFailed(obj)
 	}
 	return false, "", nil

pkg/kudoctl/clog/log.go

@@ -40,7 +40,7 @@ func (l *Level) Get() interface{} {
 	return *l
 }
 
-//// Required for pflag.Value defined: https://godoc.org/github.com/spf13/pflag#Value
+// Required for pflag.Value defined: https://godoc.org/github.com/spf13/pflag#Value
 
 // String is part of the flag.Value interface.
 func (l *Level) String() string {
@@ -62,7 +62,7 @@ func (l *Level) Type() string {
 	return fmt.Sprint(*l)
 }
 
-//// pflag.Value interface ends
+// pflag.Value interface ends
 
 type loggingT struct {
 	verbosity Level // V logging level, the value of the -v flag/

pkg/kudoctl/clog/log_test.go

@@ -33,7 +33,7 @@ func TestLevelCheck(t *testing.T) {
 
 func TestDefaultPrintLevel(t *testing.T) {
 
-	//default is verbosity of 0 -v=0 or not supplied
+	// default is verbosity of 0 -v=0 or not supplied
 	var buf bytes.Buffer
 
 	logging.out = &buf
@@ -53,7 +53,7 @@ func TestDefaultPrintLevel(t *testing.T) {
 }
 
 func TestErrorf(t *testing.T) {
-	//default is verbosity of 0 -v=0 or not supplied
+	// default is verbosity of 0 -v=0 or not supplied
 	var buf bytes.Buffer
 
 	logging.verbosity = Level(0)

pkg/kudoctl/cmd/params/parser.go

@@ -119,13 +119,15 @@ func mergeParams(paramsFromCmdline map[string]string, paramsFromFiles map[string
 // detailed error message.
 func parseParameter(raw string) (key string, param string, err *string) {
 
+	// TODO (kensipe): this function and calling code should be refactored to NOT use `err` for strings.
 	var errMsg string
 	s := strings.SplitN(raw, "=", 2)
-	if len(s) < 2 {
+	switch {
+	case len(s) < 2:
 		errMsg = fmt.Sprintf("parameter not set: %+v", raw)
-	} else if s[0] == "" {
+	case s[0] == "":
 		errMsg = fmt.Sprintf("parameter name can not be empty: %+v", raw)
-	} else if s[1] == "" {
+	case s[1] == "":
 		errMsg = fmt.Sprintf("parameter value can not be empty: %+v", raw)
 	}
 	if errMsg != "" {

pkg/kudoctl/cmd/params/parser_test.go

@@ -146,10 +146,8 @@ func TestGetParameterMap(t *testing.T) {
 			if len(test.expectedError) == 0 {
 				assert.NoError(t, err)
 				assert.Equal(t, test.expected, params)
-			} else {
-				if assert.Error(t, err) {
-					assert.Equal(t, test.expectedError, err.Error())
-				}
+			} else if assert.Error(t, err) {
+				assert.Equal(t, test.expectedError, err.Error())
 			}
 		})
 	}

pkg/kudoctl/cmd/plan.go

@@ -54,7 +54,7 @@ func NewPlanHistoryCmd() *cobra.Command {
 	return cmd
 }
 
-//NewPlanStatusCmd creates a new command that shows the status of an instance by looking at its current plan
+// NewPlanStatusCmd creates a new command that shows the status of an instance by looking at its current plan
 func NewPlanStatusCmd(out io.Writer) *cobra.Command {
 	options := &plan.StatusOptions{Out: out}
 	cmd := &cobra.Command{

pkg/kudoctl/cmd/plan/plan_history.go

@@ -63,12 +63,13 @@ func planHistory(options *Options, settings *env.Settings) error {
 		plan := instance.Status.PlanStatus[p]
 		msg := "never run" // this is for the cases when status was not yet populated
 
-		if plan.LastUpdatedTimestamp != nil && !plan.LastUpdatedTimestamp.IsZero() { // plan already finished
+		switch {
+		case plan.LastUpdatedTimestamp != nil && !plan.LastUpdatedTimestamp.IsZero():
 			t := plan.LastUpdatedTimestamp.Format(timeLayout)
 			msg = fmt.Sprintf("last finished run at %s (%s)", t, string(plan.Status))
-		} else if plan.Status.IsRunning() {
+		case plan.Status.IsRunning():
 			msg = "is running"
-		} else if plan.Status != "" {
+		case plan.Status != "":
 			msg = string(plan.Status)
 		}
 		historyDisplay := fmt.Sprintf("%s (%s)", plan.Name, msg)

pkg/kudoctl/cmd/prompt/operator.go

@@ -182,7 +182,7 @@ func ForParameter(planNames []string, paramNameList []string) (*packages.Paramet
 		parameter.Required = &t
 	}
 
-	//PlanNameList
+	// PlanNameList
 	if Confirm("Add Trigger Plan (defaults to deploy)") {
 		var trigger string
 		if len(planNames) == 0 {

pkg/kudoctl/cmd/test.go

@@ -131,9 +131,9 @@ For more detailed documentation, visit: https://kudo.dev/docs/testing`,
 				return errors.New("only one of --start-control-plane and --start-kind can be set")
 			}
 
-			//if isSet(flags, "start-kudo") {
-			//	//TODO (kensipe): switch to a new way to start kudo (outside of kuttl)
-			//}
+			// if isSet(flags, "start-kudo") {
+			// TODO (kensipe): switch to a new way to start kudo (outside of kuttl)
+			// }
 
 			if isSet(flags, "skip-delete") {
 				options.SkipDelete = skipDelete

pkg/kudoctl/cmd/testdata/deploy-kudo-ns.yaml.golden

@@ -435,7 +435,7 @@ spec:
               type: array
             planStatus:
               additionalProperties:
-                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                       | Never executed |                               |                               v |    Error    |<------>|    Pending     |       ^                       |       |                       v       |               +-------+--------+       |               +-------+--------+       |                       |       v                       v | Fatal error |        |    Complete    |"
+                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                        | Never executed |                                |                                v |    Error    |<------>|    Pending     |        ^                       |        |                       v        |               +-------+--------+        |               +-------+--------+        |                       |        v                       v | Fatal error |        |    Complete    |"
                 properties:
                   lastUpdatedTimestamp:
                     format: date-time

pkg/kudoctl/cmd/testdata/deploy-kudo-sa.yaml.golden

@@ -435,7 +435,7 @@ spec:
               type: array
             planStatus:
               additionalProperties:
-                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                       | Never executed |                               |                               v |    Error    |<------>|    Pending     |       ^                       |       |                       v       |               +-------+--------+       |               +-------+--------+       |                       |       v                       v | Fatal error |        |    Complete    |"
+                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                        | Never executed |                                |                                v |    Error    |<------>|    Pending     |        ^                       |        |                       v        |               +-------+--------+        |               +-------+--------+        |                       |        v                       v | Fatal error |        |    Complete    |"
                 properties:
                   lastUpdatedTimestamp:
                     format: date-time

pkg/kudoctl/cmd/testdata/deploy-kudo.json.golden

@@ -598,7 +598,7 @@
                   "description": "slice would be enough here but we cannot use slice because order of sequence in yaml is considered significant while here it's not",
                   "type": "object",
                   "additionalProperties": {
-                    "description": "PlanStatus is representing status of a plan \n These are valid states and transitions \n                       | Never executed |                               |                               v |    Error    |\u003c------\u003e|    Pending     |       ^                       |       |                       v       |               +-------+--------+       |               +-------+--------+       |                       |       v                       v | Fatal error |        |    Complete    |",
+                    "description": "PlanStatus is representing status of a plan \n These are valid states and transitions \n                        | Never executed |                                |                                v |    Error    |\u003c------\u003e|    Pending     |        ^                       |        |                       v        |               +-------+--------+        |               +-------+--------+        |                       |        v                       v | Fatal error |        |    Complete    |",
                     "type": "object",
                     "properties": {
                       "lastUpdatedTimestamp": {

pkg/kudoctl/cmd/testdata/deploy-kudo.yaml.golden

@@ -435,7 +435,7 @@ spec:
               type: array
             planStatus:
               additionalProperties:
-                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                       | Never executed |                               |                               v |    Error    |<------>|    Pending     |       ^                       |       |                       v       |               +-------+--------+       |               +-------+--------+       |                       |       v                       v | Fatal error |        |    Complete    |"
+                description: "PlanStatus is representing status of a plan \n These are valid states and transitions \n                        | Never executed |                                |                                v |    Error    |<------>|    Pending     |        ^                       |        |                       v        |               +-------+--------+        |               +-------+--------+        |                       |        v                       v | Fatal error |        |    Complete    |"
                 properties:
                   lastUpdatedTimestamp:
                     format: date-time

pkg/kudoctl/cmd/update_test.go

@@ -78,14 +78,14 @@ func TestUpdate(t *testing.T) {
 		}
 
 		err := update(testInstance.Name, c, &updateOptions{Parameters: tt.parameters}, env.DefaultSettings)
-		if err != nil {
+		switch {
+		case err != nil:
 			if !strings.Contains(err.Error(), tt.errMessageContains) {
 				t.Errorf("%s: expected error '%s' but got '%v'", tt.name, tt.errMessageContains, err)
 			}
-		} else if tt.errMessageContains != "" {
+		case tt.errMessageContains != "":
 			t.Errorf("%s: expected error '%s' but got nil", tt.name, tt.errMessageContains)
-		} else {
-			// the upgrade should have passed without error
+		default:
 			instance, err := c.GetInstance(testInstance.Name, installNamespace)
 			if err != nil {
 				t.Errorf("%s: error when getting instance to verify the test: %v", tt.name, err)