Cherry pick empty data cache fix into release 0.5 (#224)

* Make sure that the Rego hook is well-behaved with no data cache (#222)

Fixes open-policy-agent/gatekeeper#2026

Signed-off-by: Max Smythe <[email protected]>

* Upgrade linter

Signed-off-by: Max Smythe <[email protected]>

* Upgrade workflows

Signed-off-by: Max Smythe <[email protected]>

.github/workflows/gatekeeper.yml

@@ -5,20 +5,20 @@ jobs:
   lint:
     name: "Gatekeeper Test"
     runs-on: ubuntu-latest
-    timeout-minutes: 5
+    timeout-minutes: 10
     steps:
-      - name: Set up Go 1.17
-        uses: actions/setup-go@v2
+      - name: Set up Go 1.18
+        uses: actions/setup-go@v3
         with:
-          go-version: 1.17
+          go-version: 1.18
 
       - name: Check out code into the Go module directory
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           path: go/src/github.com/open-policy-agent/frameworks
 
       - name: Check out Gatekeeper default branch
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           repository: open-policy-agent/gatekeeper
           path: go/src/github.com/open-policy-agent/gatekeeper

.github/workflows/workflow.yml

@@ -7,27 +7,30 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 5
     steps:
-    - uses: actions/checkout@v2
-      # source: https://github.com/golangci/golangci-lint-action
+    - uses: actions/checkout@v3
+    - name: Set up Go 1.18
+      uses: actions/setup-go@v3
+      with:
+        go-version: 1.18
     - name: golangci-lint
-      uses: golangci/golangci-lint-action@v2
+      uses: golangci/golangci-lint-action@v3
       with:
         # version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-        version: v1.42.1
+        version: v1.45.2
         working-directory: constraint
 
   test:
     name: "Unit test"
     runs-on: ubuntu-latest
     timeout-minutes: 5
     steps:
-      - name: Set up Go 1.17
-        uses: actions/setup-go@v2
+      - name: Set up Go 1.18
+        uses: actions/setup-go@v3
         with:
-          go-version: 1.17
+          go-version: 1.18
 
       - name: Check out code into the Go module directory
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
         with:
           path: go/src/github.com/open-policy-agent/frameworks
 
@@ -49,7 +52,7 @@ jobs:
           GOBIN: ${{ github.workspace }}/go/bin
 
       - name: Codecov Upload
-        uses: codecov/codecov-action@v1
+        uses: codecov/codecov-action@v3
         with:
           flags: unittests
           file: go/src/github.com/open-policy-agent/frameworks/constraint/cover.out

constraint/Makefile

@@ -1,3 +1,10 @@
+# When updating this, make sure to update the corresponding action in
+# workflow.yaml
+GOLANGCI_LINT_VERSION := v1.45.2
+
+# Detects the location of the user golangci-lint cache.
+GOLANGCI_LINT_CACHE := $(shell pwd)/.tmp/golangci-lint
+
 all: test
 
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
@@ -47,8 +54,14 @@ manifests:
 	sed -i '/- externaldata.gatekeeper.sh_providers.yaml/d' .staging/templatecrd/kustomization.yaml
 	kustomize build .staging/templatecrd --output=.staging/templatecrd/crd.yaml
 
+# lint runs a dockerized golangci-lint, and should give consistent results
+# across systems.
+# Source: https://golangci-lint.run/usage/install/#docker
 lint:
-	golangci-lint -v run ./... --timeout 5m
+	docker run --rm -v $(shell pwd):/app \
+		-v ${GOLANGCI_LINT_CACHE}:/root/.cache/golangci-lint \
+		-w /app golangci/golangci-lint:${GOLANGCI_LINT_VERSION}-alpine \
+		golangci-lint run -v
 
 # Generate code
 # Not working?  Try running `make gen-dependencies`

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")

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()
@@ -281,28 +281,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
 	}

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

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
+}
 `
 )