Skip to content

Commit

Permalink
test: fix broken integration tests
Browse files Browse the repository at this point in the history
Introduce custom error types for `EvaluationEnvironment` and make use of
them to properly identify not found policies.

This is required to properly return a 404 response inside of the UI.

This regression was detected by the integration tests.

Signed-off-by: Flavio Castelli <[email protected]>
  • Loading branch information
flavio committed Dec 13, 2023
1 parent 592ecda commit 94873a1
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 44 deletions.
94 changes: 57 additions & 37 deletions src/workers/evaluation_environment.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::{anyhow, Result};
use policy_evaluator::{
admission_response::AdmissionResponse,
callback_requests::CallbackRequest,
Expand All @@ -14,6 +13,7 @@ use tracing::debug;

use crate::communication::EvalRequest;
use crate::config::PolicyMode;
use crate::workers::error::{EvaluationError, Result};

Check failure on line 16 in src/workers/evaluation_environment.rs

View workflow job for this annotation

GitHub Actions / Clippy

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`

Check failure on line 16 in src/workers/evaluation_environment.rs

View workflow job for this annotation

GitHub Actions / Check

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`

Check failure on line 16 in src/workers/evaluation_environment.rs

View workflow job for this annotation

GitHub Actions / Test Suite

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`

Check failure on line 16 in src/workers/evaluation_environment.rs

View workflow job for this annotation

GitHub Actions / End to end tests

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`
use crate::workers::{
policy_evaluation_settings::PolicyEvaluationSettings,
precompiled_policy::{PrecompiledPolicies, PrecompiledPolicy},
Expand Down Expand Up @@ -84,18 +84,23 @@ impl EvaluationEnvironment {
};

for (policy_id, policy) in policies {
let precompiled_policy = precompiled_policies
.get(&policy.url)
.ok_or_else(|| anyhow!("cannot find policy settings of {}", policy_id))?;

eval_env.register(
engine,
policy_id,
precompiled_policy,
policy,
callback_handler_tx.clone(),
policy_evaluation_limit_seconds,
)?;
let precompiled_policy = precompiled_policies.get(&policy.url).ok_or_else(|| {
EvaluationError::BootstrapFailure(format!(
"cannot find policy settings of {}",
policy_id
))
})?;

eval_env
.register(
engine,
policy_id,
precompiled_policy,
policy,
callback_handler_tx.clone(),
policy_evaluation_limit_seconds,
)
.map_err(|e| EvaluationError::BootstrapFailure(e.to_string()))?;
}

Ok(eval_env)
Expand Down Expand Up @@ -158,7 +163,10 @@ impl EvaluationEnvironment {
let policy_eval_settings = PolicyEvaluationSettings {
policy_mode: policy.policy_mode.clone(),
allowed_to_mutate: policy.allowed_to_mutate.unwrap_or(false),
settings: policy.settings_to_json()?.unwrap_or_default(),
settings: policy
.settings_to_json()
.map_err(|e| EvaluationError::InternalError(e.to_string()))?
.unwrap_or_default(),
};
self.policy_id_to_settings
.insert(policy_id.to_owned(), policy_eval_settings);
Expand All @@ -179,37 +187,44 @@ impl EvaluationEnvironment {
self.policy_id_to_settings
.get(policy_id)
.map(|settings| settings.policy_mode.clone())
.ok_or(anyhow!("cannot find policy with ID {policy_id}"))
.ok_or(EvaluationError::PolicyNotFound(policy_id.to_string()))
}

/// Given a policy ID, returns true if the policy is allowed to mutate
pub fn get_policy_allowed_to_mutate(&self, policy_id: &str) -> Result<bool> {
self.policy_id_to_settings
.get(policy_id)
.map(|settings| settings.allowed_to_mutate)
.ok_or(anyhow!("cannot find policy with ID {policy_id}"))
.ok_or(EvaluationError::PolicyNotFound(policy_id.to_string()))
}

/// Perform a request validation
pub fn validate(&self, policy_id: &str, req: &EvalRequest) -> Result<AdmissionResponse> {
let settings = self.policy_id_to_settings.get(policy_id).ok_or(anyhow!(
"cannot find settings for policy with ID {policy_id}"
))?;
let settings = self
.policy_id_to_settings
.get(policy_id)
.ok_or(EvaluationError::PolicyNotFound(policy_id.to_string()))?;

let mut evaluator = self.rehydrate(policy_id)?;

let eval_ctx = self.policy_id_to_eval_ctx.get(policy_id).ok_or(anyhow!(
"cannot find evaluation context for policy with ID {policy_id}"
))?;
let eval_ctx =
self.policy_id_to_eval_ctx
.get(policy_id)
.ok_or(EvaluationError::InternalError(format!(
"cannot find evaluation context for policy with ID {policy_id}"
)))?;

Ok(evaluator.validate(req.req.clone(), &settings.settings, eval_ctx))
}

/// Validate the settings the user provided for the given policy
pub fn validate_settings(&self, policy_id: &str) -> Result<SettingsValidationResponse> {
let settings = self.policy_id_to_settings.get(policy_id).ok_or(anyhow!(
"cannot find settings for policy with ID {policy_id}"
))?;
let settings =
self.policy_id_to_settings
.get(policy_id)
.ok_or(EvaluationError::InternalError(format!(
"cannot find settings for policy with ID {policy_id}"
)))?;

let mut evaluator = self.rehydrate(policy_id)?;

Expand All @@ -221,21 +236,20 @@ impl EvaluationEnvironment {
let module_digest = self
.policy_id_to_module_digest
.get(policy_id)
.ok_or(anyhow!(
"cannot find module_digest for policy with ID {policy_id}"
))?;
.ok_or(EvaluationError::PolicyNotFound(policy_id.to_string()))?;
let policy_evaluator_pre = self
.module_digest_to_policy_evaluator_pre
.get(module_digest)
.ok_or(anyhow!(
"cannot find PolicyEvaluatorPre for policy with ID {policy_id}"
))?;
.ok_or(EvaluationError::PolicyNotFound(policy_id.to_string()))?;

let eval_ctx = self.policy_id_to_eval_ctx.get(policy_id).ok_or(anyhow!(
"cannot find evaluation context for policy with ID {policy_id}"
))?;
let eval_ctx = self
.policy_id_to_eval_ctx
.get(policy_id)
.ok_or(EvaluationError::PolicyNotFound(policy_id.to_string()))?;

policy_evaluator_pre.rehydrate(eval_ctx)
policy_evaluator_pre.rehydrate(eval_ctx).map_err(|e| {
EvaluationError::WebAssemblyError(format!("cannot rehydrate PolicyEvaluatorPre: {e}"))
})
}
}

Expand All @@ -249,7 +263,11 @@ fn create_wasmtime_module(
// full control of the precompiled module. This is generated by the
// WorkerPool thread
unsafe { wasmtime::Module::deserialize(engine, &precompiled_policy.precompiled_module) }
.map_err(|e| anyhow!("could not rehydrate wasmtime::Module {policy_url}: {e:?}"))
.map_err(|e| {
EvaluationError::WebAssemblyError(format!(
"could not rehydrate wasmtime::Module {policy_url}: {e:?}"
))
})
}

/// Internal function, takes care of creating the `PolicyEvaluator` instance for the given policy
Expand All @@ -269,7 +287,9 @@ fn create_policy_evaluator_pre(
policy_evaluator_builder.enable_epoch_interruptions(limit, limit);
}

policy_evaluator_builder.build_pre()
policy_evaluator_builder.build_pre().map_err(|e| {
EvaluationError::WebAssemblyError(format!("cannot build PolicyEvaluatorPre {e}"))
})
}

#[cfg(test)]
Expand Down
1 change: 1 addition & 0 deletions src/workers/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub(crate) mod error;

Check failure on line 1 in src/workers/mod.rs

View workflow job for this annotation

GitHub Actions / Clippy

file not found for module `error`

Check failure on line 1 in src/workers/mod.rs

View workflow job for this annotation

GitHub Actions / Check

file not found for module `error`

Check failure on line 1 in src/workers/mod.rs

View workflow job for this annotation

GitHub Actions / Test Suite

file not found for module `error`

Check failure on line 1 in src/workers/mod.rs

View workflow job for this annotation

GitHub Actions / End to end tests

file not found for module `error`
mod evaluation_environment;
mod policy_evaluation_settings;
pub(crate) mod pool;
Expand Down
19 changes: 12 additions & 7 deletions src/workers/worker.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::Result;
use policy_evaluator::{
admission_response::{AdmissionResponse, AdmissionResponseStatus},
policy_evaluator::ValidateRequest,
Expand All @@ -10,7 +9,10 @@ use tracing::{error, info, info_span};
use crate::communication::{EvalRequest, RequestOrigin};
use crate::config::PolicyMode;
use crate::metrics::{self};
use crate::workers::EvaluationEnvironment;
use crate::workers::{
error::{EvaluationError, Result},

Check failure on line 13 in src/workers/worker.rs

View workflow job for this annotation

GitHub Actions / Clippy

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`

Check failure on line 13 in src/workers/worker.rs

View workflow job for this annotation

GitHub Actions / Check

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`

Check failure on line 13 in src/workers/worker.rs

View workflow job for this annotation

GitHub Actions / Test Suite

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`

Check failure on line 13 in src/workers/worker.rs

View workflow job for this annotation

GitHub Actions / End to end tests

unresolved imports `crate::workers::error::EvaluationError`, `crate::workers::error::Result`
EvaluationEnvironment,
};

pub(crate) struct Worker {
evaluation_environment: Arc<EvaluationEnvironment>,
Expand Down Expand Up @@ -101,13 +103,16 @@ impl Worker {
let span = info_span!(parent: &req.parent_span, "policy_eval");
let _enter = span.enter();

let admission_response = self.evaluate(&req).unwrap_or_else(|e| {
AdmissionResponse::reject_internal_server_error(
let admission_response = match self.evaluate(&req) {
Ok(ar) => Some(ar),
Err(EvaluationError::PolicyNotFound(_)) => None,
Err(e) => Some(AdmissionResponse::reject_internal_server_error(
req.req.uid().to_owned(),
e.to_string(),
)
});
if let Err(e) = req.resp_chan.send(Some(admission_response)) {
)),
};

if let Err(e) = req.resp_chan.send(admission_response) {
error!("cannot send response back: {e:?}");
}
}
Expand Down

0 comments on commit 94873a1

Please sign in to comment.