diff --git a/CHANGELOG.md b/CHANGELOG.md index b5830bab7c..ddedfcef00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ## DFX +### feat(frontend-canister): add warning if config is provided in `.ic-assets.json` but not used + ### fix(frontend-canister): Allow overwriting default HTTP Headers for assets in frontend canister Allows to overwrite `Content-Type`, `Content-Encoding`, and `Cache-Control` HTTP headers with custom values via `.ic-assets.json5` config file. Example `.ic-assets.json5` file: diff --git a/e2e/tests-dfx/assetscanister.bash b/e2e/tests-dfx/assetscanister.bash index 15e55becc6..cddde95e67 100644 --- a/e2e/tests-dfx/assetscanister.bash +++ b/e2e/tests-dfx/assetscanister.bash @@ -539,3 +539,79 @@ CHERRIES" "$stdout" assert_not_match "etag: my-etag" assert_match "etag: \"[a-z0-9]{64}\"" } + +@test "asset configuration via .ic-assets.json5 - detect unused config" { + install_asset assetscanister + + dfx_start + + mkdir src/e2e_project_frontend/assets/somedir + touch src/e2e_project_frontend/assets/somedir/upload-me.txt + echo '[ + { + "match": "nevermatchme", + "cache": { "max_age": 2000 } + } + ]' > src/e2e_project_frontend/assets/.ic-assets.json5 + echo '[ + { + "match": "upload-me.txt", + "headers": { "key": "value" } + }, + { + "match": "nevermatchme", + "headers": {}, + "ignore": false + }, + { + "match": "nevermatchmetoo", + "headers": null, + "ignore": false + }, + { + "match": "non-matcher", + "headers": {"x-header": "x-value"}, + "ignore": false + }, + { + "match": "/thanks-for-not-stripping-forward-slash", + "headers": {"x-header": "x-value"}, + "ignore": false + } + ]' > src/e2e_project_frontend/assets/somedir/.ic-assets.json5 + + assert_command dfx deploy + assert_match 'WARNING: 1 unmatched configuration in .*/src/e2e_project_frontend/assets/.ic-assets.json config file:' + assert_contains '{ + "match": "nevermatchme", + "cache": { + "max_age": 2000 + } +}' + assert_match 'WARNING: 4 unmatched configurations in .*/src/e2e_project_frontend/assets/somedir/.ic-assets.json config file:' + assert_contains '{ + "match": "nevermatchme", + "headers": {}, + "ignore": false +} +{ + "match": "nevermatchmetoo", + "headers": {}, + "ignore": false +} +{ + "match": "non-matcher", + "headers": { + "x-header": "x-value" + }, + "ignore": false +}' + # splitting this up into two checks, because the order is different on macos vs ubuntu + assert_contains '{ + "match": "/thanks-for-not-stripping-forward-slash", + "headers": { + "x-header": "x-value" + }, + "ignore": false +}' +} diff --git a/src/canisters/frontend/ic-asset/src/asset_config.rs b/src/canisters/frontend/ic-asset/src/asset_config.rs index d4265136dd..1b4d0e5813 100644 --- a/src/canisters/frontend/ic-asset/src/asset_config.rs +++ b/src/canisters/frontend/ic-asset/src/asset_config.rs @@ -1,49 +1,66 @@ use anyhow::{bail, Context}; use derivative::Derivative; -use globset::{Glob, GlobMatcher}; +use globset::GlobMatcher; use serde::{Deserialize, Serialize}; -use serde_json::Value; use std::{ collections::HashMap, fs, path::{Path, PathBuf}, - sync::Arc, + sync::{Arc, Mutex}, }; pub(crate) const ASSETS_CONFIG_FILENAME_JSON: &str = ".ic-assets.json"; pub(crate) const ASSETS_CONFIG_FILENAME_JSON5: &str = ".ic-assets.json5"; +/// A final piece of metadata assigned to the asset +#[derive(Debug, Default, PartialEq, Eq, Serialize, Clone)] +pub struct AssetConfig { + pub(crate) cache: Option, + pub(crate) headers: Option, + pub(crate) ignore: Option, +} + pub(crate) type HeadersConfig = HashMap; -type ConfigMap = HashMap>; #[derive(Deserialize, Serialize, Debug, Default, Clone, PartialEq, Eq)] pub(crate) struct CacheConfig { pub(crate) max_age: Option, } -#[derive(Derivative)] -#[derivative(Debug)] -struct AssetConfigRule { - #[derivative(Debug(format_with = "fmt_glob_field"))] +/// A single configuration object, from `.ic-assets.json` config file +#[derive(Derivative, Clone, Serialize)] +#[derivative(Debug, PartialEq)] +pub struct AssetConfigRule { + #[derivative( + Debug(format_with = "rule_utils::glob_fmt"), + PartialEq(compare_with = "rule_utils::glob_cmp") + )] + #[serde(serialize_with = "rule_utils::glob_serialize")] r#match: GlobMatcher, + #[serde(skip_serializing_if = "Option::is_none")] cache: Option, + #[serde( + serialize_with = "rule_utils::headers_serialize", + skip_serializing_if = "Maybe::is_absent" + )] headers: Maybe, + #[serde(skip_serializing_if = "Option::is_none")] ignore: Option, + #[serde(skip_serializing)] + used: bool, } -#[derive(Deserialize, Debug)] +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] enum Maybe { Null, Absent, Value(T), } -fn fmt_glob_field( - field: &GlobMatcher, - formatter: &mut std::fmt::Formatter, -) -> Result<(), std::fmt::Error> { - formatter.write_str(field.glob().glob())?; - Ok(()) +impl Default for Maybe { + fn default() -> Self { + Self::Absent + } } impl AssetConfigRule { @@ -54,27 +71,28 @@ impl AssetConfigRule { } } +type ConfigNode = Arc>; +type ConfigMap = HashMap; + +/// The main public interface for aggregating `.ic-assets.json` files +/// nested in directories. Each sub/directory will be represented +/// as `AssetConfigTreeNode`. #[derive(Debug)] -pub(crate) struct AssetSourceDirectoryConfiguration { +pub struct AssetSourceDirectoryConfiguration { config_map: ConfigMap, } -#[derive(Debug, Default, PartialEq, Eq, Serialize, Clone)] -pub(crate) struct AssetConfig { - pub(crate) cache: Option, - pub(crate) headers: Option, - pub(crate) ignore: Option, -} - +/// A directory or subdirectory with assets. #[derive(Debug, Default)] struct AssetConfigTreeNode { - pub parent: Option>, + pub parent: Option, pub rules: Vec, + pub origin: PathBuf, } impl AssetSourceDirectoryConfiguration { /// Constructs config tree for assets directory. - pub(crate) fn load(root_dir: &Path) -> anyhow::Result { + pub fn load(root_dir: &Path) -> anyhow::Result { if !root_dir.has_root() { bail!("root_dir paramenter is expected to be canonical path") } @@ -84,7 +102,8 @@ impl AssetSourceDirectoryConfiguration { Ok(Self { config_map }) } - pub(crate) fn get_asset_config(&self, canonical_path: &Path) -> anyhow::Result { + /// Fetches the configuration for the asset. + pub fn get_asset_config(&mut self, canonical_path: &Path) -> anyhow::Result { let parent_dir = canonical_path.parent().with_context(|| { format!( "unable to get the parent directory for asset path: {:?}", @@ -100,82 +119,49 @@ impl AssetSourceDirectoryConfiguration { parent_dir ) })? + .lock() + .unwrap() .get_config(canonical_path)) } -} - -#[derive(Deserialize)] -#[serde(deny_unknown_fields)] -struct InterimAssetConfigRule { - r#match: String, - cache: Option, - #[serde(default, deserialize_with = "deser_headers")] - headers: Maybe, - ignore: Option, -} - -impl Default for Maybe { - fn default() -> Self { - Self::Absent - } -} -fn deser_headers<'de, D>(deserializer: D) -> Result, D::Error> -where - D: serde::Deserializer<'de>, -{ - match serde_json::value::Value::deserialize(deserializer)? { - Value::Object(v) => Ok(Maybe::Value( - v.into_iter() - .map(|(k, v)| (k, v.to_string().trim_matches('"').to_string())) - .collect::>(), - )), - Value::Null => Ok(Maybe::Null), - _ => Err(serde::de::Error::custom( - "wrong data format for field `headers` (only map or null are allowed)", - )), - } -} + /// Returns a collection of unused configuration objects from all `.ic-assets.json` files + pub fn get_unused_configs(&self) -> HashMap> { + let mut hm = HashMap::new(); + // aggregate + for node in self.config_map.values() { + let config_node = &node.lock().unwrap(); -impl AssetConfigRule { - fn from_interim( - InterimAssetConfigRule { - r#match, - cache, - headers, - ignore, - }: InterimAssetConfigRule, - config_file_parent_dir: &Path, - ) -> anyhow::Result { - let glob = Glob::new( - config_file_parent_dir - .join(&r#match) - .to_str() - .with_context(|| { - format!( - "cannot combine {} and {} into a string (to be later used as a glob pattern)", - config_file_parent_dir.display(), - r#match - ) - })?, - ) - .with_context(|| format!("{} is not a valid glob pattern", r#match))?.compile_matcher(); + let origin = config_node.origin.clone(); - Ok(Self { - r#match: glob, - cache, - headers, - ignore, - }) + for rule in config_node.rules.clone() { + if !rule.used { + hm.entry(origin.clone()) + .and_modify(|v: &mut Vec| v.push(rule.clone())) + .or_insert_with(|| vec![rule.clone()]); + } + } + } + // dedup and remove full path from "match" field + for (path, rules) in hm.iter_mut() { + rules.sort_by_key(|v| v.r#match.glob().to_string()); + rules.dedup(); + for mut rule in rules { + let prefix_path = format!("{}/", path.display()); + let modified_glob = rule.r#match.glob().to_string(); + let original_glob = &modified_glob.strip_prefix(&prefix_path); + if let Some(og) = original_glob { + let original_glob = globset::Glob::new(og).unwrap().compile_matcher(); + rule.r#match = original_glob; + } + } + } + hm } } impl AssetConfigTreeNode { - fn load( - parent: Option>, - dir: &Path, - configs: &mut HashMap>, - ) -> anyhow::Result<()> { + /// Constructs config tree for assets directory in a recursive fashion. + fn load(parent: Option, dir: &Path, configs: &mut ConfigMap) -> anyhow::Result<()> { let config_path: Option; match ( dir.join(ASSETS_CONFIG_FILENAME_JSON).exists(), @@ -190,7 +176,6 @@ impl AssetConfigTreeNode { )) } (true, false) => config_path = Some(dir.join(ASSETS_CONFIG_FILENAME_JSON)), - (false, true) => config_path = Some(dir.join(ASSETS_CONFIG_FILENAME_JSON5)), (false, false) => config_path = None, } @@ -199,7 +184,7 @@ impl AssetConfigTreeNode { let content = fs::read_to_string(&config_path).with_context(|| { format!("unable to read config file: {}", config_path.display()) })?; - let interim_rules: Vec = json5::from_str(&content) + let interim_rules: Vec = json5::from_str(&content) .with_context(|| { format!( "malformed JSON asset config file: {}", @@ -213,7 +198,11 @@ impl AssetConfigTreeNode { let parent_ref = match parent { Some(p) if rules.is_empty() => p, - _ => Arc::new(Self { parent, rules }), + _ => Arc::new(Mutex::new(Self { + parent, + rules, + origin: dir.to_path_buf(), + })), }; configs.insert(dir.to_path_buf(), parent_ref.clone()); @@ -227,15 +216,20 @@ impl AssetConfigTreeNode { Ok(()) } - fn get_config(&self, canonical_path: &Path) -> AssetConfig { + /// Fetches asset config in a recursive fashion. + /// Marks config rules as *used*, whenever the rule' glob patter matched querried file. + fn get_config(&mut self, canonical_path: &Path) -> AssetConfig { let base_config = match &self.parent { - Some(parent) => parent.get_config(canonical_path), + Some(parent) => parent.clone().lock().unwrap().get_config(canonical_path), None => AssetConfig::default(), }; self.rules - .iter() + .iter_mut() .filter(|rule| rule.applies(canonical_path)) - .fold(base_config, |acc, x| acc.merge(x)) + .fold(base_config, |acc, x| { + x.used = true; + acc.merge(x) + }) } } @@ -258,6 +252,126 @@ impl AssetConfig { } } +/// This module contains various utilities needed for serialization/deserialization +/// and pretty-printing of the `AssetConfigRule` data structure. +mod rule_utils { + use super::{AssetConfigRule, CacheConfig, HeadersConfig, Maybe}; + use anyhow::Context; + use globset::{Glob, GlobMatcher}; + use serde::{Deserialize, Serializer}; + use serde_json::Value; + use std::collections::HashMap; + use std::fmt; + use std::path::Path; + + pub(super) fn glob_cmp(a: &GlobMatcher, b: &GlobMatcher) -> bool { + a.glob() == b.glob() + } + + pub(super) fn glob_fmt( + field: &GlobMatcher, + formatter: &mut fmt::Formatter, + ) -> Result<(), fmt::Error> { + formatter.write_str(field.glob().glob())?; + Ok(()) + } + + pub(super) fn glob_serialize(bytes: &GlobMatcher, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&bytes.glob().to_string()) + } + + pub(super) fn headers_serialize( + headers: &super::Maybe, + serializer: S, + ) -> Result + where + S: Serializer, + { + use serde::ser::SerializeMap; + match headers { + super::Maybe::Null => serializer.serialize_map(Some(0))?.end(), + super::Maybe::Value(hm) => { + let mut map = serializer.serialize_map(Some(hm.len()))?; + for (k, v) in hm { + map.serialize_entry(k, &v)?; + } + map.end() + } + super::Maybe::Absent => unreachable!(), // this option is already skipped via `skip_serialization_with` + } + } + + fn headers_deserialize<'de, D>(deserializer: D) -> Result, D::Error> + where + D: serde::Deserializer<'de>, + { + match serde_json::value::Value::deserialize(deserializer)? { + Value::Object(v) => Ok(Maybe::Value( + v.into_iter() + .map(|(k, v)| (k, v.to_string().trim_matches('"').to_string())) + .collect::>(), + )), + Value::Null => Ok(Maybe::Null), + _ => Err(serde::de::Error::custom( + "wrong data format for field `headers` (only map or null are allowed)", + )), + } + } + + impl Maybe { + pub(super) fn is_absent(&self) -> bool { + matches!(*self, Self::Absent) + } + } + + #[derive(Deserialize)] + #[serde(deny_unknown_fields)] + pub(super) struct InterimAssetConfigRule { + r#match: String, + cache: Option, + #[serde(default, deserialize_with = "headers_deserialize")] + headers: Maybe, + ignore: Option, + } + + impl AssetConfigRule { + pub(super) fn from_interim( + InterimAssetConfigRule { + r#match, + cache, + headers, + ignore, + }: InterimAssetConfigRule, + config_file_parent_dir: &Path, + ) -> anyhow::Result { + let glob = Glob::new( + config_file_parent_dir + .join(&r#match) + .to_str() + .with_context(|| { + format!( + "cannot combine {} and {} into a string (to be later used as a glob pattern)", + config_file_parent_dir.display(), + r#match + ) + })?, + ) + .with_context(|| format!("{} is not a valid glob pattern", r#match))?.compile_matcher(); + + Ok(Self { + r#match: glob, + cache, + headers, + ignore, + used: false, + }) + } + } +} + #[cfg(test)] mod with_tempdir { @@ -326,7 +440,7 @@ mod with_tempdir { let assets_temp_dir = create_temporary_assets_directory(Some(cfg), 7).unwrap(); let assets_dir = assets_temp_dir.path().canonicalize()?; - let assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; for f in ["nested/the-thing.txt", "nested/deep/the-next-thing.toml"] { assert_eq!( assets_config.get_asset_config(assets_dir.join(f).as_path())?, @@ -367,7 +481,7 @@ mod with_tempdir { let assets_temp_dir = create_temporary_assets_directory(cfg, 7).unwrap(); let assets_dir = assets_temp_dir.path().canonicalize()?; - let assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; for f in ["nested/the-thing.txt", "nested/deep/the-next-thing.toml"] { assert_eq!( assets_config.get_asset_config(assets_dir.join(f).as_path())?, @@ -442,7 +556,7 @@ mod with_tempdir { )])); let assets_temp_dir = create_temporary_assets_directory(cfg, 1).unwrap(); let assets_dir = assets_temp_dir.path().canonicalize()?; - let assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; let parsed_asset_config = assets_config.get_asset_config(assets_dir.join("index.html").as_path())?; let expected_asset_config = AssetConfig { @@ -515,7 +629,7 @@ mod with_tempdir { let assets_temp_dir = create_temporary_assets_directory(cfg, 7).unwrap(); let assets_dir = assets_temp_dir.path().canonicalize()?; - let assets_config = dbg!(AssetSourceDirectoryConfiguration::load(&assets_dir))?; + let mut assets_config = dbg!(AssetSourceDirectoryConfiguration::load(&assets_dir))?; for f in [ "index.html", "js/index.js", @@ -567,7 +681,7 @@ mod with_tempdir { )])); let assets_temp_dir = create_temporary_assets_directory(cfg, 0).unwrap(); let assets_dir = assets_temp_dir.path().canonicalize()?; - let assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; assert_eq!( assets_config.get_asset_config(assets_dir.join("index.html").as_path())?, AssetConfig { @@ -652,7 +766,7 @@ mod with_tempdir { let cfg = Some(HashMap::new()); let assets_temp_dir = create_temporary_assets_directory(cfg, 0).unwrap(); let assets_dir = assets_temp_dir.path().canonicalize()?; - let assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; + let mut assets_config = AssetSourceDirectoryConfiguration::load(&assets_dir)?; assert_eq!( assets_config.get_asset_config(assets_dir.join("doesnt.exists").as_path())?, AssetConfig::default() diff --git a/src/canisters/frontend/ic-asset/src/sync.rs b/src/canisters/frontend/ic-asset/src/sync.rs index 26e096adb8..2c872ba8e4 100644 --- a/src/canisters/frontend/ic-asset/src/sync.rs +++ b/src/canisters/frontend/ic-asset/src/sync.rs @@ -4,11 +4,10 @@ use crate::asset_canister::protocol::{AssetDetails, BatchOperationKind}; use crate::asset_config::{ AssetConfig, AssetSourceDirectoryConfiguration, ASSETS_CONFIG_FILENAME_JSON, }; -use crate::params::CanisterCallParams; - use crate::operations::{ create_new_assets, delete_obsolete_assets, set_encodings, unset_obsolete_encodings, }; +use crate::params::CanisterCallParams; use crate::plumbing::{make_project_assets, AssetDescriptor, ProjectAsset}; use anyhow::{bail, Context}; use ic_utils::Canister; @@ -73,9 +72,9 @@ fn gather_asset_descriptors(dirs: &[&Path]) -> anyhow::Result anyhow::Result>(); + + for e in entries { let source = e.path().canonicalize().with_context(|| { format!( "unable to canonicalize the path when gathering asset descriptors: {}", @@ -123,6 +124,18 @@ fn gather_asset_descriptors(dirs: &[&Path]) -> anyhow::Result 1 { "s" } else { "" }, + path=config_path.display() + ); + for rule in rules { + println!("{}", serde_json::to_string_pretty(&rule).unwrap()); + } + } } Ok(asset_descriptors.into_values().collect()) }