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 #22179: Make rudderc techniques work in edge cases #4607

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions policies/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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/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/technique.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/module-types/template/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ serde = { version = "1", features = ["derive"] }
serde_json = "1"
# we want stable order in produced config files
# json and urlencode provide built-in filters
minijinja = { version = "0.26.0", features = ["preserve_order", "json", "urlencode"] }
minijinja = { version = "0.27.0", features = ["preserve_order", "json", "urlencode"] }
mustache = "0.9.0"
rudder_module_type = { path = "../../rudder-module-type" }

Expand Down
47 changes: 27 additions & 20 deletions policies/rudderc/src/backends/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use std::path::Path;

use crate::backends::Windows;
use crate::compiler::RESOURCES_DIR;
use crate::ir::technique::LeafReporting;
use crate::{
backends::Backend,
ir,
Expand Down Expand Up @@ -274,31 +275,37 @@ impl SectionType {
// stack overflow threat.
modules
.into_iter()
.map(|r| match r {
ItemKind::Block(b) => SectionType::SectionBlock(SectionBlock {
.filter_map(|r| match r {
ItemKind::Block(b) => Some(SectionType::SectionBlock(SectionBlock {
name: b.name,
component: true,
multivalued: true,
section: SectionType::from(b.items),
reporting: b.reporting.to_string(),
}),
ItemKind::Method(m) => SectionType::Section(Section {
name: m.name,
id: m.id.clone(),
component: true,
multivalued: true,
report_keys: ReportKeys {
value: ReportKey {
// the class_parameter value
value: m
.params
.get(&m.info.unwrap().class_parameter)
.unwrap()
.clone(),
id: m.id,
},
},
}),
})),
ItemKind::Method(m) => {
if m.reporting == LeafReporting::Disabled {
None
} else {
Some(SectionType::Section(Section {
name: m.name,
id: m.id.clone(),
component: true,
multivalued: true,
report_keys: ReportKeys {
value: ReportKey {
// the class_parameter value
value: m
.params
.get(&m.info.unwrap().class_parameter)
.unwrap()
.clone(),
id: m.id,
},
},
}))
}
}
ItemKind::Module(_) => todo!(),
})
.collect()
Expand Down
2 changes: 1 addition & 1 deletion policies/rudderc/src/backends/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl Windows {

fn technique(src: Technique, resources: &Path) -> Result<String> {
let technique = TechniqueTemplate {
id: src.name.as_str(),
id: &src.id.to_string(),
has_modules: !Windows::list_resources(resources)?.is_empty(),
// FIXME: add content
methods: vec![],
Expand Down
27 changes: 26 additions & 1 deletion policies/rudderc/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use anyhow::{anyhow, bail, Context, Result};
use log::warn;
use rudder_commons::Target;

use crate::ir::technique::{Block, BlockReportingMode};
use crate::{
backends::{backend, metadata_backend, Backend},
doc,
Expand Down Expand Up @@ -118,7 +119,10 @@ fn methods_metadata(
}
check_method(m)?;
}
ItemKind::Block(b) => methods_metadata(&mut b.items, info)?,
ItemKind::Block(b) => {
check_block(b)?;
methods_metadata(&mut b.items, info)?
}
ItemKind::Module(_) => todo!(),
};
}
Expand Down Expand Up @@ -162,3 +166,24 @@ fn check_method(method: &mut Method) -> Result<()> {
}
Ok(())
}

/// Check block consistency
fn check_block(block: &Block) -> Result<()> {
match &block.reporting.mode {
BlockReportingMode::Focus => {
if block.reporting.id.is_none() {
bail!("Missing id of focused report in block '{}'", block.name)
}
}
m => {
if block.reporting.id.is_some() {
bail!(
"Reporting mode {} does not expect an id in block '{}'",
m,
block.name
)
}
}
}
Ok(())
}
44 changes: 34 additions & 10 deletions policies/rudderc/src/ir/technique.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,32 +186,56 @@ pub struct Method {
pub info: Option<&'static MethodInfo>,
}

#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)]
pub struct BlockReporting {
#[serde(default)]
pub mode: BlockReportingMode,
#[serde(default)]
pub id: Option<String>,
}

impl fmt::Display for BlockReporting {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"{}",
match self.mode {
BlockReportingMode::Disabled => "disabled".to_string(),
BlockReportingMode::Weighted => "weighted".to_string(),
BlockReportingMode::WorstCaseWeightedOne => "worst-case-weighted-one".to_string(),
BlockReportingMode::WorstCaseWeightedSum => "worst-case-weighted-sum".to_string(),
BlockReportingMode::Focus => format!("focus:{}", self.id.as_ref().unwrap()),
}
)
}
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub enum BlockReporting {
pub enum BlockReportingMode {
#[serde(rename = "worst-case-weighted-sum")]
WorstCaseWeightedSum,
#[serde(rename = "worst-case-weighted-one")]
WorstCaseWeightedOne,
#[serde(rename = "focus")]
Focus(String),
Focus,
#[serde(rename = "weighted")]
#[serde(alias = "enabled")]
Weighted,
#[serde(rename = "disabled")]
Disabled,
}

impl fmt::Display for BlockReporting {
impl fmt::Display for BlockReportingMode {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
f,
"{}",
match self {
Self::Disabled => "disabled".to_string(),
Self::Weighted => "weighted".to_string(),
Self::WorstCaseWeightedOne => "worst-case-weighted-one".to_string(),
Self::WorstCaseWeightedSum => "worst-case-weighted-sum".to_string(),
Self::Focus(s) => format!("focus({s})"),
Self::Disabled => "disabled",
Self::Weighted => "weighted",
Self::WorstCaseWeightedOne => "worst-case-weighted-one",
Self::WorstCaseWeightedSum => "worst-case-weighted-sum",
Self::Focus => "focus",
}
)
}
Expand All @@ -225,9 +249,9 @@ pub enum LeafReporting {
Disabled,
}

impl Default for BlockReporting {
impl Default for BlockReportingMode {
fn default() -> Self {
BlockReporting::Weighted
BlockReportingMode::Weighted
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
function NTP
function min
{
[CmdletBinding()]
param (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
function NTP
function ntp
{
[CmdletBinding()]
param (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@ items:
version: "2.3.4"
- name: "my block"
id: b9e259a1-51c5-40b3-98ef-0eeaf52dca98
reporting: worst-case-weighted-one
reporting:
mode: worst-case-weighted-one
items:
- name: "NTP service"
id: cf06e919-02b7-41a7-a03f-4239592f3c12
method: package_install
params:
name: ntp
files:
- "file.cfg"
89 changes: 89 additions & 0 deletions policies/rudderc/tests/cases/general/reporting/metadata.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<TECHNIQUE name="Test various reporting options">
<DESCRIPTION/>
<USEMETHODREPORTING>true</USEMETHODREPORTING>
<MULTIINSTANCE>false</MULTIINSTANCE>
<POLICYGENERATION>separated</POLICYGENERATION>
<AGENT type="cfengine-community,cfengine-nova">
<BUNDLES>
<NAME>reporting</NAME>
</BUNDLES>
<FILES>
<FILE name="technique.cf">
<INCLUDED>true</INCLUDED>
</FILE>
</FILES>
</AGENT>
<AGENT type="dsc">
<BUNDLES>
<NAME>Reporting</NAME>
</BUNDLES>
<FILES>
<FILE name="technique.ps1">
<INCLUDED>true</INCLUDED>
</FILE>
</FILES>
</AGENT>
<SECTIONS>
<SECTION name="No block without condition" id="a86ce2e5-d5b6-45cc-87e8-c11cca71d908" component="true" multivalued="true">
<REPORTKEYS>
<VALUE id="a86ce2e5-d5b6-45cc-87e8-c11cca71d908">
htop
</VALUE>
</REPORTKEYS>
</SECTION>
<SECTION name="No block with condition" id="b86ce2e5-d5b6-45cc-87e8-c11cca71d907" component="true" multivalued="true">
<REPORTKEYS>
<VALUE id="b86ce2e5-d5b6-45cc-87e8-c11cca71d907">
htop
</VALUE>
</REPORTKEYS>
</SECTION>
<SECTION name="A simpl block" reporting="worst-case-weighted-one" component="true" multivalued="true">
<SECTION name="NTP service" id="df06e919-02b7-41a7-a03f-4239592f3c12" component="true" multivalued="true">
<REPORTKEYS>
<VALUE id="df06e919-02b7-41a7-a03f-4239592f3c12">
ntp
</VALUE>
</REPORTKEYS>
</SECTION>
</SECTION>
<SECTION name="A simple block" reporting="worst-case-weighted-one" component="true" multivalued="true">
<SECTION name="NTP service" id="df06e919-02b7-41a7-a03f-4239592f3c45" component="true" multivalued="true">
<REPORTKEYS>
<VALUE id="df06e919-02b7-41a7-a03f-4239592f3c45">
ntp
</VALUE>
</REPORTKEYS>
</SECTION>
</SECTION>
<SECTION name="A nested block" reporting="worst-case-weighted-one" component="true" multivalued="true">
<SECTION name="A simple block inside" reporting="worst-case-weighted-one" component="true" multivalued="true">
<SECTION name="NTP service" id="cf06e919-02b7-41a7-a03f-4239592f3c14" component="true" multivalued="true">
<REPORTKEYS>
<VALUE id="cf06e919-02b7-41a7-a03f-4239592f3c14">
ntp
</VALUE>
</REPORTKEYS>
</SECTION>
</SECTION>
<SECTION name="Another block inside" reporting="worst-case-weighted-sum" component="true" multivalued="true">
<SECTION name="NTP service" id="cf06e919-02b7-41a7-a03f-4239592f3c13" component="true" multivalued="true">
<REPORTKEYS>
<VALUE id="cf06e919-02b7-41a7-a03f-4239592f3c13">
ntp
</VALUE>
</REPORTKEYS>
</SECTION>
</SECTION>
<SECTION name="Another block inside" reporting="focus:cf06e919-02b7-41a7-a03f-4239592f3c21" component="true" multivalued="true">
<SECTION name="Enabled reporting" id="cf06e919-02b7-41a7-a03f-4239592f3d12" component="true" multivalued="true">
<REPORTKEYS>
<VALUE id="cf06e919-02b7-41a7-a03f-4239592f3d12">
ntp
</VALUE>
</REPORTKEYS>
</SECTION>
</SECTION>
</SECTION>
</SECTIONS>
</TECHNIQUE>
Loading