Simple Policy Compiler #924
Conversation
TristonianJones
force-pushed
the
cel-policy
branch
from
1efae68
to
cb9832e
Compare
| p.iss.ReportErrorAtID(0, err.Error()) | ||
| return nil | ||
| } | ||
| return p.parsePolicy(p, docNode.Content[0]) |
There was a problem hiding this comment.
Consider producing an error to prohibit multiple YAML documents in a single file (via ---)
| ruleRoot, _ := env.Compile("true") | ||
| opt := cel.NewStaticOptimizer(&ruleComposer{rule: rule}) | ||
| ruleExprAST, iss := opt.Optimize(env, ruleRoot) | ||
| return ruleExprAST, iss.Append(iss) |
There was a problem hiding this comment.
append doesn't appear to be needed here
| exprSrc := c.relSource(v.Expression()) | ||
| varAST, exprIss := ruleEnv.CompileSource(exprSrc) | ||
| if exprIss.Err() == nil { | ||
| ruleEnv, err = ruleEnv.Extend(cel.Variable(fmt.Sprintf("variables.%s", v.Name().Value), varAST.OutputType())) |
There was a problem hiding this comment.
Not needed now but for the future -- I imagine it might be helpful exposing an option to control the prefix for variables being exposed depending on context. For time being, you could consider pulling this out into a constant.
| val := node.Content[i+1] | ||
| if val.Style == yaml.FoldedStyle || val.Style == yaml.LiteralStyle { | ||
| val.Line++ | ||
| val.Column = key.Column + 2 |
There was a problem hiding this comment.
YAML doesn't strictly impose 2 spaces for indentations. Consider adding by 1 to allow any number of spaces to be matched. Otherwise, the compilation error will look odd if someone accidentally uses an indentation level of 1.
| condAST, condIss := ruleEnv.CompileSource(condSrc) | ||
| iss = iss.Append(condIss) | ||
| if m.HasOutput() && m.HasRule() { | ||
| iss.ReportErrorAtID(m.Condition().ID, "either output or rule may be set but not both") |
There was a problem hiding this comment.
Can we add a test for this?
| // expression value. | ||
| func (opt *ruleComposer) Optimize(ctx *cel.OptimizerContext, a *ast.AST) *ast.AST { | ||
| ruleExpr, _ := optimizeRule(ctx, opt.rule) | ||
| ctx.UpdateExpr(a.Expr(), ruleExpr) |
There was a problem hiding this comment.
Does this handle setting multiple macro calls (e.g: multiple binds)? I maybe wrong, but it seemed like this only handles updates of a single macro expression. Or does this just have to be moved to within the loop of where new bind macros are created? Either way, you may want to add unparser tests for compiled policies.
| // Match declares a condition (defaults to true) as well as an output or a rule. | ||
| // Either the output or the rule field may be set, but not both. | ||
| type Match struct { | ||
| condition ValueString |
There was a problem hiding this comment.
Any reason for condition being a non-pointer?
| } | ||
|
|
||
| func (defaultTagVisitor) VariableTag(ctx ParserContext, id int64, fieldName string, node *yaml.Node, v *Variable) { | ||
| ctx.ReportErrorAtID(id, "unsupported match tag: %s", fieldName) |
There was a problem hiding this comment.
| ctx.ReportErrorAtID(id, "unsupported match tag: %s", fieldName) | |
| ctx.ReportErrorAtID(id, "unsupported variable tag: %s", fieldName) |
| if p.assertYamlType(id, node, yamlMap) == nil { | ||
| return policy | ||
| } | ||
| for i := 0; i < len(node.Content); i += 2 { |
There was a problem hiding this comment.
I'm reasonably certain that the parser would enforce the yaml tags to be a key-value pair, but it would help to either add a check for this or at least a comment on why +=2 is safe.
Introduce a module for compiling a graph of expressions into a single expression.
The input format is YAML and allows for customization of the tag handling such that
CEL-based policy formats like those found in Kubernetes Admission Control could be
supported using a common foundation.
Currently, the compilation toolchain only supports first-match semantics for expression
evaluation; however, alternative evaluation semantics such as
or,and,last-match,and
aggregateevaluation semantics may be supported in the future.