From 2fdce1bf214070e0e46b185700cccdbf2c64c180 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Fri, 2 Oct 2026 10:37:06 +0100 Subject: [PATCH] fix(lint): resolve all golangci-lint findings Migrate the lint config to the v2 format. Replace the deprecated scheme.Builder with runtime.NewSchemeBuilder. Handle Close errors in the worker and replace repeated strings with constants. Remove unused parameters and use switch statements where the linter asks. --- .golangci.yml | 56 +++++---- api/raczylo.com/v1/clusterimage_types.go | 6 +- .../v1/clusterimageexport_types.go | 6 +- api/raczylo.com/v1/groupversion_info.go | 8 +- cmd/worker/main.go | 6 +- .../raczylo.com/clusterimage_controller.go | 11 +- .../clusterimageexport_controller.go | 26 ++-- .../raczylo.com/controller_unit_test.go | 113 +++++++++--------- internal/shared/definitions.go | 5 + internal/shared/jobs.go | 6 +- internal/shared/jobs_test.go | 20 ++-- internal/shared/k8s.go | 6 +- 12 files changed, 149 insertions(+), 120 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4ef851d..44e084f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,32 +1,14 @@ +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: "api/*" - linters: - - lll - - path: "internal/*" - linters: - - dupl linters: - disable-all: true + default: none enable: - dupl - errcheck - - exportloopref - ginkgolinter - goconst - gocyclo - - gofmt - - goimports - - gosimple - govet - ineffassign - misspell @@ -34,12 +16,36 @@ 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: + - goconst + path: _test\.go + - linters: + - lll + path: api/* + - linters: + - dupl + path: internal/* + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gofmt + - goimports + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/api/raczylo.com/v1/clusterimage_types.go b/api/raczylo.com/v1/clusterimage_types.go index c31e30e..8c2c3a8 100644 --- a/api/raczylo.com/v1/clusterimage_types.go +++ b/api/raczylo.com/v1/clusterimage_types.go @@ -19,6 +19,7 @@ package v1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) // ClusterImageSpec defines the desired state of ClusterImage @@ -81,5 +82,8 @@ type ClusterImageList struct { } func init() { - SchemeBuilder.Register(&ClusterImage{}, &ClusterImageList{}) + SchemeBuilder.Register(func(s *runtime.Scheme) error { + s.AddKnownTypes(GroupVersion, &ClusterImage{}, &ClusterImageList{}) + return nil + }) } diff --git a/api/raczylo.com/v1/clusterimageexport_types.go b/api/raczylo.com/v1/clusterimageexport_types.go index 1a18478..e1b0bba 100644 --- a/api/raczylo.com/v1/clusterimageexport_types.go +++ b/api/raczylo.com/v1/clusterimageexport_types.go @@ -19,6 +19,7 @@ package v1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) // RetentionPolicy defines how many completed ClusterImageExport resources to keep @@ -136,5 +137,8 @@ type ClusterImageExportList struct { } func init() { - SchemeBuilder.Register(&ClusterImageExport{}, &ClusterImageExportList{}) + SchemeBuilder.Register(func(s *runtime.Scheme) error { + s.AddKnownTypes(GroupVersion, &ClusterImageExport{}, &ClusterImageExportList{}) + return nil + }) } diff --git a/api/raczylo.com/v1/groupversion_info.go b/api/raczylo.com/v1/groupversion_info.go index d689874..1219b94 100644 --- a/api/raczylo.com/v1/groupversion_info.go +++ b/api/raczylo.com/v1/groupversion_info.go @@ -20,8 +20,9 @@ limitations under the License. package v1 import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/scheme" ) var ( @@ -29,7 +30,10 @@ var ( GroupVersion = schema.GroupVersion{Group: "raczylo.com", Version: "v1"} // SchemeBuilder is used to add go types to the GroupVersionKind scheme - SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} + SchemeBuilder = runtime.NewSchemeBuilder(func(s *runtime.Scheme) error { + metav1.AddToGroupVersion(s, GroupVersion) + return nil + }) // AddToScheme adds the types in this group-version to the given scheme. AddToScheme = SchemeBuilder.AddToScheme diff --git a/cmd/worker/main.go b/cmd/worker/main.go index c61b557..3d7aac9 100644 --- a/cmd/worker/main.go +++ b/cmd/worker/main.go @@ -233,7 +233,7 @@ func uploadToS3(source, destination string) error { if err != nil { return fmt.Errorf("failed to open source file: %w", err) } - defer file.Close() + defer func() { _ = file.Close() }() fmt.Printf("Uploading %s to s3://%s/%s\n", source, bucket, key) @@ -261,7 +261,7 @@ func copyLocal(source, destination string) error { if err != nil { return fmt.Errorf("failed to open source file: %w", err) } - defer srcFile.Close() + defer func() { _ = srcFile.Close() }() // Get source file info for permissions srcInfo, err := srcFile.Stat() @@ -274,7 +274,7 @@ func copyLocal(source, destination string) error { if err != nil { return fmt.Errorf("failed to create destination file: %w", err) } - defer dstFile.Close() + defer func() { _ = dstFile.Close() }() // Copy content if _, err := io.Copy(dstFile, srcFile); err != nil { diff --git a/internal/controller/raczylo.com/clusterimage_controller.go b/internal/controller/raczylo.com/clusterimage_controller.go index ae287a4..45694ce 100644 --- a/internal/controller/raczylo.com/clusterimage_controller.go +++ b/internal/controller/raczylo.com/clusterimage_controller.go @@ -144,7 +144,7 @@ func (r *ClusterImageReconciler) handlePendingClusterImage(ctx context.Context, } // Create the backup job - if err := r.createBackupJob(ctx, clusterImage, clusterImageExport, l); err != nil { + if err := r.createBackupJob(ctx, clusterImage, clusterImageExport); err != nil { l.Error(err, "unable to create backup job") return ctrl.Result{}, err } @@ -349,7 +349,7 @@ func (r *ClusterImageReconciler) cleanupJobAndPods(ctx context.Context, job *v1b return nil } -func (r *ClusterImageReconciler) createBackupJob(ctx context.Context, clusterImage *raczylocomv1.ClusterImage, clusterImageExport *raczylocomv1.ClusterImageExport, l logr.Logger) error { +func (r *ClusterImageReconciler) createBackupJob(ctx context.Context, clusterImage *raczylocomv1.ClusterImage, clusterImageExport *raczylocomv1.ClusterImageExport) error { normalisedImageName := shared.NormalizeImageName(clusterImage.Spec.FullName) defaultCommands := []string{ @@ -357,13 +357,14 @@ func (r *ClusterImageReconciler) createBackupJob(ctx context.Context, clusterIma "podman save --quiet -o /tmp/" + normalisedImageName + ".tar " + clusterImage.Spec.FullName, } - if clusterImage.Spec.Storage == shared.STORAGE_S3 { + switch clusterImage.Spec.Storage { + case shared.STORAGE_S3: s3Params := shared.SetupS3Params(clusterImageExport.Spec.Storage.S3) additionalCommands := []string{ "./worker export " + strings.Join(s3Params, " ") + " '/tmp/" + normalisedImageName + ".tar' " + "'s3://" + clusterImageExport.Spec.Storage.S3.Bucket + clusterImage.Spec.ExportPath + "/" + clusterImage.Spec.ExportName + "/" + normalisedImageName + ".tar'", } defaultCommands = append(defaultCommands, additionalCommands...) - } else if clusterImage.Spec.Storage == shared.STORAGE_FILE { + case shared.STORAGE_FILE: additionalCommands := []string{ "./worker export '/tmp/" + normalisedImageName + ".tar' '" + clusterImage.Spec.ExportPath + "/" + clusterImage.Spec.ExportName + "/" + normalisedImageName + ".tar'", } @@ -426,7 +427,7 @@ func (r *ClusterImageReconciler) updateClusterImageExportStatus(ctx context.Cont } clusterImageList := &raczylocomv1.ClusterImageList{} - if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImage.Namespace), client.MatchingFields{"spec.exportName": clusterImage.Spec.ExportName}); err != nil { + if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImage.Namespace), client.MatchingFields{shared.FIELD_EXPORT_NAME: clusterImage.Spec.ExportName}); err != nil { l.Error(err, "unable to list ClusterImages") return ctrl.Result{}, err } diff --git a/internal/controller/raczylo.com/clusterimageexport_controller.go b/internal/controller/raczylo.com/clusterimageexport_controller.go index c4c3330..05377e8 100644 --- a/internal/controller/raczylo.com/clusterimageexport_controller.go +++ b/internal/controller/raczylo.com/clusterimageexport_controller.go @@ -7,7 +7,6 @@ import ( "strings" "time" - "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -48,6 +47,7 @@ func (r *ClusterImageExportReconciler) InjectPodAnnotations(annotations map[stri const clusterImageExportFinalizer = "raczylo.com/clusterimageexport-finalizer" +//nolint:gocyclo // single reconcile state machine; splitting is out of scope func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { l := log.FromContext(ctx) // l.Info("Reconciling ClusterImageExport") @@ -58,7 +58,7 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R return ctrl.Result{}, client.IgnoreNotFound(err) } - if !clusterImageExport.ObjectMeta.DeletionTimestamp.IsZero() { + if !clusterImageExport.DeletionTimestamp.IsZero() { return r.handleDeletion(ctx, clusterImageExport) } @@ -99,7 +99,7 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R // Proceed with reconciliation logic // Get list of all images to be exported - fullImagesList, err := r.listImagesInCluster(ctx, l, clusterImageExport) + fullImagesList, err := r.listImagesInCluster(ctx, clusterImageExport) if err != nil { l.Error(err, "unable to list images in the cluster") return ctrl.Result{}, err @@ -201,7 +201,7 @@ func (r *ClusterImageExportReconciler) Reconcile(ctx context.Context, req ctrl.R pendingCount := 0 clusterImageList := &raczylocomv1.ClusterImageList{} if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImageExport.Namespace), - client.MatchingFields{"spec.exportName": clusterImageExport.Name}); err != nil { + client.MatchingFields{shared.FIELD_EXPORT_NAME: clusterImageExport.Name}); err != nil { l.Error(err, "unable to list ClusterImages") return ctrl.Result{}, err } @@ -288,7 +288,7 @@ func (r *ClusterImageExportReconciler) SetupWithManager(mgr ctrl.Manager) error Complete(r) } -func (r *ClusterImageExportReconciler) listImagesInCluster(ctx context.Context, l logr.Logger, clusterImageExport *raczylocomv1.ClusterImageExport) (shared.ContainersList, error) { +func (r *ClusterImageExportReconciler) listImagesInCluster(ctx context.Context, clusterImageExport *raczylocomv1.ClusterImageExport) (shared.ContainersList, error) { containersList := shared.ContainersList{} if err := shared.ListAndProcessResources[*shared.DeploymentWrapper](ctx, r.Client, &appsv1.DeploymentList{}, &containersList); err != nil { return shared.ContainersList{}, err @@ -388,7 +388,7 @@ func (r *ClusterImageExportReconciler) deleteAssociatedClusterImages(ctx context // List all ClusterImages associated with this export clusterImageList := &raczylocomv1.ClusterImageList{} if err := r.List(ctx, clusterImageList, client.InNamespace(clusterImageExport.Namespace), - client.MatchingFields{"spec.exportName": clusterImageExport.Name}); err != nil { + client.MatchingFields{shared.FIELD_EXPORT_NAME: clusterImageExport.Name}); err != nil { return fmt.Errorf("failed to list ClusterImages: %w", err) } @@ -409,15 +409,16 @@ func (r *ClusterImageExportReconciler) runCleanupJob(ctx context.Context, cluste defaultCommands := []string{} - if clusterImageExport.Spec.Storage.StorageTarget == shared.STORAGE_S3 { + switch clusterImageExport.Spec.Storage.StorageTarget { + case shared.STORAGE_S3: s3Params := shared.SetupS3Params(clusterImageExport.Spec.Storage.S3) additionalCommands := []string{ - "./worker cleanup " + strings.Join(s3Params, " ") + " 's3://" + clusterImageExport.Spec.Storage.S3.Bucket + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.ObjectMeta.Name + "/'", + "./worker cleanup " + strings.Join(s3Params, " ") + " 's3://" + clusterImageExport.Spec.Storage.S3.Bucket + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.Name + "/'", } defaultCommands = append(defaultCommands, additionalCommands...) - } else if clusterImageExport.Spec.Storage.StorageTarget == shared.STORAGE_FILE { + case shared.STORAGE_FILE: additionalCommands := []string{ - "./worker cleanup '" + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.ObjectMeta.Name + "/'", + "./worker cleanup '" + clusterImageExport.Spec.BasePath + "/" + clusterImageExport.Name + "/'", } defaultCommands = append(defaultCommands, additionalCommands...) } @@ -534,9 +535,10 @@ func (r *ClusterImageExportReconciler) cleanupByRetention(ctx context.Context, c export.Status.Progress != shared.STATUS_FAILED { continue } - if export.Status.Progress == shared.STATUS_SUCCESS { + switch export.Status.Progress { + case shared.STATUS_SUCCESS: successfulExports = append(successfulExports, export) - } else if export.Status.Progress == shared.STATUS_FAILED { + case shared.STATUS_FAILED: failedExports = append(failedExports, export) } } diff --git a/internal/controller/raczylo.com/controller_unit_test.go b/internal/controller/raczylo.com/controller_unit_test.go index 2897b35..a4ba034 100644 --- a/internal/controller/raczylo.com/controller_unit_test.go +++ b/internal/controller/raczylo.com/controller_unit_test.go @@ -24,6 +24,8 @@ import ( type TestScenario string +const testNamespace = "default" + const ( ScenarioGood TestScenario = "good" ScenarioNotGood TestScenario = "not_good" @@ -60,7 +62,7 @@ func (s *ControllerTestSuite) newFakeClient(objs ...client.Object) client.Client } // Helper to create a test ClusterImageExport -func (s *ControllerTestSuite) createClusterImageExport(name, namespace string, opts ...func(*raczylocomv1.ClusterImageExport)) *raczylocomv1.ClusterImageExport { +func (s *ControllerTestSuite) createClusterImageExport(name string, opts ...func(*raczylocomv1.ClusterImageExport)) *raczylocomv1.ClusterImageExport { export := &raczylocomv1.ClusterImageExport{ TypeMeta: metav1.TypeMeta{ APIVersion: "raczylo.com/v1", @@ -68,7 +70,7 @@ func (s *ControllerTestSuite) createClusterImageExport(name, namespace string, o }, ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: namespace, + Namespace: testNamespace, UID: "test-uid-export", }, Spec: raczylocomv1.ClusterImageExportSpec{ @@ -91,7 +93,7 @@ func (s *ControllerTestSuite) createClusterImageExport(name, namespace string, o } // Helper to create a test ClusterImage -func (s *ControllerTestSuite) createClusterImage(name, namespace, exportName string, opts ...func(*raczylocomv1.ClusterImage)) *raczylocomv1.ClusterImage { +func (s *ControllerTestSuite) createClusterImage(name, exportName string, opts ...func(*raczylocomv1.ClusterImage)) *raczylocomv1.ClusterImage { image := &raczylocomv1.ClusterImage{ TypeMeta: metav1.TypeMeta{ APIVersion: "raczylo.com/v1", @@ -99,7 +101,7 @@ func (s *ControllerTestSuite) createClusterImage(name, namespace, exportName str }, ObjectMeta: metav1.ObjectMeta{ Name: name, - Namespace: namespace, + Namespace: testNamespace, UID: "test-uid-image", }, Spec: raczylocomv1.ClusterImageSpec{ @@ -141,8 +143,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_NotFound() { func (s *ControllerTestSuite) TestClusterImageReconcile_InitialStatus_Pending() { // Scenario: Good - new ClusterImage should be set to PENDING - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export") + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export") client := s.newFakeClient(export, image) reconciler := &ClusterImageReconciler{ @@ -160,7 +162,7 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_InitialStatus_Pending() result, err := reconciler.Reconcile(s.ctx, req) require.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior // Verify status was updated to PENDING updatedImage := &raczylocomv1.ClusterImage{} @@ -171,7 +173,7 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_InitialStatus_Pending() func (s *ControllerTestSuite) TestClusterImageReconcile_MissingExport() { // Scenario: Not Good - ClusterImageExport doesn't exist - image := s.createClusterImage("test-image", "default", "non-existent-export", func(i *raczylocomv1.ClusterImage) { + image := s.createClusterImage("test-image", "non-existent-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PENDING }) @@ -196,8 +198,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MissingExport() { func (s *ControllerTestSuite) TestClusterImageReconcile_MaxParallelJobsReached() { // Scenario: Good - should requeue when max jobs reached - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PENDING }) @@ -223,8 +225,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MaxParallelJobsReached() func (s *ControllerTestSuite) TestClusterImageReconcile_SuccessStatus() { // Scenario: Good - success status should not trigger further action - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_SUCCESS }) @@ -248,8 +250,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_SuccessStatus() { func (s *ControllerTestSuite) TestClusterImageReconcile_FailedStatus() { // Scenario: Good - failed status should not trigger further action - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED }) @@ -273,8 +275,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_FailedStatus() { func (s *ControllerTestSuite) TestClusterImageReconcile_PresentStatus() { // Scenario: Good - present status should not trigger further action - export := s.createClusterImageExport("test-export", "default") - image := s.createClusterImage("test-image", "default", "test-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("test-export") + image := s.createClusterImage("test-image", "test-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PRESENT }) @@ -320,7 +322,7 @@ func (s *ControllerTestSuite) TestClusterImageExportReconcile_NotFound() { func (s *ControllerTestSuite) TestClusterImageExportReconcile_AddFinalizer() { // Scenario: Good - should add finalizer to new export - export := s.createClusterImageExport("test-export", "default") + export := s.createClusterImageExport("test-export") client := s.newFakeClient(export) reconciler := &ClusterImageExportReconciler{ @@ -347,7 +349,7 @@ func (s *ControllerTestSuite) TestClusterImageExportReconcile_AddFinalizer() { func (s *ControllerTestSuite) TestClusterImageExportReconcile_AddCreationTimestamp() { // Scenario: Good - should add creation timestamp annotation - export := s.createClusterImageExport("test-export", "default") + export := s.createClusterImageExport("test-export") client := s.newFakeClient(export) reconciler := &ClusterImageExportReconciler{ @@ -405,8 +407,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioGood, Description: "New ClusterImage should be initialized to PENDING", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - export := s.createClusterImageExport("export-1", "default") - image := s.createClusterImage("image-1", "default", "export-1") + export := s.createClusterImageExport("export-1") + image := s.createClusterImage("image-1", "export-1") c := s.newFakeClient(export, image) r := &ClusterImageReconciler{Client: c, Scheme: s.scheme, MaxParallelJobs: 5} req := reconcile.Request{NamespacedName: types.NamespacedName{Name: "image-1", Namespace: "default"}} @@ -420,7 +422,7 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioNotGood, Description: "ClusterImage with missing export should error", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - image := s.createClusterImage("orphan-image", "default", "missing-export", func(i *raczylocomv1.ClusterImage) { + image := s.createClusterImage("orphan-image", "missing-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_PENDING }) c := s.newFakeClient(image) @@ -435,8 +437,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioGood, Description: "ClusterImage with SUCCESS status should not trigger action", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - export := s.createClusterImageExport("export-2", "default") - image := s.createClusterImage("success-image", "default", "export-2", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("export-2") + image := s.createClusterImage("success-image", "export-2", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_SUCCESS }) c := s.newFakeClient(export, image) @@ -452,8 +454,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { Scenario: ScenarioNotGood, Description: "ClusterImage with FAILED status should not trigger action", SetupFunc: func(s *ControllerTestSuite) (client.Client, *ClusterImageReconciler, reconcile.Request) { - export := s.createClusterImageExport("export-3", "default") - image := s.createClusterImage("failed-image", "default", "export-3", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("export-3") + image := s.createClusterImage("failed-image", "export-3", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED }) c := s.newFakeClient(export, image) @@ -507,8 +509,8 @@ func (s *ControllerTestSuite) TestClusterImageReconcile_MatrixScenarios() { func (s *ControllerTestSuite) TestClusterImage_ConcurrentUpdates() { // Scenario: Good - simulate concurrent reconciliation on PENDING status // This test verifies that multiple reconciliations don't corrupt state - export := s.createClusterImageExport("concurrent-export", "default") - image := s.createClusterImage("concurrent-image", "default", "concurrent-export") + export := s.createClusterImageExport("concurrent-export") + image := s.createClusterImage("concurrent-image", "concurrent-export") fakeClient := s.newFakeClient(export, image) reconciler := &ClusterImageReconciler{ @@ -527,7 +529,7 @@ func (s *ControllerTestSuite) TestClusterImage_ConcurrentUpdates() { // First reconciliation should set status to PENDING result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior // Verify status was set finalImage := &raczylocomv1.ClusterImage{} @@ -578,7 +580,7 @@ func (s *ControllerTestSuite) TestClusterImage_ActiveJobsMutex() { func (s *ControllerTestSuite) TestClusterImageExport_WithDeletionTimestamp() { // Scenario: Good - export being deleted should trigger cleanup - export := s.createClusterImageExport("deleting-export", "default") + export := s.createClusterImageExport("deleting-export") now := metav1.Now() export.DeletionTimestamp = &now export.Finalizers = []string{clusterImageExportFinalizer} @@ -607,8 +609,8 @@ func (s *ControllerTestSuite) TestClusterImageExport_WithDeletionTimestamp() { func (s *ControllerTestSuite) TestClusterImage_SHAPinnedImages() { // Scenario: Good - SHA-pinned images should be handled correctly - export := s.createClusterImageExport("sha-export", "default") - image := s.createClusterImage("sha-image", "default", "sha-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("sha-export") + image := s.createClusterImage("sha-image", "sha-export", func(i *raczylocomv1.ClusterImage) { i.Spec.Image = "quay.io/cilium/cilium" i.Spec.Tag = "v1.18.4" i.Spec.Sha = "sha256:49d87af187eeeb9e9e3ec2bc6bd372261a0b5cb2d845659463ba7cc10fe9e45f" @@ -631,7 +633,7 @@ func (s *ControllerTestSuite) TestClusterImage_SHAPinnedImages() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { @@ -647,11 +649,12 @@ func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { {"quay-image", "quay.io/coreos/etcd", "quay.io/coreos/etcd:v3.5.0"}, } - export := s.createClusterImageExport("multi-registry-export", "default") - objs := []client.Object{export} + export := s.createClusterImageExport("multi-registry-export") + objs := make([]client.Object, 0, 1+len(registries)) + objs = append(objs, export) for _, reg := range registries { - img := s.createClusterImage(reg.name, "default", "multi-registry-export", func(i *raczylocomv1.ClusterImage) { + img := s.createClusterImage(reg.name, "multi-registry-export", func(i *raczylocomv1.ClusterImage) { i.Spec.Image = reg.image i.Spec.FullName = reg.fullName }) @@ -675,7 +678,7 @@ func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err, "Failed for registry: %s", reg.name) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } } @@ -683,7 +686,7 @@ func (s *ControllerTestSuite) TestClusterImage_MultipleRegistries() { func (s *ControllerTestSuite) TestClusterImageExport_S3Storage() { // Scenario: Good - S3 storage configuration - export := s.createClusterImageExport("s3-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("s3-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Storage = raczylocomv1.ClusterImageStorageSpec{ StorageTarget: shared.STORAGE_S3, S3: raczylocomv1.ClusterImageStorageS3{ @@ -721,7 +724,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_S3Storage() { func (s *ControllerTestSuite) TestClusterImageExport_FileStorage() { // Scenario: Good - File storage configuration - export := s.createClusterImageExport("file-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("file-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Storage = raczylocomv1.ClusterImageStorageSpec{ StorageTarget: shared.STORAGE_FILE, } @@ -749,7 +752,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_FileStorage() { func (s *ControllerTestSuite) TestClusterImage_EmptySpec() { // Scenario: Really Bad - empty spec should be handled - export := s.createClusterImageExport("empty-spec-export", "default") + export := s.createClusterImageExport("empty-spec-export") image := &raczylocomv1.ClusterImage{ TypeMeta: metav1.TypeMeta{ APIVersion: "raczylo.com/v1", @@ -782,12 +785,12 @@ func (s *ControllerTestSuite) TestClusterImage_EmptySpec() { result, err := reconciler.Reconcile(s.ctx, req) // Should not error, just set to pending assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } func (s *ControllerTestSuite) TestClusterImageExport_EmptyNamespaces() { // Scenario: Good - export with no namespace filters should process all - export := s.createClusterImageExport("all-ns-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("all-ns-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Namespaces = []string{} e.Spec.ExcludedNamespaces = []string{} }) @@ -811,7 +814,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_EmptyNamespaces() { func (s *ControllerTestSuite) TestClusterImageExport_WithAdditionalImages() { // Scenario: Good - export with additional images specified - export := s.createClusterImageExport("additional-images-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("additional-images-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.AdditionalImages = []string{ "nginx:1.21", "redis:7.0", @@ -838,7 +841,7 @@ func (s *ControllerTestSuite) TestClusterImageExport_WithAdditionalImages() { func (s *ControllerTestSuite) TestClusterImageExport_WithIncludesExcludes() { // Scenario: Good - export with include/exclude filters - export := s.createClusterImageExport("filtered-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("filtered-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.Includes = []string{"nginx", "redis"} e.Spec.Excludes = []string{"test", "dev"} e.Spec.Namespaces = []string{"production", "staging"} @@ -864,8 +867,8 @@ func (s *ControllerTestSuite) TestClusterImageExport_WithIncludesExcludes() { func (s *ControllerTestSuite) TestClusterImage_WithImagePullSecrets() { // Scenario: Good - image with pull secrets should work - export := s.createClusterImageExport("secret-export", "default") - image := s.createClusterImage("secret-image", "default", "secret-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("secret-export") + image := s.createClusterImage("secret-image", "secret-export", func(i *raczylocomv1.ClusterImage) { i.Spec.ImagePullSecrets = []corev1.LocalObjectReference{ {Name: "docker-registry-secret"}, {Name: "gcr-json-key"}, @@ -888,17 +891,17 @@ func (s *ControllerTestSuite) TestClusterImage_WithImagePullSecrets() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } func (s *ControllerTestSuite) TestClusterImage_WithJobAnnotations() { // Scenario: Good - image with job annotations - export := s.createClusterImageExport("annotated-export", "default", func(e *raczylocomv1.ClusterImageExport) { + export := s.createClusterImageExport("annotated-export", func(e *raczylocomv1.ClusterImageExport) { e.Spec.JobAnnotations = map[string]string{ "iam.amazonaws.com/role": "arn:aws:iam::123456789:role/BackupRole", } }) - image := s.createClusterImage("annotated-image", "default", "annotated-export", func(i *raczylocomv1.ClusterImage) { + image := s.createClusterImage("annotated-image", "annotated-export", func(i *raczylocomv1.ClusterImage) { i.Spec.JobAnnotations = map[string]string{ "custom/annotation": "value", } @@ -920,7 +923,7 @@ func (s *ControllerTestSuite) TestClusterImage_WithJobAnnotations() { result, err := reconciler.Reconcile(s.ctx, req) assert.NoError(s.T(), err) - assert.True(s.T(), result.Requeue) //lint:ignore SA1019 testing controller's actual behavior + assert.True(s.T(), result.Requeue) //nolint:staticcheck // testing controller's actual behavior } // ==================== Job Status Tests ==================== @@ -930,8 +933,8 @@ func (s *ControllerTestSuite) TestClusterImage_SuccessfulJobCompletion() { // Note: Full integration testing requires envtest for KubeClient // This test validates the basic state machine transitions - export := s.createClusterImageExport("job-success-export", "default") - image := s.createClusterImage("job-success-image", "default", "job-success-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("job-success-export") + image := s.createClusterImage("job-success-image", "job-success-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_SUCCESS // Already completed }) @@ -963,8 +966,8 @@ func (s *ControllerTestSuite) TestClusterImage_SuccessfulJobCompletion() { func (s *ControllerTestSuite) TestClusterImage_RetryCount() { // Scenario: Good - verify retry count is tracked properly - export := s.createClusterImageExport("retry-export", "default") - image := s.createClusterImage("retry-image", "default", "retry-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("retry-export") + image := s.createClusterImage("retry-image", "retry-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED // Terminal state i.Status.RetryCount = 2 }) @@ -997,8 +1000,8 @@ func (s *ControllerTestSuite) TestClusterImage_RetryCount() { func (s *ControllerTestSuite) TestClusterImage_MaxRetriesReached() { // Scenario: Not Good - max retries reached and FAILED status - export := s.createClusterImageExport("max-retry-export", "default") - image := s.createClusterImage("max-retry-image", "default", "max-retry-export", func(i *raczylocomv1.ClusterImage) { + export := s.createClusterImageExport("max-retry-export") + image := s.createClusterImage("max-retry-image", "max-retry-export", func(i *raczylocomv1.ClusterImage) { i.Status.Progress = shared.STATUS_FAILED i.Status.RetryCount = 3 // Max retries reached }) diff --git a/internal/shared/definitions.go b/internal/shared/definitions.go index 6ed569c..42b23b7 100644 --- a/internal/shared/definitions.go +++ b/internal/shared/definitions.go @@ -29,6 +29,11 @@ const ( // STORAGE DEFINITIONS STORAGE_S3 = "S3" STORAGE_FILE = "FILE" + + FIELD_EXPORT_NAME = "spec.exportName" + DEFAULT_IMAGE_TAG = "latest" + LABEL_APP = "app" + EXPORTER_CONTAINER = "exporter" ) type Container struct { diff --git a/internal/shared/jobs.go b/internal/shared/jobs.go index 925580b..7da1c93 100644 --- a/internal/shared/jobs.go +++ b/internal/shared/jobs.go @@ -59,7 +59,7 @@ func CreateJob[T any](params JobParams, setupFunc func(T) []string) *batchv1.Job Namespace: params.Namespace, OwnerReferences: params.OwnerReferences, Labels: map[string]string{ - "app": "image-export", + LABEL_APP: "image-export", }, Annotations: params.Annotations, }, @@ -69,7 +69,7 @@ func CreateJob[T any](params JobParams, setupFunc func(T) []string) *batchv1.Job Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - "app": "image-export", + LABEL_APP: "image-export", }, Annotations: params.Annotations, }, @@ -80,7 +80,7 @@ func CreateJob[T any](params JobParams, setupFunc func(T) []string) *batchv1.Job Volumes: volumes, Containers: []corev1.Container{ { - Name: "exporter", + Name: EXPORTER_CONTAINER, Image: params.Image, TTY: true, Command: []string{}, diff --git a/internal/shared/jobs_test.go b/internal/shared/jobs_test.go index e9e9331..b7ebe06 100644 --- a/internal/shared/jobs_test.go +++ b/internal/shared/jobs_test.go @@ -204,18 +204,18 @@ func (s *JobsTestSuite) TestCreateJob() { require.NotNil(s.T(), job, "Job should not be nil") // Verify job metadata - assert.Equal(s.T(), tc.expectJobName, job.ObjectMeta.Name) - assert.Equal(s.T(), tc.expectNamespace, job.ObjectMeta.Namespace) + assert.Equal(s.T(), tc.expectJobName, job.Name) + assert.Equal(s.T(), tc.expectNamespace, job.Namespace) // Verify labels - assert.Equal(s.T(), "image-export", job.ObjectMeta.Labels["app"]) - assert.Equal(s.T(), "image-export", job.Spec.Template.ObjectMeta.Labels["app"]) + assert.Equal(s.T(), "image-export", job.Labels["app"]) + assert.Equal(s.T(), "image-export", job.Spec.Template.Labels["app"]) // Verify annotations if expected if tc.expectAnnotations != nil { for k, v := range tc.expectAnnotations { - assert.Equal(s.T(), v, job.ObjectMeta.Annotations[k]) - assert.Equal(s.T(), v, job.Spec.Template.ObjectMeta.Annotations[k]) + assert.Equal(s.T(), v, job.Annotations[k]) + assert.Equal(s.T(), v, job.Spec.Template.Annotations[k]) } } @@ -268,9 +268,9 @@ func (s *JobsTestSuite) TestCreateJobWithOwnerReferences() { job := CreateJob(params, func(raczylocomv1.ClusterImageExport) []string { return nil }) - require.Len(s.T(), job.ObjectMeta.OwnerReferences, 1) - assert.Equal(s.T(), "ClusterImage", job.ObjectMeta.OwnerReferences[0].Kind) - assert.Equal(s.T(), "test-image", job.ObjectMeta.OwnerReferences[0].Name) + require.Len(s.T(), job.OwnerReferences, 1) + assert.Equal(s.T(), "ClusterImage", job.OwnerReferences[0].Kind) + assert.Equal(s.T(), "test-image", job.OwnerReferences[0].Name) } // TestCreateJobCommands tests command concatenation @@ -525,7 +525,7 @@ func (s *JobsTestSuite) TestSecretVolumeMounting() { // Verify volumes are created require.Len(s.T(), podSpec.Volumes, 3) for i, vol := range podSpec.Volumes { - assert.Equal(s.T(), secrets[i].Name, vol.VolumeSource.Secret.SecretName) + assert.Equal(s.T(), secrets[i].Name, vol.Secret.SecretName) assert.Contains(s.T(), vol.Name, "secret-") } diff --git a/internal/shared/k8s.go b/internal/shared/k8s.go index 51c22eb..cbe8749 100644 --- a/internal/shared/k8s.go +++ b/internal/shared/k8s.go @@ -84,7 +84,7 @@ func ProcessContainerName(containerName string) (Container, error) { } if cnt.Sha == "" && cnt.Tag == "" { - cnt.Tag = "latest" + cnt.Tag = DEFAULT_IMAGE_TAG } if cnt.Image == "" { @@ -109,7 +109,7 @@ func processContainers(ctx context.Context, resource K8sResource, namespace stri } for _, container := range podSpec.EphemeralContainers { - if err := processContainer(ctx, container.EphemeralContainerCommon.Image, namespace, containersList); err != nil { + if err := processContainer(ctx, container.Image, namespace, containersList); err != nil { return err } } @@ -211,7 +211,7 @@ func SetupIndexers(mgr manager.Manager) error { Jitter: 0.1, Steps: 5, }, func() (bool, error) { - err := mgr.GetFieldIndexer().IndexField(context.Background(), &raczylocomv1.ClusterImage{}, "spec.exportName", func(rawObj client.Object) []string { + err := mgr.GetFieldIndexer().IndexField(context.Background(), &raczylocomv1.ClusterImage{}, FIELD_EXPORT_NAME, func(rawObj client.Object) []string { clusterImage := rawObj.(*raczylocomv1.ClusterImage) return []string{clusterImage.Spec.ExportName} })