Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: filesystemStore fails to prepend target file hashes on Windows #274

Merged
merged 6 commits into from
Jul 26, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# go enforces lf line endings
*.go eol=lf

# testdata should not be mangled by git
*.json binary
joshuagl marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
run:
strategy:
matrix:
os: [ubuntu-latest, macos-latest]
os: [ubuntu-latest, macos-latest, windows-latest]
go-version: ${{ fromJSON(needs.get-go-versions.outputs.matrix) }}
runs-on: ${{ matrix.os }}
needs: get-go-versions
Expand Down Expand Up @@ -54,7 +54,7 @@ jobs:
strategy:
matrix:
go-version: ${{ fromJSON(needs.get-go-versions.outputs.matrix) }}
os: [ubuntu-latest, macos-latest]
os: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.os }}
needs: get-go-versions
steps:
Expand Down
45 changes: 39 additions & 6 deletions client/testdata/go-tuf/generator/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package generator
import (
"encoding/json"
"fmt"
"io"
"io/fs"
"io/ioutil"
"log"
"os"
"os/exec"
"path/filepath"
"time"

Expand All @@ -28,9 +29,41 @@ func assertNoError(err error) {
}
}

func copyRepo(src string, dst string) {
cmd := exec.Command("cp", "-r", src, dst)
znewman01 marked this conversation as resolved.
Show resolved Hide resolved
assertNoError(cmd.Run())
// copyRepo recursively copies all regular files and directories under src
// to dst. In the case where any destination file/directory exists
// (including dst itself), an error is returned.
func copyRepo(src string, dst string) error {
copyToDest := func(path string, info fs.FileInfo, err error) error {
if err != nil {
return err
}
rel, err := filepath.Rel(src, path)
if err != nil {
return err
}
target := filepath.Join(dst, rel)
mode := info.Mode()
if mode.IsDir() {
return os.Mkdir(target, mode.Perm())
} else if mode.IsRegular() {
sfile, err := os.Open(path)
if err != nil {
return err
}
defer sfile.Close()
dfile, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode.Perm())
if err != nil {
return err
}
defer dfile.Close()
if _, err := io.Copy(dfile, sfile); err != nil {
return err
}
return nil
}
return fmt.Errorf("unknown mode %v", mode)
}
return filepath.Walk(src, copyToDest)
}

