Don't lower-case the keys when parsing config.yaml (#1038)

`struct DockerConfig` is set up so that its serde Deserializer expects a field
named `createOptions`. However the config crate's YAML parser normalizes
all keys to lower case. Even if the user sets `agent.config.createOptions` in
`config.yaml`, the serde Deserializer will see a key named `createoptions` and
discard it as unknown. The actual `create_options` field is then set to an
empty default.

This prevents users from having custom create options for the Edge Agent
container.

With this change, the code no longer use the `config::File` source to parse
`config.yaml`. Instead, it uses a new `YamlFileSource` that also impls
`config::Source`. This alternative source has similar code to `config::File`
but does not lower-case the field names.

Since the `config::Environment` source still lower-cases the keys that it
parses from environment variables, it does mean that it will not be able to
override keys in the config.yaml that are of mixed-case. For example,
`IOTEDGE_FOO` will override `foo` and not `Foo`. In practice,
all the top-level keys that could be easily overridden are completely
lower-case, so it is not likely to break any customers.

Fixes #1036

edgelet/edgelet-config/Cargo.toml

@@ -17,6 +17,7 @@ serde_json = "1.0"
 sha2 = "0.7.0"
 url = "1.7"
 url_serde = "0.2"
+yaml-rust = "0.4"
 
 edgelet-core = { path = "../edgelet-core" }
 edgelet-utils = { path = "../edgelet-utils" }

edgelet/edgelet-config/src/lib.rs

@@ -3,6 +3,7 @@
 #![deny(rust_2018_idioms, warnings)]
 #![deny(clippy::all, clippy::pedantic)]
 #![allow(
+    clippy::default_trait_access,
     clippy::doc_markdown, // clippy want the "IoT" of "IoT Hub" in a code fence
     clippy::module_name_repetitions,
     clippy::shadow_unrelated,
@@ -13,7 +14,7 @@ use std::fs::OpenOptions;
 use std::io::Read;
 use std::path::{Path, PathBuf};
 
-use config::{Config, Environment, File, FileFormat};
+use config::{Config, Environment};
 use failure::{Context, Fail};
 use log::{debug, Level};
 use regex::Regex;
@@ -27,6 +28,9 @@ use edgelet_core::crypto::MemoryKey;
 use edgelet_core::ModuleSpec;
 use edgelet_utils::log_failure;
 
+mod yaml_file_source;
+use yaml_file_source::YamlFileSource;
+
 /// This is the name of the network created by the iotedged
 const DEFAULT_NETWORKID: &str = "azure-iot-edge";
 
@@ -264,9 +268,9 @@ where
             })
         });
         let mut config = Config::default();
-        config.merge(File::from_str(DEFAULTS, FileFormat::Yaml))?;
+        config.merge(YamlFileSource::String(DEFAULTS))?;
         if let Some(file) = filename {
-            config.merge(File::with_name(file).required(true))?;
+            config.merge(YamlFileSource::File(file.into()))?;
         }
 
         config.merge(Environment::with_prefix("iotedge"))?;
