Add workflow label UI support - #1227
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
11b7388 to
5d48836
Compare
df02c3e to
a7b511b
Compare
a7b511b to
c03960d
Compare
c03960d to
79fa3c7
Compare
* feat(config): add workflow label policy model * Document label policy example in chart values Call out the YAML gotcha that unquoted off parses as boolean false. * Apply label policy config review fixes Match the file's plain-default style, note why the policy cap reuses the per-workflow label cap, trim loader tests to plumbing coverage, and drop a framework-behavior assertion. * Regenerate API artifacts for literal config defaults * Clarify label policy docs and fix reserved-key test residue Document the enforcement modes and empty-allow-list behavior where admins read them (model docstrings, which are the OpenAPI descriptions), align test naming with sibling files, replace reserved-key test cases orphaned by the deny-list removal with syntactically invalid keys, and assert the patched policy lands in config history. * Drop DB-config-mode-only label tests ConfigMap mode is the supported config path, so drop the legacy config PATCH replace test (config history) and the legacy Postgres serialization round-trip test. The label policy models and ConfigMap loader coverage remain. * Drop regenerated OpenAPI artifacts from the config PR This PR's server change (labels_config on WorkflowConfig) is real, but the regenerated OpenAPI contract and its derived TypeScript client/mocks are not carried here: - generated.ts / generated-mocks.ts: regenerating drags in ~1600 lines of pre-existing normalization drift (main's committed client is stale vs the repo's orval), and no UI code consumes the config policy types. - openapi.json: regenerating additionally renames config_history schemas (ConfigHistoryType -> OperableConfigHistoryType) unrelated to labels -- confirmed by regenerating on clean main, which emits the same rename. All OpenAPI-derived artifacts regenerate once, together, in the UI slice (#1227). This PR stays a pure backend/config change. * Use a generic 'project' key in label policy tests Replace the NVIDIA-internal 'PPP' example key with the generic 'project' in the label-policy config tests. 'team' would collide with the existing distinct-policy example, so 'project' is used.
79fa3c7 to
018e279
Compare
018e279 to
481ec10
Compare
481ec10 to
5d6324e
Compare
5d6324e to
0cfa33a
Compare
24e72d5 to
9e08b93
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## jiaenr/osmo-6501-b8-label-metrics #1227 +/- ##
=====================================================================
+ Coverage 66.91% 67.00% +0.09%
=====================================================================
Files 202 203 +1
Lines 26076 26109 +33
Branches 3942 3952 +10
=====================================================================
+ Hits 17448 17494 +46
+ Misses 7887 7854 -33
- Partials 741 761 +20
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
40b9e81 to
e290029
Compare
* Enforce workflow label policy at submission
Evaluate the curated label policy inside validate_workflow_spec for
submit, resubmit-by-id, restart, and validation_only alike. Warn-mode
violations ride the submit response and are recomputed on the
get-workflow response for every status; enforce-mode violations reject
with a usage error and leave no workflow row. Repeatable --label
key=value query parameters override the YAML on submit and
resubmit-by-id, which also carries the stored labels forward. Emit the
osmo_label_validation_total{key,outcome} counter at the gate.
* Apply admission review simplifications
Table-drive the policy messages, type the outcome as a Literal, emit
the validation counter once per evaluation, collapse the
construct_workflow_dict labels guard, add the missing return
annotation, and default the label query parameter to an empty list
like env_vars. Consolidate repeated rendered-spec construction in the
policy tests.
* Drop the workflow-label reserved-key deny-list
Any syntactically valid Kubernetes label key is accepted, including
system domains such as app.kubernetes.io/name, which the deny-list
wrongly blocked. Protection against user labels actuating scheduling or
platform machinery comes from merge order at stamping time: system
labels are applied after the user map on every path.
* Polish admission gate docs and test organization
Document the gate contract on validate_workflow_label_policy, type the
metric outcome vocabulary, note the labels rewrite guard rationale,
add test class docstrings, and regenerate API artifacts for the policy
model descriptions.
* Drop regenerated OpenAPI artifacts and use a generic label key
Match the merged config PR (#1220): revert openapi.json + generated
client/mocks to main (the whole OpenAPI contract regenerates once in the
UI slice #1227), and replace the NVIDIA-internal 'PPP' example key with
the generic 'project' in the admission tests.
* Add optional per-policy assert_message appended to label messages
An admin can set assert_message on a label policy to append one line of
guidance (for example where to look up valid values) to that key's warn
and enforce messages, without disclosing the allow_list. Named to match
RESOURCE_VALIDATION's assert_message field for config consistency. Empty
by default, so OSS defaults and messages stay deployment-neutral; the
NVIDIA AI Hub pointer lives only in internal ConfigMap values. Validated
as a single line of at most 256 characters.
e290029 to
e421961
Compare
* Add workflow label list filters Compile repeatable label=key=value selectors into fixed SQL predicates: exact values use GIN-indexable JSONB containment, glob patterns use LIKE with escaped literals, alternation branches OR together, and a bare * collapses to key existence. no_label=key is NULL-inclusive so pre-feature rows count as missing. List responses now carry the labels map. Selector parsing happens before any SQL, so malformed filters reject as usage errors and user input never reaches the query text unbound. * Bind label filter keys as SQL parameters Bind label keys with %s like every other get_workflows filter instead of hand-rendering SQL literals (psycopg2 interpolates client-side, so the planner still sees literal keys and the GIN index stays eligible), collapse the glob-to-LIKE translation into chained replaces, drop the off-topic warning-column test, and trim the planner test to the shipped GIN index asserting on plan shape rather than index names, which the isolation fixture regenerates. * Polish list filter naming and document no_label Name the parsed selector and key lists for what they hold, document the LIKE escape contract and the no_label parameter semantics, point the fixture docstring at its mirror, and regenerate API artifacts. * Drop regenerated OpenAPI artifacts and use a generic label key Match merged #1220: revert openapi.json + generated client/mocks to main (regenerated once in the UI slice #1227) and replace the NVIDIA-internal 'PPP' example key with the generic 'project' in the list-filter tests.
e421961 to
0de02dd
Compare
0de02dd to
81f16ec
Compare
Workflow list gains a Labels column and label/no-label filter chips wired to the list API; the detail page shows the stored labels and the recomputed warn-mode policy warnings; the resubmit panel carries labels forward and lets users edit them; the new-workflow submit page reads labels from the spec editor with an optional override editor. Adds a shared workflow-labels formatting module, adapter shims for the new response fields, and unit plus Playwright coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Compute the detail-page label entries once per render, replace a warnings useMemo with a stable module constant, type ResubmitParams priority to drop a cast, and remove two shim tests that exercised generated URL builders already covered end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move the module constant below imports, refresh stale JSDoc, document the shared label helpers and their Python cap mirror, unify warning toast order and list keys across the two submit surfaces, align copy with the app's terminology, and trim a duplicated e2e assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The UI slice owns the single regeneration of the OpenAPI-derived artifacts (openapi.json + generated client + mocks) for the whole label stack — the backend slices (#1220 and the B4-B8 PRs) deliberately carry none, so this avoids spreading generator drift across the stack. Regenerated from the full server surface (config policy, admission, list filters). Also replace the NVIDIA-internal 'PPP' example key with the generic 'project' in the UI tests and e2e journeys. Verified: tsc, eslint, vitest (1017), and production build all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run Prettier on the files touched by the PPP->project rename so format:check (part of ui-build's validate:coverage) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The label editor's other controls use the 'workflow label' phrasing
('Workflow label key/value N', 'Remove workflow label N'), but the add
button's accessible name was just 'Add label' — inconsistent, and the
resubmit/submit e2e journeys query 'Add workflow label', so the resubmit
test could never find the button. Align the button's aria-label and
visible text with the convention.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
81f16ec to
38556a5
Compare
Regenerate openapi.json + generated client/mocks from the current server (B5 admission incl. the help_text->assert_message rename, B6 filters, all now merged). Picks up an assert_message schema field the previously-frozen contract was missing. Verified: type-check, lint, format, vitest (1027), build all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Issue - None
[OSMO-6501] Track B9 of the workflow-labels PR split — the final feature PR of the stack.
label/no_labelfilter chips (selector syntax with*and(a|b)passes through to the list API).workflow-labelsformatting module; API types consumed through the adapter layer per the repo convention.Stacked on #1226; functionally depends on #1223 and #1224. Extracted from prototype #1194.
Testing
Verified against
main's Next.js 16.2.11 (#1230):pnpm vitest run— 46 files, 1017 tests pass (includes new actions, columns, search-fields, shim, and label-formatting tests)pnpm exec tsc --noEmit,pnpm exec eslint src --max-warnings 0pnpm buildpassesChecklist
🤖 Generated with Claude Code