Fix typos and minor mistakes (#196)

I ran inspect code to find typos and various unintentional bits of code.
All of these changes are minor and don't result in any behavioral
changes. Generally they show up as I'm editing code, and this makes them
go away.

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

constraint/.golangci.yaml

@@ -35,6 +35,7 @@ linters:
   - govet
   - ineffassign
   - misspell
+  - nilerr
   - revive # replacement for golint
   - staticcheck
   - structcheck

constraint/cmd/rewrite-compatibility/main.go

@@ -34,7 +34,9 @@ opa test -v rewrite/lib/ rewrite/validator/
 meld policy-library/lib/ rewrite/lib/
 meld policy-library/validator/ rewrite/validator/
 `,
-	RunE: rootCmdFn,
+	RunE: func(_ *cobra.Command, _ []string) error {
+		return rootCmdFn()
+	},
 }
 
 var (
@@ -115,7 +117,7 @@ func compileSrcs(
 	return nil
 }
 
-func rootCmdFn(cmd *cobra.Command, args []string) error {
+func rootCmdFn() error {
 	return compileSrcs(cts, libs, pkgPrefix, oldRoot, newRoot)
 }
 

constraint/pkg/apis/externaldata/v1alpha1/v1alpha1_suite_test.go

@@ -16,43 +16,31 @@ limitations under the License.
 package v1alpha1
 
 import (
-	"log"
-	"os"
 	"path/filepath"
 	"testing"
 
 	"k8s.io/client-go/kubernetes/scheme"
-	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 )
 
-var (
-	cfg *rest.Config
-	c   client.Client
-)
-
-func TestMain(m *testing.M) {
-	t := &envtest.Environment{
+func TestNewClient(t *testing.T) {
+	te := &envtest.Environment{
 		CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "..", "deploy", "crds.yaml")},
 	}
 
 	err := SchemeBuilder.AddToScheme(scheme.Scheme)
 	if err != nil {
-		log.Fatal(err)
-	}
-
-	if cfg, err = t.Start(); err != nil {
-		log.Fatal(err)
+		t.Fatal(err)
 	}
 
-	if c, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}); err != nil {
-		log.Fatal(err)
+	cfg, err := te.Start()
+	if err != nil {
+		t.Fatal(err)
 	}
 
-	code := m.Run()
-	if err := t.Stop(); err != nil {
-		log.Fatal(err)
+	_, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
+	if err != nil {
+		t.Fatal(err)
 	}
-	os.Exit(code)
 }

constraint/pkg/client/client.go

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

constraint/pkg/client/client_test.go

@@ -166,7 +166,7 @@ func TestClient_AddData(t *testing.T) {
 			}
 
 			if r == nil {
-				t.Fatal("got AddTemplate() == nil, want non-nil")
+				t.Fatal("got AddData() == nil, want non-nil")
 			}
 
 			if diff := cmp.Diff(tc.wantHandled, r.Handled, cmpopts.EquateEmpty()); diff != "" {
@@ -461,7 +461,7 @@ func TestClient_RemoveTemplate(t *testing.T) {
 
 			r, err := c.RemoveTemplate(context.Background(), tc.template)
 			if err != nil {
-				t.Errorf("err = %v; want nil", err)
+				t.Fatalf("err = %v; want nil", err)
 			}
 
 			if diff := cmp.Diff(tc.wantHandled, r.Handled, cmpopts.EquateEmpty()); diff != "" {
@@ -482,7 +482,7 @@ func TestClient_RemoveTemplate_ByNameOnly(t *testing.T) {
 		{
 			name:        "Good Template",
 			handler:     &handlertest.Handler{},
-			template:    cts.New(),
+			template:    cts.New(cts.OptTargets(cts.Target(handlertest.HandlerName, cts.ModuleDeny))),
 			wantHandled: map[string]bool{handlertest.HandlerName: true},
 			wantError:   nil,
 		},
@@ -527,7 +527,7 @@ func TestClient_RemoveTemplate_ByNameOnly(t *testing.T) {
 
 			r, err := c.RemoveTemplate(context.Background(), sparseTemplate)
 			if err != nil {
-				t.Errorf("err = %v; want nil", err)
+				t.Fatalf("err = %v; want nil", err)
 			}
 
 			if diff := cmp.Diff(tc.wantHandled, r.Handled, cmpopts.EquateEmpty()); diff != "" {
@@ -677,15 +677,15 @@ func TestClient_GetTemplate_ByNameOnly(t *testing.T) {
 }
 
 func TestClient_RemoveTemplate_CascadingDelete(t *testing.T) {
-	handler := &handlertest.Handler{}
+	h := &handlertest.Handler{}
 
 	d := local.New()
 	b, err := client.NewBackend(client.Driver(d))
 	if err != nil {
 		t.Fatalf("Could not create backend: %s", err)
 	}
 
-	c, err := b.NewClient(client.Targets(handler))
+	c, err := b.NewClient(client.Targets(h))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -966,8 +966,8 @@ func TestClient_RemoveConstraint(t *testing.T) {
 				t.Fatalf("Could not create backend: %s", err)
 			}
 
-			handler := &handlertest.Handler{}
-			c, err := b.NewClient(client.Targets(handler))
+			h := &handlertest.Handler{}
+			c, err := b.NewClient(client.Targets(h))
 			if err != nil {
 				t.Fatal(err)
 			}

constraint/pkg/client/drivers/interface.go

@@ -27,7 +27,7 @@ type Driver interface {
 	// AddTemplate adds the template source code to OPA
 	AddTemplate(ct *templates.ConstraintTemplate) error
 	// RemoveTemplate removes the template source code from OPA
-	RemoveTemplate(ctx context.Context, ct *templates.ConstraintTemplate) error
+	RemoveTemplate(ct *templates.ConstraintTemplate) error
 	// AddConstraint inserts validated constraint into OPA
 	AddConstraint(ctx context.Context, constraint *unstructured.Unstructured) error
 	// RemoveConstraint removes a constraint from OPA

constraint/pkg/client/drivers/local/local.go

@@ -585,11 +585,17 @@ func (d *Driver) AddTemplate(templ *templates.ConstraintTemplate) error {
 }
 
 // RemoveTemplate implements driver.Driver.
-// TODO: Cascading deletes.
-func (d *Driver) RemoveTemplate(ctx context.Context, templ *templates.ConstraintTemplate) error {
-	if err := validateTargets(templ); err != nil {
+func (d *Driver) RemoveTemplate(templ *templates.ConstraintTemplate) error {
+	if len(templ.Spec.Targets) != 1 {
+		// TODO: Properly handle multi-target deletes.
+		//  This is likely trivially covered by compiler sharding since we'll
+		//  control how data is stored per-Template.
+		//  https://github.com/open-policy-agent/frameworks/issues/197
+		// The template has an unexpected number of targets at removal time, so do
+		// nothing.
 		return nil
 	}
+
 	targetHandler := templ.Spec.Targets[0].Target
 	kind := templ.Spec.CRD.Spec.Names.Kind
 	namePrefix := createTemplatePath(targetHandler, kind)

constraint/pkg/client/drivers/local/local_unit_test.go

@@ -509,7 +509,8 @@ violation[{"msg": "msg"}] {
 			if len(dr.modules) == 0 {
 				t.Errorf("driver failed to add module")
 			}
-			gotErr = dr.RemoveTemplate(context.Background(), tmpl)
+
+			gotErr = dr.RemoveTemplate(tmpl)
 			if gotErr != nil {
 				t.Errorf("err = %v; want nil", gotErr)
 			}

constraint/pkg/client/drivers/remote/remote.go

@@ -99,7 +99,7 @@ func (d *driver) AddTemplate(ct *templates.ConstraintTemplate) error {
 }
 
 // RemoveTemplate implements driver.Driver.
-func (d *driver) RemoveTemplate(ctx context.Context, ct *templates.ConstraintTemplate) error {
+func (d *driver) RemoveTemplate(ct *templates.ConstraintTemplate) error {
 	panic("not implemented")
 }
 

constraint/pkg/externaldata/request.go

@@ -24,7 +24,7 @@ type Request struct {
 	Keys []string `json:"keys,omitempty"`
 }
 
-// NewRequest creates a new request for the external data provider.
+// NewProviderRequest creates a new request for the external data provider.
 func NewProviderRequest(keys []string) *ProviderRequest {
 	return &ProviderRequest{
 		APIVersion: "externaldata.gatekeeper.sh/v1alpha1",
@@ -35,6 +35,7 @@ func NewProviderRequest(keys []string) *ProviderRequest {
 	}
 }
 
+// ProviderKind strings are special string constants for Providers.
 // +kubebuilder:validation:Enum=ProviderRequestKind;ProviderResponseKind
 type ProviderKind string
 

constraint/pkg/externaldata/response.go

@@ -42,7 +42,7 @@ type Response struct {
 	SystemError string `json:"systemError,omitempty"`
 }
 
-// Items is the struct that contains the key, value or error from a provider response.
+// Item is the struct that contains the key, value or error from a provider response.
 type Item struct {
 	// Key is the request from the provider.
 	Key string `json:"key,omitempty"`

constraint/pkg/schema/structural.go

@@ -41,5 +41,5 @@ func CRDSchema(sch *runtime.Scheme, version string) (*schema.Structural, error)
 		return structural, nil
 	}
 
-	return nil, fmt.Errorf("No CRD version '%q'", version)
+	return nil, fmt.Errorf("no CRD version %q", version)
 }