Skip to content

Commit

Permalink
[gk-test] parse Templates + Constraints in Suites (#1419)
Browse files Browse the repository at this point in the history
* parse Templates + Constraints in Suites

Make "gator test" parse ConstraintTemplates and Constraints specified by
Suites. The focus is on:
1) Creating the OPA client
2) Compiling Templates
3) Instantiating Constraints

When errors occur here we don't return an error - we return an
ErrorResult which will eventually be printed in a way similar to test
failures and not as a runtime error executing tests.

There's several error paths we can't easily test - I've marked those
with comments.

This PR does not actually run test Suites - that functionality would
easily double the length of this already-large PR.

Note that we do not simply exist immediately if a Suite contains no
Tests - we support Suites which define no tests, in which case we just
compile the Template and Constraint, returning a failure if either fails
to be added to the OPA client.

We should be cautious when adding tests to suite_test.go. Each takes
about 100ms due to the significant costs of creating an OPA client,
compiling a Template, and then instantiating a Constraint. There is no
good way around this as part of the behavior we need to test is the
ability to trigger and catch errors from a real OPA client.

Signed-off-by: Will Beason <willbeason@google.com>

* Upgrade frameworks/constraint dep

This lets us use version-independent logic.

Fix reviewer comments.

Signed-off-by: Will Beason <willbeason@google.com>

* Use future-compatible result types

As stated elsewhere, the purpose of this PR is NOT the result types.
They are not final and merely exist as stubs. They will be given a
complete treatment in a dedicated PR.

Similarly, this PR does not handle printing output. There will be a PR
for this, and the as-is state of not printing output is not final.

Signed-off-by: Will Beason <willbeason@google.com>
  • Loading branch information
Will Beason committed Jul 13, 2021
1 parent a88df07 commit 86b495b
Show file tree
Hide file tree
Showing 26 changed files with 1,980 additions and 107 deletions.
28 changes: 18 additions & 10 deletions cmd/gator/test/test.go
@@ -1,6 +1,7 @@
package test

