Enable additional linters (#2318)

* Enable additional linter and formatter

Author: johscheuer

Makefile

@@ -37,9 +37,9 @@ GOLANGCI_LINT_PKG=github.com/golangci/golangci-lint/v2/cmd/[email protected]
 GOLANGCI_LINT=$(GOBIN)/golangci-lint
 GORELEASER_PKG=github.com/goreleaser/goreleaser/[email protected]
 GORELEASER=$(GOBIN)/goreleaser
-GO_LINES_PKG=github.com/segmentio/golines@v0.11.0
+GO_LINES_PKG=github.com/segmentio/golines@v0.12.2
 GO_LINES=$(GOBIN)/golines
-GO_IMPORTS_PKG=golang.org/x/tools/cmd/goimports@v0.20.0
+GO_IMPORTS_PKG=golang.org/x/tools/cmd/goimports@v0.34.0
 GO_IMPORTS=$(GOBIN)/goimports
 
 BUILD_DEPS?=
@@ -93,7 +93,7 @@ clean:
 	find . -name "cover.out" -delete
 
 clean-deps:
-	@rm $(CONTROLLER_GEN) $(KUSTOMIZE) $(GOLANGCI_LINT) $(GORELEASER)
+	@rm $(CONTROLLER_GEN) $(KUSTOMIZE) $(GOLANGCI_LINT) $(GORELEASER) $(GO_LINES) $(GO_IMPORTS)
 
 test_if_changed: cover.out
 
@@ -121,35 +121,34 @@ bin/kubectl-fdb: ${GO_SRC} $(GORELEASER)
 	$(GORELEASER) build --single-target --skip validate --clean
 	@mkdir -p bin
 	@touch $@
-# TODO test here that the tag is set properly
 
 # Run against the configured Kubernetes cluster in ~/.kube/config
 run: generate manifests
 	go run ./main.go
 
 # Install CRDs into a cluster
-install: manifests
-	kustomize build config/crd | kubectl $(KUBECTL_ARGS) apply -f -
+install: $(KUSTOMIZE) manifests
+	$(KUSTOMIZE) build config/crd | kubectl $(KUBECTL_ARGS) apply -f -
 
 # Uninstall CRDs from a cluster
-uninstall: manifests
-	kustomize build config/crd | kubectl $(KUBECTL_ARGS) delete -f -
+uninstall: $(KUSTOMIZE) manifests
+	$(KUSTOMIZE) build config/crd | kubectl $(KUBECTL_ARGS) delete -f -
 
 # Apply config to the local development environment based on environment
 # variables.
-config/development/kustomization.yaml: config/development/kustomization.yaml.sample
+config/development/kustomization.yaml: $(KUSTOMIZE) config/development/kustomization.yaml.sample
 	cp config/development/kustomization.yaml.sample config/development/kustomization.yaml
-	cd config/development && kustomize edit set image foundationdb/fdb-kubernetes-operator=${IMG}
+	cd config/development && $(KUSTOMIZE) edit set image foundationdb/fdb-kubernetes-operator=${IMG}
 ifneq "$(SIDECAR_IMG)" ""
-	cd config/development && kustomize edit set image foundationdb/foundationdb-kubernetes-sidecar=${SIDECAR_IMG}
+	cd config/development && $(KUSTOMIZE) edit set image foundationdb/foundationdb-kubernetes-sidecar=${SIDECAR_IMG}
 endif
 ifeq "$(REMOTE_BUILD)" "1"
-	cd config/development && kustomize edit add patch --path=remote_build.yaml
+	cd config/development && $(KUSTOMIZE) edit add patch --path=remote_build.yaml
 endif
 
 # Deploy controller in the configured Kubernetes cluster in ~/.kube/config
-deploy: install manifests config/development/kustomization.yaml
-	kustomize build config/development | kubectl $(KUBECTL_ARGS) apply -f -
+deploy: $(KUSTOMIZE) install manifests config/development/kustomization.yaml
+	$(KUSTOMIZE) build config/development | kubectl $(KUBECTL_ARGS) apply -f -
 
 # Generate manifests e.g. CRD, RBAC etc.
 manifests: ${MANIFESTS}
@@ -158,22 +157,21 @@ ${MANIFESTS}: ${CONTROLLER_GEN} ${GO_SRC}
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./api/..." paths="./controllers/..." output:crd:artifacts:config=config/crd/bases
 
 # Run go fmt against code
-fmt: bin/fmt_check
+fmt: deps bin/fmt_check
 
-# TODO johscheuer: enable those new command in a new PR.
 bin/fmt_check: ${GO_ALL}
-	# $(GO_LINES) -w .
-	@go fmt $$(go list ./...)
-	#$(GO_IMPORTS) -w $(GO_SRC)
-	#$(GOLANGCI_LINT) run --fix
+	# We make use of gofmt with golines because of: https://github.com/segmentio/golines/issues/155
+	@$(GO_LINES) -w --base-formatter=gofmt --ignored-dirs=./e2e/chaos-mesh --ignore-generated $(GO_SRC)
+	@$(GO_IMPORTS) -w $(GO_SRC)
+	@$(GOLANGCI_LINT) run --fix
 	@mkdir -p bin
 	@touch $@
 
 # Run go vet against code
 vet: bin/vet_check
 
 bin/vet_check: ${GO_ALL}
-	go vet ./...
+	@go vet ./...
 	@mkdir -p bin
 	@touch $@
 
@@ -203,22 +201,22 @@ rebuild-operator: container-build container-push-if-remote deploy bounce
 bounce:
 	kubectl $(KUBECTL_ARGS) delete pod -l app=fdb-kubernetes-operator-controller-manager
 
-samples: kustomize ${SAMPLES}
+samples: $(KUSTOMIZE) ${SAMPLES}
 
-config/samples/deployment.yaml: config/deployment/*.yaml
-	kustomize build ./config/deployment > $@
+config/samples/deployment.yaml: $(KUSTOMIZE) config/deployment/*.yaml
+	$(KUSTOMIZE) build ./config/deployment > $@
 
-config/samples/cluster.yaml: config/tests/base/*.yaml
-	kustomize build ./config/tests/base/ > $@
+config/samples/cluster.yaml: $(KUSTOMIZE) config/tests/base/*.yaml
+	$(KUSTOMIZE) build ./config/tests/base/ > $@
 
-config/samples/backup.yaml: config/tests/backup/base/*.yaml
-	kustomize build ./config/tests/backup/base > $@
+config/samples/backup.yaml: $(KUSTOMIZE) config/tests/backup/base/*.yaml
+	$(KUSTOMIZE) build ./config/tests/backup/base > $@
 
-config/samples/restore.yaml: config/tests/backup/base/*.yaml
-	kustomize build ./config/tests/backup/restore > $@
+config/samples/restore.yaml: $(KUSTOMIZE) config/tests/backup/base/*.yaml
+	$(KUSTOMIZE) build ./config/tests/backup/restore > $@
 
-config/samples/client.yaml: config/tests/client/*.yaml
-	kustomize build ./config/tests/client > $@
+config/samples/client.yaml: $(KUSTOMIZE) config/tests/client/*.yaml
+	$(KUSTOMIZE) build ./config/tests/client > $@
 
 bin/po-docgen: cmd/po-docgen/*.go
 	go build -o bin/po-docgen cmd/po-docgen/main.go  cmd/po-docgen/api.go

api/v1beta2/foundationdb_custom_parameter.go

@@ -82,16 +82,31 @@ func (customParameters FoundationDBCustomParameters) ValidateCustomParameters()
 		}
 
 		if _, ok := protectedParameters[parameterName]; ok {
-			violations = append(violations, fmt.Sprintf("found protected customParameter: %s, please remove this parameter from the customParameters list", parameterName))
+			violations = append(
+				violations,
+				fmt.Sprintf(
+					"found protected customParameter: %s, please remove this parameter from the customParameters list",
+					parameterName,
+				),
+			)
 		}
 
 		if _, ok := fdbMonitorGeneralParameters[parameterName]; ok {
-			violations = append(violations, fmt.Sprintf("found general or fdbmonitor customParameter: %s, please remove this parameter from the customParameters list as they are not supported", parameterName))
+			violations = append(
+				violations,
+				fmt.Sprintf(
+					"found general or fdbmonitor customParameter: %s, please remove this parameter from the customParameters list as they are not supported",
+					parameterName,
+				),
+			)
 		}
 	}
 
 	if len(violations) > 0 {
-		return fmt.Errorf("found the following customParameters violations:\n%s", strings.Join(violations, "\n"))
+		return fmt.Errorf(
+			"found the following customParameters violations:\n%s",
+			strings.Join(violations, "\n"),
+		)
 	}
 
 	return nil

api/v1beta2/foundationdb_custom_parameter_test.go

@@ -67,32 +67,44 @@ var _ = Describe("FoundationDBCustomParameters", func() {
 					"test=test",
 				},
 				nil),
-			Entry("custom parameters that sets protected knob",
+			Entry(
+				"custom parameters that sets protected knob",
 				FoundationDBCustomParameters{
 					"datadir=test",
 				},
-				errors.New("found the following customParameters violations:\nfound protected customParameter: datadir, please remove this parameter from the customParameters list"),
+				errors.New(
+					"found the following customParameters violations:\nfound protected customParameter: datadir, please remove this parameter from the customParameters list",
+				),
 			),
-			Entry("duplicate custom parameters",
+			Entry(
+				"duplicate custom parameters",
 				FoundationDBCustomParameters{
 					"test=test",
 					"test=test",
 				},
-				errors.New("found the following customParameters violations:\nfound duplicated customParameter: test"),
+				errors.New(
+					"found the following customParameters violations:\nfound duplicated customParameter: test",
+				),
 			),
-			Entry("duplicate custom parameters",
+			Entry(
+				"duplicate custom parameters",
 				FoundationDBCustomParameters{
 					"test=test",
 					"datadir=test",
 					"test=test",
 				},
-				errors.New("found the following customParameters violations:\nfound protected customParameter: datadir, please remove this parameter from the customParameters list\nfound duplicated customParameter: test"),
+				errors.New(
+					"found the following customParameters violations:\nfound protected customParameter: datadir, please remove this parameter from the customParameters list\nfound duplicated customParameter: test",
+				),
 			),
-			Entry("custom parameters that sets general knob",
+			Entry(
+				"custom parameters that sets general knob",
 				FoundationDBCustomParameters{
 					"restart-delay",
 				},
-				errors.New("found the following customParameters violations:\nfound general or fdbmonitor customParameter: restart-delay, please remove this parameter from the customParameters list as they are not supported"),
+				errors.New(
+					"found the following customParameters violations:\nfound general or fdbmonitor customParameter: restart-delay, please remove this parameter from the customParameters list as they are not supported",
+				),
 			),
 		)
 	})

api/v1beta2/foundationdb_database_configuration.go

@@ -188,7 +188,9 @@ func (configuration *DatabaseConfiguration) FailOver() DatabaseConfiguration {
 //
 // This will fill in defaults of -1 for some fields that have a default of 0,
 // and will ensure that the region configuration is ordered consistently.
-func (configuration DatabaseConfiguration) NormalizeConfiguration(cluster *FoundationDBCluster) DatabaseConfiguration {
+func (configuration DatabaseConfiguration) NormalizeConfiguration(
+	cluster *FoundationDBCluster,
+) DatabaseConfiguration {
 	result := configuration.DeepCopy()
 
 	if result.RemoteLogs == 0 {
@@ -273,7 +275,10 @@ func (configuration DatabaseConfiguration) getRegion(id string, priority int) Re
 	}
 
 	if len(matchingRegion.DataCenters) == 0 {
-		matchingRegion.DataCenters = append(matchingRegion.DataCenters, DataCenter{ID: id, Priority: priority})
+		matchingRegion.DataCenters = append(
+			matchingRegion.DataCenters,
+			DataCenter{ID: id, Priority: priority},
+		)
 	}
 
 	return matchingRegion
@@ -298,7 +303,9 @@ func (configuration DatabaseConfiguration) getRegion(id string, priority int) Re
 // The default LogRouters value will be equal to 3 times the Logs value when
 // the UsableRegions is greater than 1. It will be equal to -1 when the
 // UsableRegions is less than or equal to 1.
-func (configuration *DatabaseConfiguration) GetRoleCountsWithDefaults(faultTolerance int) RoleCounts {
+func (configuration *DatabaseConfiguration) GetRoleCountsWithDefaults(
+	faultTolerance int,
+) RoleCounts {
 	counts := configuration.RoleCounts.DeepCopy()
 	if counts.Storage == 0 {
 		counts.Storage = 2*faultTolerance + 1
@@ -356,7 +363,9 @@ func (configuration *DatabaseConfiguration) GetRoleCountsWithDefaults(faultToler
 // converge on the final configuration.
 //
 // See: https://apple.github.io/foundationdb/configuration.html#migrating-a-database-to-use-a-region-configuration
-func (configuration DatabaseConfiguration) GetNextConfigurationChange(finalConfiguration DatabaseConfiguration) DatabaseConfiguration {
+func (configuration DatabaseConfiguration) GetNextConfigurationChange(
+	finalConfiguration DatabaseConfiguration,
+) DatabaseConfiguration {
 	if !reflect.DeepEqual(configuration.Regions, finalConfiguration.Regions) {
 		result := configuration.DeepCopy()
 		currentPriorities := configuration.getRegionPriorities()
@@ -368,7 +377,10 @@ func (configuration DatabaseConfiguration) GetNextConfigurationChange(finalConfi
 		for regionIndex, region := range result.Regions {
 			for _, dataCenter := range region.DataCenters {
 				if dataCenter.Satellite == 0 {
-					result.Regions[regionIndex] = finalConfiguration.getRegion(dataCenter.ID, dataCenter.Priority)
+					result.Regions[regionIndex] = finalConfiguration.getRegion(
+						dataCenter.ID,
+						dataCenter.Priority,
+					)
 					break
 				}
 			}
@@ -397,7 +409,10 @@ func (configuration DatabaseConfiguration) GetNextConfigurationChange(finalConfi
 				if len(result.Regions) == 0 {
 					priority = 1
 				}
-				result.Regions = append(result.Regions, finalConfiguration.getRegion(regionToAdd, priority))
+				result.Regions = append(
+					result.Regions,
+					finalConfiguration.getRegion(regionToAdd, priority),
+				)
 				currentPriorities[regionToAdd] = priority
 			}
 		}
@@ -653,9 +668,15 @@ func (configuration DatabaseConfiguration) AreSeparatedProxiesConfigured() bool
 // string "commit_proxies=%d grv_proxies=%d", otherwise just
 // "proxies=%d" using the correct counts of the configuration object.
 func (configuration DatabaseConfiguration) GetProxiesString() string {
-	counts := configuration.GetRoleCountsWithDefaults(DesiredFaultTolerance(configuration.RedundancyMode))
+	counts := configuration.GetRoleCountsWithDefaults(
+		DesiredFaultTolerance(configuration.RedundancyMode),
+	)
 	if configuration.AreSeparatedProxiesConfigured() {
-		return fmt.Sprintf(" commit_proxies=%d grv_proxies=%d", counts.CommitProxies, counts.GrvProxies)
+		return fmt.Sprintf(
+			" commit_proxies=%d grv_proxies=%d",
+			counts.CommitProxies,
+			counts.GrvProxies,
+		)
 	}
 
 	return fmt.Sprintf(" proxies=%d", counts.Proxies)
@@ -757,7 +778,9 @@ func (configuration DatabaseConfiguration) GetConfigurationString() (string, err
 //
 // Deprecated: Use ClearMissingVersionFlags instead on the live configuration
 // instead.
-func (configuration *DatabaseConfiguration) FillInDefaultVersionFlags(liveConfiguration DatabaseConfiguration) {
+func (configuration *DatabaseConfiguration) FillInDefaultVersionFlags(
+	liveConfiguration DatabaseConfiguration,
+) {
 	if configuration.LogSpill == 0 {
 		configuration.LogSpill = liveConfiguration.LogSpill
 	}