Skip to content

Commit

Permalink
Update as per feedback. Use io.FS instead of raw directories.
Browse files Browse the repository at this point in the history 
Signed-off-by: Ville Aikas <vaikas@chainguard.dev>
  • Loading branch information
@vaikas
vaikas committed Sep 27, 2022
1 parent 3f7ec27 commit 8719f08
Show file tree
Hide file tree
Showing 3 changed files with 157 additions and 108 deletions.
32 changes: 16 additions & 16 deletions client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (f *fakeRemoteStore) get(name string, store map[string]*fakeFile) (io.ReadC
// because the set/delete methods are not part of the Interface, we need to
// switch on the underlying implementation.
// Also readMeta method is convenience for ease of testing.
func (s *ClientSuite) setMeta(path string, data []byte) error {
func (s *ClientSuite) setRemoteMeta(path string, data []byte) error {
switch impl := s.remote.(type) {
case *fakeRemoteStore:
impl.meta[path] = newFakeFile(data)
Expand All @@ -89,7 +89,7 @@ func (s *ClientSuite) setMeta(path string, data []byte) error {
}
}

func (s *ClientSuite) setTarget(path string, data []byte) error {
func (s *ClientSuite) setRemoteTarget(path string, data []byte) error {
switch impl := s.remote.(type) {
case *fakeRemoteStore:
impl.targets[path] = newFakeFile(data)
Expand All @@ -109,7 +109,7 @@ func (s *ClientSuite) deleteMeta(path string) error {
case *FileRemoteStore:
return impl.deleteMeta(path)
default:
return fmt.Errorf("non-supoprted RemoteStore, got %+v", impl)
return fmt.Errorf("non-supported RemoteStore, got %+v", impl)
}
}

Expand All @@ -121,7 +121,7 @@ func (s *ClientSuite) deleteTarget(path string) error {
case *FileRemoteStore:
return impl.deleteTarget(path)
default:
return fmt.Errorf("non-supoprted RemoteStore, got %+v", impl)
return fmt.Errorf("non-supported RemoteStore, got %+v", impl)
}
}

Expand Down Expand Up @@ -183,7 +183,7 @@ func (s *ClientSuite) SetUpTest(c *C) {

// create a remote store containing valid repo files
if s.useFileStore {
s.remote, s.tmpDir, err = newTestFileStore()
s.remote, s.tmpDir, err = newTestFileStoreFS()
if err != nil {
c.Fatalf("failed to create new FileStore: %v", err)
}
Expand All @@ -192,7 +192,7 @@ func (s *ClientSuite) SetUpTest(c *C) {
}
s.syncRemote(c)
for path, data := range targetFiles {
s.setTarget(path, data)
s.setRemoteTarget(path, data)
}

s.expiredTime = time.Now().Add(time.Hour)
Expand Down Expand Up @@ -242,7 +242,7 @@ func (s *ClientSuite) syncRemote(c *C) {
meta, err := s.store.GetMeta()
c.Assert(err, IsNil)
for name, data := range meta {
if err := s.setMeta(name, data); err != nil {
if err := s.setRemoteMeta(name, data); err != nil {
panic(fmt.Sprintf("setMetadata failed: %v", err))
}
}
Expand Down Expand Up @@ -855,7 +855,7 @@ func (s *ClientSuite) TestLocalExpired(c *C) {
}

func (s *ClientSuite) TestTimestampTooLarge(c *C) {
s.setMeta("timestamp.json", make([]byte, defaultTimestampDownloadLimit+1))
s.setRemoteMeta("timestamp.json", make([]byte, defaultTimestampDownloadLimit+1))
_, err := s.newClient(c).Update()
c.Assert(err, Equals, ErrMetaTooLarge{"timestamp.json", defaultTimestampDownloadLimit + 1, defaultTimestampDownloadLimit})
}
Expand Down Expand Up @@ -997,7 +997,7 @@ func (s *ClientSuite) TestUpdateMixAndMatchAttack(c *C) {
if err != nil {
c.Fatal("missing remote targets.json")
}
s.setMeta("targets.json", oldTargets)
s.setRemoteMeta("targets.json", oldTargets)

// check update returns ErrWrongSize for targets.json
_, err = client.Update()
Expand All @@ -1008,7 +1008,7 @@ func (s *ClientSuite) TestUpdateMixAndMatchAttack(c *C) {
c.Assert(s.repo.Snapshot(), IsNil)
c.Assert(s.repo.Timestamp(), IsNil)
s.syncRemote(c)
s.setMeta("targets.json", oldTargets)
s.setRemoteMeta("targets.json", oldTargets)

// check update returns ErrWrongHash
_, err = client.Update()
Expand All @@ -1034,7 +1034,7 @@ func (s *ClientSuite) TestUpdateReplayAttack(c *C) {
c.Assert(client.timestampVer > version, Equals, true)

// replace remote timestamp.json with the old one
s.setMeta("timestamp.json", oldTimestamp)
s.setRemoteMeta("timestamp.json", oldTimestamp)

// check update returns ErrLowVersion
_, err = client.Update()
Expand Down Expand Up @@ -1067,7 +1067,7 @@ func (s *ClientSuite) TestUpdateForkTimestamp(c *C) {
c.Assert(newVersion > version, Equals, true)

// generate a new, different timestamp with the *same version*
s.setMeta("timestamp.json", oldTimestamp)
s.setRemoteMeta("timestamp.json", oldTimestamp)
c.Assert(s.repo.Timestamp(), IsNil)
c.Assert(client.timestampVer, Equals, newVersion) // double-check: same version?
s.syncRemote(c)
Expand Down Expand Up @@ -1199,31 +1199,31 @@ func (s *ClientSuite) TestDownloadOK(c *C) {
func (s *ClientSuite) TestDownloadWrongSize(c *C) {
client := s.updatedClient(c)
// Update with a file that's incorrect size.
s.setTarget("foo.txt", []byte("wrong-size"))
s.setRemoteTarget("foo.txt", []byte("wrong-size"))
var dest testDestination
c.Assert(client.Download("foo.txt", &dest), DeepEquals, ErrWrongSize{"foo.txt", 10, 3})
c.Assert(dest.deleted, Equals, true)
}

func (s *ClientSuite) TestDownloadTargetTooLong(c *C) {
client := s.updatedClient(c)
s.setTarget("foo.txt", []byte("foo-ooo"))
s.setRemoteTarget("foo.txt", []byte("foo-ooo"))
var dest testDestination
c.Assert(client.Download("foo.txt", &dest), DeepEquals, ErrWrongSize{"foo.txt", 7, 3})
c.Assert(dest.deleted, Equals, true)
}

func (s *ClientSuite) TestDownloadTargetTooShort(c *C) {
client := s.updatedClient(c)
s.setTarget("foo.txt", []byte("fo"))
s.setRemoteTarget("foo.txt", []byte("fo"))
var dest testDestination
c.Assert(client.Download("foo.txt", &dest), DeepEquals, ErrWrongSize{"foo.txt", 2, 3})
c.Assert(dest.deleted, Equals, true)
}

func (s *ClientSuite) TestDownloadTargetCorruptData(c *C) {
client := s.updatedClient(c)
s.setTarget("foo.txt", []byte("ooo"))
s.setRemoteTarget("foo.txt", []byte("ooo"))
var dest testDestination
assertWrongHash(c, client.Download("foo.txt", &dest))
c.Assert(dest.deleted, Equals, true)
Expand Down
90 changes: 48 additions & 42 deletions client/file_store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,62 +6,76 @@ import (
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
)

type FileStoreOptions struct {
MetadataPath string
TargetsPath string
}

// FileRemoteStore provides a RemoteStore interface compatible
// implementation that can be used where the RemoteStore is on local disk. This
// is useful for example in air-gapped environments where there's no
// implementation that can be used where the RemoteStore is backed by a
// fs.FS. This is useful for example in air-gapped environments where there's no
// possibility to make outbound network connections.
// If targetsDir is "", metadataDir/targets is used.
func NewFileRemoteStore(metadataDir, targetsDir string) (*FileRemoteStore, error) {
if metadataDir == "" {
return nil, errors.New("metadataDir can't be empty")
// By having this be a fs.FS instead of directories allows the repository to
// be backed by something that's not persisted to disk.
func NewFileRemoteStore(fsys fs.FS, targetDir string) (*FileRemoteStore, error) {
if fsys == nil {
return nil, errors.New("nil fs.FS")
}
t := targetDir
if t == "" {
t = "targets"
}
f := &FileRemoteStore{
metadataDir: metadataDir,
// Make sure directory exists
d, err := fsys.Open(t)
if err != nil {
return nil, fmt.Errorf("failed to open targets directory %s: %w", t, err)
}
fi, err := d.Stat()
if err != nil {
return nil, fmt.Errorf("failed to stat targets directory %s: %w", t, err)
}
if targetsDir != "" {
f.targetsDir = targetsDir
} else {
f.targetsDir = filepath.Join(f.metadataDir, "targets")
if !fi.IsDir() {
return nil, fmt.Errorf("targets directory not a directory %s", t)
}

// Make sure metadataDir and targetsDir exist and are directories before
// proceeding.
for _, dir := range []string{f.metadataDir, f.targetsDir} {
s, err := os.Stat(dir)
if err != nil {
return nil, fmt.Errorf("failed to stat %s: %s", dir, err)
}
if !s.IsDir() {
return nil, fmt.Errorf("not a directory %s", dir)
}
fsysT, err := fs.Sub(fsys, t)
if err != nil {
return nil, fmt.Errorf("failed to open targets directory %s: %w", t, err)
}
return f, nil
return &FileRemoteStore{fsys: fsys, targetDir: fsysT}, nil
}

type FileRemoteStore struct {
metadataDir string
targetsDir string
// Meta directory fs
fsys fs.FS
// Target directory fs.
targetDir fs.FS
// In order to be able to make write operations (create, delete) we can't
// use fs.FS for it (it's read only), so we have to know the underlying
// directory that add/delete test methods can use. This is only necessary
// for testing purposes.
testDir string
}

func (f *FileRemoteStore) GetMeta(name string) (io.ReadCloser, int64, error) {
rc, b, err := f.get(filepath.Join(f.metadataDir, name))
rc, b, err := f.get(f.fsys, name)
return handleErrors(name, rc, b, err)
}

func (f *FileRemoteStore) GetTarget(name string) (io.ReadCloser, int64, error) {
rc, b, err := f.get(filepath.Join(f.targetsDir, name))
rc, b, err := f.get(f.targetDir, name)
return handleErrors(name, rc, b, err)
}

func (f *FileRemoteStore) get(fsys fs.FS, s string) (io.ReadCloser, int64, error) {
if !fs.ValidPath(s) {
return nil, 0, fmt.Errorf("invalid path %s", s)
}

b, err := fs.ReadFile(fsys, s)
if err != nil {
return nil, -1, err
}
return io.NopCloser(bytes.NewReader(b)), int64(len(b)), nil
}

// handleErrors converts NotFound errors to something that TUF knows how to
// handle properly. For example, when looking for n+1 root files, this is a
// signal that it will stop looking.
Expand All @@ -74,11 +88,3 @@ func handleErrors(name string, rc io.ReadCloser, b int64, err error) (io.ReadClo
}
return rc, b, err
}

func (f *FileRemoteStore) get(s string) (io.ReadCloser, int64, error) {
b, err := os.ReadFile(s)
if err != nil {
return nil, -1, err
}
return io.NopCloser(bytes.NewReader(b)), int64(len(b)), nil
}
Loading

0 comments on commit 8719f08

Please sign in to comment.