import (
"context"
"errors"
"fmt"
"io/fs"
Expand Down Expand Up @@ -46,7 +47,7 @@ var Cmd = &cobra.Command{
RunE: runE,
}

func runE(_ *cobra.Command, args []string) error {
func runE(cmd *cobra.Command, args []string) error {
path := args[0]

// Convert path to be absolute. Allowing for relative and absolute paths
Expand Down Expand Up @@ -81,22 +82,29 @@ func runE(_ *cobra.Command, args []string) error {
return fmt.Errorf("compiling filter: %w", err)
}

return runSuites(fileSystem, suites, filter)
return runSuites(cmd.Context(), fileSystem, suites, filter)
}

func runSuites(fileSystem fs.FS, suites []gktest.Suite, filter gktest.Filter) error {
func runSuites(ctx context.Context, fileSystem fs.FS, suites []gktest.Suite, filter gktest.Filter) error {
isFailure := false
for _, s := range suites {
if !filter.MatchesSuite(s) {
continue
for i := range suites {
s := &suites[i]

c, err := gktest.NewOPAClient()
if err != nil {
return err
}

results := s.Run(fileSystem, filter)
for _, result := range results {
if result.IsFailure() {
suiteResult := s.Run(ctx, c, fileSystem, filter)
for _, testResult := range suiteResult.TestResults {
if testResult.Error != nil {
isFailure = true
}
fmt.Println(result.String())
for _, caseResult := range testResult.CaseResults {
if caseResult.Error != nil {
isFailure = true
}
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions go.mod
Expand Up @@ -10,7 +10,7 @@ require (
github.com/go-logr/logr v0.4.0
github.com/go-logr/zapr v0.2.0
github.com/go-openapi/spec v0.20.3 // indirect
github.com/google/go-cmp v0.5.4
github.com/google/go-cmp v0.5.6
github.com/google/uuid v1.1.2
github.com/mitchellh/mapstructure v1.4.1 // indirect
github.com/onsi/ginkgo v1.15.0
Expand All @@ -27,7 +27,7 @@ require (
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba
gopkg.in/yaml.v2 v2.4.0
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b
k8s.io/api v0.20.2
k8s.io/apiextensions-apiserver v0.20.2
k8s.io/apimachinery v0.20.2
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Expand Up @@ -310,6 +310,8 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M=
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g=
github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
Expand Down
29 changes: 29 additions & 0 deletions pkg/gktest/client.go
@@ -0,0 +1,29 @@
package gktest

import (
"context"

"github.com/open-policy-agent/frameworks/constraint/pkg/client"
"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
"github.com/open-policy-agent/frameworks/constraint/pkg/types"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

type Client interface {
// AddTemplate adds a Template to the Client. Templates define the structure
// and parameters of potential Constraints.
AddTemplate(ctx context.Context, templ *templates.ConstraintTemplate) (*types.Responses, error)

// AddConstraint adds a Constraint to the Client. Must map to one of the
// previously-added Templates.
//
// Returns an error if the referenced Template does not exist, or the
// Constraint does not match the structure defined by the referenced Template.
AddConstraint(ctx context.Context, constraint *unstructured.Unstructured) (*types.Responses, error)

// AddData adds the state of the cluster. For use in referential Constraints.
AddData(ctx context.Context, data interface{}) (*types.Responses, error)

// Review runs all Constraints against obj.
Review(ctx context.Context, obj interface{}, opts ...client.QueryOpt) (*types.Responses, error)
}
55 changes: 28 additions & 27 deletions pkg/gktest/filter.go
@@ -1,58 +1,59 @@
package gktest

// Filter filters suites and tests to run.
// Filter filters tests and cases to run.
type Filter struct{}

// NewFilter parses run into a Filter for selecting suites and individual tests
// to run.
// NewFilter parses run into a Filter for selecting constraint tests and
// individual cases to run.
//
// Empty string results in a Filter which matches all tests.
// Empty string results in a Filter which matches all tests and their cases.
//
// Examples:
// 1) NewFiler("require-foo-label//missing-label")
// Matches tests in suites containing the string "require-foo-label" and tests
// containing the string "missing-label". So this would match all of the
// Matches tests containing the string "require-foo-label"
// and cases containing the string "missing-label". So this would match all of the
// following:
// - Suite: "require-foo-label", Test: "missing-label"
// - Suite: "not-require-foo-label, Test: "not-missing-label"
// - Suite: "require-foo-label-and-bar-annotation", Test: "missing-label-and-annotation"
// - Test: "require-foo-label", Case: "missing-label"
// - Test: "not-require-foo-label, Case: "not-missing-label"
// - Test: "require-foo-label-and-bar-annotation", Case: "missing-label-and-annotation"
//
// 2) NewFilter("missing-label")
// Matches tests which either have a name containing "missing-label" or which
// are in a suite containing "missing-label". Matches the following:
// - Suite: "forbid-missing-label", Test: "with-foo-label"
// - Suite: "required-labels", Test: "missing-label"
// are in a constraint test containing "missing-label". Matches the following:
// - Test: "forbid-missing-label", Case: "with-foo-label"
// - Test: "required-labels", Case: "missing-label"
//
// 3) NewFilter("^require-foo-label$//")
// Matches tests in suites which exactly match "require-foo-label". Matches the
// Matches tests in constraints which exactly match "require-foo-label". Matches the
// following:
// - Suite: "require-foo-label", Test: "with-foo-label"
// - Suite: "require-foo-label", Test: "no-labels"
// - Test: "require-foo-label", Case: "with-foo-label"
// - Test: "require-foo-label", Case: "no-labels"
//
// 4) NewFilter("//empty-object")
// Matches tests whose names contain the string "empty-object". Matches the
// following:
// - Suite: "forbid-foo-label", Test: "empty-object"
// - Suite: "forbid-foo-label", Test: "another-empty-object"
// - Suite: "require-bar-annotation", Test: "empty-object"
// - Test: "forbid-foo-label", Case: "empty-object"
// - Test: "forbid-foo-label", Case: "another-empty-object"
// - Test: "require-bar-annotation", Case: "empty-object"
func NewFilter(run string) (Filter, error) {
return Filter{}, nil
}

// MatchesSuite filters the set of test suites to run by suite name and the tests
// contained in the suite. Returns true if the suite should be run.
// MatchesTest filters the set of constraint tests to run by constraint name
// and the tests contained in the constraint. Returns true if tests in the constraint
// should be run.
//
// If a suite regex was specified, returns true if the suite regex matches
// `suite`.
// If a suite regex was not specified but a test regex was, returns true if at
// least one test in `tests` matches the test regex.
func (f Filter) MatchesSuite(suite Suite) bool {
// If a constraint regex was specified, returns true if the constraint regex
// matches `constraint`.
// If a constraint regex was not specified but a test regex was, returns true if
// at least one test in `tests` matches the test regex.
func (f Filter) MatchesTest(c Test) bool {
return true
}

// MatchesTest filters the set of tests to run by name.
// MatchesCase filters the set of tests to run by name.
//
// Returns true if the test regex matches test.
func (f Filter) MatchesTest(testCase TestCase) bool {
func (f Filter) MatchesCase(t Case) bool {
return true
}
21 changes: 21 additions & 0 deletions pkg/gktest/opa.go
@@ -0,0 +1,21 @@
package gktest

import (
opaclient "github.com/open-policy-agent/frameworks/constraint/pkg/client"
"github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/local"
"github.com/open-policy-agent/gatekeeper/pkg/target"
)

func NewOPAClient() (*opaclient.Client, error) {
driver := local.New(local.Tracing(false))
backend, err := opaclient.NewBackend(opaclient.Driver(driver))
if err != nil {
return nil, err
}

c, err := backend.NewClient(opaclient.Targets(&target.K8sValidationTarget{}))
if err != nil {
return nil, err
}
return c, err
}
47 changes: 39 additions & 8 deletions pkg/gktest/result.go
@@ -1,14 +1,45 @@
package gktest

// Result is the structured form of the results of running a test.
type Result struct{}
// SuiteResult is the Result of running a Suite of tests.
type SuiteResult struct {
// Path is the absolute path to the file which defines Suite.
Path string

func (r Result) String() string {
return ""
// Error is the error which stopped the Suite from executing.
// If defined, TestResults is empty.
Error error

// TestResults are the results of running the tests for each defined
// Template/Constraint pair.
TestResults []TestResult
}

// TestResult is the results of:
// 1) Compiling the ConstraintTemplate,
// 2) Instantiating the Constraint, and
// 3) Running all Tests defined for the Constraint.
type TestResult struct {
// Name is the name given to the Template/Constraint pair under test.
Name string

// Error is the error which prevented running tests for this Constraint.
// If defined, CaseResults is empty.
Error error

// CaseResults are individual results for all tests defined for this Constraint.
CaseResults []CaseResult
}

// IsFailure returns true if either a test failed or there was a problem
// executing tests.
func (r Result) IsFailure() bool {
return false
// CaseResult is the result of evaluating a Constraint against a kubernetes
// object, and comparing the result with the expected result.
type CaseResult struct {
// Name is the name given to this test for the Constraint under test.
Name string

// Error is the either:
// 1) why this test failed, or
// 2) the error which prevented running this test.
// We don't need to distinguish between 1 and 2 - they are both treated as
// test failures.
Error error
}

0 comments on commit 86b495b

Please sign in to comment.