Serialize YAML null as string "null" in nested references

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`.

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`

python/reclass_rs/reclass_rs.pyi

@@ -15,6 +15,7 @@ __all__: list[str] = [
 @final
 class CompatFlag(Enum):
     ComposeNodeNameLiteralDots = "ComposeNodeNameLiteralDots"
+    NestedReferenceNullAsNone = "NestedReferenceNullAsNone"
 
 @final
 class Config:

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()
         }
     }

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.

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)
 }

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

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<String> {
+    pub(crate) fn raw_string(&self, opts: &RenderOpts) -> Result<String> {
         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 {

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();
 }
 

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,15 +363,15 @@ 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"}"#);
 }
 
 #[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"}"#);
 }

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}}

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"