From 6a4e93eaa67b170d90acc3d8968acde54ea4c760 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Tue, 11 Aug 2026 12:14:52 +0200 Subject: [PATCH 1/4] feat(sdk): Add microversion-aware schema-variant selector Generated code emits one request struct per microversion break (e.g. create_20, create_233, ...); a resource's BODY_SCHEMA is therefore not single-valued for these operations. select_schema() picks the variant whose [min_version, max_version] range covers a given negotiated version, reusing the same bound-checking negotiate_microversion() already does (factored out into version_range_compatible() so the two don't duplicate the floor/ceiling rules). Degrades to always selecting the one schema for single-variant operations (the common case today). Signed-off-by: Artem Goncharov --- sdk/core/src/api.rs | 4 + sdk/core/src/api/rest_endpoint.rs | 52 +++++++--- sdk/core/src/api/schema_variant.rs | 146 +++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 15 deletions(-) create mode 100644 sdk/core/src/api/schema_variant.rs diff --git a/sdk/core/src/api.rs b/sdk/core/src/api.rs index 300146a93..4644673fa 100644 --- a/sdk/core/src/api.rs +++ b/sdk/core/src/api.rs @@ -456,6 +456,7 @@ mod params; pub mod query; mod raw; pub mod rest_endpoint; +pub mod schema_variant; mod wait; pub use self::error::ApiError; @@ -467,6 +468,9 @@ pub use self::client::RestClient; pub use self::rest_endpoint::RestEndpoint; pub use self::rest_endpoint::check_response_error; +pub use self::schema_variant::SchemaVariant; +pub use self::schema_variant::select_schema; + #[cfg(feature = "async")] pub use self::client::AsyncClient; #[cfg(feature = "sync")] diff --git a/sdk/core/src/api/rest_endpoint.rs b/sdk/core/src/api/rest_endpoint.rs index 4f191c2f4..812f537c1 100644 --- a/sdk/core/src/api/rest_endpoint.rs +++ b/sdk/core/src/api/rest_endpoint.rs @@ -117,6 +117,35 @@ pub trait RestEndpoint { } } +/// Whether a `[min, max]` range (endpoint or schema-variant bounds) overlaps +/// a `[cloud_min, cloud_max]` range, using the same two rules +/// [`negotiate_microversion`] and [`crate::api::schema_variant::select_schema`] +/// both need: +/// +/// - the range's floor must not be newer than anything the other side will +/// ever support (`min <= other_max`, when `other_max` is known); +/// - the range's ceiling, when bounded, must not be older than the other +/// side's floor (`max >= other_min`, when both are known). +pub(crate) fn version_range_compatible( + min: ApiVersion, + max: Option, + other_min: Option, + other_max: Option, +) -> bool { + if let Some(other_max) = other_max + && min > other_max + { + return false; + } + if let Some(max) = max + && let Some(other_min) = other_min + && max < other_min + { + return false; + } + true +} + /// Compute the microversion to send for this endpoint against a cloud's /// discovered range, without touching any request. /// @@ -168,21 +197,14 @@ where ) }; - // The endpoint's floor is newer than anything this cloud will ever - // support. - if let Some(cmax) = cloud_max - && ep_min > cmax - { - return Err(incompatible(ep_min)); - } - - // The variant's ceiling is older than what this cloud's floor requires - // — it's been superseded by a newer variant on this cloud. - if let Some(emax) = ep_max - && let Some(cmin) = cloud_min - && emax < cmin - { - return Err(incompatible(emax)); + if !version_range_compatible(ep_min, ep_max, cloud_min, cloud_max) { + // Attribute the failure to whichever bound is actually the + // culprit, matching the pre-existing error-reporting order. + let culprit = match cloud_max { + Some(cmax) if ep_min > cmax => ep_min, + _ => ep_max.unwrap_or(ep_min), + }; + return Err(incompatible(culprit)); } // Send the highest version both sides agree is valid: raise the diff --git a/sdk/core/src/api/schema_variant.rs b/sdk/core/src/api/schema_variant.rs new file mode 100644 index 000000000..6223cedb4 --- /dev/null +++ b/sdk/core/src/api/schema_variant.rs @@ -0,0 +1,146 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//! Selecting the right `BODY_SCHEMA` among a microversioned operation's +//! vendored variants. +//! +//! Generated code emits one request struct per microversion break (e.g. +//! `create_20`, `create_233`, ...), each carrying its own `BODY_SCHEMA` +//! (see [`crate::api::rest_endpoint::RestEndpoint::min_version`] / +//! `max_version` for the equivalent struct-selection bounds). Consumers that +//! need a schema *before* committing to a specific variant struct -- e.g. to +//! render an editor template or validate a YAML buffer against the right +//! shape -- use [`select_schema`] against the cloud's discovered version +//! instead. + +use crate::api::rest_endpoint::version_range_compatible; +use crate::types::ApiVersion; + +/// One microversion-scoped request-body schema, as vendored from a +/// generated `create_NN.rs`-style module. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct SchemaVariant { + /// The microversion this variant was introduced at. `None` means it + /// applies from the service's unversioned/earliest floor. + pub min_version: Option, + /// The microversion this variant stops applying at (inclusive). `None` + /// means it is still current -- valid for every version at or above + /// `min_version`. + pub max_version: Option, + /// The `BODY_SCHEMA` constant for this variant. + pub schema: &'static str, +} + +/// Select the schema whose microversion range covers `negotiated`. +/// +/// When multiple variants match (their ranges overlap `negotiated`), the one +/// with the highest `min_version` wins -- the most specific/newest +/// applicable variant. Returns `None` when no variant covers `negotiated`, +/// including the empty-slice case. +/// +/// A single-variant slice (the common case today -- most operations have +/// exactly one `BODY_SCHEMA`) degrades to always returning that one schema, +/// since an unbounded `[None, None]` range is compatible with anything. +pub fn select_schema(variants: &[SchemaVariant], negotiated: ApiVersion) -> Option<&'static str> { + variants + .iter() + .filter(|v| { + version_range_compatible( + v.min_version.unwrap_or(ApiVersion::new(0, 0)), + v.max_version, + Some(negotiated), + Some(negotiated), + ) + }) + .max_by_key(|v| v.min_version.unwrap_or(ApiVersion::new(0, 0))) + .map(|v| v.schema) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn v(major: u8, minor: u8) -> ApiVersion { + ApiVersion::new(major, minor) + } + + #[test] + fn empty_slice_selects_nothing() { + assert_eq!(select_schema(&[], v(2, 5)), None); + } + + #[test] + fn single_unbounded_variant_always_matches() { + let variants = [SchemaVariant { + min_version: None, + max_version: None, + schema: "SG_RULE", + }]; + assert_eq!(select_schema(&variants, v(2, 0)), Some("SG_RULE")); + assert_eq!(select_schema(&variants, v(2, 99)), Some("SG_RULE")); + } + + #[test] + fn picks_highest_matching_non_overlapping_variant() { + let variants = [ + SchemaVariant { + min_version: Some(v(2, 0)), + max_version: Some(v(2, 32)), + schema: "CREATE_20", + }, + SchemaVariant { + min_version: Some(v(2, 33)), + max_version: Some(v(2, 66)), + schema: "CREATE_233", + }, + SchemaVariant { + min_version: Some(v(2, 67)), + max_version: None, + schema: "CREATE_267", + }, + ]; + assert_eq!(select_schema(&variants, v(2, 1)), Some("CREATE_20")); + assert_eq!(select_schema(&variants, v(2, 40)), Some("CREATE_233")); + assert_eq!(select_schema(&variants, v(2, 90)), Some("CREATE_267")); + } + + #[test] + fn version_below_every_variant_selects_nothing() { + let variants = [SchemaVariant { + min_version: Some(v(2, 33)), + max_version: None, + schema: "CREATE_233", + }]; + assert_eq!(select_schema(&variants, v(2, 0)), None); + } + + #[test] + fn unbounded_latest_variant_wins_ties_via_highest_min_version() { + let variants = [ + SchemaVariant { + min_version: Some(v(2, 0)), + max_version: None, + schema: "OLD", + }, + SchemaVariant { + min_version: Some(v(2, 50)), + max_version: None, + schema: "NEW", + }, + ]; + // Both ranges are open-ended and both cover 2.80; the newer variant + // (higher min_version) is the one actually in effect. + assert_eq!(select_schema(&variants, v(2, 80)), Some("NEW")); + } +} From 986d5f5c6b89cbc3f15bb1e1a6ddf40d2b1dea3a Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Tue, 11 Aug 2026 12:15:01 +0200 Subject: [PATCH 2/4] feat(tui): Validate editor buffer against BODY_SCHEMA Extends the create-via-external-editor retry loop (from the earlier crash-on-bad-YAML fix) with a second validation stage: once a buffer parses as YAML, check it against the resource's schema (new ResourceBehaviour::editor_schema, wired through Action::Edit) via jsonschema, collecting every violation -- required fields, enums, ranges -- not just the first, before it's ever sent as EditResult. Failures reopen the editor with the errors prepended as comments, same pattern as the existing parse-error path. Also drops the schema to a temp file and prepends a yaml-language-server modeline when one is available, so editors that understand it get completion/live validation for free. editor_schema defaults to None (no resource emits BODY_SCHEMA in this repo yet, only the codegenerator side does so far), so this is a no-op until a resource opts in. Signed-off-by: Artem Goncharov --- Cargo.lock | 247 +++++++++++++++++- openstack_tui/Cargo.toml | 3 + openstack_tui/src/action.rs | 6 + openstack_tui/src/app.rs | 106 +++++++- openstack_tui/src/components.rs | 1 + .../src/components/editor_validation.rs | 192 ++++++++++++++ .../src/components/generic_resource_view.rs | 1 + .../network/security_group_rules.rs | 26 ++ .../src/components/resource_behaviour.rs | 15 ++ openstack_tui/src/tui.rs | 24 ++ 10 files changed, 605 insertions(+), 16 deletions(-) create mode 100644 openstack_tui/src/components/editor_validation.rs diff --git a/Cargo.lock b/Cargo.lock index f85fdc8e3..96ff157f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -61,6 +61,20 @@ dependencies = [ "subtle", ] +[[package]] +name = "ahash" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" +dependencies = [ + "cfg-if", + "getrandom 0.3.4", + "once_cell", + "serde", + "version_check", + "zerocopy", +] + [[package]] name = "aho-corasick" version = "1.1.4" @@ -454,7 +468,16 @@ version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0700ddab506f33b20a03b13996eccd309a48e5ff77d0d95926aa0210fb4e95f1" dependencies = [ - "bit-vec", + "bit-vec 0.6.3", +] + +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec 0.8.0", ] [[package]] @@ -463,6 +486,12 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "349f9b6a179ed607305526ca489b34ad0a41aed5f7980fa90eb03160b69598fb" +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "1.3.2" @@ -524,6 +553,12 @@ dependencies = [ "cipher", ] +[[package]] +name = "borrow-or-share" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc0b364ead1874514c8c2855ab558056ebfeb775653e7ae45ff72f28f8f3166c" + [[package]] name = "bstr" version = "1.13.0" @@ -550,6 +585,12 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "64fa3c856b712db6612c019f14756e64e4bcea13337a6b33b696333a9eaa2d06" +[[package]] +name = "bytecount" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "175812e0be2bccb6abe50bb8d566126198344f707e304f45c648fd8f2cc0365e" + [[package]] name = "bytemuck" version = "1.25.2" @@ -1698,6 +1739,15 @@ dependencies = [ "zeroize", ] +[[package]] +name = "email_address" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e079f19b08ca6239f47f8ba8509c11cf3ea30095831f7fed61441475edd8c449" +dependencies = [ + "serde", +] + [[package]] name = "embedded-io" version = "0.4.0" @@ -1854,10 +1904,21 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b95f7c0680e4142284cf8b22c14a476e87d61b004a3a0861872b32ef7ead40a2" dependencies = [ - "bit-set", + "bit-set 0.5.3", "regex", ] +[[package]] +name = "fancy-regex" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "476de73bddf2ef8490aa4ee8f1cf40b430bf1d56c48c22080e5186952cd580e6" +dependencies = [ + "bit-set 0.8.0", + "regex-automata", + "regex-syntax", +] + [[package]] name = "fast-srgb8" version = "1.0.0" @@ -1930,6 +1991,17 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "fluent-uri" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc74ac4d8359ae70623506d512209619e5cf8f347124910440dbc221714b328e" +dependencies = [ + "borrow-or-share", + "ref-cast", + "serde", +] + [[package]] name = "fnv" version = "1.0.7" @@ -1966,6 +2038,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fraction" +version = "0.15.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e076045bb43dac435333ed5f04caf35c7463631d0dae2deb2638d94dd0a5b872" +dependencies = [ + "lazy_static", + "num", +] + [[package]] name = "fs-set-times" version = "0.20.3" @@ -2131,9 +2213,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" dependencies = [ "cfg-if", + "js-sys", "libc", "r-efi 5.3.0", "wasip2", + "wasm-bindgen", ] [[package]] @@ -2909,6 +2993,58 @@ dependencies = [ "serde_json", ] +[[package]] +name = "jsonschema" +version = "0.49.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59ec8a241beed129f06114aa68007e905ca350e7baeb6e17a7631bb7978d91b2" +dependencies = [ + "ahash", + "bytecount", + "data-encoding", + "email_address", + "fancy-regex 0.19.0", + "fraction", + "getrandom 0.3.4", + "idna", + "itoa", + "jsonschema-regex", + "jsonschema-value", + "num-cmp", + "num-traits", + "percent-encoding", + "referencing", + "regex", + "serde", + "serde_json", + "strum", + "unicode-general-category", + "uuid-simd", +] + +[[package]] +name = "jsonschema-regex" +version = "0.49.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91994f45017ed5e66aa8e59b8415f4cb033a6380d7200387b7cf117595fbdf85" +dependencies = [ + "regex-syntax", +] + +[[package]] +name = "jsonschema-value" +version = "0.49.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7ec7637f83e510868ae6ed625f7ebfbbde4554ee8ce49854caa5126a8b9b9ecb" +dependencies = [ + "ahash", + "bytecount", + "fraction", + "num-cmp", + "num-traits", + "serde_json", +] + [[package]] name = "kasuari" version = "0.4.12" @@ -3189,6 +3325,12 @@ dependencies = [ "autocfg", ] +[[package]] +name = "micromap" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c2a86d3146ed3995b5913c414f6664344b9617457320782e64f0bb44afd49d74" + [[package]] name = "mime" version = "0.3.17" @@ -3255,6 +3397,20 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "num" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" +dependencies = [ + "num-bigint", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + [[package]] name = "num-bigint" version = "0.4.8" @@ -3281,6 +3437,21 @@ dependencies = [ "zeroize", ] +[[package]] +name = "num-cmp" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63335b2e2c34fae2fb0aa2cecfd9f0832a1e24b3b32ecec612c3426d46dc8aaa" + +[[package]] +name = "num-complex" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" +dependencies = [ + "num-traits", +] + [[package]] name = "num-conv" version = "0.2.2" @@ -3317,6 +3488,17 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -4453,6 +4635,7 @@ dependencies = [ "eyre", "futures", "itertools 0.15.0", + "jsonschema", "lazy_static", "open", "openstack_sdk", @@ -4461,11 +4644,13 @@ dependencies = [ "secrecy", "serde", "serde_json", + "serde_path_to_error", "serde_yaml", "signal-hook 0.4.4", "strip-ansi-escapes", "structable", "strum", + "tempfile", "thiserror 2.0.19", "tokio", "tokio-util", @@ -4515,6 +4700,12 @@ dependencies = [ "num-traits", ] +[[package]] +name = "outref" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a80800c0488c3a21695ea981a54918fbb37abf04f4d0720c453632255e2ff0e" + [[package]] name = "owo-colors" version = "4.3.0" @@ -5369,6 +5560,23 @@ dependencies = [ "syn 3.0.2", ] +[[package]] +name = "referencing" +version = "0.49.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6efa2154ea6f5ce0fdecdd2a8d18f2fa1a39a8fbba91564f555a592e4dce8278" +dependencies = [ + "ahash", + "fluent-uri", + "getrandom 0.3.4", + "hashbrown 0.17.1", + "itoa", + "micromap", + "parking_lot", + "percent-encoding", + "serde_json", +] + [[package]] name = "regalloc2" version = "0.15.2" @@ -5909,6 +6117,17 @@ dependencies = [ "zmij", ] +[[package]] +name = "serde_path_to_error" +version = "0.1.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "10a9ff822e371bb5403e391ecd83e182e0e77ba7f6fe0160b795797109d1b457" +dependencies = [ + "itoa", + "serde", + "serde_core", +] + [[package]] name = "serde_regex" version = "1.2.0" @@ -6491,7 +6710,7 @@ dependencies = [ "anyhow", "base64 0.22.1", "bitflags 2.13.1", - "fancy-regex", + "fancy-regex 0.11.0", "filedescriptor", "finl_unicode", "fixedbitset", @@ -6938,6 +7157,12 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142" +[[package]] +name = "unicode-general-category" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b993bddc193ae5bd0d623b49ec06ac3e9312875fdae725a975c51db1cc1677f" + [[package]] name = "unicode-ident" version = "1.0.24" @@ -7083,6 +7308,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "uuid-simd" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b082222b4f6619906941c17eb2297fff4c2fb96cb60164170522942a200bd8" +dependencies = [ + "outref", + "vsimd", +] + [[package]] name = "valuable" version = "0.1.1" @@ -7101,6 +7336,12 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" +[[package]] +name = "vsimd" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c3082ca00d5a5ef149bb8b555a72ae84c9c59f7250f013ac822ac2e49b19c64" + [[package]] name = "vte" version = "0.14.1" diff --git a/openstack_tui/Cargo.toml b/openstack_tui/Cargo.toml index 10bc8a675..1057cd8f1 100644 --- a/openstack_tui/Cargo.toml +++ b/openstack_tui/Cargo.toml @@ -32,6 +32,7 @@ edit = "0.1.5" eyre = { workspace = true } futures = { workspace = true } itertools = { workspace = true } +jsonschema = { version = "0.49.9", default-features = false } lazy_static = "^1.5" open.workspace = true openstack_sdk = { path = "../openstack_sdk", version = "^0.22", default-features = false, features = ["async", "block_storage", "compute", "dns", "identity", "image", "load_balancer", "network"] } @@ -40,11 +41,13 @@ ratatui = { version = "^0.30", features = ["serde", "macros", "crossterm"] } secrecy = "0.10.3" serde = { workspace = true } serde_json = { workspace = true } +serde_path_to_error = "0.1.20" serde_yaml = "^0.9" signal-hook = "^0.4" strip-ansi-escapes = "^0.2" structable = { workspace = true } strum = { version = "^0.28", features = ["derive"] } +tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } tokio-util = { workspace = true } diff --git a/openstack_tui/src/action.rs b/openstack_tui/src/action.rs index 6c53742a4..da6d90956 100644 --- a/openstack_tui/src/action.rs +++ b/openstack_tui/src/action.rs @@ -137,6 +137,12 @@ pub enum Action { /// Edit. Open the default editor to get the user input for the operation. Edit { template: String, + /// JSON Schema of the request body, when the resource has one + /// (`ResourceBehaviour::editor_schema`). Used to validate the + /// edited buffer beyond what plain YAML parsing catches (required + /// fields, enums, ranges, ...) before it is sent back as + /// `Action::EditResult`. + schema: Option, original_action: Box, }, EditResult { diff --git a/openstack_tui/src/app.rs b/openstack_tui/src/app.rs index f88a90605..a858eb3c5 100644 --- a/openstack_tui/src/app.rs +++ b/openstack_tui/src/app.rs @@ -25,9 +25,9 @@ use crate::{ cloud_worker::{AuthAction, Cloud}, components::{ Component, auth_helper::AuthHelper, cloud_select_popup::CloudSelect, - confirm_popup::ConfirmPopup, describe::Describe, error_popup::ErrorPopup, header::Header, - home::Home, project_select_popup::ProjectSelect, region_select_popup::RegionSelect, - resource_select_popup::ApiRequestSelect, + confirm_popup::ConfirmPopup, describe::Describe, editor_validation, + error_popup::ErrorPopup, header::Header, home::Home, project_select_popup::ProjectSelect, + region_select_popup::RegionSelect, resource_select_popup::ApiRequestSelect, }, config::Config, error::TuiError, @@ -482,7 +482,7 @@ impl App { } Action::Suspend => self.should_suspend = true, Action::Resume => self.should_suspend = false, - Action::ClearScreen => tui.terminal.clear()?, + Action::ClearScreen => tui.clear()?, Action::Resize(w, h) => self.handle_resize(tui, w, h)?, Action::Render => self.render(tui)?, Action::Clouds(_) @@ -551,17 +551,68 @@ impl App { } Action::Edit { ref template, + ref schema, ref original_action, } => { tui.exit()?; - let mut buffer = template.clone(); - // Retry until the buffer parses as valid YAML, or the user abandons the - // edit by clearing the buffer (only comments/whitespace left). + + // When a schema is available, drop it next to the buffer as a temp + // file and point editors that understand it (yaml-language-server) at + // it via a modeline comment -- completion/live validation for free in + // VS Code / neovim+yamlls, harmless elsewhere. Kept alive for the + // whole retry loop so the modeline path stays valid across re-edits. + let schema_temp_file = schema.as_ref().and_then(|s| { + let mut f = tempfile::Builder::new().suffix(".json").tempfile().ok()?; + std::io::Write::write_all(&mut f, s.as_bytes()).ok()?; + Some(f) + }); + let mut buffer = match &schema_temp_file { + Some(f) => format!( + "# yaml-language-server: $schema={}\n{template}", + f.path().display() + ), + None => template.clone(), + }; + + // Marks the boundary between a (regenerated-every-retry) error banner + // and the user's actual content. Without this, each retry appended a + // fresh banner on top of `edited`, which already contained the banner + // from the previous retry -- stale errors piled up instead of being + // replaced. Splitting on this marker lets each retry discard whatever + // banner is currently above it before prepending the new one. + const CONTENT_MARKER: &str = + "# ----- content below this line; messages above are regenerated on every retry, do not edit them -----"; + let strip_banner = |edited: &str| -> String { + match edited.split_once(CONTENT_MARKER) { + Some((_, rest)) => rest.trim_start_matches('\n').to_string(), + None => edited.to_string(), + } + }; + + // Retry until the buffer parses as valid YAML and (when a schema is + // available) validates against it, or the user abandons the edit -- + // by clearing the buffer (only comments/whitespace left), quitting the + // editor without changing anything (e.g. plain `:q`), or exiting the + // editor in a way that produces no usable result (non-zero exit, e.g. + // `:cq` or a killed editor process). let parsed = loop { - let edited = edit::edit(&buffer)?; + let edited = match edit::edit(&buffer) { + Ok(edited) => edited, + Err(err) => { + tracing::warn!( + "Editor did not return usable content, aborting edit: {err}" + ); + break None; + } + }; tracing::debug!("after editing: '{}'", edited); - let abandoned = edited.lines().all(|line| { + if edited == buffer { + break None; + } + + let content = strip_banner(&edited); + let abandoned = content.lines().all(|line| { let trimmed = line.trim(); trimmed.is_empty() || trimmed.starts_with('#') }); @@ -569,17 +620,46 @@ impl App { break None; } - match serde_yaml::from_str::(&edited) { - Ok(value) => break Some(value), + match serde_yaml::from_str::(&content) { + Ok(value) => { + // Blank optional fields parse as explicit YAML `null`, but + // BODY_SCHEMA property types are rarely declared nullable + // (the OpenAPI convention for "optional" is to omit the + // key) -- validating/sending them as-is would flag every + // blank optional field as a type mismatch. + let value = editor_validation::strip_null_fields(value); + if let Some(schema_str) = schema { + let errors = editor_validation::validate_body(schema_str, &value); + if !errors.is_empty() { + let comments = errors + .iter() + .map(|e| format!("# {e}")) + .collect::>() + .join("\n"); + buffer = format!( + "# Schema validation failed:\n{comments}\n# Fix the errors below and save to retry, or clear everything below the marker to abort.\n{CONTENT_MARKER}\n{content}" + ); + continue; + } + } + break Some(value); + } Err(err) => { buffer = format!( - "# Error parsing YAML: {err}\n# Fix the error below and save to retry, or clear the buffer entirely to abort.\n{edited}" + "# Error parsing YAML: {err}\n# Fix the error below and save to retry, or clear everything below the marker to abort.\n{CONTENT_MARKER}\n{content}" ); } } }; + drop(schema_temp_file); tui.enter()?; - tui.terminal.clear()?; + // Deferred via Action::ClearScreen (as the suspend/resume path above + // does) rather than calling tui.terminal.clear() here directly: that + // call reads the cursor position via a DSR terminal query, which can + // race with stray output the just-exited $EDITOR left behind and time + // out ("cursor position could not be read"). Routing through the + // action queue gives the terminal a tick to settle first. + self.action_tx.send(Action::ClearScreen)?; if let Some(result) = parsed { self.action_tx.send(Action::EditResult { result, diff --git a/openstack_tui/src/components.rs b/openstack_tui/src/components.rs index 70f42bf74..491eac1e3 100644 --- a/openstack_tui/src/components.rs +++ b/openstack_tui/src/components.rs @@ -30,6 +30,7 @@ pub mod compute; pub mod confirm_popup; pub mod describe; pub mod dns; +pub mod editor_validation; pub mod error_popup; pub mod generic_resource_view; pub mod header; diff --git a/openstack_tui/src/components/editor_validation.rs b/openstack_tui/src/components/editor_validation.rs new file mode 100644 index 000000000..1f6cc102b --- /dev/null +++ b/openstack_tui/src/components/editor_validation.rs @@ -0,0 +1,192 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +//! Post-YAML-parse validation for the create/edit-via-external-editor flow. +//! +//! Plain `serde_yaml`/`serde_json` parsing (already looped-on-failure in +//! `app.rs`) only catches syntax errors. This module adds the next stage: +//! checking the parsed buffer against a resource's `BODY_SCHEMA` (when it +//! has one, via `ResourceBehaviour::editor_schema`) -- required fields, +//! enums, ranges and the like that a `serde` struct alone can't express. + +use serde::de::DeserializeOwned; +use serde_json::Value; + +/// Drop object keys whose value is `null`, recursively. +/// +/// The editor template leaves optional fields blank (`field:`), which YAML +/// parses as an explicit `null` rather than omitting the key. Most +/// `BODY_SCHEMA` property entries are typed as a plain `"string"`/`"integer"` +/// (nullability isn't declared -- the OpenAPI convention for "optional" is +/// to omit the key, not send `null`), so validating the buffer as-is flags +/// *every* blank optional field as a type mismatch, making it look like +/// they're all mandatory. Stripping nulls before validation (and before the +/// value is used to build the request) restores "blank means omitted" while +/// still letting a truly required field left blank fail validation, since +/// its key is removed too. +pub fn strip_null_fields(value: Value) -> Value { + match value { + Value::Object(map) => Value::Object( + map.into_iter() + .filter(|(_, v)| !v.is_null()) + .map(|(k, v)| (k, strip_null_fields(v))) + .collect(), + ), + Value::Array(arr) => Value::Array(arr.into_iter().map(strip_null_fields).collect()), + other => other, + } +} + +/// Validate `instance` against `schema` (a `BODY_SCHEMA`-style JSON Schema +/// string), collecting *every* violation instead of stopping at the first, +/// each formatted as `: `. +/// +/// A schema that fails to compile is treated as "nothing to check against" +/// (logged, not surfaced as a user-facing error) rather than permanently +/// blocking the create/edit flow on a codegen-side bug. +pub fn validate_body(schema: &str, instance: &Value) -> Vec { + let schema_value: Value = match serde_json::from_str(schema) { + Ok(v) => v, + Err(err) => { + tracing::warn!("BODY_SCHEMA is not valid JSON, skipping validation: {err}"); + return Vec::new(); + } + }; + let validator = match jsonschema::validator_for(&schema_value) { + Ok(v) => v, + Err(err) => { + tracing::warn!("BODY_SCHEMA is not a valid JSON Schema, skipping validation: {err}"); + return Vec::new(); + } + }; + validator + .iter_errors(instance) + .map(|err| format!("{}: {}", err.instance_path(), err)) + .collect() +} + +/// Deserialize `data` into `T`, reporting the field path on failure instead +/// of serde's default "invalid type" message with no location. +pub fn deserialize_with_path(data: &Value) -> Result { + serde_path_to_error::deserialize(data.clone()).map_err(|err| err.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde::Deserialize; + use serde_json::json; + + #[test] + fn strip_null_fields_drops_top_level_nulls() { + let value = json!({"direction": "ingress", "protocol": null, "port_range_min": null}); + assert_eq!(strip_null_fields(value), json!({"direction": "ingress"})); + } + + #[test] + fn strip_null_fields_recurses_into_nested_objects() { + let value = json!({"security_group_rule": {"direction": "ingress", "protocol": null}}); + assert_eq!( + strip_null_fields(value), + json!({"security_group_rule": {"direction": "ingress"}}) + ); + } + + #[test] + fn strip_null_fields_leaves_non_null_values_untouched() { + let value = json!({"port_range_min": 80, "tags": ["a", "b"]}); + assert_eq!(strip_null_fields(value.clone()), value); + } + + #[test] + fn validate_body_flags_blank_optional_field_unless_stripped() { + let schema = json!({ + "type": "object", + "required": ["direction"], + "properties": { + "direction": {"type": "string"}, + "protocol": {"type": "string"} + } + }) + .to_string(); + let instance = json!({"direction": "ingress", "protocol": null}); + assert!(!validate_body(&schema, &instance).is_empty()); + assert!(validate_body(&schema, &strip_null_fields(instance)).is_empty()); + } + + #[test] + fn validate_body_returns_no_errors_for_valid_instance() { + let schema = json!({ + "type": "object", + "required": ["direction"], + "properties": { + "direction": {"type": "string", "enum": ["ingress", "egress"]} + } + }) + .to_string(); + let instance = json!({"direction": "ingress"}); + assert!(validate_body(&schema, &instance).is_empty()); + } + + #[test] + fn validate_body_reports_missing_required_field() { + let schema = json!({ + "type": "object", + "required": ["direction"] + }) + .to_string(); + let errors = validate_body(&schema, &json!({})); + assert_eq!(errors.len(), 1); + assert!(errors[0].contains("direction")); + } + + #[test] + fn validate_body_reports_all_violations_not_just_the_first() { + let schema = json!({ + "type": "object", + "required": ["direction", "ethertype"], + "properties": { + "port_range_min": {"type": "integer", "maximum": 65535} + } + }) + .to_string(); + let errors = validate_body(&schema, &json!({"port_range_min": 70000})); + assert_eq!(errors.len(), 3, "{errors:?}"); + } + + #[test] + fn validate_body_skips_malformed_schema_without_panicking() { + let errors = validate_body("not json", &json!({"a": 1})); + assert!(errors.is_empty()); + } + + #[derive(Debug, Deserialize, PartialEq)] + struct Sample { + name: String, + } + + #[test] + fn deserialize_with_path_succeeds_for_matching_shape() { + let data = json!({"name": "foo"}); + let sample: Sample = deserialize_with_path(&data).unwrap(); + assert_eq!(sample, Sample { name: "foo".into() }); + } + + #[test] + fn deserialize_with_path_reports_field_path_on_mismatch() { + let data = json!({"name": 42}); + let err = deserialize_with_path::(&data).unwrap_err(); + assert!(err.contains("name"), "{err}"); + } +} diff --git a/openstack_tui/src/components/generic_resource_view.rs b/openstack_tui/src/components/generic_resource_view.rs index b79eef6eb..c19ff29f6 100644 --- a/openstack_tui/src/components/generic_resource_view.rs +++ b/openstack_tui/src/components/generic_resource_view.rs @@ -252,6 +252,7 @@ where { return Ok(Some(Action::Edit { template, + schema: B::editor_schema(&action).map(String::from), original_action: Box::new(Action::PerformApiRequest(api_request)), })); } diff --git a/openstack_tui/src/components/network/security_group_rules.rs b/openstack_tui/src/components/network/security_group_rules.rs index 363292259..60c536932 100644 --- a/openstack_tui/src/components/network/security_group_rules.rs +++ b/openstack_tui/src/components/network/security_group_rules.rs @@ -127,6 +127,17 @@ security_group_rule: NetworkSecurityGroupRuleApiRequest::Create(Box::new(create)), )) } + fn editor_schema(action: &Action) -> Option<&'static str> { + if let Action::ResourceOp { + key, + op: crate::action::ResourceOp::Create, + } = action + && *key == Self::view_key() + { + return Some(NetworkSecurityGroupRuleCreate::BODY_SCHEMA); + } + None + } fn handle_mutation_response(request: &ApiRequest, data: &Value) -> Option> { if let ApiRequest::Network(NetworkApiRequest::SecurityGroupRule(req)) = request { if let NetworkSecurityGroupRuleApiRequest::Delete(del) = &**req { @@ -329,6 +340,21 @@ mod tests { assert!(NetworkSecurityGroupRulesBehaviour::clear_data_on_filter_change()); } + #[test] + fn editor_schema_returns_body_schema_for_create() { + let schema = NetworkSecurityGroupRulesBehaviour::editor_schema(&Action::ResourceOp { + key: crate::mode::NETWORK_SECURITY_GROUP_RULE, + op: crate::action::ResourceOp::Create, + }); + assert_eq!(schema, Some(NetworkSecurityGroupRuleCreate::BODY_SCHEMA)); + assert!(schema.unwrap().contains("\"IPv4\"")); + } + + #[test] + fn editor_schema_ignores_other_actions() { + assert!(NetworkSecurityGroupRulesBehaviour::editor_schema(&Action::Tick).is_none()); + } + #[test] fn editor_template_ignores_other_actions() { let filter = NetworkSecurityGroupRuleList::default(); diff --git a/openstack_tui/src/components/resource_behaviour.rs b/openstack_tui/src/components/resource_behaviour.rs index 8a80545a9..1981c6ef9 100644 --- a/openstack_tui/src/components/resource_behaviour.rs +++ b/openstack_tui/src/components/resource_behaviour.rs @@ -151,6 +151,16 @@ pub trait ResourceBehaviour { None } + /// JSON Schema of the request body backing `editor_template`'s + /// template, if this resource has one. When present, the edited buffer + /// is validated against it (required fields, enums, ranges, ...) before + /// being sent back for `deserialize_edit_result`, catching mistakes + /// plain YAML parsing can't. Default is no schema (parse-only + /// validation, the pre-existing behaviour). + fn editor_schema(_action: &Action) -> Option<&'static str> { + None + } + /// Map an action to a singular API request that should populate the describe pane, /// returning the (display actions, api request) tuple. Default returns None. fn action_to_singular_request( @@ -246,6 +256,11 @@ mod tests { assert_eq!(DefaultBehaviour::mode(), Mode::Resource("test.item")); } + #[test] + fn editor_schema_default_is_none() { + assert!(DefaultBehaviour::editor_schema(&Action::Tick).is_none()); + } + #[test] fn action_to_request_default_is_none() { let value = serde_json::json!({"id": "a"}); diff --git a/openstack_tui/src/tui.rs b/openstack_tui/src/tui.rs index 05f238da9..95f0f76b7 100644 --- a/openstack_tui/src/tui.rs +++ b/openstack_tui/src/tui.rs @@ -253,6 +253,30 @@ impl Tui { pub async fn next_event(&mut self) -> Option { self.event_rx.recv().await } + + /// Clear the whole screen without ratatui's `Terminal::clear()` cursor round-trip. + /// + /// `Terminal::clear()` snapshots the cursor position via a synchronous DSR terminal + /// query (`ESC[6n`) and restores it afterward. That query races against `start()`'s + /// `EventStream`, which asynchronously owns stdin once the event loop is running -- + /// the DSR reply lands in the event stream instead of the synchronous poll, so the + /// query reliably times out ("cursor position could not be read") any time this is + /// called after `enter()`/`start()` have run, e.g. on return from an external editor. + /// + /// `Terminal::resize()` (called here with the current, unchanged area) clears the + /// screen and resets the back buffer the same way -- forcing a full redraw on the + /// next `draw()` -- but only touches the cursor for `Viewport::Inline`, so for the + /// `Fullscreen` viewport this app uses it's cursor-query-free. A raw + /// `backend.clear_region()` alone clears the visible terminal but leaves ratatui's + /// back buffer believing nothing changed, so the next `draw()` only repaints the + /// diff against stale content -- the screen stays black until unrelated redraws + /// accumulate enough diffs to repaint it. + pub fn clear(&mut self) -> Result<()> { + let area = self.terminal.size()?; + self.terminal + .resize(ratatui::layout::Rect::new(0, 0, area.width, area.height))?; + Ok(()) + } } impl Deref for Tui { From baced139c514db78afd2def52de38c1d5634b581 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Tue, 11 Aug 2026 16:59:24 +0200 Subject: [PATCH 3/4] feat: Extend the ResourceBehavior trait prepare further editor work Signed-off-by: Artem Goncharov --- .../src/components/resource_behaviour.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/openstack_tui/src/components/resource_behaviour.rs b/openstack_tui/src/components/resource_behaviour.rs index 1981c6ef9..0fa930858 100644 --- a/openstack_tui/src/components/resource_behaviour.rs +++ b/openstack_tui/src/components/resource_behaviour.rs @@ -39,6 +39,25 @@ pub trait GeneratedResourceBehaviour { let _ = action; None } + + /// Return a YAML editor template for a create action. Mirrors + /// `ResourceBehaviour::editor_template`; resources that need to prefill a field from the + /// filter should call this, then post-process the returned template string, rather than + /// duplicating the field list/comments here. + fn editor_template(_action: &Action, _filter: &Self::Filter) -> Option<(String, ApiRequest)> { + None + } + + /// Deserialize the edited YAML back into an ApiRequest. Mirrors + /// `ResourceBehaviour::deserialize_edit_result`. + fn deserialize_edit_result(_data: &Value) -> Option { + None + } + + /// JSON Schema backing `editor_template`'s template. Mirrors `ResourceBehaviour::editor_schema`. + fn editor_schema(_action: &Action) -> Option<&'static str> { + None + } } /// Behaviour specifics for a particular OpenStack resource. From 38442e16c2adaad3785a769b5dc7d7068659ef12 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Tue, 11 Aug 2026 17:18:58 +0200 Subject: [PATCH 4/4] chore: Address cargo-deny issues Signed-off-by: Artem Goncharov --- Cargo.toml | 1 + deny.toml | 4 +++- openstack_tui/Cargo.toml | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 858fcc544..5ac630bfb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,6 +73,7 @@ hyper = { version = "^1.11" } hyper-util = { version = "^0.1" } inventory = { version = "0.3" } itertools = { version = "^0.15" } +jsonschema = { version = "0.49", default-features = false } json-patch = { version = "^4.2" } lazy_static = { version = "^1.5" } open = { version = "^5.4" } diff --git a/deny.toml b/deny.toml index 2142c3d6c..51d2b63bb 100644 --- a/deny.toml +++ b/deny.toml @@ -77,7 +77,8 @@ ignore = [ { id = "RUSTSEC-2026-0222", reason = "wasmtime as dep of extism"}, { id = "RUSTSEC-2026-0247", reason = "dep of wasmtime"}, { id = "RUSTSEC-2026-0250", reason = "dep of wasmtime"}, - { id = "RUSTSEC-2026-0251", reason = "dep of wasmtime"} + { id = "RUSTSEC-2026-0251", reason = "dep of wasmtime"}, + { id = "RUSTSEC-2026-0253", reason = "ratatui dep"} ] # If this is true, then cargo deny will use the git executable to fetch advisory database. # If this is false, then it uses a built-in git library. @@ -101,6 +102,7 @@ allow = [ "ISC", "LicenseRef-ring", "MIT", + "MIT-0", "MPL-2.0", "NCSA", "OpenSSL", diff --git a/openstack_tui/Cargo.toml b/openstack_tui/Cargo.toml index 1057cd8f1..25953211d 100644 --- a/openstack_tui/Cargo.toml +++ b/openstack_tui/Cargo.toml @@ -32,7 +32,7 @@ edit = "0.1.5" eyre = { workspace = true } futures = { workspace = true } itertools = { workspace = true } -jsonschema = { version = "0.49.9", default-features = false } +jsonschema = { workspace = true } lazy_static = "^1.5" open.workspace = true openstack_sdk = { path = "../openstack_sdk", version = "^0.22", default-features = false, features = ["async", "block_storage", "compute", "dns", "identity", "image", "load_balancer", "network"] }