fixup! fix: Enable more linters

Signed-off-by: Mateus Oliveira <[email protected]>

.github/workflows/ci.yml

@@ -36,7 +36,9 @@ jobs:
     # TODO coverage report
 
       - name: Go linters
-        run: make lint
+        uses: golangci/golangci-lint-action@v4
+        with:
+          version: v1.55.2
 
   container-check:
     runs-on: ubuntu-latest

.golangci.yml

@@ -2,7 +2,12 @@
 run:
   skip-dirs-use-default: false
   modules-download-mode: readonly
-  allow-parallel-runners: false
+  allow-parallel-runners: true
+  skip-dirs:
+    - test/*
+  skip-files:
+    - internal/controller/velerobackup_controller.go
+    - internal/controller/velerobackup_controller_test.go
 
 output:
   format: colored-line-number
@@ -107,8 +112,14 @@ issues:
         - revive
       text: "^struct-tag: unknown option 'inline' in JSON tag$"
     - linters:
-        - stylecheck
-      text: "ST1000:|ST1020:|ST1021:|ST1022:"
+        - revive
+      text: "^add-constant: avoid magic numbers like '0', create a named constant for it$"
+    - linters:
+        - revive
+      text: "^add-constant: avoid magic numbers like '1', create a named constant for it$"
+    # - linters:
+    #     - stylecheck
+    #   text: "ST1000:|ST1020:|ST1021:|ST1022:"
   max-issues-per-linter: 0
   max-same-issues: 0
 

Makefile

@@ -166,7 +166,7 @@ GOLANGCI_LINT = $(LOCALBIN)/golangci-lint-$(GOLANGCI_LINT_VERSION)
 KUSTOMIZE_VERSION ?= v5.3.0
 CONTROLLER_TOOLS_VERSION ?= v0.14.0
 ENVTEST_VERSION ?= latest
-GOLANGCI_LINT_VERSION ?= v1.56.2
+GOLANGCI_LINT_VERSION ?= v1.55.2
 
 .PHONY: kustomize
 kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary.

api/v1alpha1/nonadminbackup_types.go

@@ -23,27 +23,27 @@ import (
 
 // NonAdminBackupSpec defines the desired state of NonAdminBackup
 type NonAdminBackupSpec struct {
-	// NonAdminBackup log level (use debug for the most logging, leave unset for default)
-	// +optional
-	// +kubebuilder:validation:Enum=trace;debug;info;warning;error;fatal;panic
-	LogLevel string `json:"logLevel,omitempty"`
-
 	// https://github.com/vmware-tanzu/velero/blob/main/pkg/apis/velero/v1/backup_types.go
 
 	// BackupSpec defines the specification for a Velero backup.
 	BackupSpec *velerov1api.BackupSpec `json:"backupSpec,omitempty"`
 
 	// BackupStatus captures the current status of a Velero backup.
 	BackupStatus *velerov1api.BackupStatus `json:"backupStatus,omitempty"`
+
+	// NonAdminBackup log level (use debug for the most logging, leave unset for default)
+	// +optional
+	// +kubebuilder:validation:Enum=trace;debug;info;warning;error;fatal;panic
+	LogLevel string `json:"logLevel,omitempty"`
 }
 
 // NonAdminBackupStatus defines the observed state of NonAdminBackup
 type NonAdminBackupStatus struct {
 	Conditions []metav1.Condition `json:"conditions,omitempty"`
 }
 
-//+kubebuilder:object:root=true
-//+kubebuilder:subresource:status
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
 
 // NonAdminBackup is the Schema for the nonadminbackups API
 type NonAdminBackup struct {
@@ -54,7 +54,7 @@ type NonAdminBackup struct {
 	Status NonAdminBackupStatus `json:"status,omitempty"`
 }
 
-//+kubebuilder:object:root=true
+// +kubebuilder:object:root=true
 
 // NonAdminBackupList contains a list of NonAdminBackup
 type NonAdminBackupList struct {

cmd/main.go

@@ -49,7 +49,7 @@ func init() {
 	utilruntime.Must(nacv1alpha1.AddToScheme(scheme))
 
 	utilruntime.Must(velerov1api.AddToScheme(scheme))
-	//+kubebuilder:scaffold:scheme
+	// +kubebuilder:scaffold:scheme
 }
 
 func main() {
@@ -130,7 +130,7 @@ func main() {
 		setupLog.Error(err, "unable to create controller", "controller", "NonAdminBackup")
 		os.Exit(1)
 	}
-	//+kubebuilder:scaffold:builder
+	// +kubebuilder:scaffold:builder
 
 	if err = (&controller.VeleroBackupReconciler{
 		Client: mgr.GetClient(),

internal/controller/common_k8s.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2024.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controller
 
 import (
@@ -14,7 +30,7 @@ const (
 	ManagedByLabelValue          = "oadp-nac-controller"
 	NabOriginNameAnnotation      = "openshift.io/oadp-nab-origin-name"
 	NabOriginNamespaceAnnotation = "openshift.io/oadp-nab-origin-namespace"
-	NabOriginUuidAnnotation      = "openshift.io/oadp-nab-origin-uuid"
+	NabOriginUUIDAnnotation      = "openshift.io/oadp-nab-origin-uuid"
 )
 
 const (
@@ -29,22 +45,24 @@ func CreateLabelsForNac(labels map[string]string) map[string]string {
 
 	mergedLabels, err := mergeUniqueKeyTOfTMaps(defaultLabels, labels)
 	if err != nil {
-		fmt.Println("Error merging labels:", err)
+		// TODO logger
+		_, _ = fmt.Println("Error merging labels:", err)
 		return defaultLabels
 	}
 	return mergedLabels
 }
 
-func CreateAnnotationsForNac(ownerNamespace string, ownerName string, ownerUuid string, existingAnnotations map[string]string) map[string]string {
+func CreateAnnotationsForNac(ownerNamespace string, ownerName string, ownerUUID string, existingAnnotations map[string]string) map[string]string {
 	defaultAnnotations := map[string]string{
 		NabOriginNamespaceAnnotation: ownerNamespace,
 		NabOriginNameAnnotation:      ownerName,
-		NabOriginUuidAnnotation:      ownerUuid,
+		NabOriginUUIDAnnotation:      ownerUUID,
 	}
 
 	mergedAnnotations, err := mergeUniqueKeyTOfTMaps(defaultAnnotations, existingAnnotations)
 	if err != nil {
-		fmt.Println("Error merging annotations:", err)
+		// TODO logger
+		_, _ = fmt.Println("Error merging annotations:", err)
 		return defaultAnnotations
 	}
 	return mergedAnnotations

internal/controller/common_k8s_test.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2024.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controller
 
 import (
@@ -8,71 +24,64 @@ import (
 )
 
 func TestMergeUniqueKeyTOfTMaps(t *testing.T) {
-	type args struct {
-		userLabels []map[string]string
-	}
+	const (
+		d     = "d"
+		delta = "delta"
+	)
 	tests := []struct {
 		name    string
-		args    args
 		want    map[string]string
+		args    []map[string]string
 		wantErr bool
 	}{
 		{
 			name: "append unique labels together",
-			args: args{
-				userLabels: []map[string]string{
-					{"a": "a"},
-					{"b": "b"},
-				},
+			args: []map[string]string{
+				{"a": "alpha"},
+				{"b": "beta"},
 			},
 			want: map[string]string{
-				"a": "a",
-				"b": "b",
+				"a": "alpha",
+				"b": "beta",
 			},
 		},
 		{
 			name: "append unique labels together, with valid duplicates",
-			args: args{
-				userLabels: []map[string]string{
-					{"a": "a"},
-					{"b": "b"},
-					{"b": "b"},
-				},
+			args: []map[string]string{
+				{"c": "gamma"},
+				{d: delta},
+				{d: delta},
 			},
 			want: map[string]string{
-				"a": "a",
-				"b": "b",
+				"c": "gamma",
+				d:   delta,
 			},
 		},
 		{
 			name: "append unique labels together - nil sandwich",
-			args: args{
-				userLabels: []map[string]string{
-					{"a": "a"},
-					nil,
-					{"b": "b"},
-				},
+			args: []map[string]string{
+				{"x": "chi"},
+				nil,
+				{"y": "psi"},
 			},
 			want: map[string]string{
-				"a": "a",
-				"b": "b",
+				"x": "chi",
+				"y": "psi",
 			},
 		},
 		{
 			name: "should error when append duplicate label keys with different value together",
-			args: args{
-				userLabels: []map[string]string{
-					{"a": "a"},
-					{"a": "b"},
-				},
+			args: []map[string]string{
+				{"key": "value-1"},
+				{"key": "value-2"},
 			},
 			want:    nil,
 			wantErr: true,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := mergeUniqueKeyTOfTMaps(tt.args.userLabels...)
+			got, err := mergeUniqueKeyTOfTMaps(tt.args...)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("mergeUniqueKeyTOfTMaps() error = %v, wantErr %v", err, tt.wantErr)
 				return
@@ -111,17 +120,17 @@ func TestCreateAnnotationsForNac(t *testing.T) {
 
 	ownerName := "testOwner"
 	ownerNamespace := "testNamespace"
-	ownerUuid := "f2c4d2c3-58d3-46ec-bf03-5940f567f7f8"
+	ownerUUID := "f2c4d2c3-58d3-46ec-bf03-5940f567f7f8"
 
 	expectedAnnotations := map[string]string{
 		NabOriginNamespaceAnnotation: ownerNamespace,
 		NabOriginNameAnnotation:      ownerName,
-		NabOriginUuidAnnotation:      ownerUuid,
+		NabOriginUUIDAnnotation:      ownerUUID,
 		"existingKey1":               "existingValue1",
 		"existingKey2":               "existingValue2",
 	}
 
-	mergedAnnotations := CreateAnnotationsForNac(ownerNamespace, ownerName, ownerUuid, existingAnnotations)
+	mergedAnnotations := CreateAnnotationsForNac(ownerNamespace, ownerName, ownerUUID, existingAnnotations)
 	assert.Equal(t, expectedAnnotations, mergedAnnotations, "Merged annotations should match expected annotations")
 
 	// Merging annotations with conflicts
@@ -132,9 +141,9 @@ func TestCreateAnnotationsForNac(t *testing.T) {
 	expectedAnnotationsWithConflict := map[string]string{
 		NabOriginNameAnnotation:      ownerName,
 		NabOriginNamespaceAnnotation: ownerNamespace,
-		NabOriginUuidAnnotation:      ownerUuid,
+		NabOriginUUIDAnnotation:      ownerUUID,
 	}
 
-	mergedAnnotationsWithConflict := CreateAnnotationsForNac(ownerNamespace, ownerName, ownerUuid, existingAnnotationsWithConflict)
+	mergedAnnotationsWithConflict := CreateAnnotationsForNac(ownerNamespace, ownerName, ownerUUID, existingAnnotationsWithConflict)
 	assert.Equal(t, expectedAnnotationsWithConflict, mergedAnnotationsWithConflict, "Merged annotations should match expected annotations with conflict")
 }

internal/controller/common_nab.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2024.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controller
 
 import (
@@ -11,7 +27,6 @@ import (
 )
 
 func GetVeleroBackupSpecFromNonAdminBackup(nonAdminBackup *nacv1alpha1.NonAdminBackup) (*velerov1api.BackupSpec, error) {
-
 	if nonAdminBackup == nil {
 		return nil, fmt.Errorf("nonAdminBackup is nil")
 	}

internal/controller/common_nab_test.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2024.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controller
 
 import (