Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add golangci-lint #1254

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# Documentation reference https://github.com/golangci/golangci-lint/blob/v1.55.2/.golangci.reference.yml
run:
skip-dirs-use-default: false
modules-download-mode: readonly
allow-parallel-runners: false

output:
format: colored-line-number
print-issued-lines: true
print-linter-name: true
uniq-by-line: true
sort-results: true

linters-settings:
dogsled:
max-blank-identifiers: 2
errcheck:
check-type-assertions: true
check-blank: true
goconst:
min-len: 3
min-occurrences: 5
gofmt:
simplify: true
govet:
enable-all: true
misspell:
locale: US
nakedret:
max-func-lines: 30
nolintlint:
allow-unused: false
allow-no-explanation: []
require-explanation: true
require-specific: true
revive:
enable-all-rules: true
rules:
- name: line-length-limit
arguments: [120]
unparam:
check-exported: true
whitespace:
multi-if: false
multi-func: false

linters:
disable-all: true
enable:
- asasalint
- asciicheck
- bidichk
- bodyclose
- dogsled
- dupword
- durationcheck
- errcheck
- errchkjson
- exportloopref
- ginkgolinter
- goconst
- gofmt
- goheader
- goimports
- goprintffuncname
- gosec
- gosimple
- govet
- importas
- ineffassign
- misspell
- nakedret
- nilerr
- noctx
- nolintlint
- nosprintfhostport
- revive
- staticcheck
- stylecheck
- unconvert
- unparam
- unused
- usestdlibvars
- whitespace
fast: false

issues:
exclude-use-default: false
exclude-rules:
- linters:
- revive
text: "^struct-tag: unknown option 'inline' in JSON tag$"
max-issues-per-linter: 0
max-same-issues: 0

severity:
default-severity: error
case-sensitive: false
45 changes: 37 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -174,18 +174,12 @@ $(ENVTEST): ## Download envtest-setup locally if necessary.
envtest: $(ENVTEST)

.PHONY: test
test: vet envtest ## Run Go linter and unit tests and check Go code format and if api and bundle folders are up to date.
test: envtest ## Run unit tests; run Go linters checks; and check if api and bundle folders are up to date.
KUBEBUILDER_ASSETS="$(ENVTESTPATH)" go test -mod=mod ./controllers/... ./pkg/... -coverprofile cover.out
@make fmt-isupdated
@make lint
@make api-isupdated
@make bundle-isupdated

.PHONY: fmt-isupdated
fmt-isupdated: TEMP:= $(shell mktemp -d)
fmt-isupdated:
@cp -r ./ $(TEMP) && cd $(TEMP) && make fmt && cd - && diff -ruN . $(TEMP)/ && echo "Go code is formatted" || (echo "Go code is not formatted, run 'make fmt' to format" && exit 1)
@chmod -R 777 $(TEMP) && rm -rf $(TEMP)

.PHONY: api-isupdated
api-isupdated: TEMP:= $(shell mktemp -d)
api-isupdated:
Expand Down Expand Up @@ -532,3 +526,38 @@ test-e2e-cleanup: login-required
$(OC_CLI) delete restore -n $(OADP_TEST_NAMESPACE) --all --wait=false
for restore_name in $(shell $(OC_CLI) get restore -n $(OADP_TEST_NAMESPACE) -o name);do $(OC_CLI) patch "$$restore_name" -n $(OADP_TEST_NAMESPACE) -p '{"metadata":{"finalizers":null}}' --type=merge;done
rm -rf $(SETTINGS_TMP)

## Location to install dependencies to
LOCALBIN ?= $(shell pwd)/bin
$(LOCALBIN):
mkdir -p $(LOCALBIN)

GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)
GOLANGCI_LINT_VERSION ?= v1.55.2

.PHONY: golangci-lint
golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary.
$(GOLANGCI_LINT): $(LOCALBIN)
$(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint,${GOLANGCI_LINT_VERSION})

# go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist
# $1 - target path with name of binary with version
# $2 - package url which can be installed
# $3 - specific version of package
define go-install-tool
@[ -f $(1) ] || { \
set -e; \
package=$(2)@$(3) ;\
echo "Downloading $${package}" ;\
GOBIN=$(LOCALBIN) go install $${package} ;\
mv "$$(echo "$(1)" | sed "s/-$(3)$$//")" $(1) ;\
}
endef

.PHONY: lint
lint: golangci-lint ## Run Go linters checks against all project's Go files.
$(GOLANGCI_LINT) run

