Skip to content

Commit

Permalink
group policy: cleanups
Browse files Browse the repository at this point in the history
* Rename `PolicyId` to `PolicyID`
* Config: policy group policies are now stored inside of a key named
  `policies` instead of `members`
* `EvaluationEnvironment`: introduce builder patter, remove duplication
  between policies and group policies

Signed-off-by: Flavio Castelli <fcastelli@suse.com>
  • Loading branch information
flavio committed Jul 15, 2024
1 parent e3d3568 commit 94841fe
Show file tree
Hide file tree
Showing 9 changed files with 382 additions and 322 deletions.
8 changes: 4 additions & 4 deletions src/api/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use tracing::info;

use crate::{
config::PolicyMode,
evaluation::{errors::EvaluationError, EvaluationEnvironment, PolicyId},
evaluation::{errors::EvaluationError, EvaluationEnvironment, PolicyID},
metrics,
};

Expand All @@ -34,7 +34,7 @@ pub(crate) fn evaluate(
request_origin: RequestOrigin,
) -> Result<AdmissionResponse, EvaluationError> {
let start_time = Instant::now();
let policy_id: PolicyId = policy_id.parse()?;
let policy_id: PolicyID = policy_id.parse()?;

let vanilla_validation_response = match evaluation_environment
.clone()
Expand Down Expand Up @@ -147,7 +147,7 @@ pub(crate) fn evaluate(
// - A policy might be running in "Monitor" mode, that always
// accepts the request (without mutation), logging the answer
fn validation_response_with_constraints(
policy_id: &PolicyId,
policy_id: &PolicyID,
policy_mode: &PolicyMode,
allowed_to_mutate: bool,
validation_response: AdmissionResponse,
Expand Down Expand Up @@ -204,7 +204,7 @@ mod tests {
use super::*;

lazy_static! {
static ref POLICY_ID: PolicyId = PolicyId::Policy("policy-id".to_string());
static ref POLICY_ID: PolicyID = PolicyID::Policy("policy-id".to_string());
}

fn create_evaluation_environment_that_accepts_request(
Expand Down
21 changes: 10 additions & 11 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,23 +207,23 @@ fn policies(matches: &clap::ArgMatches) -> Result<HashMap<String, PolicyOrPolicy

// Validate the policies and policy groups:
// - ensure policy names do not contain a '/' character
// - ensure names of policy group members do not contain a '/' character
// - ensure names of policy group's policies do not contain a '/' character
fn validate_policies(policies: &HashMap<String, PolicyOrPolicyGroup>) -> Result<()> {
for (name, policy) in policies.iter() {
if name.contains('/') {
return Err(anyhow!("policy name '{}' contains a '/' character", name));

Check warning on line 214 in src/config.rs

View check run for this annotation

Codecov / codecov/patch

src/config.rs#L211-L214

Added lines #L211 - L214 were not covered by tests
}
if let PolicyOrPolicyGroup::PolicyGroup { members, .. } = policy {
let members_with_invalid_name: Vec<String> = members
if let PolicyOrPolicyGroup::PolicyGroup { policies, .. } = policy {

Check warning on line 216 in src/config.rs

View check run for this annotation

Codecov / codecov/patch

src/config.rs#L216

Added line #L216 was not covered by tests
let policies_with_invalid_name: Vec<String> = policies
.iter()
.filter_map(|(id, _)| if id.contains('/') { Some(id) } else { None })

Check warning on line 219 in src/config.rs

View check run for this annotation

Codecov / codecov/patch

src/config.rs#L219

Added line #L219 was not covered by tests
.cloned()
.collect();
if !members_with_invalid_name.is_empty() {
if !policies_with_invalid_name.is_empty() {
return Err(anyhow!(
"policy group '{}' contains members with invalid names: {:?}",
"policy group '{}' contains policies with invalid names: {:?}",
name,
members_with_invalid_name
policies_with_invalid_name

Check warning on line 226 in src/config.rs

View check run for this annotation

Codecov / codecov/patch

src/config.rs#L223-L226

Added lines #L223 - L226 were not covered by tests
));
}
}
Expand Down Expand Up @@ -307,7 +307,7 @@ pub enum PolicyOrPolicyGroupSettings {
PolicyGroup {
expression: String,
message: String,
members: Vec<String>,
policies: Vec<String>,
},
}

Expand Down Expand Up @@ -357,8 +357,7 @@ pub enum PolicyOrPolicyGroup {
policy_mode: PolicyMode,
/// The policies that make up for this group
/// Key is a unique identifier
#[serde(rename = "policies")]
members: HashMap<String, PolicyGroupMember>,
policies: HashMap<String, PolicyGroupMember>,
/// The expression that is used to evaluate the group of policies
expression: String,
/// The message that is returned when the group of policies evaluates to false
Expand All @@ -376,12 +375,12 @@ impl PolicyOrPolicyGroup {
PolicyOrPolicyGroup::PolicyGroup {
expression,
message,
members: policies,
policies,
..
} => Ok(PolicyOrPolicyGroupSettings::PolicyGroup {
expression: expression.clone(),
message: message.clone(),
members: policies.keys().cloned().collect(),
policies: policies.keys().cloned().collect(),
}),
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/evaluation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@ pub(crate) mod precompiled_policy;
#[mockall_double::double]
pub(crate) use evaluation_environment::EvaluationEnvironment;

pub(crate) use evaluation_environment::EvaluationEnvironmentBuilder;

pub(crate) mod policy_id;
pub(crate) use policy_id::PolicyId;
pub(crate) use policy_id::PolicyID;
Loading

0 comments on commit 94841fe

Please sign in to comment.