refactor: adopt cinc-api data bag, policy and cookbook helpers

apps/cinc/cmd/client.go

@@ -1,7 +1,6 @@
 package cmd
 
 import (
-	"encoding/json"
 	"fmt"
 	"os"
 
@@ -111,12 +110,8 @@ cinc client edit worker-01`,
 
 			var updated cinc.APIClient
 			if inputFile != "" {
-				data, err := os.ReadFile(inputFile)
-				if err != nil {
-					return fmt.Errorf("cinc: read %s: %w", inputFile, err)
-				}
-				if err := json.Unmarshal(data, &updated); err != nil {
-					return fmt.Errorf("cinc: parse %s: %w", inputFile, err)
+				if updated, err = readJSONFile[cinc.APIClient](inputFile); err != nil {
+					return err
 				}
 			} else {
 				current, _, err := c.Clients.Get(cmd.Context(), name)

apps/cinc/cmd/common.go

@@ -133,19 +133,37 @@ func resolveSecret(cmd *cobra.Command, profile config.Profile) ([]byte, error) {
 	return nil, errors.New("we need an encrypted data bag secret but couldn't find one. Pass --secret-file <path> (or --secret <literal>), set $CINC_SECRET_FILE, or add a secret_file key to your credentials profile.")
 }
 
-// readSecretFile reads a secret file the way Chef does: its contents with
-// leading and trailing whitespace stripped. A read error, or a file with
-// nothing left once stripped, becomes a conversational message.
+// readSecretFile reads a secret file the way Chef's load_secret does (see
+// cinc.LoadDataBagSecret: the ends are stripped, the contents must be
+// UTF-8). A file we can't read, or one with nothing left once stripped,
+// becomes a conversational message.
 func readSecretFile(path string) ([]byte, error) {
+	secret, err := cinc.LoadDataBagSecret(path)
+	var pathErr *fs.PathError
+	switch {
+	case errors.Is(err, cinc.ErrEmptyDataBagSecret):
+		return nil, fmt.Errorf("the data bag secret file at %s is empty. Put the shared secret in it, or point --secret-file at the right file.", path)
+	case errors.As(err, &pathErr):
+		return nil, fmt.Errorf("we couldn't read the data bag secret at %s: %w", path, pathErr.Err)
+	case err != nil:
+		return nil, fmt.Errorf("we can't use the data bag secret at %s: %w", path, err)
+	}
+	return secret, nil
+}
+
+// readJSONFile reads the JSON document a --file flag points at into a T.
+// Every `create`/`edit --file` path goes through it, so they all report a
+// missing file or bad JSON the same way.
+func readJSONFile[T any](path string) (T, error) {
+	var v T
 	data, err := os.ReadFile(path)
 	if err != nil {
-		return nil, fmt.Errorf("can't read the data bag secret at %s: %w", path, err)
+		return v, fmt.Errorf("we couldn't read %s: %w", path, err)
 	}
-	secret := bytes.TrimSpace(data)
-	if len(secret) == 0 {
-		return nil, fmt.Errorf("the data bag secret file at %s is empty. Put the shared secret in it, or point --secret-file at the right file.", path)
+	if err := json.Unmarshal(data, &v); err != nil {
+		return v, fmt.Errorf("%s isn't valid JSON: %w", path, err)
 	}
-	return secret, nil
+	return v, nil
 }
 
 // resolveClient builds a server client from the --config and --profile
@@ -155,6 +173,13 @@ func resolveClient(cmd *cobra.Command) (*cinc.Client, error) {
 	if err != nil {
 		return nil, err
 	}
+	return clientForProfile(cmd, profile)
+}
+
+// clientForProfile builds a server client for an already-resolved profile,
+// for commands that need the profile for something else too (the data bag
+// secret, say) and shouldn't load the credentials file twice.
+func clientForProfile(cmd *cobra.Command, profile config.Profile) (*cinc.Client, error) {
 	c, err := client.New(profile)
 	if errors.Is(err, config.ErrMissingServerURL) {
 		return nil, missingServerURLError(cmd)

apps/cinc/cmd/cookbook.go

@@ -28,8 +28,8 @@ func newCookbookCmd() *cobra.Command {
 }
 
 // newCookbookDownloadCmd builds the `cinc cookbook download <name> [version]`
-// command. With no version it resolves the "_latest" sentinel the Chef Server
-// exposes for the highest semver. Every file in the version's manifest is
+// command. With no version it asks the server for cinc.LatestVersion, which
+// resolves to the highest version. Every file in the version's manifest is
 // written under <dir>/<name>-<version>/ (recreating the cookbook layout),
 // where <dir> defaults to the current directory and is overridable with
 // --dir, matching knife's `cookbook download`.
@@ -49,18 +49,19 @@ cinc cookbook download nginx 1.2.0 --dir ./cookbooks`,
 				return err
 			}
 			name := args[0]
