From ba1167ec1bd885bad7deedf04d48e05e13f5249a Mon Sep 17 00:00:00 2001 From: Joseph Lewis III Date: Thu, 10 Jan 2019 08:04:56 -0800 Subject: [PATCH 1/3] initial addition of policies --- pkg/broker/policy/policy.go | 93 ++++++++++++++++++++++++++++++++ pkg/broker/policy/policy_test.go | 80 +++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) create mode 100644 pkg/broker/policy/policy.go create mode 100644 pkg/broker/policy/policy_test.go diff --git a/pkg/broker/policy/policy.go b/pkg/broker/policy/policy.go new file mode 100644 index 000000000..e1e38f978 --- /dev/null +++ b/pkg/broker/policy/policy.go @@ -0,0 +1,93 @@ +// Copyright 2019 the Service Broker Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package policy + +import ( + "fmt" + "reflect" +) + +// Conditions are a set of values that can be compared with a base truth and +// return true if all of the facets of the condition match. +// +// Blank facets are assumed to match everything. +type Condition struct { + ServiceId string `json:"service_id"` + ServiceName string `json:"service_name"` +} + +// AppliesTo returns true if all the facets of this condition match the given +// truth. +func (cond *Condition) AppliesTo(truth Condition) bool { + if cond.ServiceId != "" && cond.ServiceId != truth.ServiceId { + return false + } + + if cond.ServiceName != "" && cond.ServiceName != truth.ServiceName { + return false + } + + return true +} + +// Policy combines a condition with several sets of values that are set if +// the condition holds true. +type Policy struct { + Comment string `json:"//"` + Condition Condition `json:"if"` + + Actions map[string]interface{} `json:"then"` +} + +type PolicyList struct { + Policies []Policy `json:"policy"` + Assertions []Policy `json:"assert"` +} + +// CheckAssertions tests each assertion in the Assertions list against the +// policies list. The conditoin is used as the ground truth and the +// actions are used as the expected output. If the actions don't match then +// an error is returned. +func (pl *PolicyList) CheckAssertions() error { + for i, assertion := range pl.Assertions { + expected := assertion.Actions + actual := pl.Apply(assertion.Condition) + + if !reflect.DeepEqual(actual, expected) { + return fmt.Errorf("Error in assertion %d, comment: %q, expected: %v, actual: %v", i+1, assertion.Comment, expected, actual) + } + } + + return nil +} + +// Apply runs through the list of policies, first to last, and cascades the +// values of each if they match the given condition, returning the merged +// map at the end. +func (pl *PolicyList) Apply(groundTruth Condition) map[string]interface{} { + out := make(map[string]interface{}) + + for _, policy := range pl.Policies { + if !policy.Condition.AppliesTo(groundTruth) { + continue + } + + for k, v := range policy.Actions { + out[k] = v + } + } + + return out +} diff --git a/pkg/broker/policy/policy_test.go b/pkg/broker/policy/policy_test.go new file mode 100644 index 000000000..f3893e14f --- /dev/null +++ b/pkg/broker/policy/policy_test.go @@ -0,0 +1,80 @@ +// Copyright 2019 the Service Broker Project Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package policy + +import "testing" + +func TestCondition_AppliesTo(t *testing.T) { + cases := map[string]struct { + Condition Condition + Truth Condition + Expected bool + }{ + "matches everything": { + Condition: Condition{}, + Truth: Condition{}, + Expected: true, + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + actual := tc.Condition.AppliesTo(tc.Truth) + + if tc.Expected != actual { + t.Errorf("Expected condition to apply? %t but was: %t", tc.Expected, actual) + } + }) + } +} + +const examplePolicy = ` +{ + "policies":[ + { + "//":"always applies", + "if":{}, + "then":{ + "cascade-true": true, + "cascade-false": true + + } + }, + { + "//":"some comment here", + "if":{ + "service_name": "cloud-storage" + }, + "then":{ + "cascade-false": false + } + } + ], + "assert":[ + { + "//":"cascading works correctly", + "if":{ + "service_name": "cloud-storage" + }, + "then":{ + "cascade-true": true, + "cascade-false": false + } + } + + + ], +} +` From 133e8158bb053efb76512fe1183c82229e5a7fa9 Mon Sep 17 00:00:00 2001 From: Joseph Lewis III Date: Thu, 10 Jan 2019 11:12:12 -0800 Subject: [PATCH 2/3] finished up policy tests --- pkg/broker/policy/policy.go | 120 ++++++++++--- pkg/broker/policy/policy_test.go | 283 ++++++++++++++++++++++++++----- utils/set.go | 9 + utils/set_test.go | 12 ++ 4 files changed, 365 insertions(+), 59 deletions(-) diff --git a/pkg/broker/policy/policy.go b/pkg/broker/policy/policy.go index e1e38f978..a0a71dd81 100644 --- a/pkg/broker/policy/policy.go +++ b/pkg/broker/policy/policy.go @@ -14,32 +14,72 @@ package policy +/* + Package policy defines a way to create simple cascading rule systems similar + to CSS. + + A policy is broken into two parts, conditions and declarations. Conditions + are the test that is run when it'd determined if a rule should fire. + Declarations are the values that are set by the rule. + + Rules are executed in a low to high precidence order, and values are merged + with the values from higher precidence rules overwriting values with the same + keys that were set earlier. + + Rules systems can be painfully difficult to debug and test. This is especially + true because they're often built as a safe way for non-programmers to modify + business process and don't have any way to programatically assert their logic + without resorting to hand-testing. + + To combat this issue, this rules system introduces three separate concepts. + + 1. Conditions are all a strict equality check. + 2. Rules are executed from top-to-bottom, eliminating the need for complex + state analysis and backtracking algorithms. + 3. There is a built-in system for assertion checking that's exposed to the + rule authors. +*/ + import ( + "bytes" + "encoding/json" "fmt" "reflect" + + "github.com/GoogleCloudPlatform/gcp-service-broker/pkg/validation" + "github.com/GoogleCloudPlatform/gcp-service-broker/utils" ) // Conditions are a set of values that can be compared with a base truth and // return true if all of the facets of the condition match. -// -// Blank facets are assumed to match everything. -type Condition struct { - ServiceId string `json:"service_id"` - ServiceName string `json:"service_name"` -} +type Condition map[string]string // AppliesTo returns true if all the facets of this condition match the given // truth. -func (cond *Condition) AppliesTo(truth Condition) bool { - if cond.ServiceId != "" && cond.ServiceId != truth.ServiceId { - return false +func (cond Condition) AppliesTo(truth Condition) bool { + for k, v := range cond { + truthValue, ok := truth[k] + if !ok || v != truthValue { + return false + } } - if cond.ServiceName != "" && cond.ServiceName != truth.ServiceName { - return false + return true +} + +// ValidateKeys ensures all of the keys of the condition exist in the set of +// allowed keys. +func (cond Condition) ValidateKeys(allowedKeys []string) error { + allowedSet := utils.NewStringSet(allowedKeys...) + condKeys := utils.NewStringSetFromStringMapKeys(cond) + + invalidKeys := condKeys.Minus(allowedSet) + + if invalidKeys.IsEmpty() { + return nil } - return true + return fmt.Errorf("unknown condition keys: %v condition keys must one of: %v, check their capitalization and spelling", invalidKeys, allowedKeys) } // Policy combines a condition with several sets of values that are set if @@ -48,25 +88,45 @@ type Policy struct { Comment string `json:"//"` Condition Condition `json:"if"` - Actions map[string]interface{} `json:"then"` + Declarations map[string]interface{} `json:"then"` } +// PolicyList contains the set of policies. type PolicyList struct { - Policies []Policy `json:"policy"` - Assertions []Policy `json:"assert"` + // Policies are ordered from least to greatest precidence. + Policies []Policy `json:"policy" validate:"dive"` + + // Assertions are used to validate the ordering of Policies. + Assertions []Policy `json:"assert" validate:"dive"` +} + +// Validate checks that the PolicyList struct is valid, that the keys for the +// conditions are valid and that all assertions hold. +func (pl *PolicyList) Validate(validConditionKeys []string) error { + if err := validation.ValidateStruct(pl); err != nil { + return fmt.Errorf("invalid PolicyList: %v", err) + } + + for i, pol := range pl.Policies { + if err := pol.Condition.ValidateKeys(validConditionKeys); err != nil { + return fmt.Errorf("error in policy[%d], comment: %q, error: %v", i, pol.Comment, err) + } + } + + return pl.CheckAssertions() } // CheckAssertions tests each assertion in the Assertions list against the -// policies list. The conditoin is used as the ground truth and the +// policies list. The condition is used as the ground truth and the // actions are used as the expected output. If the actions don't match then // an error is returned. func (pl *PolicyList) CheckAssertions() error { for i, assertion := range pl.Assertions { - expected := assertion.Actions + expected := assertion.Declarations actual := pl.Apply(assertion.Condition) if !reflect.DeepEqual(actual, expected) { - return fmt.Errorf("Error in assertion %d, comment: %q, expected: %v, actual: %v", i+1, assertion.Comment, expected, actual) + return fmt.Errorf("error in assertion[%d], comment: %q, expected: %v, actual: %v", i, assertion.Comment, expected, actual) } } @@ -84,10 +144,32 @@ func (pl *PolicyList) Apply(groundTruth Condition) map[string]interface{} { continue } - for k, v := range policy.Actions { + for k, v := range policy.Declarations { out[k] = v } } return out } + +// NewPolicyListFromJson creates a PolicyList from the given JSON version. +// It will fail on invalid condition names and failed assertions. +// +// Exactly one of PolicyList or error will be returned. +func NewPolicyListFromJson(value json.RawMessage, validConditionKeys []string) (*PolicyList, error) { + decoder := json.NewDecoder(bytes.NewBuffer(value)) + + // be noisy if the structure is invalid + decoder.DisallowUnknownFields() + + pl := &PolicyList{} + if err := decoder.Decode(pl); err != nil { + return nil, fmt.Errorf("couldn't decode PolicyList from JSON: %v", err) + } + + if err := pl.Validate(validConditionKeys); err != nil { + return nil, err + } + + return pl, nil +} diff --git a/pkg/broker/policy/policy_test.go b/pkg/broker/policy/policy_test.go index f3893e14f..8148c2e57 100644 --- a/pkg/broker/policy/policy_test.go +++ b/pkg/broker/policy/policy_test.go @@ -14,7 +14,11 @@ package policy -import "testing" +import ( + "errors" + "reflect" + "testing" +) func TestCondition_AppliesTo(t *testing.T) { cases := map[string]struct { @@ -22,11 +26,26 @@ func TestCondition_AppliesTo(t *testing.T) { Truth Condition Expected bool }{ - "matches everything": { + "blank-condition": { Condition: Condition{}, - Truth: Condition{}, + Truth: Condition{"service_id": "my-service-id", "service_name": "service-name"}, + Expected: true, + }, + "partial-condition": { + Condition: Condition{"service_id": "my-service-id"}, + Truth: Condition{"service_id": "my-service-id", "service_name": "service-name"}, Expected: true, }, + "mismatching-condition": { + Condition: Condition{"service_id": "abc"}, + Truth: Condition{"service_id": "my-service-id", "service_name": "service-name"}, + Expected: false, + }, + "key-not-in-truth": { + Condition: Condition{"service_id": ""}, + Truth: Condition{}, + Expected: false, + }, } for tn, tc := range cases { @@ -40,41 +59,225 @@ func TestCondition_AppliesTo(t *testing.T) { } } -const examplePolicy = ` -{ - "policies":[ - { - "//":"always applies", - "if":{}, - "then":{ - "cascade-true": true, - "cascade-false": true - - } - }, - { - "//":"some comment here", - "if":{ - "service_name": "cloud-storage" - }, - "then":{ - "cascade-false": false - } - } - ], - "assert":[ - { - "//":"cascading works correctly", - "if":{ - "service_name": "cloud-storage" - }, - "then":{ - "cascade-true": true, - "cascade-false": false - } - } - - - ], +func TestCondition_ValidateKeys(t *testing.T) { + cases := map[string]struct { + Condition Condition + AllowedKeys []string + Expected error + }{ + "blank-condition": { + Condition: Condition{}, + AllowedKeys: []string{"service_id"}, + Expected: nil, + }, + "good-condition": { + Condition: Condition{"service_id": "my-service-id"}, + AllowedKeys: []string{"service_id"}, + Expected: nil, + }, + "key-mismatch": { + Condition: Condition{"service_name": "abc"}, + AllowedKeys: []string{"service_id"}, + Expected: errors.New("unknown condition keys: [service_name] condition keys must one of: [service_id], check their capitalization and spelling"), + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + actual := tc.Condition.ValidateKeys(tc.AllowedKeys) + + if !reflect.DeepEqual(tc.Expected, actual) { + t.Errorf("Expected error: %v got %v", tc.Expected, actual) + } + }) + } +} + +func TestPolicyList_Validate(t *testing.T) { + cases := map[string]struct { + Policy PolicyList + AllowedKeys []string + Expected error + }{ + "blank-policy": { + Policy: PolicyList{}, + AllowedKeys: []string{"a", "b"}, + Expected: nil, + }, + "good-policy": { + Policy: PolicyList{ + Policies: []Policy{ + {Condition: Condition{"a": "a-value"}, Declarations: map[string]interface{}{"a-fired": true}}, + {Condition: Condition{"b": "b-value"}, Declarations: map[string]interface{}{"b-fired": true}}, + }, + Assertions: []Policy{ + {Condition: Condition{"a": "a-value", "b": "b-value"}, Declarations: map[string]interface{}{"a-fired": true, "b-fired": true}}, + }, + }, + AllowedKeys: []string{"a", "b"}, + Expected: nil, + }, + + "bad-keys": { + Policy: PolicyList{ + Policies: []Policy{ + {Condition: Condition{"unknown": "a-value"}, Comment: "some-user-comment"}, + }, + }, + AllowedKeys: []string{"a", "b"}, + Expected: errors.New(`error in policy[0], comment: "some-user-comment", error: unknown condition keys: [unknown] condition keys must one of: [a b], check their capitalization and spelling`), + }, + "bad-assertion": { + Policy: PolicyList{ + Policies: []Policy{ + {Condition: Condition{"a": "a-value"}, Declarations: map[string]interface{}{"out": false}}, + }, + + Assertions: []Policy{ + {Condition: Condition{"a": "a-value"}, Declarations: map[string]interface{}{"out": true}, Comment: "some-assertion"}, + }, + }, + AllowedKeys: []string{"a", "b"}, + Expected: errors.New(`error in assertion[0], comment: "some-assertion", expected: map[out:true], actual: map[out:false]`), + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + actual := tc.Policy.Validate(tc.AllowedKeys) + + if !reflect.DeepEqual(tc.Expected, actual) { + t.Errorf("Expected error: %v got %v", tc.Expected, actual) + } + }) + } +} + +func TestPolicyList_Apply(t *testing.T) { + cases := map[string]struct { + Policy PolicyList + Truth map[string]string + Expected map[string]interface{} + }{ + "cascading-overwrite": { + Policy: PolicyList{ + Policies: []Policy{ + {Condition: Condition{}, Declarations: map[string]interface{}{"last-fired": 0}}, + {Condition: Condition{}, Declarations: map[string]interface{}{"last-fired": 1}}, + }, + }, + Truth: map[string]string{}, + Expected: map[string]interface{}{ + "last-fired": 1, + }, + }, + "cascading-merge": { + Policy: PolicyList{ + Policies: []Policy{ + {Condition: Condition{}, Declarations: map[string]interface{}{"first": 0}}, + {Condition: Condition{}, Declarations: map[string]interface{}{"second": 1}}, + }, + }, + Truth: map[string]string{}, + Expected: map[string]interface{}{ + "first": 0, + "second": 1, + }, + }, + "no-conditions-match": { + Policy: PolicyList{ + Policies: []Policy{ + {Condition: Condition{"a": "true"}, Declarations: map[string]interface{}{"first": 0}}, + {Condition: Condition{"a": "true"}, Declarations: map[string]interface{}{"second": 1}}, + }, + }, + Truth: map[string]string{}, + Expected: map[string]interface{}{}, + }, + "partial-conditions-match": { + Policy: PolicyList{ + Policies: []Policy{ + {Condition: Condition{"a": "true"}, Declarations: map[string]interface{}{"last-fired": 0}}, + {Condition: Condition{"a": "false"}, Declarations: map[string]interface{}{"last-fired": 1}}, + }, + }, + Truth: map[string]string{"a": "true"}, + Expected: map[string]interface{}{ + "last-fired": 0, + }, + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + actual := tc.Policy.Apply(tc.Truth) + + if !reflect.DeepEqual(tc.Expected, actual) { + t.Errorf("Expected: %v got %v", tc.Expected, actual) + } + }) + } +} + +func TestNewPolicyListFromJson(t *testing.T) { + cases := map[string]struct { + Json string + AllowedKeys []string + Expected error + }{ + "invalid-json": { + Json: `invalid-json`, + AllowedKeys: []string{}, + Expected: errors.New("couldn't decode PolicyList from JSON: invalid character 'i' looking for beginning of value"), + }, + "unknown-field": { + Json: `{"unknown-field":[]}`, + AllowedKeys: []string{}, + Expected: errors.New(`couldn't decode PolicyList from JSON: json: unknown field "unknown-field"`), + }, + "bad-key": { + Json: `{"policy":[ + {"//":"user-comment", "if":{"unknown-condition":""}} + ]}`, + AllowedKeys: []string{}, + Expected: errors.New(`error in policy[0], comment: "user-comment", error: unknown condition keys: [unknown-condition] condition keys must one of: [], check their capitalization and spelling`), + }, + "bad-assertion": { + Json: `{ + "policy":[ + {"if":{}, "then":{"foo":"bar"}}, + {"if":{}, "then":{"foo":"bazz"}} + ], + "assert":[{"//":"check bad-value", "if":{}, "then":{"foo":"bad-value"}}] + }`, + AllowedKeys: []string{}, + Expected: errors.New(`error in assertion[0], comment: "check bad-value", expected: map[foo:bad-value], actual: map[foo:bazz]`), + }, + "good-fizzbuzz": { + Json: `{ + "policy": [ + {"if": {}, "then": {"print":"{{number}}"}}, + {"if": {"multiple-of-3":"true"}, "then": {"print":"fizz"}}, + {"if": {"multiple-of-5":"true"}, "then": {"print":"buzz"}}, + {"if": {"multiple-of-3":"true", "multiple-of-5":"true"}, "then": {"print":"fizzbuzz"}} + ], + "assert": [{"if":{"multiple-of-3":"true", "multiple-of-5":"true"}, "then":{"print":"fizzbuzz"}}] + }`, + AllowedKeys: []string{"multiple-of-3", "multiple-of-5"}, + Expected: nil, + }, + } + + for tn, tc := range cases { + t.Run(tn, func(t *testing.T) { + pl, err := NewPolicyListFromJson([]byte(tc.Json), tc.AllowedKeys) + if pl == nil && err == nil || pl != nil && err != nil { + t.Fatalf("Expected exactly one of PolicyList and err to be nil PolicyList: %v, Error: %v", pl, err) + } + + if !reflect.DeepEqual(err, tc.Expected) { + t.Errorf("Expected error: %v got: %v", tc.Expected, err) + } + }) + } } -` diff --git a/utils/set.go b/utils/set.go index 88af69671..e655ea1d0 100644 --- a/utils/set.go +++ b/utils/set.go @@ -27,6 +27,15 @@ func NewStringSet(contents ...string) StringSet { return set } +// NewStringSet creates a new string set with the given contents. +func NewStringSetFromStringMapKeys(contents map[string]string) StringSet { + set := StringSet{} + for k, _ := range contents { + set.Add(k) + } + return set +} + // StringSet is a set data structure for strings type StringSet map[string]bool diff --git a/utils/set_test.go b/utils/set_test.go index 099610409..17758a21d 100644 --- a/utils/set_test.go +++ b/utils/set_test.go @@ -41,6 +41,18 @@ func ExampleNewStringSet() { // Output: true } +func ExampleNewStringSetFromStringMapKeys() { + m := map[string]string{ + "a": "some a value", + "b": "some b value", + } + + set := NewStringSetFromStringMapKeys(m) + fmt.Println(set) + + // Output: [a b] +} + func ExampleStringSet_ToSlice() { a := NewStringSet() a.Add("z") From 469235a0044cfee71fb7da6c99f43c87d26dd73c Mon Sep 17 00:00:00 2001 From: Joseph Lewis III Date: Wed, 16 Jan 2019 08:16:50 -0800 Subject: [PATCH 3/3] must one of -> must be one of --- pkg/broker/policy/policy.go | 2 +- pkg/broker/policy/policy_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/broker/policy/policy.go b/pkg/broker/policy/policy.go index a0a71dd81..a982c427b 100644 --- a/pkg/broker/policy/policy.go +++ b/pkg/broker/policy/policy.go @@ -79,7 +79,7 @@ func (cond Condition) ValidateKeys(allowedKeys []string) error { return nil } - return fmt.Errorf("unknown condition keys: %v condition keys must one of: %v, check their capitalization and spelling", invalidKeys, allowedKeys) + return fmt.Errorf("unknown condition keys: %v condition keys must be one of: %v, check their capitalization and spelling", invalidKeys, allowedKeys) } // Policy combines a condition with several sets of values that are set if diff --git a/pkg/broker/policy/policy_test.go b/pkg/broker/policy/policy_test.go index 8148c2e57..baa27df62 100644 --- a/pkg/broker/policy/policy_test.go +++ b/pkg/broker/policy/policy_test.go @@ -78,7 +78,7 @@ func TestCondition_ValidateKeys(t *testing.T) { "key-mismatch": { Condition: Condition{"service_name": "abc"}, AllowedKeys: []string{"service_id"}, - Expected: errors.New("unknown condition keys: [service_name] condition keys must one of: [service_id], check their capitalization and spelling"), + Expected: errors.New("unknown condition keys: [service_name] condition keys must be one of: [service_id], check their capitalization and spelling"), }, } @@ -125,7 +125,7 @@ func TestPolicyList_Validate(t *testing.T) { }, }, AllowedKeys: []string{"a", "b"}, - Expected: errors.New(`error in policy[0], comment: "some-user-comment", error: unknown condition keys: [unknown] condition keys must one of: [a b], check their capitalization and spelling`), + Expected: errors.New(`error in policy[0], comment: "some-user-comment", error: unknown condition keys: [unknown] condition keys must be one of: [a b], check their capitalization and spelling`), }, "bad-assertion": { Policy: PolicyList{ @@ -240,7 +240,7 @@ func TestNewPolicyListFromJson(t *testing.T) { {"//":"user-comment", "if":{"unknown-condition":""}} ]}`, AllowedKeys: []string{}, - Expected: errors.New(`error in policy[0], comment: "user-comment", error: unknown condition keys: [unknown-condition] condition keys must one of: [], check their capitalization and spelling`), + Expected: errors.New(`error in policy[0], comment: "user-comment", error: unknown condition keys: [unknown-condition] condition keys must be one of: [], check their capitalization and spelling`), }, "bad-assertion": { Json: `{