From 1ab12869ac7d55e88e6e5605d4e99f28db7d66e7 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 8 Sep 2020 10:58:58 +0200 Subject: [PATCH 1/5] Make storage file writes atomic --- controllers/helmchart_controller.go | 12 +- controllers/helmrepository_controller.go | 13 +- controllers/helmrepository_controller_test.go | 2 +- controllers/storage.go | 113 ++- internal/fs/LICENSE | 27 + internal/fs/fs.go | 346 +++++++++ internal/fs/fs_test.go | 657 ++++++++++++++++++ internal/fs/rename.go | 30 + internal/fs/rename_windows.go | 41 ++ 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 14 files changed, 1201 insertions(+), 44 deletions(-) create mode 100644 internal/fs/LICENSE create mode 100644 internal/fs/fs.go create mode 100644 internal/fs/fs_test.go create mode 100644 internal/fs/rename.go create mode 100644 internal/fs/rename_windows.go create mode 120000 internal/fs/testdata/symlinks/dir-symlink create mode 120000 internal/fs/testdata/symlinks/file-symlink create mode 120000 internal/fs/testdata/symlinks/invalid-symlink create mode 120000 internal/fs/testdata/symlinks/windows-file-symlink create mode 100644 internal/fs/testdata/test.file diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index c6e7b720..24b7858f 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -17,8 +17,10 @@ limitations under the License. package controllers import ( + "bytes" "context" "fmt" + "io" "io/ioutil" "net/url" "os" @@ -255,12 +257,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err } - chartBytes, err := ioutil.ReadAll(res) - if err != nil { - return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err - } - - sum := r.Storage.Checksum(chartBytes) + var buf bytes.Buffer + sum := r.Storage.Checksum(io.TeeReader(res, &buf)) artifact := r.Storage.ArtifactFor(chart.Kind, chart.GetObjectMeta(), fmt.Sprintf("%s-%s-%s.tgz", cv.Name, cv.Version, sum), cv.Version, sum) @@ -280,7 +278,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, defer unlock() // save artifact to storage - err = r.Storage.WriteFile(artifact, chartBytes) + err = r.Storage.AtomicWriteFile(artifact, &buf, 0644) if err != nil { err = fmt.Errorf("unable to write chart file: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 5e082381..e5696352 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -17,6 +17,7 @@ limitations under the License. package controllers import ( + "bytes" "context" "fmt" "io/ioutil" @@ -212,23 +213,23 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - data, err := ioutil.ReadAll(res) + b, err := ioutil.ReadAll(res) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - i := &repo.IndexFile{} - if err := yaml.Unmarshal(data, i); err != nil { + i := repo.IndexFile{} + if err := yaml.Unmarshal(b, &i); err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } i.SortEntries() - index, err := yaml.Marshal(i) + b, err = yaml.Marshal(&i) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - sum := r.Storage.Checksum(index) + sum := r.Storage.Checksum(bytes.NewReader(b)) artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), fmt.Sprintf("index-%s.yaml", sum), i.Generated.Format(time.RFC3339Nano), sum) @@ -248,7 +249,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou defer unlock() // save artifact to storage - err = r.Storage.WriteFile(artifact, index) + err = r.Storage.AtomicWriteFile(artifact, bytes.NewReader(b), 0644) if err != nil { err = fmt.Errorf("unable to write repository index file: %w", err) return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index 90b20cab..c596ca73 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -140,7 +140,7 @@ var _ = Describe("HelmRepositoryReconciler", func() { Eventually(func() error { r := &sourcev1.HelmRepository{} return k8sClient.Get(context.Background(), key, r) - }).ShouldNot(Succeed()) + }, timeout, interval).ShouldNot(Succeed()) exists := func(path string) bool { // wait for tmp sync on macOS diff --git a/controllers/storage.go b/controllers/storage.go index a137a39e..6435de27 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -36,6 +36,7 @@ import ( "github.com/fluxcd/pkg/lockedfile" sourcev1 "github.com/fluxcd/source-controller/api/v1alpha1" + "github.com/fluxcd/source-controller/internal/fs" ) const ( @@ -95,7 +96,8 @@ func (s *Storage) RemoveAll(artifact sourcev1.Artifact) error { return os.RemoveAll(dir) } -// RemoveAllButCurrent removes all files for the given artifact base dir excluding the current one +// RemoveAllButCurrent removes all files for the given v1alpha1.Artifact base dir, +// excluding the current one. func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) @@ -120,8 +122,8 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error { return nil } -// ArtifactExist returns a boolean indicating whether the artifact exists in storage and is a -// regular file. +// ArtifactExist returns a boolean indicating whether the v1alpha1.Artifact exists in storage +// and is a regular file. func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { fi, err := os.Lstat(s.LocalPath(artifact)) if err != nil { @@ -130,9 +132,10 @@ func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { return fi.Mode().IsRegular() } -// Archive creates a tar.gz to the artifact path from the given dir excluding any VCS specific -// files and directories, or any of the excludes defined in the excludeFiles. -func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) error { +// Archive atomically creates a tar.gz to the v1alpha1.Artifact path from the given dir, +// excluding any VCS specific files and directories, or any of the excludes defined in +// the excludeFiles. +func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) (err error) { if _, err := os.Stat(dir); err != nil { return err } @@ -141,22 +144,53 @@ func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1. if err != nil { return err } - matcher := gitignore.NewMatcher(ps) - gzFile, err := os.Create(s.LocalPath(artifact)) + localPath := s.LocalPath(artifact) + tmpGzFile, err := ioutil.TempFile(filepath.Split(localPath)) if err != nil { return err } - defer gzFile.Close() - - gw := gzip.NewWriter(gzFile) - defer gw.Close() + tmpName := tmpGzFile.Name() + defer func() { + if err != nil { + os.Remove(tmpName) + } + }() + gw := gzip.NewWriter(tmpGzFile) tw := tar.NewWriter(gw) - defer tw.Close() + if err := writeToArchiveExcludeMatches(dir, matcher, tw); err != nil { + tw.Close() + gw.Close() + tmpGzFile.Close() + return err + } - return filepath.Walk(dir, func(p string, fi os.FileInfo, err error) error { + if err := tw.Close(); err != nil { + gw.Close() + tmpGzFile.Close() + return err + } + if err := gw.Close(); err != nil { + tmpGzFile.Close() + return err + } + if err := tmpGzFile.Close(); err != nil { + return err + } + + if err := os.Chmod(tmpName, 0644); err != nil { + return err + } + + return fs.RenameWithFallback(tmpName, localPath) +} + +// writeToArchiveExcludeMatches walks over the given dir and writes any regular file that does +// not match the given gitignore.Matcher. +func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer *tar.Writer) error { + fn := func(p string, fi os.FileInfo, err error) error { if err != nil { return err } @@ -187,33 +221,48 @@ func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1. } header.Name = relFilePath - if err := tw.WriteHeader(header); err != nil { + if err := writer.WriteHeader(header); err != nil { return err } f, err := os.Open(p) if err != nil { + f.Close() return err } - if _, err := io.Copy(tw, f); err != nil { + if _, err := io.Copy(writer, f); err != nil { f.Close() return err } return f.Close() - }) + } + return filepath.Walk(dir, fn) } -// WriteFile writes the given bytes to the artifact path if the checksum differs -func (s *Storage) WriteFile(artifact sourcev1.Artifact, data []byte) error { +// AtomicWriteFile atomically writes a file to the v1alpha1.Artifact Path. +func (s *Storage) AtomicWriteFile(artifact sourcev1.Artifact, reader io.Reader, mode os.FileMode) (err error) { localPath := s.LocalPath(artifact) - sum := s.Checksum(data) - if file, err := os.Stat(localPath); !os.IsNotExist(err) && !file.IsDir() { - if fb, err := ioutil.ReadFile(localPath); err == nil && sum == s.Checksum(fb) { - return nil - } + tmpFile, err := ioutil.TempFile(filepath.Split(localPath)) + if err != nil { + return err } - - return ioutil.WriteFile(localPath, data, 0644) + tmpName := tmpFile.Name() + defer func() { + if err != nil { + os.Remove(tmpName) + } + }() + if _, err := io.Copy(tmpFile, reader); err != nil { + tmpFile.Close() + return err + } + if err := tmpFile.Close(); err != nil { + return err + } + if err := os.Chmod(tmpName, mode); err != nil { + return err + } + return fs.RenameWithFallback(tmpName, localPath) } // Symlink creates or updates a symbolic link for the given artifact @@ -241,12 +290,14 @@ func (s *Storage) Symlink(artifact sourcev1.Artifact, linkName string) (string, return url, nil } -// Checksum returns the SHA1 checksum for the given bytes as a string -func (s *Storage) Checksum(b []byte) string { - return fmt.Sprintf("%x", sha1.Sum(b)) +// Checksum returns the SHA1 checksum for the data of the given io.Reader as a string. +func (s *Storage) Checksum(reader io.Reader) string { + h := sha1.New() + _, _ = io.Copy(h, reader) + return fmt.Sprintf("%x", h.Sum(nil)) } -// Lock creates a file lock for the given artifact +// Lock creates a file lock for the given v1alpha1.Artifact. func (s *Storage) Lock(artifact sourcev1.Artifact) (unlock func(), err error) { lockFile := s.LocalPath(artifact) + ".lock" mutex := lockedfile.MutexAt(lockFile) @@ -262,6 +313,8 @@ func (s *Storage) LocalPath(artifact sourcev1.Artifact) string { return filepath.Join(s.BasePath, artifact.Path) } +// getPatterns collects ignore patterns from the given reader and returns them +// as a gitignore.Pattern slice. func getPatterns(reader io.Reader, path []string) []gitignore.Pattern { var ps []gitignore.Pattern scanner := bufio.NewScanner(reader) diff --git a/internal/fs/LICENSE b/internal/fs/LICENSE new file mode 100644 index 00000000..a2dd15fa --- /dev/null +++ b/internal/fs/LICENSE @@ -0,0 +1,27 @@ +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 new file mode 100644 index 00000000..c8ece049 --- /dev/null +++ b/internal/fs/fs.go @@ -0,0 +1,346 @@ +// 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" + "io/ioutil" + "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 := ioutil.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 new file mode 100644 index 00000000..eba87eba --- /dev/null +++ b/internal/fs/fs_test.go @@ -0,0 +1,657 @@ +// 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" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "runtime" + "sync" + "testing" +) + +var ( + mu sync.Mutex +) + +func TestRenameWithFallback(t *testing.T) { + dir, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + 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, 0777); err != nil { + t.Fatal(err) + } + + dstpath := filepath.Join(dir, "b") + if err = os.MkdirAll(dstpath, 0777); 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + srcdir := filepath.Join(dir, "src") + if err := os.MkdirAll(srcdir, 0755); 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, 0755); 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 := ioutil.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 + + cleanup := setupInaccessibleDir(t, func(dir string) error { + srcdir = filepath.Join(dir, "src") + return os.MkdirAll(srcdir, 0755) + }) + defer cleanup() + + dir, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + srcdir = filepath.Join(dir, "src") + if err = os.MkdirAll(srcdir, 0755); err != nil { + t.Fatal(err) + } + + cleanup := setupInaccessibleDir(t, func(dir string) error { + dstdir = filepath.Join(dir, "dst") + return nil + }) + defer cleanup() + + 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + srcdir = filepath.Join(dir, "src") + if _, err = os.Create(srcdir); 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) + } + + 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + srcdir = filepath.Join(dir, "src") + if err = os.MkdirAll(srcdir, 0755); err != nil { + t.Fatal(err) + } + + dstdir = filepath.Join(dir, "dst") + if err = os.MkdirAll(dstdir, 0755); err != nil { + t.Fatal(err) + } + + if err = CopyDir(srcdir, dstdir); 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(..., 0222) 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + srcdir = filepath.Join(dir, "src") + if err = os.MkdirAll(srcdir, 0755); 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, 0222); 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + 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 := ioutil.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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + 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 := ioutil.ReadFile(symlink) + if err != nil { + t.Fatalf("%+v", err) + } + dstb, err := ioutil.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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer cleanUpDir(dir) + + // 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, 0755); err != nil && !os.IsExist(err) { + t.Fatalf("%+v", fmt.Errorf("unable to create temp directory: %s", fullPath)) + } + + err = ioutil.WriteFile(fullPath+"src", []byte(nil), 0644) + 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + srcf, err := os.Create(filepath.Join(dir, "srcfile")) + if err != nil { + t.Fatal(err) + } + srcf.Close() + + var dstdir string + + cleanup := setupInaccessibleDir(t, func(dir string) error { + dstdir = filepath.Join(dir, "dir") + return os.Mkdir(dstdir, 0777) + }) + defer cleanup() + + 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. +// +// This function returns a cleanup function that removes all the temporary +// files this function creates. It is the caller's responsibility to call +// this function before the test is done running, whether there's an error or not. +func setupInaccessibleDir(t *testing.T, op func(dir string) error) func() { + dir, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + return nil // keep compiler happy + } + + subdir := filepath.Join(dir, "dir") + + cleanup := func() { + if err := os.Chmod(subdir, 0777); err != nil { + t.Error(err) + } + if err := os.RemoveAll(dir); err != nil { + t.Error(err) + } + } + + if err := os.Mkdir(subdir, 0777); err != nil { + cleanup() + t.Fatal(err) + return nil + } + + if err := op(subdir); err != nil { + cleanup() + t.Fatal(err) + return nil + } + + if err := os.Chmod(subdir, 0666); err != nil { + cleanup() + t.Fatal(err) + return nil + } + + return cleanup +} + +func TestIsDir(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + + var dn string + + cleanup := setupInaccessibleDir(t, func(dir string) error { + dn = filepath.Join(dir, "dir") + return os.Mkdir(dn, 0777) + }) + defer cleanup() + + 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, err := ioutil.TempDir("", "dep") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + dirPath := filepath.Join(dir, "directory") + if err = os.MkdirAll(dirPath, 0777); 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 + ) + + cleanup := 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) + }) + defer cleanup() + + 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 new file mode 100644 index 00000000..a1b4a411 --- /dev/null +++ b/internal/fs/rename.go @@ -0,0 +1,30 @@ +// 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. + +// +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 new file mode 100644 index 00000000..3b565057 --- /dev/null +++ b/internal/fs/rename_windows.go @@ -0,0 +1,41 @@ +// 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. + +// +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 new file mode 120000 index 00000000..777ebd01 --- /dev/null +++ b/internal/fs/testdata/symlinks/dir-symlink @@ -0,0 +1 @@ +../../testdata \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/file-symlink b/internal/fs/testdata/symlinks/file-symlink new file mode 120000 index 00000000..4c52274d --- /dev/null +++ b/internal/fs/testdata/symlinks/file-symlink @@ -0,0 +1 @@ +../test.file \ No newline at end of file diff --git a/internal/fs/testdata/symlinks/invalid-symlink b/internal/fs/testdata/symlinks/invalid-symlink new file mode 120000 index 00000000..0edf4f30 --- /dev/null +++ b/internal/fs/testdata/symlinks/invalid-symlink @@ -0,0 +1 @@ +/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 new file mode 120000 index 00000000..af1d6c8f --- /dev/null +++ b/internal/fs/testdata/symlinks/windows-file-symlink @@ -0,0 +1 @@ +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 new file mode 100644 index 00000000..e69de29b From 42706a342be52902674d089e755763ecd4ccd9c7 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 10 Sep 2020 11:55:59 +0200 Subject: [PATCH 2/5] Calculate checksums during file writes --- api/v1alpha1/gitrepository_types.go | 79 +++++------- api/v1alpha1/helmchart_types.go | 2 +- api/v1alpha1/helmrepository_types.go | 79 +++++------- controllers/gitrepository_controller.go | 54 ++------- controllers/helmchart_controller.go | 54 +++++---- controllers/helmrepository_controller.go | 60 +++------ controllers/storage.go | 148 ++++++++++++++++------- controllers/storage_test.go | 2 +- go.mod | 2 +- go.sum | 4 +- 10 files changed, 230 insertions(+), 254 deletions(-) diff --git a/api/v1alpha1/gitrepository_types.go b/api/v1alpha1/gitrepository_types.go index 2be87e6b..1226c77a 100644 --- a/api/v1alpha1/gitrepository_types.go +++ b/api/v1alpha1/gitrepository_types.go @@ -122,62 +122,45 @@ const ( GitOperationFailedReason string = "GitOperationFailed" ) -// GitRepositoryReady sets the given artifact and url on the -// GitRepository and resets the conditions to SourceCondition of -// type Ready with status true and the given reason and message. -// It returns the modified GitRepository. -func GitRepositoryReady(repository GitRepository, artifact Artifact, url, reason, message string) GitRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionTrue, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } - repository.Status.URL = url - - if repository.Status.Artifact != nil { - if repository.Status.Artifact.Path != artifact.Path { - repository.Status.Artifact = &artifact - } - } else { - repository.Status.Artifact = &artifact - } - - return repository -} - // GitRepositoryProgressing resets the conditions of the GitRepository // to SourceCondition of type Ready with status unknown and // progressing reason and message. It returns the modified GitRepository. func GitRepositoryProgressing(repository GitRepository) GitRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionUnknown, - LastTransitionTime: metav1.Now(), - Reason: ProgressingReason, - Message: "reconciliation in progress", - }, - } + repository.Status.URL = "" + repository.Status.Artifact = nil + repository.Status.Conditions = []SourceCondition{} + SetGitRepositoryCondition(&repository, ReadyCondition, corev1.ConditionUnknown, ProgressingReason, "reconciliation in progress") return repository } -// GitRepositoryNotReady resets the conditions of the GitRepository -// to SourceCondition of type Ready with status false and the given -// reason and message. It returns the modified GitRepository. +// SetGitRepositoryCondition sets the given condition with the given status, reason and message +// on the GitRepository. +func SetGitRepositoryCondition(repository *GitRepository, condition string, status corev1.ConditionStatus, reason, message string) { + repository.Status.Conditions = filterOutSourceCondition(repository.Status.Conditions, condition) + repository.Status.Conditions = append(repository.Status.Conditions, SourceCondition{ + Type: condition, + Status: status, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + }) +} + +// GitRepositoryReady sets the given artifact and url on the GitRepository +// and sets the ReadyCondition to True, with the given reason and +// message. It returns the modified GitRepository. +func GitRepositoryReady(repository GitRepository, artifact Artifact, url, reason, message string) GitRepository { + repository.Status.Artifact = &artifact + repository.Status.URL = url + SetGitRepositoryCondition(&repository, ReadyCondition, corev1.ConditionTrue, reason, message) + return repository +} + +// GitRepositoryNotReady sets the ReadyCondition on the given GitRepository +// to False, with the given reason and message. It returns the modified +// GitRepository. func GitRepositoryNotReady(repository GitRepository, reason, message string) GitRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionFalse, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } + SetGitRepositoryCondition(&repository, ReadyCondition, corev1.ConditionFalse, reason, message) return repository } diff --git a/api/v1alpha1/helmchart_types.go b/api/v1alpha1/helmchart_types.go index f58f9be3..487928e3 100644 --- a/api/v1alpha1/helmchart_types.go +++ b/api/v1alpha1/helmchart_types.go @@ -92,7 +92,7 @@ const ( ChartPackageSucceededReason string = "ChartPackageSucceeded" ) -// HelmReleaseProgressing resets any failures and registers progress toward reconciling the given HelmRelease +// HelmChartProgressing resets any failures and registers progress toward reconciling the given HelmChart // by setting the ReadyCondition to ConditionUnknown for ProgressingReason. func HelmChartProgressing(chart HelmChart) HelmChart { chart.Status.URL = "" diff --git a/api/v1alpha1/helmrepository_types.go b/api/v1alpha1/helmrepository_types.go index b34f6b63..f9d16f11 100644 --- a/api/v1alpha1/helmrepository_types.go +++ b/api/v1alpha1/helmrepository_types.go @@ -76,62 +76,45 @@ const ( IndexationSucceededReason string = "IndexationSucceed" ) -// HelmRepositoryReady sets the given artifact and url on the -// HelmRepository and resets the conditions to SourceCondition of -// type Ready with status true and the given reason and message. -// It returns the modified HelmRepository. -func HelmRepositoryReady(repository HelmRepository, artifact Artifact, url, reason, message string) HelmRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionTrue, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } - repository.Status.URL = url - - if repository.Status.Artifact != nil { - if repository.Status.Artifact.Path != artifact.Path { - repository.Status.Artifact = &artifact - } - } else { - repository.Status.Artifact = &artifact - } - - return repository -} - // HelmRepositoryProgressing resets the conditions of the HelmRepository // to SourceCondition of type Ready with status unknown and // progressing reason and message. It returns the modified HelmRepository. func HelmRepositoryProgressing(repository HelmRepository) HelmRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionUnknown, - LastTransitionTime: metav1.Now(), - Reason: ProgressingReason, - Message: "reconciliation in progress", - }, - } + repository.Status.URL = "" + repository.Status.Artifact = nil + repository.Status.Conditions = []SourceCondition{} + SetHelmRepositoryCondition(&repository, ReadyCondition, corev1.ConditionUnknown, ProgressingReason, "reconciliation in progress") return repository } -// HelmRepositoryNotReady resets the conditions of the HelmRepository -// to SourceCondition of type Ready with status false and the given -// reason and message. It returns the modified HelmRepository. +// SetHelmRepositoryCondition sets the given condition with the given status, +// reason and message on the HelmRepository. +func SetHelmRepositoryCondition(repository *HelmRepository, condition string, status corev1.ConditionStatus, reason, message string) { + repository.Status.Conditions = filterOutSourceCondition(repository.Status.Conditions, condition) + repository.Status.Conditions = append(repository.Status.Conditions, SourceCondition{ + Type: condition, + Status: status, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + }) +} + +// HelmRepositoryReady sets the given artifact and url on the HelmRepository +// and sets the ReadyCondition to True, with the given reason and +// message. It returns the modified HelmRepository. +func HelmRepositoryReady(repository HelmRepository, artifact Artifact, url, reason, message string) HelmRepository { + repository.Status.Artifact = &artifact + repository.Status.URL = url + SetHelmRepositoryCondition(&repository, ReadyCondition, corev1.ConditionTrue, reason, message) + return repository +} + +// HelmRepositoryNotReady sets the ReadyCondition on the given HelmRepository +// to False, with the given reason and message. It returns the modified +// HelmRepository. func HelmRepositoryNotReady(repository HelmRepository, reason, message string) HelmRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionFalse, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } + SetHelmRepositoryCondition(&repository, ReadyCondition, corev1.ConditionFalse, reason, message) return repository } diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index d88f02b1..ff6a44cc 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -28,7 +28,6 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" @@ -100,13 +99,7 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro } // set initial status - if reset, status := r.shouldResetStatus(repository); reset { - repository.Status = status - if err := r.Status().Update(ctx, &repository); err != nil { - log.Error(err, "unable to update status") - return ctrl.Result{Requeue: true}, err - } - } else { + if repository.Generation == 0 || repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { repository = sourcev1.GitRepositoryProgressing(repository) if err := r.Status().Update(ctx, &repository); err != nil { log.Error(err, "unable to update status") @@ -202,6 +195,12 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour return sourcev1.GitRepositoryNotReady(repository, sourcev1.GitOperationFailedReason, err.Error()), err } + // return early on unchanged revision + artifact := r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), revision, fmt.Sprintf("%s.tar.gz", commit.Hash.String())) + if repository.GetArtifact() != nil && repository.GetArtifact().Revision == revision { + return repository, nil + } + // verify PGP signature if repository.Spec.Verification != nil { err := r.verify(ctx, types.NamespacedName{ @@ -213,11 +212,6 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour } } - // TODO(hidde): implement checksum when https://github.com/fluxcd/source-controller/pull/133 - // has been merged. - artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), - fmt.Sprintf("%s.tar.gz", commit.Hash.String()), revision, "") - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { @@ -234,7 +228,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour defer unlock() // archive artifact and check integrity - if err := r.Storage.Archive(artifact, tmpGit, repository.Spec); err != nil { + if err := r.Storage.Archive(&artifact, tmpGit, repository.Spec); err != nil { err = fmt.Errorf("storage archive error: %w", err) return sourcev1.GitRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err } @@ -250,32 +244,6 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour return sourcev1.GitRepositoryReady(repository, artifact, url, sourcev1.GitOperationSucceedReason, message), nil } -// shouldResetStatus returns a boolean indicating if the status of the -// given repository should be reset. -func (r *GitRepositoryReconciler) shouldResetStatus(repository sourcev1.GitRepository) (bool, sourcev1.GitRepositoryStatus) { - resetStatus := false - if repository.Status.Artifact != nil { - if !r.Storage.ArtifactExist(*repository.Status.Artifact) { - resetStatus = true - } - } - - if len(repository.Status.Conditions) == 0 || resetStatus { - resetStatus = true - } - - return resetStatus, sourcev1.GitRepositoryStatus{ - Conditions: []sourcev1.SourceCondition{ - { - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionUnknown, - Reason: sourcev1.InitializingReason, - LastTransitionTime: metav1.Now(), - }, - }, - } -} - // verify returns an error if the PGP signature can't be verified func (r *GitRepositoryReconciler) verify(ctx context.Context, publicKeySecret types.NamespacedName, commit *object.Commit) error { if commit.PGPSignature == "" { @@ -304,10 +272,10 @@ func (r *GitRepositoryReconciler) verify(ctx context.Context, publicKeySecret ty // the given repository. func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository, all bool) error { if all { - return r.Storage.RemoveAll(r.Storage.ArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "", "")) + return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "")) } - if repository.Status.Artifact != nil { - return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact) + if repository.GetArtifact() != nil { + return r.Storage.RemoveAllButCurrent(*repository.GetArtifact()) } return nil } diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 24b7858f..23d72423 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -17,15 +17,12 @@ limitations under the License. package controllers import ( - "bytes" "context" "fmt" - "io" "io/ioutil" "net/url" "os" "path" - "path/filepath" "strings" "time" @@ -197,6 +194,11 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err } + // return early on unchanged chart version + if repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version { + return chart, nil + } + // TODO(hidde): according to the Helm source the first item is not // always the correct one to pick, check for updates once in awhile. // Ref: https://github.com/helm/helm/blob/v3.3.0/pkg/downloader/chart_downloader.go#L241 @@ -257,10 +259,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err } - var buf bytes.Buffer - sum := r.Storage.Checksum(io.TeeReader(res, &buf)) - artifact := r.Storage.ArtifactFor(chart.Kind, chart.GetObjectMeta(), - fmt.Sprintf("%s-%s-%s.tgz", cv.Name, cv.Version, sum), cv.Version, sum) + artifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version)) // create artifact dir err = r.Storage.MkdirAll(artifact) @@ -278,8 +277,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, defer unlock() // save artifact to storage - err = r.Storage.AtomicWriteFile(artifact, &buf, 0644) - if err != nil { + if err := r.Storage.AtomicWriteFile(&artifact, res, 0644); err != nil { err = fmt.Errorf("unable to write chart file: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } @@ -315,7 +313,7 @@ func (r *HelmChartReconciler) getChartRepositoryWithArtifact(ctx context.Context return repository, err } - if repository.Status.Artifact == nil { + if repository.GetArtifact() == nil { err = fmt.Errorf("no repository index artifact found in HelmRepository '%s'", repository.Name) } @@ -360,14 +358,11 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, } // return early on unchanged chart version - if chart.Status.Artifact != nil && chartMetadata.Version == chart.Status.Artifact.Revision { + if chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version { return chart, nil } - // TODO(hidde): implement checksum when https://github.com/fluxcd/source-controller/pull/133 - // has been merged. - artifact := r.Storage.ArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), - fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version), chartMetadata.Version, "") + artifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version, fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version)) // create artifact dir err = r.Storage.MkdirAll(artifact) @@ -386,22 +381,35 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, // package chart pkg := action.NewPackage() - pkg.Destination = filepath.Dir(r.Storage.LocalPath(artifact)) - _, err = pkg.Run(chartPath, nil) + pkg.Destination = tmpDir + src, err := pkg.Run(chartPath, nil) if err != nil { err = fmt.Errorf("chart package error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err } + // copy chart package + cf, err := os.Open(src) + if err != nil { + err = fmt.Errorf("failed to open chart package: %w", err) + return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + } + if err := r.Storage.Copy(&artifact, cf); err != nil { + cf.Close() + err = fmt.Errorf("failed to copy chart package to storage: %w", err) + return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err + } + cf.Close() + // update symlink - chartUrl, err := r.Storage.Symlink(artifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name)) + cUrl, err := r.Storage.Symlink(artifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name)) if err != nil { err = fmt.Errorf("storage error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } message := fmt.Sprintf("Fetched and packaged revision: %s", artifact.Revision) - return sourcev1.HelmChartReady(chart, artifact, chartUrl, sourcev1.ChartPackageSucceededReason, message), nil + return sourcev1.HelmChartReady(chart, artifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil } // getGitRepositoryWithArtifact attempts to get the GitRepository for the given @@ -424,7 +432,7 @@ func (r *HelmChartReconciler) getGitRepositoryWithArtifact(ctx context.Context, return repository, err } - if repository.Status.Artifact == nil { + if repository.GetArtifact() == nil { err = fmt.Errorf("no artifact found for GitRepository '%s'", repository.Name) } @@ -435,10 +443,10 @@ func (r *HelmChartReconciler) getGitRepositoryWithArtifact(ctx context.Context, // the given chart. func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart, all bool) error { if all { - return r.Storage.RemoveAll(r.Storage.ArtifactFor(chart.Kind, chart.GetObjectMeta(), "", "", "")) + return r.Storage.RemoveAll(r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), "", "")) } - if chart.Status.Artifact != nil { - return r.Storage.RemoveAllButCurrent(*chart.Status.Artifact) + if chart.GetArtifact() != nil { + return r.Storage.RemoveAllButCurrent(*chart.GetArtifact()) } return nil } diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index e5696352..50de5f9b 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -30,7 +30,6 @@ import ( "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" @@ -42,6 +41,7 @@ import ( "github.com/fluxcd/pkg/recorder" "github.com/fluxcd/pkg/runtime/predicates" + sourcev1 "github.com/fluxcd/source-controller/api/v1alpha1" "github.com/fluxcd/source-controller/internal/helm" ) @@ -104,13 +104,7 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err } // set initial status - if reset, status := r.shouldResetStatus(repository); reset { - repository.Status = status - if err := r.Status().Update(ctx, &repository); err != nil { - log.Error(err, "unable to update status") - return ctrl.Result{Requeue: true}, err - } - } else { + if repository.Generation == 0 || repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { repository = sourcev1.HelmRepositoryProgressing(repository) if err := r.Status().Update(ctx, &repository); err != nil { log.Error(err, "unable to update status") @@ -207,7 +201,6 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou } clientOpts = append(clientOpts, getter.WithTimeout(repository.GetTimeout())) - res, err := c.Get(u.String(), clientOpts...) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err @@ -217,21 +210,24 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - i := repo.IndexFile{} if err := yaml.Unmarshal(b, &i); err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - i.SortEntries() + // return early on unchanged generation + if repository.GetArtifact() != nil && repository.GetArtifact().Revision == i.Generated.Format(time.RFC3339Nano) { + return repository, nil + } + + i.SortEntries() b, err = yaml.Marshal(&i) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - sum := r.Storage.Checksum(bytes.NewReader(b)) - artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), - fmt.Sprintf("index-%s.yaml", sum), i.Generated.Format(time.RFC3339Nano), sum) + artifact := r.Storage.NewArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), i.Generated.Format(time.RFC3339Nano), + fmt.Sprintf("index-%s.yaml", url.PathEscape(i.Generated.Format(time.RFC3339Nano)))) // create artifact dir err = r.Storage.MkdirAll(artifact) @@ -249,8 +245,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou defer unlock() // save artifact to storage - err = r.Storage.AtomicWriteFile(artifact, bytes.NewReader(b), 0644) - if err != nil { + if err := r.Storage.AtomicWriteFile(&artifact, bytes.NewReader(b), 0644); err != nil { err = fmt.Errorf("unable to write repository index file: %w", err) return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err } @@ -266,41 +261,14 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou return sourcev1.HelmRepositoryReady(repository, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil } -// shouldResetStatus returns a boolean indicating if the status of the -// given repository should be reset. -func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRepository) (bool, sourcev1.HelmRepositoryStatus) { - resetStatus := false - if repository.Status.Artifact != nil { - if !r.Storage.ArtifactExist(*repository.Status.Artifact) { - resetStatus = true - } - } - - // set initial status - if len(repository.Status.Conditions) == 0 { - resetStatus = true - } - - return resetStatus, sourcev1.HelmRepositoryStatus{ - Conditions: []sourcev1.SourceCondition{ - { - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionUnknown, - Reason: sourcev1.InitializingReason, - LastTransitionTime: metav1.Now(), - }, - }, - } -} - // gc performs a garbage collection on all but current artifacts of // the given repository. func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository, all bool) error { if all { - return r.Storage.RemoveAll(r.Storage.ArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "", "")) + return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "")) } - if repository.Status.Artifact != nil { - return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact) + if repository.GetArtifact() != nil { + return r.Storage.RemoveAllButCurrent(*repository.GetArtifact()) } return nil } diff --git a/controllers/storage.go b/controllers/storage.go index 6435de27..15648411 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -23,6 +23,7 @@ import ( "compress/gzip" "crypto/sha1" "fmt" + "hash" "io" "io/ioutil" "os" @@ -62,7 +63,6 @@ func NewStorage(basePath string, hostname string, timeout time.Duration) (*Stora if f, err := os.Stat(basePath); os.IsNotExist(err) || !f.IsDir() { return nil, fmt.Errorf("invalid dir path: %s", basePath) } - return &Storage{ BasePath: basePath, Hostname: hostname, @@ -70,18 +70,23 @@ func NewStorage(basePath string, hostname string, timeout time.Duration) (*Stora }, nil } -// ArtifactFor returns an artifact for the v1alpha1.Source. -func (s *Storage) ArtifactFor(kind string, metadata metav1.Object, fileName, revision, checksum string) sourcev1.Artifact { +// NewArtifactFor returns a new v1alpha1.Artifact. +func (s *Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) sourcev1.Artifact { path := sourcev1.ArtifactPath(kind, metadata.GetNamespace(), metadata.GetName(), fileName) - url := fmt.Sprintf("http://%s/%s", s.Hostname, path) - - return sourcev1.Artifact{ - Path: path, - URL: url, - Revision: revision, - Checksum: checksum, - LastUpdateTime: metav1.Now(), + artifact := sourcev1.Artifact{ + Path: path, + Revision: revision, } + s.SetArtifactURL(&artifact) + return artifact +} + +// SetArtifactURL sets the URL on the given v1alpha1.Artifact. +func (s Storage) SetArtifactURL(artifact *sourcev1.Artifact) { + if artifact.Path == "" { + return + } + artifact.URL = fmt.Sprintf("http://%s/%s", s.Hostname, artifact.Path) } // MkdirAll calls os.MkdirAll for the given v1alpha1.Artifact base dir. @@ -132,12 +137,12 @@ func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { return fi.Mode().IsRegular() } -// Archive atomically creates a tar.gz to the v1alpha1.Artifact path from the given dir, -// excluding any VCS specific files and directories, or any of the excludes defined in -// the excludeFiles. -func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) (err error) { - if _, err := os.Stat(dir); err != nil { - return err +// Archive atomically archives the given directory as a tarball to the given v1alpha1.Artifact +// path, excluding any VCS specific files and directories, or any of the excludes defined in +// the excludeFiles. If successful, it sets the checksum and last update time on the artifact. +func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) (err error) { + if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() { + return fmt.Errorf("invalid dir path: %s", dir) } ps, err := loadExcludePatterns(dir, spec) @@ -146,37 +151,40 @@ func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1. } matcher := gitignore.NewMatcher(ps) - localPath := s.LocalPath(artifact) - tmpGzFile, err := ioutil.TempFile(filepath.Split(localPath)) + localPath := s.LocalPath(*artifact) + tf, err := ioutil.TempFile(filepath.Split(localPath)) if err != nil { return err } - tmpName := tmpGzFile.Name() + tmpName := tf.Name() defer func() { if err != nil { os.Remove(tmpName) } }() - gw := gzip.NewWriter(tmpGzFile) + h := newHash() + mw := io.MultiWriter(h, tf) + + gw := gzip.NewWriter(mw) tw := tar.NewWriter(gw) if err := writeToArchiveExcludeMatches(dir, matcher, tw); err != nil { tw.Close() gw.Close() - tmpGzFile.Close() + tf.Close() return err } if err := tw.Close(); err != nil { gw.Close() - tmpGzFile.Close() + tf.Close() return err } if err := gw.Close(); err != nil { - tmpGzFile.Close() + tf.Close() return err } - if err := tmpGzFile.Close(); err != nil { + if err := tf.Close(); err != nil { return err } @@ -184,7 +192,13 @@ func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1. return err } - return fs.RenameWithFallback(tmpName, localPath) + if err := fs.RenameWithFallback(tmpName, localPath); err != nil { + return err + } + + artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil)) + artifact.LastUpdateTime = metav1.Now() + return nil } // writeToArchiveExcludeMatches walks over the given dir and writes any regular file that does @@ -239,33 +253,81 @@ func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer return filepath.Walk(dir, fn) } -// AtomicWriteFile atomically writes a file to the v1alpha1.Artifact Path. -func (s *Storage) AtomicWriteFile(artifact sourcev1.Artifact, reader io.Reader, mode os.FileMode) (err error) { - localPath := s.LocalPath(artifact) - tmpFile, err := ioutil.TempFile(filepath.Split(localPath)) +// AtomicWriteFile atomically writes the io.Reader contents to the v1alpha1.Artifact path. +// If successful, it sets the checksum and last update time on the artifact. +func (s *Storage) AtomicWriteFile(artifact *sourcev1.Artifact, reader io.Reader, mode os.FileMode) (err error) { + localPath := s.LocalPath(*artifact) + tf, err := ioutil.TempFile(filepath.Split(localPath)) if err != nil { return err } - tmpName := tmpFile.Name() + tfName := tf.Name() defer func() { if err != nil { - os.Remove(tmpName) + os.Remove(tfName) } }() - if _, err := io.Copy(tmpFile, reader); err != nil { - tmpFile.Close() + + h := newHash() + mw := io.MultiWriter(h, tf) + + if _, err := io.Copy(mw, reader); err != nil { + tf.Close() return err } - if err := tmpFile.Close(); err != nil { + if err := tf.Close(); err != nil { return err } - if err := os.Chmod(tmpName, mode); err != nil { + + if err := os.Chmod(tfName, mode); err != nil { return err } - return fs.RenameWithFallback(tmpName, localPath) + + if err := fs.RenameWithFallback(tfName, localPath); err != nil { + return err + } + + artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil)) + artifact.LastUpdateTime = metav1.Now() + return nil } -// Symlink creates or updates a symbolic link for the given artifact +// Copy atomically copies the io.Reader contents to the v1alpha1.Artifact path. +// If successful, it sets the checksum and last update time on the artifact. +func (s *Storage) Copy(artifact *sourcev1.Artifact, reader io.Reader) (err error) { + localPath := s.LocalPath(*artifact) + tf, err := ioutil.TempFile(filepath.Split(localPath)) + if err != nil { + return err + } + tfName := tf.Name() + defer func() { + if err != nil { + os.Remove(tfName) + } + }() + + h := newHash() + mw := io.MultiWriter(h, tf) + + if _, err := io.Copy(mw, reader); err != nil { + tf.Close() + return err + } + if err := tf.Close(); err != nil { + return err + } + + if err := fs.RenameWithFallback(tfName, localPath); err != nil { + return err + } + + artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil)) + artifact.LastUpdateTime = metav1.Now() + return nil +} + +// Symlink creates or updates a symbolic link for the given v1alpha1.Artifact // and returns the URL for the symlink. func (s *Storage) Symlink(artifact sourcev1.Artifact, linkName string) (string, error) { localPath := s.LocalPath(artifact) @@ -285,14 +347,13 @@ func (s *Storage) Symlink(artifact sourcev1.Artifact, linkName string) (string, return "", err } - parts := strings.Split(artifact.URL, "/") - url := strings.Replace(artifact.URL, parts[len(parts)-1], linkName, 1) + url := fmt.Sprintf("http://%s/%s", s.Hostname, filepath.Join(filepath.Dir(artifact.Path), linkName)) return url, nil } // Checksum returns the SHA1 checksum for the data of the given io.Reader as a string. func (s *Storage) Checksum(reader io.Reader) string { - h := sha1.New() + h := newHash() _, _ = io.Copy(h, reader) return fmt.Sprintf("%x", h.Sum(nil)) } @@ -356,3 +417,8 @@ func loadExcludePatterns(dir string, spec sourcev1.GitRepositorySpec) ([]gitigno return ps, nil } + +// newHash returns a new SHA1 hash. +func newHash() hash.Hash { + return sha1.New() +} diff --git a/controllers/storage_test.go b/controllers/storage_test.go index 307f8676..a90a7229 100644 --- a/controllers/storage_test.go +++ b/controllers/storage_test.go @@ -159,7 +159,7 @@ func createArchive(t *testing.T, storage *Storage, filenames []string, sourceIgn t.Fatalf("artifact directory creation failed: %v", err) } - if err := storage.Archive(artifact, gitDir, spec); err != nil { + if err := storage.Archive(&artifact, gitDir, spec); err != nil { t.Fatalf("archiving failed: %v", err) } diff --git a/go.mod b/go.mod index 171657c2..18c7f7a8 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/fluxcd/pkg/helmtestserver v0.0.1 github.com/fluxcd/pkg/lockedfile v0.0.5 github.com/fluxcd/pkg/recorder v0.0.6 - github.com/fluxcd/pkg/runtime v0.0.0-20200909163337-e7e634246495 + github.com/fluxcd/pkg/runtime v0.0.1 github.com/fluxcd/pkg/ssh v0.0.5 github.com/fluxcd/pkg/untar v0.0.5 github.com/fluxcd/source-controller/api v0.0.14 diff --git a/go.sum b/go.sum index 87881d05..9ef50f4a 100644 --- a/go.sum +++ b/go.sum @@ -210,8 +210,8 @@ github.com/fluxcd/pkg/lockedfile v0.0.5 h1:C3T8wfdff1UY1bvplmCkGOLrdMWJHO8Q8+tdl github.com/fluxcd/pkg/lockedfile v0.0.5/go.mod h1:uAtPUBId6a2RqO84MTH5HKGX0SbM1kNW3Wr/FhYyDVA= github.com/fluxcd/pkg/recorder v0.0.6 h1:me/n8syeeGXz50OXoPX3jgIj9AtinvhHdKT9Dy+MbHs= github.com/fluxcd/pkg/recorder v0.0.6/go.mod h1:IfQxfVRSNsWs3B0Yp5B6ObEWwKHILlAx8N7XkoDdhFg= -github.com/fluxcd/pkg/runtime v0.0.0-20200909163337-e7e634246495 h1:zhtLz8iRtJWK+jKq9vi9Si4QbcAC2KvQZpQ55DRzLsU= -github.com/fluxcd/pkg/runtime v0.0.0-20200909163337-e7e634246495/go.mod h1:cU1t0+Ld39pZjMyrrHukw1E++OZFNHxG2qAExfDWQ34= +github.com/fluxcd/pkg/runtime v0.0.1 h1:h8jztHVF9UMGD7XBQSfXDdw80bpT6BOkd0xe4kknPL0= +github.com/fluxcd/pkg/runtime v0.0.1/go.mod h1:cU1t0+Ld39pZjMyrrHukw1E++OZFNHxG2qAExfDWQ34= github.com/fluxcd/pkg/ssh v0.0.5 h1:rnbFZ7voy2JBlUfMbfyqArX2FYaLNpDhccGFC3qW83A= github.com/fluxcd/pkg/ssh v0.0.5/go.mod h1:7jXPdXZpc0ttMNz2kD9QuMi3RNn/e0DOFbj0Tij/+Hs= github.com/fluxcd/pkg/testserver v0.0.2 h1:SoaMtO9cE5p/wl2zkGudzflnEHd9mk68CGjZOo7w0Uk= From 7a3a5938d384da9db0958c5c7b760e72091c37d0 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 10 Sep 2020 13:53:26 +0200 Subject: [PATCH 3/5] Mark resources as progressing on spec changes --- api/v1alpha1/gitrepository_types.go | 6 +++ api/v1alpha1/helmchart_types.go | 6 +++ api/v1alpha1/helmrepository_types.go | 6 +++ ...rce.toolkit.fluxcd.io_gitrepositories.yaml | 5 +++ .../source.toolkit.fluxcd.io_helmcharts.yaml | 5 +++ ...ce.toolkit.fluxcd.io_helmrepositories.yaml | 5 +++ controllers/gitrepository_controller.go | 3 +- controllers/helmchart_controller.go | 3 +- controllers/helmrepository_controller.go | 3 +- docs/api/source.md | 39 +++++++++++++++++++ 10 files changed, 78 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/gitrepository_types.go b/api/v1alpha1/gitrepository_types.go index 1226c77a..c795765e 100644 --- a/api/v1alpha1/gitrepository_types.go +++ b/api/v1alpha1/gitrepository_types.go @@ -99,6 +99,11 @@ type GitRepositoryVerification struct { // GitRepositoryStatus defines the observed state of a Git repository. type GitRepositoryStatus struct { + // ObservedGeneration is the last observed generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions holds the conditions for the GitRepository. // +optional Conditions []SourceCondition `json:"conditions,omitempty"` @@ -126,6 +131,7 @@ const ( // to SourceCondition of type Ready with status unknown and // progressing reason and message. It returns the modified GitRepository. func GitRepositoryProgressing(repository GitRepository) GitRepository { + repository.Status.ObservedGeneration = repository.Generation repository.Status.URL = "" repository.Status.Artifact = nil repository.Status.Conditions = []SourceCondition{} diff --git a/api/v1alpha1/helmchart_types.go b/api/v1alpha1/helmchart_types.go index 487928e3..b398fbfd 100644 --- a/api/v1alpha1/helmchart_types.go +++ b/api/v1alpha1/helmchart_types.go @@ -62,6 +62,11 @@ type LocalHelmChartSourceReference struct { // HelmChartStatus defines the observed state of the HelmChart. type HelmChartStatus struct { + // ObservedGeneration is the last observed generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions holds the conditions for the HelmChart. // +optional Conditions []SourceCondition `json:"conditions,omitempty"` @@ -95,6 +100,7 @@ const ( // HelmChartProgressing resets any failures and registers progress toward reconciling the given HelmChart // by setting the ReadyCondition to ConditionUnknown for ProgressingReason. func HelmChartProgressing(chart HelmChart) HelmChart { + chart.Status.ObservedGeneration = chart.Generation chart.Status.URL = "" chart.Status.Artifact = nil chart.Status.Conditions = []SourceCondition{} diff --git a/api/v1alpha1/helmrepository_types.go b/api/v1alpha1/helmrepository_types.go index f9d16f11..1870ff32 100644 --- a/api/v1alpha1/helmrepository_types.go +++ b/api/v1alpha1/helmrepository_types.go @@ -54,6 +54,11 @@ type HelmRepositorySpec struct { // HelmRepositoryStatus defines the observed state of the HelmRepository. type HelmRepositoryStatus struct { + // ObservedGeneration is the last observed generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions holds the conditions for the HelmRepository. // +optional Conditions []SourceCondition `json:"conditions,omitempty"` @@ -80,6 +85,7 @@ const ( // to SourceCondition of type Ready with status unknown and // progressing reason and message. It returns the modified HelmRepository. func HelmRepositoryProgressing(repository HelmRepository) HelmRepository { + repository.Status.ObservedGeneration = repository.Generation repository.Status.URL = "" repository.Status.Artifact = nil repository.Status.Conditions = []SourceCondition{} diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml index e60d5852..81a2249b 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml @@ -153,6 +153,7 @@ spec: - url type: object conditions: + description: Conditions holds the conditions for the GitRepository. items: description: SourceCondition contains condition information for a source. @@ -182,6 +183,10 @@ spec: - type type: object type: array + observedGeneration: + description: ObservedGeneration is the last observed generation. + format: int64 + type: integer url: description: URL is the download link for the artifact output of the last repository sync. diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml index d1125575..b0a22796 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml @@ -125,6 +125,7 @@ spec: - url type: object conditions: + description: Conditions holds the conditions for the HelmChart. items: description: SourceCondition contains condition information for a source. @@ -154,6 +155,10 @@ spec: - type type: object type: array + observedGeneration: + description: ObservedGeneration is the last observed generation. + format: int64 + type: integer url: description: URL is the download link for the last chart pulled. type: string diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index 8f7f675e..25016356 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -105,6 +105,7 @@ spec: - url type: object conditions: + description: Conditions holds the conditions for the HelmRepository. items: description: SourceCondition contains condition information for a source. @@ -134,6 +135,10 @@ spec: - type type: object type: array + observedGeneration: + description: ObservedGeneration is the last observed generation. + format: int64 + type: integer url: description: URL is the download link for the last index fetched. type: string diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index ff6a44cc..d1d0815a 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -99,7 +99,8 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro } // set initial status - if repository.Generation == 0 || repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { + if repository.Generation != repository.Status.ObservedGeneration || + repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { repository = sourcev1.GitRepositoryProgressing(repository) if err := r.Status().Update(ctx, &repository); err != nil { log.Error(err, "unable to update status") diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 23d72423..76c42613 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -104,7 +104,8 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } // set initial status - if chart.Generation == 0 || chart.GetArtifact() != nil && !r.Storage.ArtifactExist(*chart.GetArtifact()) { + if chart.Generation != chart.Status.ObservedGeneration || + chart.GetArtifact() != nil && !r.Storage.ArtifactExist(*chart.GetArtifact()) { chart = sourcev1.HelmChartProgressing(chart) if err := r.Status().Update(ctx, &chart); err != nil { log.Error(err, "unable to update status") diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 50de5f9b..7e7b6238 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -104,7 +104,8 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err } // set initial status - if repository.Generation == 0 || repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { + if repository.Generation != repository.Status.ObservedGeneration || + repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { repository = sourcev1.HelmRepositoryProgressing(repository) if err := r.Status().Update(ctx, &repository); err != nil { log.Error(err, "unable to update status") diff --git a/docs/api/source.md b/docs/api/source.md index b7294757..49c6bfd0 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -744,6 +744,18 @@ are.