-			version := "_latest"
+			version := cinc.LatestVersion
 			if len(args) == 2 {
 				version = args[1]
 			}
-			// Resolve the concrete version first so "_latest" never leaks into
-			// the destination directory name.
+			// Fetch the manifest first so the directory is named after the
+			// concrete version, never "_latest", then download from that same
+			// manifest.
 			cb, _, err := c.Cookbooks.Get(cmd.Context(), name, version)
 			if err != nil {
 				return err
 			}
 			destDir := filepath.Join(dir, name+"-"+cb.Version)
-			if err := c.Cookbooks.Download(cmd.Context(), name, cb.Version, destDir); err != nil {
+			if err := c.Cookbooks.DownloadFiles(cmd.Context(), cb, destDir); err != nil {
 				return err
 			}
 			fmt.Fprintf(cmd.OutOrStdout(), "Downloaded cookbook %q version %s to %s\n", name, cb.Version, destDir)
@@ -72,8 +73,8 @@ cinc cookbook download nginx 1.2.0 --dir ./cookbooks`,
 }
 
 // newCookbookShowCmd builds the `cinc cookbook show <name> [version]`
-// command. With no version the command resolves the special "_latest"
-// sentinel that the Chef Server exposes for the highest semver.
+// command. With no version it shows cinc.LatestVersion, which the server
+// resolves to the highest version.
 func newCookbookShowCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "show <name> [version]",
@@ -92,7 +93,7 @@ cinc cookbook show nginx 1.2.0`,
 			if err != nil {
 				return err
 			}
-			version := "_latest"
+			version := cinc.LatestVersion
 			if len(args) == 2 {
 				version = args[1]
 			}

apps/cinc/cmd/cookbook_download_test.go

@@ -16,8 +16,8 @@ import (
 // bodies, since cinc-api verifies every download. The manifest's file URLs point back at this
 // same server, mirroring how the real server hands out bookshelf URLs.
 // requestedVersions records, in order, the version segments the manifest was
-// fetched under, so tests can assert "_latest" resolution (the command fetches
-// once to resolve the concrete version, then the download re-fetches it).
+// fetched under, so tests can assert "_latest" resolution and that the
+// manifest is fetched only once.
 func cookbookDownloadServer(t *testing.T, requestedVersions *[]string) *httptest.Server {
 	t.Helper()
 	var base string
@@ -63,10 +63,11 @@ func TestCookbookDownloadWritesFilesUnderNameVersionDir(t *testing.T) {
 		t.Fatalf("cinc cookbook download: %v", err)
 	}
 
-	// No explicit version => the first fetch resolves the "_latest" sentinel
-	// server-side, and "_latest" never appears in the destination dir name.
-	if len(requested) == 0 || requested[0] != "_latest" {
-		t.Errorf("first manifest fetch under %v, want it to start with _latest", requested)
+	// No explicit version => one fetch resolves the "_latest" sentinel
+	// server-side, the files come from that same manifest, and "_latest"
+	// never appears in the destination dir name.
+	if len(requested) != 1 || requested[0] != "_latest" {
+		t.Errorf("manifest fetched under %v, want exactly one fetch, of _latest", requested)
 	}
 
 	cbDir := filepath.Join(destParent, "nginx-1.2.0")
@@ -97,10 +98,8 @@ func TestCookbookDownloadAcceptsExplicitVersion(t *testing.T) {
 	if err := root.Execute(); err != nil {
 		t.Fatalf("cinc cookbook download nginx 1.2.0: %v", err)
 	}
-	for _, v := range requested {
-		if v != "1.2.0" {
-			t.Errorf("manifest fetched under version %q, want only 1.2.0", v)
-		}
+	if len(requested) != 1 || requested[0] != "1.2.0" {
+		t.Errorf("manifest fetched under %v, want exactly one fetch, of 1.2.0", requested)
 	}
 	if _, err := os.Stat(filepath.Join(destParent, "nginx-1.2.0", "metadata.rb")); err != nil {
 		t.Errorf("expected cookbook downloaded to nginx-1.2.0/: %v", err)

apps/cinc/cmd/databag.go

@@ -2,10 +2,8 @@ package cmd
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
 	"fmt"
-	"os"
 	"slices"
 
 	cinc "github.com/cinc-project/cinc-api"
@@ -41,21 +39,7 @@ func newDataBagShowCmd() *cobra.Command {
 		Example: `Show the item IDs in a data bag.
 cinc databag show passwords`,
 		Args: cobra.ExactArgs(1),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			format, err := resolveFormat(cmd)
-			if err != nil {
-				return err
-			}
-			c, err := resolveClient(cmd)
-			if err != nil {
-				return err
-			}
-			ids, err := fetchDataBagItemIDs(cmd.Context(), c, args[0])
-			if err != nil {
-				return err
-			}
-			return printer.New(cmd.OutOrStdout(), format).List(ids)
-		},
+		RunE: runListDataBagItems,
 	}
 }
 
@@ -120,7 +104,6 @@ cinc databag create passwords mysql`,
 			}
 
 			id := args[1]
-			item["id"] = id
 			if _, _, err := c.DataBags.Items(bag).Create(cmd.Context(), item); err != nil {
 				return err
 			}
@@ -132,27 +115,37 @@ cinc databag create passwords mysql`,
 	return cmd
 }
 
-// loadOrEditNewItem produces a DataBagItem either by reading the
-// given file or by opening the editor on a stub item carrying just
-// the id field.
+// loadOrEditNewItem produces the new item id, either from --file (see
+// readDataBagItemFile) or by opening the editor on a stub carrying just
+// the id.
 func loadOrEditNewItem(id, inputFile string) (cinc.DataBagItem, error) {
 	if inputFile != "" {
-		data, err := os.ReadFile(inputFile)
-		if err != nil {
-			return nil, fmt.Errorf("cinc: read %s: %w", inputFile, err)
-		}
-		if err := validateDataBagItem(data); err != nil {
-			return nil, err
-		}
-		var item cinc.DataBagItem
-		if err := json.Unmarshal(data, &item); err != nil {
-			return nil, fmt.Errorf("cinc: parse %s: %w", inputFile, err)
-		}
-		return item, nil
+		return readDataBagItemFile(inputFile, id)
 	}
 	return editDataBagItem(cinc.DataBagItem{"id": id})
 }
 
+// readDataBagItemFile reads the item id from the JSON file at path. The
+// file has to be a whole item, "id" included, and that id has to be the one
+// on the command line. We refuse a mismatch rather than quietly storing the
+// file under the argument's id: a file naming another item is almost always
+// the wrong file (or one copied from another item), and for an edit that
+// would overwrite one item with the content meant for another. Nothing has
+// been sent yet, so refusing costs the user one fix.
+func readDataBagItemFile(path, id string) (cinc.DataBagItem, error) {
+	item, err := readJSONFile[cinc.DataBagItem](path)
+	if err != nil {
+		return nil, err
+	}
+	if item.Validate() != nil {
+		return nil, fmt.Errorf("%s doesn't say which item it is. Add \"id\": %q to it.", path, id)
+	}
+	if item.ID() != id {
+		return nil, fmt.Errorf("%s is the item %q, but you asked for %q. Change the file's id to %q, or name %q on the command line.", path, item.ID(), id, id, item.ID())
+	}
+	return item, nil
+}
+
 // newDataBagItemCmd builds the `cinc databag item` command group.
 // Data bag items are arbitrary JSON documents (always carrying an
 // "id" key) so the item-level verbs live under their own subgroup.
@@ -171,8 +164,8 @@ func newDataBagItemCmd() *cobra.Command {
 
 // newDataBagItemCreateCmd builds `cinc databag item create <bag> <id>`. The
 // bag must already exist (use `cinc databag create` to make one). Without
-// --file the built-in JSON editor opens on a stub carrying just the id; the
-// path arg's id always pins the item identifier. This is the item-scoped
+// --file the built-in JSON editor opens on a stub carrying just the id; a
+// file or edit naming a different id is refused. This is the item-scoped
 // equivalent of the two-arg `databag create <bag> <item>` form.
 func newDataBagItemCreateCmd() *cobra.Command {
 	var inputFile string
@@ -192,7 +185,6 @@ cinc databag item create passwords mysql`,
 			if err != nil {
 				return err
 			}
-			item["id"] = id
 			if _, _, err := c.DataBags.Items(bag).Create(cmd.Context(), item); err != nil {
 				return err
 			}
@@ -267,22 +259,26 @@ func newDataBagItemListCmd() *cobra.Command {
 		Example: `List the item IDs in a data bag.
 cinc databag item list passwords`,
 		Args: cobra.ExactArgs(1),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			format, err := resolveFormat(cmd)
-			if err != nil {
-				return err
-			}
-			c, err := resolveClient(cmd)
-			if err != nil {
-				return err
-			}
-			ids, err := fetchDataBagItemIDs(cmd.Context(), c, args[0])
-			if err != nil {
-				return err
-			}
-			return printer.New(cmd.OutOrStdout(), format).List(ids)
-		},
+		RunE: runListDataBagItems,
+	}
+}
+
+// runListDataBagItems prints the item IDs in the bag args[0]. It backs both
+// `databag show` and `databag item list`, which are the same question.
+func runListDataBagItems(cmd *cobra.Command, args []string) error {
+	format, err := resolveFormat(cmd)
+	if err != nil {
+		return err
+	}
+	c, err := resolveClient(cmd)
+	if err != nil {
+		return err
 	}
+	ids, err := fetchDataBagItemIDs(cmd.Context(), c, args[0])
+	if err != nil {
+		return err
+	}
+	return printer.New(cmd.OutOrStdout(), format).List(ids)
 }
 
 // fetchDataBagItemIDs returns the sorted item IDs within a single bag.
@@ -303,8 +299,8 @@ func fetchDataBagItemIDs(ctx context.Context, c *cinc.Client, bag string) ([]str
 // It fetches the item, opens its JSON in the built-in editor (same
 // engine as `cinc client edit`), validates on save, and PUTs the
 // result back. `--file` reads the updated JSON from disk for
-// scripted use. The path arg's id pins the item identifier so an
-// edit can never accidentally rename the item out from under itself.
+// scripted use. A file or edit naming a different id is refused, so an
+// edit can never rename the item or overwrite it with another's content.
 func newDataBagItemEditCmd() *cobra.Command {
 	var inputFile string
 	cmd := &cobra.Command{
@@ -323,14 +319,7 @@ cinc databag item edit passwords mysql`,
 
 			var updated cinc.DataBagItem
 			if inputFile != "" {
-				data, err := os.ReadFile(inputFile)
-				if err != nil {
-					return fmt.Errorf("cinc: read %s: %w", inputFile, err)
-				}
-				if err := json.Unmarshal(data, &updated); err != nil {
-					return fmt.Errorf("cinc: parse %s: %w", inputFile, err)
-				}
-				if err := validateDataBagItem(data); err != nil {
+				if updated, err = readDataBagItemFile(inputFile, id); err != nil {
 					return err
 				}
 			} else {
@@ -348,7 +337,6 @@ cinc databag item edit passwords mysql`,
 				}
 				updated = edited
 			}
-			updated["id"] = id
 
 			if _, _, err := items.Update(cmd.Context(), updated); err != nil {
 				return err
@@ -361,19 +349,6 @@ cinc databag item edit passwords mysql`,
 	return cmd
 }
 
-// validateDataBagItem checks that b is valid JSON and contains a
-// non-empty string "id" key. It is exported via the editor seam.
-func validateDataBagItem(b []byte) error {
-	var item cinc.DataBagItem
-	if err := json.Unmarshal(b, &item); err != nil {
-		return err
-	}
-	if item.ID() == "" {
-		return errors.New("data bag item is missing a non-empty \"id\" field")
-	}
-	return nil
-}
-
 // newDataBagDeleteCmd builds the `cinc databag delete <name>` command.
 func newDataBagDeleteCmd() *cobra.Command {
 	return &cobra.Command{