Add helper functions for ipcidr #102
Conversation
Issacwww
changed the title
There was a problem hiding this comment.
Thanks so much for your contribution! The test cases are great.
I have a few style nits around error handling and such. I think the big change I'd really like to see is to move to using the cidr crate for CIDR range parsing, then for your helper functions, internally lean on the types exposed by that and the standard library before finally converting to Strings at the end of the helper functions.
sources/api/schnauzer/src/helpers.rs
Outdated
| @@ -907,6 +910,128 @@ pub fn goarch( | |||
| Ok(()) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
For the helpers that you created, you will need to add them to the template registries that are used when templates are rendered. There are currently two codepaths that set up registries, and adding the functions to both is fine:
sources/api/schnauzer/src/helpers.rs
Outdated
| Ok(()) | ||
| } | ||
|
|
||
| fn validate_and_parse_cidr(cidr: &str) -> Result<&str, RenderError> { |
There was a problem hiding this comment.
Rather than using String return types, could we add a dependency on the cidr crate and then use the AnyIpCidr enum for parsing?
AnyIpCidr implements FromStr, so you should be able to do something like:
let parsed_cidr: AnyIpCidr = cidr.parse()
.context(error::InvalidCidrSnafu { cidr: cidr.to_string() })?;For this to work, you would also need to modify the InvalidCidr error to have a source of type cidr::errors::NetworkParseError.
I believe you could then just call first_address to get the IP part of the CIDR range, and return that as an IpAddr.
sources/api/schnauzer/src/helpers.rs
Outdated
| use serde::Serialize; | ||
| use serde_json::json; | ||
|
|
||
| const TEMPLATE: &str = r#" |
|
Open a new one #116 that resolved merge conflict and comments |
Description of changes:
Add new helper function for EKS to config the template
Testing done:
Added unit test and
cargo testpassedTerms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.