func newRepo(dir string) *tuf.Repo {
Expand Down Expand Up @@ -103,7 +136,7 @@ func generateRepos(dir string, roleKeys map[string][][]*data.PrivateKey, consist
// Setup the repo.
stepName := fmt.Sprintf("%d", i)
d := filepath.Join(dir, stepName)
copyRepo(oldDir, d)
assertNoError(copyRepo(oldDir, d))
repo := newRepo(d)
addKeys(repo, keys)

Expand All @@ -125,7 +158,7 @@ func generateRepos(dir string, roleKeys map[string][][]*data.PrivateKey, consist
// Add another target file to make sure the workflow worked.
stepName := fmt.Sprintf("%d", i)
d := filepath.Join(dir, stepName)
copyRepo(oldDir, d)
assertNoError(copyRepo(oldDir, d))
repo := newRepo(d)
addKeys(repo, keys)
addTargets(repo, d, map[string][]byte{stepName: []byte(stepName)})
Expand Down
4 changes: 2 additions & 2 deletions data/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
"path/filepath"
"path"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -237,7 +237,7 @@ func (d *DelegatedRole) MatchesPath(file string) (bool, error) {
}

for _, pattern := range d.Paths {
if matched, _ := filepath.Match(pattern, file); matched {
if matched, _ := path.Match(pattern, file); matched {
return true, nil
}
}
Expand Down
56 changes: 29 additions & 27 deletions local_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,44 +311,44 @@ func (f *fileSystemStore) createDirs() error {

func (f *fileSystemStore) WalkStagedTargets(paths []string, targetsFn TargetsWalkFunc) error {
if len(paths) == 0 {
walkFunc := func(path string, info os.FileInfo, err error) error {
walkFunc := func(fpath string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() || !info.Mode().IsRegular() {
return nil
}
rel, err := filepath.Rel(filepath.Join(f.stagedDir(), "targets"), path)
rel, err := filepath.Rel(filepath.Join(f.stagedDir(), "targets"), fpath)
if err != nil {
return err
}
file, err := os.Open(path)
file, err := os.Open(fpath)
if err != nil {
return err
}
defer file.Close()
return targetsFn(rel, file)
return targetsFn(filepath.ToSlash(rel), file)
}
return filepath.Walk(filepath.Join(f.stagedDir(), "targets"), walkFunc)
}

// check all the files exist before processing any files
for _, path := range paths {
realPath := filepath.Join(f.stagedDir(), "targets", path)
if _, err := os.Stat(realPath); err != nil {
realFilepath := filepath.Join(f.stagedDir(), "targets", path)
if _, err := os.Stat(realFilepath); err != nil {
if os.IsNotExist(err) {
return ErrFileNotFound{realPath}
return ErrFileNotFound{realFilepath}
}
return err
}
}

for _, path := range paths {
realPath := filepath.Join(f.stagedDir(), "targets", path)
file, err := os.Open(realPath)
realFilepath := filepath.Join(f.stagedDir(), "targets", path)
file, err := os.Open(realFilepath)
if err != nil {
if os.IsNotExist(err) {
return ErrFileNotFound{realPath}
return ErrFileNotFound{realFilepath}
}
return err
}
Expand All @@ -373,23 +373,24 @@ func (f *fileSystemStore) Commit(consistentSnapshot bool, versions map[string]in
isTarget := func(path string) bool {
return strings.HasPrefix(path, "targets/")
}
copyToRepo := func(path string, info os.FileInfo, err error) error {
copyToRepo := func(fpath string, info os.FileInfo, err error) error {
if err != nil {
return err
}
if info.IsDir() || !info.Mode().IsRegular() {
return nil
}
rel, err := filepath.Rel(f.stagedDir(), path)
rel, err := filepath.Rel(f.stagedDir(), fpath)
if err != nil {
return err
}
relpath := filepath.ToSlash(rel)

var paths []string
if isTarget(rel) {
paths = computeTargetPaths(consistentSnapshot, rel, hashes)
if isTarget(relpath) {
paths = computeTargetPaths(consistentSnapshot, relpath, hashes)
} else {
paths = computeMetadataPaths(consistentSnapshot, rel, versions)
paths = computeMetadataPaths(consistentSnapshot, relpath, versions)
}
var files []io.Writer
for _, path := range paths {
Expand All @@ -400,7 +401,7 @@ func (f *fileSystemStore) Commit(consistentSnapshot bool, versions map[string]in
defer file.Close()
files = append(files, file)
}
staged, err := os.Open(path)
staged, err := os.Open(fpath)
if err != nil {
return err
}
Expand All @@ -411,43 +412,44 @@ func (f *fileSystemStore) Commit(consistentSnapshot bool, versions map[string]in
return nil
}
// Checks if target file should be deleted
needsRemoval := func(path string) bool {
needsRemoval := func(fpath string) bool {
if consistentSnapshot {
// strip out the hash
name := strings.SplitN(filepath.Base(path), ".", 2)
name := strings.SplitN(filepath.Base(fpath), ".", 2)
if len(name) != 2 || name[1] == "" {
return false
}
path = filepath.Join(filepath.Dir(path), name[1])
fpath = filepath.Join(filepath.Dir(fpath), name[1])
}
_, ok := hashes[path]
_, ok := hashes[filepath.ToSlash(fpath)]
return !ok
}
// Checks if folder is empty
folderNeedsRemoval := func(path string) bool {
f, err := os.Open(path)
folderNeedsRemoval := func(fpath string) bool {
f, err := os.Open(fpath)
if err != nil {
return false
}
defer f.Close()
_, err = f.Readdirnames(1)
return err == io.EOF
}
removeFile := func(path string, info os.FileInfo, err error) error {
removeFile := func(fpath string, info os.FileInfo, err error) error {
if err != nil {
return err
}
rel, err := filepath.Rel(f.repoDir(), path)
rel, err := filepath.Rel(f.repoDir(), fpath)
if err != nil {
return err
}
if !info.IsDir() && isTarget(rel) && needsRemoval(rel) {
relpath := filepath.ToSlash(rel)
if !info.IsDir() && isTarget(relpath) && needsRemoval(rel) {
// Delete the target file
if err := os.Remove(path); err != nil {
if err := os.Remove(fpath); err != nil {
return err
}
// Delete the target folder too if it's empty
targetFolder := filepath.Dir(path)
targetFolder := filepath.Dir(fpath)
if folderNeedsRemoval(targetFolder) {
if err := os.Remove(targetFolder); err != nil {
return err
Expand Down
31 changes: 16 additions & 15 deletions repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"reflect"
"sort"
Expand Down Expand Up @@ -1047,20 +1048,20 @@ func (rs *RepoSuite) TestConsistentSnapshot(c *C) {
c.Assert(err, IsNil)

// root.json, targets.json and snapshot.json should exist at both versioned and unversioned paths
for _, path := range []string{"root.json", "targets.json", "snapshot.json"} {
repoPath := filepath.Join("repository", path)
if path != "root.json" {
c.Assert(len(hashes[path]) > 0, Equals, true)
for _, meta := range []string{"root.json", "targets.json", "snapshot.json"} {
repoPath := path.Join("repository", meta)
if meta != "root.json" {
c.Assert(len(hashes[meta]) > 0, Equals, true)
}
tmp.assertHashedFilesNotExist(repoPath, hashes[path])
tmp.assertVersionedFileExist(repoPath, versions[path])
tmp.assertHashedFilesNotExist(repoPath, hashes[meta])
tmp.assertVersionedFileExist(repoPath, versions[meta])
tmp.assertExists(repoPath)
}

// target files should exist at hashed but not unhashed paths
for _, path := range []string{"targets/foo.txt", "targets/dir/bar.txt"} {
repoPath := filepath.Join("repository", path)
tmp.assertHashedFilesExist(repoPath, hashes[path])
for _, target := range []string{"targets/foo.txt", "targets/dir/bar.txt"} {
repoPath := path.Join("repository", target)
tmp.assertHashedFilesExist(repoPath, hashes[target])
tmp.assertNotExist(repoPath)
}

Expand All @@ -1087,13 +1088,13 @@ func (rs *RepoSuite) TestConsistentSnapshot(c *C) {
c.Assert(err, IsNil)

// root.json, targets.json and snapshot.json should exist at both versioned and unversioned paths
for _, path := range []string{"root.json", "targets.json", "snapshot.json"} {
repoPath := filepath.Join("repository", path)
if path != "root.json" {
c.Assert(len(hashes[path]) > 0, Equals, true)
for _, meta := range []string{"root.json", "targets.json", "snapshot.json"} {
repoPath := path.Join("repository", meta)
if meta != "root.json" {
c.Assert(len(hashes[meta]) > 0, Equals, true)
}
tmp.assertHashedFilesNotExist(repoPath, hashes[path])
tmp.assertVersionedFileExist(repoPath, versions[path])
tmp.assertHashedFilesNotExist(repoPath, hashes[meta])
tmp.assertVersionedFileExist(repoPath, versions[meta])
tmp.assertExists(repoPath)
}

Expand Down