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.

Sign up for GitHub

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

Move attribute parsing to attribute trait #65

Merged
merged 4 commits into from
Aug 13, 2024
Merged

Move attribute parsing to attribute trait #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc03ad4
Move attribute parsing to attribute trait
adam-cattermole Aug 9, 2024
40ecfaf
Use default value if present
adam-cattermole Aug 9, 2024
d044a3e
Set allow(dead_code) on get_attribute
adam-cattermole Aug 9, 2024
e5a20de
Update log messages
adam-cattermole Aug 13, 2024
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
119 changes: 119 additions & 0 deletions src/attribute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
use crate::configuration::Path;
use crate::filter::http_context::Filter;
use chrono::{DateTime, FixedOffset};
use proxy_wasm::traits::Context;

pub trait Attribute {
fn parse(raw_attribute: Vec<u8>) -> Result<Self, String>
where
Self: Sized;
}

impl Attribute for String {
Copy link
Member

Choose a reason for hiding this comment

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

I think this could "just" be the TryInto trait, but it is that against the orphan rules

fn parse(raw_attribute: Vec<u8>) -> Result<Self, String> {
String::from_utf8(raw_attribute).map_err(|err| {
format!(
"parse: failed to parse selector String value, error: {}",
err
)
})
}
}

impl Attribute for i64 {
fn parse(raw_attribute: Vec<u8>) -> Result<Self, String> {
if raw_attribute.len() != 8 {
return Err(format!(
"parse: Int value expected to be 8 bytes, but got {}",
raw_attribute.len()
));
}
Ok(i64::from_le_bytes(
raw_attribute[..8]
.try_into()
.expect("This has to be 8 bytes long!"),
))
}
}

impl Attribute for u64 {
fn parse(raw_attribute: Vec<u8>) -> Result<Self, String> {
if raw_attribute.len() != 8 {
return Err(format!(
"parse: UInt value expected to be 8 bytes, but got {}",
raw_attribute.len()
));
}
Ok(u64::from_le_bytes(
raw_attribute[..8]
.try_into()
.expect("This has to be 8 bytes long!"),
))
}
}

impl Attribute for f64 {
fn parse(raw_attribute: Vec<u8>) -> Result<Self, String> {
if raw_attribute.len() != 8 {
return Err(format!(
"parse: Float value expected to be 8 bytes, but got {}",
raw_attribute.len()
));
}
Ok(f64::from_le_bytes(
raw_attribute[..8]
.try_into()
.expect("This has to be 8 bytes long!"),
))
}
}

impl Attribute for Vec<u8> {
fn parse(raw_attribute: Vec<u8>) -> Result<Self, String> {
Ok(raw_attribute)
}
}

impl Attribute for bool {
fn parse(raw_attribute: Vec<u8>) -> Result<Self, String> {
if raw_attribute.len() != 1 {
return Err(format!(
"parse: Bool value expected to be 1 byte, but got {}",
raw_attribute.len()
));
}
Ok(raw_attribute[0] & 1 == 1)
}
}

impl Attribute for DateTime<FixedOffset> {
fn parse(raw_attribute: Vec<u8>) -> Result<Self, String> {
if raw_attribute.len() != 8 {
return Err(format!(
"parse: Timestamp expected to be 8 bytes, but got {}",
raw_attribute.len()
));
}

let nanos = i64::from_le_bytes(
raw_attribute.as_slice()[..8]
.try_into()
.expect("This has to be 8 bytes long!"),
);
Ok(DateTime::from_timestamp_nanos(nanos).into())
}
}

#[allow(dead_code)]
pub fn get_attribute<T>(f: &Filter, attr: &str) -> Result<T, String>
Comment on lines +107 to +108
Copy link
Member

Choose a reason for hiding this comment

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

This is apparently still dead_code, but I also wonder how this is going to be useful... <T> needs to be known at compile time here, so that we need to know at method invocation time what the type of attr... is this ever going to be the case that this is known before runtime?

Copy link
Member Author

@adam-cattermole adam-cattermole Aug 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

As an example when I was building some request stuff for auth I was using it like this (providing a default to infer types):

attr.set_source(build_peer(
    get_attribute(&self, "source.address").unwrap_or("".to_string()),
    get_attribute(&self, "source.port").unwrap_or(0i64) as u32,
));

But then again now I think about it, these should probably remain unset if we can't get the attribute and not be a default value.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that actually makes sense... forgot that in the case of auth, there is a set of attributes (both their name and type) that need to be retrieved no matter what... so yes, that fn would be useful for that. The unwrap_or I'm less confident about it indeed as well

where
T: Attribute,
{
match f.get_property(Path::from(attr).tokens()) {
None => Err(format!(
"#{} get_attribute: not found: {}",
f.context_id, attr
)),
Some(attribute_bytes) => T::parse(attribute_bytes),
}
}
62 changes: 15 additions & 47 deletions src/configuration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
use crate::policy::Policy;
use crate::policy_index::PolicyIndex;
use std::cell::OnceCell;
use std::fmt::{Debug, Display, Formatter};
use std::sync::Arc;

