Skip to content

Commit

Permalink
Add support for "any" stack * in StackId
Browse files Browse the repository at this point in the history
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 serde-rs/serde#939 (comment)

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
  • Loading branch information
schneems committed Oct 20, 2021
1 parent 2ca9fff commit 034b2a2
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
73 changes: 67 additions & 6 deletions src/data/buildpack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -58,12 +59,37 @@ pub struct Buildpack {
}

#[derive(Deserialize, Debug)]
#[serde(try_from = "StackUnchecked")]
pub struct Stack {
pub id: StackId,
pub mixins: Vec<String>,
}

// 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<String>,
}

impl TryFrom<StackUnchecked> for Stack {
type Error = BuildpackTomlError;

fn try_from(value: StackUnchecked) -> Result<Self, Self::Error> {
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<Group>,
Expand Down Expand Up @@ -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
/// ```
Expand Down Expand Up @@ -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
/// ```
Expand All @@ -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)]
Expand All @@ -211,7 +247,7 @@ impl FromStr for StackId {

fn from_str(value: &str) -> Result<Self, Self::Err> {
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);
Expand All @@ -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),
}

Expand Down Expand Up @@ -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::<BuildpackToml<toml::value::Table>>(raw);
assert!(&result.is_err());
}

#[test]
fn buildpack_api_display() {
assert_eq!(BuildpackApi { major: 1, minor: 0 }.to_string(), "1.0");
Expand Down

0 comments on commit 034b2a2

Please sign in to comment.