From a5a0494eea7899601f00798866db4a82152d02e7 Mon Sep 17 00:00:00 2001 From: Roberto Hidalgo Date: Mon, 27 Nov 2023 19:42:35 -0600 Subject: [PATCH] more tests, less failures to find repo configs --- .milpa/commands/joao/test/unit.sh | 1 + cmd/flush_test.go | 116 +++++++++++++++++++++++ internal/op-client/cli.go | 10 +- internal/op-client/connect.go | 6 +- internal/op-client/op.go | 23 ++++- internal/testdata/fixtures.go | 25 +++++ internal/testdata/opconnect/opconnect.go | 16 ++-- pkg/config/config.go | 3 +- pkg/config/lookup.go | 1 + pkg/config/util.go | 68 +++++++++---- 10 files changed, 230 insertions(+), 39 deletions(-) create mode 100644 cmd/flush_test.go diff --git a/.milpa/commands/joao/test/unit.sh b/.milpa/commands/joao/test/unit.sh index ac6ffc3..9b9c02e 100644 --- a/.milpa/commands/joao/test/unit.sh +++ b/.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" diff --git a/cmd/flush_test.go b/cmd/flush_test.go new file mode 100644 index 0000000..76fa878 --- /dev/null +++ b/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) + } +} diff --git a/internal/op-client/cli.go b/internal/op-client/cli.go index d99a526..f37143c 100644 --- a/internal/op-client/cli.go +++ b/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 { - logrus.Infof("Creating new item: %s/%s", item.Vault.ID, item.Title) +func (b *CLI) Create(vault string, item *op.Item) error { + 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 } diff --git a/internal/op-client/connect.go b/internal/op-client/connect.go index 4bc942c..37f1c6b 100644 --- a/internal/op-client/connect.go +++ b/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 { - _, err := b.client.CreateItem(item, item.Vault.ID) +func (b *Connect) Create(vault string, item *op.Item) error { + _, err := b.client.CreateItem(item, vault) return err } diff --git a/internal/op-client/op.go b/internal/op-client/op.go index 7ba9281..06e6c92 100644 --- a/internal/op-client/op.go +++ b/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)) { - return client.Create(item) + if ItemMissingError(name, err) { + return client.Create(vault, item) } return fmt.Errorf("could not fetch remote 1password item to compare against: %w", err) diff --git a/internal/testdata/fixtures.go b/internal/testdata/fixtures.go index e5f29a9..3d02f4c 100644 --- a/internal/testdata/fixtures.go +++ b/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) } diff --git a/internal/testdata/opconnect/opconnect.go b/internal/testdata/opconnect/opconnect.go index 986bf40..ccfc4dc 100644 --- a/internal/testdata/opconnect/opconnect.go +++ b/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 diff --git a/pkg/config/config.go b/pkg/config/config.go index 051950a..91414e1 100644 --- a/pkg/config/config.go +++ b/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) } } diff --git a/pkg/config/lookup.go b/pkg/config/lookup.go index 35deb69..73f6849 100644 --- a/pkg/config/lookup.go +++ b/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-- { diff --git a/pkg/config/util.go b/pkg/config/util.go index 922748e..3fbe710 100644 --- a/pkg/config/util.go +++ b/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{ - "DirName": func() string { - return filepath.Base(filepath.Dir(path)) - }, - "FileName": func() string { - return strings.Split(filepath.Base(path), ".")[0] - }, - }).Parse(rmc.NameTemplate)) + if name == "" { + if rmc.NameTemplate == "" { + rmc.NameTemplate = "{{ DirName }}:{{ FileName}}" + } + logrus.Tracef("Generating name for path %s from template %s", path, rmc.NameTemplate) - var nameBuf bytes.Buffer - err = tpl.Execute(&nameBuf, nil) - if err != nil { - return "", "", err + tpl := template.Must(template.New("help").Funcs(template.FuncMap{ + "DirName": func() string { + return filepath.Base(filepath.Dir(path)) + }, + "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 {