.PHONY: lint-fix
lint-fix: golangci-lint ## Fix Go linters issues.
$(GOLANGCI_LINT) run --fix
2 changes: 1 addition & 1 deletion controllers/bsl.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (r *DPAReconciler) UpdateCredentialsSecretLabels(secretName string, namespa
return false, err
}

r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "SecretLabelled", fmt.Sprintf("Secret %s has been labelled", secretName))
r.EventRecorder.Event(&secret, corev1.EventTypeNormal, "SecretLabelled", fmt.Sprintf("Secret %s has been labeled", secretName))
}
return true, nil
}
Expand Down
10 changes: 4 additions & 6 deletions controllers/bucket_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ package controllers
import (
"context"
"fmt"
"os"
"strconv"
"time"

"github.com/openshift/oadp-operator/pkg/common"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
"os"
"strconv"
"time"

"github.com/go-logr/logr"
oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
Expand Down Expand Up @@ -159,12 +160,10 @@ func (b BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl

// SetupWithManager sets up the controller with the Manager.
func (b *BucketReconciler) SetupWithManager(mgr ctrl.Manager) error {

return ctrl.NewControllerManagedBy(mgr).
For(&oadpv1alpha1.CloudStorage{}).
WithEventFilter(bucketPredicate()).
Complete(b)

}

func bucketPredicate() predicate.Predicate {
Expand Down Expand Up @@ -216,7 +215,6 @@ func (b *BucketReconciler) WaitForSecret(namespace, name string) (*corev1.Secret
}

err := wait.PollImmediate(5*time.Second, timeout, func() (bool, error) {

err := b.Client.Get(context.Background(), key, &secret)
if err != nil {
if errors.IsNotFound(err) {
Expand Down
7 changes: 0 additions & 7 deletions controllers/dpa_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ func (r *DPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
Message: err.Error(),
},
)

} else {
apimeta.SetStatusCondition(&dpa.Status.Conditions,
metav1.Condition{
Expand Down Expand Up @@ -167,10 +166,8 @@ func (l *labelHandler) Create(evt event.CreateEvent, q workqueue.RateLimitingInt
Name: dpaname,
Namespace: namespace,
}})

}
func (l *labelHandler) Delete(evt event.DeleteEvent, q workqueue.RateLimitingInterface) {

namespace := evt.Object.GetNamespace()
dpaname := evt.Object.GetLabels()[namespace+".dataprotectionapplication"]
if evt.Object.GetLabels()[oadpv1alpha1.OadpOperatorLabel] == "" || dpaname == "" {
Expand All @@ -180,7 +177,6 @@ func (l *labelHandler) Delete(evt event.DeleteEvent, q workqueue.RateLimitingInt
Name: dpaname,
Namespace: namespace,
}})

}
func (l *labelHandler) Update(evt event.UpdateEvent, q workqueue.RateLimitingInterface) {
namespace := evt.ObjectNew.GetNamespace()
Expand All @@ -192,10 +188,8 @@ func (l *labelHandler) Update(evt event.UpdateEvent, q workqueue.RateLimitingInt
Name: dpaname,
Namespace: namespace,
}})

}
func (l *labelHandler) Generic(evt event.GenericEvent, q workqueue.RateLimitingInterface) {

namespace := evt.Object.GetNamespace()
dpaname := evt.Object.GetLabels()[namespace+".dataprotectionapplication"]
if evt.Object.GetLabels()[oadpv1alpha1.OadpOperatorLabel] == "" || dpaname == "" {
Expand All @@ -205,7 +199,6 @@ func (l *labelHandler) Generic(evt event.GenericEvent, q workqueue.RateLimitingI
Name: dpaname,
Namespace: namespace,
}})

}

type ReconcileFunc func(logr.Logger) (bool, error)
Expand Down
2 changes: 0 additions & 2 deletions controllers/nodeagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,6 @@ func (r *DPAReconciler) ReconcileFsRestoreHelperConfig(log logr.Logger) (bool, e
}

op, err := controllerutil.CreateOrPatch(r.Context, r.Client, &fsRestoreHelperCM, func() error {

// update the Config Map
err := r.updateFsRestoreHelperCM(&fsRestoreHelperCM, &dpa)
return err
Expand All @@ -416,7 +415,6 @@ func (r *DPAReconciler) ReconcileFsRestoreHelperConfig(log logr.Logger) (bool, e
}

func (r *DPAReconciler) updateFsRestoreHelperCM(fsRestoreHelperCM *corev1.ConfigMap, dpa *oadpv1alpha1.DataProtectionApplication) error {

// Setting controller owner reference on the FS restore helper CM
err := controllerutil.SetControllerReference(dpa, fsRestoreHelperCM, r.Scheme)
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion controllers/nodeagent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2489,7 +2489,6 @@ func TestDPAReconciler_buildNodeAgentDaemonset(t *testing.T) {
}

func TestDPAReconciler_updateFsRestoreHelperCM(t *testing.T) {

tests := []struct {
name string
fsRestoreHelperCM *v1.ConfigMap
Expand Down
7 changes: 1 addition & 6 deletions controllers/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ func registryName(bsl *velerov1.BackupStorageLocation) string {
}

func (r *DPAReconciler) getProviderSecret(secretName string) (corev1.Secret, error) {

secret := corev1.Secret{}
key := types.NamespacedName{
Name: secretName,
Expand Down Expand Up @@ -244,7 +243,6 @@ func replaceCarriageReturn(data map[string][]byte, logger logr.Logger) map[strin
}

func (r *DPAReconciler) getSecretNameAndKeyforBackupLocation(bslspec oadpv1alpha1.BackupLocation) (string, string) {

if bslspec.CloudStorage != nil {
if bslspec.CloudStorage.Credential != nil {
return bslspec.CloudStorage.Credential.Name, bslspec.CloudStorage.Credential.Key
Expand All @@ -258,7 +256,7 @@ func (r *DPAReconciler) getSecretNameAndKeyforBackupLocation(bslspec oadpv1alpha
}

func (r *DPAReconciler) getSecretNameAndKey(bslSpec *velerov1.BackupStorageLocationSpec, plugin oadpv1alpha1.DefaultPlugin) (string, string) {
// Assume default values unless user has overriden them
// Assume default values unless user has overridden them
secretName := credentials.PluginSpecificFields[plugin].SecretName
secretKey := credentials.PluginSpecificFields[plugin].PluginSecretKey
if _, ok := bslSpec.Config["credentialsFile"]; ok {
Expand All @@ -283,7 +281,6 @@ func (r *DPAReconciler) getSecretNameAndKey(bslSpec *velerov1.BackupStorageLocat
}

func (r *DPAReconciler) parseAWSSecret(secret corev1.Secret, secretKey string, matchProfile string) (string, string, error) {

AWSAccessKey, AWSSecretKey, profile := "", "", ""
splitString := strings.Split(string(secret.Data[secretKey]), "\n")
keyNameRegex, err := regexp.Compile(`\[.*\]`)
Expand Down Expand Up @@ -368,7 +365,6 @@ func (r *DPAReconciler) parseAWSSecret(secret corev1.Secret, secretKey string, m
}

func (r *DPAReconciler) parseAzureSecret(secret corev1.Secret, secretKey string) (azureCredentials, error) {

azcreds := azureCredentials{}

splitString := strings.Split(string(secret.Data[secretKey]), "\n")
Expand Down Expand Up @@ -577,7 +573,6 @@ func (r *DPAReconciler) ReconcileRegistryRoutes(log logr.Logger) (bool, error) {
}

func (r *DPAReconciler) ReconcileRegistryRouteConfigs(log logr.Logger) (bool, error) {

// Now for each of these bsl instances, create a registry route cm for each of them
registryRouteCM := corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Expand Down
1 change: 0 additions & 1 deletion controllers/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ func TestDPAReconciler_getSecretNameAndKeyforBackupLocation(t *testing.T) {
}

func TestDPAReconciler_populateAWSRegistrySecret(t *testing.T) {

tests := []struct {
name string
bsl *velerov1.BackupStorageLocation
Expand Down
1 change: 0 additions & 1 deletion controllers/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ func (r *DPAReconciler) ValidateDataProtectionCR(log logr.Logger) (bool, error)

// Check the Velero flags 'no-secret' or 'no-default-backup-location' are not set
if !dpa.Spec.Configuration.Velero.HasFeatureFlag("no-secret") || dpa.Spec.Configuration.Velero.NoDefaultBackupLocation {

// Check if the BSL secret key configured in the DPA exists with a secret data
secretName, secretKey := r.getSecretNameAndKeyforBackupLocation(location)
bslSecret, err := r.getProviderSecret(secretName)
Expand Down
Loading