Skip to content

Commit

Permalink
secrets: Fix bug about not wait for secrets.json to become available
Browse files Browse the repository at this point in the history
When we added support for CSI directories, we introduced a bug that we
no longer wait for secrets.json to become available, because os.Stat
call would fail with file not exist error immediately.

Since in CSI directories case the directory would be guaranteed to be
available immediately (unlike the vault case), assume file not exist
errors from os.Stat as using file instead of directory, and let
filewatcher.New to handle the waiting as before. Also add a test for it.

While I'm here, also remove the usages of os.IsNotExist and os.IsExist
follow the docs.
  • Loading branch information
fishy committed Nov 16, 2022
1 parent 5fc5cda commit a7cee85
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 5 deletions.
2 changes: 1 addition & 1 deletion filewatcher/filewatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ func New(ctx context.Context, cfg Config) (*Result, error) {

var err error
data, mtime, files, err = openAndParse(cfg.Path, cfg.Parser, limit, hardLimit)
if errors.Is(err, os.ErrNotExist) {
if errors.Is(err, fs.ErrNotExist) {
time.Sleep(InitialReadInterval)
continue
}
Expand Down
4 changes: 2 additions & 2 deletions filewatcher/filewatcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,14 +304,14 @@ func updateDirWithContents(tb testing.TB, dst string, contents map[string]string
for p, content := range contents {
path := filepath.Join(dir, p)
parent := filepath.Dir(path)
if err := os.Mkdir(parent, 0777); err != nil && !os.IsExist(err) {
if err := os.Mkdir(parent, 0777); err != nil && !errors.Is(err, fs.ErrExist) {
tb.Fatalf("Failed to create directory %q for %q: %v", parent, path, err)
}
if err := os.WriteFile(path, []byte(content), 0666); err != nil {
tb.Fatalf("Failed to write file %q: %v", path, err)
}
}
if err := os.RemoveAll(dst); err != nil && !os.IsNotExist(err) {
if err := os.RemoveAll(dst); err != nil && !errors.Is(err, fs.ErrNotExist) {
tb.Fatalf("Failed to remove %q: %v", dst, err)
}
if err := os.Rename(dir, dst); err != nil {
Expand Down
5 changes: 3 additions & 2 deletions secrets/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package secrets

import (
"context"
"errors"
"io"
"io/fs"
"os"
Expand Down Expand Up @@ -45,12 +46,12 @@ func NewStore(ctx context.Context, path string, logger log.Wrapper, middlewares
}
store.secretHandler(middlewares...)
fileInfo, err := os.Stat(path)
if err != nil {
if err != nil && !errors.Is(err, fs.ErrNotExist) {
return nil, err
}

parser := store.parser
if fileInfo.IsDir() {
if fileInfo != nil && fileInfo.IsDir() {
parser = filewatcher.WrapDirParser(store.dirParser)
}

Expand Down
25 changes: 25 additions & 0 deletions secrets/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package secrets_test
import (
"context"
"os"
"path/filepath"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -490,3 +491,27 @@ func TestAddMiddleware(t *testing.T) {
)
}
}

func TestNewStoreWaitBeforeAvailable(t *testing.T) {
const (
writeDelay = time.Second / 5
timeout = time.Second * 5
)
ctx, cancel := context.WithTimeout(context.Background(), timeout)
t.Cleanup(cancel)
dir := t.TempDir()
path := filepath.Join(dir, "secrets.json")
go func() {
// delay create and write secrets.json file
// note that we must also delay creating the file here.
time.Sleep(writeDelay)
if err := os.WriteFile(path, []byte(specificationExample), 0666); err != nil {
t.Errorf("Failed to write %q: %v", path, err)
}
}()
store, err := secrets.NewStore(ctx, path, nil)
if err != nil {
t.Fatalf("NewStore failed with %v", err)
}
store.Close()
}

0 comments on commit a7cee85

Please sign in to comment.