From 6f8abdd52cd1b0ed00738bd35b0f1b27d57da074 Mon Sep 17 00:00:00 2001 From: Max Smythe Date: Wed, 4 May 2022 13:04:29 -0700 Subject: [PATCH] Make sure that the Rego hook is well-behaved with no data cache Fixes https://github.com/open-policy-agent/gatekeeper/issues/2026 Signed-off-by: Max Smythe --- constraint/pkg/client/client_test.go | 2 +- constraint/pkg/client/drivers/local/driver.go | 34 +++++++---- .../client/drivers/local/driver_unit_test.go | 58 +++++++++++++++++++ constraint/pkg/client/drivers/local/rego.go | 9 ++- 4 files changed, 89 insertions(+), 14 deletions(-) diff --git a/constraint/pkg/client/client_test.go b/constraint/pkg/client/client_test.go index d1df32734..903a3cccc 100644 --- a/constraint/pkg/client/client_test.go +++ b/constraint/pkg/client/client_test.go @@ -821,7 +821,7 @@ func TestClient_RemoveTemplate_CascadingDelete(t *testing.T) { sLower := strings.ToLower(s) if strings.Contains(sLower, "cascadingdelete") { - t.Errorf("Template not removed from cache: %s", s) + t.Errorf("Constraint not removed from cache: %s", s) } finalPreserved := strings.Count(sLower, "stillpersists") diff --git a/constraint/pkg/client/drivers/local/driver.go b/constraint/pkg/client/drivers/local/driver.go index 2d65773de..060c1c8d0 100644 --- a/constraint/pkg/client/drivers/local/driver.go +++ b/constraint/pkg/client/drivers/local/driver.go @@ -90,7 +90,7 @@ func (d *Driver) AddTemplate(ctx context.Context, templ *templates.ConstraintTem func (d *Driver) RemoveTemplate(ctx context.Context, templ *templates.ConstraintTemplate) error { kind := templ.Spec.CRD.Spec.Names.Kind - constraintParent := storage.Path{"constraint", kind} + constraintParent := storage.Path{"constraints", kind} d.mtx.Lock() defer d.mtx.Unlock() @@ -282,28 +282,38 @@ func (d *Driver) Query(ctx context.Context, target string, constraints []*unstru } func (d *Driver) Dump(ctx context.Context) (string, error) { - dt := make(map[string]map[string]rego.ResultSet) + // we want to create: + // targetName.modules.kind.moduleName = contents + // targetName.data = data + dt := make(map[string]map[string]interface{}) compilers := d.compilers.list() for targetName, targetCompilers := range compilers { - targetData := make(map[string]rego.ResultSet) + targetModules := make(map[string]map[string]string) for kind, compiler := range targetCompilers { - rs, _, err := d.eval(ctx, compiler, targetName, []string{"data"}, nil) - if err != nil { - return "", err + kindModules := make(map[string]string) + for modname, contents := range compiler.Modules { + kindModules[modname] = contents.String() } - targetData[kind] = rs + targetModules[kind] = kindModules } + dt[targetName] = map[string]interface{}{} + dt[targetName]["modules"] = targetModules - dt[targetName] = targetData - } + emptyCompiler := ast.NewCompiler().WithCapabilities(d.compilers.capabilities) - resp := map[string]interface{}{ - "data": dt, + rs, _, err := d.eval(ctx, emptyCompiler, targetName, []string{}, nil) + if err != nil { + return "", err + } + + if len(rs) != 0 && len(rs[0].Expressions) != 0 { + dt[targetName]["data"] = rs[0].Expressions[0].Value + } } - b, err := json.MarshalIndent(resp, "", " ") + b, err := json.MarshalIndent(dt, "", " ") if err != nil { return "", err } diff --git a/constraint/pkg/client/drivers/local/driver_unit_test.go b/constraint/pkg/client/drivers/local/driver_unit_test.go index b00a9a005..592048029 100644 --- a/constraint/pkg/client/drivers/local/driver_unit_test.go +++ b/constraint/pkg/client/drivers/local/driver_unit_test.go @@ -27,9 +27,67 @@ fooisbar[msg] { input.foo == "bar" msg := "input.foo is bar" } +` + + AlwaysViolate string = ` + package foobar + + violation[{"msg": "always violate"}] { + true + } ` ) +func TestDriver_Query(t *testing.T) { + d, err := New() + if err != nil { + t.Fatal(err) + } + + tmpl := cts.New(cts.OptTargets(cts.Target(cts.MockTargetHandler, AlwaysViolate))) + ctx := context.Background() + + if err := d.AddTemplate(ctx, tmpl); err != nil { + t.Fatalf("got AddTemplate() error = %v, want %v", err, nil) + } + + if err := d.AddConstraint(ctx, cts.MakeConstraint(t, "Fakes", "foo-1")); err != nil { + t.Fatalf("got AddConstraint() error = %v, want %v", err, nil) + } + + res, _, err := d.Query( + ctx, + cts.MockTargetHandler, + []*unstructured.Unstructured{cts.MakeConstraint(t, "Fakes", "foo-1")}, + map[string]interface{}{"hi": "there"}, + ) + if err != nil { + t.Fatalf("got Query() error = %v, want %v", err, nil) + } + if len(res) == 0 { + t.Fatalf("got 0 errors on normal query; want 1") + } + + // Remove data to make sure our rego hook is well-behaved when + // there is no external data root + if err := d.RemoveData(ctx, cts.MockTargetHandler, nil); err != nil { + t.Fatalf("got RemoveData() error = %v, want %v", err, nil) + } + + res, _, err = d.Query( + ctx, + cts.MockTargetHandler, + []*unstructured.Unstructured{cts.MakeConstraint(t, "Fakes", "foo-1")}, + map[string]interface{}{"hi": "there"}, + ) + if err != nil { + t.Fatalf("got Query() (#2) error = %v, want %v", err, nil) + } + if len(res) == 0 { + t.Fatalf("got 0 errors on data-less query; want 1") + } +} + func TestDriver_AddTemplate(t *testing.T) { testCases := []struct { name string diff --git a/constraint/pkg/client/drivers/local/rego.go b/constraint/pkg/client/drivers/local/rego.go index 2760bd129..9b016ef5b 100644 --- a/constraint/pkg/client/drivers/local/rego.go +++ b/constraint/pkg/client/drivers/local/rego.go @@ -34,7 +34,8 @@ violation[response] { } # Run the Template with Constraint. - data.template.violation[r] with input as inp with data.inventory as data.external + inventory[inv] + data.template.violation[r] with input as inp with data.inventory as inv # Construct the response, defaulting "details" to empty object if it is not # specified. @@ -45,6 +46,12 @@ violation[response] { } } +inventory[inv] { + inv = data.external +} +inventory[{}] { + not data.external +} ` )