From 034b2a2f4be62f08ced2ed34a4c3a3ab06b521c4 Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Tue, 19 Oct 2021 15:21:51 -0500 Subject: [PATCH] Add support for "any" stack `*` in StackId When a `Stack` struct has a `StackId` of `*` then it should have no mixins. This is validated by a "shadow" struct deserializing working from https://dev.to/equalma/validate-fields-and-types-in-serde-with-tryfrom-c2n. Based on the comment from https://github.com/serde-rs/serde/issues/939#issuecomment-939514114 In my own words it works like this: We create a duplicate struct and use serde's normal `derive(Deserialize)` on it. Then we specify that the `Stack` struct should be converted using the duplicate struct with `#[serde(try_from = "StackUnchecked")]. Finally we implement `try_from` and include our special logic that checks to see we don't have a `*` stack id AND any mixins. I also hard wrapped a few comments and put backpacks around special characters in the error output to make them clearer Close #104 --- CHANGELOG.md | 1 + src/data/buildpack.rs | 73 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 873b2a35..52959843 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- Stack id in `buildpack.toml` can now be `*` indicating "any" stack - LayerContentMetadata values (build, cache, launch) are now under a "types" key - Allow ProcessType to contain a dot (`.`) character diff --git a/src/data/buildpack.rs b/src/data/buildpack.rs index fd1541f6..825c578a 100644 --- a/src/data/buildpack.rs +++ b/src/data/buildpack.rs @@ -3,6 +3,7 @@ use lazy_static::lazy_static; use regex::Regex; use semver::Version; use serde::{de, Deserialize}; +use std::convert::TryFrom; use std::fmt::{Display, Formatter}; use std::{fmt, str::FromStr}; use thiserror; @@ -58,12 +59,37 @@ pub struct Buildpack { } #[derive(Deserialize, Debug)] +#[serde(try_from = "StackUnchecked")] pub struct Stack { pub id: StackId, + pub mixins: Vec, +} + +// Used as a "shadow" struct to store +// potentially invalid `Stack` data when deserializing +// https://dev.to/equalma/validate-fields-and-types-in-serde-with-tryfrom-c2n +#[derive(Deserialize)] +struct StackUnchecked { + pub id: StackId, + #[serde(default)] pub mixins: Vec, } +impl TryFrom for Stack { + type Error = BuildpackTomlError; + + fn try_from(value: StackUnchecked) -> Result { + let StackUnchecked { id, mixins } = value; + + if id.as_str() == "*" && !mixins.is_empty() { + Err(BuildpackTomlError::InvalidStarStack(mixins.join(", "))) + } else { + Ok(Stack { id, mixins }) + } + } +} + #[derive(Deserialize, Debug)] pub struct Order { group: Vec, @@ -150,7 +176,10 @@ impl<'de> de::Deserialize<'de> for BuildpackApi { } } -/// buildpack.toml Buildpack Id. This is a newtype wrapper around a String. It MUST only contain numbers, letters, and the characters ., /, and -. It also cannot be `config` or `app`. Use [`std::str::FromStr`] to create a new instance of this struct. +/// buildpack.toml Buildpack Id. This is a newtype wrapper around a String. +/// It MUST only contain numbers, letters, and the characters ., /, and -. +/// It also cannot be `config` or `app`. +/// Use [`std::str::FromStr`] to create a new instance of this struct. /// /// # Examples /// ``` @@ -189,7 +218,11 @@ impl BuildpackId { } } -/// buildpack.toml Stack Id. This is a newtype wrapper around a String. It MUST only contain numbers, letters, and the characters ., /, and -. Use [`std::str::FromStr`] to create a new instance of this struct. +/// buildpack.toml Stack Id. This is a newtype wrapper around a String. +/// It MUST only contain numbers, letters, and the characters ., /, and -. +/// or be `*`. +/// +/// Use [`std::str::FromStr`] to create a new instance of this struct. /// /// # Examples /// ``` @@ -201,6 +234,9 @@ impl BuildpackId { /// /// let invalid = StackId::from_str("!nvalid"); /// assert!(invalid.is_err()); +/// +/// let invalid = StackId::from_str("*"); +/// assert!(invalid.is_ok()); /// ``` #[derive(Deserialize, Debug)] @@ -211,7 +247,7 @@ impl FromStr for StackId { fn from_str(value: &str) -> Result { lazy_static! { - static ref RE: Regex = Regex::new(r"^[[:alnum:]./-]+$").unwrap(); + static ref RE: Regex = Regex::new(r"^([[:alnum:]./-]+|\*)$").unwrap(); } let string = String::from(value); @@ -231,15 +267,18 @@ impl StackId { #[derive(thiserror::Error, Debug)] pub enum BuildpackTomlError { - #[error("Found `{0}` but value MUST only contain numbers, letters, and the characters ., /, and -. Value MUST NOT be 'config' or 'app'.")] + #[error("Found `{0}` but value MUST only contain numbers, letters, and the characters `.`, `/`, and `-`. Value MUST NOT be 'config' or 'app'.")] InvalidBuildpackApi(String), + #[error("Stack with id `*` MUST not contain mixins. mixins: [{0}]")] + InvalidStarStack(String), + #[error( - "Found `{0}` but value MUST only contain numbers, letters, and the characters ., /, and -." + "Found `{0}` but value MUST only contain numbers, letters, and the characters `.`, `/`, and `-`. or only `*`" )] InvalidStackId(String), - #[error("Found `{0}` but value MUST only contain numbers, letters, and the characters ., /, and -. Value MUST NOT be 'config' or 'app'.")] + #[error("Found `{0}` but value MUST only contain numbers, letters, and the characters `.`, `/`, and `-`. Value MUST NOT be 'config' or 'app'.")] InvalidBuildpackId(String), } @@ -360,6 +399,28 @@ id = "io.buildpacks.stacks.bionic" } } + #[test] + fn cannot_use_star_stack_id_with_mixins() { + let raw = r#" +api = "0.4" + +[buildpack] +id = "foo/bar" +name = "Bar Buildpack" +version = "0.0.1" + +[[stacks]] +id = "*" +mixins = ["yolo"] + +[metadata] +checksum = "awesome" +"#; + + let result = toml::from_str::>(raw); + assert!(&result.is_err()); + } + #[test] fn buildpack_api_display() { assert_eq!(BuildpackApi { major: 1, minor: 0 }.to_string(), "1.0");