more tests, less failures to find repo configs

.milpa/commands/joao/test/unit.sh

@@ -15,6 +15,7 @@ gotestsum --format testname -- "$MILPA_ARG_SPEC" "${args[@]}" || exit 2
 
 [[ ! "${MILPA_OPT_COVERAGE}" ]] && exit
 @milpa.log info "Building coverage report"
+sed -i '' '/internal\/testdata/d' coverage.out
 go tool cover -html=coverage.out -o coverage.html || @milpa.fail "could not build reports"
 go tool cover -func=coverage.out | tail -n 1
 @milpa.log complete "Coverage report ready at coverage.html"

cmd/flush_test.go

@@ -0,0 +1,116 @@
+package cmd_test
+
+import (
+	"bytes"
+	"os"
+	"regexp"
+	"strings"
+	"testing"
+
+	. "git.rob.mx/nidito/joao/cmd"
+	"git.rob.mx/nidito/joao/internal/testdata"
+	"git.rob.mx/nidito/joao/internal/testdata/opconnect"
+	"git.rob.mx/nidito/joao/pkg/config"
+	"github.com/spf13/cobra"
+)
+
+func TestFlush(t *testing.T) {
+	testdata.EnableDebugLogging()
+	testdata.MockOPConnect(t)
+	out := &bytes.Buffer{}
+	cmd := &cobra.Command{}
+	cmd.Flags().Bool("dry-run", false, "")
+	cmd.Flags().Bool("redact", false, "")
+	cmd.SetOut(out)
+	cmd.SetErr(out)
+
+	Flush.SetBindings()
+	Flush.Cobra = cmd
+	err := Flush.Run(cmd, []string{testdata.YAML("test")})
+
+	if err != nil {
+		t.Fatalf("could not flush: %s", err)
+	}
+
+	expected := ""
+
+	if got := out.String(); strings.TrimSpace(got) != expected {
+		t.Fatalf("did not get expected output:\nwanted: %s\ngot: %s", expected, got)
+	}
+
+	item, err := opconnect.Get("some:test", "example")
+	if err != nil {
+		t.Fatalf("unexpected error getting flushed config: %s", err)
+	}
+
+	cfg, err := config.FromOP(item)
+	if err != nil {
+		t.Fatalf("unexpected error translating flushed config: %s", err)
+	}
+
+	serialized, err := cfg.AsYAML()
+	if err != nil {
+		t.Fatalf("unexpected error serializing config as yaml: %s", err)
+	}
+
+	data, err := os.ReadFile(testdata.YAML("test"))
+	if err != nil {
+		t.Fatalf("unexpected error reading fixture: %s", err)
+	}
+	if bytes.Equal(serialized, data) {
+		t.Fatalf("did not get expected serialization after flush.\n wanted:\n%s\n\ngot:\n%s", serialized, data)
+	}
+}
+
+func TestFlushRedacted(t *testing.T) {
+	testdata.EnableDebugLogging()
+	testdata.MockOPConnect(t)
+	out := &bytes.Buffer{}
+	cmd := &cobra.Command{}
+	cmd.Flags().Bool("dry-run", false, "")
+	cmd.Flags().Bool("redact", true, "")
+	cmd.SetOut(out)
+	cmd.SetErr(out)
+
+	Flush.SetBindings()
+	Flush.Cobra = cmd
+	path, cleanup := testdata.TempYAML(t, "test")
+	defer cleanup()
+	err := Flush.Run(cmd, []string{path})
+
+	if err != nil {
+		t.Fatalf("could not flush: %s", err)
+	}
+
+	expected := ""
+
+	if got := out.String(); strings.TrimSpace(got) != expected {
+		t.Fatalf("did not get expected output:\nwanted: %s\ngot: %s", expected, got)
+	}
+
+	item, err := opconnect.Get("some:test", "example")
+	if err != nil {
+		t.Fatalf("unexpected error getting flushed config: %s", err)
+	}
+
+	cfg, err := config.FromOP(item)
+	if err != nil {
+		t.Fatalf("unexpected error translating flushed config: %s", err)
+	}
+
+	serialized, err := cfg.AsYAML(config.OutputModeRedacted)
+	if err != nil {
+		t.Fatalf("unexpected error serializing redacted config as yaml: %s", err)
+	}
+
+	data, err := os.ReadFile(testdata.YAML("test"))
+	if err != nil {
+		t.Fatalf("unexpected error reading fixture: %s", err)
+	}
+
+	pat := regexp.MustCompile(`!!secret\.+\n`)
+	redactedData := pat.ReplaceAll(data, []byte("!!secret\n"))
+	if bytes.Equal(serialized, redactedData) {
+		t.Fatalf("did not get expected redacted serialization after flush.\n wanted:\n%s\n\ngot:\n%s", serialized, redactedData)
+	}
+}

