Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove direct storage.Path requirement in handler #208

Merged
merged 8 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
uses: golangci/golangci-lint-action@v2
with:
# version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
version: v1.42.1
version: v1.45.2
working-directory: constraint

test:
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Lint cache
/constraint/.tmp

# Binaries for programs and plugins
*.exe
Expand Down
15 changes: 14 additions & 1 deletion constraint/Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# When updating this, make sure to update the corresponding action in
# workflow.yaml
GOLANGCI_LINT_VERSION := v1.45.2

# Detects the location of the user golangci-lint cache.
GOLANGCI_LINT_CACHE := $(shell pwd)/.tmp/golangci-lint

all: test

# Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
Expand Down Expand Up @@ -47,8 +54,14 @@ manifests:
sed -i '/- externaldata.gatekeeper.sh_providers.yaml/d' .staging/templatecrd/kustomization.yaml
kustomize build .staging/templatecrd --output=.staging/templatecrd/crd.yaml

# lint runs a dockerized golangci-lint, and should give consistent results
# across systems.
# Source: https://golangci-lint.run/usage/install/#docker
lint:
golangci-lint -v run ./... --timeout 5m
docker run --rm -v $(shell pwd):/app \
-v ${GOLANGCI_LINT_CACHE}:/root/.cache/golangci-lint \
-w /app golangci/golangci-lint:${GOLANGCI_LINT_VERSION}-alpine \
golangci-lint run -v

# Generate code
# Not working? Try running `make gen-dependencies`
Expand Down
12 changes: 5 additions & 7 deletions constraint/pkg/handler/cache.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package handler

import "github.com/open-policy-agent/opa/storage"

// Cacher is a type - usually a Handler - which needs to cache state.
// Handlers only need implement this interface if they have need of a cache.
// Handlers which do not implement Cacher are assumed to be stateless from
Expand All @@ -22,25 +20,25 @@ type Cacher interface {
type Cache interface {
// Add inserts a new object into Cache with identifier key. If an object
// already exists, replaces the object at key.
Add(relPath storage.Path, object interface{}) error
Add(relPath []string, object interface{}) error

// Remove deletes the object at key from Cache. Deletion succeeds if key
// does not exist.
// Remove always succeeds; if for some reason key cannot be deleted the application
// should panic.
Remove(relPath storage.Path)
Remove(relPath []string)
}

type NoCache struct{}

func (n NoCache) Add(relPath storage.Path, object interface{}) error {
func (n NoCache) Add(relPath []string, object interface{}) error {
return nil
}

func (n NoCache) Get(relPath storage.Path) (interface{}, error) {
func (n NoCache) Get(relPath []string) (interface{}, error) {
return nil, nil
}

func (n NoCache) Remove(relPath storage.Path) {}
func (n NoCache) Remove(relPath []string) {}

var _ Cache = NoCache{}
3 changes: 1 addition & 2 deletions constraint/pkg/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package handler
import (
"github.com/open-policy-agent/frameworks/constraint/pkg/client/crds"
"github.com/open-policy-agent/frameworks/constraint/pkg/core/constraints"
"github.com/open-policy-agent/opa/storage"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

Expand All @@ -27,7 +26,7 @@ type TargetHandler interface {
// data.inventory.x.y.z would return []string{"x", "y", "z"}
// inventoryFormat: the data as an object that can be cast into JSON and suitable for storage in the inventory
// err: any error encountered
ProcessData(data interface{}) (handle bool, key storage.Path, inventoryFormat interface{}, err error)
ProcessData(data interface{}) (handle bool, key []string, inventoryFormat interface{}, err error)

// HandleReview determines if this target handler will handle an individual
// resource review and if so, builds the `review` field of the input object.
Expand Down
8 changes: 4 additions & 4 deletions constraint/pkg/handler/handlertest/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type Cache struct {
var _ handler.Cache = &Cache{}

// Add inserts object into Cache if object is a Namespace.
func (c *Cache) Add(key storage.Path, object interface{}) error {
func (c *Cache) Add(key []string, object interface{}) error {
obj, ok := object.(*Object)
if !ok {
return fmt.Errorf("%w: got object type %T, want %T", ErrInvalidType, object, &Object{})
Expand All @@ -34,10 +34,10 @@ func (c *Cache) Add(key storage.Path, object interface{}) error {
return fmt.Errorf("%w: must specify one of Name or Namespace", ErrInvalidObject)
}

c.Namespaces.Store(key.String(), object)
c.Namespaces.Store(storage.Path(key).String(), object)
return nil
}

func (c *Cache) Remove(key storage.Path) {
c.Namespaces.Delete(key.String())
func (c *Cache) Remove(key []string) {
c.Namespaces.Delete(storage.Path(key).String())
}
3 changes: 1 addition & 2 deletions constraint/pkg/handler/handlertest/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

"github.com/open-policy-agent/frameworks/constraint/pkg/core/constraints"
"github.com/open-policy-agent/frameworks/constraint/pkg/handler"
"github.com/open-policy-agent/opa/storage"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)
Expand Down Expand Up @@ -42,7 +41,7 @@ func (h *Handler) GetName() string {
return TargetName
}

func (h *Handler) ProcessData(obj interface{}) (bool, storage.Path, interface{}, error) {
func (h *Handler) ProcessData(obj interface{}) (bool, []string, interface{}, error) {
switch o := obj.(type) {
case *Object:
if h.ProcessDataError != nil {
Expand Down
3 changes: 2 additions & 1 deletion constraint/pkg/handler/handlertest/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

"github.com/open-policy-agent/frameworks/constraint/pkg/core/constraints"
"github.com/open-policy-agent/opa/storage"
)

var (
Expand Down Expand Up @@ -40,7 +41,7 @@ func (m Matcher) Match(review interface{}) (bool, error) {
}

key := Object{Namespace: reviewObj.Object.Namespace}.Key()
_, exists := m.Cache.Namespaces.Load(key.String())
_, exists := m.Cache.Namespaces.Load(storage.Path(key).String())
if !exists {
return false, fmt.Errorf("%w: namespace %q not in cache",
ErrNotFound, m.Namespace)
Expand Down
6 changes: 1 addition & 5 deletions constraint/pkg/handler/handlertest/object.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package handlertest

import (
"github.com/open-policy-agent/opa/storage"
)

// Object is a test object under review. The idea is to represent objects just
// complex enough to showcase (and test) the features of frameworks's Client,
// Drivers, and Handlers.
Expand All @@ -20,7 +16,7 @@ type Object struct {
Data string `json:"data"`
}

func (o Object) Key() storage.Path {
func (o Object) Key() []string {
if o.Namespace == "" {
return []string{"cluster", o.Name}
}
Expand Down