@@ -406,6 +410,8 @@ mod tests {
     static GOOD_SETTINGS_TG: &str = "test/linux/sample_settings.tg.yaml";
     #[cfg(unix)]
     static GOOD_SETTINGS_DPS_SYM_KEY: &str = "test/linux/sample_settings.dps.sym.yaml";
+    #[cfg(unix)]
+    static GOOD_SETTINGS_CASE_SENSITIVE: &str = "test/linux/case_sensitive.yaml";
 
     #[cfg(windows)]
     static GOOD_SETTINGS: &str = "test/windows/sample_settings.yaml";
@@ -419,6 +425,8 @@ mod tests {
     static GOOD_SETTINGS_TG: &str = "test/windows/sample_settings.tg.yaml";
     #[cfg(windows)]
     static GOOD_SETTINGS_DPS_SYM_KEY: &str = "test/windows/sample_settings.dps.sym.yaml";
+    #[cfg(windows)]
+    static GOOD_SETTINGS_CASE_SENSITIVE: &str = "test/windows/case_sensitive.yaml";
 
     fn unwrap_manual_provisioning(p: &Provisioning) -> String {
         match p {
@@ -573,4 +581,17 @@ mod tests {
         };
         assert_eq!("some-network", moby2.network());
     }
+
+    #[test]
+    fn case_of_names_of_keys_is_preserved() {
+        let settings =
+            Settings::<DockerConfig>::new(Some(Path::new(GOOD_SETTINGS_CASE_SENSITIVE))).unwrap();
+
+        let env = settings.agent().env();
+        assert_eq!(env.get("AbC").map(AsRef::as_ref), Some("VAluE1"));
+        assert_eq!(env.get("DeF").map(AsRef::as_ref), Some("VAluE2"));
+
+        let create_options = settings.agent().config().create_options();
+        assert_eq!(create_options.hostname(), Some("VAluE3"));
+    }
 }

edgelet/edgelet-config/src/yaml_file_source.rs

@@ -0,0 +1,152 @@
+use std::borrow::Cow;
+use std::collections::HashMap;
+use std::fs::File;
+use std::io::Read;
+use std::path::PathBuf;
+
+use config::{ConfigError, Source, Value};
+use yaml_rust::{Yaml, YamlLoader};
+
+/// This is similar to [`config::File`] in that it parses a YAML string or file and implements `config::Source`.
+///
+/// We use this to parse our config.yaml instead of `config::File` because `config::File` lower-cases all field names that it reads.
+/// This causes issues with fields like `agent.config.createOptions` since the config crate returns `agent.config.createoptions`
+/// which the serde deserializer ignores.
+#[derive(Clone, Debug)]
+pub(crate) enum YamlFileSource {
+    File(PathBuf),
+    String(&'static str),
+}
+
+impl Source for YamlFileSource {
+    fn clone_into_box(&self) -> Box<dyn Source + Send + Sync> {
+        Box::new(self.clone())
+    }
+
+    fn collect(&self) -> Result<HashMap<String, Value>, ConfigError> {
+        let origin = match self {
+            YamlFileSource::File(path) => Some(path.to_string_lossy().into_owned()),
+            YamlFileSource::String(_) => None,
+        };
+
+        let contents = match self {
+            YamlFileSource::File(path) => {
+                let mut file =
+                    File::open(path).map_err(|err| ConfigError::Foreign(Box::new(err)))?;
+                let mut contents = String::new();
+                let _ = file
+                    .read_to_string(&mut contents)
+                    .map_err(|err| ConfigError::Foreign(Box::new(err)))?;
+                Cow::Owned(contents)
+            }
+
+            YamlFileSource::String(s) => Cow::Borrowed(*s),
+        };
+
+        let docs = YamlLoader::load_from_str(&*contents)
+            .map_err(|err| ConfigError::Foreign(Box::new(err)))?;
+
+        let mut docs = docs.into_iter();
+        let doc = match docs.next() {
+            Some(doc) => {
+                if docs.next().is_some() {
+                    return Err(ConfigError::Foreign(Box::new(
+                        YamlFileSourceError::MoreThanOneDocument,
+                    )));
+                }
+
+                doc
+            }
+
+            None => Yaml::Hash(Default::default()),
+        };
+
+        let mut result = HashMap::new();
+
+        if let Yaml::Hash(hash) = doc {
+            for (key, value) in hash {
+                if let Yaml::String(key) = key {
+                    result.insert(key, from_yaml_value(origin.as_ref(), value)?);
+                }
+            }
+        }
+
+        Ok(result)
+    }
+}
+
+/// Identical to https://github.com/mehcode/config-rs/blob/0.8.0/src/file/format/yaml.rs#L32-L68
+/// except that it does not lower-case hash keys.
+///
+/// Unfortunately the `ValueKind` enum used by the `Value` constructor is not exported from the crate.
+/// It does however impl `From` for the various corresponding standard types, so this code uses those.
+/// The only difference is the fallback `_` case at the end.
+fn from_yaml_value(uri: Option<&String>, value: Yaml) -> Result<Value, ConfigError> {
+    match value {
+        Yaml::String(value) => Ok(Value::new(uri, value)),
+        Yaml::Real(value) => {
+            // TODO: Figure out in what cases this can fail?
+            Ok(Value::new(
+                uri,
+                value
+                    .parse::<f64>()
+                    .map_err(|err| ConfigError::Foreign(Box::new(err)))?,
+            ))
+        }
+        Yaml::Integer(value) => Ok(Value::new(uri, value)),
+        Yaml::Boolean(value) => Ok(Value::new(uri, value)),
+        Yaml::Hash(table) => {
+            let mut m = HashMap::new();
+            for (key, value) in table {
+                if let Yaml::String(key) = key {
+                    m.insert(key, from_yaml_value(uri, value)?);
+                }
+                // TODO: should we do anything for non-string keys?
+            }
+            Ok(Value::new(uri, m))
+        }
+        Yaml::Array(array) => {
+            let mut l = vec![];
+
+            for value in array {
+                l.push(from_yaml_value(uri, value)?);
+            }
+
+            Ok(Value::new(uri, l))
+        }
+
+        // 1. Yaml NULL
+        // 2. BadValue – It shouldn't be possible to hit BadValue as this only happens when
+        //               using the index trait badly or on a type error but we send back nil.
+        // 3. Alias – No idea what to do with this and there is a note in the lib that its
+        //            not fully supported yet anyway
+        //
+        // The original function returns `Value::new(uri, ValueKind::Nil)` here.
+        // Since `ValueKind` is private, we have to return Err instead. It shouldn't be a problem for our use case
+        // since we don't expect null / bad value / alias.
+        value => Err(ConfigError::Foreign(Box::new(
+            YamlFileSourceError::UnrecognizedYamlValue(value),
+        ))),
+    }
+}
+
+#[derive(Debug)]
+enum YamlFileSourceError {
+    MoreThanOneDocument,
+    UnrecognizedYamlValue(Yaml),
+}
+
+impl std::fmt::Display for YamlFileSourceError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            YamlFileSourceError::MoreThanOneDocument => {
+                write!(f, "more than one YAML document provided")
+            }
+            YamlFileSourceError::UnrecognizedYamlValue(value) => {
+                write!(f, "unrecognized YAML value {:?}", value)
+            }
+        }
+    }
+}
+
+impl std::error::Error for YamlFileSourceError {}