+observedGeneration
+ +int64 + + + +(Optional) +

ObservedGeneration is the last observed generation.

+ + + + conditions
@@ -753,6 +765,7 @@ are.

(Optional) +

Conditions holds the conditions for the GitRepository.

@@ -921,6 +934,18 @@ Kubernetes meta/v1.Duration +observedGeneration
+ +int64 + + + +(Optional) +

ObservedGeneration is the last observed generation.

+ + + + conditions
 @@ -930,6 +955,7 @@ Kubernetes meta/v1.Duration (Optional) +

Conditions holds the conditions for the HelmChart.

@@ -1059,6 +1085,18 @@ Kubernetes meta/v1.Duration +observedGeneration
+ +int64 + + + +(Optional) +

ObservedGeneration is the last observed generation.

+ + + + conditions
 @@ -1068,6 +1106,7 @@ Kubernetes meta/v1.Duration (Optional) +

Conditions holds the conditions for the HelmRepository.

From d03f4fa4c46a71743e2cdfec92ad1b67fbed45a3 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 10 Sep 2020 14:11:38 +0200 Subject: [PATCH 4/5] Change advertised artifact URLs on hostname change --- controllers/gitrepository_controller.go | 4 ++++ controllers/helmchart_controller.go | 14 ++++++++++---- controllers/helmrepository_controller.go | 9 ++++++--- controllers/storage.go | 12 ++++++++++++ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index d1d0815a..d62603bf 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -199,6 +199,10 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour // return early on unchanged revision artifact := r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), revision, fmt.Sprintf("%s.tar.gz", commit.Hash.String())) if repository.GetArtifact() != nil && repository.GetArtifact().Revision == revision { + if artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } return repository, nil } diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 76c42613..9ebe88d7 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -196,7 +196,12 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, } // return early on unchanged chart version + artifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version)) if repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version { + if artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } return chart, nil } @@ -260,8 +265,6 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err } - artifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version)) - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { @@ -359,12 +362,15 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, } // return early on unchanged chart version + artifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version, fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version)) if chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version { + if artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } return chart, nil } - artifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version, fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version)) - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 7e7b6238..a368ac40 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -217,7 +217,13 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou } // return early on unchanged generation + artifact := r.Storage.NewArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), i.Generated.Format(time.RFC3339Nano), + fmt.Sprintf("index-%s.yaml", url.PathEscape(i.Generated.Format(time.RFC3339Nano)))) if repository.GetArtifact() != nil && repository.GetArtifact().Revision == i.Generated.Format(time.RFC3339Nano) { + if artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } return repository, nil } @@ -227,9 +233,6 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - artifact := r.Storage.NewArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), i.Generated.Format(time.RFC3339Nano), - fmt.Sprintf("index-%s.yaml", url.PathEscape(i.Generated.Format(time.RFC3339Nano)))) - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { diff --git a/controllers/storage.go b/controllers/storage.go index 15648411..41883cc9 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -26,6 +26,7 @@ import ( "hash" "io" "io/ioutil" + "net/url" "os" "path/filepath" "strings" @@ -89,6 +90,17 @@ func (s Storage) SetArtifactURL(artifact *sourcev1.Artifact) { artifact.URL = fmt.Sprintf("http://%s/%s", s.Hostname, artifact.Path) } +// SetHostname sets the hostname of the given URL string to the current Storage.Hostname +// and returns the result. +func (s Storage) SetHostname(URL string) string { + u, err := url.Parse(URL) + if err != nil { + return "" + } + u.Host = s.Hostname + return u.String() +} + // MkdirAll calls os.MkdirAll for the given v1alpha1.Artifact base dir. func (s *Storage) MkdirAll(artifact sourcev1.Artifact) error { dir := filepath.Dir(s.LocalPath(artifact)) From f08febdffa72cbf8ef6f1799772ea9badabe7152 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 10 Sep 2020 14:21:16 +0200 Subject: [PATCH 5/5] Bump build-push-action version --- .github/workflows/release.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 87a6e721..50d3474c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,7 +40,7 @@ jobs: username: fluxcdbot password: ${{ secrets.DOCKER_FLUXCD_PASSWORD }} - name: Publish amd64 image - uses: docker/build-push-action@v2-build-push + uses: docker/build-push-action@v2 with: push: ${{ github.event_name != 'pull_request' }} builder: ${{ steps.buildx.outputs.name }} @@ -51,7 +51,7 @@ jobs: ghcr.io/fluxcd/source-controller:${{ steps.get_version.outputs.VERSION }} docker.io/fluxcd/source-controller:${{ steps.get_version.outputs.VERSION }} - name: Publish arm64 image - uses: docker/build-push-action@v2-build-push + uses: docker/build-push-action@v2 with: push: ${{ github.event_name != 'pull_request' }} builder: ${{ steps.buildx.outputs.name }}