improve vault and item parsing

pkg/onepassword/items.go

@@ -3,7 +3,7 @@ package onepassword
 import (
 	"context"
 	"fmt"
-	"strings"
+	"regexp"
 
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
@@ -59,9 +59,11 @@ func GetOnePasswordItemByPath(ctx context.Context, opClient opclient.Client, pat
 }
 
 func ParseVaultAndItemFromPath(path string) (string, string, error) {
-	splitPath := strings.Split(path, "/")
-	if len(splitPath) == 4 && splitPath[0] == "vaults" && splitPath[2] == "items" {
-		return splitPath[1], splitPath[3], nil
+	r := regexp.MustCompile("vaults/(.*)/items/(.*)")
+	splitPath := r.FindStringSubmatch(path)
+
+	if len(splitPath) == 3 {
+		return splitPath[1], splitPath[2], nil
 	}
 	return "", "", fmt.Errorf(
 		"%q is not an acceptable path for One Password item. Must be of the format: `vaults/{vault_id}/items/{item_id}`",

pkg/onepassword/items_test.go

@@ -0,0 +1,138 @@
+package onepassword
+
+import (
+	"fmt"
+	"testing"
+)
+
+const badPathErrFmt = "%q is not an acceptable path for One Password item. " +
+	"Must be of the format: `vaults/{vault_id}/items/{item_id}`"
+
+func TestParseVaultAndItemFromPath(t *testing.T) {
+	cases := []struct {
+		Path  string
+		Vault string
+		Item  string
+		Error error
+	}{
+		{
+			"vaults/foo/items/bar",
+			"foo",
+			"bar",
+			nil,
+		},
+		{
+			"vaults/foo/items/bar/baz",
+			"foo",
+			"bar/baz",
+			nil,
+		},
+		{
+			"vaults/foo/bar/items/baz",
+			"foo/bar",
+			"baz",
+			nil,
+		},
+		{
+			"foo/bar",
+			"",
+			"",
+			fmt.Errorf(badPathErrFmt, "foo/bar"),
+		},
+		{
+			"vaults/foo1/items/bar1/vaults/foo2/items/bar2",
+			"foo1/items/bar1/vaults/foo2",
+			"bar2",
+			nil,
+		},
+		{
+			"items/bar/vaults/foo",
+			"",
+			"",
+			fmt.Errorf(badPathErrFmt, "items/bar/vaults/foo"),
+		},
+		{
+			"vaults/foo",
+			"",
+			"",
+			fmt.Errorf(badPathErrFmt, "vaults/foo"),
+		},
+		{
+			"vaults/foo/items/bar/",
+			"foo",
+			"bar/",
+			nil,
+		},
+		{
+			"vaults/abc123-def456/items/xyz789-uvw012",
+			"abc123-def456",
+			"xyz789-uvw012",
+			nil,
+		},
+		{
+			"vaults/a/items/b",
+			"a",
+			"b",
+			nil,
+		},
+		{
+			"",
+			"",
+			"",
+			fmt.Errorf(badPathErrFmt, ""),
+		},
+		{
+			"vaults//foo/items/bar",
+			"/foo",
+			"bar",
+			nil,
+		},
+		{
+			"vaults/foo/items/",
+			"foo",
+			"",
+			nil,
+		},
+		{
+			"vaults/items",
+			"",
+			"",
+			fmt.Errorf(badPathErrFmt, "vaults/items"),
+		},
+		{
+			"vaults/foo bar/items/baz",
+			"foo bar",
+			"baz",
+			nil,
+		},
+		{
+			"vaults/日本/items/条目",
+			"日本",
+			"条目",
+			nil,
+		},
+		{
+			"prefix/vaults/foo/items/bar",
+			"foo",
+			"bar",
+			nil,
+		},
+	}
+
+	for _, c := range cases {
+		vault, item, err := ParseVaultAndItemFromPath(c.Path)
+
+		if err != c.Error && err.Error() != c.Error.Error() {
+			t.Errorf("unexpected error %v: %v", err, c.Error)
+		}
+
+		if vault != c.Vault {
+			t.Errorf("couldn't extract vault out of path %s: %s", c.Path, vault)
+		}
+
+		if item != c.Item {
+			t.Errorf("couldn't extract item out of path %s: %s != %s", c.Path, item, c.Item)
+		}
+
+	}
+}