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

perf: Reduce template compilation time by ~16% #206

Merged
merged 6 commits into from
Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 2 additions & 1 deletion constraint/cmd/rewrite-compatibility/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ func compileSrcs(
libs []string,
newPkgPrefix string,
oldRoot string,
newRoot string) error {
newRoot string,
) error {
if len(cts) == 0 && len(libs) == 0 {
return fmt.Errorf("must specify --ct or --lib or both")
}
Expand Down
1 change: 1 addition & 0 deletions constraint/pkg/client/client_addtemplate_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ func BenchmarkClient_AddTemplate_Parallel(b *testing.B) {
for i := range cts {
cts[i] = makeConstraintTemplate(i, tc.module, tc.libs...)
}
b.ResetTimer()

for i := 0; i < b.N; i++ {
b.StopTimer()
Expand Down
80 changes: 39 additions & 41 deletions constraint/pkg/client/drivers/local/compilers.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,37 +116,29 @@ func (d *Compilers) list() map[string]map[string]*ast.Compiler {
return result
}

type TargetModule struct {
Rego string
Libs []string
}

// parseConstraintTemplate validates the rego in template target by parsing
// rego modules.
func parseConstraintTemplate(templ *templates.ConstraintTemplate, externs []string) (map[string]TargetModule, error) {
func parseConstraintTemplate(templ *templates.ConstraintTemplate, externs []string) (map[string][]*ast.Module, error) {
rr, err := regorewriter.New(regorewriter.NewPackagePrefixer(templateLibPrefix), []string{libRoot}, externs)
if err != nil {
return nil, fmt.Errorf("creating rego rewriter: %w", err)
}

mods := make(map[string]TargetModule)
mods := make(map[string][]*ast.Module)
for _, target := range templ.Spec.Targets {
targetMods, err := parseConstraintTemplateTarget(rr, target)
if err != nil {
return nil, err
}

mods[target.Target] = TargetModule{
Rego: target.Rego,
Libs: targetMods,
}
mods[target.Target] = targetMods
}

return mods, nil
}

func parseConstraintTemplateTarget(rr *regorewriter.RegoRewriter, targetSpec templates.Target) ([]string, error) {
entryPoint, err := parseModule(targetSpec.Rego)
func parseConstraintTemplateTarget(rr *regorewriter.RegoRewriter, targetSpec templates.Target) ([]*ast.Module, error) {
entryPoint, err := parseModule(templatePath, targetSpec.Rego)
if err != nil {
return nil, fmt.Errorf("%w: %v", clienterrors.ErrInvalidConstraintTemplate, err)
}
Expand All @@ -170,7 +162,14 @@ func parseConstraintTemplateTarget(rr *regorewriter.RegoRewriter, targetSpec tem
rr.AddEntryPointModule(templatePath, entryPoint)
for idx, libSrc := range targetSpec.Libs {
libPath := fmt.Sprintf(`%s["lib_%d"]`, templateLibPrefix, idx)
if err = rr.AddLib(libPath, libSrc); err != nil {

m, err := parseModule(libPath, libSrc)
if err != nil {
return nil, fmt.Errorf("%w: %v",
clienterrors.ErrInvalidConstraintTemplate, err)
}

if err = rr.AddLib(libPath, m); err != nil {
return nil, fmt.Errorf("%w: %v",
clienterrors.ErrInvalidConstraintTemplate, err)
}
Expand All @@ -182,44 +181,28 @@ func parseConstraintTemplateTarget(rr *regorewriter.RegoRewriter, targetSpec tem
clienterrors.ErrInvalidConstraintTemplate, err)
}

var mods []string
err = sources.ForEachModule(func(m *regorewriter.Module) error {
content, err2 := m.Content()
if err2 != nil {
return err2
}
mods = append(mods, string(content))
return nil
})

if err != nil {
return nil, fmt.Errorf("%w: %v",
clienterrors.ErrInvalidConstraintTemplate, err)
var mods []*ast.Module
for _, m := range sources.EntryPoints {
mods = append(mods, m.Module)
}
for _, m := range sources.Libs {
mods = append(mods, m.Module)
}

return mods, nil
}

func compileTemplateTarget(module TargetModule, capabilities *ast.Capabilities, printEnabled bool) (*ast.Compiler, error) {
func compileTemplateTarget(module []*ast.Module, capabilities *ast.Capabilities, printEnabled bool) (*ast.Compiler, error) {
compiler := ast.NewCompiler().
WithCapabilities(capabilities).
WithEnablePrintStatements(printEnabled)

modules := make(map[string]*ast.Module)

builtinModule, err := ast.ParseModule(hookModulePath, hookModule)
if err != nil {
return nil, fmt.Errorf("%w: %v", clienterrors.ErrParse, err)
}
modules[hookModulePath] = builtinModule
modules := make(map[string]*ast.Module, len(module)+1)
modules[hookModulePath] = hookModule

for i, lib := range module.Libs {
for i, lib := range module {
libPath := fmt.Sprintf("%s%d", templatePath, i)
libModule, err := ast.ParseModule(libPath, lib)
if err != nil {
return nil, fmt.Errorf("%w: %v", clienterrors.ErrParse, err)
}
modules[libPath] = libModule
modules[libPath] = lib
}

compiler.Compile(modules)
Expand All @@ -229,3 +212,18 @@ func compileTemplateTarget(module TargetModule, capabilities *ast.Capabilities,

return compiler, nil
}

// parseModule parses the module and also fails empty modules.
func parseModule(path, rego string) (*ast.Module, error) {
module, err := ast.ParseModule(path, rego)
if err != nil {
return nil, err
}

if module == nil {
return nil, fmt.Errorf("%w: module %q is empty",
clienterrors.ErrInvalidModule, templatePath)
}

return module, nil
}
15 changes: 0 additions & 15 deletions constraint/pkg/client/drivers/local/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,21 +307,6 @@ func (d *Driver) Dump(ctx context.Context) (string, error) {
return string(b), nil
}

// parseModule parses the module and also fails empty modules.
func parseModule(rego string) (*ast.Module, error) {
module, err := ast.ParseModule(templatePath, rego)
if err != nil {
return nil, err
}

if module == nil {
return nil, fmt.Errorf("%w: module %q is empty",
clienterrors.ErrInvalidModule, templatePath)
}

return module, nil
}

// rewriteModulePackage rewrites the module's package path to path.
func rewriteModulePackage(module *ast.Module) error {
pathParts := ast.Ref([]*ast.Term{ast.VarTerm(templatePath)})
Expand Down
14 changes: 13 additions & 1 deletion constraint/pkg/client/drivers/local/rego.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package local

import "github.com/open-policy-agent/opa/ast"

const (
// templatePath is the path the Template's Rego code is stored.
// Must match the "data.xxx.violation[r]" path in hookModule.
Expand All @@ -16,7 +18,7 @@ const (
// This removes boilerplate that would otherwise need to be present in every
// Template's Rego code. The violation's response is written to a standard
// location we can read from to see if any violations occurred.
hookModule = `
hookModuleRego = `
package hooks

# Determine if the object under review violates any passed Constraints.
Expand Down Expand Up @@ -45,3 +47,13 @@ violation[response] {

`
)

var hookModule *ast.Module

func init() {
var err error
hookModule, err = parseModule(hookModulePath, hookModuleRego)
if err != nil {
panic(err)
}
}
35 changes: 15 additions & 20 deletions constraint/pkg/client/drivers/to_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,47 +37,42 @@ func ToResults(constraints map[ConstraintKey]*unstructured.Unstructured, resultS
func ToResult(constraints map[ConstraintKey]*unstructured.Unstructured, r rego.Result) (*types.Result, error) {
result := &types.Result{}

resultMapBinding, found := r.Bindings["result"]
resultMap, found, err := unstructured.NestedMap(r.Bindings, "result")
if err != nil {
return nil, fmt.Errorf("extracting result binding: %v", err)
}

if !found {
return nil, errors.New("no binding for result")
}

resultMap, ok := resultMapBinding.(map[string]interface{})
if !ok {
return nil, fmt.Errorf("result binding was %T but want %T",
resultMapBinding, map[string]interface{}{})
message, found, err := unstructured.NestedString(resultMap, "msg")
if err != nil {
return nil, fmt.Errorf("extracting message binding: %v", err)
}

messageBinding, found := resultMap["msg"]
if !found {
return nil, errors.New("no binding for msg")
}

message, ok := messageBinding.(string)
if !ok {
return nil, fmt.Errorf("message binding was %T but want %T",
messageBinding, "")
}
result.Msg = message

result.Metadata = map[string]interface{}{
"details": resultMap["details"],
}

keyBinding, found := resultMap["key"]
if !found {
return nil, errors.New("no binding for Constraint key")
keyMap, found, err := unstructured.NestedStringMap(resultMap, "key")
if err != nil {
return nil, fmt.Errorf("extracting key binding: %v", err)
}

keyMap, ok := keyBinding.(map[string]interface{})
if !ok {
return nil, fmt.Errorf("key binding was %T but want %T",
keyBinding, map[string]interface{}{})
if !found {
return nil, errors.New("no binding for Constraint key")
}

key := ConstraintKey{
Kind: keyMap["kind"].(string),
Name: keyMap["name"].(string),
Kind: keyMap["kind"],
Name: keyMap["name"],
}

constraint := constraints[key]
Expand Down
2 changes: 1 addition & 1 deletion constraint/pkg/client/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func TestClient_Review(t *testing.T) {
toReview: handlertest.NewReview("", "foo", "qux"),
wantResults: []*types.Result{{
Target: handlertest.TargetName,
Msg: `template0:7: eval_conflict_error: functions must not produce multiple outputs for same inputs`,
Msg: `template:8: eval_conflict_error: functions must not produce multiple outputs for same inputs`,
EnforcementAction: constraints.EnforcementActionDeny,
Constraint: cts.MakeConstraint(t, clienttest.KindRuntimeError, "constraint"),
}},
Expand Down
48 changes: 0 additions & 48 deletions constraint/pkg/client/rego_helpers.go

This file was deleted.

71 changes: 0 additions & 71 deletions constraint/pkg/client/rego_helpers_test.go

This file was deleted.

Loading