From 97d977e80b9e775022bd2ac5063ece87c5688140 Mon Sep 17 00:00:00 2001 From: Jaydipkumar Arvindbhai Gabani Date: Wed, 31 Jul 2024 10:29:23 -0700 Subject: [PATCH] feat: adding scopedenforcementactions (#403) * adding scopedenforcementactions Signed-off-by: Jaydip Gabani * adding tests Signed-off-by: Jaydip Gabani * fixing tests Signed-off-by: Jaydip Gabani * refactoring template client and query Signed-off-by: Jaydip Gabani * refatoring queryopts to reviewopts Signed-off-by: Jaydip Gabani * updating ccomments Signed-off-by: Jaydip Gabani * removing EP variables Signed-off-by: Jaydip Gabani * generic webhook EP Signed-off-by: Jaydip Gabani * simplifying constraintToBinding Signed-off-by: Jaydip Gabani * adding comments Signed-off-by: Jaydip Gabani * checking lowercase eps, fixing nil variable access Signed-off-by: Jaydip Gabani * fixing file perm Signed-off-by: Jaydip Gabani * adding tests for case sensitivity and missing enforcementaction Signed-off-by: Jaydip Gabani * updating gk-webhook EP Signed-off-by: Jaydip Gabani * fixing test Signed-off-by: Jaydip Gabani * adding enforcement action and scoped enforcement actions in result and response spec Signed-off-by: Jaydip Gabani * fixing faulty test Signed-off-by: Jaydip Gabani * refactoring code for simplycity Signed-off-by: Jaydip Gabani * removing comments, removing * from review opts and client opts Signed-off-by: Jaydip Gabani * mandating client to be initialized with enforcment point Signed-off-by: Jaydip Gabani * case sensitive check for actions Signed-off-by: Jaydip Gabani * updating code comments Signed-off-by: Jaydip Gabani * removing comments Signed-off-by: Jaydip Gabani * removing enforcement action check while adding constriant to template client Signed-off-by: Jaydip Gabani * removing all enforcement points from matches Signed-off-by: Jaydip Gabani * preserving enforcement point names Signed-off-by: Jaydip Gabani --------- Signed-off-by: Jaydip Gabani Signed-off-by: Jaydipkumar Arvindbhai Gabani --- constraint/deploy/tools.go | 1 + constraint/pkg/apis/constraints/apis.go | 117 +++++++- constraint/pkg/apis/constraints/apis_test.go | 213 ++++++++++++++ .../apis/templates/zz_generated.deepcopy.go | 1 + constraint/pkg/client/client.go | 49 +++- constraint/pkg/client/client_internal_test.go | 11 + constraint/pkg/client/client_opts.go | 7 + constraint/pkg/client/client_opts_test.go | 4 +- constraint/pkg/client/client_test.go | 28 +- constraint/pkg/client/clienttest/client.go | 1 + .../pkg/client/clienttest/cts/constraints.go | 68 +++++ .../pkg/client/clienttest/cts/schema.go | 23 ++ constraint/pkg/client/constraint_client.go | 55 +++- constraint/pkg/client/crds/schema.go | 23 ++ constraint/pkg/client/drivers/fake/fake.go | 6 +- constraint/pkg/client/drivers/interface.go | 3 +- .../pkg/client/drivers/k8scel/driver.go | 20 +- .../k8scel/transform/make_vap_objects.go | 30 +- .../k8scel/transform/make_vap_objects_test.go | 53 ++-- constraint/pkg/client/drivers/query_opts.go | 26 -- constraint/pkg/client/drivers/rego/driver.go | 9 +- .../client/drivers/rego/driver_unit_test.go | 57 ++-- constraint/pkg/client/drivers/to_result.go | 13 +- constraint/pkg/client/e2e_test.go | 263 +++++++++++++++--- constraint/pkg/client/errors.go | 19 +- constraint/pkg/client/new_client.go | 5 + constraint/pkg/client/new_client_test.go | 9 +- constraint/pkg/client/reviews/review_opts.go | 34 +++ constraint/pkg/client/template_client.go | 25 +- constraint/pkg/regorewriter/regorewriter.go | 2 +- constraint/pkg/types/validation.go | 3 + 31 files changed, 979 insertions(+), 199 deletions(-) create mode 100644 constraint/pkg/apis/constraints/apis_test.go delete mode 100644 constraint/pkg/client/drivers/query_opts.go create mode 100644 constraint/pkg/client/reviews/review_opts.go diff --git a/constraint/deploy/tools.go b/constraint/deploy/tools.go index 71938e109..a3201a22e 100644 --- a/constraint/deploy/tools.go +++ b/constraint/deploy/tools.go @@ -1,3 +1,4 @@ +//go:build tools // +build tools // This existence of this package allows vendoring of the manifests in this directory by go 1.13+. diff --git a/constraint/pkg/apis/constraints/apis.go b/constraint/pkg/apis/constraints/apis.go index 4dbb15318..e4ddaf67d 100644 --- a/constraint/pkg/apis/constraints/apis.go +++ b/constraint/pkg/apis/constraints/apis.go @@ -5,18 +5,37 @@ import ( "fmt" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" ) +type ScopedEnforcementAction struct { + Action string `json:"action"` + EnforcementPoints []EnforcementPoint `json:"enforcementPoints"` +} + +type EnforcementAction string + +type EnforcementPoint struct { + Name string `json:"name"` +} + const ( // Group is the API Group of Constraints. Group = "constraints.gatekeeper.sh" - // EnforcementActionDeny indicates that if a review fails validation for a + // AllEnforcementPoints is a wildcard to indicate all enforcement points. + AllEnforcementPoints = "*" +) + +const ( + // Deny indicates that if a review fails validation for a // Constraint, that it should be rejected. Errors encountered running // validation are treated as failing validation. // // This is the default EnforcementAction. - EnforcementActionDeny = "deny" + Deny EnforcementAction = "deny" + Warn EnforcementAction = "warn" + Scoped EnforcementAction = "scoped" ) var ( @@ -26,6 +45,11 @@ var ( // ErrSchema is a specific error that a Constraint failed schema validation. ErrSchema = errors.New("schema validation failed") + + // ErrMissingRequiredField is a specific error that a field is missing from a Constraint. + ErrMissingRequiredField = errors.New("missing required field") + + ErrInvalidSpecEnforcementAction = errors.New("scopedEnforcementActions value must be a [{action: string, enforcementPoints: [{name: string}]}]") ) // GetEnforcementAction returns a Constraint's enforcementAction, which indicates @@ -40,8 +64,95 @@ func GetEnforcementAction(constraint *unstructured.Unstructured) (string, error) } if !found { - return EnforcementActionDeny, nil + return string(Deny), nil } return action, nil } + +func IsEnforcementActionScoped(action string) bool { + return action == string(Scoped) +} + +// GetEnforcementActionsForEP returns a map of enforcement actions for enforcement points passed in. +func GetEnforcementActionsForEP(constraint *unstructured.Unstructured, eps []string) (map[string][]string, error) { + scopedActions, found, err := getNestedFieldAsArray(constraint.Object, "spec", "scopedEnforcementActions") + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrInvalidSpecEnforcementAction, err) + } + if !found { + return nil, fmt.Errorf("%w: spec.scopedEnforcementActions must be defined", ErrMissingRequiredField) + } + + scopedEnforcementActions, err := convertToSliceScopedEnforcementAction(scopedActions) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrInvalidConstraint, err) + } + + enforcementPointsToActionsMap := make(map[string]map[string]bool) + for _, ep := range eps { + enforcementPointsToActionsMap[ep] = make(map[string]bool) + } + for _, scopedEA := range scopedEnforcementActions { + for _, enforcementPoint := range scopedEA.EnforcementPoints { + epName := enforcementPoint.Name + if epName == AllEnforcementPoints { + for _, ep := range eps { + enforcementPointsToActionsMap[ep][scopedEA.Action] = true + } + break + } + if _, ok := enforcementPointsToActionsMap[epName]; ok { + enforcementPointsToActionsMap[epName][scopedEA.Action] = true + } + } + } + enforcementActionsForEPs := make(map[string][]string) + for ep, actions := range enforcementPointsToActionsMap { + if len(actions) == 0 { + continue + } + enforcementActionsForEPs[ep] = make([]string, 0, len(actions)) + for action := range actions { + enforcementActionsForEPs[ep] = append(enforcementActionsForEPs[ep], action) + } + } + + return enforcementActionsForEPs, nil +} + +// Helper function to access nested fields as an array. +func getNestedFieldAsArray(obj map[string]interface{}, fields ...string) ([]interface{}, bool, error) { + value, found, err := unstructured.NestedFieldNoCopy(obj, fields...) + if err != nil { + return nil, false, err + } + if !found { + return nil, false, nil + } + if arr, ok := value.([]interface{}); ok { + return arr, true, nil + } + return nil, false, nil +} + +// Helper function to convert a value to a []ScopedEnforcementAction. +func convertToSliceScopedEnforcementAction(value interface{}) ([]ScopedEnforcementAction, error) { + var result []ScopedEnforcementAction + arr, ok := value.([]interface{}) + if !ok { + return nil, ErrInvalidSpecEnforcementAction + } + for _, v := range arr { + m, ok := v.(map[string]interface{}) + if !ok { + return nil, ErrInvalidSpecEnforcementAction + } + scopedEA := &ScopedEnforcementAction{} + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(m, scopedEA); err != nil { + return nil, err + } + result = append(result, *scopedEA) + } + return result, nil +} diff --git a/constraint/pkg/apis/constraints/apis_test.go b/constraint/pkg/apis/constraints/apis_test.go new file mode 100644 index 000000000..99254f822 --- /dev/null +++ b/constraint/pkg/apis/constraints/apis_test.go @@ -0,0 +1,213 @@ +package constraints + +import ( + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +const ( + // WebhookEnforcementPoint is the enforcement point for admission. + WebhookEnforcementPoint = "validation.gatekeeper.sh" + + // AuditEnforcementPoint is the enforcement point for audit. + AuditEnforcementPoint = "audit.gatekeeper.sh" + + // GatorEnforcementPoint is the enforcement point for gator cli. + GatorEnforcementPoint = "gator.gatekeeper.sh" +) + +func TestGetEnforcementActionsForEP(t *testing.T) { + tests := []struct { + name string + constraint *unstructured.Unstructured + eps []string + expected map[string]map[string]bool + err error + }{ + { + name: "wildcard enforcement point", + constraint: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "scopedEnforcementActions": []interface{}{ + map[string]interface{}{ + "enforcementPoints": []interface{}{ + map[string]interface{}{ + "name": AuditEnforcementPoint, + }, + map[string]interface{}{ + "name": WebhookEnforcementPoint, + }, + }, + "action": "warn", + }, + map[string]interface{}{ + "enforcementPoints": []interface{}{ + map[string]interface{}{ + "name": "*", + }, + }, + "action": "deny", + }, + }, + }, + }, + }, + expected: map[string]map[string]bool{ + AuditEnforcementPoint: { + "warn": true, + "deny": true, + }, + WebhookEnforcementPoint: { + "warn": true, + "deny": true, + }, + GatorEnforcementPoint: { + "deny": true, + }, + }, + eps: []string{AuditEnforcementPoint, WebhookEnforcementPoint, GatorEnforcementPoint}, + }, + { + name: "Actions for selective enforcement point with case sensitive input", + constraint: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "scopedEnforcementActions": []interface{}{ + map[string]interface{}{ + "enforcementPoints": []interface{}{ + map[string]interface{}{ + "name": AuditEnforcementPoint, + }, + map[string]interface{}{ + "name": "Validation.Gatekeeper.Sh", + }, + }, + "action": "warn", + }, + map[string]interface{}{ + "enforcementPoints": []interface{}{ + map[string]interface{}{ + "name": "*", + }, + }, + "action": "deny", + }, + }, + }, + }, + }, + expected: map[string]map[string]bool{ + WebhookEnforcementPoint: { + "deny": true, + }, + GatorEnforcementPoint: { + "deny": true, + }, + }, + eps: []string{WebhookEnforcementPoint, GatorEnforcementPoint}, + }, + { + name: "wildcard enforcement point in scoped enforcement action, get actions for two enforcement points", + constraint: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "scopedEnforcementActions": []interface{}{ + map[string]interface{}{ + "enforcementPoints": []interface{}{ + map[string]interface{}{ + "name": AuditEnforcementPoint, + }, + map[string]interface{}{ + "name": WebhookEnforcementPoint, + }, + }, + "action": "warn", + }, + map[string]interface{}{ + "enforcementPoints": []interface{}{ + map[string]interface{}{ + "name": AllEnforcementPoints, + }, + }, + "action": "deny", + }, + }, + }, + }, + }, + expected: map[string]map[string]bool{ + AuditEnforcementPoint: { + "warn": true, + "deny": true, + }, + WebhookEnforcementPoint: { + "warn": true, + "deny": true, + }, + }, + eps: []string{WebhookEnforcementPoint, AuditEnforcementPoint}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actions, err := GetEnforcementActionsForEP(tt.constraint, tt.eps) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + got := make(map[string]map[string]bool) + for ep, actions := range actions { + got[ep] = make(map[string]bool) + for _, action := range actions { + got[ep][action] = true + } + } + + if !reflect.DeepEqual(got, tt.expected) { + t.Errorf("Expected %v, got %v", tt.expected, actions) + } + }) + } +} + +func TestIsEnforcementActionScoped(t *testing.T) { + tests := []struct { + name string + action string + want bool + }{ + { + name: "scoped enforcement action", + action: "scoped", + want: true, + }, + { + name: "Scoped enforcement action", + action: "Scoped", + want: false, + }, + { + name: "Non-scoped enforcement action", + action: "Deny", + want: false, + }, + { + name: "Empty enforcement action", + action: "", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IsEnforcementActionScoped(tt.action) + if got != tt.want { + t.Errorf("Expected %v, got %v", tt.want, got) + } + }) + } +} diff --git a/constraint/pkg/apis/templates/zz_generated.deepcopy.go b/constraint/pkg/apis/templates/zz_generated.deepcopy.go index f1958b881..0b174a36a 100644 --- a/constraint/pkg/apis/templates/zz_generated.deepcopy.go +++ b/constraint/pkg/apis/templates/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated /* diff --git a/constraint/pkg/client/client.go b/constraint/pkg/client/client.go index 5a2a1468c..809c675cb 100644 --- a/constraint/pkg/client/client.go +++ b/constraint/pkg/client/client.go @@ -14,6 +14,7 @@ import ( regoSchema "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/rego/schema" "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors" clienterrors "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors" + "github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews" "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/instrumentation" @@ -59,6 +60,9 @@ type Client struct { // templates is a map from a Template's name to its entry. templates map[string]*templateClient + + // enforcementPoints is array of enforcement points for which this client may be used. + enforcementPoints []string } // driverForTemplate returns the driver to be used for a template according @@ -363,7 +367,7 @@ func (c *Client) AddConstraint(ctx context.Context, constraint *unstructured.Uns return resp, err } - changed, err := cached.AddConstraint(constraintWithDefaults) + changed, err := cached.AddConstraint(constraintWithDefaults, c.enforcementPoints) if err != nil { return resp, err } @@ -619,10 +623,27 @@ func (c *Client) RemoveData(ctx context.Context, data interface{}) (*types.Respo return resp, &errMap } -// Review makes sure the provided object satisfies all stored constraints. +// Review makes sure the provided object satisfies constraints applicable for specific enforcement points. // On error, the responses return value will still be populated so that // partial results can be analyzed. -func (c *Client) Review(ctx context.Context, obj interface{}, opts ...drivers.QueryOpt) (*types.Responses, error) { +func (c *Client) Review(ctx context.Context, obj interface{}, opts ...reviews.ReviewOpt) (*types.Responses, error) { + var eps []string + cfg := &reviews.ReviewCfg{} + for _, opt := range opts { + opt(cfg) + } + if cfg.EnforcementPoint == "" { + cfg.EnforcementPoint = apiconstraints.AllEnforcementPoints + } + for _, ep := range c.enforcementPoints { + if cfg.EnforcementPoint == apiconstraints.AllEnforcementPoints || cfg.EnforcementPoint == ep { + eps = append(eps, ep) + } + } + if eps == nil { + return nil, fmt.Errorf("%w, supported enforcement points: %v", ErrUnsupportedEnforcementPoints, c.enforcementPoints) + } + responses := types.NewResponses() errMap := make(clienterrors.ErrorMap) @@ -647,6 +668,8 @@ func (c *Client) Review(ctx context.Context, obj interface{}, opts ...drivers.Qu constraintsByTarget := make(map[string][]*unstructured.Unstructured) autorejections := make(map[string][]constraintMatchResult) + scopedEnforcementActionsByTarget := make(map[string]map[string][]string) + enforcementActionByTarget := make(map[string]map[string]string) var templateList []*templateClient @@ -659,18 +682,23 @@ func (c *Client) Review(ctx context.Context, obj interface{}, opts ...drivers.Qu for target, review := range reviews { var targetConstraints []*unstructured.Unstructured - + targetScopedEnforcementActions := make(map[string][]string) + targetEnforcementAction := make(map[string]string) for _, template := range templateList { - matchingConstraints := template.Matches(target, review) + matchingConstraints := template.Matches(target, review, eps) for _, matchResult := range matchingConstraints { if matchResult.error == nil { targetConstraints = append(targetConstraints, matchResult.constraint) + targetScopedEnforcementActions[matchResult.constraint.GetName()] = matchResult.scopedEnforcementActions + targetEnforcementAction[matchResult.constraint.GetName()] = matchResult.enforcementAction } else { autorejections[target] = append(autorejections[target], matchResult) } } } constraintsByTarget[target] = targetConstraints + scopedEnforcementActionsByTarget[target] = targetScopedEnforcementActions + enforcementActionByTarget[target] = targetEnforcementAction } for target, review := range reviews { @@ -682,6 +710,15 @@ func (c *Client) Review(ctx context.Context, obj interface{}, opts ...drivers.Qu continue } + for i := range resp.Results { + if val, ok := scopedEnforcementActionsByTarget[target][resp.Results[i].Constraint.GetName()]; ok { + resp.Results[i].ScopedEnforcementActions = val + } + if val, ok := enforcementActionByTarget[target][resp.Results[i].Constraint.GetName()]; ok { + resp.Results[i].EnforcementAction = val + } + } + for _, autorejection := range autorejections[target] { resp.AddResult(autorejection.ToResult()) } @@ -711,7 +748,7 @@ func (c *Client) Review(ctx context.Context, obj interface{}, opts ...drivers.Qu return responses, &errMap } -func (c *Client) review(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...drivers.QueryOpt) (*types.Response, []*instrumentation.StatsEntry, error) { +func (c *Client) review(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...reviews.ReviewOpt) (*types.Response, []*instrumentation.StatsEntry, error) { var results []*types.Result var stats []*instrumentation.StatsEntry var tracesBuilder strings.Builder diff --git a/constraint/pkg/client/client_internal_test.go b/constraint/pkg/client/client_internal_test.go index 01d010933..51b58b600 100644 --- a/constraint/pkg/client/client_internal_test.go +++ b/constraint/pkg/client/client_internal_test.go @@ -62,6 +62,7 @@ func TestMultiDriverAddTemplate(t *testing.T) { Driver(driverA), Driver(driverB), Driver(driverC), + EnforcementPoints("test"), ) if err != nil { t.Fatal(err) @@ -743,6 +744,7 @@ func TestMultiDriverAddTemplate(t *testing.T) { Driver(driverC), Driver(driverB), Driver(driverA), + EnforcementPoints("test"), ) if err != nil { t.Fatal(err) @@ -830,6 +832,7 @@ func TestMultiDriverRemoveTemplate(t *testing.T) { Driver(driverA), Driver(driverB), Driver(driverC), + EnforcementPoints("test"), ) if err != nil { t.Fatal(err) @@ -909,6 +912,7 @@ func TestDriverForTemplate(t *testing.T) { options: []Opt{ Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverA")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( @@ -923,6 +927,7 @@ func TestDriverForTemplate(t *testing.T) { options: []Opt{ Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverA")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( @@ -938,6 +943,7 @@ func TestDriverForTemplate(t *testing.T) { Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverA")), Driver(fake.New("driverB")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( @@ -953,6 +959,7 @@ func TestDriverForTemplate(t *testing.T) { Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverB")), Driver(fake.New("driverA")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( @@ -967,6 +974,7 @@ func TestDriverForTemplate(t *testing.T) { options: []Opt{ Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverA")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( @@ -982,6 +990,7 @@ func TestDriverForTemplate(t *testing.T) { options: []Opt{ Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverB")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( @@ -998,6 +1007,7 @@ func TestDriverForTemplate(t *testing.T) { Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverA")), Driver(fake.New("driverB")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( @@ -1014,6 +1024,7 @@ func TestDriverForTemplate(t *testing.T) { Targets(&handlertest.Handler{Name: ptr.To[string]("h1")}), Driver(fake.New("driverB")), Driver(fake.New("driverA")), + EnforcementPoints("test"), }, template: cts.New(cts.OptTargets( cts.TargetCustomEngines( diff --git a/constraint/pkg/client/client_opts.go b/constraint/pkg/client/client_opts.go index 0a70fb9b5..13a8ff902 100644 --- a/constraint/pkg/client/client_opts.go +++ b/constraint/pkg/client/client_opts.go @@ -72,3 +72,10 @@ func IgnoreNoReferentialDriverWarning(ignore bool) Opt { return nil } } + +func EnforcementPoints(eps ...string) Opt { + return func(client *Client) error { + client.enforcementPoints = eps + return nil + } +} diff --git a/constraint/pkg/client/client_opts_test.go b/constraint/pkg/client/client_opts_test.go index c6234aaab..59b5e13a9 100644 --- a/constraint/pkg/client/client_opts_test.go +++ b/constraint/pkg/client/client_opts_test.go @@ -11,7 +11,7 @@ import ( ) func TestAddingDrivers(t *testing.T) { - c, err := NewClient(Targets(&handlertest.Handler{Name: ptr.To[string]("foo")}), Driver(fake.New("driver1")), Driver(fake.New("driver2"))) + c, err := NewClient(Targets(&handlertest.Handler{Name: ptr.To[string]("foo")}), Driver(fake.New("driver1")), Driver(fake.New("driver2")), EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -27,7 +27,7 @@ func TestAddingDrivers(t *testing.T) { } func TestNoDuplicates(t *testing.T) { - _, err := NewClient(Targets(&handlertest.Handler{Name: ptr.To[string]("foo")}), Driver(fake.New("driver1")), Driver(fake.New("driver1"))) + _, err := NewClient(Targets(&handlertest.Handler{Name: ptr.To[string]("foo")}), Driver(fake.New("driver1")), Driver(fake.New("driver1")), EnforcementPoints("test")) if err == nil { t.Fatal("expected error, got none") } diff --git a/constraint/pkg/client/client_test.go b/constraint/pkg/client/client_test.go index b42830f12..120fb7de1 100644 --- a/constraint/pkg/client/client_test.go +++ b/constraint/pkg/client/client_test.go @@ -61,7 +61,7 @@ func TestBackend_NewClient_InvalidTargetName(t *testing.T) { t.Fatal(err) } - _, err = client.NewClient(client.Targets(tc.handler), client.Driver(d)) + _, err = client.NewClient(client.Targets(tc.handler), client.Driver(d), client.EnforcementPoints("test")) if !errors.Is(err, tc.wantError) { t.Errorf("got NewClient() error = %v, want %v", err, tc.wantError) @@ -143,7 +143,7 @@ func TestClient_AddData(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.handler1, tc.handler2), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.handler1, tc.handler2), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -248,7 +248,7 @@ func TestClient_RemoveData(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.handler1, tc.handler2), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.handler1, tc.handler2), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -405,7 +405,7 @@ r = 5 t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.targets...), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.targets...), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -515,7 +515,7 @@ func TestClient_RemoveTemplate(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -577,7 +577,7 @@ func TestClient_RemoveTemplate_ByNameOnly(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -642,7 +642,7 @@ func TestClient_GetTemplate(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -709,7 +709,7 @@ func TestClient_GetTemplate_ByNameOnly(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Driver(d), client.Targets(tc.handler)) + c, err := client.NewClient(client.Driver(d), client.Targets(tc.handler), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -748,7 +748,7 @@ func TestClient_RemoveTemplate_CascadingDelete(t *testing.T) { if err != nil { t.Fatal(err) } - c, err := client.NewClient(client.Targets(h), client.Driver(d)) + c, err := client.NewClient(client.Targets(h), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -982,7 +982,7 @@ func TestClient_AddConstraint(t *testing.T) { target = &handlertest.Handler{} } - c, err := client.NewClient(client.Targets(target), client.Driver(d)) + c, err := client.NewClient(client.Targets(target), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -1230,7 +1230,7 @@ func TestClient_AddConstraint_withDefaultParams(t *testing.T) { baseConstraint.Object["spec"] = tc.constraintSpec } - c, err := client.NewClient(client.Targets(target), client.Driver(fake.New("fake"))) + c, err := client.NewClient(client.Targets(target), client.Driver(fake.New("fake")), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -1337,7 +1337,7 @@ func TestClient_RemoveConstraint(t *testing.T) { t.Fatal(err) } h := &handlertest.Handler{} - c, err := client.NewClient(client.Targets(h), client.Driver(d)) + c, err := client.NewClient(client.Targets(h), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -1429,7 +1429,7 @@ violation[{"msg": "msg"}] { t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.handler), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } @@ -1731,7 +1731,7 @@ func TestClient_CreateCRD(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Targets(tc.targets...), client.Driver(d)) + c, err := client.NewClient(client.Targets(tc.targets...), client.Driver(d), client.EnforcementPoints("test")) if err != nil { t.Fatal(err) } diff --git a/constraint/pkg/client/clienttest/client.go b/constraint/pkg/client/clienttest/client.go index 1dfdea8ef..1a3104703 100644 --- a/constraint/pkg/client/clienttest/client.go +++ b/constraint/pkg/client/clienttest/client.go @@ -17,6 +17,7 @@ func defaults() []client.Opt { return []client.Opt{ client.Driver(d), client.Targets(&handlertest.Handler{Cache: &handlertest.Cache{}}), + client.EnforcementPoints("audit.gatekeeper.sh"), } } diff --git a/constraint/pkg/client/clienttest/cts/constraints.go b/constraint/pkg/client/clienttest/cts/constraints.go index 148ab9409..74171166e 100644 --- a/constraint/pkg/client/clienttest/cts/constraints.go +++ b/constraint/pkg/client/clienttest/cts/constraints.go @@ -41,6 +41,74 @@ func MakeConstraint(t testing.TB, kind, name string, args ...ConstraintArg) *uns return u } +func MakeConstraintWithoutActions(t testing.TB, kind, name string, args ...ConstraintArg) *unstructured.Unstructured { + t.Helper() + + u := &unstructured.Unstructured{Object: make(map[string]interface{})} + + u.SetGroupVersionKind(schema.GroupVersionKind{ + Group: constraints.Group, + Version: "v1beta1", + Kind: kind, + }) + u.SetName(name) + err := unstructured.SetNestedField(u.Object, make(map[string]interface{}), "spec", "parameters") + if err != nil { + t.Fatal(err) + } + + for _, arg := range args { + err = arg(u) + if err != nil { + t.Fatal(err) + } + } + + return u +} + +func MakeScopedEnforcementConstraint(t testing.TB, kind, name string, globalAction string, actions []string, eps ...string) *unstructured.Unstructured { + t.Helper() + + scopedEnforcementActions := make([]interface{}, len(actions)) + + for i, action := range actions { + enfocementPoints := make([]interface{}, len(eps)) + for j, point := range eps { + enfocementPoints[j] = map[string]interface{}{"name": point} + } + scopedEnforcementActions[i] = map[string]interface{}{ + "enforcementPoints": enfocementPoints, + "action": action, + } + } + + u := &unstructured.Unstructured{Object: map[string]interface{}{ + "spec": map[string]interface{}{ + "scopedEnforcementActions": scopedEnforcementActions, + }, + }} + + u.SetGroupVersionKind(schema.GroupVersionKind{ + Group: constraints.Group, + Version: "v1beta1", + Kind: kind, + }) + u.SetName(name) + + err := unstructured.SetNestedField(u.Object, make(map[string]interface{}), "spec", "parameters") + if err != nil { + t.Fatal(err) + } + + err = unstructured.SetNestedField(u.Object, globalAction, "spec", "enforcementAction") + if err != nil { + t.Fatal(err) + } + + return u +} + type ConstraintArg func(*unstructured.Unstructured) error // MatchNamespace modifies the Constraint to only match objects with the passed diff --git a/constraint/pkg/client/clienttest/cts/schema.go b/constraint/pkg/client/clienttest/cts/schema.go index 83cff203a..4004897e3 100644 --- a/constraint/pkg/client/clienttest/cts/schema.go +++ b/constraint/pkg/client/clienttest/cts/schema.go @@ -10,6 +10,29 @@ type PropMap map[string]apiextensions.JSONSchemaProps func ExpectedSchema(pm PropMap) *apiextensions.JSONSchemaProps { defaultEnforcementAction := apiextensions.JSON("deny") pm["enforcementAction"] = apiextensions.JSONSchemaProps{Type: "string", Default: &defaultEnforcementAction} + pm["scopedEnforcementActions"] = apiextensions.JSONSchemaProps{ + Type: "array", + Default: nil, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "action": {Type: "string"}, + "enforcementPoints": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": {Type: "string"}, + }, + }, + }, + }, + }, + }, + }, + } p := Prop( PropMap{ "metadata": Prop(PropMap{ diff --git a/constraint/pkg/client/constraint_client.go b/constraint/pkg/client/constraint_client.go index dd02b9ea1..0f801d031 100644 --- a/constraint/pkg/client/constraint_client.go +++ b/constraint/pkg/client/constraint_client.go @@ -3,6 +3,7 @@ package client import ( "fmt" + apiconstraints "github.com/open-policy-agent/frameworks/constraint/pkg/apis/constraints" "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors" "github.com/open-policy-agent/frameworks/constraint/pkg/core/constraints" "github.com/open-policy-agent/frameworks/constraint/pkg/types" @@ -16,24 +17,51 @@ type constraintClient struct { // constraint is a copy of the original Constraint added to Client. constraint *unstructured.Unstructured + // matchers are the per-target Matchers for this Constraint. + matchers map[string]constraints.Matcher + // enforcementAction is what should be done if the Constraint is violated or // fails to run on a review. + // passed to constraintMatchResult.enforcementAction. enforcementAction string - // matchers are the per-target Matchers for this Constraint. - matchers map[string]constraints.Matcher + // enforcementActionsForEP stores precompiled enforcement actions for each enforcement point. + enforcementActionsForEP map[string][]string } func (c *constraintClient) getConstraint() *unstructured.Unstructured { return c.constraint.DeepCopy() } -func (c *constraintClient) matches(target string, review interface{}) *constraintMatchResult { +func (c *constraintClient) matches(target string, review interface{}, enforcementPoints ...string) *constraintMatchResult { matcher, found := c.matchers[target] if !found { return nil } + enforcementActions := make(map[string]bool) + if apiconstraints.IsEnforcementActionScoped(c.enforcementAction) { + for _, ep := range enforcementPoints { + if acts, found := c.enforcementActionsForEP[ep]; found { + for _, act := range acts { + enforcementActions[act] = true + } + } + } + } + + // If enforcement action is scoped, constraint does not include enforcement point that needs to be enforced then there is no action to be taken. + if len(enforcementActions) == 0 && apiconstraints.IsEnforcementActionScoped(c.enforcementAction) { + return nil + } + + // If enforcement action is not scoped or constraint needs to be enforced for matching enforcement point, + // Then we need to match the constraint with review. Compute enforcement actions for the enforcement point. + // Pass the enforcement actions and c.enforcementAction to constraintMatchResult. + var actions []string + for action := range enforcementActions { + actions = append(actions, action) + } matches, err := matcher.Match(review) // We avoid DeepCopying the Constraint out of the Client cache here, only @@ -47,14 +75,17 @@ func (c *constraintClient) matches(target string, review interface{}) *constrain // determine if the Constraint matched, so we assume it violated the // Constraint. return &constraintMatchResult{ - constraint: c.constraint, - error: fmt.Errorf("%w: %v", errors.ErrAutoreject, err), - enforcementAction: c.enforcementAction, + constraint: c.constraint, + error: fmt.Errorf("%w: %v", errors.ErrAutoreject, err), + enforcementAction: c.enforcementAction, + scopedEnforcementActions: actions, } case matches: // Fill in Constraint, so we can pass it to the Driver to run. return &constraintMatchResult{ - constraint: c.constraint, + constraint: c.constraint, + enforcementAction: c.enforcementAction, + scopedEnforcementActions: actions, } default: // No match and no error, so no need to record a result. @@ -66,8 +97,9 @@ type constraintMatchResult struct { // constraint is a pointer to the Constraint. Not safe for modification. constraint *unstructured.Unstructured // enforcementAction, if specified, is the immediate action to take. - // Only filled in if error is non-nil. enforcementAction string + // scopedEnforcementActions are action to take for specific enforcement point. + scopedEnforcementActions []string // error is a problem encountered while attempting to run the Constraint's // Matcher. error error @@ -75,8 +107,9 @@ type constraintMatchResult struct { func (r *constraintMatchResult) ToResult() *types.Result { return &types.Result{ - Msg: r.error.Error(), - Constraint: r.constraint, - EnforcementAction: r.enforcementAction, + Msg: r.error.Error(), + Constraint: r.constraint, + EnforcementAction: r.enforcementAction, + ScopedEnforcementActions: r.scopedEnforcementActions, } } diff --git a/constraint/pkg/client/crds/schema.go b/constraint/pkg/client/crds/schema.go index f13c0dcc9..530ae2a42 100644 --- a/constraint/pkg/client/crds/schema.go +++ b/constraint/pkg/client/crds/schema.go @@ -13,6 +13,29 @@ func CreateSchema(templ *templates.ConstraintTemplate, target MatchSchemaProvide props := map[string]apiextensions.JSONSchemaProps{ "match": target.MatchSchema(), "enforcementAction": {Type: "string", Default: &defaultEnforcementAction}, + "scopedEnforcementActions": { + Type: "array", + Default: nil, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "action": {Type: "string"}, + "enforcementPoints": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": {Type: "string"}, + }, + }, + }, + }, + }, + }, + }, + }, } if templ.Spec.CRD.Spec.Validation != nil && templ.Spec.CRD.Spec.Validation.OpenAPIV3Schema != nil { diff --git a/constraint/pkg/client/drivers/fake/fake.go b/constraint/pkg/client/drivers/fake/fake.go index d7f364c60..498b20219 100644 --- a/constraint/pkg/client/drivers/fake/fake.go +++ b/constraint/pkg/client/drivers/fake/fake.go @@ -10,6 +10,7 @@ import ( apiconstraints "github.com/open-policy-agent/frameworks/constraint/pkg/apis/constraints" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/fake/schema" + "github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" "github.com/open-policy-agent/frameworks/constraint/pkg/types" "github.com/open-policy-agent/opa/storage" @@ -197,7 +198,7 @@ func (d *Driver) RemoveData(_ context.Context, _ string, _ storage.Path) error { return nil } -func (d *Driver) Query(_ context.Context, _ string, constraints []*unstructured.Unstructured, _ interface{}, _ ...drivers.QueryOpt) (*drivers.QueryResponse, error) { +func (d *Driver) Query(_ context.Context, _ string, constraints []*unstructured.Unstructured, _ interface{}, _ ...reviews.ReviewOpt) (*drivers.QueryResponse, error) { results := []*types.Result{} for i := range constraints { constraint := constraints[i] @@ -205,7 +206,8 @@ func (d *Driver) Query(_ context.Context, _ string, constraints []*unstructured. Msg: fmt.Sprintf("rejected by driver %s: %s", d.name, d.code[strings.ToLower(constraint.GetObjectKind().GroupVersionKind().Kind)]), Constraint: constraint, // TODO: the engine should not determine the enforcement action -- that does not work with CEL KEP - EnforcementAction: apiconstraints.EnforcementActionDeny, + ScopedEnforcementActions: []string{string(apiconstraints.Deny)}, + EnforcementAction: string(apiconstraints.Scoped), } results = append(results, result) } diff --git a/constraint/pkg/client/drivers/interface.go b/constraint/pkg/client/drivers/interface.go index 090c732a5..651e6e080 100644 --- a/constraint/pkg/client/drivers/interface.go +++ b/constraint/pkg/client/drivers/interface.go @@ -3,6 +3,7 @@ package drivers import ( "context" + "github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" "github.com/open-policy-agent/opa/storage" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -43,7 +44,7 @@ type Driver interface { // Query runs the passed target's Constraints against review. // Returns a QueryResponse type. // Returns an error if there was a problem executing the Query. - Query(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...QueryOpt) (*QueryResponse, error) + Query(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...reviews.ReviewOpt) (*QueryResponse, error) // Dump outputs the entire state of compiled Templates, added Constraints, and // cached data used for referential Constraints. diff --git a/constraint/pkg/client/drivers/k8scel/driver.go b/constraint/pkg/client/drivers/k8scel/driver.go index 9eb4621c0..dbb9b33b4 100755 --- a/constraint/pkg/client/drivers/k8scel/driver.go +++ b/constraint/pkg/client/drivers/k8scel/driver.go @@ -8,10 +8,10 @@ import ( "sync" "time" - apiconstraints "github.com/open-policy-agent/frameworks/constraint/pkg/apis/constraints" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers" pSchema "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/schema" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/k8scel/transform" + "github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" "github.com/open-policy-agent/frameworks/constraint/pkg/instrumentation" "github.com/open-policy-agent/frameworks/constraint/pkg/types" @@ -150,8 +150,8 @@ func (d *Driver) RemoveData(_ context.Context, _ string, _ storage.Path) error { return nil } -func (d *Driver) Query(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...drivers.QueryOpt) (*drivers.QueryResponse, error) { - cfg := &drivers.QueryCfg{} +func (d *Driver) Query(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...reviews.ReviewOpt) (*drivers.QueryResponse, error) { + cfg := &reviews.ReviewCfg{} for _, opt := range opts { opt(cfg) } @@ -200,20 +200,12 @@ func (d *Driver) Query(ctx context.Context, target string, constraints []*unstru // TODO: should namespace be made available, if possible? Generally that context should be present response := validator.Validate(ctx, versionedAttr.GetResource(), versionedAttr, constraint, nil, celAPI.PerCallLimit, nil) - enforcementAction, found, err := unstructured.NestedString(constraint.Object, "spec", "enforcementAction") - if err != nil { - return nil, err - } - if !found { - enforcementAction = apiconstraints.EnforcementActionDeny - } for _, decision := range response.Decisions { if decision.Action == validating.ActionDeny { results = append(results, &types.Result{ - Target: target, - Msg: decision.Message, - Constraint: constraint, - EnforcementAction: enforcementAction, + Target: target, + Msg: decision.Message, + Constraint: constraint, }) } } diff --git a/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go b/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go index e1493d637..3edba376d 100644 --- a/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go +++ b/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects.go @@ -75,20 +75,24 @@ func TemplateToPolicyDefinition(template *templates.ConstraintTemplate) (*admiss return policy, nil } -func ConstraintToBinding(constraint *unstructured.Unstructured) (*admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding, error) { - enforcementActionStr, err := apiconstraints.GetEnforcementAction(constraint) - if err != nil { - return nil, err +// ConstraintToBinding converts a Constraint to a ValidatingAdmissionPolicyBinding. +// Accepts a list of enforcement actions to apply to the binding. +// If the enforcement action is not recognized, returns an error. +func ConstraintToBinding(constraint *unstructured.Unstructured, actions []string) (*admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding, error) { + if len(actions) == 0 { + return nil, fmt.Errorf("%w: enforcement actions must be provided", ErrBadEnforcementAction) } + var enforcementActions []admissionregistrationv1beta1.ValidationAction - var enforcementAction admissionregistrationv1beta1.ValidationAction - switch enforcementActionStr { - case apiconstraints.EnforcementActionDeny: - enforcementAction = admissionregistrationv1beta1.Deny - case "warn": - enforcementAction = admissionregistrationv1beta1.Warn - default: - return nil, fmt.Errorf("%w: unrecognized enforcement action %s, must be `warn` or `deny`", ErrBadEnforcementAction, enforcementActionStr) + for _, action := range actions { + switch action { + case string(apiconstraints.Deny): + enforcementActions = append(enforcementActions, admissionregistrationv1beta1.Deny) + case string(apiconstraints.Warn): + enforcementActions = append(enforcementActions, admissionregistrationv1beta1.Warn) + default: + return nil, fmt.Errorf("%w: unrecognized enforcement action %s, must be `warn` or `deny`", ErrBadEnforcementAction, action) + } } binding := &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ @@ -102,7 +106,7 @@ func ConstraintToBinding(constraint *unstructured.Unstructured) (*admissionregis ParameterNotFoundAction: ptr.To[admissionregistrationv1beta1.ParameterNotFoundActionType](admissionregistrationv1beta1.AllowAction), }, MatchResources: &admissionregistrationv1beta1.MatchResources{}, - ValidationActions: []admissionregistrationv1beta1.ValidationAction{enforcementAction}, + ValidationActions: enforcementActions, }, } objectSelectorMap, found, err := unstructured.NestedMap(constraint.Object, "spec", "match", "labelSelector") diff --git a/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects_test.go b/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects_test.go index b5fe14441..6586a5dc0 100644 --- a/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects_test.go +++ b/constraint/pkg/client/drivers/k8scel/transform/make_vap_objects_test.go @@ -241,8 +241,7 @@ func TestTemplateToPolicyDefinition(t *testing.T) { } } -func newTestConstraint(enforcementAction string, namespaceSelector, labelSelector *metav1.LabelSelector) *unstructured.Unstructured { - constraint := &unstructured.Unstructured{} +func newTestConstraint(enforcementAction string, namespaceSelector, labelSelector *metav1.LabelSelector, constraint *unstructured.Unstructured) *unstructured.Unstructured { constraint.SetGroupVersionKind(rschema.GroupVersionKind{Group: constraints.Group, Version: "v1beta1", Kind: "FooTemplate"}) constraint.SetName("foo-name") if namespaceSelector != nil { @@ -273,14 +272,16 @@ func newTestConstraint(enforcementAction string, namespaceSelector, labelSelecto func TestConstraintToBinding(t *testing.T) { tests := []struct { - name string - constraint *unstructured.Unstructured - expectedErr error - expected *admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding + name string + constraint *unstructured.Unstructured + enforcementActions []string + expectedErr error + expected *admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding }{ { - name: "empty constraint", - constraint: newTestConstraint("", nil, nil), + name: "empty constraint", + constraint: newTestConstraint("", nil, nil, &unstructured.Unstructured{}), + enforcementActions: []string{"deny"}, expected: &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "gatekeeper-foo-name", @@ -297,8 +298,9 @@ func TestConstraintToBinding(t *testing.T) { }, }, { - name: "with object selector", - constraint: newTestConstraint("", nil, &metav1.LabelSelector{MatchLabels: map[string]string{"match": "yes"}}), + name: "with object selector", + constraint: newTestConstraint("", nil, &metav1.LabelSelector{MatchLabels: map[string]string{"match": "yes"}}, &unstructured.Unstructured{}), + enforcementActions: []string{"deny"}, expected: &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "gatekeeper-foo-name", @@ -317,8 +319,9 @@ func TestConstraintToBinding(t *testing.T) { }, }, { - name: "with namespace selector", - constraint: newTestConstraint("", &metav1.LabelSelector{MatchLabels: map[string]string{"match": "yes"}}, nil), + name: "with namespace selector", + enforcementActions: []string{"deny"}, + constraint: newTestConstraint("", &metav1.LabelSelector{MatchLabels: map[string]string{"match": "yes"}}, nil, &unstructured.Unstructured{}), expected: &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "gatekeeper-foo-name", @@ -337,8 +340,9 @@ func TestConstraintToBinding(t *testing.T) { }, }, { - name: "with both selectors", - constraint: newTestConstraint("", &metav1.LabelSelector{MatchLabels: map[string]string{"matchNS": "yes"}}, &metav1.LabelSelector{MatchLabels: map[string]string{"match": "yes"}}), + name: "with both selectors", + enforcementActions: []string{"deny"}, + constraint: newTestConstraint("", &metav1.LabelSelector{MatchLabels: map[string]string{"matchNS": "yes"}}, &metav1.LabelSelector{MatchLabels: map[string]string{"match": "yes"}}, &unstructured.Unstructured{}), expected: &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "gatekeeper-foo-name", @@ -358,8 +362,9 @@ func TestConstraintToBinding(t *testing.T) { }, }, { - name: "with explicit deny", - constraint: newTestConstraint("deny", nil, nil), + name: "with explicit deny", + enforcementActions: []string{"deny"}, + constraint: newTestConstraint("deny", nil, nil, &unstructured.Unstructured{}), expected: &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "gatekeeper-foo-name", @@ -376,8 +381,9 @@ func TestConstraintToBinding(t *testing.T) { }, }, { - name: "with warn", - constraint: newTestConstraint("warn", nil, nil), + name: "with warn", + enforcementActions: []string{"warn"}, + constraint: newTestConstraint("warn", nil, nil, &unstructured.Unstructured{}), expected: &admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "gatekeeper-foo-name", @@ -394,15 +400,16 @@ func TestConstraintToBinding(t *testing.T) { }, }, { - name: "unrecognized enforcement action", - constraint: newTestConstraint("magicunicorns", nil, nil), - expected: nil, - expectedErr: ErrBadEnforcementAction, + name: "unrecognized enforcement action", + enforcementActions: []string{"magicunicorns"}, + constraint: newTestConstraint("magicunicorns", nil, nil, &unstructured.Unstructured{}), + expected: nil, + expectedErr: ErrBadEnforcementAction, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - binding, err := ConstraintToBinding(test.constraint) + binding, err := ConstraintToBinding(test.constraint, test.enforcementActions) if !errors.Is(err, test.expectedErr) { t.Errorf("unexpected error. got %v; wanted %v", err, test.expectedErr) } diff --git a/constraint/pkg/client/drivers/query_opts.go b/constraint/pkg/client/drivers/query_opts.go deleted file mode 100644 index 4b244cc2f..000000000 --- a/constraint/pkg/client/drivers/query_opts.go +++ /dev/null @@ -1,26 +0,0 @@ -package drivers - -type QueryCfg struct { - TracingEnabled bool - StatsEnabled bool -} - -// QueryOpt specifies optional arguments for Query driver calls. -type QueryOpt func(*QueryCfg) - -// Tracing enables Rego tracing for a single query. -// If tracing is enabled for the Driver, Tracing(false) does not disable Tracing. -func Tracing(enabled bool) QueryOpt { - return func(cfg *QueryCfg) { - cfg.TracingEnabled = enabled - } -} - -// Stats(true) enables the driver to return evaluation stats for a single -// query. If stats is enabled for the Driver at construction time, then -// Stats(false) does not disable Stats for this single query. -func Stats(enabled bool) QueryOpt { - return func(cfg *QueryCfg) { - cfg.StatsEnabled = enabled - } -} diff --git a/constraint/pkg/client/drivers/rego/driver.go b/constraint/pkg/client/drivers/rego/driver.go index 44e088434..8f40529fa 100644 --- a/constraint/pkg/client/drivers/rego/driver.go +++ b/constraint/pkg/client/drivers/rego/driver.go @@ -15,6 +15,7 @@ import ( "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/rego/schema" clienterrors "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors" + "github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews" "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" "github.com/open-policy-agent/frameworks/constraint/pkg/externaldata" "github.com/open-policy-agent/frameworks/constraint/pkg/instrumentation" @@ -196,8 +197,8 @@ func (d *Driver) RemoveData(ctx context.Context, target string, path storage.Pat // input is the already-parsed Rego Value to use as input. // Returns the Rego results, the trace if requested, or an error if there was // a problem executing the query. -func (d *Driver) eval(ctx context.Context, compiler *ast.Compiler, target string, path []string, input ast.Value, opts ...drivers.QueryOpt) (rego.ResultSet, *string, error) { - cfg := &drivers.QueryCfg{} +func (d *Driver) eval(ctx context.Context, compiler *ast.Compiler, target string, path []string, input ast.Value, opts ...reviews.ReviewOpt) (rego.ResultSet, *string, error) { + cfg := &reviews.ReviewCfg{} for _, opt := range opts { opt(cfg) } @@ -241,7 +242,7 @@ func (d *Driver) eval(ctx context.Context, compiler *ast.Compiler, target string return res, t, err } -func (d *Driver) Query(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...drivers.QueryOpt) (*drivers.QueryResponse, error) { +func (d *Driver) Query(ctx context.Context, target string, constraints []*unstructured.Unstructured, review interface{}, opts ...reviews.ReviewOpt) (*drivers.QueryResponse, error) { if len(constraints) == 0 { return nil, nil } @@ -264,7 +265,7 @@ func (d *Driver) Query(ctx context.Context, target string, constraints []*unstru d.mtx.RLock() defer d.mtx.RUnlock() - cfg := &drivers.QueryCfg{} + cfg := &reviews.ReviewCfg{} for _, opt := range opts { opt(cfg) } diff --git a/constraint/pkg/client/drivers/rego/driver_unit_test.go b/constraint/pkg/client/drivers/rego/driver_unit_test.go index c17d00c40..f1fb00d33 100644 --- a/constraint/pkg/client/drivers/rego/driver_unit_test.go +++ b/constraint/pkg/client/drivers/rego/driver_unit_test.go @@ -16,9 +16,9 @@ import ( "github.com/open-policy-agent/frameworks/constraint/pkg/apis/constraints" "github.com/open-policy-agent/frameworks/constraint/pkg/apis/externaldata/unversioned" "github.com/open-policy-agent/frameworks/constraint/pkg/client/clienttest/cts" - "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/rego/schema" clienterrors "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors" + "github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews" "github.com/open-policy-agent/frameworks/constraint/pkg/externaldata" "github.com/open-policy-agent/frameworks/constraint/pkg/handler/handlertest" "github.com/open-policy-agent/frameworks/constraint/pkg/instrumentation" @@ -153,6 +153,14 @@ func TestDriver_Query(t *testing.T) { t.Fatalf("got AddConstraint() error = %v, want %v", err, nil) } + if err := d.AddConstraint(ctx, cts.MakeScopedEnforcementConstraint(t, "Fakes", "foo-2", "scoped", []string{"deny", "warn"}, "audit", "gator")); err != nil { + t.Fatalf("got AddConstraint() error = %v, want %v", err, nil) + } + + if err := d.AddConstraint(ctx, cts.MakeScopedEnforcementConstraint(t, "Fakes", "foo-3", "scoped", []string{"deny", "warn"}, "ep", "gator")); err != nil { + t.Fatalf("got AddConstraint() error = %v, want %v", err, nil) + } + qr, err := d.Query( ctx, cts.MockTargetHandler, @@ -184,6 +192,23 @@ func TestDriver_Query(t *testing.T) { if len(qr.Results) == 0 { t.Fatalf("got 0 errors on data-less query; want 1") } + + if err := d.RemoveData(ctx, cts.MockTargetHandler, nil); err != nil { + t.Fatalf("got RemoveData() error = %v, want %v", err, nil) + } + + qr, err = d.Query( + ctx, + cts.MockTargetHandler, + []*unstructured.Unstructured{cts.MakeScopedEnforcementConstraint(t, "Fakes", "foo-2", "scoped", []string{"deny", "warn"}, "audit", "gator")}, + map[string]interface{}{"hi": "there"}, + ) + if err != nil { + t.Fatalf("got Query() (#2) error = %v, want %v", err, nil) + } + if len(qr.Results) == 0 { + t.Fatalf("got 0 errors on data-less query; want 1") + } } // TestDriver_Query_Stats tests that StatsEntries are returned for both @@ -242,26 +267,26 @@ func TestDriver_Query_Stats(t *testing.T) { name string driverArgs []Arg constraints []*unstructured.Unstructured - opts []drivers.QueryOpt + opts []reviews.ReviewOpt expectedStatsEntries []*instrumentation.StatsEntry }{ { name: "violations; no stats enabled", constraints: []*unstructured.Unstructured{c1}, - opts: []drivers.QueryOpt{}, + opts: []reviews.ReviewOpt{}, expectedStatsEntries: []*instrumentation.StatsEntry{}, }, { name: "no violations; no stats enabled", constraints: []*unstructured.Unstructured{c2}, - opts: []drivers.QueryOpt{}, + opts: []reviews.ReviewOpt{}, expectedStatsEntries: []*instrumentation.StatsEntry{}, }, { name: "violations; stats enabled", constraints: []*unstructured.Unstructured{c1}, - opts: []drivers.QueryOpt{ - drivers.Stats(true), + opts: []reviews.ReviewOpt{ + reviews.Stats(true), }, expectedStatsEntries: []*instrumentation.StatsEntry{ { @@ -301,7 +326,7 @@ func TestDriver_Query_Stats(t *testing.T) { { name: "violations; stats enabled w driver args", constraints: []*unstructured.Unstructured{c1}, - opts: []drivers.QueryOpt{}, + opts: []reviews.ReviewOpt{}, driverArgs: []Arg{GatherStats()}, expectedStatsEntries: []*instrumentation.StatsEntry{ { @@ -341,8 +366,8 @@ func TestDriver_Query_Stats(t *testing.T) { { name: "no violations; stats enabled", constraints: []*unstructured.Unstructured{c2}, - opts: []drivers.QueryOpt{ - drivers.Stats(true), + opts: []reviews.ReviewOpt{ + reviews.Stats(true), }, expectedStatsEntries: []*instrumentation.StatsEntry{ { @@ -382,7 +407,7 @@ func TestDriver_Query_Stats(t *testing.T) { { name: "no violations; stats enabled w driver args", constraints: []*unstructured.Unstructured{c2}, - opts: []drivers.QueryOpt{}, + opts: []reviews.ReviewOpt{}, driverArgs: []Arg{GatherStats()}, expectedStatsEntries: []*instrumentation.StatsEntry{ { @@ -422,8 +447,8 @@ func TestDriver_Query_Stats(t *testing.T) { { name: "violations and no violations; stats enabled", constraints: []*unstructured.Unstructured{c1, c2}, - opts: []drivers.QueryOpt{ - drivers.Stats(true), + opts: []reviews.ReviewOpt{ + reviews.Stats(true), }, expectedStatsEntries: []*instrumentation.StatsEntry{ { @@ -463,8 +488,8 @@ func TestDriver_Query_Stats(t *testing.T) { { name: "violations and no violations; stats enabled; multiple kinds", constraints: []*unstructured.Unstructured{c1, c2, c3, c4}, - opts: []drivers.QueryOpt{ - drivers.Stats(true), + opts: []reviews.ReviewOpt{ + reviews.Stats(true), }, expectedStatsEntries: []*instrumentation.StatsEntry{ { @@ -536,8 +561,8 @@ func TestDriver_Query_Stats(t *testing.T) { { name: "violations; stats enabled; driver args on", constraints: []*unstructured.Unstructured{c1}, - opts: []drivers.QueryOpt{ - drivers.Stats(true), + opts: []reviews.ReviewOpt{ + reviews.Stats(true), }, driverArgs: []Arg{ Tracing(true), diff --git a/constraint/pkg/client/drivers/to_result.go b/constraint/pkg/client/drivers/to_result.go index 7f26d1835..72e8af9c6 100644 --- a/constraint/pkg/client/drivers/to_result.go +++ b/constraint/pkg/client/drivers/to_result.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" - apiconstraints "github.com/open-policy-agent/frameworks/constraint/pkg/apis/constraints" "github.com/open-policy-agent/frameworks/constraint/pkg/types" "github.com/open-policy-agent/opa/rego" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -74,20 +73,10 @@ func ToResult(constraints map[ConstraintKey]*unstructured.Unstructured, r rego.R Kind: keyMap["kind"], Name: keyMap["name"], } - constraint := constraints[key] + // DeepCopy the result so we don't leak internal state. result.Constraint = constraint.DeepCopy() - enforcementAction, found, err := unstructured.NestedString(constraint.Object, "spec", "enforcementAction") - if err != nil { - return nil, err - } - if !found { - enforcementAction = apiconstraints.EnforcementActionDeny - } - - result.EnforcementAction = enforcementAction - return result, nil } diff --git a/constraint/pkg/client/e2e_test.go b/constraint/pkg/client/e2e_test.go index 17aaedb38..3f200d0ba 100644 --- a/constraint/pkg/client/e2e_test.go +++ b/constraint/pkg/client/e2e_test.go @@ -12,9 +12,9 @@ import ( "github.com/open-policy-agent/frameworks/constraint/pkg/client" "github.com/open-policy-agent/frameworks/constraint/pkg/client/clienttest" "github.com/open-policy-agent/frameworks/constraint/pkg/client/clienttest/cts" - "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers" "github.com/open-policy-agent/frameworks/constraint/pkg/client/drivers/rego" clienterrors "github.com/open-policy-agent/frameworks/constraint/pkg/client/errors" + "github.com/open-policy-agent/frameworks/constraint/pkg/client/reviews" "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/handler/handlertest" @@ -27,16 +27,17 @@ import ( func TestClient_Review(t *testing.T) { tests := []struct { - name string - namespaces []string - targets []handler.TargetHandler - templates []*templates.ConstraintTemplate - constraints []*unstructured.Unstructured - inventory []*handlertest.Object - toReview interface{} - - wantResults []*types.Result - wantErr error + name string + namespaces []string + targets []handler.TargetHandler + templates []*templates.ConstraintTemplate + constraints []*unstructured.Unstructured + inventory []*handlertest.Object + toReview interface{} + enforcementPointFromReview string + enforcementPointSupportedByClient []string + wantResults []*types.Result + wantErr error }{ { name: "empty client", @@ -68,7 +69,24 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: "denied", - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), + Constraint: cts.MakeConstraint(t, clienttest.KindDeny, "constraint"), + }}, + }, + { + name: "deny all wihtout enforcementAction", + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeConstraintWithoutActions(t, clienttest.KindDeny, "constraint"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: []*types.Result{{ + Target: handlertest.TargetName, + Msg: "denied", + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindDeny, "constraint"), }}, }, @@ -112,7 +130,7 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: "denied", - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindDeny, "constraint"), }}, }, @@ -148,7 +166,7 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: "denied with library", - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindDenyImport, "constraint"), }}, }, @@ -192,7 +210,7 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: "got qux but want bar for data", - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindCheckData, "constraint", cts.WantData("bar")), }}, }, @@ -210,7 +228,7 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: `template:8: eval_conflict_error: functions must not produce multiple outputs for same inputs`, - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindRuntimeError, "constraint"), }}, }, @@ -229,7 +247,7 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: `unable to match constraints: not found: namespace "aaa" not in cache`, - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindCheckData, "constraint", cts.WantData("bar"), cts.MatchNamespace("aaa")), }}, @@ -251,7 +269,7 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: `unable to match constraints: not found: namespace "aaa" not in cache`, - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindCheckData, "constraint", cts.WantData("bar"), cts.MatchNamespace("aaa")), }, { @@ -279,7 +297,7 @@ func TestClient_Review(t *testing.T) { wantResults: []*types.Result{{ Target: handlertest.TargetName, Msg: "got qux but want bar for data", - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindCheckData, "constraint", cts.WantData("bar"), cts.MatchNamespace("billing")), }}, @@ -323,7 +341,7 @@ func TestClient_Review(t *testing.T) { Target: "foo2", Msg: "denied", Constraint: cts.MakeConstraint(t, cts.MockTemplate, "bar"), - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), }}, }, { @@ -362,7 +380,7 @@ func TestClient_Review(t *testing.T) { Target: handlertest.TargetName, Msg: "duplicate data bar", Constraint: cts.MakeConstraint(t, clienttest.KindForbidDuplicates, "constraint"), - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), }}, }, { @@ -381,7 +399,7 @@ func TestClient_Review(t *testing.T) { Target: handlertest.TargetName, Msg: "bad data", Constraint: cts.MakeConstraint(t, clienttest.KindFuture, "constraint"), - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), }}, }, { @@ -398,13 +416,190 @@ func TestClient_Review(t *testing.T) { toReview: handlertest.NewReview("", "foo", "3"), wantResults: nil, }, + { + name: "deny with scoped audit EP", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "audit", "webhook"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: []*types.Result{{ + Target: handlertest.TargetName, + Msg: "denied", + ScopedEnforcementActions: []string{string(constraints.Deny)}, + EnforcementAction: string(constraints.Scoped), + Constraint: cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "audit", "webhook"), + }}, + enforcementPointFromReview: "audit", + enforcementPointSupportedByClient: []string{"audit"}, + }, + { + name: "deny with scoped audit EP and webhook caller", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "audit"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: nil, + enforcementPointFromReview: "webhook", + enforcementPointSupportedByClient: []string{"audit"}, + wantErr: client.ErrUnsupportedEnforcementPoints, + }, + { + name: "deny with scoped test EP and test caller", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: []*types.Result{{ + Target: handlertest.TargetName, + Msg: "denied", + ScopedEnforcementActions: []string{string(constraints.Deny)}, + EnforcementAction: string(constraints.Scoped), + Constraint: cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test"), + }}, + enforcementPointFromReview: "test", + enforcementPointSupportedByClient: []string{"test"}, + }, + { + name: "scoped enforcement action without caller source EP in review", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: []*types.Result{{ + Target: handlertest.TargetName, + Msg: "denied", + ScopedEnforcementActions: []string{string(constraints.Deny)}, + EnforcementAction: string(constraints.Scoped), + Constraint: cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test"), + }}, + enforcementPointFromReview: "", + enforcementPointSupportedByClient: []string{"test"}, + }, + { + name: "client initialized for all EP, specific scopedEnforcementActions in constraints, without sourceEP", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test", "audit"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: []*types.Result{{ + Target: handlertest.TargetName, + Msg: "denied", + ScopedEnforcementActions: []string{string(constraints.Deny)}, + EnforcementAction: string(constraints.Scoped), + Constraint: cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test", "audit"), + }}, + enforcementPointFromReview: "", + enforcementPointSupportedByClient: []string{"test", "audit"}, + }, + { + name: "client initialized for all EP, specific scopedEnforcementActions in constraints, with sourceEP", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test", "audit"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: []*types.Result{{ + Target: handlertest.TargetName, + Msg: "denied", + ScopedEnforcementActions: []string{string(constraints.Deny)}, + EnforcementAction: string(constraints.Scoped), + Constraint: cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test", "audit"), + }}, + enforcementPointFromReview: "audit", + enforcementPointSupportedByClient: []string{"test", "audit"}, + }, + { + name: "enforcementAction and scopedEnforcementAction provided. enforcementAction: deny with scoped enforcement action", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Deny), []string{string(constraints.Deny)}, "test"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: []*types.Result{{ + Target: handlertest.TargetName, + Msg: "denied", + EnforcementAction: string(constraints.Deny), + Constraint: cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Deny), []string{string(constraints.Deny)}, "test"), + }}, + enforcementPointFromReview: "", + enforcementPointSupportedByClient: []string{"test"}, + }, + { + name: "case sensitive enforcement points", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "Test.gateKeeper.sh"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: nil, + wantErr: client.ErrUnsupportedEnforcementPoints, + enforcementPointFromReview: "teSt.gAtekeeper.sh", + enforcementPointSupportedByClient: []string{"tEst.Gatekeeper.sh"}, + }, + { + name: "review caller is webhook and constraint is for audit", + namespaces: nil, + targets: []handler.TargetHandler{&handlertest.Handler{}}, + templates: []*templates.ConstraintTemplate{ + clienttest.TemplateDeny(), + }, + constraints: []*unstructured.Unstructured{ + cts.MakeScopedEnforcementConstraint(t, clienttest.KindDeny, "constraint", string(constraints.Scoped), []string{string(constraints.Deny)}, "test", "audit"), + }, + toReview: handlertest.NewReview("", "foo", "bar"), + wantResults: nil, + enforcementPointFromReview: "webhook", + enforcementPointSupportedByClient: []string{"webhook"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - c := clienttest.New(t, client.Targets(tt.targets...)) + opts := []client.Opt{client.Targets(tt.targets...)} + if tt.enforcementPointSupportedByClient != nil { + opts = append(opts, client.EnforcementPoints(tt.enforcementPointSupportedByClient...)) + } + + c := clienttest.New(t, opts...) for _, ns := range tt.namespaces { _, err := c.AddData(ctx, &handlertest.Object{Namespace: ns}) @@ -434,7 +629,7 @@ func TestClient_Review(t *testing.T) { } } - responses, err := c.Review(ctx, tt.toReview) + responses, err := c.Review(ctx, tt.toReview, reviews.EnforcementPoint(tt.enforcementPointFromReview)) if !errors.Is(err, tt.wantErr) { t.Fatalf("got error %v, want %v", err, tt.wantErr) } @@ -481,7 +676,7 @@ func TestClient_Review_Details(t *testing.T) { want := []*types.Result{{ Target: handlertest.TargetName, Msg: "got qux but want bar for data", - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), Constraint: cts.MakeConstraint(t, clienttest.KindCheckData, "constraint", cts.WantData("bar")), Metadata: map[string]interface{}{"details": map[string]interface{}{"got": "qux"}}, @@ -517,7 +712,7 @@ func TestClient_Review_Print(t *testing.T) { Target: handlertest.TargetName, Msg: "denied", Constraint: cts.MakeConstraint(t, clienttest.KindDenyPrint, "denyprint"), - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), }, }, wantPrint: []string{"denied!"}, @@ -529,7 +724,7 @@ func TestClient_Review_Print(t *testing.T) { Target: handlertest.TargetName, Msg: "denied", Constraint: cts.MakeConstraint(t, clienttest.KindDenyPrint, "denyprint"), - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), }, }, wantPrint: nil, @@ -547,7 +742,7 @@ func TestClient_Review_Print(t *testing.T) { t.Fatal(err) } - c, err := client.NewClient(client.Targets(&handlertest.Handler{}), client.Driver(d)) + c, err := client.NewClient(client.Targets(&handlertest.Handler{}), client.Driver(d), client.EnforcementPoints("audit")) if err != nil { t.Fatal(err) } @@ -604,7 +799,7 @@ func TestE2E_RemoveConstraint(t *testing.T) { Target: handlertest.TargetName, Msg: "denied", Constraint: cts.MakeConstraint(t, clienttest.KindDeny, "foo"), - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), }} if diff := cmp.Diff(want, got, cmpopts.IgnoreFields(types.Result{}, "Metadata")); diff != "" { @@ -653,7 +848,7 @@ func TestE2E_RemoveTemplate(t *testing.T) { Target: handlertest.TargetName, Msg: "denied", Constraint: cts.MakeConstraint(t, clienttest.KindDeny, "foo"), - EnforcementAction: constraints.EnforcementActionDeny, + EnforcementAction: string(constraints.Deny), }} if diff := cmp.Diff(want, got, cmpopts.IgnoreFields(types.Result{}, "Metadata")); diff != "" { @@ -711,7 +906,7 @@ func TestE2E_Tracing(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - c := clienttest.New(t) + c := clienttest.New(t, client.EnforcementPoints("audit")) if tt.deny { _, err := c.AddTemplate(ctx, clienttest.TemplateDeny()) @@ -737,7 +932,7 @@ func TestE2E_Tracing(t *testing.T) { obj := handlertest.Review{Object: handlertest.Object{Name: "bar"}} - rsps, err := c.Review(ctx, obj, drivers.Tracing(tt.tracingEnabled)) + rsps, err := c.Review(ctx, obj, reviews.Tracing(tt.tracingEnabled)) if err != nil { t.Fatal(err) } @@ -820,7 +1015,7 @@ func TestE2E_Tracing_Unmatched(t *testing.T) { obj := handlertest.Review{Object: handlertest.Object{Name: "bar", Namespace: "ns"}} - rsps, err := c.Review(ctx, obj, drivers.Tracing(tt.tracingEnabled)) + rsps, err := c.Review(ctx, obj, reviews.Tracing(tt.tracingEnabled)) if err != nil { t.Fatal(err) } @@ -866,7 +1061,7 @@ func TestE2E_DriverStats(t *testing.T) { obj := handlertest.Review{Object: handlertest.Object{Name: "bar"}} - rsps, err := c.Review(ctx, obj, drivers.Stats(tt.statsEnabled)) + rsps, err := c.Review(ctx, obj, reviews.Stats(tt.statsEnabled)) if err != nil { t.Fatal(err) } diff --git a/constraint/pkg/client/errors.go b/constraint/pkg/client/errors.go index b6c918df4..27412623b 100644 --- a/constraint/pkg/client/errors.go +++ b/constraint/pkg/client/errors.go @@ -5,15 +5,16 @@ import ( ) var ( - ErrCreatingBackend = errors.New("unable to create backend") - ErrNoDriverName = errors.New("driver has no name") - ErrNoReferentialDriver = errors.New("no driver that supports referential constraints added") - ErrDuplicateDriver = errors.New("duplicate drivers of the same name") - ErrCreatingClient = errors.New("unable to create client") - ErrMissingConstraint = errors.New("missing Constraint") - ErrMissingConstraintTemplate = errors.New("missing ConstraintTemplate") - ErrInvalidModule = errors.New("invalid module") - ErrReview = errors.New("target.HandleReview failed") + ErrCreatingBackend = errors.New("unable to create backend") + ErrNoDriverName = errors.New("driver has no name") + ErrNoReferentialDriver = errors.New("no driver that supports referential constraints added") + ErrDuplicateDriver = errors.New("duplicate drivers of the same name") + ErrCreatingClient = errors.New("unable to create client") + ErrMissingConstraint = errors.New("missing Constraint") + ErrMissingConstraintTemplate = errors.New("missing ConstraintTemplate") + ErrInvalidModule = errors.New("invalid module") + ErrReview = errors.New("target.HandleReview failed") + ErrUnsupportedEnforcementPoints = errors.New("enforcement point not supported by client") ) // IsUnrecognizedConstraintError returns true if err is an ErrMissingConstraint. diff --git a/constraint/pkg/client/new_client.go b/constraint/pkg/client/new_client.go index 64d153e17..ada8010e7 100644 --- a/constraint/pkg/client/new_client.go +++ b/constraint/pkg/client/new_client.go @@ -25,5 +25,10 @@ func NewClient(opts ...Opt) (*Client, error) { ErrCreatingClient) } + if len(c.enforcementPoints) == 0 { + return nil, fmt.Errorf("%w: must specify at least one enforcement point with client.EnforcementPoints", + ErrCreatingClient) + } + return c, nil } diff --git a/constraint/pkg/client/new_client_test.go b/constraint/pkg/client/new_client_test.go index 41211fe80..bceb1fbbb 100644 --- a/constraint/pkg/client/new_client_test.go +++ b/constraint/pkg/client/new_client_test.go @@ -21,10 +21,15 @@ func TestNewClient(t *testing.T) { wantError: client.ErrCreatingClient, }, { - name: "with handler", - clientOpts: []client.Opt{client.Targets(&handlertest.Handler{})}, + name: "with handler with enforcement point", + clientOpts: []client.Opt{client.Targets(&handlertest.Handler{}), client.EnforcementPoints("test")}, wantError: nil, }, + { + name: "without enforcement points", + clientOpts: []client.Opt{client.Targets(&handlertest.Handler{})}, + wantError: client.ErrCreatingClient, + }, } for _, tc := range testCases { diff --git a/constraint/pkg/client/reviews/review_opts.go b/constraint/pkg/client/reviews/review_opts.go new file mode 100644 index 000000000..e4940f2bb --- /dev/null +++ b/constraint/pkg/client/reviews/review_opts.go @@ -0,0 +1,34 @@ +package reviews + +type ReviewCfg struct { + TracingEnabled bool + StatsEnabled bool + EnforcementPoint string +} + +// ReviewOpt specifies optional arguments for Query driver calls. +type ReviewOpt func(*ReviewCfg) + +// Tracing enables Rego tracing for a single query. +// If tracing is enabled for the Driver, Tracing(false) does not disable Tracing. +func Tracing(enabled bool) ReviewOpt { + return func(cfg *ReviewCfg) { + cfg.TracingEnabled = enabled + } +} + +// Stats(true) enables the driver to return evaluation stats for a single +// query. If stats is enabled for the Driver at construction time, then +// Stats(false) does not disable Stats for this single query. +func Stats(enabled bool) ReviewOpt { + return func(cfg *ReviewCfg) { + cfg.StatsEnabled = enabled + } +} + +// EnforcementPoint specifies the enforcement point to use for the query. +func EnforcementPoint(ep string) ReviewOpt { + return func(cfg *ReviewCfg) { + cfg.EnforcementPoint = ep + } +} diff --git a/constraint/pkg/client/template_client.go b/constraint/pkg/client/template_client.go index a5ef56c36..e79b9f5b0 100644 --- a/constraint/pkg/client/template_client.go +++ b/constraint/pkg/client/template_client.go @@ -95,12 +95,24 @@ func (e *templateClient) Update(templ *templates.ConstraintTemplate, crd *apiext // Returns true and no error if the Constraint was changed successfully. // Returns false and no error if the Constraint was not updated due to being // identical to the stored version. -func (e *templateClient) AddConstraint(constraint *unstructured.Unstructured) (bool, error) { +func (e *templateClient) AddConstraint(constraint *unstructured.Unstructured, enforcementPoints []string) (bool, error) { + enforcementActionsForEPs := make(map[string][]string) enforcementAction, err := apiconstraints.GetEnforcementAction(constraint) if err != nil { return false, err } + if apiconstraints.IsEnforcementActionScoped(enforcementAction) { + enforcementActionsForEPs, err = apiconstraints.GetEnforcementActionsForEP(constraint, enforcementPoints) + if err != nil { + return false, err + } + } else { + for _, ep := range enforcementPoints { + enforcementActionsForEPs[ep] = append(enforcementActionsForEPs[ep], enforcementAction) + } + } + // Compare with the already-existing Constraint. // If identical, exit early. cached, found := e.constraints[constraint.GetName()] @@ -117,9 +129,10 @@ func (e *templateClient) AddConstraint(constraint *unstructured.Unstructured) (b delete(cpy.Object, statusField) e.constraints[constraint.GetName()] = &constraintClient{ - constraint: cpy, - matchers: matchers, - enforcementAction: enforcementAction, + constraint: cpy, + matchers: matchers, + enforcementAction: enforcementAction, + enforcementActionsForEP: enforcementActionsForEPs, } return true, nil @@ -144,11 +157,11 @@ func (e *templateClient) RemoveConstraint(name string) { // against the passed review. // // ignoredTargets specifies the targets whose matchers to not run. -func (e *templateClient) Matches(target string, review interface{}) map[string]constraintMatchResult { +func (e *templateClient) Matches(target string, review interface{}, enforcementPoints []string) map[string]constraintMatchResult { result := make(map[string]constraintMatchResult) for name, constraint := range e.constraints { - cResult := constraint.matches(target, review) + cResult := constraint.matches(target, review, enforcementPoints...) if cResult != nil { result[name] = *cResult } diff --git a/constraint/pkg/regorewriter/regorewriter.go b/constraint/pkg/regorewriter/regorewriter.go index ab3c9d19c..815ae6642 100644 --- a/constraint/pkg/regorewriter/regorewriter.go +++ b/constraint/pkg/regorewriter/regorewriter.go @@ -162,7 +162,7 @@ func (r *RegoRewriter) addPathFromFs(path string, slice *[]*Module) error { } walkFn := func(path string, info os.FileInfo, _ error) error { - if info.IsDir() || !strings.HasSuffix(path, ".rego") { + if info == nil || (info.IsDir() || !strings.HasSuffix(path, ".rego")) { return nil } return r.addFileFromFs(path, slice) diff --git a/constraint/pkg/types/validation.go b/constraint/pkg/types/validation.go index f6a5ea404..2b40c1bc1 100644 --- a/constraint/pkg/types/validation.go +++ b/constraint/pkg/types/validation.go @@ -28,6 +28,9 @@ type Result struct { // The enforcement action of the constraint EnforcementAction string `json:"enforcementAction,omitempty"` + + // The scoped actions of the constraint + ScopedEnforcementActions []string `json:"scopedActions,omitempty"` } // Response is a collection of Constraint violations for a particular Target.