edgelet/edgelet-config/test/linux/case_sensitive.yaml

@@ -0,0 +1,29 @@
+provisioning:
+  source: "manual"
+  device_connection_string: "HostName=something.something.com;DeviceId=something;SharedAccessKey=QXp1cmUgSW9UIEVkZ2U="
+agent:
+  name: "edgeAgent"
+  type: "docker"
+  env:
+    AbC: "VAluE1"
+    DeF: "VAluE2"
+  config:
+    image: "microsoft/azureiotedge-agent:1.0"
+    auth: {}
+    createOptions:
+      Hostname: VAluE3
+hostname: "localhost"
+
+connect:
+  workload_uri: "http://localhost:8081"
+  management_uri: "http://localhost:8080"
+
+listen:
+  workload_uri: "http://0.0.0.0:8081"
+  management_uri: "http://0.0.0.0:8080"
+
+homedir: "/tmp"
+
+moby_runtime:
+  uri: "http://localhost:2375"
+  network: "azure-iot-edge"

edgelet/edgelet-config/test/windows/case_sensitive.yaml

@@ -0,0 +1,29 @@
+provisioning:
+  source: "manual"
+  device_connection_string: "HostName=something.something.com;DeviceId=something;SharedAccessKey=QXp1cmUgSW9UIEVkZ2U="
+agent:
+  name: "edgeAgent"
+  type: "docker"
+  env:
+    AbC: "VAluE1"
+    DeF: "VAluE2"
+  config:
+    image: "microsoft/azureiotedge-agent:1.0"
+    auth: {}
+    createOptions:
+      Hostname: VAluE3
+hostname: "localhost"
+
+connect:
+  workload_uri: "http://localhost:8081"
+  management_uri: "http://localhost:8080"
+
+listen:
+  workload_uri: "http://0.0.0.0:8081"
+  management_uri: "http://0.0.0.0:8080"
+
+homedir: "C:\\Temp"
+
+moby_runtime:
+  uri: "npipe://./pipe/iotedge_moby_engine"
+  network: "azure-iot-edge"