From a4cc55d02399097ccd61b914019c2570f226f348 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 10 May 2023 13:39:49 +0200 Subject: [PATCH] storage: change methods to value receiver Given: - None of the methods of the `Storage` are mutating the storage itself. - It must be instantiated to be usable, as there is a strict reliance on values. - The struct itself is light. This seems to be more fitting. Signed-off-by: Hidde Beydals --- internal/controller/storage.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/internal/controller/storage.go b/internal/controller/storage.go index d618e25f1..ef1ac7978 100644 --- a/internal/controller/storage.go +++ b/internal/controller/storage.go @@ -85,7 +85,7 @@ func NewStorage(basePath string, hostname string, artifactRetentionTTL time.Dura } // NewArtifactFor returns a new v1.Artifact. -func (s *Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) v1.Artifact { +func (s Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) v1.Artifact { path := v1.ArtifactPath(kind, metadata.GetNamespace(), metadata.GetName(), fileName) artifact := v1.Artifact{ Path: path, @@ -118,18 +118,18 @@ func (s Storage) SetHostname(URL string) string { } // MkdirAll calls os.MkdirAll for the given v1.Artifact base dir. -func (s *Storage) MkdirAll(artifact v1.Artifact) error { +func (s Storage) MkdirAll(artifact v1.Artifact) error { dir := filepath.Dir(s.LocalPath(artifact)) return os.MkdirAll(dir, 0o700) } // Remove calls os.Remove for the given v1.Artifact path. -func (s *Storage) Remove(artifact v1.Artifact) error { +func (s Storage) Remove(artifact v1.Artifact) error { return os.Remove(s.LocalPath(artifact)) } // RemoveAll calls os.RemoveAll for the given v1.Artifact base dir. -func (s *Storage) RemoveAll(artifact v1.Artifact) (string, error) { +func (s Storage) RemoveAll(artifact v1.Artifact) (string, error) { var deletedDir string dir := filepath.Dir(s.LocalPath(artifact)) // Check if the dir exists. @@ -141,7 +141,7 @@ func (s *Storage) RemoveAll(artifact v1.Artifact) (string, error) { } // RemoveAllButCurrent removes all files for the given v1.Artifact base dir, excluding the current one. -func (s *Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) { +func (s Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) { deletedFiles := []string{} localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) @@ -174,7 +174,7 @@ func (s *Storage) RemoveAllButCurrent(artifact v1.Artifact) ([]string, error) { // 1. collect all artifact files with an expired ttl // 2. if we satisfy maxItemsToBeRetained, then return // 3. else, collect all artifact files till the latest n files remain, where n=maxItemsToBeRetained -func (s *Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItemsToBeRetained int, ttl time.Duration) (garbageFiles []string, _ error) { +func (s Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItemsToBeRetained int, ttl time.Duration) (garbageFiles []string, _ error) { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) artifactFilesWithCreatedTs := make(map[time.Time]string) @@ -261,7 +261,7 @@ func (s *Storage) getGarbageFiles(artifact v1.Artifact, totalCountLimit, maxItem // GarbageCollect removes all garbage files in the artifact dir according to the provided // retention options. -func (s *Storage) GarbageCollect(ctx context.Context, artifact v1.Artifact, timeout time.Duration) ([]string, error) { +func (s Storage) GarbageCollect(ctx context.Context, artifact v1.Artifact, timeout time.Duration) ([]string, error) { delFilesChan := make(chan []string) errChan := make(chan error) // Abort if it takes more than the provided timeout duration. @@ -323,7 +323,7 @@ func stringInSlice(a string, list []string) bool { } // ArtifactExist returns a boolean indicating whether the v1.Artifact exists in storage and is a regular file. -func (s *Storage) ArtifactExist(artifact v1.Artifact) bool { +func (s Storage) ArtifactExist(artifact v1.Artifact) bool { fi, err := os.Lstat(s.LocalPath(artifact)) if err != nil { return false @@ -334,7 +334,7 @@ func (s *Storage) ArtifactExist(artifact v1.Artifact) bool { // VerifyArtifact verifies if the Digest of the v1.Artifact matches the digest // of the file in Storage. It returns an error if the digests don't match, or // if it can't be verified. -func (s *Storage) VerifyArtifact(artifact v1.Artifact) error { +func (s Storage) VerifyArtifact(artifact v1.Artifact) error { if artifact.Digest == "" { return fmt.Errorf("artifact has no digest") } @@ -382,7 +382,7 @@ func SourceIgnoreFilter(ps []gitignore.Pattern, domain []string) ArchiveFileFilt // directories and any ArchiveFileFilter matches. While archiving, any environment specific data (for example, // the user and group name) is stripped from file headers. // If successful, it sets the digest and last update time on the artifact. -func (s *Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFilter) (err error) { +func (s Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileFilter) (err error) { if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() { return fmt.Errorf("invalid dir path: %s", dir) } @@ -504,7 +504,7 @@ func (s *Storage) Archive(artifact *v1.Artifact, dir string, filter ArchiveFileF // AtomicWriteFile atomically writes the io.Reader contents to the v1.Artifact path. // If successful, it sets the digest and last update time on the artifact. -func (s *Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode os.FileMode) (err error) { +func (s Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode os.FileMode) (err error) { localPath := s.LocalPath(*artifact) tf, err := os.CreateTemp(filepath.Split(localPath)) if err != nil { @@ -546,7 +546,7 @@ func (s *Storage) AtomicWriteFile(artifact *v1.Artifact, reader io.Reader, mode // Copy atomically copies the io.Reader contents to the v1.Artifact path. // If successful, it sets the digest and last update time on the artifact. -func (s *Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) { +func (s Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) { localPath := s.LocalPath(*artifact) tf, err := os.CreateTemp(filepath.Split(localPath)) if err != nil { @@ -584,7 +584,7 @@ func (s *Storage) Copy(artifact *v1.Artifact, reader io.Reader) (err error) { // CopyFromPath atomically copies the contents of the given path to the path of the v1.Artifact. // If successful, the digest and last update time on the artifact is set. -func (s *Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) { +func (s Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) { f, err := os.Open(path) if err != nil { return err @@ -599,7 +599,7 @@ func (s *Storage) CopyFromPath(artifact *v1.Artifact, path string) (err error) { } // CopyToPath copies the contents in the (sub)path of the given artifact to the given path. -func (s *Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) error { +func (s Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) error { // create a tmp directory to store artifact tmp, err := os.MkdirTemp("", "flux-include-") if err != nil { @@ -638,7 +638,7 @@ func (s *Storage) CopyToPath(artifact *v1.Artifact, subPath, toPath string) erro } // Symlink creates or updates a symbolic link for the given v1.Artifact and returns the URL for the symlink. -func (s *Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) { +func (s Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) link := filepath.Join(dir, linkName) @@ -660,14 +660,14 @@ func (s *Storage) Symlink(artifact v1.Artifact, linkName string) (string, error) } // Lock creates a file lock for the given v1.Artifact. -func (s *Storage) Lock(artifact v1.Artifact) (unlock func(), err error) { +func (s Storage) Lock(artifact v1.Artifact) (unlock func(), err error) { lockFile := s.LocalPath(artifact) + ".lock" mutex := lockedfile.MutexAt(lockFile) return mutex.Lock() } // LocalPath returns the secure local path of the given artifact (that is: relative to the Storage.BasePath). -func (s *Storage) LocalPath(artifact v1.Artifact) string { +func (s Storage) LocalPath(artifact v1.Artifact) string { if artifact.Path == "" { return "" }