Skip to content
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

Fixes #22147: Don't require resources to be listed in yaml technique #4595

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
217 changes: 173 additions & 44 deletions policies/Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion policies/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ test-docs: $(DOC_EXAMPLES_CF)
test: libs
cargo test --locked
# Parse lib
cargo run --bin rudderc --quiet -- -o /tmp/test.cf rudderc/tests/cases/general/ntp.yml -l target/repos/ncf/tree/30_generic_methods/ -l target/repos/dsc/plugin/src/ncf/30_generic_methods/
cargo run --bin rudderc --quiet -- -o /tmp/test.cf rudderc/tests/cases/general/ntp/ntp.yml -l target/repos/ncf/tree/30_generic_methods/ -l target/repos/dsc/plugin/src/ncf/30_generic_methods/

%.cf: %.yml libs
cargo run --bin rudderc --quiet -- --quiet -l target/repos/ncf/tree/30_generic_methods/ --output $@ --target cf $<
Expand Down
2 changes: 1 addition & 1 deletion policies/rudderc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ serde = { version = "1", features = ["derive"] }
serde_yaml = "0.9"
serde_json = "1"
log = "0.4"
env_logger = "0.9"
env_logger = "0.10"
walkdir = "2"
once_cell = "1"
rudder_commons = { path = "../rudder-commons" }
Expand Down
37 changes: 35 additions & 2 deletions policies/rudderc/src/backends.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
//! Target != Backend, we could have different target compiled by the same backend

use crate::backends::metadata::Metadata;
use anyhow::Result;
use anyhow::{Error, Result};
use rudder_commons::Target;
use std::path::Path;
use walkdir::WalkDir;

pub use self::{unix::Unix, windows::Windows};
use crate::ir::Technique;
Expand All @@ -20,7 +22,38 @@ pub mod windows;
/// A backend is something that can generate final code for a given language from an IR
pub trait Backend {
// For now, we only generate one file content
fn generate(&self, policy: Technique) -> Result<String>;
fn generate(&self, policy: Technique, resources: &Path) -> Result<String>;

/// List resources in directory
///
/// Note: We only support UTF-8 file names.
fn list_resources(path: &Path) -> Result<Vec<String>>
where
Self: Sized,
{
if path.is_dir() {
WalkDir::new(path)
// We need a stable order
.sort_by_file_name()
.into_iter()
// Only select files
.filter(|r| r.as_ref().map(|e| e.file_type().is_file()).unwrap_or(true))
.map(|e| {
e.map(|e| {
e.path()
// relative path
.strip_prefix(path)
.unwrap()
.to_string_lossy()
.to_string()
})
.map_err(|e| e.into())
})
.collect::<Result<Vec<String>, Error>>()
} else {
Ok(vec![])
}
}
}

/// Select the right backend
Expand Down
22 changes: 14 additions & 8 deletions policies/rudderc/src/backends/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@
//! WARNING: We do not implement the format exhaustively but only the parts we need to
//! generate our metadata file. There are parts that are could be made optional, and others are missing.

use anyhow::Result;
use anyhow::{Error, Result};
use quick_xml::se::Serializer;
use rudder_commons::{Target, ALL_TARGETS};
use serde::Serialize;
use std::path::Path;

