From 8b5a7b3847c12df33605bf3cc601ba53e5926523 Mon Sep 17 00:00:00 2001 From: Becky Huang Date: Fri, 14 Jan 2022 10:53:22 -0800 Subject: [PATCH] provide driver AddConstraint and RemoveConstraint (#170) Signed-off-by: Becky Huang --- constraint/pkg/client/client.go | 56 ++------ constraint/pkg/client/drivers/interface.go | 6 + constraint/pkg/client/drivers/local/args.go | 4 +- constraint/pkg/client/drivers/local/local.go | 129 ++++++++++++++++-- .../pkg/client/drivers/remote/remote.go | 10 ++ constraint/pkg/client/errors/errors.go | 2 + 6 files changed, 148 insertions(+), 59 deletions(-) diff --git a/constraint/pkg/client/client.go b/constraint/pkg/client/client.go index 07cb96c27..f03a47789 100644 --- a/constraint/pkg/client/client.go +++ b/constraint/pkg/client/client.go @@ -414,15 +414,6 @@ func createConstraintGKPath(target string, gk schema.GroupKind) string { return constraintPathMerge(target, createConstraintGKSubPath(gk)) } -// createConstraintPath returns the storage path for a given constraint: constraints..cluster.... -func createConstraintPath(target string, constraint *unstructured.Unstructured) (string, error) { - p, err := createConstraintSubPath(constraint) - if err != nil { - return "", err - } - return constraintPathMerge(target, p), nil -} - // constraintPathMerge is a shared function for creating constraint paths to // ensure uniformity, it is not meant to be called directly. func constraintPathMerge(target, subpath string) string { @@ -470,7 +461,6 @@ func (c *Client) AddConstraint(ctx context.Context, constraint *unstructured.Uns defer c.mtx.Unlock() resp := types.NewResponses() - errMap := make(clienterrors.ErrorMap) entry, err := c.getTemplateEntry(constraint, false) if err != nil { return resp, err @@ -493,27 +483,14 @@ 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 { + return resp, err + } for _, target := range entry.Targets { - relPath, err := createConstraintPath(target, constraint) - // If we ever create multi-target constraints we will need to handle this more cleverly. - // the short-circuiting question, cleanup, etc. - if err != nil { - errMap[target] = err - continue - } - if err := c.backend.driver.PutData(ctx, relPath, constraint.Object); err != nil { - errMap[target] = err - continue - } resp.Handled[target] = true } - - if len(errMap) == 0 { - c.constraints[constraint.GroupVersionKind().GroupKind()][subPath] = constraint.DeepCopy() - return resp, nil - } - - return resp, &errMap + c.constraints[constraint.GroupVersionKind().GroupKind()][subPath] = constraint.DeepCopy() + return resp, nil } // RemoveConstraint removes a constraint from OPA. On error, the responses @@ -527,7 +504,6 @@ func (c *Client) RemoveConstraint(ctx context.Context, constraint *unstructured. func (c *Client) removeConstraintNoLock(ctx context.Context, constraint *unstructured.Unstructured) (*types.Responses, error) { resp := types.NewResponses() - errMap := make(clienterrors.ErrorMap) entry, err := c.getTemplateEntry(constraint, false) if err != nil { return resp, err @@ -536,26 +512,14 @@ 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 { + return resp, err + } for _, target := range entry.Targets { - relPath, err := createConstraintPath(target, constraint) - // If we ever create multi-target constraints we will need to handle this more cleverly. - // the short-circuiting question, cleanup, etc. - if err != nil { - errMap[target] = err - continue - } - if _, err := c.backend.driver.DeleteData(ctx, relPath); err != nil { - errMap[target] = err - } resp.Handled[target] = true } - if len(errMap) == 0 { - // If we ever create multi-target constraints we will need to handle this more cleverly. - // the short-circuiting question, cleanup, etc. - delete(c.constraints[constraint.GroupVersionKind().GroupKind()], subPath) - return resp, nil - } - return resp, &errMap + delete(c.constraints[constraint.GroupVersionKind().GroupKind()], subPath) + return resp, nil } // getConstraintNoLock gets the currently recognized constraint without the lock. diff --git a/constraint/pkg/client/drivers/interface.go b/constraint/pkg/client/drivers/interface.go index 55f1185cc..970f0c91c 100644 --- a/constraint/pkg/client/drivers/interface.go +++ b/constraint/pkg/client/drivers/interface.go @@ -3,6 +3,8 @@ package drivers import ( "context" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" "github.com/open-policy-agent/frameworks/constraint/pkg/types" ) @@ -27,6 +29,10 @@ type Driver interface { AddTemplate(ct *templates.ConstraintTemplate) error // RemoveTemplate removes the template source code from OPA RemoveTemplate(ctx context.Context, ct *templates.ConstraintTemplate) error + // AddConstraint inserts validated constraint into OPA + AddConstraint(ctx context.Context, constraint *unstructured.Unstructured) error + // RemoveConstraint removes a constraint from OPA + RemoveConstraint(ctx context.Context, constraint *unstructured.Unstructured) error PutData(ctx context.Context, path string, data interface{}) error DeleteData(ctx context.Context, path string) (bool, error) Query(ctx context.Context, path string, input interface{}, opts ...QueryOpt) (*types.Response, error) diff --git a/constraint/pkg/client/drivers/local/args.go b/constraint/pkg/client/drivers/local/args.go index 3db95ff8a..1ea781413 100644 --- a/constraint/pkg/client/drivers/local/args.go +++ b/constraint/pkg/client/drivers/local/args.go @@ -21,7 +21,9 @@ func Defaults() Arg { if d.modules == nil { d.modules = make(map[string]*ast.Module) } - + if d.templateHandler == nil { + d.templateHandler = make(map[string][]string) + } if d.storage == nil { d.storage = inmem.New() } diff --git a/constraint/pkg/client/drivers/local/local.go b/constraint/pkg/client/drivers/local/local.go index 88cb0b22c..7a55fa655 100644 --- a/constraint/pkg/client/drivers/local/local.go +++ b/constraint/pkg/client/drivers/local/local.go @@ -8,12 +8,15 @@ import ( "fmt" "io/ioutil" "net/http" + "path" "sort" "strings" "sync" "time" clienterrors "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" "github.com/open-policy-agent/frameworks/constraint/pkg/handler" "github.com/open-policy-agent/frameworks/constraint/pkg/regorewriter" @@ -70,18 +73,20 @@ func New(args ...Arg) drivers.Driver { var _ drivers.Driver = &Driver{} type Driver struct { - modulesMux sync.RWMutex - compiler *ast.Compiler - modules map[string]*ast.Module - storage storage.Store - capabilities *ast.Capabilities - traceEnabled bool - printEnabled bool - printHook print.Hook - providerCache *externaldata.ProviderCache - externs []string - // handlers is a map from handler name to the respective handler - handlers map[string]handler.TargetHandler + modulesMux sync.RWMutex + compiler *ast.Compiler + modules map[string]*ast.Module + // templateHandler stores the template kind and the mapping target handlers + templateHandler map[string][]string + storage storage.Store + capabilities *ast.Capabilities + traceEnabled bool + printEnabled bool + printHook print.Hook + providerCache *externaldata.ProviderCache + externs []string + // handlers is a map from handler name to the respective handler + handlers map[string]handler.TargetHandler } func (d *Driver) Init() error { @@ -566,6 +571,9 @@ func (d *Driver) AddTemplate(templ *templates.ConstraintTemplate) error { if err = d.putModules(namePrefix, mods); err != nil { return fmt.Errorf("%w: %v", clienterrors.ErrCompile, err) } + d.templateHandler[templ.Spec.CRD.Spec.Names.Kind] = []string{ + templ.Spec.Targets[0].Target, + } return nil } @@ -578,9 +586,51 @@ func (d *Driver) RemoveTemplate(ctx context.Context, templ *templates.Constraint kind := templ.Spec.CRD.Spec.Names.Kind namePrefix := createTemplatePath(targetHandler, kind) _, err := d.deleteModules(namePrefix) + delete(d.templateHandler, templ.Spec.CRD.Spec.Names.Kind) return err } +func (d *Driver) AddConstraint(ctx context.Context, constraint *unstructured.Unstructured) error { + handlers, err := d.getTargetHandlers(constraint) + if err != nil { + return err + } + for _, target := range handlers { + relPath, err := createConstraintPath(target, constraint) + // If we ever create multi-target constraints we will need to handle this more cleverly. + // the short-circuiting question, cleanup, etc. + if err != nil { + return err + } + if err := d.PutData(ctx, relPath, constraint.Object); err != nil { + return err + } + } + return nil +} + +func (d *Driver) RemoveConstraint(ctx context.Context, constraint *unstructured.Unstructured) error { + handlers, err := d.getTargetHandlers(constraint) + if err != nil { + if errors.Is(err, ErrMissingConstraintTemplate) { + return nil + } + return err + } + for _, target := range handlers { + relPath, err := createConstraintPath(target, constraint) + // If we ever create multi-target constraints we will need to handle this more cleverly. + // the short-circuiting question, cleanup, etc. + if err != nil { + return err + } + if _, err := d.DeleteData(ctx, relPath); err != nil { + return err + } + } + return nil +} + // templateLibPrefix returns the new lib prefix for the libs that are specified in the CT. func templateLibPrefix(target, name string) string { return fmt.Sprintf("libs.%s.%s", target, name) @@ -668,6 +718,61 @@ func validateTargets(templ *templates.ConstraintTemplate) error { } } +// createConstraintGKPath returns the subpath for given a constraint GK. +func createConstraintGKSubPath(gk schema.GroupKind) string { + return "/" + path.Join("cluster", gk.Group, gk.Kind) +} + +// createConstraintSubPath returns the key where we will store the constraint +// for each target: cluster.... +func createConstraintSubPath(constraint *unstructured.Unstructured) (string, error) { + if constraint.GetName() == "" { + return "", fmt.Errorf("%w: missing name", ErrInvalidConstraint) + } + + gvk := constraint.GroupVersionKind() + if gvk.Group == "" { + return "", fmt.Errorf("%w: empty group for constrant %q", + ErrInvalidConstraint, constraint.GetName()) + } + + if gvk.Kind == "" { + return "", fmt.Errorf("%w: empty kind for constraint %q", + ErrInvalidConstraint, constraint.GetName()) + } + + return path.Join(createConstraintGKSubPath(gvk.GroupKind()), constraint.GetName()), nil +} + +// constraintPathMerge is a shared function for creating constraint paths to +// ensure uniformity, it is not meant to be called directly. +func constraintPathMerge(target, subpath string) string { + return "/" + path.Join("constraints", target, subpath) +} + +// createConstraintPath returns the storage path for a given constraint: constraints..cluster.... +func createConstraintPath(target string, constraint *unstructured.Unstructured) (string, error) { + p, err := createConstraintSubPath(constraint) + if err != nil { + return "", err + } + return constraintPathMerge(target, p), nil +} + +func (d *Driver) getTargetHandlers(constraint *unstructured.Unstructured) ([]string, error) { + kind := constraint.GetKind() + handlers, ok := d.templateHandler[kind] + if !ok { + var known []string + for k := range d.templateHandler { + known = append(known, k) + } + return nil, fmt.Errorf("%w: Constraint kind %q is not recognized, known kinds %v", + ErrMissingConstraintTemplate, kind, known) + } + return handlers, nil +} + func (d *Driver) SetExterns(fields []string) { d.externs = fields } diff --git a/constraint/pkg/client/drivers/remote/remote.go b/constraint/pkg/client/drivers/remote/remote.go index 05cbfe6fe..5a94e4254 100644 --- a/constraint/pkg/client/drivers/remote/remote.go +++ b/constraint/pkg/client/drivers/remote/remote.go @@ -9,6 +9,8 @@ import ( "net/url" "strings" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers" @@ -103,6 +105,14 @@ func (d *driver) RemoveTemplate(ctx context.Context, ct *templates.ConstraintTem panic("not implemented") } +func (d *driver) AddConstraint(ctx context.Context, constraint *unstructured.Unstructured) error { + panic("not implemented") +} + +func (d *driver) RemoveConstraint(ctx context.Context, constraint *unstructured.Unstructured) error { + panic("not implemented") +} + func (d *driver) PutData(_ context.Context, path string, data interface{}) error { return d.opa.PutData(path, data) } diff --git a/constraint/pkg/client/errors/errors.go b/constraint/pkg/client/errors/errors.go index 2bdd42043..14f7c6970 100644 --- a/constraint/pkg/client/errors/errors.go +++ b/constraint/pkg/client/errors/errors.go @@ -14,5 +14,7 @@ var ( ErrTransaction = errors.New("error committing data") ErrInvalidConstraintTemplate = errors.New("invalid ConstraintTemplate") + ErrInvalidConstraint = errors.New("invalid Constraint") + ErrMissingConstraintTemplate = errors.New("missing ConstraintTemplate") ErrInvalidModule = errors.New("invalid module") )