diff --git a/hack/lib/helm-validation.js b/hack/lib/helm-validation.js index 3d446fc..83c25e6 100644 --- a/hack/lib/helm-validation.js +++ b/hack/lib/helm-validation.js @@ -121,6 +121,10 @@ function parseVersions(file) { // (catalogLayer in swiss/internal/compose): its values, with the fields the // variant spells first-class projected on top, so they win over the same keys // in values. image.digest is not projected. +// +// Each declared input is set to its default, else its example, standing in for +// the answer swiss puts in the derived layer at deploy, so the chart renders +// every path swiss fills. values never sets one; validate:schema refuses that. function catalogValues(variant) { const values = structuredClone(variant.values ?? {}); const set = (keys, value) => { @@ -136,6 +140,7 @@ function catalogValues(variant) { set(["image", "repository"], variant.image.repository); set(["image", "tag"], String(variant.image.tag ?? "")); } + for (const input of variant.inputs ?? []) set(input.key.split("."), "default" in input ? input.default : input.example); return values; } diff --git a/hack/lib/helm-validation.test.js b/hack/lib/helm-validation.test.js index ff2abd2..ca08bd3 100644 --- a/hack/lib/helm-validation.test.js +++ b/hack/lib/helm-validation.test.js @@ -196,3 +196,20 @@ test("variant values get swiss's projection of servedName, requires.gpus and the const [v2] = parseVersions(path.join(dir, "v2.yaml")); assert.deepEqual(catalogValues(v2), { model: { name: "served", gpus: "2" } }); }); + +test("a declared input renders with its default, else its example", () => { + const variant = { + servedName: "m", + requires: { gpus: 8 }, + inputs: [ + { key: "storageKVOffload.mooncake.master", title: "t", example: "mooncake-master.example:50051" }, + { key: "storageKVOffload.mooncake.device", title: "t", example: "mlx5_0", default: "mlx5_3" }, + ], + values: { storageKVOffload: { enabled: true, mooncake: { protocol: "rdma" } } }, + }; + assert.deepEqual(catalogValues(variant).storageKVOffload, { + enabled: true, + mooncake: { protocol: "rdma", master: "mooncake-master.example:50051", device: "mlx5_3" }, + }); + assert.equal(variant.values.storageKVOffload.mooncake.master, undefined); +}); diff --git a/hack/lib/inputs.js b/hack/lib/inputs.js new file mode 100644 index 0000000..b3bc384 --- /dev/null +++ b/hack/lib/inputs.js @@ -0,0 +1,58 @@ +"use strict"; +// A variant's inputs, past what the schema can see. A key is declared once, and +// values leaves it alone: swiss puts the answer, the site's or else the input's +// default, in the derived layer. The example and the default are valid answers. +// swiss repeats these checks when it loads a variant (internal/catalog). + +// Problems with one variant's inputs, each line prefixed with at. +function inputProblems(variant, at = "") { + const inputs = Array.isArray(variant?.inputs) ? variant.inputs : []; + const lines = []; + const declared = new Set(); + for (const [i, input] of inputs.entries()) { + if (typeof input?.key !== "string") continue; // the schema reports it + const where = `${at}inputs[${i}] ${input.key}: `; + if (declared.has(input.key)) lines.push(`${where}declared twice`); + declared.add(input.key); + const set = valueSetAt(variant.values, input.key.split(".")); + if (set) lines.push(`${where}values.${set} sets it; give the value as the input's default`); + const type = input.type ?? "string"; + let pattern; + if (input.pattern !== undefined && type !== "string") { + lines.push(`${where}pattern is only for a string input, and this one is ${type}`); + } else if (input.pattern !== undefined) { + try { + pattern = new RegExp(`^(?:${input.pattern})$`, "u"); + } catch (err) { + lines.push(`${where}pattern does not compile: ${err.message}`); + } + } + const answerProblem = (v) => { + const ok = type === "integer" ? Number.isInteger(v) : typeof v === type; + if (!ok) return `${JSON.stringify(v)} is not ${type === "integer" ? "an" : "a"} ${type}`; + if (pattern && !pattern.test(v)) return `${JSON.stringify(v)} does not match the pattern`; + return ""; + }; + for (const field of ["example", "default"]) { + if (!(field in input)) continue; + const problem = answerProblem(input[field]); + if (problem) lines.push(`${where}${field} ${problem}`); + } + } + return lines; +} + +// The prefix of keys at which values already says something about a dotted +// key: the value itself, a value under it, or a non-map on the way to it. +// Empty when values leaves the key alone. +function valueSetAt(values, keys) { + let node = values; + for (const [i, key] of keys.entries()) { + if (node === null || typeof node !== "object" || Array.isArray(node)) return i > 0 ? keys.slice(0, i).join(".") : ""; + if (!Object.hasOwn(node, key)) return ""; + node = node[key]; + } + return keys.join("."); +} + +module.exports = { inputProblems }; diff --git a/hack/lib/inputs.test.js b/hack/lib/inputs.test.js new file mode 100644 index 0000000..752d1a0 --- /dev/null +++ b/hack/lib/inputs.test.js @@ -0,0 +1,62 @@ +"use strict"; +const test = require("node:test"); +const assert = require("node:assert/strict"); +const { inputProblems } = require("./inputs"); + +const master = { + key: "storageKVOffload.mooncake.master", + title: "Mooncake master", + pattern: "[^\\s:/]+:[0-9]+", + example: "mooncake-master.example:50051", +}; +const variant = (inputs, values = {}) => ({ requires: { gpus: 8 }, inputs, values }); + +test("a declared input that values leaves alone, with a valid example and default, passes", () => { + const values = { storageKVOffload: { enabled: true, mooncake: { protocol: "rdma" } } }; + assert.deepEqual(inputProblems(variant([master], values)), []); + assert.deepEqual(inputProblems(variant([{ ...master, default: "mc:50051" }], values)), []); + assert.deepEqual(inputProblems({ requires: { gpus: 1 } }), []); +}); + +test("values may not set an input: at its key, under it, or with a non-map on the way", () => { + for (const [values, at] of [ + [{ storageKVOffload: { mooncake: { master: "mc:50051" } } }, "storageKVOffload.mooncake.master"], + [{ storageKVOffload: { mooncake: { master: { host: "h" } } } }, "storageKVOffload.mooncake.master"], + [{ storageKVOffload: { mooncake: "off" } }, "storageKVOffload.mooncake"], + ]) { + assert.deepEqual(inputProblems(variant([master], values), "v: "), [ + `v: inputs[0] storageKVOffload.mooncake.master: values.${at} sets it; give the value as the input's default`, + ]); + } +}); + +test("the default is an answer: of the input's type, matching its pattern", () => { + const one = (input) => inputProblems(variant([{ key: "a.b", title: "t", ...input }])); + assert.match(one({ example: "h:1", default: "no-port", pattern: master.pattern })[0], /default "no-port" does not match the pattern$/); + assert.match(one({ example: "h:1", default: "", pattern: master.pattern })[0], /default "" does not match the pattern$/); + assert.match(one({ example: 1, default: "8", type: "integer" })[0], /default "8" is not an integer$/); + assert.deepEqual(one({ example: false, default: true, type: "boolean" }), []); +}); + +test("a key is declared once", () => { + assert.deepEqual(inputProblems(variant([master, master])), ["inputs[1] storageKVOffload.mooncake.master: declared twice"]); +}); + +test("the example is an answer: of the input's type, matching its pattern", () => { + const one = (input) => inputProblems(variant([{ key: "a.b", title: "t", ...input }])); + assert.deepEqual(one({ example: "mooncake-master:50051", pattern: master.pattern }), []); + assert.match(one({ example: "no-port", pattern: master.pattern })[0], /example "no-port" does not match the pattern$/); + assert.match(one({ example: "8", type: "integer" })[0], /example "8" is not an integer$/); + assert.match(one({ example: 1.5, type: "integer" })[0], /is not an integer$/); + assert.deepEqual(one({ example: 8, type: "integer" }), []); + assert.deepEqual(one({ example: false, type: "boolean" }), []); + assert.match(one({ example: 8 })[0], /example 8 is not a string$/); +}); + +test("a pattern compiles, and only a string input has one", () => { + assert.match(inputProblems(variant([{ ...master, pattern: "(" }]))[0], /pattern does not compile/); + assert.match( + inputProblems(variant([{ key: "a.b", title: "t", type: "integer", pattern: "[0-9]+", example: 1 }]))[0], + /pattern is only for a string input, and this one is integer$/, + ); +}); diff --git a/hack/validate-schema.js b/hack/validate-schema.js index baa6c77..857e5cb 100644 --- a/hack/validate-schema.js +++ b/hack/validate-schema.js @@ -27,6 +27,7 @@ const { checkIndexV2, } = require("./lib/catalog"); const { reportProblem } = require("./lib/report"); +const { inputProblems } = require("./lib/inputs"); const { paint, marks, plural } = require("./lib/style"); const { @@ -225,6 +226,7 @@ function variantProblems(v, at) { if (v.requires[key] !== undefined) lines.push(`${at}requires.${key} is set, but requires.gpus 0 is CPU only`); } } + lines.push(...inputProblems(v, at)); return lines; } diff --git a/schema/variant.schema.json b/schema/variant.schema.json index 44d9f53..dd9d5f7 100644 --- a/schema/variant.schema.json +++ b/schema/variant.schema.json @@ -48,6 +48,9 @@ "requires": { "$ref": "version.schema.json#/$defs/variant/properties/requires" }, + "inputs": { + "$ref": "version.schema.json#/$defs/variant/properties/inputs" + }, "values": { "$ref": "version.schema.json#/$defs/variant/properties/values" }, diff --git a/schema/version.schema.json b/schema/version.schema.json index ee2bb9c..92980b2 100644 --- a/schema/version.schema.json +++ b/schema/version.schema.json @@ -168,6 +168,52 @@ } ] }, + "inputs": { + "description": "Chart values every deploy must answer, such as the address of a service the site runs. Unlike requires, which swiss interprets and checks against the cluster, an input names one chart value by its key, and swiss only relays its answer: swiss puts the answer in the derived layer -- the site profile's inputs, else the input's default -- and the deploy's overrides go over it. swiss refuses to compose a plan that leaves one without a valid answer. values never sets an input's key. model.localPath is never one: swiss answers it from the site's model.pathTemplate.", + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "additionalProperties": false, + "required": ["key", "title", "example"], + "properties": { + "key": { + "description": "The chart value's dotted key, as helm --set takes it: storageKVOffload.mooncake.master. Unique within the variant.", + "type": "string", + "pattern": "^[A-Za-z_][A-Za-z0-9_-]*(\\.[A-Za-z_][A-Za-z0-9_-]*)*$", + "not": { "enum": ["model.localPath"] } + }, + "title": { + "description": "What the deploy form labels the field with.", + "type": "string", + "minLength": 1 + }, + "description": { + "description": "What the value is, for the deploy form.", + "type": "string", + "minLength": 1 + }, + "type": { + "description": "What an answer is.", + "enum": ["string", "integer", "boolean"], + "default": "string" + }, + "pattern": { + "description": "A regular expression a string answer matches, whole. Keep to the syntax Go and JavaScript read alike: swiss checks it, and so does the deploy form.", + "type": "string", + "minLength": 1 + }, + "example": { + "description": "A value of the right shape, never a real one: the deploy form's placeholder, and what CI renders the chart with when there is no default. Never deployed. Must be of the input's type and match its pattern.", + "type": ["string", "integer", "boolean"] + }, + "default": { + "description": "The answer when the site profile gives none, such as the value of the cluster the variant was tuned on. Must be of the input's type and match its pattern.", + "type": ["string", "integer", "boolean"] + } + } + } + }, "values": { "description": "Chart values this variant sets. Any object", "type": "object"