diff --git a/.github/workflows/lint-golangci.yml b/.github/workflows/lint-golangci.yml index 11106de..fc65ca9 100644 --- a/.github/workflows/lint-golangci.yml +++ b/.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 diff --git a/.golangci.yaml b/.golangci.yaml index eeb3a9b..e63bb97 100644 --- a/.golangci.yaml +++ b/.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 diff --git a/Makefile b/Makefile index e1c81e8..dbfd94f 100644 --- a/Makefile +++ b/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: diff --git a/api/v1alpha1/sample_types.go b/api/v1alpha1/sample_types.go index 014dfb0..0b77bbf 100644 --- a/api/v1alpha1/sample_types.go +++ b/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 ( diff --git a/controllers/sample_controller_rendered_resources.go b/controllers/sample_controller_rendered_resources.go index dfd02dd..364637c 100644 --- a/controllers/sample_controller_rendered_resources.go +++ b/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 } diff --git a/controllers/sample_controller_rendered_resources_test.go b/controllers/sample_controller_rendered_resources_test.go index c3f7288..695a0bf 100644 --- a/controllers/sample_controller_rendered_resources_test.go +++ b/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{}) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index c40fc0e..eb3c74d 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -22,33 +22,31 @@ import ( "testing" "time" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/manager" - - "github.com/kyma-project/template-operator/controllers" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" operatorkymaprojectiov1alpha1 "github.com/kyma-project/template-operator/api/v1alpha1" - //+kubebuilder:scaffold:imports + "github.com/kyma-project/template-operator/controllers" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to // http://onsi.github.io/ginkgo/ to learn more about Ginkgo. var ( - k8sClient client.Client //nolint:gochecknoglobals - k8sManager manager.Manager //nolint:gochecknoglobals - testEnv *envtest.Environment //nolint:gochecknoglobals - ctx context.Context //nolint:gochecknoglobals - cancel context.CancelFunc //nolint:gochecknoglobals - reconciler *controllers.SampleReconciler //nolint:gochecknoglobals + k8sClient client.Client + k8sManager manager.Manager + testEnv *envtest.Environment + ctx context.Context + cancel context.CancelFunc + reconciler *controllers.SampleReconciler ) const ( diff --git a/main.go b/main.go index f5c8a2e..0017edc 100644 --- a/main.go +++ b/main.go @@ -22,12 +22,9 @@ import ( "os" "time" - "k8s.io/apimachinery/pkg/runtime" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" + machineryruntime "k8s.io/apimachinery/pkg/runtime" + machineryutilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) - // to ensure that exec-entrypoint and run can make use of them. - _ "k8s.io/client-go/plugin/pkg/client/auth" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -36,7 +33,10 @@ import ( "github.com/kyma-project/template-operator/api/v1alpha1" "github.com/kyma-project/template-operator/controllers" - //+kubebuilder:scaffold:imports + + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) + // to ensure that exec-entrypoint and run can make use of them. + _ "k8s.io/client-go/plugin/pkg/client/auth" ) const ( @@ -45,11 +45,7 @@ const ( failureBaseDelayDefault = 1 * time.Second failureMaxDelayDefault = 1000 * time.Second operatorName = "template-operator" -) - -var ( - scheme = runtime.NewScheme() - setupLog = ctrl.Log.WithName("setup") + webhookPort = 9443 ) type FlagVar struct { @@ -65,16 +61,20 @@ type FlagVar struct { printVersion bool } -func init() { //nolint:gochecknoinits - utilruntime.Must(clientgoscheme.AddToScheme(scheme)) - utilruntime.Must(controllers.AddToScheme(scheme)) +func registerSchemes(scheme *machineryruntime.Scheme) { + machineryutilruntime.Must(clientgoscheme.AddToScheme(scheme)) + machineryutilruntime.Must(controllers.AddToScheme(scheme)) //+kubebuilder:scaffold:scheme } -//nolint:gochecknoglobals +//nolint:gochecknoglobals // used to embed static binary version during release builds var buildVersion = "not_provided" func main() { + scheme := machineryruntime.NewScheme() + setupLog := ctrl.Log.WithName("setup") + registerSchemes(scheme) + flagVar := defineFlagVar() opts := zap.Options{ Development: true, @@ -106,7 +106,8 @@ func main() { BindAddress: flagVar.metricsAddr, }, WebhookServer: webhook.NewServer(webhook.Options{ - Port: 9443}), + Port: webhookPort, + }), HealthProbeBindAddress: flagVar.probeAddr, LeaderElection: flagVar.enableLeaderElection, LeaderElectionID: "76223278.kyma-project.io",