Skip to content

Commit

Permalink
update and consolidate golangci-lint config
Browse files Browse the repository at this point in the history
Prior to this commit golangci-lint was installed by task and run in buildkite,
installed by an action and run in github actions, and there existed two `.golangci.yml` files.

This scattered configuration was frustrating to deal with and resulted in
linting rules being applied inconsistently (aside from the golangci's own
inconsistency).

This commit collapses the configurations down to a single file in
`src/go/k8s/.golangci.yml`, updates the configurations to fix some
deprecations, removes the github action for linting in favor of builtkite,
installed golangci-lint via nix, and fixes pre-existing errors discovered by
fixing the configuration spaghetti.
  • Loading branch information
chrisseto committed Jun 25, 2024
1 parent ff0c0c1 commit e2560d7
Show file tree
Hide file tree
Showing 11 changed files with 22 additions and 230 deletions.
56 changes: 0 additions & 56 deletions .github/workflows/lint-golang.yml

This file was deleted.

127 changes: 0 additions & 127 deletions .golangci.yml

This file was deleted.

4 changes: 2 additions & 2 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@
pkgs.gawk # GNU awk, used by some build scripts.
pkgs.gnused # Stream Editor, used by some build scripts.
pkgs.go-task
pkgs.go_1_21
pkgs.go_1_22
pkgs.golangci-lint
pkgs.openssl
pkgs.setup-envtest # Kubernetes provided test utilities
# TODO(chrisseto): Migrate taskfile to using dependencies from
# this flake.
# pkgs.goreleaser
# pkgs.actionlint # Github Workflow definition linter https://github.com/rhysd/actionlint
# pkgs.gofumpt
# pkgs.golangci-lint
# pkgs.goreleaser
# pkgs.gotools
# pkgs.kind
Expand Down
22 changes: 8 additions & 14 deletions src/go/k8s/.golangci.yml
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
run:
skip-files:
- internal/util/pod/pod.go
go: '1.22'
linters-settings:
# If gofumpt is run outside a module, it assumes Go 1.0 rather than the
# latest Go. We always want the latest formatting.
#
# https://github.com/mvdan/gofumpt/issues/137
gofumpt:
lang-version: "1.21"
dupl:
threshold: 100
funlen:
lines: 100
statements: 50
gci:
local-prefixes: github.com/redpanda-data/redpanda-operator/src/go/k8s
sections:
- standard
- default
- prefix(github.com/redpanda-data/redpanda-operator/src/go/k8s)
skip-generated: true
goconst:
min-len: 2
min-occurrences: 2
Expand All @@ -23,11 +20,6 @@ linters-settings:
- opinionated
- performance
- style
gomnd:
settings:
mnd:
# don't include the "operation" and "assign"
checks: argument,case,condition,return
govet:
check-shadowing: true
maligned:
Expand Down Expand Up @@ -97,6 +89,8 @@ linters:
# - wsl

issues:
exclude-files:
- internal/util/pod/pod.go
exclude:
# errcheck: Almost all programs ignore errors on these functions and in most cases it's ok
- Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked
Expand Down
4 changes: 2 additions & 2 deletions src/go/k8s/cmd/configurator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func registerAdvertisedKafkaAPI(
return nil
}