use crate::backends::Windows;
use crate::{
Expand All @@ -23,8 +24,8 @@ use crate::{
pub struct Metadata;

impl Backend for Metadata {
fn generate(&self, technique: ir::Technique) -> Result<String> {
let technique: Technique = technique.into();
fn generate(&self, technique: ir::Technique, resources: &Path) -> Result<String> {
let technique: Technique = (technique, resources).try_into()?;
Self::xml(technique)
}
}
Expand All @@ -37,11 +38,16 @@ impl Metadata {
}
}

impl From<ir::Technique> for Technique {
fn from(src: ir::Technique) -> Self {
impl TryFrom<(ir::Technique, &Path)> for Technique {
type Error = Error;

fn try_from(data: (ir::Technique, &Path)) -> Result<Self> {
let (src, resources) = data;
let files = Metadata::list_resources(resources)?;

let agent = ALL_TARGETS
.iter()
.map(|t| Agent::from(src.id.to_string(), *t, src.files.clone()))
.map(|t| Agent::from(src.id.to_string(), *t, files.clone()))
.collect();
let multi_instance = !src.parameters.is_empty();
let policy_generation = if src.parameters.is_empty() {
Expand Down Expand Up @@ -75,7 +81,7 @@ impl From<ir::Technique> for Technique {
sections.push(SectionType::SectionInput(section));
}

Technique {
Ok(Technique {
name: src.name,
description: src.description,
// false is for legacy techniques, we only use the modern reporting
Expand All @@ -84,7 +90,7 @@ impl From<ir::Technique> for Technique {
multi_instance,
policy_generation: policy_generation.to_string(),
sections: Sections { section: sections },
}
})
}
}

Expand Down
9 changes: 5 additions & 4 deletions policies/rudderc/src/backends/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use anyhow::Result;
use log::trace;
use std::path::Path;

use super::Backend;
use crate::{
Expand Down Expand Up @@ -35,7 +36,7 @@ impl Unix {
}

impl Backend for Unix {
fn generate(&self, technique: ir::Technique) -> Result<String> {
fn generate(&self, technique: ir::Technique, resources: &Path) -> Result<String> {
fn resolve_module(r: ItemKind, context: Condition) -> Result<Vec<(Promise, Bundle)>> {
match r {
ItemKind::Block(r) => {
Expand All @@ -57,10 +58,10 @@ impl Backend for Unix {
let mut main_bundle = Bundle::agent(technique.id.clone());
// separate bundles for each method call
let mut call_bundles = vec![];
if !technique.files.is_empty() {
if !Unix::list_resources(resources)?.is_empty() {
main_bundle.add_promise_group(vec![Promise::string(
"modules_dir",
"${this.promise_dirname}/modules",
"resources_dir",
"${this.promise_dirname}/resources",
)]);
};
for item in technique.items {
Expand Down
11 changes: 6 additions & 5 deletions policies/rudderc/src/backends/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use super::Backend;
use crate::ir::Technique;
use std::path::Path;

use anyhow::Result;
use askama::Template;
Expand All @@ -16,8 +17,8 @@ impl Default for Windows {
}

impl Backend for Windows {
fn generate(&self, technique: Technique) -> Result<String> {
Ok(Self::technique(technique))
fn generate(&self, technique: Technique, resources: &Path) -> Result<String> {
Self::technique(technique, resources)
}
}

Expand Down Expand Up @@ -74,13 +75,13 @@ impl Windows {
Self::pascal_case(s)
}

fn technique(src: Technique) -> String {
fn technique(src: Technique, resources: &Path) -> Result<String> {
let technique = TechniqueTemplate {
id: src.name.as_str(),
has_modules: !src.files.is_empty(),
has_modules: !Windows::list_resources(resources)?.is_empty(),
// FIXME: add content
methods: vec![],
};
technique.render().unwrap()
technique.render().map_err(|e| e.into())
}
}
8 changes: 6 additions & 2 deletions policies/rudderc/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ use crate::{
logs::ok_output,
};

pub const RESOURCES_DIR: &str = "resources";

fn read_methods(libraries: &[PathBuf]) -> Result<HashMap<String, MethodInfo>> {
let mut methods = HashMap::new();
for library in libraries {
Expand Down Expand Up @@ -60,7 +62,8 @@ pub fn compile(libraries: &[PathBuf], input: &Path, target: Target) -> Result<St

// TODO other checks and optimizations here

backend(target).generate(policy)
let resources_path = input.parent().unwrap().join(RESOURCES_DIR);
backend(target).generate(policy, resources_path.as_path())
}

/// Compile metadata file
Expand All @@ -70,7 +73,8 @@ pub fn metadata(input: &Path) -> Result<String> {
"Generating metadata",
format!("{} v{} ({})", policy.name, policy.version, input.display()),
);
metadata_backend().generate(policy)
let resources_path = input.parent().unwrap().join(RESOURCES_DIR);
metadata_backend().generate(policy, resources_path.as_path())
}

/// Compute the output of the JSON file for the webapp
Expand Down
4 changes: 0 additions & 4 deletions policies/rudderc/src/ir/technique.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,6 @@ pub struct Technique {
#[serde(default)]
#[serde(skip_serializing_if = "Vec::is_empty")]
pub parameters: Vec<Parameter>,
#[serde(skip_serializing_if = "Vec::is_empty")]
#[serde(default)]
// Don't use path, let's assume UTF-8 everywhere
pub files: Vec<String>,
}

#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
bundle agent ntp {

vars:
"modules_dir" string => "${this.promise_dirname}/modules";
"resources_dir" string => "${this.promise_dirname}/resources";

methods:
"d86ce2e5-d5b6-45cc-87e8-c11cca71d907" usebundle => call_d86ce2e5-d5b6-45cc-87e8-c11cca71d907("htop", "2.3.4", "", "");
Expand Down
Empty file.
2 changes: 1 addition & 1 deletion policies/rudderc/tests/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rudder_commons::Target;
use test_generator::test_resources;

/// Compiles all files in `cases`. Files ending in `.fail.yml` are expected to fail.
#[test_resources("tests/cases/*/*.yml")]
#[test_resources("tests/cases/*/*/*.yml")]
fn compile(filename: &str) {
for t in [Target::Unix, Target::Windows, Target::Metadata] {
compile_file(Path::new(filename), t);
Expand Down