From b6913a8bc6e73f97d2600523b263407b4711fd33 Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Fri, 25 Jul 2025 18:20:32 +0200 Subject: [PATCH] chore: Move wasm policy behind the feature flag Since wasm is now not the preferred way of policy enforcement hide it behind the not default feature flag - user must explicitly understand the consequences. --- Cargo.lock | 30 +++++++++++------ Cargo.toml | 6 +++- doc/src/policy.md | 3 +- src/bin/keystone.rs | 9 +++-- src/error.rs | 4 +++ src/policy.rs | 81 +++++++++++++++++++++++++++++---------------- 6 files changed, 91 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33c0e505..2fa514bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2115,9 +2115,9 @@ dependencies = [ [[package]] name = "hyper-util" -version = "0.1.15" +version = "0.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f66d5bd4c6f02bf0542fad85d626775bab9258cf795a4256dcaf3161114d1df" +checksum = "8d9b05277c7e8da2c93a568989bb6207bef0112e8d17df7a6eda4a3cf143bc5e" dependencies = [ "base64 0.22.1", "bytes", @@ -2131,7 +2131,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2", + "socket2 0.6.0", "system-configuration", "tokio", "tower-service", @@ -2332,9 +2332,9 @@ dependencies = [ [[package]] name = "io-uring" -version = "0.7.8" +version = "0.7.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b86e202f00093dcba4275d4636b93ef9dd75d025ae560d2521b45ea28ab49013" +checksum = "d93587f37623a1a17d94ef2bc9ada592f5465fe7732084ab7beefabe5c77c0c4" dependencies = [ "bitflags", "cfg-if", @@ -3337,9 +3337,9 @@ dependencies = [ [[package]] name = "postcard" -version = "1.1.2" +version = "1.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6c1de96e20f51df24ca73cafcc4690e044854d803259db27a00a461cb3b9d17a" +checksum = "6764c3b5dd454e283a30e6dfe78e9b31096d9e32036b5d1eaac7a6119ccb9a24" dependencies = [ "cobs", "embedded-io 0.4.0", @@ -3524,7 +3524,7 @@ dependencies = [ "quinn-udp", "rustc-hash", "rustls", - "socket2", + "socket2 0.5.10", "thiserror 2.0.12", "tokio", "tracing", @@ -3561,7 +3561,7 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2", + "socket2 0.5.10", "tracing", "windows-sys 0.59.0", ] @@ -4601,6 +4601,16 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "socket2" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "233504af464074f9d066d7b5416c5f9b894a5862a6506e306f7b816cdd6f1807" +dependencies = [ + "libc", + "windows-sys 0.59.0", +] + [[package]] name = "spin" version = "0.9.8" @@ -5166,7 +5176,7 @@ dependencies = [ "pin-project-lite", "signal-hook-registry", "slab", - "socket2", + "socket2 0.5.10", "tokio-macros", "windows-sys 0.52.0", ] diff --git a/Cargo.toml b/Cargo.toml index 1d03acd6..4a1d9677 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,7 @@ eyre = { version = "0.6" } fernet = { version = "0.2" } futures-util = { version = "0.3" } mockall_double = { version = "0.3" } -opa-wasm = { version = "^0.1" } +opa-wasm = { version = "^0.1", optional = true } openidconnect = { version = "4.0" } regex = { version = "1.11"} reqwest = { version = "0.12", features = ["json"] } @@ -77,6 +77,10 @@ thirtyfour = "0.36.0" tracing-test = { version = "0.2" } url = { version = "2.5" } +[features] +default = [] +wasm = ["dep:opa-wasm"] + [profile.release] strip = true debug = false diff --git a/doc/src/policy.md b/doc/src/policy.md index f678b6ef..4e9cf75d 100644 --- a/doc/src/policy.md +++ b/doc/src/policy.md @@ -24,7 +24,8 @@ crate happening for the big policy files. The investigation is in progress, so it is preferred not to rely on this method anyway. While running OPA as a WASM eliminates any networking communication, it heavily reduces feature set. In particular hot policy reload, decision logging, external calls done by the -policies themselves are not possible by design. +policies themselves are not possible by design. Using this way of policy +enforcement requires `wasm` feature enabled. All the policies currently are using the same policy names and definitions as the original Keystone to keep the deviation as less as possible. For the newly diff --git a/src/bin/keystone.rs b/src/bin/keystone.rs index a8a7da53..86331bd4 100644 --- a/src/bin/keystone.rs +++ b/src/bin/keystone.rs @@ -38,6 +38,7 @@ use uuid::Uuid; use openstack_keystone::api; use openstack_keystone::config::Config; +use openstack_keystone::error::KeystoneError; use openstack_keystone::federation::FederationApi; use openstack_keystone::keystone::{Service, ServiceState}; use openstack_keystone::plugin_manager::PluginManager; @@ -115,8 +116,12 @@ async fn main() -> Result<(), Report> { let policy = if let Some(opa_base_url) = &cfg.api_policy.opa_base_url { PolicyFactory::http(opa_base_url.clone()).await? } else { - let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("policy.wasm"); - PolicyFactory::from_wasm(&path).await? + #[cfg(feature = "wasm")] + { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("policy.wasm"); + PolicyFactory::from_wasm(&path).await? + } + return Err(KeystoneError::PolicyEnforcementNotAvailable)?; }; let shared_state = Arc::new(Service::new(cfg, conn, provider, policy)?); diff --git a/src/error.rs b/src/error.rs index 8a4a759b..71fbddc7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -60,6 +60,10 @@ pub enum KeystoneError { source: PolicyError, }, + /// Policy engine is not available. + #[error("policy enforcement is requested, but not available with the enabled features")] + PolicyEnforcementNotAvailable, + #[error(transparent)] ResourceError { #[from] diff --git a/src/policy.rs b/src/policy.rs index c2958837..a364dd92 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -14,6 +14,7 @@ #[cfg(test)] use mockall::mock; +#[cfg(feature = "wasm")] use opa_wasm::{ Runtime, wasmtime::{Config, Engine, Module, OptLevel, Store}, @@ -22,12 +23,13 @@ use reqwest::{Client, Url}; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; +#[cfg(feature = "wasm")] use std::path::Path; use std::sync::Arc; use std::time::SystemTime; use thiserror::Error; -use tokio::io::AsyncRead; -use tokio::io::AsyncReadExt; +#[cfg(feature = "wasm")] +use tokio::io::{AsyncRead, AsyncReadExt}; use tracing::{Level, debug, trace}; use crate::token::Token; @@ -62,6 +64,7 @@ pub enum PolicyError { #[error(transparent)] UrlParse(#[from] url::ParseError), + #[cfg(feature = "wasm")] #[error(transparent)] Wasm(#[from] opa_wasm::wasmtime::Error), } @@ -73,13 +76,16 @@ pub struct PolicyFactory { http_client: Option>, /// OPA url address. base_url: Option, + #[cfg(feature = "wasm")] /// WASM engine. engine: Option, + #[cfg(feature = "wasm")] /// WASM module. module: Option, } impl PolicyFactory { + #[cfg(feature = "wasm")] #[tracing::instrument(name = "policy.from_defaults", err)] pub async fn from_defaults() -> Result { let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("policy.wasm"); @@ -87,23 +93,25 @@ impl PolicyFactory { PolicyFactory::load(file).await } + #[cfg(feature = "wasm")] #[tracing::instrument(name = "policy.from_wasm", err)] pub async fn from_wasm(path: &Path) -> Result { let file = tokio::fs::File::open(path).await?; PolicyFactory::load(file).await } + #[allow(clippy::needless_update)] #[tracing::instrument(name = "policy.http", err)] pub async fn http(url: Url) -> Result { let client = Client::new(); Ok(Self { http_client: Some(Arc::new(client)), base_url: Some(url.join("/v1/data/")?), - engine: None, - module: None, + ..Default::default() }) } + #[cfg(feature = "wasm")] #[tracing::instrument(name = "policy.load", skip(source), err)] pub async fn load( mut source: impl AsyncRead + std::marker::Unpin, @@ -138,27 +146,32 @@ impl PolicyFactory { Ok(factory) } + #[allow(clippy::needless_update)] #[tracing::instrument(name = "policy.instantiate", level = Level::TRACE, skip_all, err)] pub async fn instantiate(&self) -> Result { - if let (Some(engine), Some(module)) = (&self.engine, &self.module) { - let mut store = Store::new(engine, ()); - let runtime = Runtime::new(&mut store, module).await?; - - let instance = runtime.without_data(&mut store).await?; - Ok(Policy { - http_client: self.http_client.clone(), - base_url: self.base_url.clone(), - store: Some(store), - instance: Some(instance), - }) - } else { - Ok(Policy { - http_client: self.http_client.clone(), - base_url: self.base_url.clone(), - store: None, - instance: None, - }) + #[cfg(feature = "wasm")] + { + if let (Some(engine), Some(module)) = (&self.engine, &self.module) { + let mut store = Store::new(engine, ()); + let runtime = Runtime::new(&mut store, module).await?; + + let instance = runtime.without_data(&mut store).await?; + return Ok(Policy { + http_client: self.http_client.clone(), + base_url: self.base_url.clone(), + #[cfg(feature = "wasm")] + store: Some(store), + #[cfg(feature = "wasm")] + instance: Some(instance), + }); + } } + + Ok(Policy { + http_client: self.http_client.clone(), + base_url: self.base_url.clone(), + ..Default::default() + }) } } @@ -182,10 +195,13 @@ mock! { } } +#[derive(Default)] pub struct Policy { http_client: Option>, base_url: Option, + #[cfg(feature = "wasm")] store: Option>, + #[cfg(feature = "wasm")] instance: Option>, } @@ -248,13 +264,22 @@ impl Policy { }); let span = tracing::Span::current(); - let res = if let (Some(store), Some(instance)) = (&mut self.store, &self.instance) { - tracing::Span::current().record("input", serde_json::to_string(&input)?); - let [res]: [OpaResponse; 1] = instance - .evaluate(store, policy_name.as_ref(), &input) - .await?; + let wasm_res: Option = None; - res.result + #[cfg(feature = "wasm")] + { + opa_res = if let (Some(store), Some(instance)) = (&mut self.store, &self.instance) { + tracing::Span::current().record("input", serde_json::to_string(&input)?); + let [res]: [OpaResponse; 1] = instance + .evaluate(store, policy_name.as_ref(), &input) + .await?; + + Some(res.result) + }; + } + + let res = if let Some(opa_res) = wasm_res { + opa_res } else if let (Some(client), Some(base_url)) = (&self.http_client, &self.base_url) { trace!("checking policy decision with OPA using http"); let url = base_url.join(policy_name.as_ref())?;