if len(c.subdomain) > 0 {
if c.subdomain != "" {
data := utils.NewEndpointTemplateData(int(index), c.hostIP)
ep, err := utils.ComputeEndpoint(c.externalConnectivityKafkaEndpointTemplate, data)
if err != nil {
Expand Down Expand Up @@ -374,7 +374,7 @@ func registerAdvertisedPandaproxyAPI(
}

// Pandaproxy uses the Kafka API subdomain.
if len(c.subdomain) > 0 {
if c.subdomain != "" {
data := utils.NewEndpointTemplateData(int(index), c.hostIP)
ep, err := utils.ComputeEndpoint(c.externalConnectivityPandaProxyEndpointTemplate, data)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions src/go/k8s/internal/controller/redpanda/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ func (r *ClusterReconciler) createExternalNodesList(
}
}

if externalKafkaListener != nil && len(externalKafkaListener.External.Subdomain) > 0 {
if externalKafkaListener != nil && externalKafkaListener.External.Subdomain != "" {
address, err := subdomainAddress(externalKafkaListener.External.EndpointTemplate, &pod, externalKafkaListener.External.Subdomain, getNodePort(&nodePortSvc, resources.ExternalListenerName))
if err != nil {
return nil, err
Expand All @@ -707,7 +707,7 @@ func (r *ClusterReconciler) createExternalNodesList(
))
}

if externalAdminListener != nil && len(externalAdminListener.External.Subdomain) > 0 {
if externalAdminListener != nil && externalAdminListener.External.Subdomain != "" {
address, err := subdomainAddress(externalAdminListener.External.EndpointTemplate, &pod, externalAdminListener.External.Subdomain, getNodePort(&nodePortSvc, resources.AdminPortExternalName))
if err != nil {
return nil, err
Expand All @@ -721,7 +721,7 @@ func (r *ClusterReconciler) createExternalNodesList(
))
}

if externalProxyListener != nil && len(externalProxyListener.External.Subdomain) > 0 {
if externalProxyListener != nil && externalProxyListener.External.Subdomain != "" {
address, err := subdomainAddress(externalProxyListener.External.EndpointTemplate, &pod, externalProxyListener.External.Subdomain, getNodePort(&nodePortSvc, resources.PandaproxyPortExternalName))
if err != nil {
return nil, err
Expand All @@ -744,7 +744,7 @@ func (r *ClusterReconciler) createExternalNodesList(
}
}

if schemaRegistryConf != nil && schemaRegistryConf.External != nil && len(schemaRegistryConf.External.Subdomain) > 0 {
if schemaRegistryConf != nil && schemaRegistryConf.External != nil && schemaRegistryConf.External.Subdomain != "" {
prefix := ""
if schemaRegistryConf.External.Endpoint != "" {
prefix = fmt.Sprintf("%s.", schemaRegistryConf.External.Endpoint)
Expand All @@ -756,7 +756,7 @@ func (r *ClusterReconciler) createExternalNodesList(
)
}

if externalProxyListener != nil && len(externalProxyListener.External.Subdomain) > 0 {
if externalProxyListener != nil && externalProxyListener.External.Subdomain != "" {
result.PandaproxyIngress = &externalProxyListener.External.Subdomain
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ var _ = Describe("RedPandaCluster controller", func() {
len(rc.Status.Nodes.ExternalAdmin) == 1 &&
len(rc.Status.Nodes.ExternalPandaproxy) == 1 &&
len(rc.Status.Nodes.SchemaRegistry.ExternalNodeIPs) == 1 &&
len(rc.Status.Nodes.SchemaRegistry.Internal) > 0 &&
rc.Status.Nodes.SchemaRegistry.Internal != "" &&
rc.Status.Nodes.SchemaRegistry.External == "" // Without subdomain the external address is empty
}, timeout, interval).Should(BeTrue())
})
Expand Down Expand Up @@ -449,7 +449,7 @@ var _ = Describe("RedPandaCluster controller", func() {
err := k8sClient.Get(context.Background(), key, &cluster)
return err == nil &&
cluster.Status.Nodes.SchemaRegistry != nil &&
len(cluster.Status.Nodes.SchemaRegistry.Internal) > 0 &&
cluster.Status.Nodes.SchemaRegistry.Internal != "" &&
len(cluster.Status.Nodes.Internal) > 0
}, timeout, interval).Should(BeTrue())
})
Expand Down
2 changes: 1 addition & 1 deletion src/go/k8s/pkg/resources/configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func init() {
tag := cfg.Field(i).Tag
yamlTag := tag.Get("yaml")
parts := strings.Split(yamlTag, ",")
if len(parts) > 0 && len(parts[0]) > 0 {
if len(parts) > 0 && parts[0] != "" {
knownNodeProperties[fmt.Sprintf("redpanda.%s", parts[0])] = true
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/go/k8s/pkg/resources/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func preparePVCResource(
pvc.Spec.Resources.Requests[corev1.ResourceStorage] = storage.Capacity
}

if len(storage.StorageClassName) > 0 {
if storage.StorageClassName != "" {
pvc.Spec.StorageClassName = &storage.StorageClassName
}
return pvc
Expand Down
16 changes: 0 additions & 16 deletions taskfiles/dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@ version: '3'

vars:
TASK_VERSION: '3.19.1'
GOLANG_VERSION: '1.21.3'
KUBECTL_VERSION: '1.28.2'
KUBECTL_INSTALL_DIR: '{{.BUILD_ROOT}}/tools/kubectl/{{.KUBECTL_VERSION}}'
GOLANGCI_LINT_VERSION: '1.55.2'
GOLANGCI_LINT_INSTALL_DIR: '{{.BUILD_ROOT}}/tools/golangci-lint/{{.GOLANGCI_LINT_VERSION}}'
HELM_VERSION: '3.6.3'
HELM_INSTALL_DIR: '{{.BUILD_ROOT}}/tools/helm/{{.HELM_VERSION}}'
KIND_VERSION: '0.20.0'
Expand Down Expand Up @@ -87,19 +84,6 @@ tasks:
status:
- test -f '{{.KIND_INSTALL_DIR}}/bin/kind'

install-golangci-lint:
desc: install golangci-lint
vars:
GOLANGCI_LINT_URL_DEFAULT: 'https://github.com/golangci/golangci-lint/releases/download/v{{.GOLANGCI_LINT_VERSION}}/golangci-lint-{{.GOLANGCI_LINT_VERSION}}-{{OS}}-{{ARCH}}.tar.gz'
GOLANGCI_LINT_URL: '{{default .GOLANGCI_LINT_URL_DEFAULT .GOLANGCI_LINT_URL}}'
cmds:
- mkdir -p '{{.GOLANGCI_LINT_INSTALL_DIR}}/bin'
- curl -sSLf --retry 3 --retry-connrefused --retry-delay 2 --retry-all-errors -o '{{.GOLANGCI_LINT_INSTALL_DIR}}/bin/golangci-lint-{{.GOLANGCI_LINT_VERSION}}-{{OS}}-{{ARCH}}.tar.gz' '{{.GOLANGCI_LINT_URL}}'
- tar -xz -C '{{.GOLANGCI_LINT_INSTALL_DIR}}/bin' -f '{{.GOLANGCI_LINT_INSTALL_DIR}}/bin/golangci-lint-{{.GOLANGCI_LINT_VERSION}}-{{OS}}-{{ARCH}}.tar.gz' --strip 1 --wildcards '*/golangci-lint'
- rm '{{.GOLANGCI_LINT_INSTALL_DIR}}/bin/golangci-lint-{{.GOLANGCI_LINT_VERSION}}-{{OS}}-{{ARCH}}.tar.gz'
status:
- test -f '{{.GOLANGCI_LINT_INSTALL_DIR}}/bin/golangci-lint'

install-docker-tag-list:
desc: install docker-tag-list
cmds:
Expand Down
5 changes: 1 addition & 4 deletions taskfiles/k8s.yml
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,10 @@ tasks:

run-golangci-lint:
desc: run golangci-lint
deps:
- task: :dev:install-golangci-lint
dir: 'src/go/k8s'
cmds:
- |
PATH="{{.GOLANGCI_LINT_INSTALL_DIR}}/bin:$PATH"
golangci-lint run --timeout 8m --go {{.GOLANG_VERSION}} -v
golangci-lint run --timeout 8m -v
build-tag-and-push-images:
desc: builds, tags and pushes images to dockerhub
Expand Down

0 comments on commit e2560d7

Please sign in to comment.