Add --additionalParameterJSON to agents import and tools import

README.md

@@ -202,6 +202,18 @@ rossoctl agents import --deployment-type sandbox from-image \
 # over --envVarsURL, whichever order the flags appear in.
 rossoctl agents import from-image --name orders --containerImage ghcr.io/x/y:latest \
     --envVar LOG_LEVEL=debug --envVar 'TAGS=a,b,c'
+# --additionalParameterJSON sends request fields the CLI has no flag for. Its value
+# is a JSON dict, or the name of a file containing one — a value starting with '{'
+# is the document itself, anything else is a filename.
+rossoctl agents import from-image --name orders --containerImage ghcr.io/x/y:latest \
+    --additionalParameterJSON '{"serviceAccount":"orders-sa"}'
+# Repeatable: the dicts are merged, a later one winning a key they share, and the
+# result is overlaid onto the request body. Merging is by top-level key, so a
+# repeated key is replaced whole rather than combined with what it had. Keys that
+# name a field the other flags set — containerImage here — override them.
+rossoctl agents import from-image --name orders --containerImage ghcr.io/x/y:latest \
+    --additionalParameterJSON ./base.json \
+    --additionalParameterJSON '{"containerImage":"ghcr.io/x/y:pinned"}'
 
 # `agents --namespace` overrides the context's namespace for any agents subcommand
 rossoctl agents --namespace team2 get orders    # -> GET /agents/team2/orders
@@ -231,6 +243,10 @@ rossoctl tools import from-image --name weather-mcp --containerImage ghcr.io/x/y
     --envVar LOG_LEVEL=debug --envVar 'TAGS=a,b,c'
 # --ports sets service ports as name:port:targetPort[:protocol] (default http:9090:9090:TCP); a bare "port" = http:port:port:TCP
 rossoctl tools import from-image --name weather-mcp --containerImage ghcr.io/x/y:latest --ports grpc:9000:9001:TCP,8080
+# --additionalParameterJSON works as it does for agents: a JSON dict or a file
+# containing one, repeatable, merged by top-level key, and winning over the flags above
+rossoctl tools import from-image --name weather-mcp --containerImage ghcr.io/x/y:latest \
+    --additionalParameterJSON '{"serviceAccount":"mcp-sa"}' --additionalParameterJSON ./extra.json
 
 # A tool built from source reports "Building" until its build finishes, which is
 # often longer than the 60s default, so allow for it. A failed build reports

cmd/additionalparams.go

