diff --git a/README-extensions.md b/README-extensions.md index 3766197..7ee008e 100644 --- a/README-extensions.md +++ b/README-extensions.md @@ -21,9 +21,17 @@ For example, given the following inventory, the node's internal path is parsed a ``` However, optionally, reclass-rs can be configured to handle `compose_node_name` the same way that kapicorp-reclass does, by naively splitting node names on each dot. -To enable this compatibility mode, set `compat_flags: ['ComposeNodeNameLiteralDots']` in your inventory's `reclass-config.yml`. +To enable this compatibility mode, set `reclass_rs_compat_flags: ['ComposeNodeNameLiteralDots']` in your inventory's `reclass-config.yml`. In compatibility mode, the node's internal path for the previous inventory is `['path', 'to', 'the', 'node']`. +## Handling of YAML `null` values in nested references + +By default, reclass-rs resolves YAML `null` values in nested references to the string `null`. +This behavior ensures that references which resolve to YAML `null` are correctly preserved when using them as reference default values (see below). + +Optionally, reclass-rs can be configured to preserve the kapicorp-reclass behavior of resolving YAML `null` values in nested references to string "None". +To enable this compatibility mode, set `reclass_rs_compat_flags: ['NestedReferenceNullAsNone']` in your inventory's `reclass-config.yml`. + ## Verbose warnings Reclass-rs supports boolean config option `verbose_warnings`. diff --git a/README.md b/README.md index 7a97d92..ee0d446 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ The implementation currently supports the following features of Kapicorp Reclass * Merging referenced lists and dictionaries * Constant parameters * Nested references + * By default reclass-rs uses a non-compatible mode which resolves YAML `null` values in nested references to string "null". * References in class names * Loading classes with relative names * Loading Reclass configuration options from `reclass-config.yaml` diff --git a/python/reclass_rs/reclass_rs.pyi b/python/reclass_rs/reclass_rs.pyi index dd0de04..281be08 100644 --- a/python/reclass_rs/reclass_rs.pyi +++ b/python/reclass_rs/reclass_rs.pyi @@ -15,6 +15,7 @@ __all__: list[str] = [ @final class CompatFlag(Enum): ComposeNodeNameLiteralDots = "ComposeNodeNameLiteralDots" + NestedReferenceNullAsNone = "NestedReferenceNullAsNone" @final class Config: diff --git a/src/config.rs b/src/config.rs index ccec795..cb75292 100644 --- a/src/config.rs +++ b/src/config.rs @@ -25,6 +25,13 @@ pub enum CompatFlag { /// file path when rendering fields `path` and `parts` in `NodeInfoMeta` when /// `compose-node-name` is enabled. ComposeNodeNameLiteralDots, + /// This flag enables Python Reclass-compatible rendering of nested references that resolve to + /// a YAML `null` value. When this flag is set, YAML `null` values encountered during nested + /// reference resolution will be serialized as `"None"`. + /// + /// By default, if this flag isn't enabled, reclass-rs will serialize YAML `null` values as + /// string `"null"` during nested reference resolution. + NestedReferenceNullAsNone, } #[pymethods] @@ -43,6 +50,9 @@ impl TryFrom<&str> for CompatFlag { "compose-node-name-literal-dots" | "compose_node_name_literal_dots" | "ComposeNodeNameLiteralDots" => Ok(Self::ComposeNodeNameLiteralDots), + "nested-reference-null-as-none" + | "nested_reference_null_as_none" + | "NestedReferenceNullAsNone" => Ok(Self::NestedReferenceNullAsNone), _ => Err(anyhow!("Unknown compatibility flag '{value}'")), } } @@ -166,11 +176,13 @@ impl ClassMapping { } } +#[allow(clippy::struct_excessive_bools)] #[derive(Clone, Debug, Default)] pub struct RenderOpts { pub ignore_overwritten_missing_references: bool, pub verbose_warnings: bool, pub(crate) preserve_resolve_error_in_flattened: bool, + pub(crate) nested_reference_null_as_none: bool, } #[pyclass(from_py_object)] @@ -490,6 +502,9 @@ impl From<&Config> for RenderOpts { Self { ignore_overwritten_missing_references: value.ignore_overwritten_missing_references, verbose_warnings: value.verbose_warnings, + nested_reference_null_as_none: value + .compatflags + .contains(&CompatFlag::NestedReferenceNullAsNone), ..Default::default() } } diff --git a/src/node/mod.rs b/src/node/mod.rs index d8b566e..838bf63 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -234,7 +234,7 @@ impl Node { let mut state = ResolveState::default(); clstoken .render(&root.parameters, &mut state, &r.config.get_render_opts())? - .raw_string()? + .raw_string(&r.config.get_render_opts())? } else { // If Token::parse() returns None, the class name can't contain any references, // just convert cls into an owned String. diff --git a/src/refs/mod.rs b/src/refs/mod.rs index 1304884..9708121 100644 --- a/src/refs/mod.rs +++ b/src/refs/mod.rs @@ -46,8 +46,8 @@ impl ResolveState { /// Pushes mapping key into the `current_keys` list. If possible, the provided value is /// formatted with `raw_string()`. Additionally, unprocessed `String` values are pushed as-is. /// This function will return an error when it's called with a `Value::ValueList`. - pub(crate) fn push_mapping_key(&mut self, key: &Value) -> Result<()> { - let kstr = match key.raw_string() { + pub(crate) fn push_mapping_key(&mut self, key: &Value, opts: &RenderOpts) -> Result<()> { + let kstr = match key.raw_string(opts) { Ok(s) => s, Err(_) => match key { Value::String(s) => Ok(s.clone()), @@ -173,7 +173,7 @@ impl Token { .interpolate(params, state, opts) } else { Ok(Value::Literal( - self.resolve(params, state, opts)?.raw_string()?, + self.resolve(params, state, opts)?.raw_string(opts)?, )) } } @@ -374,7 +374,7 @@ fn interpolate_token_slice( while v.is_string() { v = v.interpolate(params, &mut st, opts)?; } - res.push_str(&v.raw_string()?); + res.push_str(&v.raw_string(opts)?); } Ok(res) } diff --git a/src/types/mapping.rs b/src/types/mapping.rs index d16403e..d3a5bc5 100644 --- a/src/types/mapping.rs +++ b/src/types/mapping.rs @@ -401,7 +401,7 @@ impl Mapping { && let Some(p) = p { let mut st = state.clone(); - st.push_mapping_key(k)?; + st.push_mapping_key(k, opts)?; if let Some(errmsg) = p.as_resolve_error() { if opts.ignore_overwritten_missing_references { #[cfg(not(feature = "bench"))] @@ -476,7 +476,7 @@ impl Mapping { // either manage to interpolate a value (in which case it doesn't contain a loop) or we // don't and the whole interpolation is aborted. let mut st = state.clone(); - st.push_mapping_key(k)?; + st.push_mapping_key(k, opts)?; let iv = v.interpolate(root, &mut st, opts); let v = if let Err(e) = iv { // convert interpolation errors into `Value::ResolveError` when interpolating diff --git a/src/types/value.rs b/src/types/value.rs index f8aa736..77cc0d2 100644 --- a/src/types/value.rs +++ b/src/types/value.rs @@ -481,11 +481,20 @@ impl Value { /// strings which match Python's `str()` for other types. /// #[inline] - pub(crate) fn raw_string(&self) -> Result { + pub(crate) fn raw_string(&self, opts: &RenderOpts) -> Result { match self { Value::Literal(s) => Ok(s.clone()), - // We serialize Null as `None` to be compatible with Python's str() - Value::Null => Ok("None".to_string()), + // We serialize Null as `null` unless the `NestedReferenceNullAsNone` compat flag is + // set, in which case we serialize Null as `None`. The default serialization isn't + // compatible with Python's str(), but ensures that a reclass-rs reference default + // value which is a reference that resolves to a Null value is preserved correctly. + Value::Null => { + if opts.nested_reference_null_as_none { + Ok("None".to_string()) + } else { + Ok("null".to_string()) + } + } // We need custom formatting for bool instead of `format!("{b}")`, so that this // function returns strings which match Python's `str()` implementation. Value::Bool(b) => match b { diff --git a/src/types/value/value_flattened_tests.rs b/src/types/value/value_flattened_tests.rs index dc90744..f5fdddc 100644 --- a/src/types/value/value_flattened_tests.rs +++ b/src/types/value/value_flattened_tests.rs @@ -42,7 +42,8 @@ test_flattened_simple! { fn test_flattened_string() { let v = Value::String("foo".into()); let mut st = ResolveState::default(); - st.push_mapping_key(&"test".into()).unwrap(); + st.push_mapping_key(&"test".into(), &RenderOpts::default()) + .unwrap(); v.flattened(&mut st, &RenderOpts::default()).unwrap(); } diff --git a/src/types/value/value_tests.rs b/src/types/value/value_tests.rs index a4b50ac..b8b827c 100644 --- a/src/types/value/value_tests.rs +++ b/src/types/value/value_tests.rs @@ -294,45 +294,65 @@ fn test_strip_prefix() { #[test] fn test_raw_string_literal() { assert_eq!( - Value::Literal("foo".into()).raw_string().unwrap(), + Value::Literal("foo".into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "foo".to_string() ); } #[test] fn test_raw_string_null() { - assert_eq!(Value::Null.raw_string().unwrap(), "None".to_string()); + assert_eq!( + Value::Null.raw_string(&RenderOpts::default()).unwrap(), + "null".to_string() + ); +} + +#[test] +fn test_raw_string_null_as_none() { + let opts = RenderOpts { + nested_reference_null_as_none: true, + ..RenderOpts::default() + }; + assert_eq!(Value::Null.raw_string(&opts).unwrap(), "None".to_string()); } #[test] fn test_raw_string_number() { assert_eq!( - Value::Number(5.into()).raw_string().unwrap(), + Value::Number(5.into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "5".to_string() ); assert_eq!( - Value::Number((-1).into()).raw_string().unwrap(), + Value::Number((-1).into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "-1".to_string() ); assert_eq!( - Value::Number(3.14.into()).raw_string().unwrap(), + Value::Number(3.14.into()) + .raw_string(&RenderOpts::default()) + .unwrap(), "3.14".to_string() ); assert_eq!( Value::Number(serde_yaml::Number::from(f64::INFINITY)) - .raw_string() + .raw_string(&RenderOpts::default()) .unwrap(), ".inf".to_string() ); assert_eq!( Value::Number(serde_yaml::Number::from(f64::NEG_INFINITY)) - .raw_string() + .raw_string(&RenderOpts::default()) .unwrap(), "-.inf".to_string() ); assert_eq!( Value::Number(serde_yaml::Number::from(f64::NAN)) - .raw_string() + .raw_string(&RenderOpts::default()) .unwrap(), ".nan".to_string() ); @@ -343,7 +363,7 @@ fn test_raw_string_mapping() { let mut m = Value::Mapping(Mapping::from_str("{foo: foo, bar: true, baz: 1.23}").unwrap()); // turn string values into literals by calling flatten m.render(&Mapping::new(), &RenderOpts::default()).unwrap(); - let mstr = m.raw_string().unwrap(); + let mstr = m.raw_string(&RenderOpts::default()).unwrap(); // NOTE(sg): serde_json output is sorted by keys assert_eq!(mstr, r#"{"bar":true,"baz":1.23,"foo":"foo"}"#); } @@ -351,7 +371,7 @@ fn test_raw_string_mapping() { #[test] fn test_raw_string_sequence() { let v = Value::Sequence(vec!["foo".into(), 3.14.into(), Value::Bool(true)]); - let vstr = v.raw_string().unwrap(); + let vstr = v.raw_string(&RenderOpts::default()).unwrap(); assert_eq!(vstr, r#"["foo",3.14,true]"#); } @@ -364,7 +384,7 @@ fn test_raw_string_mapping_nonstring_keys() { let m = Value::Mapping(m) .rendered(&Mapping::new(), &RenderOpts::default()) .unwrap(); - let mstr = m.raw_string().unwrap(); + let mstr = m.raw_string(&RenderOpts::default()).unwrap(); // NOTE(sg): serde_json output is sorted by keys assert_eq!(mstr, r#"{"3.14":true,"null":1.23,"true":"foo"}"#); } diff --git a/tests/inventory-reference-default-values/nodes/n12.yml b/tests/inventory-reference-default-values/nodes/n12.yml new file mode 100644 index 0000000..9a2e5fd --- /dev/null +++ b/tests/inventory-reference-default-values/nodes/n12.yml @@ -0,0 +1,13 @@ +parameters: + facts: + level: level + data: + "null": null + nonestr: None + bool: true + num: 1.25 + nullref: ${data:${facts:level}::${data:null}} + nulldefault: ${data:${facts:level}::~} + boolref: ${data:${facts:level}::${data:bool}} + numref: ${data:${facts:level}::${data:num}} + nonestrref: ${data:${facts:level}::${data:nonestr}} diff --git a/tests/test_inventory_reference_default_values.py b/tests/test_inventory_reference_default_values.py index 8c43129..e4d3e10 100644 --- a/tests/test_inventory_reference_default_values.py +++ b/tests/test_inventory_reference_default_values.py @@ -61,3 +61,35 @@ def test_inventory_reference_default_values(): n11 = inv["nodes"]["n11"]["parameters"] assert n11["text"] == n11["expected"] + + n12 = inv["nodes"]["n12"]["parameters"] + assert n12["nullref"] is None + assert n12["nulldefault"] is None + assert n12["boolref"] + assert n12["numref"] == 1.25 + assert n12["nonestrref"] == "None" + + +def test_inventory_reference_default_values_null_as_none(): + config_options = { + "nodes_uri": "nodes", + "classes_uri": "classes", + "reclass_rs_compat_flags": ["NestedReferenceNullAsNone"], + } + c = reclass_rs.Config.from_dict( + "./tests/inventory-reference-default-values", config_options + ) + assert c is not None + + r = reclass_rs.Reclass.from_config(c) + assert r is not None + + inv = r.inventory().as_dict() + n12 = inv["nodes"]["n12"]["parameters"] + # With NestedReferenceNullAsNone compat mode the nested ref to `data.null` is resolved as + # string "None" which isn't recognized as a YAML `null` by reclass-rs's default value handling. + assert n12["nullref"] == "None" + assert n12["nulldefault"] is None + assert n12["boolref"] + assert n12["numref"] == 1.25 + assert n12["nonestrref"] == "None"