From 17b6ecde33e210bf1470785b6645f5898a54250c Mon Sep 17 00:00:00 2001 From: Robert Graeff Date: Mon, 19 May 2025 16:09:57 +0200 Subject: [PATCH 1/3] update submodule to commit 80bf --- hack/common | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hack/common b/hack/common index 3fc4627..80bfca5 160000 --- a/hack/common +++ b/hack/common @@ -1 +1 @@ -Subproject commit 3fc4627445a29eec865f52a151ab5e17b570a2f6 +Subproject commit 80bfca5ddf3186e97df6b7d1acdea035f6ba4632 From f17e2b012bbed55e9ed61ed04a060488fc0d08d5 Mon Sep 17 00:00:00 2001 From: Robert Graeff Date: Mon, 19 May 2025 16:16:38 +0200 Subject: [PATCH 2/3] lint config --- .golangci.yaml | 48 ++++++++++++++++++++++++++---------------------- Taskfile.yaml | 1 - 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 7964748..9a69a7f 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,29 +1,15 @@ +version: "2" run: - timeout: 5m allow-parallel-runners: true - -issues: - # don't skip warning about doc comments - # don't exclude the default set of lint - exclude-use-default: false - # restore some of the defaults - # (fill in the rest as needed) - exclude-rules: - - path: "internal/*" - linters: - - dupl linters: - disable-all: true + default: none enable: + - copyloopvar - dupl - errcheck - - copyloopvar - ginkgolinter - goconst - gocyclo - - gofmt - - goimports - - gosimple - govet - ineffassign - misspell @@ -31,12 +17,30 @@ linters: - prealloc - revive - staticcheck - - typecheck - unconvert - unparam - unused - -linters-settings: - revive: + settings: + revive: + rules: + - name: comment-spacings + exclusions: + generated: lax rules: - - name: comment-spacings + - linters: + - dupl + path: internal/* + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gofmt + - goimports + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/Taskfile.yaml b/Taskfile.yaml index 1176f31..e957b88 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -14,4 +14,3 @@ includes: REPO_URL: 'https://github.com/openmcp-project/openmcp-operator' GENERATE_DOCS_INDEX: "true" CHART_COMPONENTS: "[]" - LINTER_VERSION: "v1.64.4" From 47452fd065a3dd156f384ebd319e970089724b44 Mon Sep 17 00:00:00 2001 From: Robert Graeff Date: Mon, 19 May 2025 16:24:20 +0200 Subject: [PATCH 3/3] fix lint findings --- internal/controllers/provider/controller.go | 4 +- .../controllers/provider/install/installer.go | 42 +++++++++---------- internal/controllers/scheduler/controller.go | 8 ++-- .../controllers/scheduler/controller_test.go | 6 +-- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/internal/controllers/provider/controller.go b/internal/controllers/provider/controller.go index 7d703b0..1f9d7d7 100644 --- a/internal/controllers/provider/controller.go +++ b/internal/controllers/provider/controller.go @@ -40,7 +40,7 @@ type ProviderReconciler struct { } func (r *ProviderReconciler) ControllerName() string { - return strings.ToLower(r.GroupVersionKind.Kind) + return strings.ToLower(r.Kind) } func (r *ProviderReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, err error) { @@ -78,7 +78,7 @@ func (r *ProviderReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r // Therefore, the ProviderReconciler watches also jobs. The present method handles the job events and creates a reconcile request. func (r *ProviderReconciler) HandleJob(_ context.Context, job client.Object) []reconcile.Request { providerKind, found := job.GetLabels()[install.ProviderKindLabel] - if !found || providerKind != r.GroupVersionKind.Kind { + if !found || providerKind != r.Kind { return nil } diff --git a/internal/controllers/provider/install/installer.go b/internal/controllers/provider/install/installer.go index 6f02a89..6253705 100644 --- a/internal/controllers/provider/install/installer.go +++ b/internal/controllers/provider/install/installer.go @@ -7,7 +7,7 @@ import ( "github.com/openmcp-project/controller-utils/pkg/logging" "github.com/openmcp-project/controller-utils/pkg/readiness" - . "github.com/openmcp-project/controller-utils/pkg/resources" + "github.com/openmcp-project/controller-utils/pkg/resources" v1 "k8s.io/api/batch/v1" core "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -38,19 +38,19 @@ func (a *Installer) InstallInitJob(ctx context.Context) (completed bool, err err values := NewValues(a.Provider, a.DeploymentSpec, a.Environment) - if err := CreateOrUpdateResource(ctx, a.PlatformClient, NewNamespaceMutator(values.Namespace(), nil, nil)); err != nil { + if err := resources.CreateOrUpdateResource(ctx, a.PlatformClient, resources.NewNamespaceMutator(values.Namespace(), nil, nil)); err != nil { return false, err } - if err = CreateOrUpdateResource(ctx, a.PlatformClient, newInitServiceAccountMutator(values)); err != nil { + if err = resources.CreateOrUpdateResource(ctx, a.PlatformClient, newInitServiceAccountMutator(values)); err != nil { return false, err } - if err = CreateOrUpdateResource(ctx, a.PlatformClient, newInitClusterRoleMutator(values)); err != nil { + if err = resources.CreateOrUpdateResource(ctx, a.PlatformClient, newInitClusterRoleMutator(values)); err != nil { return false, err } - if err = CreateOrUpdateResource(ctx, a.PlatformClient, newInitClusterRoleBindingMutator(values)); err != nil { + if err = resources.CreateOrUpdateResource(ctx, a.PlatformClient, newInitClusterRoleBindingMutator(values)); err != nil { return false, err } @@ -61,7 +61,7 @@ func (a *Installer) InstallInitJob(ctx context.Context) (completed bool, err err }) var job *v1.Job found := true - job, err = GetResource(ctx, a.PlatformClient, j) + job, err = resources.GetResource(ctx, a.PlatformClient, j) if err != nil { if apierrors.IsNotFound(err) { found = false @@ -72,7 +72,7 @@ func (a *Installer) InstallInitJob(ctx context.Context) (completed bool, err err if !found { // Job does not exist, create it - if err := CreateOrUpdateResource(ctx, a.PlatformClient, j); err != nil { + if err := resources.CreateOrUpdateResource(ctx, a.PlatformClient, j); err != nil { return false, fmt.Errorf("failed to create job %s/%s: %w", values.Namespace(), a.Provider.GetName(), err) } return false, nil @@ -83,10 +83,10 @@ func (a *Installer) InstallInitJob(ctx context.Context) (completed bool, err err return false, fmt.Errorf("failed to cleanup job pods %s/%s: %w", values.Namespace(), a.Provider.GetName(), err) } - if err := DeleteResource(ctx, a.PlatformClient, j); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, j); err != nil { return false, fmt.Errorf("failed to delete job %s/%s: %w", values.Namespace(), a.Provider.GetName(), err) } - if err := CreateOrUpdateResource(ctx, a.PlatformClient, j); err != nil { + if err := resources.CreateOrUpdateResource(ctx, a.PlatformClient, j); err != nil { return false, fmt.Errorf("failed to re-create job %s/%s: %w", values.Namespace(), a.Provider.GetName(), err) } return false, nil @@ -101,15 +101,15 @@ func (a *Installer) InstallProvider(ctx context.Context) error { values := NewValues(a.Provider, a.DeploymentSpec, a.Environment) - if err := CreateOrUpdateResource(ctx, a.PlatformClient, newProviderServiceAccountMutator(values)); err != nil { + if err := resources.CreateOrUpdateResource(ctx, a.PlatformClient, newProviderServiceAccountMutator(values)); err != nil { return err } - if err := CreateOrUpdateResource(ctx, a.PlatformClient, newProviderClusterRoleBindingMutator(values)); err != nil { + if err := resources.CreateOrUpdateResource(ctx, a.PlatformClient, newProviderClusterRoleBindingMutator(values)); err != nil { return err } - if err := CreateOrUpdateResource(ctx, a.PlatformClient, newDeploymentMutator(values)); err != nil { + if err := resources.CreateOrUpdateResource(ctx, a.PlatformClient, newDeploymentMutator(values)); err != nil { return err } @@ -119,7 +119,7 @@ func (a *Installer) InstallProvider(ctx context.Context) error { func (a *Installer) CheckProviderReadiness(ctx context.Context) readiness.CheckResult { values := NewValues(a.Provider, a.DeploymentSpec, a.Environment) - depl, err := GetResource(ctx, a.PlatformClient, newDeploymentMutator(values)) + depl, err := resources.GetResource(ctx, a.PlatformClient, newDeploymentMutator(values)) if err != nil { return readiness.NewFailedResult(err) } @@ -131,31 +131,31 @@ func (a *Installer) UninstallProvider(ctx context.Context) (deleted bool, err er values := NewValues(a.Provider, a.DeploymentSpec, a.Environment) - if err := DeleteResource(ctx, a.PlatformClient, newDeploymentMutator(values)); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, newDeploymentMutator(values)); err != nil { return false, err } - if err := DeleteResource(ctx, a.PlatformClient, newProviderClusterRoleBindingMutator(values)); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, newProviderClusterRoleBindingMutator(values)); err != nil { return false, err } - if err := DeleteResource(ctx, a.PlatformClient, newProviderServiceAccountMutator(values)); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, newProviderServiceAccountMutator(values)); err != nil { return false, err } - if err := DeleteResource(ctx, a.PlatformClient, newJobMutator(values, a.DeploymentSpec, nil)); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, newJobMutator(values, a.DeploymentSpec, nil)); err != nil { return false, err } - if err := DeleteResource(ctx, a.PlatformClient, newInitClusterRoleBindingMutator(values)); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, newInitClusterRoleBindingMutator(values)); err != nil { return false, err } - if err := DeleteResource(ctx, a.PlatformClient, newInitClusterRoleMutator(values)); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, newInitClusterRoleMutator(values)); err != nil { return false, err } - if err := DeleteResource(ctx, a.PlatformClient, newInitServiceAccountMutator(values)); err != nil { + if err := resources.DeleteResource(ctx, a.PlatformClient, newInitServiceAccountMutator(values)); err != nil { return false, err } @@ -194,7 +194,7 @@ func (a *Installer) isJobFailed(job *v1.Job) bool { } func (a *Installer) isGenerationUpToDate(ctx context.Context, job *v1.Job) bool { - genJob := job.ObjectMeta.Annotations[ProviderGenerationLabel] + genJob := job.Annotations[ProviderGenerationLabel] if genJob == "" { return false } diff --git a/internal/controllers/scheduler/controller.go b/internal/controllers/scheduler/controller.go index 6332aa2..7d2edeb 100644 --- a/internal/controllers/scheduler/controller.go +++ b/internal/controllers/scheduler/controller.go @@ -81,7 +81,7 @@ func (r *ClusterScheduler) reconcile(ctx context.Context, log logging.Logger, re log.Info("Resource not found") return ReconcileResult{} } - return ReconcileResult{ReconcileError: errutils.WithReason(fmt.Errorf("unable to get resource '%s' from cluster: %w", req.NamespacedName.String(), err), cconst.ReasonPlatformClusterInteractionProblem)} + return ReconcileResult{ReconcileError: errutils.WithReason(fmt.Errorf("unable to get resource '%s' from cluster: %w", req.String(), err), cconst.ReasonPlatformClusterInteractionProblem)} } // handle operation annotation @@ -125,7 +125,7 @@ func (r *ClusterScheduler) handleCreateOrUpdate(ctx context.Context, req reconci if controllerutil.AddFinalizer(cr, clustersv1alpha1.ClusterRequestFinalizer) { log.Info("Adding finalizer") if err := r.PlatformCluster.Client().Patch(ctx, cr, client.MergeFrom(rr.OldObject)); err != nil { - rr.ReconcileError = errutils.WithReason(fmt.Errorf("error patching finalizer on resource '%s': %w", req.NamespacedName.String(), err), cconst.ReasonPlatformClusterInteractionProblem) + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error patching finalizer on resource '%s': %w", req.String(), err), cconst.ReasonPlatformClusterInteractionProblem) return rr } } @@ -230,7 +230,7 @@ func (r *ClusterScheduler) handleCreateOrUpdate(ctx context.Context, req reconci // create Cluster resource if err := r.PlatformCluster.Client().Create(ctx, cluster); err != nil { if apierrors.IsAlreadyExists(err) { - rr.ReconcileError = errutils.WithReason(fmt.Errorf("Cluster '%s/%s' already exists, this is not supposed to happen", cluster.Namespace, cluster.Name), cconst.ReasonInternalError) + rr.ReconcileError = errutils.WithReason(fmt.Errorf("cluster '%s/%s' already exists, this is not supposed to happen", cluster.Namespace, cluster.Name), cconst.ReasonInternalError) return rr } rr.ReconcileError = errutils.WithReason(fmt.Errorf("error creating cluster '%s/%s': %w", cluster.Namespace, cluster.Name, err), cconst.ReasonPlatformClusterInteractionProblem) @@ -304,7 +304,7 @@ func (r *ClusterScheduler) handleDelete(ctx context.Context, req reconcile.Reque if controllerutil.RemoveFinalizer(cr, clustersv1alpha1.ClusterRequestFinalizer) { log.Info("Removing finalizer") if err := r.PlatformCluster.Client().Patch(ctx, cr, client.MergeFrom(rr.OldObject)); err != nil { - rr.ReconcileError = errutils.WithReason(fmt.Errorf("error removing finalizer from resource '%s': %w", req.NamespacedName.String(), err), cconst.ReasonPlatformClusterInteractionProblem) + rr.ReconcileError = errutils.WithReason(fmt.Errorf("error removing finalizer from resource '%s': %w", req.String(), err), cconst.ReasonPlatformClusterInteractionProblem) return rr } } diff --git a/internal/controllers/scheduler/controller_test.go b/internal/controllers/scheduler/controller_test.go index 38a889a..6a773e8 100644 --- a/internal/controllers/scheduler/controller_test.go +++ b/internal/controllers/scheduler/controller_test.go @@ -70,7 +70,7 @@ var _ = Describe("Scheduler", func() { Expect(cluster.Name).To(Equal(req.Status.Cluster.Name)) Expect(cluster.Namespace).To(Equal(clusterNamespace)) Expect(cluster.Name).To(HavePrefix(fmt.Sprintf("%s-", req.Spec.Purpose))) - Expect(cluster.Namespace).To(Equal(sc.Config.PurposeMappings[req.Spec.Purpose].Template.ObjectMeta.Namespace)) + Expect(cluster.Namespace).To(Equal(sc.Config.PurposeMappings[req.Spec.Purpose].Template.Namespace)) Expect(cluster.Spec.Tenancy).To(BeEquivalentTo(sc.Config.PurposeMappings[req.Spec.Purpose].Template.Spec.Tenancy)) Expect(cluster.Finalizers).To(ContainElements(req.FinalizerForCluster())) }) @@ -126,7 +126,7 @@ var _ = Describe("Scheduler", func() { Expect(cluster.Name).To(Equal(req.Status.Cluster.Name)) Expect(cluster.Namespace).To(Equal(clusterNamespace)) Expect(cluster.Name).To(HavePrefix(fmt.Sprintf("%s-", req.Spec.Purpose))) - Expect(cluster.Namespace).To(Equal(sc.Config.PurposeMappings[req.Spec.Purpose].Template.ObjectMeta.Namespace)) + Expect(cluster.Namespace).To(Equal(sc.Config.PurposeMappings[req.Spec.Purpose].Template.Namespace)) Expect(cluster.Spec.Tenancy).To(BeEquivalentTo(sc.Config.PurposeMappings[req.Spec.Purpose].Template.Spec.Tenancy)) Expect(cluster.Finalizers).To(ContainElements(req.FinalizerForCluster())) }) @@ -238,7 +238,7 @@ var _ = Describe("Scheduler", func() { Expect(cluster.Name).To(Equal(requests[0].Status.Cluster.Name)) Expect(cluster.Namespace).To(Equal(clusterNamespace)) Expect(cluster.Name).To(Equal(requests[0].Spec.Purpose)) - Expect(cluster.Namespace).To(Equal(sc.Config.PurposeMappings[requests[0].Spec.Purpose].Template.ObjectMeta.Namespace)) + Expect(cluster.Namespace).To(Equal(sc.Config.PurposeMappings[requests[0].Spec.Purpose].Template.Namespace)) Expect(cluster.Spec.Tenancy).To(BeEquivalentTo(clustersv1alpha1.TENANCY_SHARED)) Expect(cluster.Finalizers).To(ContainElements(requests[0].FinalizerForCluster())) })