Skip to content

Commit

Permalink
policy: create parent node if doesn't exist
Browse files Browse the repository at this point in the history
When a user add a policy where the parent node didn't exist, the policy
insertion would fail. Now the user will be able to add a policy node and
its parent node will be automatically created.

Signed-off-by: André Martins <andre@cilium.io>
  • Loading branch information
aanm committed Mar 22, 2017
1 parent 978a2e8 commit 65188cf
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 26 deletions.
10 changes: 5 additions & 5 deletions pkg/policy/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,12 @@ func (n *Node) UnmarshalJSON(data []byte) error {

// CanMerge returns an error if obj cannot be safely merged into an existing node
func (n *Node) CanMerge(obj *Node) error {
if obj.Name != n.Name {
return fmt.Errorf("node name mismatch %s != %s", obj.Name, n.Name)
if n.Name != obj.Name {
return fmt.Errorf("node name mismatch %q != %q", n.Name, obj.Name)
}

if obj.path != n.path {
return fmt.Errorf("node path mismatch %s != %s", obj.path, n.path)
if obj.path != "" && n.path != obj.path {
return fmt.Errorf("node path mismatch %q != %q", n.path, obj.path)
}

if !n.IsMergeable() || !obj.IsMergeable() {
Expand All @@ -426,7 +426,7 @@ func (n *Node) CanMerge(obj *Node) error {
return nil
}

// Merge incorporates the rules and children of obj into an existnig node
// Merge incorporates the rules and children of obj into an existing node
func (n *Node) Merge(obj *Node) (bool, error) {
if err := n.CanMerge(obj); err != nil {
return false, fmt.Errorf("cannot merge node: %s", err)
Expand Down
31 changes: 23 additions & 8 deletions pkg/policy/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,24 +173,30 @@ func (t *Tree) Lookup(path string) (node, parent *Node) {
return node, parent
}

// Add adds the provided policy node at the specified path
func (t *Tree) Add(path string, node *Node) (bool, error) {
var err error

// Add adds the provided policy node at the specified path.
func (t *Tree) Add(parentPath string, node *Node) (bool, error) {
if node == nil {
return false, nil
}

t.Mutex.Lock()
defer t.Mutex.Unlock()

if path, err = node.NormalizeNames(path); err != nil {
return t.add(parentPath, node)
}

// add adds the provided policy node at the specified path. Must be called with
// t.Mutex locked.
func (t *Tree) add(parentPath string, node *Node) (bool, error) {
var err error

if parentPath, err = node.NormalizeNames(parentPath); err != nil {
return false, err
}

modified := false

if path == RootNodeName && node.Name == RootNodeName {
if parentPath == RootNodeName && node.Name == RootNodeName {
node.Path()
if t.Root == nil {
t.Root = node
Expand All @@ -199,12 +205,21 @@ func (t *Tree) Add(path string, node *Node) (bool, error) {
if modified, err = t.Root.Merge(node); err != nil {
return false, err
}
// If modified we need to resolve the tree's root
if modified {
return true, t.Root.ResolveTree()
}
}
} else {
absPath := JoinPath(path, node.Name)
absPath := JoinPath(parentPath, node.Name)
_, parent := t.Lookup(absPath)
if parent == nil {
return false, fmt.Errorf("path '%s' does not point to a valid node", path)
grandParentPath, parentName := SplitNodePath(parentPath)
parent = NewNode(parentName, nil)
_, err := t.add(grandParentPath, parent)
if err != nil {
return false, fmt.Errorf("unable to add parent's node for path '%s': %s", parentPath, err)
}
}

if modified, err = parent.AddChild(node.Name, node); err != nil {
Expand Down
32 changes: 19 additions & 13 deletions pkg/policy/tree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,21 @@ func (ds *PolicyTestSuite) TestAddDelete(c *C) {
c.Assert(n, IsNil)
c.Assert(p, IsNil)

// Added a child if no root node exist must fail
added, err = tree.Add(RootNodeName, &Node{Name: "foo"})
c.Assert(added, Equals, false)
c.Assert(err, Not(IsNil))
// Added a child if no root node exist must add parents of that node
foo := &Node{Name: "foo"}
added, err = tree.Add(RootNodeName+".bar", foo)
c.Assert(added, Equals, true)
c.Assert(err, IsNil)

// The node should not exist afterwards
n, p = tree.Lookup("root.foo")
c.Assert(n, IsNil)
c.Assert(p, IsNil)
// The node should exist afterwards
n, pBar := tree.Lookup(RootNodeName + ".bar.foo")
c.Assert(n, Equals, foo)
c.Assert(pBar.Name, Equals, "bar")
c.Assert(pBar.path, Equals, RootNodeName+".bar")

// No root node should have been added
// The root node should have been added
n, p = tree.Lookup(RootNodeName)
c.Assert(n, IsNil)
c.Assert(n, Not(IsNil))
c.Assert(p, IsNil)

fooNode := Node{}
Expand All @@ -77,21 +79,25 @@ func (ds *PolicyTestSuite) TestAddDelete(c *C) {
},
}

// Add root ndoe with children, should succeed
// Add root node with children, should succeed
added, err = tree.Add(RootNodeName, &root)
c.Assert(added, Equals, true)
c.Assert(err, IsNil)

// lookup of root node should succeed now
n, p = tree.Lookup(RootNodeName)
c.Assert(n, Equals, &root)
// The "root" node was merged into the tree's root, therefore we need to
// make it the same with this hack
root.Children["bar"] = pBar
root.resolved = true
c.Assert(n, DeepEquals, &root)
c.Assert(n.Name, Equals, RootNodeName)
c.Assert(p, IsNil)

// lookup of child foo should succeed
n, p = tree.Lookup("root.foo")
c.Assert(n, Equals, &fooNode)
c.Assert(p, Equals, &root)
c.Assert(p, DeepEquals, &root)

// delete root node
deleted = tree.Delete("root", "")
Expand Down

0 comments on commit 65188cf

Please sign in to comment.