Pathpol: Change policy in options (#2349)

Change policy in pathpol.Options to ExtPolicy. This is required so an option can not only have anonymous policies, but also extend others.
scionproto · Aug 29, 2019 · 7b76f7a · 7b76f7a
1 parent 40c4563
commit 7b76f7a
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 80 deletions.
diff --git a/doc/PathPolicy.md b/doc/PathPolicy.md
@@ -26,27 +26,6 @@ Examples:
 - Match outbound IF _2_ of AS _1-ff00:0:133_: `1-ff00:0:133#0,2`
 - Match inbound or outbound IF _2_ of AS _1-ff00:0:133_: `1-ff00:0:133#2`
 
-## Operators
-
-The path policy language has the following operators:
-
-ACL:
-
-- `+` (allow predicate)
-- `-` (deny predicate)
-
-Sequence:
-
-- `?` (the preceding HP may appear at most once)
-- `+` (the preceding **ISD-level** HP must appear at least once)
-- `*` (the preceding **ISD-level** HP may appear zero or more times)
-- `|` (logical OR)
-
-Planned:
-
-- `!` (logical NOT)
-- `&` (logical AND)
-
 ## Policy
 
 A policy is defined by a policy object. It can have the following attributes:
@@ -56,6 +35,7 @@ A policy is defined by a policy object. It can have the following attributes:
 - [`sequence`](#Sequence) (space separated list of HPs, may contain operators)
 - [`options`](#Options) (list of option policies)
     - `weight` (importance level, only valid under `options`)
+    - `policy` (a policy object)
 
 Planned:
 
@@ -74,6 +54,11 @@ Planned:
 
 ### ACL
 
+#### Operators
+
+- `+` (allow predicate)
+- `-` (deny predicate)
+
 The ACL can be used to deny (blacklist) or allow (whitelist) ISDs, ASes and IFs. A deny entry is of
 the following form `- ISD-AS#IF`, where the second part is a [HP](#HP). If a deny entry matches any
 hop on a path, the path is not allowed.
@@ -101,6 +86,18 @@ but denying all other ASes in ISD _1_. The last entry makes sure that any other
 
 ### Sequence
 
+#### Operators
+
+- `?` (the preceding HP may appear at most once)
+- `+` (the preceding **ISD-level** HP must appear at least once)
+- `*` (the preceding **ISD-level** HP may appear zero or more times)
+- `|` (logical OR)
+
+Planned:
+
+- `!` (logical NOT)
+- `&` (logical AND)
+
 The sequence is a string of space separated HPs. The [operators](#Operators) can be used for
 advanced interface sequences.
 
@@ -177,14 +174,19 @@ third option which denies only hops in AS _1-ff00:0:133_, is used.
 - policy_with_options:
     options:
       - weight: 3
-        extends: option_3
+        policy:
+          extends:
+          - option_3
       - weight: 2
-        acl:
-        - "- 1-ff00:0:130#0"
-        - "- 1-ff00:0:131#0"
-        - "- 1-ff00:0:132#0"
-        - "+"
-      - extends: option_1
+        policy:
+          acl:
+          - "- 1-ff00:0:130#0"
+          - "- 1-ff00:0:131#0"
+          - "- 1-ff00:0:132#0"
+          - "+"
+      - policy:
+          extends:
+          - option_1
 
 - option_3:
     acl:

diff --git a/go/lib/pathpol/BUILD.bazel b/go/lib/pathpol/BUILD.bazel
@@ -35,5 +35,6 @@ go_test(
         "//go/lib/xtest/graph:go_default_library",
         "@com_github_golang_mock//gomock:go_default_library",
         "@com_github_stretchr_testify//assert:go_default_library",
+        "@com_github_stretchr_testify//require:go_default_library",
     ],
 )
diff --git a/go/lib/pathpol/policy.go b/go/lib/pathpol/policy.go
@@ -138,6 +138,6 @@ func (p *Policy) evalOptions(inputSet PathSet) PathSet {
 
 // Option contains a weight and a policy and is used as a list item in Policy.Options
 type Option struct {
-	Weight int
-	Policy *Policy
+	Weight int        `json:"weight"`
+	Policy *ExtPolicy `json:"policy"`
 }
diff --git a/go/lib/pathpol/policy_test.go b/go/lib/pathpol/policy_test.go
@@ -16,12 +16,14 @@
 package pathpol
 
 import (
+	"encoding/json"
 	"fmt"
 	"strings"
 	"testing"
 
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/scionproto/scion/go/lib/addr"
 	"github.com/scionproto/scion/go/lib/common"
@@ -441,12 +443,13 @@ func TestOptionsEval(t *testing.T) {
 		"one option, allow everything": {
 			Policy: NewPolicy("", nil, nil, []Option{
 				{
-					Policy: &Policy{
-						ACL: &ACL{Entries: []*ACLEntry{
-							{
-								Action: Allow,
-								Rule:   mustHopPredicate(t, "0-0#0")},
-							denyEntry}}},
+					Policy: &ExtPolicy{
+						Policy: &Policy{
+							ACL: &ACL{Entries: []*ACLEntry{
+								{
+									Action: Allow,
+									Rule:   mustHopPredicate(t, "0-0#0")},
+								denyEntry}}}},
 					Weight: 0},
 			}),
 			Src:        xtest.MustParseIA("2-ff00:0:212"),
@@ -456,18 +459,20 @@ func TestOptionsEval(t *testing.T) {
 		"two options, deny everything": {
 			Policy: NewPolicy("", nil, nil, []Option{
 				{
-					Policy: &Policy{
-						ACL: &ACL{Entries: []*ACLEntry{{
-							Action: Allow,
-							Rule:   mustHopPredicate(t, "0-0#0")},
-							denyEntry}}},
+					Policy: &ExtPolicy{
+						Policy: &Policy{
+							ACL: &ACL{Entries: []*ACLEntry{{
+								Action: Allow,
+								Rule:   mustHopPredicate(t, "0-0#0")},
+								denyEntry}}}},
 					Weight: 0},
 				{
-					Policy: &Policy{
-						ACL: &ACL{Entries: []*ACLEntry{{
-							Action: Deny,
-							Rule:   mustHopPredicate(t, "0-0#0")},
-							denyEntry}}},
+					Policy: &ExtPolicy{
+						Policy: &Policy{
+							ACL: &ACL{Entries: []*ACLEntry{{
+								Action: Deny,
+								Rule:   mustHopPredicate(t, "0-0#0")},
+								denyEntry}}}},
 					Weight: 1},
 			}),
 			Src:        xtest.MustParseIA("2-ff00:0:212"),
@@ -476,15 +481,17 @@ func TestOptionsEval(t *testing.T) {
 		},
 		"two options, first: allow everything, second: allow one path": {
 			Policy: NewPolicy("", nil, nil, []Option{
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Allow, Rule: mustHopPredicate(t, "0-0#0")},
-					denyEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Allow, Rule: mustHopPredicate(t, "0-0#0")},
+						denyEntry}}}},
 					Weight: 0},
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:110#0")},
-					{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
-					{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:111#2823")},
-					allowEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:110#0")},
+						{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
+						{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:111#2823")},
+						allowEntry}}}},
 					Weight: 1},
 			}),
 			Src:        xtest.MustParseIA("1-ff00:0:122"),
@@ -493,13 +500,15 @@ func TestOptionsEval(t *testing.T) {
 		},
 		"two options, combined": {
 			Policy: NewPolicy("", nil, nil, []Option{
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
-					allowEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
+						allowEntry}}}},
 					Weight: 0},
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Deny, Rule: mustHopPredicate(t, "2-ff00:0:210#0")},
-					allowEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Deny, Rule: mustHopPredicate(t, "2-ff00:0:210#0")},
+						allowEntry}}}},
 					Weight: 0},
 			}),
 			Src:        xtest.MustParseIA("1-ff00:0:110"),
