Merge pull request #2226 from habitat-sh/serialization-bugs

Approved by: @nobody from Nowhere
Merged by: The Sentinels

components/butterfly/src/member.rs

@@ -20,6 +20,7 @@ use std::iter::IntoIterator;
 use std::net::SocketAddr;
 use std::ops::{Deref, DerefMut};
 use std::result;
+use std::str::FromStr;
 use std::sync::{Arc, RwLock};
 use std::sync::atomic::{AtomicUsize, Ordering};
 
@@ -30,6 +31,7 @@ use uuid::Uuid;
 use serde::{Serialize, Serializer};
 use serde::ser::SerializeStruct;
 
+use error::Error;
 use message::swim::{Member as ProtoMember, Membership as ProtoMembership,
                     Membership_Health as ProtoMembership_Health, Rumor_Type};
 use rumor::RumorKey;
@@ -38,13 +40,32 @@ use rumor::RumorKey;
 const PINGREQ_TARGETS: usize = 5;
 
 /// The health of a node.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 pub enum Health {
     Alive,
     Suspect,
     Confirmed,
 }
 
+impl Default for Health {
+    fn default() -> Health {
+        Health::Alive
+    }
+}
+
+impl FromStr for Health {
+    type Err = Error;
+
+    fn from_str(value: &str) -> Result<Self, Self::Err> {
+        match value.to_lowercase().as_ref() {
+            "alive" => Ok(Health::Alive),
+            "suspect" => Ok(Health::Suspect),
+            "confirmed" => Ok(Health::Confirmed),
+            _ => Ok(Health::Alive),
+        }
+    }
+}
+
 impl From<i32> for Health {
     fn from(value: i32) -> Health {
         Self::from(ProtoMembership_Health::from_i32(value).unwrap_or(ProtoMembership_Health::ALIVE))
@@ -82,13 +103,15 @@ impl<'a> From<&'a Health> for ProtoMembership_Health {
     }
 }
 
+
 impl fmt::Display for Health {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            &Health::Alive => write!(f, "Alive"),
-            &Health::Suspect => write!(f, "Suspect"),
-            &Health::Confirmed => write!(f, "Confirmed"),
-        }
+        let value = match *self {
+            Health::Alive => "alive",
+            Health::Suspect => "suspect",
+            Health::Confirmed => "confirmed",
+        };
+        write!(f, "{}", value)
     }
 }
 

components/butterfly/src/rumor/service.rs

@@ -18,6 +18,7 @@
 
 use std::cmp::Ordering;
 use std::mem;
+use std::net::{IpAddr, Ipv4Addr};
 use std::ops::{Deref, DerefMut};
 
 use habitat_core::service::ServiceGroup;
@@ -151,18 +152,27 @@ impl Rumor for Service {
     }
 }
 
-#[derive(Debug, Default, Clone, PartialEq, Deserialize, Serialize)]
+#[derive(Debug, Clone, PartialEq, Deserialize, Serialize)]
 pub struct SysInfo {
-    pub ip: String,
+    pub ip: IpAddr,
     pub hostname: String,
-    pub gossip_ip: String,
-    // TODO: revert to u16 when deserializing in the handlebars template
-    // works properly
-    pub gossip_port: String,
-    pub http_gateway_ip: String,
-    // TODO: revert to u16 when deserializing in the handlebars template
-    // works properly
-    pub http_gateway_port: String,
+    pub gossip_ip: IpAddr,
+    pub gossip_port: u16,
+    pub http_gateway_ip: IpAddr,
+    pub http_gateway_port: u16,
+}
+
+impl Default for SysInfo {
+    fn default() -> SysInfo {
+        SysInfo {
+            ip: IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
+            hostname: String::from("localhost"),
+            gossip_ip: IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
+            gossip_port: 0,
+            http_gateway_ip: IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
+            http_gateway_port: 0,
+        }
+    }
 }
 
 #[cfg(test)]

components/butterfly/src/server/mod.rs