@@ -0,0 +1,141 @@
+package cmd
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"maps"
+	"os"
+	"strings"
+)
+
+// additionalParameterFlagName is the flag whose values loadAdditionalParameters
+// interprets. Named once so the errors below and the flag registrations in
+// agents_import.go and tools_import.go cannot drift apart.
+const additionalParameterFlagName = "additionalParameterJSON"
+
+// loadAdditionalParameters interprets repeated --additionalParameterJSON values
+// and merges them into the single dict that is overlaid onto a create request.
+//
+// Each value is either inline JSON or the name of a file containing JSON; see
+// readAdditionalParameter. Every value must decode to a JSON object, because the
+// result is merged into the request body by name — an array or a scalar has no
+// names to merge.
+//
+// Merging is shallow and last-wins: a key present in two values takes the later
+// value whole, rather than the two objects underneath being combined. So
+// '{"resources":{"limits":{"cpu":"1"}}}' followed by
+// '{"resources":{"requests":{"cpu":"1"}}}' sends only the requests, not both.
+// The rule is the one mergeEnvVars applies to a repeated variable name, and it is
+// the rule that lets a caller *replace* a nested structure the CLI or an earlier
+// file already set; a deep merge could only ever add to it.
+//
+// Returns nil, not an empty map, when no values are given: the marshaler treats
+// an empty overlay as no overlay, and nil says the same thing without allocating.
+func loadAdditionalParameters(values []string) (map[string]any, error) {
+	var merged map[string]any
+	for _, v := range values {
+		obj, err := readAdditionalParameter(v)
+		if err != nil {
+			return nil, err
+		}
+		if merged == nil {
+			merged = make(map[string]any, len(obj))
+		}
+		maps.Copy(merged, obj)
+	}
+	return merged, nil
+}
+
+// readAdditionalParameter interprets one --additionalParameterJSON value as
+// either inline JSON or a filename, and decodes it as a JSON object.
+//
+// The two are told apart by the first non-whitespace byte: '{' means the value
+// is itself the document, anything else means it names a file. Sniffing the
+// leading character rather than probing the filesystem keeps the meaning of a
+// value a property of what the user typed, so a command does not change behavior
+// because a file named '{"a":1}' happens to exist, and a misspelled filename
+// reports that the file is missing instead of being parsed as JSON and reported
+// as a syntax error.
+//
+// A value that is neither — a bare word, an array, a quoted string — is a
+// missing file, and is reported as one.
+func readAdditionalParameter(value string) (map[string]any, error) {
+	trimmed := strings.TrimSpace(value)
+	if trimmed == "" {
+		return nil, fmt.Errorf("--%s must not be empty", additionalParameterFlagName)
+	}
+
+	data := []byte(trimmed)
+	source := "inline JSON"
+	if !strings.HasPrefix(trimmed, "{") {
+		contents, err := os.ReadFile(trimmed)
+		if err != nil {
+			return nil, fmt.Errorf("--%s %q: %w", additionalParameterFlagName, value, err)
+		}
+		data = contents
+		source = fmt.Sprintf("file %q", trimmed)
+	}
+
+	obj, err := decodeJSONObject(data)
+	if err != nil {
+		return nil, fmt.Errorf("--%s: %s: %w", additionalParameterFlagName, source, err)
+	}
+	return obj, nil
+}
+
+// decodeJSONObject decodes data as a JSON object, rejecting a duplicate key and
+// anything that is not an object.
+//
+// json.Unmarshal into a map would accept both: a duplicate name silently keeps
+// the last value, and there would be no way to tell an object from any other
+// value until the type assertion. A token-driven decode is used instead so
+// '{"a":1,"a":2}' is refused rather than quietly becoming one of the two — a file
+// naming the same parameter twice is a mistake worth reporting, and unlike a
+// repeated *flag* (whose last-wins is a deliberate layering rule) nothing about
+// one document expresses an ordering intent.
+//
+// UseNumber keeps numeric literals as json.Number, so a value passes through to
+// the request body as written instead of being rendered from a float64 — 1 stays
+// 1 rather than becoming 1e+00, and a large int64 keeps its digits.
+func decodeJSONObject(data []byte) (map[string]any, error) {
+	dec := json.NewDecoder(bytes.NewReader(data))
+	dec.UseNumber()
+
+	tok, err := dec.Token()
+	if err != nil {
+		return nil, fmt.Errorf("invalid JSON: %w", err)
+	}
+	if delim, ok := tok.(json.Delim); !ok || delim != '{' {
+		return nil, fmt.Errorf("expected a JSON object")
+	}
+
+	out := make(map[string]any)
+	for dec.More() {
+		keyTok, err := dec.Token()
+		if err != nil {
+			return nil, fmt.Errorf("invalid JSON: %w", err)
+		}
+		key, ok := keyTok.(string)
+		if !ok {
+			return nil, fmt.Errorf("invalid JSON: expected an object key")
+		}
+		if _, dup := out[key]; dup {
+			return nil, fmt.Errorf("duplicate key %q", key)
+		}
+		var val any
+		if err := dec.Decode(&val); err != nil {
+			return nil, fmt.Errorf("invalid JSON for key %q: %w", key, err)
+		}
+		out[key] = val
+	}
+	// Consumes the closing '}' and confirms nothing follows it, so trailing
+	// content after a complete object is an error rather than being ignored.
+	if _, err := dec.Token(); err != nil {
+		return nil, fmt.Errorf("invalid JSON: %w", err)
+	}
+	if dec.More() {
+		return nil, fmt.Errorf("unexpected trailing content after the JSON object")
+	}
+	return out, nil
+}

cmd/additionalparams_test.go

