-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Valueless keys #12
base: master
Are you sure you want to change the base?
Valueless keys #12
Changes from all commits
f43de64
b3cae95
fe62450
269d68d
ddc591d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,13 +12,13 @@ use super::interpolation::{escape, RawOr}; | |
/// A file pointed to by an `env_file:` field. | ||
pub struct EnvFile { | ||
/// The variables found in our env file. | ||
vars: BTreeMap<String, String>, | ||
vars: BTreeMap<String, Option<String>>, | ||
} | ||
|
||
impl EnvFile { | ||
/// Read an `EnvFile` from a stream. | ||
pub fn read<R: io::Read>(input: R) -> Result<EnvFile> { | ||
let mut vars: BTreeMap<String, String> = BTreeMap::new(); | ||
let mut vars: BTreeMap<String, Option<String>> = BTreeMap::new(); | ||
let reader = io::BufReader::new(input); | ||
for line_result in reader.lines() { | ||
let line = line_result.chain_err(|| "I/O error")?; | ||
|
@@ -28,7 +28,7 @@ impl EnvFile { | |
Regex::new(r#"^\s*(:?#.*)?$"#).unwrap(); | ||
// We allow lowercase env vars even if POSIX doesn't. | ||
static ref VAR: Regex = | ||
Regex::new(r#"^([_A-Za-z][_A-Za-z0-9]*)=(.*)"#).unwrap(); | ||
Regex::new(r#"^([_A-Za-z][_A-Za-z0-9]*)(=(.*))?"#).unwrap(); | ||
} | ||
|
||
if BLANK.is_match(&line) { | ||
|
@@ -39,7 +39,7 @@ impl EnvFile { | |
.ok_or_else(|| ErrorKind::ParseEnv(line.clone()))?; | ||
vars.insert( | ||
caps.get(1).unwrap().as_str().to_owned(), | ||
caps.get(2).unwrap().as_str().to_owned(), | ||
caps.get(3).map(|v| v.as_str().to_owned()), | ||
); | ||
} | ||
Ok(EnvFile { vars: vars }) | ||
|
@@ -54,10 +54,13 @@ impl EnvFile { | |
|
||
/// Convert this `EnvFile` to the format we use for the `environment` | ||
/// member of `Service`. | ||
pub fn to_environment(&self) -> Result<BTreeMap<String, RawOr<String>>> { | ||
pub fn to_environment(&self) -> Result<BTreeMap<String, Option<RawOr<String>>>> { | ||
let mut env = BTreeMap::new(); | ||
for (k, v) in &self.vars { | ||
env.insert(k.to_owned(), escape(v)?); | ||
env.insert(k.to_owned(), match v.as_ref().map(|v| escape(v)) { | ||
None => None, | ||
Some(v) => Some(v?), | ||
}); | ||
} | ||
Ok(env) | ||
} | ||
|
@@ -79,6 +82,7 @@ fn parses_docker_compatible_env_files() { | |
# These are environment variables: | ||
FOO=foo | ||
BAR=2 | ||
BAZ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I can tell from the documentation, this is a non-standard extension to the
So unless we can find some evidence that Docker supports this extension, we should remove this code. |
||
|
||
# Docker does not currently do anything special with quotes! | ||
WEIRD="quoted" | ||
|
@@ -88,7 +92,8 @@ WEIRD="quoted" | |
let cursor = io::Cursor::new(input); | ||
let env_file = EnvFile::read(cursor).unwrap(); | ||
let env = env_file.to_environment().unwrap(); | ||
assert_eq!(env.get("FOO").unwrap().value().unwrap(), "foo"); | ||
assert_eq!(env.get("BAR").unwrap().value().unwrap(), "2"); | ||
assert_eq!(env.get("WEIRD").unwrap().value().unwrap(), "\"quoted\""); | ||
assert_eq!(env.get("FOO").unwrap().as_ref().unwrap().value().unwrap(), "foo"); | ||
assert_eq!(env.get("BAR").unwrap().as_ref().unwrap().value().unwrap(), "2"); | ||
assert_eq!(*env.get("BAZ").unwrap(), None); | ||
assert_eq!(env.get("WEIRD").unwrap().as_ref().unwrap().value().unwrap(), "\"quoted\""); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,7 +95,7 @@ impl<'de> Deserialize<'de> for ConvertToString { | |
/// | ||
/// ```text | ||
/// struct Example { | ||
/// #[serde(deserialize_with = "deserialize_hash_or_key_value_list")] | ||
/// #[serde(deserialize_with = "deserialize_map_or_key_value_list")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! |
||
/// pub args: BTreeMap<String, RawOr<String>>, | ||
/// } | ||
/// ``` | ||
|
@@ -165,7 +165,83 @@ where | |
} | ||
} | ||
|
||
deserializer.deserialize_map(MapOrKeyValueListVisitor) | ||
deserializer.deserialize_any(MapOrKeyValueListVisitor) | ||
} | ||
|
||
/// Like `deserialize_map_or_key_value_list`, but allowing missing values | ||
/// (e.g. for environment variables) | ||
pub fn deserialize_map_or_key_optional_value_list<'de, D>( | ||
deserializer: D, | ||
) -> Result<BTreeMap<String, Option<RawOr<String>>>, D::Error> | ||
where | ||
D: Deserializer<'de>, | ||
{ | ||
/// Declare an internal visitor type to handle our input. | ||
struct MapOrKeyOptionalValueListVisitor; | ||
|
||
impl<'de> Visitor<'de> for MapOrKeyOptionalValueListVisitor { | ||
type Value = BTreeMap<String, Option<RawOr<String>>>; | ||
|
||
// We have a real map. | ||
fn visit_map<V>(self, mut visitor: V) -> Result<Self::Value, V::Error> | ||
where | ||
V: MapAccess<'de>, | ||
{ | ||
let mut map: BTreeMap<String, Option<RawOr<String>>> = BTreeMap::new(); | ||
while let Some(key) = visitor.next_key::<String>()? { | ||
if map.contains_key(&key) { | ||
let msg = format!("duplicate map key: {}", &key); | ||
return Err(<V::Error as de::Error>::custom(msg)); | ||
} | ||
let ConvertToString(val) = visitor.next_value::<ConvertToString>()?; | ||
let raw_or_value = raw(val) | ||
.map_err(|e| <V::Error as de::Error>::custom(format!("{}", e)))?; | ||
map.insert(key, Some(raw_or_value)); | ||
} | ||
Ok(map) | ||
} | ||
|
||
// We have a key/value list. Yuck. | ||
fn visit_seq<V>(self, mut visitor: V) -> Result<Self::Value, V::Error> | ||
where | ||
V: SeqAccess<'de>, | ||
{ | ||
lazy_static! { | ||
// Match a key/value pair. | ||
static ref KEY_VALUE: Regex = | ||
Regex::new("^([^=]+)(=(.*))?$").unwrap(); | ||
} | ||
|
||
let mut map: BTreeMap<String, Option<RawOr<String>>> = BTreeMap::new(); | ||
while let Some(key_value) = visitor.next_element::<String>()? { | ||
let caps = KEY_VALUE.captures(&key_value).ok_or_else(|| { | ||
let msg = format!("expected KEY[=value], got: <{}>", &key_value); | ||
<V::Error as de::Error>::custom(msg) | ||
})?; | ||
let key = caps.get(1).unwrap().as_str(); | ||
let value = caps.get(3).map(|v| v.as_str()); | ||
if map.contains_key(key) { | ||
let msg = format!("duplicate map key: {}", key); | ||
return Err(<V::Error as de::Error>::custom(msg)); | ||
} | ||
let optional_raw_or_value = match value.map(|value| { | ||
raw(value.to_owned()) | ||
.map_err(|e| <V::Error as de::Error>::custom(format!("{}", e))) | ||
}) { | ||
None => None, | ||
Some(x) => Some(x?), | ||
}; | ||
map.insert(key.to_owned(), optional_raw_or_value); | ||
} | ||
Ok(map) | ||
} | ||
|
||
fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { | ||
write!(formatter, "a map or a key/[value] list") | ||
} | ||
} | ||
|
||
deserializer.deserialize_any(MapOrKeyOptionalValueListVisitor) | ||
} | ||
|
||
/// Given a map, deserialize it normally. But if we have a list of string | ||
|
@@ -211,7 +287,7 @@ where | |
} | ||
} | ||
|
||
deserializer.deserialize_map(MapOrDefaultListVisitor(PhantomData::<T>)) | ||
deserializer.deserialize_any(MapOrDefaultListVisitor(PhantomData::<T>)) | ||
} | ||
|
||
/// Deserialize either list or a single bare string as a list. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,8 +72,8 @@ pub struct Service { | |
|
||
/// Environment variables and values to supply to the container. | ||
#[serde(default, skip_serializing_if = "BTreeMap::is_empty", | ||
deserialize_with = "deserialize_map_or_key_value_list")] | ||
pub environment: BTreeMap<String, RawOr<String>>, | ||
deserialize_with = "deserialize_map_or_key_optional_value_list")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have we looked at all the other callers of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tested all of them with This raises some mildly interesting questions in cage, such as what to do if |
||
pub environment: BTreeMap<String, Option<RawOr<String>>>, | ||
|
||
/// Expose a list of ports to any containers that link to us. | ||
#[serde(default, skip_serializing_if = "Vec::is_empty")] | ||
|
@@ -98,8 +98,8 @@ pub struct Service { | |
/// Docker labels for this container, specifying various sorts of | ||
/// custom metadata. | ||
#[serde(default, skip_serializing_if = "BTreeMap::is_empty", | ||
deserialize_with = "deserialize_map_or_key_value_list")] | ||
pub labels: BTreeMap<String, RawOr<String>>, | ||
deserialize_with = "deserialize_map_or_key_optional_value_list")] | ||
pub labels: BTreeMap<String, Option<RawOr<String>>>, | ||
|
||
/// Links to other services in this file. | ||
#[serde(default, skip_serializing_if = "Vec::is_empty")] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, good, this is the main test that we need.