Skip to content

Commit

Permalink
Remove map_proto_enum and simplify proto enum conversions (#650)
Browse files Browse the repository at this point in the history
Co-authored-by: Mark Mandel <[email protected]>
  • Loading branch information
XAMPPRocky and markmandel authored Nov 29, 2022
1 parent 667caa1 commit f514c0f
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 310 deletions.
86 changes: 0 additions & 86 deletions src/filters/compress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,92 +158,6 @@ mod tests {
use crate::{endpoint::Endpoint, filters::compress::compressor::Snappy};

use super::*;
use proto::compress::{Action as ProtoAction, ActionValue, Mode as ProtoMode, ModeValue};

#[test]
fn convert_proto_config() {
let test_cases = vec![
(
"should succeed when all valid values are provided",
proto::Compress {
mode: Some(ModeValue {
value: ProtoMode::Snappy as i32,
}),
on_read: Some(ActionValue {
value: ProtoAction::Compress as i32,
}),
on_write: Some(ActionValue {
value: ProtoAction::Decompress as i32,
}),
},
Some(Config {
mode: Mode::Snappy,
on_read: Action::Compress,
on_write: Action::Decompress,
}),
),
(
"should fail when invalid mode is provided",
proto::Compress {
mode: Some(ModeValue { value: 42 }),
on_read: Some(ActionValue {
value: ProtoAction::Compress as i32,
}),
on_write: Some(ActionValue {
value: ProtoAction::Decompress as i32,
}),
},
None,
),
(
"should fail when invalid on_read is provided",
proto::Compress {
mode: Some(ModeValue {
value: ProtoMode::Snappy as i32,
}),
on_read: Some(ActionValue { value: 73 }),
on_write: Some(ActionValue {
value: ProtoAction::Decompress as i32,
}),
},
None,
),
(
"should fail when invalid on_write is provided",
proto::Compress {
mode: Some(ModeValue {
value: ProtoMode::Snappy as i32,
}),
on_read: Some(ActionValue {
value: ProtoAction::Decompress as i32,
}),
on_write: Some(ActionValue { value: 73 }),
},
None,
),
(
"should use correct default values",
proto::Compress {
mode: None,
on_read: None,
on_write: None,
},
Some(Config::default()),
),
];
for (name, proto_config, expected) in test_cases {
let result = Config::try_from(proto_config);
assert_eq!(
result.is_err(),
expected.is_none(),
"{}: error expectation does not match",
name
);
if let Some(expected) = expected {
assert_eq!(expected, result.unwrap(), "{}", name);
}
}
}

#[test]
fn default_mode_factory() {
Expand Down
67 changes: 28 additions & 39 deletions src/filters/compress/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
* limitations under the License.
*/

use std::convert::TryFrom;

use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

Expand All @@ -24,7 +22,6 @@ use super::quilkin::filters::compress::v1alpha1::{
compress::{Action as ProtoAction, ActionValue, Mode as ProtoMode, ModeValue},
Compress as ProtoConfig,
};
use crate::{filters::ConvertProtoConfigError, map_proto_enum};

/// The library to use when compressing.
#[derive(Clone, Copy, Deserialize, Debug, Eq, PartialEq, Serialize, JsonSchema)]
Expand Down Expand Up @@ -58,6 +55,14 @@ impl From<Mode> for ProtoMode {
}
}

impl From<ProtoMode> for Mode {
fn from(mode: ProtoMode) -> Self {
match mode {
ProtoMode::Snappy => Self::Snappy,
}
}
}

impl From<Mode> for ModeValue {
fn from(mode: Mode) -> Self {
ModeValue {
Expand Down Expand Up @@ -93,6 +98,16 @@ impl From<Action> for ProtoAction {
}
}

impl From<ProtoAction> for Action {
fn from(action: ProtoAction) -> Self {
match action {
ProtoAction::DoNothing => Self::DoNothing,
ProtoAction::Compress => Self::Compress,
ProtoAction::Decompress => Self::Decompress,
}
}
}

impl From<Action> for ActionValue {
fn from(action: Action) -> Self {
Self {
Expand Down Expand Up @@ -120,56 +135,30 @@ impl From<Config> for ProtoConfig {
}
}

impl TryFrom<ProtoConfig> for Config {
type Error = ConvertProtoConfigError;

fn try_from(p: ProtoConfig) -> std::result::Result<Self, Self::Error> {
impl From<ProtoConfig> for Config {
fn from(p: ProtoConfig) -> Self {
let mode = p
.mode
.map(|mode| {
map_proto_enum!(
value = mode.value,
field = "mode",
proto_enum_type = ProtoMode,
target_enum_type = Mode,
variants = [Snappy]
)
})
.transpose()?
.map(|p| p.value())
.map(Mode::from)
.unwrap_or_default();

let on_read = p
.on_read
.map(|on_read| {
map_proto_enum!(
value = on_read.value,
field = "on_read",
proto_enum_type = ProtoAction,
target_enum_type = Action,
variants = [DoNothing, Compress, Decompress]
)
})
.transpose()?
.map(|p| p.value())
.map(Action::from)
.unwrap_or_default();

let on_write = p
.on_write
.map(|on_write| {
map_proto_enum!(
value = on_write.value,
field = "on_write",
proto_enum_type = ProtoAction,
target_enum_type = Action,
variants = [DoNothing, Compress, Decompress]
)
})
.transpose()?
.map(|p| p.value())
.map(Action::from)
.unwrap_or_default();

Ok(Self {
Self {
mode,
on_read,
on_write,
})
}
}
}
48 changes: 18 additions & 30 deletions src/filters/concatenate_bytes/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,10 @@
* limitations under the License.
*/

use std::convert::TryFrom;

use base64_serde::base64_serde_type;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use crate::{filters::prelude::*, map_proto_enum};

use super::proto;

base64_serde_type!(Base64Standard, base64::STANDARD);
Expand Down Expand Up @@ -52,6 +48,16 @@ impl From<Strategy> for proto::concatenate_bytes::Strategy {
}
}

impl From<proto::concatenate_bytes::Strategy> for Strategy {
fn from(strategy: proto::concatenate_bytes::Strategy) -> Self {
match strategy {
proto::concatenate_bytes::Strategy::Append => Self::Append,
proto::concatenate_bytes::Strategy::Prepend => Self::Prepend,
proto::concatenate_bytes::Strategy::DoNothing => Self::DoNothing,
}
}
}

impl From<Strategy> for proto::concatenate_bytes::StrategyValue {
fn from(strategy: Strategy) -> Self {
Self {
Expand Down Expand Up @@ -88,42 +94,24 @@ impl From<Config> for proto::ConcatenateBytes {
}
}

impl TryFrom<proto::ConcatenateBytes> for Config {
type Error = ConvertProtoConfigError;

fn try_from(p: proto::ConcatenateBytes) -> Result<Self, Self::Error> {
impl From<proto::ConcatenateBytes> for Config {
fn from(p: proto::ConcatenateBytes) -> Self {
let on_read = p
.on_read
.map(|strategy| {
map_proto_enum!(
value = strategy.value,
field = "on_read",
proto_enum_type = proto::concatenate_bytes::Strategy,
target_enum_type = Strategy,
variants = [DoNothing, Append, Prepend]
)
})
.transpose()?
.map(|p| p.value())
.map(Strategy::from)
.unwrap_or_default();

let on_write = p
.on_write
.map(|strategy| {
map_proto_enum!(
value = strategy.value,
field = "on_write",
proto_enum_type = proto::concatenate_bytes::Strategy,
target_enum_type = Strategy,
variants = [DoNothing, Append, Prepend]
)
})
.transpose()?
.map(|p| p.value())
.map(Strategy::from)
.unwrap_or_default();

Ok(Self {
Self {
on_read,
on_write,
bytes: p.bytes,
})
}
}
}
53 changes: 0 additions & 53 deletions src/filters/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,56 +113,3 @@ impl ConvertProtoConfigError {
}
}
}

/// Returns a [`ConvertProtoConfigError`] with an error message when
/// an invalid proto enum value was provided in a filter's proto config.
#[macro_export]
macro_rules! enum_no_match_error {
(
field = $field:literal,
invalid_value = $invalid_value:ident,
enum_type = $enum_type:ty,
allowed_values = [ $( $allowed_value:tt ),+ ]
) => {
Err($crate::filters::error::ConvertProtoConfigError::new(
format!(
"invalid value `{}` provided: allowed values are {}",
$invalid_value,
vec![
$( (stringify!($allowed_value), <$enum_type>::$allowed_value as i32) ),+
]
.into_iter()
.map(|(a, b)| format!("{a} => {}", b as i32))
.collect::<Vec<_>>()
.join(", ")
),
Some($field.into()),
))
};
}

/// Maps an integer from a protobuf enum value to a target enum variant.
/// Both protobuf and target enum must have similar variants.
/// The protobuf enum variant should be cast-able to an i32
/// Returns an `OK` Result with the target enum variant otherwise [`ConvertProtoConfigError`]
/// if the provided value does not map to any enum variant.
#[macro_export]
macro_rules! map_proto_enum {
(
value = $value:expr,
field = $field:literal,
proto_enum_type = $proto_enum_type:ty,
target_enum_type = $target_enum_type:ty,
variants = [ $( $variant:tt ),+ ]
) => {
match $value {
$( v if v == <$proto_enum_type>::$variant as i32 => Ok(<$target_enum_type>::$variant) ),+,
invalid => $crate::enum_no_match_error!(
field = $field,
invalid_value = invalid,
enum_type = $proto_enum_type,
allowed_values = [ $( $variant ),+ ]
)
}
}
}
20 changes: 11 additions & 9 deletions src/filters/firewall/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use schemars::JsonSchema;
use serde::de::{self, Visitor};
use serde::{Deserialize, Deserializer, Serialize, Serializer};

use crate::{filters::ConvertProtoConfigError, map_proto_enum};
use crate::filters::ConvertProtoConfigError;

use super::proto;

Expand Down Expand Up @@ -54,6 +54,15 @@ impl From<Action> for proto::firewall::Action {
}
}

impl From<proto::firewall::Action> for Action {
fn from(action: proto::firewall::Action) -> Self {
match action {
proto::firewall::Action::Allow => Self::Allow,
proto::firewall::Action::Deny => Self::Deny,
}
}
}

/// Combination of CIDR range, port range and action to take.
#[derive(Clone, Deserialize, Debug, Eq, PartialEq, Serialize, JsonSchema)]
pub struct Rule {
Expand Down Expand Up @@ -238,14 +247,7 @@ impl TryFrom<proto::Firewall> for Config {
}

fn convert_rule(rule: &proto::firewall::Rule) -> Result<Rule, ConvertProtoConfigError> {
let action = map_proto_enum!(
value = rule.action,
field = "policy",
proto_enum_type = proto::firewall::Action,
target_enum_type = Action,
variants = [Allow, Deny]
)?;

let action = Action::from(rule.action());
let source = IpNetwork::try_from(rule.source.as_str()).map_err(|err| {
ConvertProtoConfigError::new(
format!("invalid source: {err:?}"),
Expand Down
Loading

0 comments on commit f514c0f

Please sign in to comment.