Merge pull request #4412 from r-vasquez/enable-golangci-lint-v2

rpk: enable linters Part 2.

src/go/k8s/cmd/configurator/main.go

@@ -129,7 +129,7 @@ func main() {
 		}
 	}
 
-	cfg.Redpanda.Id = int(hostIndex)
+	cfg.Redpanda.ID = int(hostIndex)
 
 	// First Redpanda node need to have cleared seed servers in order
 	// to form raft group 0
@@ -154,12 +154,12 @@ func main() {
 var errInternalPortMissing = errors.New("port configration is missing internal port")
 
 func getInternalKafkaAPIPort(cfg *config.Config) (int, error) {
-	for _, l := range cfg.Redpanda.KafkaApi {
+	for _, l := range cfg.Redpanda.KafkaAPI {
 		if l.Name == "kafka" {
 			return l.Port, nil
 		}
 	}
-	return 0, fmt.Errorf("%w %v", errInternalPortMissing, cfg.Redpanda.KafkaApi)
+	return 0, fmt.Errorf("%w %v", errInternalPortMissing, cfg.Redpanda.KafkaAPI)
 }
 
 func getInternalProxyAPIPort(cfg *config.Config) int {
@@ -192,7 +192,7 @@ func getNode(nodeName string) (*corev1.Node, error) {
 func registerAdvertisedKafkaAPI(
 	c *configuratorConfig, cfg *config.Config, index brokerID, kafkaAPIPort int,
 ) error {
-	cfg.Redpanda.AdvertisedKafkaApi = []config.NamedSocketAddress{
+	cfg.Redpanda.AdvertisedKafkaAPI = []config.NamedSocketAddress{
 		{
 			SocketAddress: config.SocketAddress{
 				Address: c.hostName + "." + c.svcFQDN,
@@ -207,7 +207,7 @@ func registerAdvertisedKafkaAPI(
 	}
 
 	if len(c.subdomain) > 0 {
-		cfg.Redpanda.AdvertisedKafkaApi = append(cfg.Redpanda.AdvertisedKafkaApi, config.NamedSocketAddress{
+		cfg.Redpanda.AdvertisedKafkaAPI = append(cfg.Redpanda.AdvertisedKafkaAPI, config.NamedSocketAddress{
 			SocketAddress: config.SocketAddress{
 				Address: fmt.Sprintf("%d.%s", index, c.subdomain),
 				Port:    c.hostPort,
@@ -222,7 +222,7 @@ func registerAdvertisedKafkaAPI(
 		return fmt.Errorf("unable to retrieve node: %w", err)
 	}
 
-	cfg.Redpanda.AdvertisedKafkaApi = append(cfg.Redpanda.AdvertisedKafkaApi, config.NamedSocketAddress{
+	cfg.Redpanda.AdvertisedKafkaAPI = append(cfg.Redpanda.AdvertisedKafkaAPI, config.NamedSocketAddress{
 		SocketAddress: config.SocketAddress{
 			Address: networking.GetPreferredAddress(node, c.externalConnectivityAddressType),
 			Port:    c.hostPort,

src/go/k8s/controllers/redpanda/cluster_controller_configuration.go

@@ -361,7 +361,7 @@ func mapStatusToCondition(
 				Type:    redpandav1alpha1.ClusterConfiguredConditionType,
 				Status:  corev1.ConditionFalse,
 				Reason:  redpandav1alpha1.ClusterConfiguredReasonUpdating,
-				Message: fmt.Sprintf("Node %d needs restart", nodeStatus.NodeId),
+				Message: fmt.Sprintf("Node %d needs restart", nodeStatus.NodeID),
 			}
 		} else if configVersion != 0 && nodeStatus.ConfigVersion != configVersion {
 			condition = &redpandav1alpha1.ClusterCondition{
@@ -399,7 +399,7 @@ func needsRestart(clusterStatus admin.ConfigStatusResponse) bool {
 func tryMapErrorToCondition(
 	err error,
 ) (*redpandav1alpha1.ClusterCondition, error) {
-	var httpErr *admin.HttpError
+	var httpErr *admin.HTTPError
 	if errors.As(err, &httpErr) {
 		if httpErr.Response != nil && httpErr.Response.StatusCode == http.StatusBadRequest {
 			return &redpandav1alpha1.ClusterCondition{

src/go/k8s/controllers/redpanda/suite_test.go

@@ -235,9 +235,9 @@ func (m *mockAdminAPI) PatchClusterConfig(
 	}
 	invalidRequest := len(newInvalid)+len(newUnknown) > 0
 	if m.directValidation && invalidRequest {
-		return admin.ClusterConfigWriteResult{}, &admin.HttpError{
+		return admin.ClusterConfigWriteResult{}, &admin.HTTPError{
 			Method: http.MethodPut,
-			Url:    "/v1/cluster_config",
+			URL:    "/v1/cluster_config",
 			Response: &http.Response{
 				Status:     "Bad Request",
 				StatusCode: 400,

src/go/k8s/pkg/resources/configmap.go

@@ -237,8 +237,8 @@ func (r *ConfigMapResource) CreateConfiguration(
 	cr := &cfg.NodeConfiguration.Redpanda
 
 	internalListener := r.pandaCluster.InternalListener()
-	cr.KafkaApi = []config.NamedSocketAddress{} // we don't want to inherit default kafka port
-	cr.KafkaApi = append(cr.KafkaApi, config.NamedSocketAddress{
+	cr.KafkaAPI = []config.NamedSocketAddress{} // we don't want to inherit default kafka port
+	cr.KafkaAPI = append(cr.KafkaAPI, config.NamedSocketAddress{
 		SocketAddress: config.SocketAddress{
 			Address: "0.0.0.0",
 			Port:    internalListener.Port,
@@ -247,7 +247,7 @@ func (r *ConfigMapResource) CreateConfiguration(
 	})
 
 	if r.pandaCluster.ExternalListener() != nil {
-		cr.KafkaApi = append(cr.KafkaApi, config.NamedSocketAddress{
+		cr.KafkaAPI = append(cr.KafkaAPI, config.NamedSocketAddress{
 			SocketAddress: config.SocketAddress{
 				Address: "0.0.0.0",
 				Port:    calculateExternalPort(internalListener.Port, r.pandaCluster.ExternalListener().Port),
@@ -262,17 +262,17 @@ func (r *ConfigMapResource) CreateConfiguration(
 		Port:    clusterCRPortOrRPKDefault(c.RPCServer.Port, cr.RPCServer.Port),
 	}
 
-	cr.AdminApi[0].Port = clusterCRPortOrRPKDefault(r.pandaCluster.AdminAPIInternal().Port, cr.AdminApi[0].Port)
-	cr.AdminApi[0].Name = AdminPortName
+	cr.AdminAPI[0].Port = clusterCRPortOrRPKDefault(r.pandaCluster.AdminAPIInternal().Port, cr.AdminAPI[0].Port)
+	cr.AdminAPI[0].Name = AdminPortName
 	if r.pandaCluster.AdminAPIExternal() != nil {
 		externalAdminAPI := config.NamedSocketAddress{
 			SocketAddress: config.SocketAddress{
-				Address: cr.AdminApi[0].Address,
-				Port:    cr.AdminApi[0].Port + 1,
+				Address: cr.AdminAPI[0].Address,
+				Port:    cr.AdminAPI[0].Port + 1,
 			},
 			Name: AdminPortExternalName,
 		}
-		cr.AdminApi = append(cr.AdminApi, externalAdminAPI)
+		cr.AdminAPI = append(cr.AdminAPI, externalAdminAPI)
 	}
 
 	cr.DeveloperMode = c.DeveloperMode
@@ -295,7 +295,7 @@ func (r *ConfigMapResource) CreateConfiguration(
 		if tlsListener.TLS.RequireClientAuth {
 			tls.TruststoreFile = fmt.Sprintf("%s/%s", tlsKafkaAPIDirCA, cmetav1.TLSCAKey)
 		}
-		cr.KafkaApiTLS = []config.ServerTLS{
+		cr.KafkaAPITLS = []config.ServerTLS{
 			tls,
 		}
 	}
@@ -317,7 +317,7 @@ func (r *ConfigMapResource) CreateConfiguration(
 		if adminAPITLSListener.TLS.RequireClientAuth {
 			adminTLS.TruststoreFile = fmt.Sprintf("%s/%s", tlsAdminAPIDirCA, cmetav1.TLSCAKey)
 		}
-		cr.AdminApiTLS = append(cr.AdminApiTLS, adminTLS)
+		cr.AdminAPITLS = append(cr.AdminAPITLS, adminTLS)
 	}
 
 	if r.pandaCluster.Spec.CloudStorage.Enabled {

src/go/k8s/pkg/resources/configuration/configuration_test.go

@@ -47,20 +47,20 @@ func TestFlatProperties(t *testing.T) {
 	err := config.SetAdditionalFlatProperties(map[string]string{"redpanda.a": "b", "node_uuid": "uuid"})
 	require.NoError(t, err)
 	assert.Equal(t, "b", config.ClusterConfiguration["a"])
-	assert.Equal(t, "uuid", config.NodeConfiguration.NodeUuid)
+	assert.Equal(t, "uuid", config.NodeConfiguration.NodeUUID)
 	assert.NotContains(t, config.NodeConfiguration.Redpanda.Other, "a")
 
 	config = configuration.GlobalConfiguration{Mode: configuration.GlobalConfigurationModeClassic}
 	err = config.SetAdditionalFlatProperties(map[string]string{"redpanda.a": "b", "node_uuid": "uuid"})
 	require.NoError(t, err)
-	assert.Equal(t, "uuid", config.NodeConfiguration.NodeUuid)
+	assert.Equal(t, "uuid", config.NodeConfiguration.NodeUUID)
 	assert.Equal(t, "b", config.NodeConfiguration.Redpanda.Other["a"])
 	assert.NotContains(t, config.ClusterConfiguration, "a")
 
 	config = configuration.GlobalConfiguration{Mode: configuration.GlobalConfigurationModeMixed}
 	err = config.SetAdditionalFlatProperties(map[string]string{"redpanda.a": "b", "node_uuid": "uuid"})
 	require.NoError(t, err)
-	assert.Equal(t, "uuid", config.NodeConfiguration.NodeUuid)
+	assert.Equal(t, "uuid", config.NodeConfiguration.NodeUUID)
 	assert.Equal(t, "b", config.NodeConfiguration.Redpanda.Other["a"])
 	assert.Equal(t, "b", config.ClusterConfiguration["a"])
 }

src/go/k8s/pkg/resources/configuration/serialization_test.go

@@ -21,7 +21,7 @@ import (
 func TestSerde(t *testing.T) {
 	conf := configuration.GlobalConfiguration{
 		NodeConfiguration: config.Config{
-			NodeUuid: "uuid",
+			NodeUUID: "uuid",
 		},
 		ClusterConfiguration: map[string]interface{}{
 			"a": "b",
@@ -33,7 +33,7 @@ func TestSerde(t *testing.T) {
 	conf2, err := ser.Deserialize(configuration.GlobalConfigurationModeCentralized)
 	require.NoError(t, err)
 	require.NotNil(t, conf2)
-	assert.Equal(t, "uuid", conf2.NodeConfiguration.NodeUuid)
+	assert.Equal(t, "uuid", conf2.NodeConfiguration.NodeUUID)
 	assert.Equal(t, "b", conf2.ClusterConfiguration["a"])
 	ser2, err := conf.Serialize()
 	require.NoError(t, err)

src/go/rpk/.golangci.yml

@@ -1,7 +1,104 @@
 run:
   allow-parallel-runners: true
 
+# golangci-lint by default ignores some staticcheck and vet raised issues that
+# are actually important to catch. The following ensures that we do not ignore
+# those tools ever.
+issues:
+  exclude-use-default: false
+
+# We opt out of all suggested linters and manually pick what we want.
+# Please do not use enable-all.
 linters:
   disable-all: true
   enable:
+  # Enabled by default linters:
+    - deadcode
     - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unused
+    - varcheck
+  # Disabled by default
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - contextcheck
+    - durationcheck
+    - errname
+    - errorlint
+    - exportloopref
+    - godot
+    - goprintffuncname
+    - misspell
+    - nilerr
+    - noctx
+    - nolintlint
+    - revive
+    - tenv
+    - unconvert
+    - wastedassign
+
+linters-settings:
+  # We do not want to enforce every usage of fmt.Errorf to use %w.
+  errorlint:
+    errorf: false
+
+  # If we want to opt out of a lint, we require an explanation.
+  nolintlint:
+    allow-leading-space: true
+    allow-unused: false
+    require-explanation: true
+    require-specific: true
+
+  # Revive is yet another metalinter with a lot of useful lints. 
+  # The below opts in to all the ones we would like to use.
+  revive:
+    ignore-generated-header: true
+    severity: warning
+    confidence: 0.8
+    error-code: 0
+    warning-code: 0
+    rules:
+      - name: atomic
+      - name: blank-imports
+      - name: bool-literal-in-expr
+      - name: call-to-gc
+      - name: confusing-results
+      - name: constant-logical-expr
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: defer
+      - name: dot-imports
+      - name: duplicated-imports
+      - name: early-return
+      - name: error-naming
+      - name: error-return
+      - name: error-strings
+      - name: errorf
+      - name: get-return
+      - name: identical-branches
+      - name: if-return
+      - name: modifies-value-receiver
+      - name: optimize-operands-order
+      - name: package-comments
+      - name: range
+      - name: range-val-in-closure
+      - name: receiver-naming
+      - name: string-of-int
+      - name: struct-tag
+      - name: superfluous-else
+      - name: time-equal
+      - name: time-naming
+      - name: unconditional-recursion
+      - name: unnecessary-stmt
+      - name: unreachable-code
+      - name: unused-parameter
+      - name: unused-receiver
+      - name: useless-break
+      - name: var-declaration
+      - name: var-naming
+      - name: waitgroup-by-value