From ac8ec2e32ad7508a8799b0e3dbbe0b1f8e2f473a Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 12 Aug 2025 12:50:25 +0300 Subject: [PATCH 1/2] Refactor bucket pkg structure Signed-off-by: Stefan Prodan --- {pkg => internal/bucket}/azure/blob.go | 0 {pkg => internal/bucket}/azure/blob_integration_test.go | 0 {pkg => internal/bucket}/azure/blob_test.go | 0 {pkg => internal/bucket}/gcp/gcp.go | 0 {pkg => internal/bucket}/gcp/gcp_test.go | 0 {pkg => internal/bucket}/minio/minio.go | 0 {pkg => internal/bucket}/minio/minio_test.go | 2 +- internal/controller/bucket_controller.go | 6 +++--- 8 files changed, 4 insertions(+), 4 deletions(-) rename {pkg => internal/bucket}/azure/blob.go (100%) rename {pkg => internal/bucket}/azure/blob_integration_test.go (100%) rename {pkg => internal/bucket}/azure/blob_test.go (100%) rename {pkg => internal/bucket}/gcp/gcp.go (100%) rename {pkg => internal/bucket}/gcp/gcp_test.go (100%) rename {pkg => internal/bucket}/minio/minio.go (100%) rename {pkg => internal/bucket}/minio/minio_test.go (99%) diff --git a/pkg/azure/blob.go b/internal/bucket/azure/blob.go similarity index 100% rename from pkg/azure/blob.go rename to internal/bucket/azure/blob.go diff --git a/pkg/azure/blob_integration_test.go b/internal/bucket/azure/blob_integration_test.go similarity index 100% rename from pkg/azure/blob_integration_test.go rename to internal/bucket/azure/blob_integration_test.go diff --git a/pkg/azure/blob_test.go b/internal/bucket/azure/blob_test.go similarity index 100% rename from pkg/azure/blob_test.go rename to internal/bucket/azure/blob_test.go diff --git a/pkg/gcp/gcp.go b/internal/bucket/gcp/gcp.go similarity index 100% rename from pkg/gcp/gcp.go rename to internal/bucket/gcp/gcp.go diff --git a/pkg/gcp/gcp_test.go b/internal/bucket/gcp/gcp_test.go similarity index 100% rename from pkg/gcp/gcp_test.go rename to internal/bucket/gcp/gcp_test.go diff --git a/pkg/minio/minio.go b/internal/bucket/minio/minio.go similarity index 100% rename from pkg/minio/minio.go rename to internal/bucket/minio/minio.go diff --git a/pkg/minio/minio_test.go b/internal/bucket/minio/minio_test.go similarity index 99% rename from pkg/minio/minio_test.go rename to internal/bucket/minio/minio_test.go index 596e61810..abb5eee5b 100644 --- a/pkg/minio/minio_test.go +++ b/internal/bucket/minio/minio_test.go @@ -817,7 +817,7 @@ func getObjectFile() string { } func loadServerCertAndClientTLSConfig() (serverCert string, serverKey string, clientConf *tls.Config, err error) { - const certsDir = "../../internal/controller/testdata/certs" + const certsDir = "../../controller/testdata/certs" clientConf = &tls.Config{} serverCert, err = filepath.Abs(filepath.Join(certsDir, "server.pem")) diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index f11078935..7603bba92 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -56,14 +56,14 @@ import ( "github.com/fluxcd/pkg/sourceignore" sourcev1 "github.com/fluxcd/source-controller/api/v1" + "github.com/fluxcd/source-controller/internal/bucket/azure" + "github.com/fluxcd/source-controller/internal/bucket/gcp" + "github.com/fluxcd/source-controller/internal/bucket/minio" intdigest "github.com/fluxcd/source-controller/internal/digest" serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/index" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" - "github.com/fluxcd/source-controller/pkg/azure" - "github.com/fluxcd/source-controller/pkg/gcp" - "github.com/fluxcd/source-controller/pkg/minio" ) // maxConcurrentBucketFetches is the upper bound on the goroutines used to From 9e789f6d9a303d117c7839f8875eda6993e5fbb6 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Tue, 12 Aug 2025 14:08:10 +0300 Subject: [PATCH 2/2] Extract storage operations to a dedicated package Signed-off-by: Stefan Prodan --- Dockerfile | 1 - go.mod | 6 +- go.sum | 12 +- internal/controller/bucket_controller.go | 3 +- internal/controller/bucket_controller_test.go | 13 +- .../controller/gitrepository_controller.go | 5 +- .../gitrepository_controller_fuzz_test.go | 5 +- .../gitrepository_controller_test.go | 13 +- internal/controller/helmchart_controller.go | 3 +- .../controller/helmchart_controller_test.go | 23 +- .../controller/helmrepository_controller.go | 3 +- .../helmrepository_controller_test.go | 13 +- .../controller/ocirepository_controller.go | 5 +- .../ocirepository_controller_test.go | 13 +- internal/controller/suite_test.go | 9 +- internal/fs/LICENSE | 27 - internal/fs/fs.go | 345 ---------- internal/fs/fs_test.go | 590 ------------------ internal/fs/rename.go | 31 - internal/fs/rename_windows.go | 42 -- internal/fs/testdata/symlinks/dir-symlink | 1 - internal/fs/testdata/symlinks/file-symlink | 1 - internal/fs/testdata/symlinks/invalid-symlink | 1 - .../fs/testdata/symlinks/windows-file-symlink | 1 - internal/fs/testdata/test.file | 0 internal/helm/chart/builder.go | 4 +- internal/helm/chart/builder_remote.go | 4 +- internal/{controller => storage}/storage.go | 18 +- .../{controller => storage}/storage_test.go | 41 +- main.go | 10 +- 30 files changed, 116 insertions(+), 1127 deletions(-) delete mode 100644 internal/fs/LICENSE delete mode 100644 internal/fs/fs.go delete mode 100644 internal/fs/fs_test.go delete mode 100644 internal/fs/rename.go delete mode 100644 internal/fs/rename_windows.go delete mode 120000 internal/fs/testdata/symlinks/dir-symlink delete mode 120000 internal/fs/testdata/symlinks/file-symlink delete mode 120000 internal/fs/testdata/symlinks/invalid-symlink delete mode 120000 internal/fs/testdata/symlinks/windows-file-symlink delete mode 100644 internal/fs/testdata/test.file rename internal/{controller => storage}/storage.go (97%) rename internal/{controller => storage}/storage_test.go (96%) diff --git a/Dockerfile b/Dockerfile index cfa615b3b..04488f5c5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -26,7 +26,6 @@ RUN go mod download # Copy source code COPY main.go main.go -COPY pkg/ pkg/ COPY internal/ internal/ ARG TARGETPLATFORM diff --git a/go.mod b/go.mod index bc88ffc47..1666141ee 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/Masterminds/semver/v3 v3.3.1 github.com/cyphar/filepath-securejoin v0.4.1 github.com/distribution/distribution/v3 v3.0.0 - github.com/docker/cli v28.3.2+incompatible + github.com/docker/cli v28.3.3+incompatible github.com/docker/go-units v0.5.0 github.com/elazarl/goproxy v1.7.2 github.com/fluxcd/cli-utils v0.36.0-flux.14 @@ -37,7 +37,7 @@ require ( github.com/fluxcd/pkg/http/transport v0.6.0 github.com/fluxcd/pkg/lockedfile v0.6.0 github.com/fluxcd/pkg/masktoken v0.7.0 - github.com/fluxcd/pkg/oci v0.51.0 + github.com/fluxcd/pkg/oci v0.52.0 github.com/fluxcd/pkg/runtime v0.78.0 github.com/fluxcd/pkg/sourceignore v0.13.0 github.com/fluxcd/pkg/ssh v0.20.0 @@ -182,7 +182,7 @@ require ( github.com/dimchansky/utfbom v1.1.1 // indirect github.com/distribution/reference v0.6.0 // indirect github.com/docker/distribution v2.8.3+incompatible // indirect - github.com/docker/docker v28.2.2+incompatible // indirect + github.com/docker/docker v28.3.3+incompatible // indirect github.com/docker/docker-credential-helpers v0.9.3 // indirect github.com/docker/go-connections v0.5.0 // indirect github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect diff --git a/go.sum b/go.sum index 214ed612e..06ce446e2 100644 --- a/go.sum +++ b/go.sum @@ -321,12 +321,12 @@ github.com/distribution/distribution/v3 v3.0.0 h1:q4R8wemdRQDClzoNNStftB2ZAfqOiN github.com/distribution/distribution/v3 v3.0.0/go.mod h1:tRNuFoZsUdyRVegq8xGNeds4KLjwLCRin/tTo6i1DhU= github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk= github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E= -github.com/docker/cli v28.3.2+incompatible h1:mOt9fcLE7zaACbxW1GeS65RI67wIJrTnqS3hP2huFsY= -github.com/docker/cli v28.3.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= +github.com/docker/cli v28.3.3+incompatible h1:fp9ZHAr1WWPGdIWBM1b3zLtgCF+83gRdVMTJsUeiyAo= +github.com/docker/cli v28.3.3+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/distribution v2.8.3+incompatible h1:AtKxIZ36LoNK51+Z6RpzLpddBirtxJnzDrHLEKxTAYk= github.com/docker/distribution v2.8.3+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= -github.com/docker/docker v28.2.2+incompatible h1:CjwRSksz8Yo4+RmQ339Dp/D2tGO5JxwYeqtMOEe0LDw= -github.com/docker/docker v28.2.2+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/docker v28.3.3+incompatible h1:Dypm25kh4rmk49v1eiVbsAtpAsYURjYkaKubwuBdxEI= +github.com/docker/docker v28.3.3+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/docker-credential-helpers v0.9.3 h1:gAm/VtF9wgqJMoxzT3Gj5p4AqIjCBS4wrsOh9yRqcz8= github.com/docker/docker-credential-helpers v0.9.3/go.mod h1:x+4Gbw9aGmChi3qTLZj8Dfn0TD20M/fuWy0E5+WDeCo= github.com/docker/go-connections v0.5.0 h1:USnMq7hx7gwdVZq1L49hLXaFtUdTADjXGp+uj1Br63c= @@ -396,8 +396,8 @@ github.com/fluxcd/pkg/lockedfile v0.6.0 h1:64RRMiPv3ZK9Y4sjI8c78kZAdfEo+Sjr2iP8a github.com/fluxcd/pkg/lockedfile v0.6.0/go.mod h1:gpdUVm7+05NIT1ZvzuNnHfnT81OhZtIySlxxkZ68pXk= github.com/fluxcd/pkg/masktoken v0.7.0 h1:pitmyOg2pUVdW+nn2Lk/xqm2TaA08uxvOC0ns3sz6bM= github.com/fluxcd/pkg/masktoken v0.7.0/go.mod h1:Lc1uoDjO1GY6+YdkK+ZqqBIBWquyV58nlSJ5S1N1IYU= -github.com/fluxcd/pkg/oci v0.51.0 h1:9oYnm+T4SCVSBif9gn80ALJkMGSERabVMDJiaMIdr7Y= -github.com/fluxcd/pkg/oci v0.51.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4= +github.com/fluxcd/pkg/oci v0.52.0 h1:rkHMtXYm21MtDrjNcR5KScqOe6C1JHPExoShuVdNm8M= +github.com/fluxcd/pkg/oci v0.52.0/go.mod h1:5J6IhHoDVYCVeBEC+4E3nPeKh7d0kjJ8IEL6NVCiTx4= github.com/fluxcd/pkg/runtime v0.78.0 h1:xwNZqnazmgURGuLiHDbzST6BI5K9fvZuNS4eMVY35Es= github.com/fluxcd/pkg/runtime v0.78.0/go.mod h1:iGhdaEq+lMJQTJNAFEPOU4gUJ7kt3yeDcJPZy7O9IUw= github.com/fluxcd/pkg/sourceignore v0.13.0 h1:ZvkzX2WsmyZK9cjlqOFFW1onHVzhPZIqDbCh96rPqbU= diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index 7603bba92..c3cf55b84 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -64,6 +64,7 @@ import ( "github.com/fluxcd/source-controller/internal/index" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" ) // maxConcurrentBucketFetches is the upper bound on the goroutines used to @@ -127,7 +128,7 @@ type BucketReconciler struct { kuberecorder.EventRecorder helper.Metrics - Storage *Storage + Storage *storage.Storage ControllerName string TokenCache *cache.TokenCache diff --git a/internal/controller/bucket_controller_test.go b/internal/controller/bucket_controller_test.go index dc4698a89..f3406f28e 100644 --- a/internal/controller/bucket_controller_test.go +++ b/internal/controller/bucket_controller_test.go @@ -50,6 +50,7 @@ import ( s3mock "github.com/fluxcd/source-controller/internal/mock/s3" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" ) // Environment variable to set the GCP Storage host for the GCP client. @@ -196,7 +197,7 @@ func TestBucketReconciler_Reconcile(t *testing.T) { func TestBucketReconciler_reconcileStorage(t *testing.T) { tests := []struct { name string - beforeFunc func(obj *sourcev1.Bucket, storage *Storage) error + beforeFunc func(obj *sourcev1.Bucket, storage *storage.Storage) error want sreconcile.Result wantErr bool assertArtifact *sourcev1.Artifact @@ -205,7 +206,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { }{ { name: "garbage collects", - beforeFunc: func(obj *sourcev1.Bucket, storage *Storage) error { + beforeFunc: func(obj *sourcev1.Bucket, storage *storage.Storage) error { revisions := []string{"a", "b", "c", "d"} for n := range revisions { v := revisions[n] @@ -255,7 +256,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { }, { name: "notices missing artifact in storage", - beforeFunc: func(obj *sourcev1.Bucket, storage *Storage) error { + beforeFunc: func(obj *sourcev1.Bucket, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/invalid.txt", Revision: "d", @@ -274,7 +275,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { }, { name: "notices empty artifact digest", - beforeFunc: func(obj *sourcev1.Bucket, storage *Storage) error { + beforeFunc: func(obj *sourcev1.Bucket, storage *storage.Storage) error { f := "empty-digest.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -305,7 +306,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { }, { name: "notices artifact digest mismatch", - beforeFunc: func(obj *sourcev1.Bucket, storage *Storage) error { + beforeFunc: func(obj *sourcev1.Bucket, storage *storage.Storage) error { f := "digest-mismatch.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -336,7 +337,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) { }, { name: "updates hostname on diff from current", - beforeFunc: func(obj *sourcev1.Bucket, storage *Storage) error { + beforeFunc: func(obj *sourcev1.Bucket, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/hostname.txt", Revision: "f", diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 379bf8a1f..7d4efc4f2 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -66,6 +66,7 @@ import ( "github.com/fluxcd/source-controller/internal/features" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" "github.com/fluxcd/source-controller/internal/util" ) @@ -131,7 +132,7 @@ type GitRepositoryReconciler struct { kuberecorder.EventRecorder helper.Metrics - Storage *Storage + Storage *storage.Storage ControllerName string TokenCache *cache.TokenCache @@ -868,7 +869,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat } // Archive directory to storage - if err := r.Storage.Archive(&artifact, dir, SourceIgnoreFilter(ps, ignoreDomain)); err != nil { + if err := r.Storage.Archive(&artifact, dir, storage.SourceIgnoreFilter(ps, ignoreDomain)); err != nil { e := serror.NewGeneric( fmt.Errorf("unable to archive artifact to storage: %w", err), sourcev1.ArchiveOperationFailedReason, diff --git a/internal/controller/gitrepository_controller_fuzz_test.go b/internal/controller/gitrepository_controller_fuzz_test.go index 1751d096e..d87a8f68b 100644 --- a/internal/controller/gitrepository_controller_fuzz_test.go +++ b/internal/controller/gitrepository_controller_fuzz_test.go @@ -64,6 +64,7 @@ import ( "github.com/fluxcd/pkg/runtime/testenv" sourcev1 "github.com/fluxcd/source-controller/api/v1" + intstorage "github.com/fluxcd/source-controller/internal/storage" ) var ( @@ -77,7 +78,7 @@ var ( cfg *rest.Config testEnv *testenv.Environment - storage *Storage + storage *intstorage.Storage examplePublicKey []byte examplePrivateKey []byte @@ -477,7 +478,7 @@ func startEnvServer(setupReconcilers func(manager.Manager)) *envtest.Environment panic(err) } defer os.RemoveAll(tmpStoragePath) - storage, err = NewStorage(tmpStoragePath, "localhost:5050", time.Minute*1, 2) + storage, err = intstorage.New(tmpStoragePath, "localhost:5050", time.Minute*1, 2) if err != nil { panic(err) } diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index 73c00a8e8..e4f473c91 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -63,6 +63,7 @@ import ( "github.com/fluxcd/source-controller/internal/features" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" ) const ( @@ -1531,7 +1532,7 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) { func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { tests := []struct { name string - beforeFunc func(obj *sourcev1.GitRepository, storage *Storage) error + beforeFunc func(obj *sourcev1.GitRepository, storage *storage.Storage) error want sreconcile.Result wantErr bool assertArtifact *sourcev1.Artifact @@ -1540,7 +1541,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { }{ { name: "garbage collects", - beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.GitRepository, storage *storage.Storage) error { revisions := []string{"a", "b", "c", "d"} for n := range revisions { v := revisions[n] @@ -1590,7 +1591,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "notices missing artifact in storage", - beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.GitRepository, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/invalid.txt", Revision: "e", @@ -1609,7 +1610,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "notices empty artifact digest", - beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.GitRepository, storage *storage.Storage) error { f := "empty-digest.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -1640,7 +1641,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "notices artifact digest mismatch", - beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.GitRepository, storage *storage.Storage) error { f := "digest-mismatch.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -1671,7 +1672,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "updates hostname on diff from current", - beforeFunc: func(obj *sourcev1.GitRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.GitRepository, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/hostname.txt", Revision: "f", diff --git a/internal/controller/helmchart_controller.go b/internal/controller/helmchart_controller.go index 19d320ecf..6559a2528 100644 --- a/internal/controller/helmchart_controller.go +++ b/internal/controller/helmchart_controller.go @@ -75,6 +75,7 @@ import ( "github.com/fluxcd/source-controller/internal/oci/notation" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" "github.com/fluxcd/source-controller/internal/util" ) @@ -132,7 +133,7 @@ type HelmChartReconciler struct { helper.Metrics RegistryClientGenerator RegistryClientGeneratorFunc - Storage *Storage + Storage *storage.Storage Getters helmgetter.Providers ControllerName string diff --git a/internal/controller/helmchart_controller_test.go b/internal/controller/helmchart_controller_test.go index 8bfa91657..dff6042c4 100644 --- a/internal/controller/helmchart_controller_test.go +++ b/internal/controller/helmchart_controller_test.go @@ -75,6 +75,7 @@ import ( snotation "github.com/fluxcd/source-controller/internal/oci/notation" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" ) func TestHelmChartReconciler_deleteBeforeFinalizer(t *testing.T) { @@ -330,7 +331,7 @@ func TestHelmChartReconciler_Reconcile(t *testing.T) { func TestHelmChartReconciler_reconcileStorage(t *testing.T) { tests := []struct { name string - beforeFunc func(obj *sourcev1.HelmChart, storage *Storage) error + beforeFunc func(obj *sourcev1.HelmChart, storage *storage.Storage) error want sreconcile.Result wantErr bool assertArtifact *sourcev1.Artifact @@ -339,7 +340,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { }{ { name: "garbage collects", - beforeFunc: func(obj *sourcev1.HelmChart, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmChart, storage *storage.Storage) error { revisions := []string{"a", "b", "c", "d"} for n := range revisions { v := revisions[n] @@ -389,7 +390,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { }, { name: "notices missing artifact in storage", - beforeFunc: func(obj *sourcev1.HelmChart, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmChart, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/invalid.txt", Revision: "d", @@ -408,7 +409,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { }, { name: "notices empty artifact digest", - beforeFunc: func(obj *sourcev1.HelmChart, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmChart, storage *storage.Storage) error { f := "empty-digest.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -439,7 +440,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { }, { name: "notices artifact digest mismatch", - beforeFunc: func(obj *sourcev1.HelmChart, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmChart, storage *storage.Storage) error { f := "digest-mismatch.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -470,7 +471,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) { }, { name: "updates hostname on diff from current", - beforeFunc: func(obj *sourcev1.HelmChart, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmChart, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/hostname.txt", Revision: "f", @@ -568,7 +569,7 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) { tmpDir := t.TempDir() - storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords) + storage, err := storage.New(tmpDir, "example.com", retentionTTL, retentionRecords) g.Expect(err).ToNot(HaveOccurred()) gitArtifact := &sourcev1.Artifact{ @@ -1185,7 +1186,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) { metadata, err := loadTestChartToOCI(chartData, testRegistryServer, "", "", "") g.Expect(err).NotTo(HaveOccurred()) - storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords) + storage, err := storage.New(tmpDir, "example.com", retentionTTL, retentionRecords) g.Expect(err).ToNot(HaveOccurred()) cachedArtifact := &sourcev1.Artifact{ @@ -1408,7 +1409,7 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) { tmpDir := t.TempDir() - storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords) + storage, err := storage.New(tmpDir, "example.com", retentionTTL, retentionRecords) g.Expect(err).ToNot(HaveOccurred()) chartsArtifact := &sourcev1.Artifact{ @@ -2884,7 +2885,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignatureNotation(t *t metadata, err := loadTestChartToOCI(chartData, server, "", "", "") g.Expect(err).NotTo(HaveOccurred()) - storage, err := NewStorage(tmpDir, server.registryHost, retentionTTL, retentionRecords) + storage, err := storage.New(tmpDir, server.registryHost, retentionTTL, retentionRecords) g.Expect(err).ToNot(HaveOccurred()) cachedArtifact := &sourcev1.Artifact{ @@ -3208,7 +3209,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignatureCosign(t *tes metadata, err := loadTestChartToOCI(chartData, server, "", "", "") g.Expect(err).NotTo(HaveOccurred()) - storage, err := NewStorage(tmpDir, server.registryHost, retentionTTL, retentionRecords) + storage, err := storage.New(tmpDir, server.registryHost, retentionTTL, retentionRecords) g.Expect(err).ToNot(HaveOccurred()) cachedArtifact := &sourcev1.Artifact{ diff --git a/internal/controller/helmrepository_controller.go b/internal/controller/helmrepository_controller.go index 2806f0c40..8c442dbd9 100644 --- a/internal/controller/helmrepository_controller.go +++ b/internal/controller/helmrepository_controller.go @@ -58,6 +58,7 @@ import ( intpredicates "github.com/fluxcd/source-controller/internal/predicates" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" ) // helmRepositoryReadyCondition contains the information required to summarize a @@ -109,7 +110,7 @@ type HelmRepositoryReconciler struct { helper.Metrics Getters helmgetter.Providers - Storage *Storage + Storage *storage.Storage ControllerName string Cache *cache.Cache diff --git a/internal/controller/helmrepository_controller_test.go b/internal/controller/helmrepository_controller_test.go index d753073d9..fb4393d93 100644 --- a/internal/controller/helmrepository_controller_test.go +++ b/internal/controller/helmrepository_controller_test.go @@ -56,6 +56,7 @@ import ( intpredicates "github.com/fluxcd/source-controller/internal/predicates" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" ) func TestHelmRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) { @@ -172,7 +173,7 @@ func TestHelmRepositoryReconciler_Reconcile(t *testing.T) { func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { tests := []struct { name string - beforeFunc func(obj *sourcev1.HelmRepository, storage *Storage) error + beforeFunc func(obj *sourcev1.HelmRepository, storage *storage.Storage) error want sreconcile.Result wantErr bool assertArtifact *sourcev1.Artifact @@ -181,7 +182,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { }{ { name: "garbage collects", - beforeFunc: func(obj *sourcev1.HelmRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmRepository, storage *storage.Storage) error { revisions := []string{"a", "b", "c", "d"} for n := range revisions { v := revisions[n] @@ -231,7 +232,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "notices missing artifact in storage", - beforeFunc: func(obj *sourcev1.HelmRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmRepository, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/invalid.txt", Revision: "d", @@ -250,7 +251,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "notices empty artifact digest", - beforeFunc: func(obj *sourcev1.HelmRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmRepository, storage *storage.Storage) error { f := "empty-digest.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -281,7 +282,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "notices artifact digest mismatch", - beforeFunc: func(obj *sourcev1.HelmRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmRepository, storage *storage.Storage) error { f := "digest-mismatch.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -312,7 +313,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) { }, { name: "updates hostname on diff from current", - beforeFunc: func(obj *sourcev1.HelmRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.HelmRepository, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/reconcile-storage/hostname.txt", Revision: "f", diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index 9b101bd9f..dd4b2e53e 100644 --- a/internal/controller/ocirepository_controller.go +++ b/internal/controller/ocirepository_controller.go @@ -77,6 +77,7 @@ import ( "github.com/fluxcd/source-controller/internal/oci/notation" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" + "github.com/fluxcd/source-controller/internal/storage" "github.com/fluxcd/source-controller/internal/util" ) @@ -139,7 +140,7 @@ type OCIRepositoryReconciler struct { helper.Metrics kuberecorder.EventRecorder - Storage *Storage + Storage *storage.Storage ControllerName string TokenCache *cache.TokenCache requeueDependency time.Duration @@ -1165,7 +1166,7 @@ func (r *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *pat ps = append(ps, sourceignore.ReadPatterns(strings.NewReader(*obj.Spec.Ignore), ignoreDomain)...) } - if err := r.Storage.Archive(&artifact, dir, SourceIgnoreFilter(ps, ignoreDomain)); err != nil { + if err := r.Storage.Archive(&artifact, dir, storage.SourceIgnoreFilter(ps, ignoreDomain)); err != nil { e := serror.NewGeneric( fmt.Errorf("unable to archive artifact to storage: %s", err), sourcev1.ArchiveOperationFailedReason, diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index fe026cad9..7f7d9cc9d 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -73,6 +73,7 @@ import ( serror "github.com/fluxcd/source-controller/internal/error" snotation "github.com/fluxcd/source-controller/internal/oci/notation" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" + "github.com/fluxcd/source-controller/internal/storage" testproxy "github.com/fluxcd/source-controller/tests/proxy" ) @@ -3083,7 +3084,7 @@ func TestOCIRepository_objectLevelWorkloadIdentityFeatureGate(t *testing.T) { func TestOCIRepository_reconcileStorage(t *testing.T) { tests := []struct { name string - beforeFunc func(obj *sourcev1.OCIRepository, storage *Storage) error + beforeFunc func(obj *sourcev1.OCIRepository, storage *storage.Storage) error want sreconcile.Result wantErr bool assertConditions []metav1.Condition @@ -3092,7 +3093,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { }{ { name: "garbage collects", - beforeFunc: func(obj *sourcev1.OCIRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.OCIRepository, storage *storage.Storage) error { revisions := []string{"a", "b", "c", "d"} for n := range revisions { @@ -3146,7 +3147,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { }, { name: "notices missing artifact in storage", - beforeFunc: func(obj *sourcev1.OCIRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.OCIRepository, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/oci-reconcile-storage/invalid.txt", Revision: "e", @@ -3165,7 +3166,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { }, { name: "notices empty artifact digest", - beforeFunc: func(obj *sourcev1.OCIRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.OCIRepository, storage *storage.Storage) error { f := "empty-digest.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -3196,7 +3197,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { }, { name: "notices artifact digest mismatch", - beforeFunc: func(obj *sourcev1.OCIRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.OCIRepository, storage *storage.Storage) error { f := "digest-mismatch.txt" obj.Status.Artifact = &sourcev1.Artifact{ @@ -3227,7 +3228,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) { }, { name: "updates hostname on diff from current", - beforeFunc: func(obj *sourcev1.OCIRepository, storage *Storage) error { + beforeFunc: func(obj *sourcev1.OCIRepository, storage *storage.Storage) error { obj.Status.Artifact = &sourcev1.Artifact{ Path: "/oci-reconcile-storage/hostname.txt", Revision: "f", diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index c4f7005f6..eeb166fb5 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -57,6 +57,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1" "github.com/fluxcd/source-controller/internal/cache" + "github.com/fluxcd/source-controller/internal/storage" // +kubebuilder:scaffold:imports ) @@ -82,7 +83,7 @@ const ( var ( k8sClient client.Client testEnv *testenv.Environment - testStorage *Storage + testStorage *storage.Storage testServer *testserver.ArtifactServer testMetricsH controller.Metrics ctx = ctrl.SetupSignalHandler() @@ -430,12 +431,12 @@ func initTestTLS() { } } -func newTestStorage(s *testserver.HTTPServer) (*Storage, error) { - storage, err := NewStorage(s.Root(), s.URL(), retentionTTL, retentionRecords) +func newTestStorage(s *testserver.HTTPServer) (*storage.Storage, error) { + st, err := storage.New(s.Root(), s.URL(), retentionTTL, retentionRecords) if err != nil { return nil, err } - return storage, nil + return st, nil } var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890") diff --git a/internal/fs/LICENSE b/internal/fs/LICENSE deleted file mode 100644 index a2dd15faf..000000000 --- a/internal/fs/LICENSE +++ /dev/null @@ -1,27 +0,0 @@ -Copyright (c) 2014 The Go Authors. All rights reserved. - -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions are -met: - - * Redistributions of source code must retain the above copyright -notice, this list of conditions and the following disclaimer. - * Redistributions in binary form must reproduce the above -copyright notice, this list of conditions and the following disclaimer -in the documentation and/or other materials provided with the -distribution. - * Neither the name of Google Inc. nor the names of its -contributors may be used to endorse or promote products derived from -this software without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/internal/fs/fs.go b/internal/fs/fs.go deleted file mode 100644 index 21cf96e69..000000000 --- a/internal/fs/fs.go +++ /dev/null @@ -1,345 +0,0 @@ -// Copyright 2016 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package fs - -import ( - "errors" - "fmt" - "io" - "os" - "path/filepath" - "runtime" - "syscall" -) - -// RenameWithFallback attempts to rename a file or directory, but falls back to -// copying in the event of a cross-device link error. If the fallback copy -// succeeds, src is still removed, emulating normal rename behavior. -func RenameWithFallback(src, dst string) error { - _, err := os.Stat(src) - if err != nil { - return fmt.Errorf("cannot stat %s: %w", src, err) - } - - err = os.Rename(src, dst) - if err == nil { - return nil - } - - return renameFallback(err, src, dst) -} - -// renameByCopy attempts to rename a file or directory by copying it to the -// destination and then removing the src thus emulating the rename behavior. -func renameByCopy(src, dst string) error { - var cerr error - if dir, _ := IsDir(src); dir { - cerr = CopyDir(src, dst) - if cerr != nil { - cerr = fmt.Errorf("copying directory failed: %w", cerr) - } - } else { - cerr = copyFile(src, dst) - if cerr != nil { - cerr = fmt.Errorf("copying file failed: %w", cerr) - } - } - - if cerr != nil { - return fmt.Errorf("rename fallback failed: cannot rename %s to %s: %w", src, dst, cerr) - } - - if err := os.RemoveAll(src); err != nil { - return fmt.Errorf("cannot delete %s: %w", src, err) - } - - return nil -} - -var ( - errSrcNotDir = errors.New("source is not a directory") - errDstExist = errors.New("destination already exists") -) - -// CopyDir recursively copies a directory tree, attempting to preserve permissions. -// Source directory must exist, destination directory must *not* exist. -func CopyDir(src, dst string) error { - src = filepath.Clean(src) - dst = filepath.Clean(dst) - - // We use os.Lstat() here to ensure we don't fall in a loop where a symlink - // actually links to a one of its parent directories. - fi, err := os.Lstat(src) - if err != nil { - return err - } - if !fi.IsDir() { - return errSrcNotDir - } - - _, err = os.Stat(dst) - if err != nil && !os.IsNotExist(err) { - return err - } - if err == nil { - return errDstExist - } - - if err = os.MkdirAll(dst, fi.Mode()); err != nil { - return fmt.Errorf("cannot mkdir %s: %w", dst, err) - } - - entries, err := os.ReadDir(src) - if err != nil { - return fmt.Errorf("cannot read directory %s: %w", dst, err) - } - - for _, entry := range entries { - srcPath := filepath.Join(src, entry.Name()) - dstPath := filepath.Join(dst, entry.Name()) - - if entry.IsDir() { - if err = CopyDir(srcPath, dstPath); err != nil { - return fmt.Errorf("copying directory failed: %w", err) - } - } else { - // This will include symlinks, which is what we want when - // copying things. - if err = copyFile(srcPath, dstPath); err != nil { - return fmt.Errorf("copying file failed: %w", err) - } - } - } - - return nil -} - -// copyFile copies the contents of the file named src to the file named -// by dst. The file will be created if it does not already exist. If the -// destination file exists, all its contents will be replaced by the contents -// of the source file. The file mode will be copied from the source. -func copyFile(src, dst string) (err error) { - if sym, err := IsSymlink(src); err != nil { - return fmt.Errorf("symlink check failed: %w", err) - } else if sym { - if err := cloneSymlink(src, dst); err != nil { - if runtime.GOOS == "windows" { - // If cloning the symlink fails on Windows because the user - // does not have the required privileges, ignore the error and - // fall back to copying the file contents. - // - // ERROR_PRIVILEGE_NOT_HELD is 1314 (0x522): - // https://msdn.microsoft.com/en-us/library/windows/desktop/ms681385(v=vs.85).aspx - if lerr, ok := err.(*os.LinkError); ok && lerr.Err != syscall.Errno(1314) { - return err - } - } else { - return err - } - } else { - return nil - } - } - - in, err := os.Open(src) - if err != nil { - return - } - defer in.Close() - - out, err := os.Create(dst) - if err != nil { - return - } - - if _, err = io.Copy(out, in); err != nil { - out.Close() - return - } - - // Check for write errors on Close - if err = out.Close(); err != nil { - return - } - - si, err := os.Stat(src) - if err != nil { - return - } - - // Temporary fix for Go < 1.9 - // - // See: https://github.com/golang/dep/issues/774 - // and https://github.com/golang/go/issues/20829 - if runtime.GOOS == "windows" { - dst = fixLongPath(dst) - } - err = os.Chmod(dst, si.Mode()) - - return -} - -// cloneSymlink will create a new symlink that points to the resolved path of sl. -// If sl is a relative symlink, dst will also be a relative symlink. -func cloneSymlink(sl, dst string) error { - resolved, err := os.Readlink(sl) - if err != nil { - return err - } - - return os.Symlink(resolved, dst) -} - -// IsDir determines is the path given is a directory or not. -func IsDir(name string) (bool, error) { - fi, err := os.Stat(name) - if err != nil { - return false, err - } - if !fi.IsDir() { - return false, fmt.Errorf("%q is not a directory", name) - } - return true, nil -} - -// IsSymlink determines if the given path is a symbolic link. -func IsSymlink(path string) (bool, error) { - l, err := os.Lstat(path) - if err != nil { - return false, err - } - - return l.Mode()&os.ModeSymlink == os.ModeSymlink, nil -} - -// fixLongPath returns the extended-length (\\?\-prefixed) form of -// path when needed, in order to avoid the default 260 character file -// path limit imposed by Windows. If path is not easily converted to -// the extended-length form (for example, if path is a relative path -// or contains .. elements), or is short enough, fixLongPath returns -// path unmodified. -// -// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath -func fixLongPath(path string) string { - // Do nothing (and don't allocate) if the path is "short". - // Empirically (at least on the Windows Server 2013 builder), - // the kernel is arbitrarily okay with < 248 bytes. That - // matches what the docs above say: - // "When using an API to create a directory, the specified - // path cannot be so long that you cannot append an 8.3 file - // name (that is, the directory name cannot exceed MAX_PATH - // minus 12)." Since MAX_PATH is 260, 260 - 12 = 248. - // - // The MSDN docs appear to say that a normal path that is 248 bytes long - // will work; empirically the path must be less then 248 bytes long. - if len(path) < 248 { - // Don't fix. (This is how Go 1.7 and earlier worked, - // not automatically generating the \\?\ form) - return path - } - - // The extended form begins with \\?\, as in - // \\?\c:\windows\foo.txt or \\?\UNC\server\share\foo.txt. - // The extended form disables evaluation of . and .. path - // elements and disables the interpretation of / as equivalent - // to \. The conversion here rewrites / to \ and elides - // . elements as well as trailing or duplicate separators. For - // simplicity it avoids the conversion entirely for relative - // paths or paths containing .. elements. For now, - // \\server\share paths are not converted to - // \\?\UNC\server\share paths because the rules for doing so - // are less well-specified. - if len(path) >= 2 && path[:2] == `\\` { - // Don't canonicalize UNC paths. - return path - } - if !isAbs(path) { - // Relative path - return path - } - - const prefix = `\\?` - - pathbuf := make([]byte, len(prefix)+len(path)+len(`\`)) - copy(pathbuf, prefix) - n := len(path) - r, w := 0, len(prefix) - for r < n { - switch { - case os.IsPathSeparator(path[r]): - // empty block - r++ - case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])): - // /./ - r++ - case r+1 < n && path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])): - // /../ is currently unhandled - return path - default: - pathbuf[w] = '\\' - w++ - for ; r < n && !os.IsPathSeparator(path[r]); r++ { - pathbuf[w] = path[r] - w++ - } - } - } - // A drive's root directory needs a trailing \ - if w == len(`\\?\c:`) { - pathbuf[w] = '\\' - w++ - } - return string(pathbuf[:w]) -} - -func isAbs(path string) (b bool) { - v := volumeName(path) - if v == "" { - return false - } - path = path[len(v):] - if path == "" { - return false - } - return os.IsPathSeparator(path[0]) -} - -func volumeName(path string) (v string) { - if len(path) < 2 { - return "" - } - // with drive letter - c := path[0] - if path[1] == ':' && - ('0' <= c && c <= '9' || 'a' <= c && c <= 'z' || - 'A' <= c && c <= 'Z') { - return path[:2] - } - // is it UNC - if l := len(path); l >= 5 && os.IsPathSeparator(path[0]) && os.IsPathSeparator(path[1]) && - !os.IsPathSeparator(path[2]) && path[2] != '.' { - // first, leading `\\` and next shouldn't be `\`. its server name. - for n := 3; n < l-1; n++ { - // second, next '\' shouldn't be repeated. - if os.IsPathSeparator(path[n]) { - n++ - // third, following something characters. its share name. - if !os.IsPathSeparator(path[n]) { - if path[n] == '.' { - break - } - for ; n < l; n++ { - if os.IsPathSeparator(path[n]) { - break - } - } - return path[:n] - } - break - } - } - } - return "" -} diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go deleted file mode 100644 index 9a1c5ef99..000000000 --- a/internal/fs/fs_test.go +++ /dev/null @@ -1,590 +0,0 @@ -// Copyright 2016 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package fs - -import ( - "fmt" - "os" - "os/exec" - "path/filepath" - "runtime" - "sync" - "testing" -) - -var ( - mu sync.Mutex -) - -func TestRenameWithFallback(t *testing.T) { - dir := t.TempDir() - - if err := RenameWithFallback(filepath.Join(dir, "does_not_exists"), filepath.Join(dir, "dst")); err == nil { - t.Fatal("expected an error for non existing file, but got nil") - } - - srcpath := filepath.Join(dir, "src") - - if srcf, err := os.Create(srcpath); err != nil { - t.Fatal(err) - } else { - srcf.Close() - } - - if err := RenameWithFallback(srcpath, filepath.Join(dir, "dst")); err != nil { - t.Fatal(err) - } - - srcpath = filepath.Join(dir, "a") - if err := os.MkdirAll(srcpath, 0o770); err != nil { - t.Fatal(err) - } - - dstpath := filepath.Join(dir, "b") - if err := os.MkdirAll(dstpath, 0o770); err != nil { - t.Fatal(err) - } - - if err := RenameWithFallback(srcpath, dstpath); err == nil { - t.Fatal("expected an error if dst is an existing directory, but got nil") - } -} - -func TestCopyDir(t *testing.T) { - dir := t.TempDir() - - srcdir := filepath.Join(dir, "src") - if err := os.MkdirAll(srcdir, 0o750); err != nil { - t.Fatal(err) - } - - files := []struct { - path string - contents string - fi os.FileInfo - }{ - {path: "myfile", contents: "hello world"}, - {path: filepath.Join("subdir", "file"), contents: "subdir file"}, - } - - // Create structure indicated in 'files' - for i, file := range files { - fn := filepath.Join(srcdir, file.path) - dn := filepath.Dir(fn) - if err := os.MkdirAll(dn, 0o750); err != nil { - t.Fatal(err) - } - - fh, err := os.Create(fn) - if err != nil { - t.Fatal(err) - } - - if _, err = fh.Write([]byte(file.contents)); err != nil { - t.Fatal(err) - } - fh.Close() - - files[i].fi, err = os.Stat(fn) - if err != nil { - t.Fatal(err) - } - } - - destdir := filepath.Join(dir, "dest") - if err := CopyDir(srcdir, destdir); err != nil { - t.Fatal(err) - } - - // Compare copy against structure indicated in 'files' - for _, file := range files { - fn := filepath.Join(srcdir, file.path) - dn := filepath.Dir(fn) - dirOK, err := IsDir(dn) - if err != nil { - t.Fatal(err) - } - if !dirOK { - t.Fatalf("expected %s to be a directory", dn) - } - - got, err := os.ReadFile(fn) - if err != nil { - t.Fatal(err) - } - - if file.contents != string(got) { - t.Fatalf("expected: %s, got: %s", file.contents, string(got)) - } - - gotinfo, err := os.Stat(fn) - if err != nil { - t.Fatal(err) - } - - if file.fi.Mode() != gotinfo.Mode() { - t.Fatalf("expected %s: %#v\n to be the same mode as %s: %#v", - file.path, file.fi.Mode(), fn, gotinfo.Mode()) - } - } -} - -func TestCopyDirFail_SrcInaccessible(t *testing.T) { - if runtime.GOOS == "windows" { - // XXX: setting permissions works differently in - // Microsoft Windows. Skipping this this until a - // compatible implementation is provided. - t.Skip("skipping on windows") - } - - var srcdir, dstdir string - - setupInaccessibleDir(t, func(dir string) error { - srcdir = filepath.Join(dir, "src") - return os.MkdirAll(srcdir, 0o750) - }) - - dir := t.TempDir() - - dstdir = filepath.Join(dir, "dst") - if err := CopyDir(srcdir, dstdir); err == nil { - t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) - } -} - -func TestCopyDirFail_DstInaccessible(t *testing.T) { - if runtime.GOOS == "windows" { - // XXX: setting permissions works differently in - // Microsoft Windows. Skipping this this until a - // compatible implementation is provided. - t.Skip("skipping on windows") - } - - var srcdir, dstdir string - - dir := t.TempDir() - - srcdir = filepath.Join(dir, "src") - if err := os.MkdirAll(srcdir, 0o750); err != nil { - t.Fatal(err) - } - - setupInaccessibleDir(t, func(dir string) error { - dstdir = filepath.Join(dir, "dst") - return nil - }) - - if err := CopyDir(srcdir, dstdir); err == nil { - t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) - } -} - -func TestCopyDirFail_SrcIsNotDir(t *testing.T) { - var srcdir, dstdir string - - dir := t.TempDir() - - srcdir = filepath.Join(dir, "src") - if _, err := os.Create(srcdir); err != nil { - t.Fatal(err) - } - - dstdir = filepath.Join(dir, "dst") - - err := CopyDir(srcdir, dstdir) - if err == nil { - t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) - } - - if err != errSrcNotDir { - t.Fatalf("expected %v error for CopyDir(%s, %s), got %s", errSrcNotDir, srcdir, dstdir, err) - } - -} - -func TestCopyDirFail_DstExists(t *testing.T) { - var srcdir, dstdir string - - dir := t.TempDir() - - srcdir = filepath.Join(dir, "src") - if err := os.MkdirAll(srcdir, 0o750); err != nil { - t.Fatal(err) - } - - dstdir = filepath.Join(dir, "dst") - if err := os.MkdirAll(dstdir, 0o750); err != nil { - t.Fatal(err) - } - - err := CopyDir(srcdir, dstdir) - if err == nil { - t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) - } - - if err != errDstExist { - t.Fatalf("expected %v error for CopyDir(%s, %s), got %s", errDstExist, srcdir, dstdir, err) - } -} - -func TestCopyDirFailOpen(t *testing.T) { - if runtime.GOOS == "windows" { - // XXX: setting permissions works differently in - // Microsoft Windows. os.Chmod(..., 0o222) below is not - // enough for the file to be readonly, and os.Chmod(..., - // 0000) returns an invalid argument error. Skipping - // this this until a compatible implementation is - // provided. - t.Skip("skipping on windows") - } - - var srcdir, dstdir string - - dir := t.TempDir() - - srcdir = filepath.Join(dir, "src") - if err := os.MkdirAll(srcdir, 0o750); err != nil { - t.Fatal(err) - } - - srcfn := filepath.Join(srcdir, "file") - srcf, err := os.Create(srcfn) - if err != nil { - t.Fatal(err) - } - srcf.Close() - - // setup source file so that it cannot be read - if err = os.Chmod(srcfn, 0o220); err != nil { - t.Fatal(err) - } - - dstdir = filepath.Join(dir, "dst") - - if err = CopyDir(srcdir, dstdir); err == nil { - t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) - } -} - -func TestCopyFile(t *testing.T) { - dir := t.TempDir() - - srcf, err := os.Create(filepath.Join(dir, "srcfile")) - if err != nil { - t.Fatal(err) - } - - want := "hello world" - if _, err := srcf.Write([]byte(want)); err != nil { - t.Fatal(err) - } - srcf.Close() - - destf := filepath.Join(dir, "destf") - if err := copyFile(srcf.Name(), destf); err != nil { - t.Fatal(err) - } - - got, err := os.ReadFile(destf) - if err != nil { - t.Fatal(err) - } - - if want != string(got) { - t.Fatalf("expected: %s, got: %s", want, string(got)) - } - - wantinfo, err := os.Stat(srcf.Name()) - if err != nil { - t.Fatal(err) - } - - gotinfo, err := os.Stat(destf) - if err != nil { - t.Fatal(err) - } - - if wantinfo.Mode() != gotinfo.Mode() { - t.Fatalf("expected %s: %#v\n to be the same mode as %s: %#v", srcf.Name(), wantinfo.Mode(), destf, gotinfo.Mode()) - } -} - -func TestCopyFileSymlink(t *testing.T) { - dir := t.TempDir() - defer cleanUpDir(dir) - - testcases := map[string]string{ - filepath.Join("./testdata/symlinks/file-symlink"): filepath.Join(dir, "dst-file"), - filepath.Join("./testdata/symlinks/windows-file-symlink"): filepath.Join(dir, "windows-dst-file"), - filepath.Join("./testdata/symlinks/invalid-symlink"): filepath.Join(dir, "invalid-symlink"), - } - - for symlink, dst := range testcases { - t.Run(symlink, func(t *testing.T) { - var err error - if err = copyFile(symlink, dst); err != nil { - t.Fatalf("failed to copy symlink: %s", err) - } - - var want, got string - - if runtime.GOOS == "windows" { - // Creating symlinks on Windows require an additional permission - // regular users aren't granted usually. So we copy the file - // content as a fall back instead of creating a real symlink. - srcb, err := os.ReadFile(symlink) - if err != nil { - t.Fatalf("%+v", err) - } - dstb, err := os.ReadFile(dst) - if err != nil { - t.Fatalf("%+v", err) - } - - want = string(srcb) - got = string(dstb) - } else { - want, err = os.Readlink(symlink) - if err != nil { - t.Fatalf("%+v", err) - } - - got, err = os.Readlink(dst) - if err != nil { - t.Fatalf("could not resolve symlink: %s", err) - } - } - - if want != got { - t.Fatalf("resolved path is incorrect. expected %s, got %s", want, got) - } - }) - } -} - -func TestCopyFileLongFilePath(t *testing.T) { - if runtime.GOOS != "windows" { - // We want to ensure the temporary fix actually fixes the issue with - // os.Chmod and long file paths. This is only applicable on Windows. - t.Skip("skipping on non-windows") - } - - dir := t.TempDir() - - // Create a directory with a long-enough path name to cause the bug in #774. - dirName := "" - for len(dir+string(os.PathSeparator)+dirName) <= 300 { - dirName += "directory" - } - - fullPath := filepath.Join(dir, dirName, string(os.PathSeparator)) - if err := os.MkdirAll(fullPath, 0o750); err != nil && !os.IsExist(err) { - t.Fatalf("%+v", fmt.Errorf("unable to create temp directory: %s", fullPath)) - } - - err := os.WriteFile(fullPath+"src", []byte(nil), 0o640) - if err != nil { - t.Fatalf("%+v", err) - } - - err = copyFile(fullPath+"src", fullPath+"dst") - if err != nil { - t.Fatalf("unexpected error while copying file: %v", err) - } -} - -// C:\Users\appveyor\AppData\Local\Temp\1\gotest639065787\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890\dir4567890 - -func TestCopyFileFail(t *testing.T) { - if runtime.GOOS == "windows" { - // XXX: setting permissions works differently in - // Microsoft Windows. Skipping this this until a - // compatible implementation is provided. - t.Skip("skipping on windows") - } - - dir := t.TempDir() - - srcf, err := os.Create(filepath.Join(dir, "srcfile")) - if err != nil { - t.Fatal(err) - } - srcf.Close() - - var dstdir string - - setupInaccessibleDir(t, func(dir string) error { - dstdir = filepath.Join(dir, "dir") - return os.Mkdir(dstdir, 0o770) - }) - - fn := filepath.Join(dstdir, "file") - if err := copyFile(srcf.Name(), fn); err == nil { - t.Fatalf("expected error for %s, got none", fn) - } -} - -// setupInaccessibleDir creates a temporary location with a single -// directory in it, in such a way that that directory is not accessible -// after this function returns. -// -// op is called with the directory as argument, so that it can create -// files or other test artifacts. -// -// If setupInaccessibleDir fails in its preparation, or op fails, t.Fatal -// will be invoked. -func setupInaccessibleDir(t *testing.T, op func(dir string) error) { - dir, err := os.MkdirTemp("", "dep") - if err != nil { - t.Fatal(err) - } - - subdir := filepath.Join(dir, "dir") - - t.Cleanup(func() { - if err := os.Chmod(subdir, 0o770); err != nil { - t.Error(err) - } - }) - - if err := os.Mkdir(subdir, 0o770); err != nil { - t.Fatal(err) - } - - if err := op(subdir); err != nil { - t.Fatal(err) - } - - if err := os.Chmod(subdir, 0o660); err != nil { - t.Fatal(err) - } -} - -func TestIsDir(t *testing.T) { - wd, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - - var dn string - - setupInaccessibleDir(t, func(dir string) error { - dn = filepath.Join(dir, "dir") - return os.Mkdir(dn, 0o770) - }) - - tests := map[string]struct { - exists bool - err bool - }{ - wd: {true, false}, - filepath.Join(wd, "testdata"): {true, false}, - filepath.Join(wd, "main.go"): {false, true}, - filepath.Join(wd, "this_file_does_not_exist.thing"): {false, true}, - dn: {false, true}, - } - - if runtime.GOOS == "windows" { - // This test doesn't work on Microsoft Windows because - // of the differences in how file permissions are - // implemented. For this to work, the directory where - // the directory exists should be inaccessible. - delete(tests, dn) - } - - for f, want := range tests { - got, err := IsDir(f) - if err != nil && !want.err { - t.Fatalf("expected no error, got %v", err) - } - - if got != want.exists { - t.Fatalf("expected %t for %s, got %t", want.exists, f, got) - } - } -} - -func TestIsSymlink(t *testing.T) { - dir := t.TempDir() - - dirPath := filepath.Join(dir, "directory") - if err := os.MkdirAll(dirPath, 0o770); err != nil { - t.Fatal(err) - } - - filePath := filepath.Join(dir, "file") - f, err := os.Create(filePath) - if err != nil { - t.Fatal(err) - } - f.Close() - - dirSymlink := filepath.Join(dir, "dirSymlink") - fileSymlink := filepath.Join(dir, "fileSymlink") - - if err = os.Symlink(dirPath, dirSymlink); err != nil { - t.Fatal(err) - } - if err = os.Symlink(filePath, fileSymlink); err != nil { - t.Fatal(err) - } - - var ( - inaccessibleFile string - inaccessibleSymlink string - ) - - setupInaccessibleDir(t, func(dir string) error { - inaccessibleFile = filepath.Join(dir, "file") - if fh, err := os.Create(inaccessibleFile); err != nil { - return err - } else if err = fh.Close(); err != nil { - return err - } - - inaccessibleSymlink = filepath.Join(dir, "symlink") - return os.Symlink(inaccessibleFile, inaccessibleSymlink) - }) - - tests := map[string]struct{ expected, err bool }{ - dirPath: {false, false}, - filePath: {false, false}, - dirSymlink: {true, false}, - fileSymlink: {true, false}, - inaccessibleFile: {false, true}, - inaccessibleSymlink: {false, true}, - } - - if runtime.GOOS == "windows" { - // XXX: setting permissions works differently in Windows. Skipping - // these cases until a compatible implementation is provided. - delete(tests, inaccessibleFile) - delete(tests, inaccessibleSymlink) - } - - for path, want := range tests { - got, err := IsSymlink(path) - if err != nil { - if !want.err { - t.Errorf("expected no error, got %v", err) - } - } - - if got != want.expected { - t.Errorf("expected %t for %s, got %t", want.expected, path, got) - } - } -} - -func cleanUpDir(dir string) { - if runtime.GOOS == "windows" { - mu.Lock() - exec.Command(`taskkill`, `/F`, `/IM`, `git.exe`).Run() - mu.Unlock() - } - if dir != "" { - os.RemoveAll(dir) - } -} diff --git a/internal/fs/rename.go b/internal/fs/rename.go deleted file mode 100644 index bad1f4778..000000000 --- a/internal/fs/rename.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2016 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build !windows -// +build !windows - -package fs - -import ( - "fmt" - "os" - "syscall" -) - -// renameFallback attempts to determine the appropriate fallback to failed rename -// operation depending on the resulting error. -func renameFallback(err error, src, dst string) error { - // Rename may fail if src and dst are on different devices; fall back to - // copy if we detect that case. syscall.EXDEV is the common name for the - // cross device link error which has varying output text across different - // operating systems. - terr, ok := err.(*os.LinkError) - if !ok { - return err - } else if terr.Err != syscall.EXDEV { - return fmt.Errorf("link error: cannot rename %s to %s: %w", src, dst, terr) - } - - return renameByCopy(src, dst) -} diff --git a/internal/fs/rename_windows.go b/internal/fs/rename_windows.go deleted file mode 100644 index fa9a0b4d9..000000000 --- a/internal/fs/rename_windows.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2016 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -//go:build windows -// +build windows - -package fs - -import ( - "fmt" - "os" - "syscall" -) - -// renameFallback attempts to determine the appropriate fallback to failed rename -// operation depending on the resulting error. -func renameFallback(err error, src, dst string) error { - // Rename may fail if src and dst are on different devices; fall back to - // copy if we detect that case. syscall.EXDEV is the common name for the - // cross device link error which has varying output text across different - // operating systems. - terr, ok := err.(*os.LinkError) - if !ok { - return err - } - - if terr.Err != syscall.EXDEV { - // In windows it can drop down to an operating system call that - // returns an operating system error with a different number and - // message. Checking for that as a fall back. - noerr, ok := terr.Err.(syscall.Errno) - - // 0x11 (ERROR_NOT_SAME_DEVICE) is the windows error. - // See https://msdn.microsoft.com/en-us/library/cc231199.aspx - if ok && noerr != 0x11 { - return fmt.Errorf("link error: cannot rename %s to %s: %w", src, dst, terr) - } - } - - return renameByCopy(src, dst) -} diff --git a/internal/fs/testdata/symlinks/dir-symlink b/internal/fs/testdata/symlinks/dir-symlink deleted file mode 120000 index 777ebd014..000000000 --- a/internal/fs/testdata/symlinks/dir-symlink +++ /dev/null @@ -1 +0,0 @@ -../../testdata \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/file-symlink b/internal/fs/testdata/symlinks/file-symlink deleted file mode 120000 index 4c52274de..000000000 --- a/internal/fs/testdata/symlinks/file-symlink +++ /dev/null @@ -1 +0,0 @@ -../test.file \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/invalid-symlink b/internal/fs/testdata/symlinks/invalid-symlink deleted file mode 120000 index 0edf4f301..000000000 --- a/internal/fs/testdata/symlinks/invalid-symlink +++ /dev/null @@ -1 +0,0 @@ -/non/existing/file \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/windows-file-symlink b/internal/fs/testdata/symlinks/windows-file-symlink deleted file mode 120000 index af1d6c8f5..000000000 --- a/internal/fs/testdata/symlinks/windows-file-symlink +++ /dev/null @@ -1 +0,0 @@ -C:/Users/ibrahim/go/src/github.com/golang/dep/internal/fs/testdata/test.file \ No newline at end of file diff --git a/internal/fs/testdata/test.file b/internal/fs/testdata/test.file deleted file mode 100644 index e69de29bb..000000000 diff --git a/internal/helm/chart/builder.go b/internal/helm/chart/builder.go index b56c8c9a3..6ac896e78 100644 --- a/internal/helm/chart/builder.go +++ b/internal/helm/chart/builder.go @@ -24,10 +24,10 @@ import ( "regexp" "strings" + sourcefs "github.com/fluxcd/pkg/oci" helmchart "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/chartutil" - "github.com/fluxcd/source-controller/internal/fs" "github.com/fluxcd/source-controller/internal/oci" ) @@ -219,7 +219,7 @@ func packageToPath(chart *helmchart.Chart, out string) error { if err != nil { return fmt.Errorf("failed to package chart: %w", err) } - if err = fs.RenameWithFallback(p, out); err != nil { + if err = sourcefs.RenameWithFallback(p, out); err != nil { return fmt.Errorf("failed to write chart to file: %w", err) } return nil diff --git a/internal/helm/chart/builder_remote.go b/internal/helm/chart/builder_remote.go index 1010d8cc1..2cfdf81b4 100644 --- a/internal/helm/chart/builder_remote.go +++ b/internal/helm/chart/builder_remote.go @@ -30,9 +30,9 @@ import ( "helm.sh/helm/v3/pkg/repo" "sigs.k8s.io/yaml" + sourcefs "github.com/fluxcd/pkg/oci" "github.com/fluxcd/pkg/runtime/transform" - "github.com/fluxcd/source-controller/internal/fs" "github.com/fluxcd/source-controller/internal/helm/chart/secureloader" "github.com/fluxcd/source-controller/internal/helm/repository" "github.com/fluxcd/source-controller/internal/oci" @@ -290,7 +290,7 @@ func validatePackageAndWriteToPath(reader io.Reader, out string) error { if err = meta.Validate(); err != nil { return fmt.Errorf("failed to validate metadata of written chart: %w", err) } - if err = fs.RenameWithFallback(tmpFile.Name(), out); err != nil { + if err = sourcefs.RenameWithFallback(tmpFile.Name(), out); err != nil { return fmt.Errorf("failed to write chart to file: %w", err) } return nil diff --git a/internal/controller/storage.go b/internal/storage/storage.go similarity index 97% rename from internal/controller/storage.go rename to internal/storage/storage.go index af4b79a70..c5c60612a 100644 --- a/internal/controller/storage.go +++ b/internal/storage/storage.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Flux authors +Copyright 2025 The Flux authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +package storage import ( "archive/tar" @@ -37,12 +37,12 @@ import ( kerrors "k8s.io/apimachinery/pkg/util/errors" "github.com/fluxcd/pkg/lockedfile" + "github.com/fluxcd/pkg/oci" "github.com/fluxcd/pkg/sourceignore" pkgtar "github.com/fluxcd/pkg/tar" v1 "github.com/fluxcd/source-controller/api/v1" intdigest "github.com/fluxcd/source-controller/internal/digest" - sourcefs "github.com/fluxcd/source-controller/internal/fs" ) const GarbageCountLimit = 1000 @@ -73,8 +73,8 @@ type Storage struct { ArtifactRetentionRecords int `json:"artifactRetentionRecords"` } -// NewStorage creates the storage helper for a given path and hostname. -func NewStorage(basePath string, hostname string, artifactRetentionTTL time.Duration, artifactRetentionRecords int) (*Storage, error) { +// New creates the storage helper for a given path and hostname. +func New(basePath string, hostname string, artifactRetentionTTL time.Duration, artifactRetentionRecords int) (*Storage, error) { if f, err := os.Stat(basePath); os.IsNotExist(err) || !f.IsDir() { return nil, fmt.Errorf("invalid dir path: %s", basePath) } @@ -480,7 +480,7 @@ func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFi return err } - if err := sourcefs.RenameWithFallback(tmpName, localPath); err != nil { + if err := oci.RenameWithFallback(tmpName, localPath); err != nil { return err } @@ -522,7 +522,7 @@ func (s Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode o return err } - if err := sourcefs.RenameWithFallback(tfName, localPath); err != nil { + if err := oci.RenameWithFallback(tfName, localPath); err != nil { return err } @@ -560,7 +560,7 @@ func (s Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) { return err } - if err := sourcefs.RenameWithFallback(tfName, localPath); err != nil { + if err := oci.RenameWithFallback(tfName, localPath); err != nil { return err } @@ -620,7 +620,7 @@ func (s Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) error if err != nil { return err } - if err := sourcefs.RenameWithFallback(fromPath, toPath); err != nil { + if err := oci.RenameWithFallback(fromPath, toPath); err != nil { return err } return nil diff --git a/internal/controller/storage_test.go b/internal/storage/storage_test.go similarity index 96% rename from internal/controller/storage_test.go rename to internal/storage/storage_test.go index 1b65ce914..a4740084b 100644 --- a/internal/controller/storage_test.go +++ b/internal/storage/storage_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020, 2021 The Flux authors +Copyright 2025 The Flux authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package controller +package storage import ( "archive/tar" @@ -24,6 +24,7 @@ import ( "errors" "fmt" "io" + "math/rand" "os" "path/filepath" "strings" @@ -39,7 +40,7 @@ import ( func TestStorageConstructor(t *testing.T) { dir := t.TempDir() - if _, err := NewStorage("/nonexistent", "hostname", time.Minute, 2); err == nil { + if _, err := New("/nonexistent", "hostname", time.Minute, 2); err == nil { t.Fatal("nonexistent path was allowable in storage constructor") } @@ -49,13 +50,13 @@ func TestStorageConstructor(t *testing.T) { } f.Close() - if _, err := NewStorage(f.Name(), "hostname", time.Minute, 2); err == nil { + if _, err := New(f.Name(), "hostname", time.Minute, 2); err == nil { os.Remove(f.Name()) t.Fatal("file path was accepted as basedir") } os.Remove(f.Name()) - if _, err := NewStorage(dir, "hostname", time.Minute, 2); err != nil { + if _, err := New(dir, "hostname", time.Minute, 2); err != nil { t.Fatalf("Valid path did not successfully return: %v", err) } } @@ -104,7 +105,7 @@ func walkTar(tarFile string, match string, dir bool) (int64, int64, bool, error) func TestStorage_Archive(t *testing.T) { dir := t.TempDir() - storage, err := NewStorage(dir, "hostname", time.Minute, 2) + storage, err := New(dir, "hostname", time.Minute, 2) if err != nil { t.Fatalf("error while bootstrapping storage: %v", err) } @@ -308,7 +309,7 @@ func TestStorage_Remove(t *testing.T) { dir := t.TempDir() - s, err := NewStorage(dir, "", 0, 0) + s, err := New(dir, "", 0, 0) g.Expect(err).ToNot(HaveOccurred()) artifact := sourcev1.Artifact{ @@ -327,7 +328,7 @@ func TestStorage_Remove(t *testing.T) { dir := t.TempDir() - s, err := NewStorage(dir, "", 0, 0) + s, err := New(dir, "", 0, 0) g.Expect(err).ToNot(HaveOccurred()) artifact := sourcev1.Artifact{ @@ -344,7 +345,7 @@ func TestStorageRemoveAllButCurrent(t *testing.T) { t.Run("bad directory in archive", func(t *testing.T) { dir := t.TempDir() - s, err := NewStorage(dir, "hostname", time.Minute, 2) + s, err := New(dir, "hostname", time.Minute, 2) if err != nil { t.Fatalf("Valid path did not successfully return: %v", err) } @@ -358,7 +359,7 @@ func TestStorageRemoveAllButCurrent(t *testing.T) { g := NewWithT(t) dir := t.TempDir() - s, err := NewStorage(dir, "hostname", time.Minute, 2) + s, err := New(dir, "hostname", time.Minute, 2) g.Expect(err).ToNot(HaveOccurred(), "failed to create new storage") artifact := sourcev1.Artifact{ @@ -419,7 +420,7 @@ func TestStorageRemoveAll(t *testing.T) { g := NewWithT(t) dir := t.TempDir() - s, err := NewStorage(dir, "hostname", time.Minute, 2) + s, err := New(dir, "hostname", time.Minute, 2) g.Expect(err).ToNot(HaveOccurred(), "failed to create new storage") artifact := sourcev1.Artifact{ @@ -445,7 +446,7 @@ func TestStorageCopyFromPath(t *testing.T) { dir := t.TempDir() - storage, err := NewStorage(dir, "hostname", time.Minute, 2) + storage, err := New(dir, "hostname", time.Minute, 2) if err != nil { t.Fatalf("error while bootstrapping storage: %v", err) } @@ -665,7 +666,7 @@ func TestStorage_getGarbageFiles(t *testing.T) { g := NewWithT(t) dir := t.TempDir() - s, err := NewStorage(dir, "hostname", tt.ttl, tt.maxItemsToBeRetained) + s, err := New(dir, "hostname", tt.ttl, tt.maxItemsToBeRetained) g.Expect(err).ToNot(HaveOccurred(), "failed to create new storage") artifact := sourcev1.Artifact{ @@ -748,7 +749,7 @@ func TestStorage_GarbageCollect(t *testing.T) { g := NewWithT(t) dir := t.TempDir() - s, err := NewStorage(dir, "hostname", time.Second*2, 2) + s, err := New(dir, "hostname", time.Second*2, 2) g.Expect(err).ToNot(HaveOccurred(), "failed to create new storage") artifact := sourcev1.Artifact{ @@ -798,7 +799,7 @@ func TestStorage_VerifyArtifact(t *testing.T) { g := NewWithT(t) dir := t.TempDir() - s, err := NewStorage(dir, "", 0, 0) + s, err := New(dir, "", 0, 0) g.Expect(err).ToNot(HaveOccurred(), "failed to create new storage") g.Expect(os.WriteFile(filepath.Join(dir, "artifact"), []byte("test"), 0o600)).To(Succeed()) @@ -851,3 +852,13 @@ func TestStorage_VerifyArtifact(t *testing.T) { g.Expect(err).ToNot(HaveOccurred()) }) } + +var letterRunes = []rune("abcdefghijklmnopqrstuvwxyz1234567890") + +func randStringRunes(n int) string { + b := make([]rune, n) + for i := range b { + b[i] = letterRunes[rand.Intn(len(letterRunes))] + } + return string(b) +} diff --git a/main.go b/main.go index ca5e20e90..114e7c7d5 100644 --- a/main.go +++ b/main.go @@ -54,6 +54,8 @@ import ( "github.com/fluxcd/pkg/runtime/probes" sourcev1 "github.com/fluxcd/source-controller/api/v1" + intstorage "github.com/fluxcd/source-controller/internal/storage" + // +kubebuilder:scaffold:imports "github.com/fluxcd/source-controller/internal/cache" @@ -436,7 +438,11 @@ func mustInitHelmCache(maxSize int, itemTTL, purgeInterval string) (*cache.Cache return cache.New(maxSize, interval), ttl } -func mustInitStorage(path string, storageAdvAddr string, artifactRetentionTTL time.Duration, artifactRetentionRecords int, artifactDigestAlgo string) *controller.Storage { +func mustInitStorage(path string, + storageAdvAddr string, + artifactRetentionTTL time.Duration, + artifactRetentionRecords int, + artifactDigestAlgo string) *intstorage.Storage { if storageAdvAddr == "" { storageAdvAddr = determineAdvStorageAddr(storageAdvAddr) } @@ -450,7 +456,7 @@ func mustInitStorage(path string, storageAdvAddr string, artifactRetentionTTL ti intdigest.Canonical = algo } - storage, err := controller.NewStorage(path, storageAdvAddr, artifactRetentionTTL, artifactRetentionRecords) + storage, err := intstorage.New(path, storageAdvAddr, artifactRetentionTTL, artifactRetentionRecords) if err != nil { setupLog.Error(err, "unable to initialise storage") os.Exit(1)