internal/op-client/cli.go

@@ -49,6 +49,8 @@ type CLI struct {
 	DryRun bool // Won't write to 1Password
 }
 
+var _ opClient = &CLI{}
+
 func invoke(dryRun bool, vault string, stdin *bytes.Buffer, args ...string) (bytes.Buffer, error) {
 	if vault != "" {
 		args = append([]string{"--vault", shellescape.Quote(vault)}, args...)
@@ -80,8 +82,8 @@ func (b *CLI) Get(vault, name string) (*op.Item, error) {
 	return item, nil
 }
 
-func (b *CLI) Create(item *op.Item) error {
+func (b *CLI) Create(vault string, item *op.Item) error {
-	logrus.Infof("Creating new item: %s/%s", item.Vault.ID, item.Title)
+	logrus.Infof("Creating new item: %s/%s", vault, item.Title)
 
 	itemJSON, err := json.Marshal(item)
 	if err != nil {
@@ -90,12 +92,12 @@ func (b *CLI) Create(item *op.Item) error {
 
 	stdin := bytes.NewBuffer(itemJSON)
 
-	_, err = invoke(b.DryRun, item.Vault.ID, stdin, "item", "create")
+	_, err = invoke(b.DryRun, vault, stdin, "item", "create")
 	if err != nil {
 		return fmt.Errorf("could not create item: %w", err)
 	}
 
-	logrus.Infof("Item %s/%s created", item.Vault.ID, item.Title)
+	logrus.Infof("Item %s/%s created", vault, item.Title)
 	return nil
 }
 

internal/op-client/connect.go

@@ -34,6 +34,8 @@ type Connect struct {
 	client connect.Client
 }
 
+var _ opClient = &Connect{}
+
 const userAgent = "nidito-joao"
 
 func NewConnect(host, token string) *Connect {
@@ -65,7 +67,7 @@ func (b *Connect) List(vault, prefix string) ([]string, error) {
 	return res, nil
 }
 
-func (b *Connect) Create(item *op.Item) error {
+func (b *Connect) Create(vault string, item *op.Item) error {
-	_, err := b.client.CreateItem(item, item.Vault.ID)
+	_, err := b.client.CreateItem(item, vault)
 	return err
 }

internal/op-client/op.go

@@ -6,16 +6,33 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/1Password/connect-sdk-go/onepassword"
 	op "github.com/1Password/connect-sdk-go/onepassword"
 	"github.com/sirupsen/logrus"
 )
 
+const itemMissingErrorSuffix = `" isn't an item.`            // Specify the item...
+const itemMissingErrorWithVault = `" isn't an item in the "` // "vaultName" vault. Specify the item...
+
+func ItemMissingError(name string, err error) bool {
+	if opErr, ok := err.(*onepassword.Error); ok {
+		return opErr.StatusCode == 404
+	}
+	needle := itemMissingErrorSuffix
+	needleWithVault := itemMissingErrorWithVault
+	if name != "" {
+		needle = fmt.Sprintf(`"%s`+itemMissingErrorSuffix, name)
+		needleWithVault = fmt.Sprintf(`"%s`+itemMissingErrorWithVault, name)
+	}
+	return strings.Contains(err.Error(), needle) || strings.Contains(err.Error(), needleWithVault)
+}
+
 var client opClient
 
 type opClient interface {
 	Get(vault, name string) (*op.Item, error)
 	Update(item *op.Item, remote *op.Item) error
-	Create(item *op.Item) error
+	Create(vault string, item *op.Item) error
 	List(vault, prefix string) ([]string, error)
 }
 
@@ -34,8 +51,8 @@ func Get(vault, name string) (*op.Item, error) {
 func Update(vault, name string, item *op.Item) error {
 	remote, err := client.Get(vault, name)
 	if err != nil {
-		if strings.Contains(err.Error(), fmt.Sprintf("\"%s\" isn't an item in ", name)) {
+		if ItemMissingError(name, err) {
-			return client.Create(item)
+			return client.Create(vault, item)
 		}
 
 		return fmt.Errorf("could not fetch remote 1password item to compare against: %w", err)

internal/testdata/fixtures.go

@@ -2,6 +2,7 @@ package testdata
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"path"
 	"runtime"
@@ -46,6 +47,30 @@ func YAML(name string) string {
 	return path.Join(FromProjectRoot(), "testdata", fmt.Sprintf("%s.yaml", name))
 }
 
+func copyFile(in, out string) error {
+	src, err := os.Open(in)
+	if err != nil {
+		return err
+	}
+	defer src.Close()
+	dst, err := os.Create(out)
+	if err != nil {
+		return err
+	}
+	defer dst.Close()
+	_, err = io.Copy(dst, src)
+	return err
+}
+
+func TempYAML(t *testing.T, name string) (string, func()) {
+	root := TempDir(t, "temp-yaml")
+	path := fmt.Sprintf("%s/%s.yaml", root, name)
+	if err := copyFile(YAML(name), path); err != nil {
+		t.Fatalf("could not create copy of fixture %s.yaml", name)
+	}
+	return path, func() { os.Remove(path) }
+}
+
 func EnableDebugLogging() {
 	logrus.SetLevel(logrus.DebugLevel)
 }

internal/testdata/opconnect/opconnect.go

@@ -95,15 +95,15 @@ func (m *Client) GetItems(vaultQuery string) ([]onepassword.Item, error) {
 }
 
 func (m *Client) GetItem(itemQuery, vaultQuery string) (*onepassword.Item, error) {
-	return get(itemQuery, vaultQuery)
+	return Get(itemQuery, vaultQuery)
 }
 
 func (m *Client) GetItemByUUID(uuid string, vaultQuery string) (*onepassword.Item, error) {
-	return get(uuid, vaultQuery)
+	return Get(uuid, vaultQuery)
 }
 
 func (m *Client) GetItemByTitle(title string, vaultQuery string) (*onepassword.Item, error) {
-	return get(title, vaultQuery)
+	return Get(title, vaultQuery)
 }
 
 func (m *Client) GetItemsByTitle(title string, vaultQuery string) ([]onepassword.Item, error) {
@@ -132,7 +132,7 @@ func (m *Client) DeleteItem(item *onepassword.Item, vaultQuery string) error {
 }
 
 func (m *Client) DeleteItemByID(itemUUID string, vaultQuery string) error {
-	item, err := get(itemUUID, vaultQuery)
+	item, err := Get(itemUUID, vaultQuery)
 	if err != nil {
 		return err
 	}
@@ -140,7 +140,7 @@ func (m *Client) DeleteItemByID(itemUUID string, vaultQuery string) error {
 }
 
 func (m *Client) DeleteItemByTitle(title string, vaultQuery string) error {
-	item, err := get(title, vaultQuery)
+	item, err := Get(title, vaultQuery)
 	if err != nil {
 		return err
 	}
@@ -187,19 +187,19 @@ func itemID() string {
 	return string(b)
 }
 
-func get(itemUUID, vaultUUID string) (*onepassword.Item, error) {
+func Get(itemUUID, vaultUUID string) (*onepassword.Item, error) {
 	for _, item := range items {
 		if (item.ID == itemUUID || item.Title == itemUUID) && item.Vault.ID == vaultUUID {
 			return item, nil
 		}
 	}
 
-	return nil, fmt.Errorf("could not retrieve item with id %s in vault %s", itemUUID, vaultUUID)
+	return nil, &onepassword.Error{StatusCode: 404, Message: fmt.Sprintf("could not retrieve item with id %s in vault %s", itemUUID, vaultUUID)}
 }
 
 func deleteItem(item *onepassword.Item, vaultUUID string) error {
 	if item.Vault.ID != vaultUUID {
-		return fmt.Errorf("could not delete item: %s: not found in vault %s", item.Title, vaultUUID)
+		return &onepassword.Error{StatusCode: 404, Message: fmt.Sprintf("could not delete item: %s: not found in vault %s", item.Title, vaultUUID)}
 	}
 	Delete(item.ID)
 	return nil

pkg/config/config.go

@@ -9,6 +9,7 @@ import (
 	"os/exec"
 	"strings"
 
+	opclient "git.rob.mx/nidito/joao/internal/op-client"
 	op "github.com/1Password/connect-sdk-go/onepassword"
 	"github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v3"
@@ -167,7 +168,7 @@ func (cfg *Config) DiffRemote(path string, redacted, asFetch bool, stdout, stder
 			return err
 		}
 
-		if !strings.Contains(err.Error(), " isn't an item in ") {
+		if !opclient.ItemMissingError("", err) {
 			return fmt.Errorf("could not fetch remote item: %w", err)
 		}
 	}

pkg/config/lookup.go

@@ -14,6 +14,7 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
+// returns repo config details, error on failure to open/parse found files
 func findRepoConfig(from string) (*opDetails, error) {
 	parts := strings.Split(from, "/")
 	for i := len(parts); i > 0; i-- {

pkg/config/util.go

@@ -29,8 +29,15 @@ func argIsYAMLFile(path string) bool {
 	return strings.HasSuffix(path, ".yaml") || strings.HasSuffix(path, ".yml")
 }
 
+// VaultAndNameFrom path/buffer reads a path (unless a buffer is provided) and gets the 1Password
+// item name and vault name:
+// first, it looks at the embedded `_config: !!joao` YAML item.
+// if it still needs a vault or name, it looks for the repo config, erroring if none found
+// otherwise, it'll fill in missing values from the found repo config
 func VaultAndNameFrom(path string, buf []byte) (name string, vault string, err error) {
 	smc := &singleModeConfig{}
+
+	// if a buffer was not provided, read from filesystem
 	if buf == nil {
 		var err error
 		buf, err = os.ReadFile(path)
@@ -39,40 +46,59 @@ func VaultAndNameFrom(path string, buf []byte) (name string, vault string, err e
 		}
 	}
 
+	// decode single-mode config
 	if err = yaml.Unmarshal(buf, &smc); err == nil && smc.Config != nil {
-		return smc.Config.Name, smc.Config.Vault, nil
+		name = smc.Config.Name
+		vault = smc.Config.Vault
 	}
 
+	// if we have both name and vault, return early
+	if name != "" && vault != "" {
+		return name, vault, nil
+	}
+
+	// look for whole-repo config
 	rmc, err := findRepoConfig(path)
 	if err != nil {
-		return "", "", err
+		return name, vault, err
 	}
 
 	if rmc == nil {
-		return "", "", fmt.Errorf("could not find repo config for %s", path)
+		// no repo config found
+		return name, vault, fmt.Errorf("could not find repo config for %s", path)
 	}
-
-	if rmc.NameTemplate == "" {
-		rmc.NameTemplate = "{{ DirName }}:{{ FileName}}"
-	}
-
 	logrus.Debugf("Found repo config at %s", rmc.Repo)
 
-	tpl := template.Must(template.New("help").Funcs(template.FuncMap{
+	if name == "" {
-		"DirName": func() string {
+		if rmc.NameTemplate == "" {
-			return filepath.Base(filepath.Dir(path))
+			rmc.NameTemplate = "{{ DirName }}:{{ FileName}}"
-		},
+		}
-		"FileName": func() string {
+		logrus.Tracef("Generating name for path %s from template %s", path, rmc.NameTemplate)
-			return strings.Split(filepath.Base(path), ".")[0]
-		},
-	}).Parse(rmc.NameTemplate))
 
-	var nameBuf bytes.Buffer
+		tpl := template.Must(template.New("help").Funcs(template.FuncMap{
-	err = tpl.Execute(&nameBuf, nil)
+			"DirName": func() string {
-	if err != nil {
+				return filepath.Base(filepath.Dir(path))
-		return "", "", err
+			},
+			"FileName": func() string {
+				return strings.Split(filepath.Base(path), ".")[0]
+			},
+		}).Parse(rmc.NameTemplate))
+
+		var nameBuf bytes.Buffer
+		err = tpl.Execute(&nameBuf, nil)
+		if err != nil {
+			return "", "", fmt.Errorf("could not generate item name for %s using template %s: %s", path, rmc.NameTemplate, err)
+		}
+		name = nameBuf.String()
+		logrus.Tracef("Setting name for path %s from repo config %s", path, name)
 	}
-	return nameBuf.String(), rmc.Vault, nil
+
+	if rmc.Vault != "" && vault == "" {
+		logrus.Tracef("Setting vault for path %s from repo config %s", path, rmc.Vault)
+		vault = rmc.Vault
+	}
+
+	return name, vault, nil
 }
 
 func isNumeric(s string) bool {