@@ -93,7 +93,7 @@ impl Server {
     /// `Trace` struct, a ring_key if you want encryption on the wire, and an optional server name.
     pub fn new<T, U, P>(swim_addr: T,
                         gossip_addr: U,
-                        member: Member,
+                        mut member: Member,
                         trace: Trace,
                         ring_key: Option<SymKey>,
                         name: Option<String>,
@@ -109,6 +109,8 @@ impl Server {
 
         match (maybe_swim_socket_addr, maybe_gossip_socket_addr) {
             (Ok(Some(swim_socket_addr)), Ok(Some(gossip_socket_addr))) => {
+                member.set_swim_port(swim_socket_addr.port() as i32);
+                member.set_gossip_port(gossip_socket_addr.port() as i32);
                 Ok(Server {
                        name: Arc::new(name.unwrap_or(String::from(member.get_id()))),
                        member_id: Arc::new(String::from(member.get_id())),

components/core/Cargo.toml

@@ -15,13 +15,15 @@ hex = "*"
 lazy_static = "*"
 libarchive = "*"
 libc = "*"
-libsodium-sys = "*"
+# JW: Temporarily use master branch of git until serde 1.0+ is available in crate release
+libsodium-sys = { git = "https://github.com/dnaq/sodiumoxide" }
 log = "*"
 regex = "*"
 serde = "*"
 serde_derive = "*"
 serde_json = "*"
-sodiumoxide = "*"
+# JW: Temporarily use master branch of git until serde 1.0+ is available in crate release
+sodiumoxide = { git = "https://github.com/dnaq/sodiumoxide" }
 time = "*"
 toml = { version = "*", features = ["serde"], default-features = false }
 url = "*"

components/core/src/config.rs

@@ -28,12 +28,18 @@ pub trait ConfigFile: DeserializeOwned + Sized {
     fn from_file<T: AsRef<Path>>(filepath: T) -> Result<Self, Self::Error> {
         let mut file = match File::open(filepath.as_ref()) {
             Ok(f) => f,
-            Err(e) => return Err(Self::Error::from(Error::ConfigFileIO(e))),
+            Err(e) => {
+                return Err(Self::Error::from(Error::ConfigFileIO(filepath.as_ref().to_path_buf(),
+                                                                 e)))
+            }
         };
         let mut raw = String::new();
         match file.read_to_string(&mut raw) {
             Ok(_) => (),
-            Err(e) => return Err(Self::Error::from(Error::ConfigFileIO(e))),
+            Err(e) => {
+                return Err(Self::Error::from(Error::ConfigFileIO(filepath.as_ref().to_path_buf(),
+                                                                 e)))
+            }
         }
         Self::from_raw(&raw)
     }

components/core/src/crypto/artifact.rs

@@ -23,14 +23,15 @@ use sodiumoxide::crypto::sign;
 
 use error::{Error, Result};
 use super::{HART_FORMAT_VERSION, SIG_HASH_TYPE, SigKeyPair};
+use super::hash;
 use super::keys::parse_name_with_rev;
 
 /// Generate and sign a package
 pub fn sign<P1: ?Sized, P2: ?Sized>(src: &P1, dst: &P2, pair: &SigKeyPair) -> Result<()>
     where P1: AsRef<Path>,
           P2: AsRef<Path>
 {
-    let hash = try!(super::hash::hash_file(&src));
+    let hash = hash::hash_file(&src)?;
     debug!("File hash for {} = {}", src.as_ref().display(), &hash);
 
     let signature = sign::sign(&hash.as_bytes(), try!(pair.secret()));
@@ -212,7 +213,7 @@ pub fn verify<P1: ?Sized, P2: ?Sized>(src: &P1, cache_key_path: &P2) -> Result<(
         }
         Err(_) => return Err(Error::CryptoError("Verification failed".to_string())),
     };
-    let computed_hash = try!(super::hash::hash_reader(&mut reader));
+    let computed_hash = hash::hash_reader(&mut reader)?;
     if computed_hash == expected_hash {
         Ok((pair.name_with_rev(), expected_hash))
     } else {

components/core/src/crypto/hash.rs

@@ -29,12 +29,12 @@ const BUF_SIZE: usize = 1024;
 /// digest size = 32 BYTES
 /// NOTE: the hashing is keyless
 pub fn hash_file<P: AsRef<Path>>(filename: &P) -> Result<String> {
-    let file = try!(File::open(filename.as_ref()));
+    let file = File::open(filename.as_ref())?;
     let mut reader = BufReader::new(file);
     hash_reader(&mut reader)
 }
 
-pub fn hash_string(data: &str) -> Result<String> {
+pub fn hash_string(data: &str) -> String {
     let mut out = [0u8; libsodium_sys::crypto_generichash_BYTES];
     let mut st = vec![0u8; (unsafe { libsodium_sys::crypto_generichash_statebytes() })];
     let pst =
@@ -47,10 +47,10 @@ pub fn hash_string(data: &str) -> Result<String> {
         libsodium_sys::crypto_generichash_update(pst, data[..].as_ptr(), data.len() as u64);
         libsodium_sys::crypto_generichash_final(pst, out.as_mut_ptr(), out.len());
     }
-    Ok(out.to_hex())
+    out.to_hex()
 }
 
-pub fn hash_bytes(data: &[u8]) -> Result<String> {
+pub fn hash_bytes(data: &[u8]) -> String {
     let mut out = [0u8; libsodium_sys::crypto_generichash_BYTES];
     let mut st = vec![0u8; (unsafe { libsodium_sys::crypto_generichash_statebytes() })];
     let pst =
@@ -63,7 +63,7 @@ pub fn hash_bytes(data: &[u8]) -> Result<String> {
         libsodium_sys::crypto_generichash_update(pst, data[..].as_ptr(), data.len() as u64);
         libsodium_sys::crypto_generichash_final(pst, out.as_mut_ptr(), out.len());
     }
-    Ok(out.to_hex())
+    out.to_hex()
 }
 
 pub fn hash_reader(reader: &mut BufReader<File>) -> Result<String> {
@@ -74,11 +74,9 @@ pub fn hash_reader(reader: &mut BufReader<File>) -> Result<String> {
             mem::transmute::<*mut u8,
                              *mut libsodium_sys::crypto_generichash_state>(st.as_mut_ptr())
         };
-
     unsafe {
         libsodium_sys::crypto_generichash_init(pst, ptr::null_mut(), 0, out.len());
     }
-
     let mut buf = [0u8; BUF_SIZE];
     loop {
         let bytes_read = try!(reader.read(&mut buf));
@@ -173,7 +171,7 @@ mod test {
             }
             file
         };
-        let computed = hash_file(&dst).unwrap();
+        let computed = hash_file(&dst);
         let expected = "ba640dc063f0ed27e60b38dbb7cf19778cf7805d9fc91eb129fb68b409d46414";
         assert_eq!(computed, expected);
     }

components/core/src/error.rs

@@ -16,6 +16,7 @@ use std::error;
 use std::io;
 use std::fmt;
 use std::num;
+use std::path::PathBuf;
 use std::result;
 use std::str;
 use std::string;
@@ -36,7 +37,7 @@ pub enum Error {
     /// An invalid path to a keyfile was given.
     BadKeyPath(String),
     /// Error reading raw contents of configuration file.
-    ConfigFileIO(io::Error),
+    ConfigFileIO(PathBuf, io::Error),
     /// Parsing error while reading a configuration file.
     ConfigFileSyntax(toml::de::Error),
     /// Expected an array of socket addrs for configuration field value.
@@ -138,7 +139,9 @@ impl fmt::Display for Error {
                 format!("Invalid keypath: {}. Specify an absolute path to a file on disk.",
                         e)
             }
-            Error::ConfigFileIO(ref e) => format!("Error reading configuration file: {}", e),
+            Error::ConfigFileIO(ref f, ref e) => {
+                format!("Error reading configuration file, {}, {}", f.display(), e)
+            }
             Error::ConfigFileSyntax(ref e) => {
                 format!("Syntax errors while parsing TOML configuration file:\n\n{}",
                         e)
@@ -266,7 +269,7 @@ impl error::Error for Error {
         match *self {
             Error::ArchiveError(ref err) => err.description(),
             Error::BadKeyPath(_) => "An absolute path to a file on disk is required",
-            Error::ConfigFileIO(_) => "Unable to read the raw contents of a configuration file",
+            Error::ConfigFileIO(_, _) => "Unable to read the raw contents of a configuration file",
             Error::ConfigFileSyntax(_) => "Error parsing contents of configuration file",
             Error::ConfigInvalidArraySocketAddr(_) => {
                 "Invalid array value of network address pair strings encountered while parsing a \

components/sup/Cargo.toml

@@ -41,7 +41,7 @@ protobuf = "*"
 rand = "*"
 regex = "*"
 router = "*"
-serde = "*"
+serde = { version = "*", features = ["rc"] }
 serde_derive = "*"
 serde_json = "*"
 tempdir = "*"