Skip to content

Commit

Permalink
store: serialise access with a file lock
Browse files Browse the repository at this point in the history
Previously the code attempts to use ioutils.AtomicWriteFile to update
meta.json but this fails on Windows if another process has the meta.json
open for reading (unlike on Unix OSes).

Example error:
```
rename C:\Users\docker\.docker\contexts\meta\<id>\.tmp-meta.json2945721760 C:\Users\docker\.docker\contexts\meta\<id>\meta.json: Access is denied.
```

So if one `docker context inspect` is running, then another `docker context`
command can fail transiently in `CreateOrUpdate`.

Avoid this problem by serialising accesses to the context filesystem db
using a lock file via github.com/gofrs/flock which uses LockFileEx
on Windows and syscall.Flock on Unix. The lock is associated with a
file descriptor / handle and released by calling Close() or when the
process exits.

Note this means that functions which could previously fail for one reason
can now fail for two (the original + a lock/unlock failure). Use
go-multierror to return all the errors. When we upgrade to go 1.20 we can
use errors.Join.

Signed-off-by: David Scott <dave.scott@docker.com>
  • Loading branch information
djs55 committed Dec 18, 2023
1 parent 8a561bb commit c37bb67
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 10 deletions.
1 change: 1 addition & 0 deletions cli/context/store/metadatastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
const (
metadataDir = "meta"
metaFile = "meta.json"
lockFile = "lock"
)

type metadataStore struct {
Expand Down
107 changes: 97 additions & 10 deletions cli/context/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ import (
"encoding/json"
"io"
"net/http"
"os"
"path"
"path/filepath"
"regexp"
"strings"

"github.com/docker/docker/errdefs"
"github.com/gofrs/flock"
"github.com/hashicorp/go-multierror"
"github.com/opencontainers/go-digest"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -98,6 +101,7 @@ type ContextTLSData struct {
// If the directory does not exist or is empty, initialize it
func New(dir string, cfg Config) *ContextStore {
metaRoot := filepath.Join(dir, metadataDir)
lockPath := filepath.Join(dir, lockFile)
tlsRoot := filepath.Join(dir, tlsDir)

return &ContextStore{
Expand All @@ -108,17 +112,44 @@ func New(dir string, cfg Config) *ContextStore {
tls: &tlsStore{
root: tlsRoot,
},
lockFile: flock.New(lockPath),
}
}

// ContextStore implements Store.
type ContextStore struct {
meta *metadataStore
tls *tlsStore
meta *metadataStore
tls *tlsStore
lockFile *flock.Flock
}

func (s *ContextStore) lock() error {
if err := os.MkdirAll(filepath.Dir(s.lockFile.Path()), 0o755); err != nil {
return errors.Wrapf(err, "creating context store lock directory")
}
if err := s.lockFile.Lock(); err != nil {
return errors.Wrapf(err, "locking context store lock")
}
return nil
}

func (s *ContextStore) unlock() error {
if err := s.lockFile.Unlock(); err != nil {
return errors.Wrapf(err, "unlocking context store lock")
}
return nil
}

// List return all contexts.
func (s *ContextStore) List() ([]Metadata, error) {
func (s *ContextStore) List() (_ []Metadata, errs error) {
if err := s.lock(); err != nil {
return nil, err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
return s.meta.list()
}

Expand All @@ -136,12 +167,28 @@ func Names(s Lister) ([]string, error) {
}

// CreateOrUpdate creates or updates metadata for the context.
func (s *ContextStore) CreateOrUpdate(meta Metadata) error {
func (s *ContextStore) CreateOrUpdate(meta Metadata) (errs error) {
if err := s.lock(); err != nil {
return err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
return s.meta.createOrUpdate(meta)
}

// Remove deletes the context with the given name, if found.
func (s *ContextStore) Remove(name string) error {
func (s *ContextStore) Remove(name string) (errs error) {
if err := s.lock(); err != nil {
return err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
if err := s.meta.remove(name); err != nil {
return errors.Wrapf(err, "failed to remove context %s", name)
}
Expand All @@ -153,13 +200,29 @@ func (s *ContextStore) Remove(name string) error {

// GetMetadata returns the metadata for the context with the given name.
// It returns an errdefs.ErrNotFound if the context was not found.
func (s *ContextStore) GetMetadata(name string) (Metadata, error) {
func (s *ContextStore) GetMetadata(name string) (_ Metadata, errs error) {
if err := s.lock(); err != nil {
return Metadata{}, err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
return s.meta.get(name)
}

// ResetTLSMaterial removes TLS data for all endpoints in the context and replaces
// it with the new data.
func (s *ContextStore) ResetTLSMaterial(name string, data *ContextTLSData) error {
func (s *ContextStore) ResetTLSMaterial(name string, data *ContextTLSData) (errs error) {
if err := s.lock(); err != nil {
return err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
if err := s.tls.remove(name); err != nil {
return err
}
Expand All @@ -178,7 +241,15 @@ func (s *ContextStore) ResetTLSMaterial(name string, data *ContextTLSData) error

// ResetEndpointTLSMaterial removes TLS data for the given context and endpoint,
// and replaces it with the new data.
func (s *ContextStore) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) error {
func (s *ContextStore) ResetEndpointTLSMaterial(contextName string, endpointName string, data *EndpointTLSData) (errs error) {
if err := s.lock(); err != nil {
return err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
if err := s.tls.removeEndpoint(contextName, endpointName); err != nil {
return err
}
Expand All @@ -195,13 +266,29 @@ func (s *ContextStore) ResetEndpointTLSMaterial(contextName string, endpointName

// ListTLSFiles returns the list of TLS files present for each endpoint in the
// context.
func (s *ContextStore) ListTLSFiles(name string) (map[string]EndpointFiles, error) {
func (s *ContextStore) ListTLSFiles(name string) (_ map[string]EndpointFiles, errs error) {
if err := s.lock(); err != nil {
return nil, err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
return s.tls.listContextData(name)
}

// GetTLSData reads, and returns the content of the given fileName for an endpoint.
// It returns an errdefs.ErrNotFound if the file was not found.
func (s *ContextStore) GetTLSData(contextName, endpointName, fileName string) ([]byte, error) {
func (s *ContextStore) GetTLSData(contextName, endpointName, fileName string) (_ []byte, errs error) {
if err := s.lock(); err != nil {
return nil, err
}
defer func() {
if err := s.unlock(); err != nil {
errs = multierror.Append(errs, err)
}
}()
return s.tls.getData(contextName, endpointName, fileName)
}

Expand Down
9 changes: 9 additions & 0 deletions cli/context/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ import (
is "gotest.tools/v3/assert/cmp"
)

func TestNew(t *testing.T) {
s := New(path.Join(t.TempDir(), "does", "not", "exist", "yet"), testCfg)
assert.Assert(t, s != nil)
// Check that the file lock works even when the directory does not exist yet.
all, err := s.List()
assert.NilError(t, err)
assert.Assert(t, len(all) == 0)
}

type endpoint struct {
Foo string `json:"a_very_recognizable_field_name"`
}
Expand Down

0 comments on commit c37bb67

Please sign in to comment.