chore: Add linter config (#281)

<!-- Thank you for your contribution. Before you submit the pull
request:
1. Follow contributing guidelines, templates, the recommended Git
workflow, and any related documentation.
2. Read and submit the required Contributor Licence Agreements
(https://github.com/kyma-project/community/blob/main/CONTRIBUTING.md#agreements-and-licenses).
3. Test your changes and attach their results to the pull request.
4. Update the relevant documentation.

If the pull request requires a decision, follow the [decision-making
process](https://github.com/kyma-project/community/blob/main/governance.md)
and replace the PR template with the [decision record
template](https://github.com/kyma-project/community/blob/main/.github/ISSUE_TEMPLATE/decision-record.md).
-->

**Description**

Changes proposed in this pull request:

- Add and adapt linter config from lifecycle-manager
- Fix linting issues

**Related issue(s)**
<!-- If you refer to a particular issue, provide its number. For
example, `Resolves #123`, `Fixes #43`, or `See also #33`. -->

---------

Co-authored-by: Badr, Nesma <nesma.badr@sap.com>

.github/workflows/lint-golangci.yml

@@ -19,3 +19,9 @@ jobs:
         with:
           version: v1.60.3
           args: --verbose
+      - name: golangci-lint for api module
+        uses: golangci/golangci-lint-action@v6.1.0
+        with:
+          version: v1.60.3
+          args: --verbose
+          working-directory: ./api

.golangci.yaml

@@ -1,7 +1,103 @@
 linters:
-  enable-all: false
-run:
-  timeout: 5m
+  enable-all: true
+  disable:
+    - contextcheck # too many false positives
+    - depguard # checks if package imports are whitelisted
+    - exhaustruct # too subjective and harms code readability
+    - exportloopref # deprecated (since v1.60.2), replaced by copyloopvar
+    - gomnd # deprecated (since v1.58.0), renamed to mnd
+    - lll
+    - nlreturn # too strict and mostly code is not more readable
+    - sqlclosecheck # not needed for this project
+    - paralleltest # should be enabled consciously for long-running tests
+    - wsl # too strict and mostly code is not more readable
+linters-settings:
+  gomoddirectives:
+    replace-local: true
+    replace-allow-list:
+      - github.com/kyma-project/template-operator/api
+  stylecheck:
+    dot-import-whitelist:
+      - github.com/onsi/ginkgo/v2
+      - github.com/onsi/gomega
+  revive:
+    severity: error
+    rules:
+      - name: comment-spacings
+        disabled: true
+      - name: dot-imports
+        severity: warning
+        disabled: true
+      - name: line-length-limit
+        severity: warning
+        disabled: true
+        arguments: [ 120 ]
+  funlen:
+    lines: 80
+  cyclop:
+    max-complexity: 20
+  nestif:
+    min-complexity: 6
+  gci:
+    sections:
+      - standard                                             # Standard packages.
+      - default                                              # Imports that could not be matched to another section type.
+      - prefix(github.com/kyma-project/template-operator)    # Imports with the specified prefix.
+      - blank                                                # Blank imports.
+      - dot                                                  # Dot imports.
+    custom-order: true
+    skip-generated: true
+  nolintlint:
+    require-explanation: true
+  ireturn:
+    allow:
+      - anon
+      - error
+      - empty
+      - stdlib
+      - Client
+      - record.EventRecorder
+      - client.Object
+      - schema.ObjectKind #api/v1beta2/moduletemplate_types.go
+      - runtime.Object #api/v1beta2/moduletemplate_types.go
+      - meta.RESTMapper #internal/declarative/v2/client_proxy.go, internal/declarative/v2/rest_getter_proxy.go
+      - client.SubResourceWriter #internal/declarative/v2/client_proxy.go
+      - openapi.Resources #internal/declarative/v2/kube_factory_proxy.go
+      - validation.Schema #internal/declarative/v2/kube_factory_proxy.go
+      - discovery.CachedDiscoveryInterface #internal/declarative/v2/rest_getter_proxy.go
+      - machineryruntime.Object #internal/declarative/v2/ssa.go
+      - v1.Layer #internal/manifest/parse.go, tests/integration/controller/manifest/manifest.go
+      - authn.Keychain #internal/manifest/spec_resolver.go, pkg/ocmextensions/cred.go
+      - ratelimiter.RateLimiter #internal/rate_limiter.go
+  varnamelen:
+    ignore-names:
+      - ok
+    ignore-type-assert-ok: true
+    ignore-map-index-ok: true
+    ignore-chan-recv-ok: true
+  exhaustruct:
+    exclude:
+      - gdfs
 issues:
   exclude-files:
     - zz_generated.deepcopy.go
+  exclude-rules:
+    - path: "_test\\.go"
+      linters:
+        - wrapcheck # Errors do not need to be wrapped in unit and integration tests
+        - err113 # Dynamic error creation in unit and integration tests is ok
+        - gochecknoglobals # Does not apply to unit and integration tests
+        - funlen # Table driven unit and integration tests exceed function length by design
+        - maintidx # Table driven unit and integration tests exceed maintainability index by design
+    - linters:
+        - lll
+      source: "^// +kubebuilder: " # Exclude lll issues for long lines starting with kubebuilder marker prefix
+    - linters:
+        - lll # Exclude lll issues for long lines starting with an url
+      source: "^// http "
+  max-issues-per-linter: 0
+  max-same-issues: 0
+output:
+  sort-results: true
+run:
+  timeout: 15m

Makefile

@@ -165,7 +165,8 @@ GOLANG_CI_LINT_VERSION ?= v1.60.3
 .PHONY: lint
 lint: ## Download & Build & Run golangci-lint against code.
 	GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANG_CI_LINT_VERSION)
-	$(LOCALBIN)/golangci-lint run
+	$(LOCALBIN)/golangci-lint run --verbose -c .golangci.yaml
+	cd api && $(LOCALBIN)/golangci-lint run --verbose -c ../.golangci.yaml
 
 .PHONY: configure-git-origin
 configure-git-origin:

api/v1alpha1/sample_types.go

@@ -17,6 +17,8 @@ limitations under the License.
 // Package v1alpha1 contains API Schema definitions for the component v1alpha1 API group
 // +kubebuilder:object:generate=true
 // +groupName=operator.kyma-project.io
+//
+//nolint:gochecknoglobals // required for utilizing the API
 package v1alpha1
 
 import (

controllers/sample_controller_rendered_resources.go

@@ -23,8 +23,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"sigs.k8s.io/controller-runtime/pkg/scheme"
-
 	"github.com/go-logr/logr"
 	errors2 "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -35,12 +33,12 @@ import (
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/scheme"
 
 	"github.com/kyma-project/template-operator/api/v1alpha1"
-
-	"sigs.k8s.io/controller-runtime/pkg/controller"
 )
 
 // SampleReconciler reconciles a Sample object.
@@ -61,13 +59,15 @@ type ManifestResources struct {
 
 var (
 	// SchemeBuilder is used to add go types to the GroupVersionKind scheme.
+	//nolint:gochecknoglobals // used to register Sample CRD on startup
 	SchemeBuilder = &scheme.Builder{GroupVersion: v1alpha1.GroupVersion}
 
 	// AddToScheme adds the types in this group-version to the given scheme.
+	//nolint:gochecknoglobals // used to register Sample CRD on startup
 	AddToScheme = SchemeBuilder.AddToScheme
 )
 
-func init() { //nolint:gochecknoinits
+func init() { //nolint:gochecknoinits // used to register Sample CRD on startup
 	SchemeBuilder.Register(&v1alpha1.Sample{}, &v1alpha1.SampleList{})
 }
 
@@ -84,7 +84,7 @@ func init() { //nolint:gochecknoinits
 func (r *SampleReconciler) SetupWithManager(mgr ctrl.Manager, rateLimiter RateLimiter) error {
 	r.Config = mgr.GetConfig()
 
-	return ctrl.NewControllerManagedBy(mgr).
+	if err := ctrl.NewControllerManagedBy(mgr).
 		For(&v1alpha1.Sample{}).
 		WithOptions(controller.Options{
 			RateLimiter: TemplateRateLimiter(
@@ -94,7 +94,10 @@ func (r *SampleReconciler) SetupWithManager(mgr ctrl.Manager, rateLimiter RateLi
 				rateLimiter.Burst,
 			),
 		}).
-		Complete(r)
+		Complete(r); err != nil {
+		return fmt.Errorf("error while setting up controller: %w", err)
+	}
+	return nil
 }
 
 // Reconcile is the entry point from the controller-runtime framework.
@@ -109,7 +112,10 @@ func (r *SampleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		// requeue (we'll need to wait for a new notification), and we can get them
 		// on deleted requests.
 		logger.Info(req.NamespacedName.String() + " got deleted!")
-		return ctrl.Result{}, client.IgnoreNotFound(err)
+		if client.IgnoreNotFound(err) != nil {
+			return ctrl.Result{}, fmt.Errorf("error while getting object: %w", err)
+		}
+		return ctrl.Result{}, nil
 	}
 
 	// check if deletionTimestamp is set, retry until it gets deleted
@@ -201,7 +207,10 @@ func (r *SampleReconciler) HandleDeletingState(ctx context.Context, objectInstan
 	resourceObjs, err := getResourcesFromLocalPath(objectInstance.Spec.ResourceFilePath, logger)
 	if err != nil && controllerutil.RemoveFinalizer(objectInstance, finalizer) {
 		// if error is encountered simply remove the finalizer and delete the reconciled resource
-		return r.Client.Update(ctx, objectInstance)
+		if err := r.Client.Update(ctx, objectInstance); err != nil {
+			return fmt.Errorf("error while removing finalizer: %w", err)
+		}
+		return nil
 	}
 	r.Event(objectInstance, "Normal", "ResourcesDelete", "deleting resources")
 
@@ -224,7 +233,10 @@ func (r *SampleReconciler) HandleDeletingState(ctx context.Context, objectInstan
 
 	// if resources are ready to be deleted, remove finalizer
 	if controllerutil.RemoveFinalizer(objectInstance, finalizer) {
-		return r.Client.Update(ctx, objectInstance)
+		if err := r.Client.Update(ctx, objectInstance); err != nil {
+			return fmt.Errorf("error while removing finalizer: %w", err)
+		}
+		return nil
 	}
 	return nil
 }
@@ -268,7 +280,7 @@ func (r *SampleReconciler) processResources(ctx context.Context, objectInstance
 	resourceObjs, err := getResourcesFromLocalPath(objectInstance.Spec.ResourceFilePath, logger)
 	if err != nil {
 		logger.Error(err, "error locating manifest of resources")
-		return err
+		return fmt.Errorf("error locating manifest of resources: %w", err)
 	}
 
 	r.Event(objectInstance, "Normal", "ResourcesInstall", "installing resources")
@@ -278,7 +290,7 @@ func (r *SampleReconciler) processResources(ctx context.Context, objectInstance
 	for _, obj := range resourceObjs.Items {
 		if err = r.ssa(ctx, obj); err != nil && !errors2.IsAlreadyExists(err) {
 			logger.Error(err, "error during installation of resources")
-			return err
+			return fmt.Errorf("error during installation of resources: %w", err)
 		}
 	}
 	return nil
@@ -295,30 +307,33 @@ func getResourcesFromLocalPath(dirPath string, logger logr.Logger) (*ManifestRes
 	err := filepath.WalkDir(dirPath, func(path string, info fs.DirEntry, err error) error {
 		// initial error
 		if err != nil {
-			return err
+			return fmt.Errorf("error while walkdir %s: %w", dirPath, err)
 		}
 		if !info.IsDir() {
 			return nil
 		}
 		dirEntries, err = os.ReadDir(dirPath)
-		return err
+		if err != nil {
+			return fmt.Errorf("error while reading directory %s: %w", dirPath, err)
+		}
+		return nil
 	})
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error while walkdir %s: %w", dirPath, err)
 	}
 
 	childCount := len(dirEntries)
 	if childCount == 0 {
-		logger.V(debugLogLevel).Info(fmt.Sprintf("no yaml file found at file path %s", dirPath))
-		return nil, nil
+		logger.V(debugLogLevel).Info("no yaml file found at file path" + dirPath)
+		return nil, nil //nolint:nilnil // nil is returned if no yaml file is found
 	} else if childCount > 1 {
-		logger.V(debugLogLevel).Info(fmt.Sprintf("more than one yaml file found at file path %s", dirPath))
-		return nil, nil
+		logger.V(debugLogLevel).Info("more than one yaml file found at file path" + dirPath)
+		return nil, nil //nolint:nilnil // nil is returned if more than one yaml file is found
 	}
 	file := dirEntries[0]
 	allowedExtns := sets.NewString(".yaml", ".yml")
 	if !allowedExtns.Has(filepath.Ext(file.Name())) {
-		return nil, nil
+		return nil, nil //nolint:nilnil // nil is returned if file is not in yaml format
 	}
 
 	fileBytes, err := os.ReadFile(filepath.Join(dirPath, file.Name()))
@@ -332,13 +347,19 @@ func getResourcesFromLocalPath(dirPath string, logger logr.Logger) (*ManifestRes
 func (r *SampleReconciler) ssaStatus(ctx context.Context, obj client.Object) error {
 	obj.SetManagedFields(nil)
 	obj.SetResourceVersion("")
-	return r.Status().Patch(ctx, obj, client.Apply,
-		&client.SubResourcePatchOptions{PatchOptions: client.PatchOptions{FieldManager: fieldOwner}})
+	if err := r.Status().Patch(ctx, obj, client.Apply,
+		&client.SubResourcePatchOptions{PatchOptions: client.PatchOptions{FieldManager: fieldOwner}}); err != nil {
+		return fmt.Errorf("error while patching status: %w", err)
+	}
+	return nil
 }
 
 // ssa patches the object using SSA.
 func (r *SampleReconciler) ssa(ctx context.Context, obj client.Object) error {
 	obj.SetManagedFields(nil)
 	obj.SetResourceVersion("")
-	return r.Patch(ctx, obj, client.Apply, client.ForceOwnership, client.FieldOwner(fieldOwner))
+	if err := r.Patch(ctx, obj, client.Apply, client.ForceOwnership, client.FieldOwner(fieldOwner)); err != nil {
+		return fmt.Errorf("error while patching object: %w", err)
+	}
+	return nil
 }

controllers/sample_controller_rendered_resources_test.go

@@ -6,14 +6,14 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/kyma-project/template-operator/api/v1alpha1"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var (
@@ -98,9 +98,9 @@ func createSampleCR(sampleName, path string) *v1alpha1.Sample {
 }
 
 func getPod(namespace, podName string) func(g Gomega) bool {
-	return func(g Gomega) bool {
+	return func(gomega Gomega) bool {
 		clientSet, err := kubernetes.NewForConfig(reconciler.Config)
-		g.Expect(err).ToNot(HaveOccurred())
+		gomega.Expect(err).ToNot(HaveOccurred())
 
 		pod, err := clientSet.CoreV1().Pods(namespace).Get(ctx, podName, metav1.GetOptions{})
 		if err != nil {
@@ -115,7 +115,7 @@ func getPod(namespace, podName string) func(g Gomega) bool {
 		})
 
 		_, err = clientSet.CoreV1().Pods(namespace).UpdateStatus(ctx, pod, metav1.UpdateOptions{})
-		g.Expect(err).ToNot(HaveOccurred())
+		gomega.Expect(err).ToNot(HaveOccurred())
 		return true
 	}
 }
@@ -127,15 +127,15 @@ type CRStatus struct {
 }
 
 func getCRStatus(sampleObjKey client.ObjectKey) func(g Gomega) CRStatus {
-	return func(g Gomega) CRStatus {
+	return func(gomega Gomega) CRStatus {
 		sampleCR := &v1alpha1.Sample{}
 		err := k8sClient.Get(ctx, sampleObjKey, sampleCR)
 		if err != nil {
 			return CRStatus{State: v1alpha1.StateError, Err: err}
 		}
-		g.Expect(err).NotTo(HaveOccurred())
+		gomega.Expect(err).NotTo(HaveOccurred())
 		condition := meta.FindStatusCondition(sampleCR.Status.Conditions, v1alpha1.ConditionTypeInstallation)
-		g.Expect(condition).ShouldNot(BeNil())
+		gomega.Expect(condition).ShouldNot(BeNil())
 		return CRStatus{
 			State:                  sampleCR.Status.State,
 			InstallConditionStatus: condition.Status,
@@ -145,9 +145,9 @@ func getCRStatus(sampleObjKey client.ObjectKey) func(g Gomega) CRStatus {
 }
 
 func checkDeleted(sampleObjKey client.ObjectKey) func(g Gomega) bool {
-	return func(g Gomega) bool {
+	return func(gomega Gomega) bool {
 		clientSet, err := kubernetes.NewForConfig(reconciler.Config)
-		g.Expect(err).ToNot(HaveOccurred())
+		gomega.Expect(err).ToNot(HaveOccurred())
 
 		// check if Pod resource is deleted
 		_, err = clientSet.CoreV1().Pods(podNs).Get(ctx, podName, metav1.GetOptions{})