use cel_interpreter::objects::ValueType;
use cel_interpreter::{Context, Expression, Value};
use cel_parser::{Atom, RelationOp};
use chrono::{DateTime, FixedOffset};
use serde::Deserialize;
use std::cell::OnceCell;
use std::fmt::{Debug, Display, Formatter};
use std::sync::Arc;

use crate::attribute::Attribute;
use crate::policy::Policy;
use crate::policy_index::PolicyIndex;

#[derive(Deserialize, Debug, Clone)]
pub struct SelectorItem {
Expand Down Expand Up @@ -168,47 +170,13 @@ impl PatternExpression {
pub fn eval(&self, raw_attribute: Vec<u8>) -> Result<bool, String> {
let cel_type = &self.compiled.get().unwrap().cel_type;
let value = match cel_type {
ValueType::String =>
Value::String(String::from_utf8(raw_attribute).map_err(|err| format!(
"pattern_expression_applies: failed to parse selector String value: {}, error: {}",
self.selector, err))?.into()),
ValueType::Int => {
if raw_attribute.len() != 8 {
return Err(format!("Int value expected to be 8 bytes, but got {}", raw_attribute.len()));
}
Value::Int(i64::from_le_bytes(raw_attribute[..8].try_into().expect("This has to be 8 bytes long!")))
},
ValueType::UInt => {
{
if raw_attribute.len() != 8 {
return Err(format!("UInt value expected to be 8 bytes, but got {}", raw_attribute.len()));
}
Value::UInt(u64::from_le_bytes(raw_attribute[..8].try_into().expect("This has to be 8 bytes long!")))
}
},
ValueType::Float => {
if raw_attribute.len() != 8 {
return Err(format!("Float value expected to be 8 bytes, but got {}", raw_attribute.len()));
}
Value::Float(f64::from_le_bytes(raw_attribute[..8].try_into().expect("This has to be 8 bytes long!")))
},
ValueType::Bytes => Value::Bytes(raw_attribute.into()),
ValueType::Bool => {
if raw_attribute.len() != 1 {
return Err(format!("Bool value expected to be 1 byte, but got {}", raw_attribute.len()));
}
Value::Bool(raw_attribute[0] & 1 == 1)
}
ValueType::Timestamp => {
{
if raw_attribute.len() != 8 {
return Err(format!("Timestamp expected to be 8 bytes, but got {}", raw_attribute.len()));
}
let nanos = i64::from_le_bytes(raw_attribute[..8].try_into().expect("This has to be 8 bytes long!"));
let time: DateTime<FixedOffset> = DateTime::from_timestamp_nanos(nanos).into();
Value::Timestamp(time)
}
}
ValueType::String => Value::String(Arc::new(Attribute::parse(raw_attribute)?)),
ValueType::Int => Value::Int(Attribute::parse(raw_attribute)?),
ValueType::UInt => Value::UInt(Attribute::parse(raw_attribute)?),
ValueType::Float => Value::Float(Attribute::parse(raw_attribute)?),
ValueType::Bytes => Value::Bytes(Arc::new(Attribute::parse(raw_attribute)?)),
ValueType::Bool => Value::Bool(Attribute::parse(raw_attribute)?),
ValueType::Timestamp => Value::Timestamp(Attribute::parse(raw_attribute)?),
// todo: Impl support for parsing these two types… Tho List/Map of what?
Comment on lines +173 to +179
Copy link
Member

Choose a reason for hiding this comment

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

Nice ❤️

// ValueType::List => {}
// ValueType::Map => {}
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod attribute;
mod configuration;
mod envoy;
mod filter;
Expand Down
15 changes: 5 additions & 10 deletions src/policy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::attribute::Attribute;
use crate::configuration::{DataItem, DataType, PatternExpression};
use crate::envoy::{RateLimitDescriptor, RateLimitDescriptor_Entry};
use crate::filter::http_context::Filter;
Expand Down Expand Up @@ -139,16 +140,10 @@ impl Policy {
}
// TODO(eastizle): not all fields are strings
// https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes
Some(attribute_bytes) => match String::from_utf8(attribute_bytes) {
Err(e) => {
debug!(
"#{} build_single_descriptor: failed to parse selector value: {}, error: {}",
filter.context_id, attribute_path, e
);
return None;
}
Ok(attribute_value) => attribute_value,
},
Some(attribute_bytes) => Attribute::parse(attribute_bytes)
.inspect_err(|e| debug!("#{} build_single_descriptor: failed to parse selector value: {}, error: {}",
filter.context_id, attribute_path, e))
.ok()?,
};
let mut descriptor_entry = RateLimitDescriptor_Entry::new();
descriptor_entry.set_key(descriptor_key);
Expand Down
Loading