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.

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

[Enhancement]: Adding support for calling functions inline #400

Merged
merged 6 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 23 additions & 5 deletions docs/KNOWN_ISSUES.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ let api_gws = Resources.*[ Type == 'AWS::ApiGateway::RestApi' ]

2. When performing `!=` comparison, if the values are incompatible like comparing a `string` to `int`, an error is thrown internally but currently suppressed and converted to `false` to satisfy the requirements of Rust’s [PartialEq](https://doc.rust-lang.org/std/cmp/trait.PartialEq.html). We are tracking to release a fix for this issue soon.
3. `exists` and `empty` checks do not display the JSON pointer path inside the document in the error messages. Both these clauses often have retrieval errors which does not maintain this traversal information today. We are tracking to resolve this issue.
4. <a name="function-limitation"></a> **No support for inline functions**
4. <a name="function-limitation"></a> **No support for calling functions inline on the LHS of an operator**

We **do not** support inline usage of functions at the moment. The support for built-in functions is currently limited to assignment of the return value to a variable.
We **do not** support inline usage of functions on the LHS of operators at the moment. The support for built-in functions when being used on the LHS of an operator is currently limited to assignment of the return value to a variable.

Consider an example wherein our template has a node named `Instances` which is a collection. We need to author a rule that checks to ensure this collection contains a certain number of minimum items, say 2.

This is currently **NOT SUPPORTED**:
These following examples are currently **NOT SUPPORTED**:

```
# Not supported at the moment
Expand All @@ -48,11 +48,20 @@ let api_gws = Resources.*[ Type == 'AWS::ApiGateway::RestApi' ]
count(Instances.*) < 2
<< Violation: We should have at least 2 instances >>
}

# OR

rule VERIFY_COUNT_RETURNS_INT {
count(Instances.*) is_int
<< Violation: We should have at least 2 instances >>
}
```

While the above code snippet might be tempting to use as it's more intuitive, we haven't made the changes required to support it in our grammar yet.
While the above code snippet might be tempting to use we haven't made the changes required to support it in our grammar yet.

> **Workaround**: Assign function value to a variable and then use this variable thereafter in all clauses that follow including the conditions.
> **Workaround**:
> When working with unary operators. assign the result of the function to a variable.
> When working with binary operators, you have the choice to have the function call on the RHS of the operator, or assign the result of the function call to a variable, and then you are free to have this variable on either side of the operator.

So, our example rule now becomes:

Expand All @@ -62,9 +71,18 @@ let api_gws = Resources.*[ Type == 'AWS::ApiGateway::RestApi' ]
rule INSTANCES_COUNT_CHECK {
let no_of_instances = count(Instances.*)

# all of the following options are valid ways to write this clause
%no_of_instances < 2
2 > %no_of_instances
2 > count(Instances.*)
<< Violation: We should have at least 2 instances >>
}

rule VERIFY_COUNT_RETURNS_INT {
let no_of_instances = count(Instances.*)
%no_of_instances is_int
}

```

5. Key names containing dashes
Expand Down
34 changes: 34 additions & 0 deletions guard/resources/validate/functions/output/failing_complex_rule.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
template.yaml Status = FAIL
FAILED rules
failing_complex_rule.guard/PARAMETERIZED_RULE_WITH_FUNCTION_CALL_IN_PARAMS FAIL
---
Evaluating data template.yaml against rules failing_complex_rule.guard
Number of non-compliant resources 1
Resource = newServer {
Type = AWS::New::Service
Rule = PARAMETERIZED_RULE_WITH_FUNCTION_CALL_IN_PARAMS {
ALL {
Rule = compare_result_of_regex_replace {
ALL {
Check = %expected EQUALS %replaced {
ComparisonError {
Error = Check was not compliant as property [/Resources/newServer/Properties/Arn[L:9,C:11]] was not present in [(resolved, Path=/Resources/newServer/Properties/Arn[L:9,C:11] Value="aws/123456789012/us-west-2/newservice-Table/extracted")]
}
PropertyPath = /Resources/newServer/Properties/Arn[L:9,C:11]
Operator = EQUAL
Value = "aws/123456789012/us-west-2/newservice-Table/extracted"
ComparedWith = ["aws/123456789012/us-west-2/newservice-Table/extracted"]
Code:
7. "Principal": "*",
8. "Actions": ["s3*", "ec2*"]
9. }
10. Arn: arn:aws:newservice:us-west-2:123456789012:Table/extracted
11. Encoded: This%20string%20will%20be%20URL%20encoded
12. Collection:

}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Resource = newServer {
ALL {
Check = %res EQUALS "a,b" {
ComparisonError {
Message = Violation: The joined value does not match the expected result
Error = Check was not compliant as property value [Path=/Resources/newServer/Collection/0[L:12,C:8] Value="a,b,c"] not equal to value [Path=[L:0,C:0] Value="a,b"].
PropertyPath = /Resources/newServer/Collection/0[L:12,C:8]
Operator = EQUAL
Expand All @@ -26,6 +25,24 @@ Resource = newServer {

}
}
Check = a,b EQUALS join(%collection, ",") {
ComparisonError {
Message = Violation: The joined value does not match the expected result
Error = Check was not compliant as property [/Resources/newServer/Collection/0[L:12,C:8]] was not present in [(resolved, Path=/Resources/newServer/Collection/0[L:12,C:8] Value="a,b,c")]
}
PropertyPath = /Resources/newServer/Collection/0[L:12,C:8]
Operator = EQUAL
Value = "a,b,c"
ComparedWith = ["a,b,c"]
Code:
10. Arn: arn:aws:newservice:us-west-2:123456789012:Table/extracted
11. Encoded: This%20string%20will%20be%20URL%20encoded
12. Collection:
13. - a
14. - b
15. - c

}
}
}
}
21 changes: 21 additions & 0 deletions guard/resources/validate/functions/rules/complex_rules.guard
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
rule compare_number_of_buckets(expected) {
%expected == 5
}

let buckets = Resources.*[ Type == 'AWS::S3::Bucket' ]

rule COMBINED_FUNCTION_AND_PARAMETERIZED_RULES when %buckets !empty {
compare_number_of_buckets(count(%buckets))
}

rule compare_result_of_regex_replace(replaced, expected) {
%replaced == %expected
}

let template = Resources.*[ Type == 'AWS::New::Service']

rule PARAMETERIZED_RULE_WITH_FUNCTION_CALL_IN_PARAMS when %template exists {
let arn = %template.Properties.Arn
let expected = "aws/123456789012/us-west-2/newservice-Table/extracted"
compare_result_of_regex_replace(regex_replace(%arn, "^arn:(\w+):(\w+):([\w0-9-]+):(\d+):(.+)$", "${1}/${4}/${3}/${2}-${5}"), %expected)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
rule compare_result_of_regex_replace(replaced, expected) {
%expected == %replaced
}

let template = Resources.*[ Type == 'AWS::New::Service']

rule PARAMETERIZED_RULE_WITH_FUNCTION_CALL_IN_PARAMS when %template exists {
let arn = %template.Properties.Arn
compare_result_of_regex_replace(regex_replace(%arn, "^arn:(\w+):(\w+):([\w0-9-]+):(\d+):(.+)$", "${1}/${4}/${3}/${2}-${5}"), "random_str")
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,8 @@ rule TEST_COLLECTION when %template !empty {

let res = join(%collection, ",")
%res == "a,b"

"a,b" == join(%collection, ",")
joshfried-aws marked this conversation as resolved.
Show resolved Hide resolved
<< Violation: The joined value does not match the expected result >>
}
}

3 changes: 3 additions & 0 deletions guard/resources/validate/functions/rules/json_parse.guard
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ rule SOME_RULE when %template !empty {

let res = json_parse(%policy)

%expected == json_parse(%policy)

%res !empty
%res == %expected

Expand All @@ -23,3 +25,4 @@ rule SOME_RULE when %template !empty {
}
}


7 changes: 1 addition & 6 deletions guard/src/commands/validate/tf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,7 @@ impl<'reporter> Reporter for TfAware<'reporter> {
output_type: OutputFormatType,
) -> crate::rules::Result<()> {
let root = data.root().unwrap();
let is_tf_plan = match data.at("/resource_changes", root) {
Ok(_resource_changes) => matches!(data.at("/terraform_version", root), Ok(_)),
_ => false,
};

if is_tf_plan {
if data.at("/resource_changes", root).is_ok() {
let failure_report = simplifed_json_from_root(root_record)?;
match output_type {
OutputFormatType::YAML => serde_yaml::to_writer(write, &failure_report)?,
Expand Down
28 changes: 23 additions & 5 deletions guard/src/rules/eval.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::exprs::*;
use super::*;
use crate::rules::eval::operators::Comparator;
use crate::rules::eval_context::{block_scope, ValueScope};
use crate::rules::eval_context::{block_scope, resolve_function, ValueScope};
use crate::rules::path_value::compare_eq;
use std::collections::HashMap;

Expand Down Expand Up @@ -1112,9 +1112,23 @@ pub(in crate::rules) fn eval_guard_access_clause<'value, 'loc: 'value>(
return Err(e);
}
},
LetValue::FunctionCall(_) => todo!(),
LetValue::FunctionCall(FunctionExpr {
parameters, name, ..
}) => match resolve_function(name, parameters, resolver) {
Ok(result) => (result, false),
Err(e) => {
resolver.end_record(
&blk_context,
RecordType::GuardClauseBlockCheck(BlockCheck {
status: Status::FAIL,
at_least_one_matches: !all,
message: Some(format!("Error {e} when handling clause, bailing")),
}),
)?;
return Err(e);
}
},
},

None => {
resolver.end_record(
&blk_context,
Expand Down Expand Up @@ -1587,8 +1601,12 @@ pub(in crate::rules) fn eval_parameterized_rule_call<'value, 'loc: 'value>(
resolver.query(&query.query)?,
);
}
// TODO: when we add inline function support
LetValue::FunctionCall(_) => unimplemented!(),
LetValue::FunctionCall(FunctionExpr {
parameters, name, ..
}) => {
let result = resolve_function(name, parameters, resolver)?;
resolved_parameters.insert((param_rule.parameter_names[idx]).as_str(), result);
}
}
}
let mut eval = ResolvedParameterContext {
Expand Down
97 changes: 42 additions & 55 deletions guard/src/rules/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,9 @@ fn query_retrieval_with_converter<'value, 'loc: 'value>(
vec![QueryResult::Literal(Rc::new(path_value.clone()))]
}

LetValue::FunctionCall(_) => todo!(),
LetValue::FunctionCall(FunctionExpr {
parameters, name, ..
}) => resolve_function(name, parameters, resolver)?,
};

let lhs = map
Expand Down Expand Up @@ -1122,33 +1124,7 @@ impl<'value, 'loc: 'value> EvalContext<'value, 'loc> for RootScope<'value, 'loc>
parameters, name, ..
}) = self.scope.function_expressions.get(variable_name)
{
validate_number_of_params(name, parameters.len())?;
let args = parameters.iter().try_fold(
vec![],
|mut args, param| -> Result<Vec<Vec<QueryResult>>> {
match param {
LetValue::Value(value) => {
args.push(vec![QueryResult::Literal(Rc::new(value.clone()))])
}
LetValue::AccessClause(clause) => {
let resolved_query = self.query(&clause.query)?;
args.push(resolved_query);
}
// TODO: when we add inline function call support
_ => unimplemented!(),
}

Ok(args)
},
)?;

let result = try_handle_function_call(name, &args)?
.into_iter()
.flatten()
.map(Rc::new)
.map(QueryResult::Resolved)
.collect::<Vec<_>>();

let result = resolve_function(name, parameters, self)?;
self.scope
.resolved_variables
.insert(variable_name, result.clone());
Expand Down Expand Up @@ -1405,33 +1381,7 @@ impl<'value, 'loc: 'value, 'eval> EvalContext<'value, 'loc> for BlockScope<'valu
parameters, name, ..
}) = self.scope.function_expressions.get(variable_name)
{
validate_number_of_params(name, parameters.len())?;
let args = parameters.iter().try_fold(
vec![],
|mut args, param| -> Result<Vec<Vec<QueryResult>>> {
match param {
LetValue::Value(value) => {
args.push(vec![QueryResult::Literal(Rc::new(value.clone()))])
}
LetValue::AccessClause(clause) => {
let resolved_query = self.query(&clause.query)?;
args.push(resolved_query);
}
// TODO: when we add inline function call support
_ => unimplemented!(),
}

Ok(args)
},
)?;

let result = try_handle_function_call(name, &args)?
.into_iter()
.flatten()
.map(Rc::new)
.map(QueryResult::Resolved)
.collect::<Vec<_>>();

let result = resolve_function(name, parameters, self)?;
self.scope
.resolved_variables
.insert(variable_name, result.clone());
Expand Down Expand Up @@ -2264,6 +2214,43 @@ pub(crate) fn simplifed_json_from_root<'value>(
})
}

pub(crate) fn resolve_function<'value, 'eval, 'loc: 'value>(
name: &str,
parameters: &'value Vec<LetValue<'loc>>,
resolver: &'eval mut dyn EvalContext<'value, 'loc>,
) -> Result<Vec<QueryResult>> {
validate_number_of_params(name, parameters.len())?;
let args =
parameters
.iter()
.try_fold(vec![], |mut args, param| -> Result<Vec<Vec<QueryResult>>> {
match param {
LetValue::Value(value) => {
args.push(vec![QueryResult::Literal(Rc::new(value.clone()))])
}
LetValue::AccessClause(clause) => {
let resolved_query = resolver.query(&clause.query)?;
args.push(resolved_query);
}
LetValue::FunctionCall(FunctionExpr {
parameters, name, ..
}) => {
let result = resolve_function(name, parameters, resolver)?;
args.push(result);
}
}

Ok(args)
})?;

Ok(try_handle_function_call(name, &args)?
.into_iter()
.flatten()
.map(Rc::new)
.map(QueryResult::Resolved)
.collect::<Vec<_>>())
}

#[cfg(test)]
#[path = "eval_context_tests.rs"]
pub(super) mod eval_context_tests;