From 261cc52cbc81abe7d09bee0bbd76416470af90ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Rozlach?= Date: Wed, 5 Oct 2016 14:08:00 +0200 Subject: [PATCH] Post-review fixes for file/zk recursive empty prefix delete --- physical/file.go | 10 +++++++--- physical/zookeeper.go | 8 ++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/physical/file.go b/physical/file.go index 3037c06689..25feee0549 100644 --- a/physical/file.go +++ b/physical/file.go @@ -41,6 +41,10 @@ func newFileBackend(conf map[string]string, logger log.Logger) (Backend, error) } func (b *FileBackend) Delete(path string) error { + if path == "" { + return nil + } + b.l.Lock() defer b.l.Unlock() @@ -49,7 +53,7 @@ func (b *FileBackend) Delete(path string) error { err := os.Remove(fullPath) if err != nil && !os.IsNotExist(err) { - return fmt.Errorf("Failed to remove `%s`: %v", fullPath, err) + return fmt.Errorf("Failed to remove %q: %v", fullPath, err) } err = b.cleanupLogicalPath(path) @@ -60,9 +64,9 @@ func (b *FileBackend) Delete(path string) error { // cleanupLogicalPath is used to remove all empty nodes, begining with deepest // one, aborting on first non-empty one, up to top-level node. func (b *FileBackend) cleanupLogicalPath(path string) error { - nodes := strings.Split(path, "/") + nodes := strings.Split(path, fmt.Sprintf("%c", os.PathSeparator)) for i := len(nodes) - 1; i > 0; i-- { - fullPath := b.Path + "/" + strings.Join(nodes[:i], "/") + fullPath := filepath.Join(b.Path, filepath.Join(nodes[:i]...)) dir, err := os.Open(fullPath) if err != nil { diff --git a/physical/zookeeper.go b/physical/zookeeper.go index 86c73319a9..7ef14f5031 100644 --- a/physical/zookeeper.go +++ b/physical/zookeeper.go @@ -242,13 +242,17 @@ func (c *ZookeeperBackend) Get(key string) (*Entry, error) { func (c *ZookeeperBackend) Delete(key string) error { defer metrics.MeasureSince([]string{"zookeeper", "delete"}, time.Now()) + if key == "" { + return nil + } + // Delete the full path fullPath := c.nodePath(key) err := c.client.Delete(fullPath, -1) // Mask if the node does not exist if err != nil && err != zk.ErrNoNode { - return fmt.Errorf("Failed to remove `%s`: %v", fullPath, err) + return fmt.Errorf("Failed to remove %q: %v", fullPath, err) } err = c.cleanupLogicalPath(key) @@ -285,7 +289,7 @@ func (c *ZookeeperBackend) List(prefix string) ([]string, error) { // and append the slash which is what Vault depends on // for iteration if stat.DataLength > 0 && stat.NumChildren > 0 { - msgFmt := "Node %s is both of data and leaf type ??" + msgFmt := "Node %q is both of data and leaf type ??" panic(fmt.Sprintf(msgFmt, childPath)) } else if stat.DataLength == 0 { // No, we cannot differentiate here on number of children as node