Skip to content

Conversation

viccuad
Copy link
Member

@viccuad viccuad commented Aug 13, 2025

Description

Fix #1318

When building the evaluators of the members of the PolicyGroup,
propagate the PolicyExecutionConfiguration from the member metadata.

Previously, we were assuming all PolicyExecutionConfiguration were the
same. This fixes the bug were the PolicyGroup has members with differing
execution configurations, such a Rust Wasm module and an OPA Wasm
module.

Test

Added an e2e test.

Tried to add a unit test for Evaluator::new(), but the amount of scaffolding is quite big, to the point that we would be basically mocking the whole Evaluator creation. Here is a try:

Click me
    #[tokio::test(flavor = "multi_thread")]
    async fn test_policy_member_execution_mode_selection() {
        use crate::config::policy_definition::{PolicyExecutionConfiguration, PolicyMember};
        use policy_evaluator::policy_group_evaluator::PolicyGroupMemberSettings;
        use std::collections::HashMap;

        // Create a dummy PolicyGroup with one member
        let member_id = "member1".to_string();
        let member = PolicyMember {
            uri: "dummy_policy.wasm".to_string(), // <- the wasm metadata contains the execution mode
            settings: PolicyGroupMemberSettings::default(),
            user_execution_cfg: PolicyExecutionConfiguration::PolicyDefined,
        };
        let mut policy_members = HashMap::new();
        policy_members.insert(member_id.clone(), member.clone());

        let policy_group = crate::config::policy_definition::PolicyDefinition::PolicyGroup {
            id: "group1".to_string(),
            policy_mode:
                policy_evaluator::admission_response_handler::policy_mode::PolicyMode::Protect,
            policy_members: policy_members.clone(),
            expression: "true".to_string(),
            message: "msg".to_string(),
        };

        let cfg = crate::config::pull_and_run::PullAndRunSettings::default();
        let local_data = crate::command::run::local_data::LocalData::default(); // <- mock missing.

        let (evaluator, _, _) = Evaluator::new(&policy_group, &cfg, &local_data) // <- determine_execution_mode() mock missing
            .await
            .expect("Evaluator::new should succeed");

        if let Evaluator::GroupPolicy { .. } = evaluator {
            for member in policy_members.values() {
                assert!(matches!(
                    member.user_execution_cfg,
                    PolicyExecutionConfiguration::PolicyDefined
                ));
            }
        } else {
            panic!("Expected GroupPolicy variant");
        }
    }

Additional Information

Tradeoff

Potential improvement

@viccuad viccuad self-assigned this Aug 13, 2025
@viccuad viccuad requested a review from a team as a code owner August 13, 2025 10:32
@github-project-automation github-project-automation bot moved this to Pending review in Kubewarden Aug 13, 2025
"container_running_as_user".to_string(),
admission_policy_group::PolicyGroupMember {
module:
"registry://ghcr.io/kubewarden/tests/container-running-as-user:v1.0.4"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

codecov bot commented Aug 13, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.50%. Comparing base (c6ed74b) to head (4a1d389).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1319      +/-   ##
==========================================
+ Coverage   87.47%   87.50%   +0.02%     
==========================================
  Files          34       34              
  Lines        4936     4947      +11     
==========================================
+ Hits         4318     4329      +11     
  Misses        618      618              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@viccuad viccuad removed this from Kubewarden Aug 13, 2025
When building the evaluators of the members of the PolicyGroup,
propagate the PolicyExecutionConfiguration from the member metadata.

Previously, we were assuming all PolicyExecutionConfiguration were the
same. This fixes the bug were the PolicyGroup has members with differing
execution configurations, such a Rust Wasm module and an OPA Wasm
module.

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Tests that each PolicyGroup member has a correct Evaluator with
their PolicyExecutionMode.

Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
Signed-off-by: Víctor Cuadrado Juan <vcuadradojuan@suse.de>
@viccuad viccuad force-pushed the fix/polgroups-execmodes branch from 255437b to 4a1d389 Compare August 14, 2025 10:23
@viccuad viccuad merged commit b616cb9 into kubewarden:main Aug 14, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running (Cluster)PolicyGroups with policies of different types fails
3 participants