Allow users to get/delete templates by name only

constraint/.golangci.yaml

@@ -6,4 +6,18 @@ linter-settings:
     locale: US
 
 linters:
-  disable: unused # https://github.com/golangci/golangci-lint/issues/337#issuecomment-536721714
+  disable-all: true
+  enable:
+    - errcheck
+    - govet
+    - ineffassign
+    - golint
+    - goconst
+    - goimports #- unused, https://github.com/golangci/golangci-lint/issues/337#issuecomment-536721714
+    - varcheck
+    - deadcode
+    - misspell
+    - typecheck
+    - structcheck
+    - staticcheck
+    - gosimple

constraint/Makefile

@@ -42,7 +42,7 @@ vet:
 	go vet ./pkg/...
 
 lint:
-	golangci-lint -v run ./... --timeout 5m
+	golangci-lint -v run ./... --timeout 5m --skip-files '(^|/)zz_generated'
 
 # Generate code
 generate: buildutils

constraint/pkg/client/backend.go

@@ -43,7 +43,7 @@ func NewBackend(opts ...BackendOpt) (*Backend, error) {
 }
 
 // NewClient creates a new client for the supplied backend
-func (b *Backend) NewClient(opts ...ClientOpt) (*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")
 	}
@@ -54,7 +54,7 @@ func (b *Backend) NewClient(opts ...ClientOpt) (*Client, error) {
 	c := &Client{
 		backend:           b,
 		constraints:       make(map[schema.GroupKind]map[string]*unstructured.Unstructured),
-		templates:         make(map[string]*templateEntry),
+		templates:         make(map[templateKey]*templateEntry),
 		allowedDataFields: fields,
 	}
 	var errs Errors

constraint/pkg/client/client.go

@@ -24,13 +24,13 @@ import (
 
 const constraintGroup = "constraints.gatekeeper.sh"
 
-type ClientOpt func(*Client) error
+type Opt func(*Client) error
 
 // Client options
 
 var targetNameRegex = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9.]*$`)
 
-func Targets(ts ...TargetHandler) ClientOpt {
+func Targets(ts ...TargetHandler) Opt {
 	return func(c *Client) error {
 		var errs Errors
 		handlers := make(map[string]TargetHandler, len(ts))
@@ -54,7 +54,7 @@ func Targets(ts ...TargetHandler) ClientOpt {
 // AllowedDataFields sets the fields under `data` that Rego in ConstraintTemplates
 // can access. If unset, all fields can be accessed. Only fields recognized by
 // the system can be enabled.
-func AllowedDataFields(fields ...string) ClientOpt {
+func AllowedDataFields(fields ...string) Opt {
 	return func(c *Client) error {
 		c.allowedDataFields = fields
 		return nil
@@ -71,7 +71,7 @@ type Client struct {
 	backend           *Backend
 	targets           map[string]TargetHandler
 	constraintsMux    sync.RWMutex
-	templates         map[string]*templateEntry
+	templates         map[templateKey]*templateEntry
 	constraints       map[schema.GroupKind]map[string]*unstructured.Unstructured
 	allowedDataFields []string
 }
@@ -164,18 +164,42 @@ func (c *Client) validateTargets(templ *templates.ConstraintTemplate) (*template
 	return targetSpec, targetHandler, nil
 }
 
+type templateKey string
+
 type keyableArtifact interface {
-	// crd is the CustomResourceDefinition created from the CT.
-	CRD() *apiextensions.CustomResourceDefinition
+	Key() templateKey
 }
 
 var _ keyableArtifact = &basicCTArtifacts{}
 
+func templateKeyFromConstraint(cst *unstructured.Unstructured) templateKey {
+	return templateKey(strings.ToLower(cst.GetKind()))
+}
+
+// rawCTArtifacts have no processing and are only useful for looking things up
+// from the cache
+type rawCTArtifacts struct {
+	// template is the template itself
+	template *templates.ConstraintTemplate
+}
+
+func (a *rawCTArtifacts) Key() templateKey {
+	return templateKey(a.template.GetName())
+}
+
+// createRawTemplateArtifacts creates the "free" artifacts for a template, avoiding more
+// complex tasks like rewriting Rego. Provides minimal validation.
+func (c *Client) createRawTemplateArtifacts(templ *templates.ConstraintTemplate) (*rawCTArtifacts, error) {
+	if templ.ObjectMeta.Name == "" {
+		return nil, errors.New("Template has no name")
+	}
+	return &rawCTArtifacts{template: templ}, nil
+}
+
 // basicCTArtifacts are the artifacts created by processing a constraint template
 // that require little compute effort
 type basicCTArtifacts struct {
-	// template is the template itself
-	template *templates.ConstraintTemplate
+	rawCTArtifacts
 
 	// namePrefix is the name prefix by which the modules will be identified during create / delete
 	// calls to the drivers.Driver interface.
@@ -208,10 +232,11 @@ type ctArtifacts struct {
 }
 
 // createBasicTemplateArtifacts creates the low-cost artifacts for a template, avoiding more
-// complex tasks like rewriting Rego. Useful for basic lookup tasks
+// complex tasks like rewriting Rego.
 func (c *Client) createBasicTemplateArtifacts(templ *templates.ConstraintTemplate) (*basicCTArtifacts, error) {
-	if templ.ObjectMeta.Name == "" {
-		return nil, errors.New("Template has no name")
+	rawArtifacts, err := c.createRawTemplateArtifacts(templ)
+	if err != nil {
+		return nil, err
 	}
 	if templ.ObjectMeta.Name != strings.ToLower(templ.Spec.CRD.Spec.Names.Kind) {
 		return nil, fmt.Errorf("Template's name %s is not equal to the lowercase of CRD's Kind: %s", templ.ObjectMeta.Name, strings.ToLower(templ.Spec.CRD.Spec.Names.Kind))
@@ -237,12 +262,12 @@ func (c *Client) createBasicTemplateArtifacts(templ *templates.ConstraintTemplat
 	entryPointPath := createTemplatePath(targetHandler.GetName(), templ.Spec.CRD.Spec.Names.Kind)
 
 	return &basicCTArtifacts{
-		template:      templ,
-		gk:            schema.GroupKind{Group: crd.Spec.Group, Kind: crd.Spec.Names.Kind},
-		crd:           crd,
-		targetHandler: targetHandler,
-		targetSpec:    targetSpec,
-		namePrefix:    entryPointPath,
+		rawCTArtifacts: *rawArtifacts,
+		gk:             schema.GroupKind{Group: crd.Spec.Group, Kind: crd.Spec.Names.Kind},
+		crd:            crd,
+		targetHandler:  targetHandler,
+		targetSpec:     targetSpec,
+		namePrefix:     entryPointPath,
 	}, nil
 }
 
@@ -358,7 +383,7 @@ func (c *Client) AddTemplate(ctx context.Context, templ *templates.ConstraintTem
 
 	cpy := templ.DeepCopy()
 	cpy.Status = templates.ConstraintTemplateStatus{}
-	c.templates[c.templatesMapKey(artifacts)] = &templateEntry{
+	c.templates[artifacts.Key()] = &templateEntry{
 		template: cpy,
 		CRD:      artifacts.crd,
 		Targets:  []string{artifacts.targetHandler.GetName()},
@@ -375,14 +400,27 @@ func (c *Client) AddTemplate(ctx context.Context, templ *templates.ConstraintTem
 func (c *Client) RemoveTemplate(ctx context.Context, templ *templates.ConstraintTemplate) (*types.Responses, error) {
 	resp := types.NewResponses()
 
-	artifacts, err := c.createBasicTemplateArtifacts(templ)
+	rawArtifacts, err := c.createRawTemplateArtifacts(templ)
 	if err != nil {
 		return resp, err
 	}
 
 	c.constraintsMux.Lock()
 	defer c.constraintsMux.Unlock()
 
+	template, err := c.getTemplateNoLock(ctx, rawArtifacts)
+	if err != nil {
+		if IsMissingTemplateError(err) {
+			return resp, nil
+		}
+		return resp, err
+	}
+
+	artifacts, err := c.createBasicTemplateArtifacts(template)
+	if err != nil {
+		return resp, err
+	}
+
 	if _, err := c.backend.driver.DeleteModules(ctx, artifacts.namePrefix); err != nil {
 		return resp, err
 	}
@@ -398,36 +436,33 @@ func (c *Client) RemoveTemplate(ctx context.Context, templ *templates.Constraint
 	if _, err := c.backend.driver.DeleteData(ctx, constraintRoot); err != nil {
 		return resp, err
 	}
-	delete(c.templates, c.templatesMapKey(artifacts))
+	delete(c.templates, artifacts.Key())
 	resp.Handled[artifacts.targetHandler.GetName()] = true
 	return resp, nil
 }
 
 // GetTemplate gets the currently recognized template.
 func (c *Client) GetTemplate(ctx context.Context, templ *templates.ConstraintTemplate) (*templates.ConstraintTemplate, error) {
 
-	artifacts, err := c.createBasicTemplateArtifacts(templ)
+	artifacts, err := c.createRawTemplateArtifacts(templ)
 	if err != nil {
 		return nil, err
 	}
 
 	c.constraintsMux.Lock()
 	defer c.constraintsMux.Unlock()
+	return c.getTemplateNoLock(ctx, artifacts)
+}
 
-	t, ok := c.templates[c.templatesMapKey(artifacts)]
+func (c *Client) getTemplateNoLock(ctx context.Context, artifacts keyableArtifact) (*templates.ConstraintTemplate, error) {
+	t, ok := c.templates[artifacts.Key()]
 	if !ok {
-		return nil, NewMissingTemplateError(c.templatesMapKey(artifacts))
+		return nil, NewMissingTemplateError(string(artifacts.Key()))
 	}
 	ret := t.template.DeepCopy()
 	return ret, nil
 }
 
-// templatesMapKey returns the key for where we will track the constraint template in
-// the templates map.
-func (c *Client) templatesMapKey(artifacts keyableArtifact) string {
-	return artifacts.CRD().Spec.Names.Kind
-}
-
 // createConstraintSubPath returns the key where we will store the constraint
 // for each target: cluster.<group>.<kind>.<name>
 func createConstraintSubPath(constraint *unstructured.Unstructured) (string, error) {
@@ -482,7 +517,7 @@ func (c *Client) getTemplateEntry(constraint *unstructured.Unstructured, lock bo
 		c.constraintsMux.RLock()
 		defer c.constraintsMux.RUnlock()
 	}
-	entry, ok := c.templates[kind]
+	entry, ok := c.templates[templateKeyFromConstraint(constraint)]
 	if !ok {
 		return nil, NewUnrecognizedConstraintError(kind)
 	}
@@ -697,7 +732,7 @@ func (c *Client) Reset(ctx context.Context) error {
 			}
 		}
 	}
-	c.templates = make(map[string]*templateEntry)
+	c.templates = make(map[templateKey]*templateEntry)
 	c.constraints = make(map[schema.GroupKind]map[string]*unstructured.Unstructured)
 	return nil
 }