@@ -508,13 +517,15 @@ func TestOptionsEval(t *testing.T) {
 		},
 		"two options, take first": {
 			Policy: NewPolicy("", nil, nil, []Option{
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
-					allowEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
+						allowEntry}}}},
 					Weight: 1},
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Deny, Rule: mustHopPredicate(t, "2-ff00:0:210#0")},
-					allowEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Deny, Rule: mustHopPredicate(t, "2-ff00:0:210#0")},
+						allowEntry}}}},
 					Weight: 0},
 			}),
 			Src:        xtest.MustParseIA("1-ff00:0:110"),
@@ -523,13 +534,15 @@ func TestOptionsEval(t *testing.T) {
 		},
 		"two options, take second": {
 			Policy: NewPolicy("", nil, nil, []Option{
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
-					allowEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Deny, Rule: mustHopPredicate(t, "1-ff00:0:120#0")},
+						allowEntry}}}},
 					Weight: 1},
-				{Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
-					{Action: Deny, Rule: mustHopPredicate(t, "2-ff00:0:210#0")},
-					allowEntry}}},
+				{Policy: &ExtPolicy{
+					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{
+						{Action: Deny, Rule: mustHopPredicate(t, "2-ff00:0:210#0")},
+						allowEntry}}}},
 					Weight: 10},
 			}),
 			Src:        xtest.MustParseIA("1-ff00:0:110"),
@@ -580,20 +593,22 @@ func TestExtends(t *testing.T) {
 					Policy: &Policy{Name: "policy1", Options: []Option{
 						{
 							Weight: 1,
-							Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{{
-								Action: Allow,
-								Rule:   mustHopPredicate(t, "0-0#0")},
-								denyEntry}}},
+							Policy: &ExtPolicy{
+								Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{{
+									Action: Allow,
+									Rule:   mustHopPredicate(t, "0-0#0")},
+									denyEntry}}}},
 						},
 					}}},
 			},
 			ExtendedPolicy: &Policy{Options: []Option{
 				{
 					Weight: 1,
-					Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{{
-						Action: Allow,
-						Rule:   mustHopPredicate(t, "0-0#0")},
-						denyEntry}},
+					Policy: &ExtPolicy{
+						Policy: &Policy{ACL: &ACL{Entries: []*ACLEntry{{
+							Action: Allow,
+							Rule:   mustHopPredicate(t, "0-0#0")},
+							denyEntry}}},
 					},
 				},
 			},
@@ -824,3 +839,23 @@ func mustHopPredicate(t *testing.T, str string) *HopPredicate {
 	xtest.FailOnErr(t, err)
 	return hp
 }
+
+func TestPolicyJsonConversion(t *testing.T) {
+	policy := NewPolicy("", nil, nil, []Option{
+		{
+			Policy: &ExtPolicy{
+				Policy: &Policy{
+					ACL: &ACL{Entries: []*ACLEntry{
+						{
+							Action: Allow,
+							Rule:   mustHopPredicate(t, "0-0#0")},
+						denyEntry}}}},
+			Weight: 0},
+	})
+	jsonPol, err := json.Marshal(policy)
+	require.NoError(t, err)
+	var pol Policy
+	err = json.Unmarshal(jsonPol, &pol)
+	assert.NoError(t, err)
+	assert.Equal(t, policy, &pol)
+}