diff --git a/internal/bsr/bsr.go b/internal/bsr/bsr.go index 2e2417dc3..028b756ae 100644 --- a/internal/bsr/bsr.go +++ b/internal/bsr/bsr.go @@ -363,7 +363,7 @@ func (s *Session) NewConnection(ctx context.Context, meta *ConnectionMeta) (*Con } name := fmt.Sprintf(connectionFile, meta.Id) - sc, err := s.container.container.SubContainer(ctx, name) + sc, err := s.container.container.SubContainer(ctx, name, storage.WithCreateFile(), storage.WithFileAccessMode(storage.WriteOnly)) if err != nil { return nil, err } @@ -465,7 +465,7 @@ func (c *Connection) NewChannel(ctx context.Context, meta *ChannelMeta) (*Channe } name := fmt.Sprintf(channelFile, meta.Id) - sc, err := c.container.container.SubContainer(ctx, name) + sc, err := c.container.container.SubContainer(ctx, name, storage.WithCreateFile(), storage.WithFileAccessMode(storage.WriteOnly)) if err != nil { return nil, err } diff --git a/internal/bsr/internal/checksum/checksum_test.go b/internal/bsr/internal/checksum/checksum_test.go index 017a49764..aec76c183 100644 --- a/internal/bsr/internal/checksum/checksum_test.go +++ b/internal/bsr/internal/checksum/checksum_test.go @@ -53,7 +53,7 @@ func TestFile(t *testing.T) { }{ { "success", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), bytes.NewBuffer([]byte{}), testString, testSum + " test\n", @@ -73,7 +73,7 @@ func TestFile(t *testing.T) { }, { "nil-checksum-writer", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), nil, testString, testSum + " test\n", @@ -83,7 +83,7 @@ func TestFile(t *testing.T) { }, { "close-error", - fstest.NewMemFile("test", 0o664, fstest.WithCloseFunc(func() error { return errors.New("close error") })), + fstest.NewWritableMemFile("test", fstest.WithCloseFunc(func() error { return errors.New("close error") })), bytes.NewBuffer([]byte{}), testString, testSum + " test\n", @@ -97,7 +97,7 @@ func TestFile(t *testing.T) { }, { "stat-error", - fstest.NewMemFile("test", 0o664, fstest.WithStatFunc(func() (fs.FileInfo, error) { return nil, errors.New("stat error") })), + fstest.NewWritableMemFile("test", fstest.WithStatFunc(func() (fs.FileInfo, error) { return nil, errors.New("stat error") })), bytes.NewBuffer([]byte{}), testString, testSum + " test\n", @@ -111,7 +111,7 @@ func TestFile(t *testing.T) { }, { "write-error", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), &badWriter{}, testString, testSum + " test\n", diff --git a/internal/bsr/internal/fstest/fs.go b/internal/bsr/internal/fstest/fs.go index b209cead8..88dcd8409 100644 --- a/internal/bsr/internal/fstest/fs.go +++ b/internal/bsr/internal/fstest/fs.go @@ -16,6 +16,11 @@ import ( "github.com/hashicorp/boundary/internal/storage" ) +const ( + defaultContainerPerm = 0o775 + defaultFilePerm = 0o644 +) + // MemFS is a storage.FS that only resides in memory. type MemFS struct { Containers map[string]*MemContainer @@ -24,7 +29,7 @@ type MemFS struct { readOnly bool } -// NewMemFS creates a MemFS. It supports WithNewFunc. +// NewMemFS creates a MemFS. It supports WithNewFunc, WithReadOnly. func NewMemFS(options ...Option) *MemFS { opts := getOpts(options...) @@ -40,17 +45,24 @@ func (m *MemFS) New(ctx context.Context, n string) (storage.Container, error) { if m.newFunc != nil { return m.newFunc(ctx, n) } + + if m.readOnly { + return nil, fmt.Errorf("cannot create new container from read-only fs") + } + if m.Containers == nil { m.Containers = make(map[string]*MemContainer) } if _, exists := m.Containers[n]; exists { return nil, fmt.Errorf("container %s already exists", n) } + c := &MemContainer{ - Name: n, - Sub: make(map[string]*MemContainer), - Files: make(map[string]*MemFile), - readOnly: m.readOnly, + Name: n, + Sub: make(map[string]*MemContainer), + Files: make(map[string]*MemFile), + mode: defaultContainerPerm, + accessMode: storage.ReadWrite, } m.Containers[n] = c return c, nil @@ -66,6 +78,7 @@ func (m *MemFS) Open(_ context.Context, n string) (storage.Container, error) { if !ok { return nil, fmt.Errorf("container %s not found", n) } + c.Closed = false return c, nil } @@ -76,8 +89,10 @@ type MemContainer struct { Sub map[string]*MemContainer Files map[string]*MemFile - Closed bool - readOnly bool + Closed bool + + accessMode storage.AccessMode + mode sfs.FileMode sync.RWMutex } @@ -91,21 +106,11 @@ func (m *MemContainer) Close() error { } // Create makes a new storage.File in the container. -func (m *MemContainer) Create(_ context.Context, n string) (storage.File, error) { +func (m *MemContainer) Create(ctx context.Context, n string) (storage.File, error) { if m.Closed { return nil, fmt.Errorf("create on closed container") } - if _, exists := m.Files[n]; exists { - return nil, fmt.Errorf("file %s already exists", n) - } - f := &MemFile{ - name: n, - Buf: bytes.NewBuffer([]byte{}), - mode: sfs.ModeAppend, - ReadOnly: m.readOnly, - } - m.Files[n] = f - return f, nil + return m.OpenFile(ctx, n, storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)) } // OpenFile creates a storage.File in the container using the provided options @@ -114,47 +119,74 @@ func (m *MemContainer) OpenFile(_ context.Context, n string, option ...storage.O m.Lock() defer m.Unlock() - opts := storage.GetOpts(option...) if m.Closed { return nil, fmt.Errorf("create on closed container") } - if _, exists := m.Files[n]; exists { - return nil, fmt.Errorf("file %s already exists", n) + opts := storage.GetOpts(option...) + + if m.accessMode == storage.ReadOnly && opts.WithFileAccessMode != storage.ReadOnly { + return nil, fmt.Errorf("cannot create writeable file in readonly container") } - var mode sfs.FileMode - - switch opts.WithCloseSyncMode { - case storage.NoSync: - mode = sfs.ModeAppend | sfs.ModeTemporary - default: - mode = sfs.ModeAppend + if opts.WithCreateFile && opts.WithFileAccessMode == storage.ReadOnly { + return nil, fmt.Errorf("cannot create file in read-only mode") } - f := &MemFile{ - name: n, - Buf: bytes.NewBuffer([]byte{}), - mode: mode, + var f *MemFile + + if opts.WithCreateFile { + // create or truncate just like os.Create + f = &MemFile{ + Buf: bytes.NewBuffer([]byte{}), + } + } else { + var ok bool + f, ok = m.Files[n] + if !ok { + return nil, fmt.Errorf("file %s does not exist", n) + } } + + f.name = n + f.syncMode = opts.WithCloseSyncMode + f.accessMode = opts.WithFileAccessMode + f.mode = defaultFilePerm + f.Closed = false m.Files[n] = f return f, nil } // SubContainer creates a new storage.Container in the container. -func (m *MemContainer) SubContainer(_ context.Context, n string, _ ...storage.Option) (storage.Container, error) { +func (m *MemContainer) SubContainer(_ context.Context, n string, option ...storage.Option) (storage.Container, error) { m.Lock() defer m.Unlock() if m.Closed { return nil, fmt.Errorf("subcontainer on closed container") } - if _, exists := m.Sub[n]; exists { - return nil, fmt.Errorf("container %s already exists", n) + opts := storage.GetOpts(option...) + + c, exists := m.Sub[n] + + if opts.WithCreateFile && opts.WithFileAccessMode == storage.ReadOnly { + return nil, fmt.Errorf("cannot create container in read-only mode") } - c := &MemContainer{ - Name: n, - Sub: make(map[string]*MemContainer), - Files: make(map[string]*MemFile), + + if opts.WithCreateFile { + if exists { + return nil, fmt.Errorf("container %s already exists", n) + } + c = &MemContainer{ + Name: n, + Sub: make(map[string]*MemContainer), + Files: make(map[string]*MemFile), + } + } else { + if !exists { + return nil, fmt.Errorf("container %s does not exist", n) + } } + c.accessMode = opts.WithFileAccessMode + c.Closed = false m.Sub[n] = c return c, nil } @@ -185,9 +217,11 @@ type MemFile struct { closeFunc CloseFunc Closed bool - ReadOnly bool OutOfSpace bool + syncMode storage.SyncMode + accessMode storage.AccessMode + sync.RWMutex } @@ -196,12 +230,32 @@ type MemFile struct { func NewMemFile(n string, mode sfs.FileMode, options ...Option) *MemFile { opts := getOpts(options...) + storageOpts := storage.GetOpts(opts.withStorageOptions...) + return &MemFile{ - name: n, - Buf: bytes.NewBuffer([]byte{}), - mode: mode, - statFunc: opts.withStatFunc, - closeFunc: opts.withCloseFunc, + name: n, + Buf: bytes.NewBuffer([]byte{}), + mode: mode, + accessMode: storageOpts.WithFileAccessMode, + syncMode: storageOpts.WithCloseSyncMode, + statFunc: opts.withStatFunc, + closeFunc: opts.withCloseFunc, + } +} + +// NewWritableMemFile is a convinence function to create a MemFile with the +// WriteOnly storage access mode. +func NewWritableMemFile(n string, options ...Option) *MemFile { + opts := getOpts(options...) + + return &MemFile{ + name: n, + Buf: bytes.NewBuffer([]byte{}), + mode: 0o664, + accessMode: storage.WriteOnly, + syncMode: storage.Asynchronous, + statFunc: opts.withStatFunc, + closeFunc: opts.withCloseFunc, } } @@ -229,6 +283,10 @@ func (m *MemFile) Read(p []byte) (int, error) { m.RLock() defer m.RUnlock() + if m.accessMode == storage.WriteOnly { + return 0, fmt.Errorf("read on write-only file") + } + if m.Closed { return 0, fmt.Errorf("read on closed file") } @@ -264,7 +322,7 @@ func (m *MemFile) Write(p []byte) (n int, err error) { return 0, fmt.Errorf("write on closed file") } - if m.ReadOnly { + if m.accessMode == storage.ReadOnly { return 0, fmt.Errorf("write on read-only file") } diff --git a/internal/bsr/internal/fstest/fs_test.go b/internal/bsr/internal/fstest/fs_test.go new file mode 100644 index 000000000..f980b5651 --- /dev/null +++ b/internal/bsr/internal/fstest/fs_test.go @@ -0,0 +1,345 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package fstest_test + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/hashicorp/boundary/internal/bsr/internal/fstest" + "github.com/hashicorp/boundary/internal/storage" + "github.com/stretchr/testify/require" +) + +func TestMemFSNew(t *testing.T) { + ctx := context.Background() + + cases := []struct { + name string + m *fstest.MemFS + n string + wantErr error + }{ + { + "default", + fstest.NewMemFS(), + "test", + nil, + }, + { + "read-only", + fstest.NewMemFS(fstest.WithReadOnly(true)), + "test", + errors.New("cannot create new container from read-only fs"), + }, + { + "custom-new-func", + fstest.NewMemFS(fstest.WithNewFunc(func(_ context.Context, _ string) (storage.Container, error) { + return nil, fmt.Errorf("custom error from new") + })), + "test", + errors.New("custom error from new"), + }, + { + "already-exists", + func() *fstest.MemFS { + f := fstest.NewMemFS() + _, err := f.New(ctx, "test") + require.NoError(t, err) + return f + }(), + "test", + errors.New("container test already exists"), + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := tc.m.New(ctx, tc.n) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + return + } + require.NoError(t, err) + require.NotNil(t, c) + }) + } +} + +func TestMemFSOpen(t *testing.T) { + ctx := context.Background() + + cases := []struct { + name string + m *fstest.MemFS + n string + wantErr error + }{ + { + "exists", + func() *fstest.MemFS { + f := fstest.NewMemFS() + _, err := f.New(ctx, "test") + require.NoError(t, err) + return f + }(), + "test", + nil, + }, + { + "does-not-exist", + fstest.NewMemFS(), + "test", + errors.New("container test not found"), + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := tc.m.Open(ctx, tc.n) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + return + } + require.NoError(t, err) + require.NotNil(t, c) + }) + } +} + +func TestMemContainerOpenFile(t *testing.T) { + ctx := context.Background() + + cases := []struct { + name string + c *fstest.MemContainer + n string + opts []storage.Option + wantErr error + }{ + { + "create", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + return cc + }(), + "test", + []storage.Option{storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)}, + nil, + }, + { + "create-already-exists", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + _, err = cc.OpenFile(ctx, "test", storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)) + require.NoError(t, err) + return cc + }(), + "test", + []storage.Option{storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)}, + nil, + }, + { + "create-read-only", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + return cc + }(), + "test", + []storage.Option{storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadOnly)}, + errors.New("cannot create file in read-only mode"), + }, + { + "read-only-does-not-exist", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + return cc + }(), + "test", + []storage.Option{storage.WithFileAccessMode(storage.ReadOnly)}, + errors.New("file test does not exist"), + }, + { + "read-only-exist", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + _, err = cc.OpenFile(ctx, "test", storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)) + require.NoError(t, err) + return cc + }(), + "test", + []storage.Option{storage.WithFileAccessMode(storage.ReadOnly)}, + nil, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := tc.c.OpenFile(ctx, tc.n, tc.opts...) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + return + } + require.NoError(t, err) + require.NotNil(t, c) + }) + } +} + +func TestMemContainerCreate(t *testing.T) { + ctx := context.Background() + + cases := []struct { + name string + c *fstest.MemContainer + n string + wantErr error + }{ + { + "create", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + return cc + }(), + "test", + nil, + }, + { + "create-already-exists", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + _, err = cc.OpenFile(ctx, "test", storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)) + require.NoError(t, err) + return cc + }(), + "test", + nil, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := tc.c.Create(ctx, tc.n) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + return + } + require.NoError(t, err) + require.NotNil(t, c) + }) + } +} + +func TestMemContainerSubContainer(t *testing.T) { + ctx := context.Background() + + cases := []struct { + name string + c *fstest.MemContainer + n string + opts []storage.Option + wantErr error + }{ + { + "create", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + return cc + }(), + "test", + []storage.Option{storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)}, + nil, + }, + { + "create-already-exists", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + _, err = cc.SubContainer(ctx, "test", storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)) + require.NoError(t, err) + return cc + }(), + "test", + []storage.Option{storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)}, + errors.New("container test already exists"), + }, + { + "create-read-only", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + return cc + }(), + "test", + []storage.Option{storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadOnly)}, + errors.New("cannot create container in read-only mode"), + }, + { + "read-only-does-not-exist", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + return cc + }(), + "test", + []storage.Option{storage.WithFileAccessMode(storage.ReadOnly)}, + errors.New("container test does not exist"), + }, + { + "read-only-exist", + func() *fstest.MemContainer { + f := fstest.NewMemFS() + c, err := f.New(ctx, "test") + require.NoError(t, err) + cc := c.(*fstest.MemContainer) + _, err = cc.SubContainer(ctx, "test", storage.WithCreateFile(), storage.WithFileAccessMode(storage.ReadWrite)) + require.NoError(t, err) + return cc + }(), + "test", + []storage.Option{storage.WithFileAccessMode(storage.ReadOnly)}, + nil, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c, err := tc.c.SubContainer(ctx, tc.n, tc.opts...) + if tc.wantErr != nil { + require.EqualError(t, err, tc.wantErr.Error()) + return + } + require.NoError(t, err) + require.NotNil(t, c) + }) + } +} diff --git a/internal/bsr/internal/fstest/options.go b/internal/bsr/internal/fstest/options.go index f74f6cf1c..a9b7def30 100644 --- a/internal/bsr/internal/fstest/options.go +++ b/internal/bsr/internal/fstest/options.go @@ -36,18 +36,20 @@ type StatFunc func() (fs.FileInfo, error) // options = how options are represented type options struct { - withNewFunc NewFunc - withCloseFunc CloseFunc - withStatFunc StatFunc - withReadOnly bool + withNewFunc NewFunc + withCloseFunc CloseFunc + withStatFunc StatFunc + withReadOnly bool + withStorageOptions []storage.Option } func getDefaultOptions() options { return options{ - withNewFunc: nil, - withCloseFunc: nil, - withStatFunc: nil, - withReadOnly: false, + withNewFunc: nil, + withCloseFunc: nil, + withStatFunc: nil, + withReadOnly: false, + withStorageOptions: nil, } } @@ -78,3 +80,10 @@ func WithReadOnly(b bool) Option { o.withReadOnly = b } } + +// WithStorageOptions is used to pass through storage options. +func WithStorageOptions(opts []storage.Option) Option { + return func(o *options) { + o.withStorageOptions = opts + } +} diff --git a/internal/bsr/internal/journal/journal_test.go b/internal/bsr/internal/journal/journal_test.go index 8146ce605..7a12c5975 100644 --- a/internal/bsr/internal/journal/journal_test.go +++ b/internal/bsr/internal/journal/journal_test.go @@ -50,7 +50,7 @@ func TestFile(t *testing.T) { }{ { "success", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), bytes.NewBuffer([]byte{}), func(w bufWriter) *journal.Journal { ww, err := journal.New(ctx, w); require.NoError(t, err); return ww }, testString, @@ -70,7 +70,7 @@ func TestFile(t *testing.T) { }, { "nil-journal-writer", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), nil, func(w bufWriter) *journal.Journal { return nil }, testString, @@ -80,7 +80,7 @@ func TestFile(t *testing.T) { }, { "close-error", - fstest.NewMemFile("test", 0o664, fstest.WithCloseFunc(func() error { return errors.New("close error") })), + fstest.NewWritableMemFile("test", fstest.WithCloseFunc(func() error { return errors.New("close error") })), bytes.NewBuffer([]byte{}), func(w bufWriter) *journal.Journal { ww, err := journal.New(ctx, w); require.NoError(t, err); return ww }, testString, @@ -90,7 +90,7 @@ func TestFile(t *testing.T) { }, { "stat-error", - fstest.NewMemFile("test", 0o664, fstest.WithStatFunc(func() (fs.FileInfo, error) { return nil, errors.New("stat error") })), + fstest.NewWritableMemFile("test", fstest.WithStatFunc(func() (fs.FileInfo, error) { return nil, errors.New("stat error") })), bytes.NewBuffer([]byte{}), func(w bufWriter) *journal.Journal { ww, err := journal.New(ctx, w); require.NoError(t, err); return ww }, testString, @@ -100,7 +100,7 @@ func TestFile(t *testing.T) { }, { "write-error", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), &badWriter{}, func(w bufWriter) *journal.Journal { ww, err := journal.New(ctx, w); require.NoError(t, err); return ww }, testString, diff --git a/internal/bsr/internal/sign/sign_test.go b/internal/bsr/internal/sign/sign_test.go index 134bcc5d6..73ea42e4f 100644 --- a/internal/bsr/internal/sign/sign_test.go +++ b/internal/bsr/internal/sign/sign_test.go @@ -61,7 +61,7 @@ func TestFile(t *testing.T) { }{ { "success", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), bytes.NewBuffer([]byte{}), keys, testString, @@ -83,7 +83,7 @@ func TestFile(t *testing.T) { }, { "nil-sign-writer", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), nil, keys, testString, @@ -94,7 +94,7 @@ func TestFile(t *testing.T) { }, { "nil-keys", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), bytes.NewBuffer([]byte{}), nil, testString, @@ -105,7 +105,7 @@ func TestFile(t *testing.T) { }, { "close-error", - fstest.NewMemFile("test", 0o664, fstest.WithCloseFunc(func() error { return errors.New("close error") })), + fstest.NewWritableMemFile("test", fstest.WithCloseFunc(func() error { return errors.New("close error") })), bytes.NewBuffer([]byte{}), keys, testString, @@ -120,7 +120,7 @@ func TestFile(t *testing.T) { }, { "write-error", - fstest.NewMemFile("test", 0o664), + fstest.NewWritableMemFile("test"), &badWriter{}, keys, testString,