@@ -0,0 +1,211 @@
+package cmd
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// renderJSON encodes a value canonically so a test can compare a decoded dict
+// against an expected shape in one string comparison. Go's encoder sorts map
+// keys, which makes the result stable.
+func renderJSON(t *testing.T, v any) string {
+	t.Helper()
+	data, err := json.Marshal(v)
+	if err != nil {
+		t.Fatalf("marshaling %#v: %v", v, err)
+	}
+	return string(data)
+}
+
+func TestLoadAdditionalParametersNone(t *testing.T) {
+	// Nil rather than an empty map: the marshaler treats an empty overlay as no
+	// overlay, and nil is how "no values given" is said without allocating.
+	got, err := loadAdditionalParameters(nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got != nil {
+		t.Errorf("got %#v, want nil for no flag values", got)
+	}
+}
+
+func TestLoadAdditionalParametersInline(t *testing.T) {
+	got, err := loadAdditionalParameters([]string{`{"a":1,"b":"two"}`})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if s := renderJSON(t, got); s != `{"a":1,"b":"two"}` {
+		t.Errorf("got %s, want the dict as written", s)
+	}
+}
+
+// TestLoadAdditionalParametersNumbersAreVerbatim verifies a numeric literal
+// survives unchanged.
+//
+// The substance is the large integer: decoding into an `any` without UseNumber
+// yields a float64, which cannot hold this value exactly and re-renders in
+// exponent form. A caller passing an ID or a byte count would see it altered.
+func TestLoadAdditionalParametersNumbersAreVerbatim(t *testing.T) {
+	got, err := loadAdditionalParameters([]string{`{"big":123456789012345678,"one":1,"frac":1.50}`})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if s := renderJSON(t, got); s != `{"big":123456789012345678,"frac":1.50,"one":1}` {
+		t.Errorf("got %s; numeric literals must pass through as written", s)
+	}
+}
+
+func TestLoadAdditionalParametersFromFile(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "extra.json")
+	if err := os.WriteFile(path, []byte(`{"resources":{"limits":{"cpu":"2"}}}`), 0o600); err != nil {
+		t.Fatalf("writing fixture: %v", err)
+	}
+
+	got, err := loadAdditionalParameters([]string{path})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if s := renderJSON(t, got); s != `{"resources":{"limits":{"cpu":"2"}}}` {
+		t.Errorf("got %s, want the file's contents", s)
+	}
+}
+
+// TestLoadAdditionalParametersMergesLastWins verifies repeated values merge, and
+// that a shared key takes the later value whole.
+//
+// The "nested" key is the assertion that matters: a deep merge would produce
+// {"a":1,"b":2} there. Shallow replacement is what lets a caller *replace* a
+// structure an earlier file set, rather than only add to it.
+func TestLoadAdditionalParametersMergesLastWins(t *testing.T) {
+	got, err := loadAdditionalParameters([]string{
+		`{"first":1,"shared":"early","nested":{"a":1}}`,
+		`{"second":2,"shared":"late","nested":{"b":2}}`,
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	const want = `{"first":1,"nested":{"b":2},"second":2,"shared":"late"}`
+	if s := renderJSON(t, got); s != want {
+		t.Errorf("got %s, want %s", s, want)
+	}
+}
+
+// TestLoadAdditionalParametersMixesInlineAndFile verifies the two forms merge
+// with each other under the same last-wins rule.
+func TestLoadAdditionalParametersMixesInlineAndFile(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "base.json")
+	if err := os.WriteFile(path, []byte(`{"fromFile":true,"shared":"file"}`), 0o600); err != nil {
+		t.Fatalf("writing fixture: %v", err)
+	}
+
+	got, err := loadAdditionalParameters([]string{path, `{"shared":"inline"}`})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if s := renderJSON(t, got); s != `{"fromFile":true,"shared":"inline"}` {
+		t.Errorf("got %s, want the inline value to win over the file's", s)
+	}
+}
+
+// TestLoadAdditionalParametersLeadingBraceIsInline verifies classification is by
+// the leading character and not by what exists on disk.
+//
+// A file whose *name* is a JSON document is created here deliberately. Probing
+// the filesystem first would read it and the assertion below would see
+// "fromFile"; sniffing '{' means the value the user typed is the document,
+// whatever the directory happens to contain.
+func TestLoadAdditionalParametersLeadingBraceIsInline(t *testing.T) {
+	dir := t.TempDir()
+	t.Chdir(dir)
+
+	const name = `{"a":1}`
+	if err := os.WriteFile(filepath.Join(dir, name), []byte(`{"fromFile":true}`), 0o600); err != nil {
+		t.Skipf("this platform cannot create a file named %q: %v", name, err)
+	}
+
+	got, err := loadAdditionalParameters([]string{name})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if s := renderJSON(t, got); s != `{"a":1}` {
+		t.Errorf("got %s; a value starting with '{' is inline JSON, not a filename", s)
+	}
+}
+
+// TestLoadAdditionalParametersLeadingWhitespace verifies an inline dict is
+// recognized despite surrounding whitespace, which a shell heredoc or a
+// copy-paste readily introduces.
+func TestLoadAdditionalParametersLeadingWhitespace(t *testing.T) {
+	got, err := loadAdditionalParameters([]string{"  \n\t{\"a\":1}\n  "})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if s := renderJSON(t, got); s != `{"a":1}` {
+		t.Errorf("got %s, want the surrounding whitespace ignored", s)
+	}
+}
+
+func TestLoadAdditionalParametersErrors(t *testing.T) {
+	missing := filepath.Join(t.TempDir(), "nope.json")
+	badJSONFile := filepath.Join(t.TempDir(), "bad.json")
+	if err := os.WriteFile(badJSONFile, []byte(`{"a":`), 0o600); err != nil {
+		t.Fatalf("writing fixture: %v", err)
+	}
+	arrayFile := filepath.Join(t.TempDir(), "array.json")
+	if err := os.WriteFile(arrayFile, []byte(`[1,2]`), 0o600); err != nil {
+		t.Fatalf("writing fixture: %v", err)
+	}
+
+	for _, tc := range []struct {
+		name  string
+		value string
+		want  string // substring the error must contain
+	}{
+		// A bare word is a filename, so the error names the missing file rather
+		// than reporting a JSON syntax problem in text the user meant as a path.
+		{"missing file", missing, "nope.json"},
+		{"bare word", "notjson", "notjson"},
+		{"empty value", "", "must not be empty"},
+		{"whitespace only", "   ", "must not be empty"},
+		{"truncated inline", `{"a":`, "invalid JSON"},
+		{"truncated file", badJSONFile, "invalid JSON"},
+		// An array has no names to merge by, so it is refused rather than dropped.
+		{"array file", arrayFile, "expected a JSON object"},
+		// A duplicate key in one document is a mistake, not a layering intent.
+		{"duplicate key", `{"a":1,"a":2}`, `duplicate key "a"`},
+		// Trailing content means the value was not the single dict it appeared to
+		// be; ignoring it would silently drop the second half.
+		{"trailing content", `{"a":1} {"b":2}`, "trailing"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := loadAdditionalParameters([]string{tc.value})
+			if err == nil {
+				t.Fatalf("expected an error for %q", tc.value)
+			}
+			if !strings.Contains(err.Error(), tc.want) {
+				t.Errorf("error = %v, want it to mention %q", err, tc.want)
+			}
+			// Every error identifies the flag, so a user with several flags on the
+			// command line knows which one to fix.
+			if !strings.Contains(err.Error(), additionalParameterFlagName) {
+				t.Errorf("error = %v, want it to name --%s", err, additionalParameterFlagName)
+			}
+		})
+	}
+}
+
+// TestLoadAdditionalParametersErrorStopsAtFirstBadValue verifies a later bad
+// value fails the whole call rather than the good values being used.
+func TestLoadAdditionalParametersErrorStopsAtFirstBadValue(t *testing.T) {
+	got, err := loadAdditionalParameters([]string{`{"a":1}`, `{"b":`})
+	if err == nil {
+		t.Fatal("expected an error for the second value")
+	}
+	if got != nil {
+		t.Errorf("got %#v, want nil; a partial merge must not be returned", got)
+	}
+}