Skip to content

Commit

Permalink
Remove unnecessary contexts
Browse files Browse the repository at this point in the history 
There were a lot of Contexts we asked for in interfaces, but didn't
actually use. Since this pollutes call sites and gives the false
impression that these calls are cancellable/etc, we shouldn't have them.

This commit removes these unnecessary contexts.

Signed-off-by: Will Beason <willbeason@google.com>
  • Loading branch information
Will Beason committed Dec 2, 2021
1 parent fef0b14 commit 44b36b7
Show file tree
Hide file tree
Showing 11 changed files with 124 additions and 152 deletions.
7 changes: 3 additions & 4 deletions constraint/pkg/client/backend.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package client

import (
"context"
"errors"
"fmt"

Expand Down Expand Up @@ -44,7 +43,7 @@ func NewBackend(opts ...BackendOpt) (*Backend, error) {
}

// NewClient creates a new client for the supplied backend.
func (b *Backend) NewClient(ctx context.Context, opts ...Opt) (*Client, error) {
func (b *Backend) NewClient(opts ...Opt) (*Client, error) {
if b.hasClient {
return nil, errors.New("currently only one client per backend is supported")
}
Expand Down Expand Up @@ -81,11 +80,11 @@ func (b *Backend) NewClient(ctx context.Context, opts ...Opt) (*Client, error) {
return nil, errors.New("no targets registered: please register a target via client.Targets()")
}

if err := b.driver.Init(ctx); err != nil {
if err := b.driver.Init(); err != nil {
return nil, err
}

if err := c.init(ctx); err != nil {
if err := c.init(); err != nil {
return nil, err
}
return c, nil
Expand Down
36 changes: 18 additions & 18 deletions constraint/pkg/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func (c *Client) createTemplateArtifacts(templ *templates.ConstraintTemplate) (*
}

// CreateCRD creates a CRD from template.
func (c *Client) CreateCRD(ctx context.Context, templ *templates.ConstraintTemplate) (*apiextensions.CustomResourceDefinition, error) {
func (c *Client) CreateCRD(templ *templates.ConstraintTemplate) (*apiextensions.CustomResourceDefinition, error) {
artifacts, err := c.createTemplateArtifacts(templ)
if err != nil {
return nil, err
Expand All @@ -361,7 +361,7 @@ func (c *Client) CreateCRD(ctx context.Context, templ *templates.ConstraintTempl
// AddTemplate adds the template source code to OPA and registers the CRD with the client for
// schema validation on calls to AddConstraint. On error, the responses return value
// will still be populated so that partial results can be analyzed.
func (c *Client) AddTemplate(ctx context.Context, templ *templates.ConstraintTemplate) (*types.Responses, error) {
func (c *Client) AddTemplate(templ *templates.ConstraintTemplate) (*types.Responses, error) {
resp := types.NewResponses()

basicArtifacts, err := c.createBasicTemplateArtifacts(templ)
Expand All @@ -370,7 +370,7 @@ func (c *Client) AddTemplate(ctx context.Context, templ *templates.ConstraintTem
}

// return immediately if no change
if cached, err := c.GetTemplate(ctx, templ); err == nil && cached.SemanticEqual(templ) {
if cached, err := c.GetTemplate(templ); err == nil && cached.SemanticEqual(templ) {
resp.Handled[basicArtifacts.targetHandler.GetName()] = true
return resp, nil
}
Expand All @@ -383,7 +383,7 @@ func (c *Client) AddTemplate(ctx context.Context, templ *templates.ConstraintTem
c.constraintsMux.Lock()
defer c.constraintsMux.Unlock()

if err := c.backend.driver.PutModules(ctx, artifacts.namePrefix, artifacts.modules); err != nil {
if err := c.backend.driver.PutModules(artifacts.namePrefix, artifacts.modules); err != nil {
return resp, err
}

Expand Down Expand Up @@ -416,7 +416,7 @@ func (c *Client) RemoveTemplate(ctx context.Context, templ *templates.Constraint
c.constraintsMux.Lock()
defer c.constraintsMux.Unlock()

template, err := c.getTemplateNoLock(ctx, rawArtifacts)
template, err := c.getTemplateNoLock(rawArtifacts)
if err != nil {
if IsMissingTemplateError(err) {
return resp, nil
Expand All @@ -429,7 +429,7 @@ func (c *Client) RemoveTemplate(ctx context.Context, templ *templates.Constraint
return resp, err
}

if _, err := c.backend.driver.DeleteModules(ctx, artifacts.namePrefix); err != nil {
if _, err := c.backend.driver.DeleteModules(artifacts.namePrefix); err != nil {
return resp, err
}

Expand All @@ -450,18 +450,18 @@ func (c *Client) RemoveTemplate(ctx context.Context, templ *templates.Constraint
}

// GetTemplate gets the currently recognized template.
func (c *Client) GetTemplate(ctx context.Context, templ *templates.ConstraintTemplate) (*templates.ConstraintTemplate, error) {
func (c *Client) GetTemplate(templ *templates.ConstraintTemplate) (*templates.ConstraintTemplate, error) {
artifacts, err := c.createRawTemplateArtifacts(templ)
if err != nil {
return nil, err
}

c.constraintsMux.Lock()
defer c.constraintsMux.Unlock()
return c.getTemplateNoLock(ctx, artifacts)
return c.getTemplateNoLock(artifacts)
}

func (c *Client) getTemplateNoLock(ctx context.Context, artifacts keyableArtifact) (*templates.ConstraintTemplate, error) {
func (c *Client) getTemplateNoLock(artifacts keyableArtifact) (*templates.ConstraintTemplate, error) {
t, ok := c.templates[artifacts.Key()]
if !ok {
return nil, NewMissingTemplateError(string(artifacts.Key()))
Expand Down Expand Up @@ -548,7 +548,7 @@ func (c *Client) AddConstraint(ctx context.Context, constraint *unstructured.Uns
return resp, err
}
// return immediately if no change
if cached, err := c.getConstraintNoLock(ctx, constraint); err == nil && constraintlib.SemanticEqual(cached, constraint) {
if cached, err := c.getConstraintNoLock(constraint); err == nil && constraintlib.SemanticEqual(cached, constraint) {
for _, target := range entry.Targets {
resp.Handled[target] = true
}
Expand Down Expand Up @@ -620,7 +620,7 @@ func (c *Client) removeConstraintNoLock(ctx context.Context, constraint *unstruc
}

// getConstraintNoLock gets the currently recognized constraint without the lock.
func (c *Client) getConstraintNoLock(ctx context.Context, constraint *unstructured.Unstructured) (*unstructured.Unstructured, error) {
func (c *Client) getConstraintNoLock(constraint *unstructured.Unstructured) (*unstructured.Unstructured, error) {
subPath, err := createConstraintSubPath(constraint)
if err != nil {
return nil, err
Expand All @@ -634,10 +634,10 @@ func (c *Client) getConstraintNoLock(ctx context.Context, constraint *unstructur
}

// GetConstraint gets the currently recognized constraint.
func (c *Client) GetConstraint(ctx context.Context, constraint *unstructured.Unstructured) (*unstructured.Unstructured, error) {
func (c *Client) GetConstraint(constraint *unstructured.Unstructured) (*unstructured.Unstructured, error) {
c.constraintsMux.Lock()
defer c.constraintsMux.Unlock()
return c.getConstraintNoLock(ctx, constraint)
return c.getConstraintNoLock(constraint)
}

// validateConstraint is an internal function that allows us to toggle whether we use a read lock
Expand All @@ -661,12 +661,12 @@ func (c *Client) validateConstraint(constraint *unstructured.Unstructured, lock

// ValidateConstraint returns an error if the constraint is not recognized or does not conform to
// the registered CRD for that constraint.
func (c *Client) ValidateConstraint(ctx context.Context, constraint *unstructured.Unstructured) error {
func (c *Client) ValidateConstraint(constraint *unstructured.Unstructured) error {
return c.validateConstraint(constraint, true)
}

// init initializes the OPA backend for the client.
func (c *Client) init(ctx context.Context) error {
func (c *Client) init() error {
for _, t := range c.targets {
hooks := fmt.Sprintf(`hooks["%s"]`, t.GetName())
templMap := map[string]string{"Target": t.GetName()}
Expand All @@ -677,7 +677,7 @@ func (c *Client) init(ctx context.Context) error {
}

moduleName := fmt.Sprintf("%s.hooks_builtin", hooks)
err := c.backend.driver.PutModule(ctx, moduleName, libBuiltin.String())
err := c.backend.driver.PutModule(moduleName, libBuiltin.String())
if err != nil {
return err
}
Expand Down Expand Up @@ -715,7 +715,7 @@ func (c *Client) init(ctx context.Context) error {
if err != nil {
return fmt.Errorf("could not re-format Rego source: %v", err)
}
if err := c.backend.driver.PutModule(ctx, modulePath, string(src)); err != nil {
if err := c.backend.driver.PutModule(modulePath, string(src)); err != nil {
return fmt.Errorf("error %s from compiled source:\n%s", err, src)
}
}
Expand All @@ -737,7 +737,7 @@ func (c *Client) Reset(ctx context.Context) error {
}
for name, v := range c.templates {
for _, t := range v.Targets {
if _, err := c.backend.driver.DeleteModule(ctx, fmt.Sprintf(`templates["%s"]["%s"]`, t, name)); err != nil {
if _, err := c.backend.driver.DeleteModule(fmt.Sprintf(`templates["%s"]["%s"]`, t, name)); err != nil {
return err
}
}
Expand Down
6 changes: 2 additions & 4 deletions constraint/pkg/client/client_addtemplate_bench_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package client

import (
"context"
"fmt"
"testing"

Expand Down Expand Up @@ -82,7 +81,6 @@ func BenchmarkClient_AddTemplate(b *testing.B) {

for i := 0; i < b.N; i++ {
b.StopTimer()
ctx := context.Background()
targets := Targets(&handler{})

d := local.New()
Expand All @@ -92,15 +90,15 @@ func BenchmarkClient_AddTemplate(b *testing.B) {
b.Fatal(err)
}

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

b.StartTimer()

for _, ct := range cts {
_, _ = c.AddTemplate(ctx, ct)
_, _ = c.AddTemplate(ct)
}
}
})
Expand Down
Loading

0 comments on commit 44b36b7

Please sign in to comment.