Skip to content

Commit

Permalink
context: avoid corrupt file writes
Browse files Browse the repository at this point in the history
Write to a tempfile then move, so that if the
process dies mid-write it doesn't corrupt the store.

Also improve error messaging so that if a file does
get corrupted, the user has some hope of figuring
out which file is broken.

For background, see:
docker/for-win#13180
docker/for-win#12561

For a repro case, see:
https://github.com/nicks/contextstore-sandbox

Signed-off-by: Nick Santos <nick.santos@docker.com>
  • Loading branch information
nicks committed Feb 21, 2023
1 parent dfb36ea commit c2487c2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
13 changes: 8 additions & 5 deletions cli/context/store/metadatastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package store

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"reflect"
"sort"

"github.com/docker/docker/errdefs"
"github.com/docker/docker/pkg/ioutils"
"github.com/fvbommel/sortorder"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -35,7 +37,7 @@ func (s *metadataStore) createOrUpdate(meta Metadata) error {
if err != nil {
return err
}
return os.WriteFile(filepath.Join(contextDir, metaFile), bytes, 0o644)
return ioutils.AtomicWriteFile(filepath.Join(contextDir, metaFile), bytes, 0o644)
}

func parseTypedOrMap(payload []byte, getter TypeGetter) (interface{}, error) {
Expand Down Expand Up @@ -65,7 +67,8 @@ func (s *metadataStore) get(name string) (Metadata, error) {
}

func (s *metadataStore) getByID(id contextdir) (Metadata, error) {
bytes, err := os.ReadFile(filepath.Join(s.contextDir(id), metaFile))
fileName := filepath.Join(s.contextDir(id), metaFile)
bytes, err := os.ReadFile(fileName)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return Metadata{}, errdefs.NotFound(errors.Wrap(err, "context not found"))
Expand All @@ -77,15 +80,15 @@ func (s *metadataStore) getByID(id contextdir) (Metadata, error) {
Endpoints: make(map[string]interface{}),
}
if err := json.Unmarshal(bytes, &untyped); err != nil {
return Metadata{}, err
return Metadata{}, fmt.Errorf("parsing %s: %v", fileName, err)
}
r.Name = untyped.Name
if r.Metadata, err = parseTypedOrMap(untyped.Metadata, s.config.contextType); err != nil {
return Metadata{}, err
return Metadata{}, fmt.Errorf("parsing %s: %v", fileName, err)
}
for k, v := range untyped.Endpoints {
if r.Endpoints[k], err = parseTypedOrMap(v, s.config.endpointTypes[k]); err != nil {
return Metadata{}, err
return Metadata{}, fmt.Errorf("parsing %s: %v", fileName, err)
}
}
return r, err
Expand Down
27 changes: 27 additions & 0 deletions cli/context/store/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import (
"bytes"
"crypto/rand"
"encoding/json"
"fmt"
"io"
"os"
"path"
"path/filepath"
"testing"

"github.com/docker/docker/errdefs"
Expand Down Expand Up @@ -230,3 +232,28 @@ func TestImportZipInvalid(t *testing.T) {
err = Import("zipInvalid", s, r)
assert.ErrorContains(t, err, "unexpected context file")
}

func TestCorruptMetadata(t *testing.T) {
tempDir := t.TempDir()
s := New(tempDir, testCfg)
err := s.CreateOrUpdate(
Metadata{
Endpoints: map[string]interface{}{
"ep1": endpoint{Foo: "bar"},
},
Metadata: context{Bar: "baz"},
Name: "source",
})
assert.NilError(t, err)

// Simulate the meta.json file getting corrupted
// by some external process.
contextDir := s.meta.contextDir(contextdirOf("source"))
contextFile := filepath.Join(contextDir, metaFile)
err = os.WriteFile(contextFile, nil, 0o600)
assert.NilError(t, err)

// Assert that the error message gives the user some clue where to look.
_, err = s.GetMetadata("source")
assert.ErrorContains(t, err, fmt.Sprintf("parsing %s: unexpected end of JSON input", contextFile))
}
3 changes: 2 additions & 1 deletion cli/context/store/tlsstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"path/filepath"

"github.com/docker/docker/errdefs"
"github.com/docker/docker/pkg/ioutils"
"github.com/pkg/errors"
)

Expand All @@ -31,7 +32,7 @@ func (s *tlsStore) createOrUpdate(name, endpointName, filename string, data []by
if err := os.MkdirAll(endpointDir, 0o700); err != nil {
return err
}
return os.WriteFile(filepath.Join(endpointDir, filename), data, 0o600)
return ioutils.AtomicWriteFile(filepath.Join(endpointDir, filename), data, 0o600)
}

func (s *tlsStore) getData(name, endpointName, filename string) ([]byte, error) {
Expand Down

0 comments on commit c2487c2

Please sign in to comment.