diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6ead28139..fd6fb0307 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,12 @@ jobs: - target: x86_64-unknown-linux-gnu os: ubuntu-latest test_args: "" - rust_ver: 1.91 + # Must match cli/config/Cargo.toml's `rust-version` (pinned + # there for yamlpatch/yamlpath, comment/anchor-preserving YAML + # edits). The workspace default build (no -p filter below) + # includes cli/config, so bumping either value without the + # other breaks this job. + rust_ver: 1.97 # - target: x86_64-unknown-linux-musl # os: ubuntu-latest # - target: x86_64-apple-darwin diff --git a/Cargo.lock b/Cargo.lock index e4d2255b9..cb62a1618 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3150,6 +3150,12 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "libyaml-rs" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e126dda6f34391ab7b444f9922055facc83c07a910da3eb16f1e4d9c45dc777" + [[package]] name = "line-clipping" version = "0.3.8" @@ -3159,6 +3165,16 @@ dependencies = [ "bitflags 2.13.2", ] +[[package]] +name = "line-index" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3e27e0ed5a392a7f5ba0b3808a2afccff16c64933312c84b57618b49d1209bd2" +dependencies = [ + "nohash-hasher", + "text-size", +] + [[package]] name = "linux-raw-sys" version = "0.4.15" @@ -3430,6 +3446,12 @@ dependencies = [ "memoffset 0.9.1", ] +[[package]] +name = "nohash-hasher" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2bf50223579dc7cdcfb3bfcacf7069ff68243f8c363f62ffa99cf000a6b9c451" + [[package]] name = "nom" version = "7.1.3" @@ -3768,6 +3790,18 @@ dependencies = [ "zeroize", ] +[[package]] +name = "openstack-cli-config" +version = "0.13.8" +dependencies = [ + "indexmap", + "serde", + "thiserror 2.0.20", + "yaml_serde", + "yamlpatch", + "yamlpath", +] + [[package]] name = "openstack-cli-container-infrastructure-management" version = "0.13.8" @@ -6124,6 +6158,12 @@ dependencies = [ "libc", ] +[[package]] +name = "self_cell" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ab42ca02749e120097e328d91d415325bdf43b1c72c4c8badf37375fe40a813" + [[package]] name = "semver" version = "1.0.28" @@ -6211,6 +6251,7 @@ version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ + "indexmap", "itoa", "memchr", "serde", @@ -6525,6 +6566,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b2231b7c3057d5e4ad0156fb3dc807d900806020c5ffa3ee6ff2c8c76fb8520" + [[package]] name = "stringmetrics" version = "2.2.2" @@ -6589,6 +6636,17 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "subfeature" +version = "1.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d17bf55e69a49b140d9298be68c319c283c5479c391c8393b70e14d32b907f82" +dependencies = [ + "memchr", + "regex", + "serde", +] + [[package]] name = "subtle" version = "2.6.1" @@ -6856,6 +6914,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "text-size" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f18aa187839b2bdb1ad2fa35ead8c4c2976b64e4363c386d45ac0f7ee85c9233" + [[package]] name = "thiserror" version = "1.0.69" @@ -7236,6 +7300,45 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "tree-sitter" +version = "0.26.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17ebdd3a5a7e28a1890b876fdbd0c3c0fe0a6336cffaa104f11b9f720c9daa29" +dependencies = [ + "cc", + "regex", + "regex-syntax", + "serde_json", + "streaming-iterator", + "tree-sitter-language", +] + +[[package]] +name = "tree-sitter-iter" +version = "1.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8ec40618be69571d6487d2ef96a84a2749c98a279a12346375cbf19b871420eb" +dependencies = [ + "tree-sitter", +] + +[[package]] +name = "tree-sitter-language" +version = "0.1.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca0d1bf6fdd806e43ae5198f82f527056d359def39e54e67a0f478ac09dac081" + +[[package]] +name = "tree-sitter-yaml" +version = "0.7.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53c223db85f05e34794f065454843b0668ebc15d240ada63e2b5939f43ce7c97" +dependencies = [ + "cc", + "tree-sitter-language", +] + [[package]] name = "try-lock" version = "0.2.5" @@ -8559,6 +8662,49 @@ dependencies = [ "hashlink", ] +[[package]] +name = "yaml_serde" +version = "0.10.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33b729a08a9a6be689bbad3e2bf8015926db54b6622cc89c3a5f7dc174b9e918" +dependencies = [ + "indexmap", + "itoa", + "libyaml-rs", + "ryu", + "serde", +] + +[[package]] +name = "yamlpatch" +version = "1.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7aec7b59ab1c3b70872ab54a84b3bbd91ab743ea8e59fc02f6f0c45c04426392" +dependencies = [ + "indexmap", + "line-index", + "serde_json", + "subfeature", + "thiserror 2.0.20", + "yaml_serde", + "yamlpath", +] + +[[package]] +name = "yamlpath" +version = "1.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a2ea2334936adb60c891686760016e723e220caee5c5b581a16b575ba85b2f0" +dependencies = [ + "line-index", + "self_cell", + "serde", + "thiserror 2.0.20", + "tree-sitter", + "tree-sitter-iter", + "tree-sitter-yaml", +] + [[package]] name = "yansi" version = "1.0.1" diff --git a/Cargo.toml b/Cargo.toml index 490c6f13b..44a16b971 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -72,6 +72,7 @@ http-body-util = "^0.1" httpmock = { version = "^0.8" } hyper = { version = "^1.11" } hyper-util = { version = "^0.1" } +indexmap = { version = "^2.14" } inventory = { version = "0.3" } itertools = { version = "^0.15" } jsonschema = { version = "0.53", default-features = false } @@ -83,6 +84,7 @@ openstack-cli-auth = { path="cli/auth", version = "0.13.8" } openstack-cli-block-storage = { path="cli/block-storage/", version = "0.13.8" } openstack-cli-catalog = { path="cli/catalog/", version = "0.13.8" } openstack-cli-compute = { path="cli/compute/", version = "0.13.8" } +openstack-cli-config = { path="cli/config/", version = "0.13.8" } openstack-cli-container-infrastructure-management = { path="cli/container-infrastructure-management/", version = "0.13.8" } openstack-cli-core = { path="cli/core", version = "0.13.8" } openstack-cli-dns = { path="cli/dns/", version = "0.13.8" } @@ -131,6 +133,13 @@ webauthn-authenticator-rs = { version = "^0.5", features = ["ctap2", "mozilla", webauthn-rs-proto = { version = "^0.5" } uuid = { version = "^1.26" } x509-parser = { version = "^0.18" } +# Comment/anchor-preserving YAML editing, used by cli/config's yaml_edit +# module. Both pin to zizmor's release train (the project they originate +# from); yaml_serde is the actively maintained fork of the now-archived +# serde_yaml, with the same API shape. +yaml_serde = { version = "^0.10" } +yamlpatch = { version = "^1.30" } +yamlpath = { version = "^1.30" } zeroize = { version = "^1.9" } [profile.dev] diff --git a/cli/config/Cargo.toml b/cli/config/Cargo.toml new file mode 100644 index 000000000..108ee050a --- /dev/null +++ b/cli/config/Cargo.toml @@ -0,0 +1,28 @@ +[package] +name = "openstack-cli-config" +description = "OpenStack CLI config commands" +version = "0.13.8" +license.workspace = true +edition.workspace = true +authors.workspace = true +# `yamlpatch`/`yamlpath` (see src/yaml_edit.rs) require a newer toolchain +# than the rest of the workspace. Scoped to this crate only: nothing else +# in the workspace depends on `openstack-cli-config`, so SDK-only consumers +# never pull this requirement in. See CONTRIBUTING.md / the PR description +# for context. +# Keep in sync with .github/workflows/ci.yml's `rust_ver` — the workspace +# default build includes this crate, so a mismatch breaks CI. +rust-version = "1.97" +homepage.workspace = true +repository.workspace = true + +[dependencies] +indexmap.workspace = true +serde.workspace = true +thiserror.workspace = true +yaml_serde.workspace = true +yamlpatch.workspace = true +yamlpath.workspace = true + +[lints] +workspace = true diff --git a/cli/config/src/lib.rs b/cli/config/src/lib.rs new file mode 100644 index 000000000..14177362d --- /dev/null +++ b/cli/config/src/lib.rs @@ -0,0 +1,21 @@ +// 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 +//! Local client configuration file operations. +//! +//! This crate is the foundation for `osc config` commands that read and +//! edit `clouds.yaml`/`secure.yaml` in place. It currently provides +//! [`yaml_edit`], a comment- and anchor-preserving YAML editor; command +//! implementations built on top of it (e.g. `clouds add`) land separately. + +pub mod yaml_edit; diff --git a/cli/config/src/yaml_edit.rs b/cli/config/src/yaml_edit.rs new file mode 100644 index 000000000..5924eab3d --- /dev/null +++ b/cli/config/src/yaml_edit.rs @@ -0,0 +1,280 @@ +// 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 + +//! Comment- and anchor-preserving edits to a `clouds.yaml`/`secure.yaml` +//! style document. +//! +//! `clouds.yaml` is frequently hand-maintained (comments, `&anchor`/`<<: +//! *anchor` merge keys) and consumed by other tools (Terraform, Ansible, +//! python-openstackclient). Round-tripping it through a generic YAML +//! (de)serializer, as an early revision of `osc config clouds add` did, +//! silently expands anchors and drops comments. This module instead +//! splices individual mapping entries into the existing document text via +//! [`yamlpatch`], leaving everything else byte-for-byte untouched. +//! +//! This module only offers the mechanical parse/locate/splice/render +//! primitives; policy (e.g. "error on a name collision unless +//! `--overwrite` was passed") is left to callers, so the same primitives +//! serve `clouds add` (insert-or-error) and future `clouds edit`/`clouds +//! remove`/`contexts *` commands (where an existing key is the expected +//! case, not an error). + +use indexmap::IndexMap; +use serde::Serialize; +use yamlpatch::{Op, Patch, apply_yaml_patches}; +use yamlpath::{Document, Route, route}; + +/// Errors from parsing or splice-editing a YAML document. +#[derive(Debug, thiserror::Error)] +pub enum YamlEditError { + /// The document text is not valid YAML. + #[error("the file is not valid YAML: {0}")] + Parse(#[from] yamlpath::QueryError), + /// The value being inserted could not be serialized to YAML. + #[error("could not serialize the value to YAML: {0}")] + Serialize(#[from] yaml_serde::Error), + /// The requested edit could not be applied (e.g. a name collision, or + /// an unsupported document shape such as a multi-line flow mapping). + #[error("the edit could not be applied: {0}")] + Edit(#[from] yamlpatch::Error), +} + +/// A parsed YAML document that supports byte-preserving splice edits. +/// +/// Untouched regions of the document (comments, anchors, formatting, +/// unrelated entries) are always preserved exactly; only the specific +/// mapping entry an edit targets is rewritten. +pub struct YamlDocument(Document); + +impl YamlDocument { + /// Parse an existing document. + pub fn parse(text: &str) -> Result { + Ok(Self(Document::new(text)?)) + } + + /// An empty mapping document, for the "target file does not exist + /// yet" case. + /// + /// Note: an entry inserted directly into this starting point renders + /// in flow style (`{...}`); there is nothing to preserve in a + /// brand-new file, so callers that want block-style output for a + /// fresh file may prefer to build it directly instead of starting + /// from this empty document. + pub fn empty_mapping() -> Self { + // "{}\n" is a fixed, always-valid YAML literal (an empty flow + // mapping), so `Document::new` cannot fail on it. + #[allow(clippy::unwrap_used)] + let doc = Document::new("{}\n").unwrap(); + Self(doc) + } + + /// Whether `parent.key` exists in the document, e.g. + /// `contains_key("clouds", "mycloud")`. + pub fn contains_key(&self, parent: &str, key: &str) -> bool { + self.0.query_exists(&route![parent, key]) + } + + /// Insert or replace the mapping entry at `parent.key` with `value`. + /// + /// The `parent` mapping is created if it does not already exist (e.g. + /// a missing top-level `clouds:`), as long as the document already has + /// *some* top-level mapping content to attach it to (as + /// [`empty_mapping`](Self::empty_mapping) provides) — a document that + /// is empty or contains only comments has no root node to create the + /// key under. An existing `parent.key` is replaced; callers that need + /// to reject a collision should check + /// [`contains_key`](Self::contains_key) first. + pub fn upsert_mapping_entry( + &mut self, + parent: &str, + key: &str, + value: &T, + ) -> Result<(), YamlEditError> { + let value = yaml_serde::to_value(value)?; + let mut updates = IndexMap::new(); + updates.insert(key.to_string(), value); + let patch = Patch { + route: Route::default(), + operation: Op::MergeInto { + key: parent.to_string(), + updates, + }, + }; + self.0 = apply_yaml_patches(&self.0, std::slice::from_ref(&patch))?; + Ok(()) + } + + /// Remove the mapping entry at `parent.key`. + pub fn remove_mapping_entry(&mut self, parent: &str, key: &str) -> Result<(), YamlEditError> { + let patch = Patch { + route: route![parent, key], + operation: Op::Remove, + }; + self.0 = apply_yaml_patches(&self.0, std::slice::from_ref(&patch))?; + Ok(()) + } + + /// The document's current YAML text. + pub fn source(&self) -> &str { + self.0.source() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(Serialize)] + struct Entry { + auth_url: String, + region_name: String, + } + + fn entry(auth_url: &str, region_name: &str) -> Entry { + Entry { + auth_url: auth_url.to_string(), + region_name: region_name.to_string(), + } + } + + #[test] + fn parse_rejects_malformed_yaml() { + assert!(YamlDocument::parse(": not yaml : [").is_err()); + } + + #[test] + fn insert_into_missing_parent_creates_it() -> Result<(), YamlEditError> { + let mut doc = YamlDocument::parse("# top comment\ncache:\n auth: true\n")?; + doc.upsert_mapping_entry( + "clouds", + "mycloud", + &entry("https://keystone:5000", "RegionOne"), + )?; + + assert!(doc.source().starts_with("# top comment\n")); + let parsed: yaml_serde::Value = + yaml_serde::from_str(doc.source()).map_err(YamlEditError::from)?; + assert_eq!( + parsed["clouds"]["mycloud"]["auth_url"].as_str(), + Some("https://keystone:5000") + ); + Ok(()) + } + + #[test] + fn insert_preserves_comments_and_other_entries() -> Result<(), YamlEditError> { + let existing = "# my clouds\nclouds:\n # existing entry\n other:\n auth_url: https://other:5000\n region_name: RegionOther\n"; + let mut doc = YamlDocument::parse(existing)?; + doc.upsert_mapping_entry("clouds", "mycloud", &entry("https://new:5000", "RegionOne"))?; + + let out = doc.source(); + assert!(out.contains("# my clouds")); + assert!(out.contains("# existing entry")); + assert!(out.contains("other:")); + assert!(out.contains("https://other:5000")); + assert!(out.contains("mycloud:")); + assert!(out.contains("https://new:5000")); + Ok(()) + } + + #[test] + fn insert_preserves_anchors_and_merge_keys() -> Result<(), YamlEditError> { + // This is the exact regression the switch away from a + // round-tripping serializer (serde_yaml) was made for: `&anchor` + // and `<<: *anchor` must survive byte-for-byte. + let existing = "clouds:\n base: &base\n region_name: RegionOne\n devstack:\n <<: *base\n auth_url: https://devstack:5000\n"; + let mut doc = YamlDocument::parse(existing)?; + doc.upsert_mapping_entry("clouds", "mycloud", &entry("https://new:5000", "RegionOne"))?; + + let out = doc.source(); + assert!( + out.contains("&base"), + "anchor definition must survive: {out}" + ); + assert!(out.contains("<<: *base"), "merge key must survive: {out}"); + Ok(()) + } + + #[test] + fn upsert_replaces_existing_entry() -> Result<(), YamlEditError> { + let existing = + "clouds:\n mycloud:\n auth_url: https://old:5000\n region_name: RegionOld\n"; + let mut doc = YamlDocument::parse(existing)?; + assert!(doc.contains_key("clouds", "mycloud")); + + doc.upsert_mapping_entry("clouds", "mycloud", &entry("https://new:5000", "RegionNew"))?; + + let out = doc.source(); + assert!(!out.contains("https://old:5000")); + assert!(out.contains("https://new:5000")); + Ok(()) + } + + #[test] + fn remove_deletes_only_targeted_entry() -> Result<(), YamlEditError> { + let existing = "# header\nclouds:\n keep:\n auth_url: https://keep:5000\n drop:\n auth_url: https://drop:5000\n"; + let mut doc = YamlDocument::parse(existing)?; + doc.remove_mapping_entry("clouds", "drop")?; + + let out = doc.source(); + assert!(out.contains("# header")); + assert!(out.contains("keep:")); + assert!(out.contains("https://keep:5000")); + assert!(!out.contains("drop:")); + assert!(!out.contains("https://drop:5000")); + Ok(()) + } + + #[test] + fn contains_key_reflects_document_state() -> Result<(), YamlEditError> { + let mut doc = YamlDocument::parse("clouds:\n existing:\n auth_url: https://x:5000\n")?; + assert!(doc.contains_key("clouds", "existing")); + assert!(!doc.contains_key("clouds", "missing")); + + doc.upsert_mapping_entry("clouds", "missing", &entry("https://x:5000", "RegionOne"))?; + assert!(doc.contains_key("clouds", "missing")); + Ok(()) + } + + #[test] + fn empty_mapping_round_trip() -> Result<(), YamlEditError> { + let mut doc = YamlDocument::empty_mapping(); + doc.upsert_mapping_entry("clouds", "mycloud", &entry("https://x:5000", "RegionOne"))?; + + let parsed: yaml_serde::Value = + yaml_serde::from_str(doc.source()).map_err(YamlEditError::from)?; + assert_eq!( + parsed["clouds"]["mycloud"]["auth_url"].as_str(), + Some("https://x:5000") + ); + Ok(()) + } + + #[test] + fn empty_mapping_renders_flow_style() -> Result<(), YamlEditError> { + // Pins the behavior called out in `empty_mapping`'s doc comment: + // a fresh file renders flow-style (`{...}`), not block-style. + // Callers that want block-style output for a brand-new file must + // build it directly instead of starting from `empty_mapping`. + let mut doc = YamlDocument::empty_mapping(); + doc.upsert_mapping_entry("clouds", "mycloud", &entry("https://x:5000", "RegionOne"))?; + + assert!( + doc.source().trim_start().starts_with('{'), + "expected flow-style output: {}", + doc.source() + ); + Ok(()) + } +}