Skip to content

Commit

Permalink
Refactor out backend code (#198)
Browse files Browse the repository at this point in the history 
* Refactor out backend code

Having the backend intermediate type for instantiating and using Client
adds complexity, code length, and unnecessary error paths.

This commit fully removes backend without breaking any existing
funcitonality.

Signed-off-by: Will Beason <[email protected]>

* Fix lint errors

Signed-off-by: Will Beason <[email protected]>
  • Loading branch information
Will Beason authored Feb 10, 2022
1 parent cd1085b commit 2c8fe2d
Show file tree
Hide file tree
Showing 10 changed files with 151 additions and 321 deletions.
88 changes: 0 additions & 88 deletions constraint/pkg/client/backend.go
View file

This file was deleted.

98 changes: 0 additions & 98 deletions constraint/pkg/client/backend_test.go
View file

This file was deleted.

34 changes: 17 additions & 17 deletions constraint/pkg/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type templateEntry struct {
}

type Client struct {
backend *Backend
driver drivers.Driver
targets map[string]handler.TargetHandler

// mtx guards access to both templates and constraints.
Expand Down Expand Up @@ -92,7 +92,7 @@ func (c *Client) AddData(ctx context.Context, data interface{}) (*types.Response
// paths passed to driver must be specific to the target to prevent key
// collisions.
driverPath := createDataPath(target, relPath)
err = c.backend.driver.PutData(ctx, driverPath, processedData)
err = c.driver.PutData(ctx, driverPath, processedData)
if err != nil {
errMap[target] = err

Expand Down Expand Up @@ -127,7 +127,7 @@ func (c *Client) RemoveData(ctx context.Context, data interface{}) (*types.Respo
continue
}

if _, err := c.backend.driver.DeleteData(ctx, createDataPath(target, relPath)); err != nil {
if _, err := c.driver.DeleteData(ctx, createDataPath(target, relPath)); err != nil {
errMap[target] = err
continue
}
Expand Down Expand Up @@ -307,11 +307,11 @@ func (c *Client) ValidateConstraintTemplate(templ *templates.ConstraintTemplate)
if _, _, err := c.ValidateConstraintTemplateBasic(templ); err != nil {
return err
}
if dr, ok := c.backend.driver.(*local.Driver); ok {
if dr, ok := c.driver.(*local.Driver); ok {
_, _, err := dr.ValidateConstraintTemplate(templ)
return err
}
return fmt.Errorf("driver %T is not supported", c.backend.driver)
return fmt.Errorf("driver %T is not supported", c.driver)
}

// AddTemplate adds the template source code to OPA and registers the CRD with the client for
Expand All @@ -334,7 +334,7 @@ func (c *Client) AddTemplate(templ *templates.ConstraintTemplate) (*types.Respon
c.mtx.Lock()
defer c.mtx.Unlock()

if err = c.backend.driver.AddTemplate(templ); err != nil {
if err = c.driver.AddTemplate(templ); err != nil {
return resp, err
}
cpy := templ.DeepCopy()
Expand Down Expand Up @@ -380,7 +380,7 @@ func (c *Client) RemoveTemplate(ctx context.Context, templ *templates.Constraint
return resp, err
}

if err := c.backend.driver.RemoveTemplate(templ); err != nil {
if err := c.driver.RemoveTemplate(templ); err != nil {
return resp, err
}

Expand All @@ -392,7 +392,7 @@ func (c *Client) RemoveTemplate(ctx context.Context, templ *templates.Constraint
delete(c.constraints, artifacts.gk)
// Also clean up root path to avoid memory leaks
constraintRoot := createConstraintGKPath(artifacts.targetHandler.GetName(), artifacts.gk)
if _, err := c.backend.driver.DeleteData(ctx, constraintRoot); err != nil {
if _, err := c.driver.DeleteData(ctx, constraintRoot); err != nil {
return resp, err
}
delete(c.templates, artifacts.Key())
Expand Down Expand Up @@ -523,7 +523,7 @@ func (c *Client) AddConstraint(ctx context.Context, constraint *unstructured.Uns
if err := c.validateConstraint(constraint, false); err != nil {
return resp, err
}
if err := c.backend.driver.AddConstraint(ctx, constraint); err != nil {
if err := c.driver.AddConstraint(ctx, constraint); err != nil {
return resp, err
}
for _, target := range entry.Targets {
Expand Down Expand Up @@ -552,7 +552,7 @@ func (c *Client) removeConstraintNoLock(ctx context.Context, constraint *unstruc
if err != nil {
return resp, err
}
if err := c.backend.driver.RemoveConstraint(ctx, constraint); err != nil {
if err := c.driver.RemoveConstraint(ctx, constraint); err != nil {
return resp, err
}
for _, target := range entry.Targets {
Expand Down Expand Up @@ -623,7 +623,7 @@ func (c *Client) init() error {
}

builtinPath := fmt.Sprintf("%s.hooks_builtin", hooks)
err := c.backend.driver.PutModule(builtinPath, libBuiltin.String())
err := c.driver.PutModule(builtinPath, libBuiltin.String())
if err != nil {
return err
}
Expand Down Expand Up @@ -672,20 +672,20 @@ func (c *Client) init() error {
ErrCreatingClient, err)
}

err = c.backend.driver.PutModule(modulePath, string(src))
err = c.driver.PutModule(modulePath, string(src))
if err != nil {
return fmt.Errorf("%w: error %s from compiled source:\n%s",
ErrCreatingClient, err, src)
}
}
if d, ok := c.backend.driver.(*local.Driver); ok {
if d, ok := c.driver.(*local.Driver); ok {
var externs []string
for _, field := range c.AllowedDataFields {
externs = append(externs, fmt.Sprintf("data.%s", field))
}
d.SetExterns(externs)
} else {
return fmt.Errorf("%w: driver %T is not supported", ErrCreatingClient, c.backend.driver)
return fmt.Errorf("%w: driver %T is not supported", ErrCreatingClient, c.driver)
}
return nil
}
Expand All @@ -712,7 +712,7 @@ TargetLoop:
continue
}
input := map[string]interface{}{"review": review}
resp, err := c.backend.driver.Query(ctx, fmt.Sprintf(`hooks["%s"].violation`, name), input, drivers.Tracing(cfg.enableTracing))
resp, err := c.driver.Query(ctx, fmt.Sprintf(`hooks["%s"].violation`, name), input, drivers.Tracing(cfg.enableTracing))
if err != nil {
errMap[name] = err
continue
Expand Down Expand Up @@ -745,7 +745,7 @@ func (c *Client) Audit(ctx context.Context, opts ...QueryOpt) (*types.Responses,
TargetLoop:
for name, target := range c.targets {
// Short-circuiting question applies here as well
resp, err := c.backend.driver.Query(ctx, fmt.Sprintf(`hooks["%s"].audit`, name), nil, drivers.Tracing(cfg.enableTracing))
resp, err := c.driver.Query(ctx, fmt.Sprintf(`hooks["%s"].audit`, name), nil, drivers.Tracing(cfg.enableTracing))
if err != nil {
errMap[name] = err
continue
Expand All @@ -767,7 +767,7 @@ TargetLoop:

// Dump dumps the state of OPA to aid in debugging.
func (c *Client) Dump(ctx context.Context) (string, error) {
return c.backend.driver.Dump(ctx)
return c.driver.Dump(ctx)
}

// knownTargets returns a sorted list of currently-known target names.
Expand Down
19 changes: 3 additions & 16 deletions constraint/pkg/client/client_addtemplate_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ import (
"fmt"
"testing"

"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/frameworks/constraint/pkg/client/clienttest"
"github.com/open-policy-agent/frameworks/constraint/pkg/core/templates"
"github.com/open-policy-agent/frameworks/constraint/pkg/handler/handlertest"
)

var modules = []struct {
Expand Down Expand Up @@ -80,24 +78,13 @@ func BenchmarkClient_AddTemplate(b *testing.B) {

for i := 0; i < b.N; i++ {
b.StopTimer()
targets := client.Targets(&handlertest.Handler{})

d := local.New()

backend, err := client.NewBackend(client.Driver(d))
if err != nil {
b.Fatal(err)
}

c, err := backend.NewClient(targets)
if err != nil {
b.Fatal(err)
}
c := clienttest.New(b)

b.StartTimer()

for _, ct := range cts {
_, err = c.AddTemplate(ct)
_, err := c.AddTemplate(ct)
if err != nil {
b.Fatal(err)
}
Expand Down
Loading

0 comments on commit 2c8fe2d

Please sign in to comment.