From 468759053a83248ca74f6e6c6ae7b5af4ad2d501 Mon Sep 17 00:00:00 2001 From: Tom Kirchner Date: Thu, 14 Nov 2019 15:50:11 -0800 Subject: [PATCH] Add modeled types for URLs and Kubernetes resources This improves errors when API requests are formatted or specified incorrectly, for example mixing up fields in user data. --- workspaces/Cargo.lock | 3 + workspaces/api/apiserver/Cargo.toml | 9 +- workspaces/api/apiserver/src/model.rs | 21 +- workspaces/api/apiserver/src/modeled_types.rs | 430 +++++++++++++++++- 4 files changed, 447 insertions(+), 16 deletions(-) diff --git a/workspaces/Cargo.lock b/workspaces/Cargo.lock index da4a43cdc6c..3717df62779 100644 --- a/workspaces/Cargo.lock +++ b/workspaces/Cargo.lock @@ -273,17 +273,20 @@ dependencies = [ "actix-web 1.0.8 (registry+https://github.com/rust-lang/crates.io-index)", "base64 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)", "cargo-readme 3.1.2 (registry+https://github.com/rust-lang/crates.io-index)", + "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.62 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "maplit 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", "nix 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", "percent-encoding 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "regex 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.101 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", "simplelog 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)", "snafu 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "systemd 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "toml 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", + "url 2.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "walkdir 2.2.9 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/workspaces/api/apiserver/Cargo.toml b/workspaces/api/apiserver/Cargo.toml index e9f7cc10adb..dad2355a001 100644 --- a/workspaces/api/apiserver/Cargo.toml +++ b/workspaces/api/apiserver/Cargo.toml @@ -9,17 +9,20 @@ build = "build.rs" [dependencies] actix-web = { version = "1.0.5", default-features = false, features = ["uds"] } base64 = "0.10" +lazy_static = "1.2" +libc = "0.2" log = "0.4" +nix = "0.15.0" percent-encoding = "2.1" +regex = "1.1" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" simplelog = "0.7" snafu = "0.5" +systemd = { version = "0.4.0", default-features = false, features = [], optional = true } toml = "0.5" +url = "2.1" walkdir = "2.2" -systemd = { version = "0.4.0", default-features = false, features = [], optional = true } -nix = "0.15.0" -libc = "0.2" [features] default = ["sd_notify"] diff --git a/workspaces/api/apiserver/src/model.rs b/workspaces/api/apiserver/src/model.rs index 3ae43d3fbcf..7c418e1c979 100644 --- a/workspaces/api/apiserver/src/model.rs +++ b/workspaces/api/apiserver/src/model.rs @@ -7,7 +7,10 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::net::Ipv4Addr; -use crate::modeled_types::{SingleLineString, ValidBase64, Identifier}; +use crate::modeled_types::{ + Identifier, KubernetesClusterName, KubernetesLabelKey, KubernetesLabelValue, + KubernetesTaintValue, SingleLineString, Url, ValidBase64, +}; ///// Primary user-visible settings @@ -47,19 +50,19 @@ pub struct KubernetesSettings { // Settings we require the user to specify, likely via user data. #[serde(skip_serializing_if = "Option::is_none")] - pub cluster_name: Option, + pub cluster_name: Option, #[serde(skip_serializing_if = "Option::is_none")] pub cluster_certificate: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub api_server: Option, + pub api_server: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub node_labels: Option>, + pub node_labels: Option>, #[serde(skip_serializing_if = "Option::is_none")] - pub node_taints: Option>, + pub node_taints: Option>, // Dynamic settings. @@ -82,10 +85,10 @@ pub struct KubernetesSettings { #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct UpdatesSettings { #[serde(skip_serializing_if = "Option::is_none")] - pub metadata_base_url: Option, + pub metadata_base_url: Option, #[serde(skip_serializing_if = "Option::is_none")] - pub target_base_url: Option, + pub target_base_url: Option, #[serde(skip_serializing_if = "Option::is_none")] pub seed: Option, @@ -95,7 +98,7 @@ pub struct UpdatesSettings { #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct ContainerImage { #[serde(skip_serializing_if = "Option::is_none")] - pub source: Option, + pub source: Option, #[serde(skip_serializing_if = "Option::is_none")] pub enabled: Option, @@ -109,7 +112,7 @@ pub struct ContainerImage { #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct NtpSettings { #[serde(skip_serializing_if = "Option::is_none")] - pub time_servers: Option>, + pub time_servers: Option>, } ///// Internal services diff --git a/workspaces/api/apiserver/src/modeled_types.rs b/workspaces/api/apiserver/src/modeled_types.rs index f85b162e5e8..3b6d05f600e 100644 --- a/workspaces/api/apiserver/src/modeled_types.rs +++ b/workspaces/api/apiserver/src/modeled_types.rs @@ -6,6 +6,8 @@ // necessary checks and returns the struct. Other traits that treat the struct like a string can // be implemented for you with the string_impls_for macro. +use lazy_static::lazy_static; +use regex::Regex; use serde::{Deserialize, Deserializer, Serialize, Serializer}; // Just need serde's Error in scope to get its trait methods use serde::de::Error as _; @@ -16,6 +18,7 @@ use std::fmt; use std::ops::Deref; pub mod error { + use regex::Regex; use snafu::Snafu; #[derive(Debug, Snafu)] @@ -32,6 +35,23 @@ pub mod error { input ))] InvalidIdentifier { input: String }, + + #[snafu(display("Given invalid URL '{}'", input))] + InvalidUrl { input: String }, + + #[snafu(display("{} must match '{}', given: {}", thing, pattern, input))] + Pattern { + thing: String, + pattern: Regex, + input: String, + }, + + // Some regexes are too big to usefully display in an error. + #[snafu(display("{} given invalid input: {}", thing, input))] + BigPattern { thing: String, input: String }, + + #[snafu(display("Given invalid cluster name '{}': {}", name, msg))] + InvalidClusterName { name: String, msg: String }, } } @@ -39,7 +59,7 @@ pub mod error { /// Pass the name of the type, and the name of the type in quotes (to be used in string error /// messages, etc.). macro_rules! string_impls_for { - ($for:ident, $for_str:expr) => ( + ($for:ident, $for_str:expr) => { impl TryFrom for $for { type Error = error::Error; @@ -54,8 +74,9 @@ macro_rules! string_impls_for { D: Deserializer<'de>, { let original = String::deserialize(deserializer)?; - Self::try_from(original) - .map_err(|e| D::Error::custom(format!("Unable to deserialize into {}: {}", $for_str, e))) + Self::try_from(original).map_err(|e| { + D::Error::custom(format!("Unable to deserialize into {}: {}", $for_str, e)) + }) } } @@ -106,7 +127,7 @@ macro_rules! string_impls_for { x.inner } } - ) + }; } // =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= @@ -282,3 +303,404 @@ mod test_valid_identifier { assert!(Identifier::try_from("💝").is_err()); } } + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + +/// Url represents a string that contains a valid URL, according to url::Url, though it also +/// allows URLs without a scheme (e.g. without "http://") because it's common. It stores the +/// original string and makes it accessible through standard traits. Its purpose is to validate +/// input for any field containing a network address. +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct Url { + inner: String, +} + +impl TryFrom<&str> for Url { + type Error = error::Error; + + fn try_from(input: &str) -> Result { + if let Ok(_) = input.parse::() { + return Ok(Url { + inner: input.to_string(), + }); + } else { + // It's very common to specify URLs without a scheme, so we add one and see if that + // fixes parsing. + let prefixed = format!("http://{}", input); + if let Ok(_) = prefixed.parse::() { + return Ok(Url { + inner: input.to_string(), + }); + } + } + error::InvalidUrl { input }.fail() + } +} + +string_impls_for!(Url, "Url"); + +#[cfg(test)] +mod test_url { + use super::Url; + use std::convert::TryFrom; + + #[test] + fn good_urls() { + for ok in &[ + "https://example.com/path", + "https://example.com", + "example.com/path", + "example.com", + "ntp://127.0.0.1/path", + "ntp://127.0.0.1", + "127.0.0.1/path", + "127.0.0.1", + "http://localhost/path", + "http://localhost", + "localhost/path", + "localhost", + ] { + Url::try_from(*ok).unwrap(); + } + } + + #[test] + fn bad_urls() { + for err in &[ + "how are you", + "weird@", + ] { + Url::try_from(*err).unwrap_err(); + } + } +} + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + +/// KubernetesName represents a string that contains a valid Kubernetes resource name. It stores +/// the original string and makes it accessible through standard traits. +// https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct KubernetesName { + inner: String, +} + +lazy_static! { + pub(crate) static ref KUBERNETES_NAME: Regex = Regex::new(r"^[0-9a-z.-]{1,253}$").unwrap(); +} + +impl TryFrom<&str> for KubernetesName { + type Error = error::Error; + + fn try_from(input: &str) -> Result { + ensure!( + KUBERNETES_NAME.is_match(input), + error::Pattern { + thing: "Kubernetes name", + pattern: KUBERNETES_NAME.clone(), + input + } + ); + Ok(KubernetesName { + inner: input.to_string(), + }) + } +} + +string_impls_for!(KubernetesName, "KubernetesName"); + +#[cfg(test)] +mod test_kubernetes_name { + use super::KubernetesName; + use std::convert::TryFrom; + + #[test] + fn good_names() { + for ok in &["howdy", "42", "18-eighteen."] { + KubernetesName::try_from(*ok).unwrap(); + } + } + + #[test] + fn bad_names() { + for err in &["", "HOWDY", "@", "hi/there", &"a".repeat(254)] { + KubernetesName::try_from(*err).unwrap_err(); + } + } +} + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + +/// KubernetesLabelKey represents a string that contains a valid Kubernetes label key. It stores +/// the original string and makes it accessible through standard traits. +// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct KubernetesLabelKey { + inner: String, +} + +lazy_static! { + pub(crate) static ref KUBERNETES_LABEL_KEY: Regex = Regex::new( + r"(?x)^ + ( # optional prefix + [[:alnum:].-]{1,253}/ # DNS label characters followed by slash + )? + [[:alnum:]] # at least one alphanumeric + ( + ([[:alnum:]._-]{0,61})? # more characters allowed in middle + [[:alnum:]] # have to end with alphanumeric + )? + $" + ) + .unwrap(); +} + +impl TryFrom<&str> for KubernetesLabelKey { + type Error = error::Error; + + fn try_from(input: &str) -> Result { + ensure!( + KUBERNETES_LABEL_KEY.is_match(input), + error::BigPattern { + thing: "Kubernetes label key", + input + } + ); + Ok(KubernetesLabelKey { + inner: input.to_string(), + }) + } +} + +string_impls_for!(KubernetesLabelKey, "KubernetesLabelKey"); + +#[cfg(test)] +mod test_kubernetes_label_key { + use super::KubernetesLabelKey; + use std::convert::TryFrom; + + #[test] + fn good_keys() { + for ok in &[ + "no-prefix", + "have.a/prefix", + "more-chars_here.now", + &"a".repeat(63), + &format!("{}/{}", "a".repeat(253), "name"), + ] { + KubernetesLabelKey::try_from(*ok).unwrap(); + } + } + + #[test] + fn bad_keys() { + for err in &[ + ".bad", + "bad.", + &"a".repeat(64), + &format!("{}/{}", "a".repeat(254), "name"), + ] { + KubernetesLabelKey::try_from(*err).unwrap_err(); + } + } +} + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + +/// KubernetesLabelValue represents a string that contains a valid Kubernetes label value. It +/// stores the original string and makes it accessible through standard traits. +// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct KubernetesLabelValue { + inner: String, +} + +lazy_static! { + pub(crate) static ref KUBERNETES_LABEL_VALUE: Regex = Regex::new( + r"(?x) + ^$ | # may be empty, or: + ^ + [[:alnum:]] # at least one alphanumeric + ( + ([[:alnum:]._-]{0,61})? # more characters allowed in middle + [[:alnum:]] # have to end with alphanumeric + )? + $ + " + ) + .unwrap(); +} + +impl TryFrom<&str> for KubernetesLabelValue { + type Error = error::Error; + + fn try_from(input: &str) -> Result { + ensure!( + KUBERNETES_LABEL_VALUE.is_match(input), + error::BigPattern { + thing: "Kubernetes label value", + input + } + ); + Ok(KubernetesLabelValue { + inner: input.to_string(), + }) + } +} + +string_impls_for!(KubernetesLabelValue, "KubernetesLabelValue"); + +#[cfg(test)] +mod test_kubernetes_label_value { + use super::KubernetesLabelValue; + use std::convert::TryFrom; + + #[test] + fn good_values() { + for ok in &["", "more-chars_here.now", &"a".repeat(63)] { + KubernetesLabelValue::try_from(*ok).unwrap(); + } + } + + #[test] + fn bad_values() { + for err in &[".bad", "bad.", &"a".repeat(64)] { + KubernetesLabelValue::try_from(*err).unwrap_err(); + } + } +} + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + +/// KubernetesTaintValue represents a string that contains a valid Kubernetes taint value, which is +/// like a label value, plus a colon, plus an "effect". It stores the original string and makes it +/// accessible through standard traits. +/// +/// Note: Kubelet won't launch if you specify an effect it doesn't know about, but we don't want to +/// gatekeep all possible values, so be careful. +// Note: couldn't find an exact spec for this. Cobbling things together, and guessing a bit as to +// the syntax of the effect. +// https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set +// https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct KubernetesTaintValue { + inner: String, +} + +lazy_static! { + pub(crate) static ref KUBERNETES_TAINT_VALUE: Regex = Regex::new( + r"(?x)^ + [[:alnum:]] # at least one alphanumeric + ( + ([[:alnum:]._-]{0,61})? # more characters allowed in middle + [[:alnum:]] # have to end with alphanumeric + )? + : # separate the label value from the effect + [[:alnum:]]{1,253} # effect + $" + ) + .unwrap(); +} + +impl TryFrom<&str> for KubernetesTaintValue { + type Error = error::Error; + + fn try_from(input: &str) -> Result { + ensure!( + KUBERNETES_TAINT_VALUE.is_match(input), + error::BigPattern { + thing: "Kubernetes taint value", + input + } + ); + Ok(KubernetesTaintValue { + inner: input.to_string(), + }) + } +} + +string_impls_for!(KubernetesTaintValue, "KubernetesTaintValue"); + +#[cfg(test)] +mod test_kubernetes_taint_value { + use super::KubernetesTaintValue; + use std::convert::TryFrom; + + #[test] + fn good_values() { + // All the examples from the docs linked above + for ok in &[ + "value:NoSchedule", + "value:PreferNoSchedule", + "value:NoExecute", + ] { + KubernetesTaintValue::try_from(*ok).unwrap(); + } + } + + #[test] + fn bad_values() { + for err in &[".bad", "bad.", &"a".repeat(254), "value:", ":effect"] { + KubernetesTaintValue::try_from(*err).unwrap_err(); + } + } +} + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + +/// KubernetesClusterName represents a string that contains a valid Kubernetes cluster name. It +/// stores the original string and makes it accessible through standard traits. +// Note: I was unable to find the rules for cluster naming. We know they have to fit into label +// values, because of the common cluster-name label, but they also can't be empty. This combines +// those two characteristics into a new type, until we find an explicit syntax. +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct KubernetesClusterName { + inner: String, +} + +impl TryFrom<&str> for KubernetesClusterName { + type Error = error::Error; + + fn try_from(input: &str) -> Result { + ensure!( + !input.is_empty(), + error::InvalidClusterName { + name: input, + msg: "must not be empty" + } + ); + ensure!( + KubernetesLabelValue::try_from(input).is_ok(), + error::InvalidClusterName { + name: input, + msg: "cluster names must be valid Kubernetes label values" + } + ); + + Ok(KubernetesClusterName { + inner: input.to_string(), + }) + } +} + +string_impls_for!(KubernetesClusterName, "KubernetesClusterName"); + +#[cfg(test)] +mod test_kubernetes_cluster_name { + use super::KubernetesClusterName; + use std::convert::TryFrom; + + #[test] + fn good_cluster_names() { + for ok in &["more-chars_here.now", &"a".repeat(63)] { + KubernetesClusterName::try_from(*ok).unwrap(); + } + } + + #[test] + fn bad_alues() { + for err in &["", ".bad", "bad.", &"a".repeat(64)] { + KubernetesClusterName::try_from(*err).unwrap_err(); + } + } +}