Skip to content

Commit

Permalink
adding loose ordering of rules
Browse files Browse the repository at this point in the history
when updating a large security rule group, one action=move API call is
made for each rule of the security rule group, the first rule is placed
according to the group position, then each succeeding rule is moved
after the previous one.

We introduce a **loose** ordering of rules where we only order newly
created rules and put them below any other rule of the security group.
This considerably reduces the amount of API calls needed to update
rules.

PaloAltoNetworks/terraform-provider-panos#378
  • Loading branch information
freedge committed Apr 3, 2023
1 parent dba4c15 commit dd730cf
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 10 deletions.
4 changes: 2 additions & 2 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,7 @@ func (c *Client) PositionFirstEntity(mvt int, rel, ent string, path, elms []stri
// Sanity checks.
if rel == ent {
return fmt.Errorf("Can't position %q in relation to itself", rel)
} else if mvt < util.MoveSkip && mvt > util.MoveBottom {
} else if mvt < util.MoveSkip && mvt > util.MoveLoose {
return fmt.Errorf("Invalid position int given: %d", mvt)
} else if (mvt == util.MoveBefore || mvt == util.MoveDirectlyBefore || mvt == util.MoveAfter || mvt == util.MoveDirectlyAfter) && rel == "" {
return fmt.Errorf("Specify 'ref' in order to perform relative group positioning")
Expand All @@ -1699,7 +1699,7 @@ func (c *Client) PositionFirstEntity(mvt int, rel, ent string, path, elms []stri
oIdx := -1

switch mvt {
case util.MoveSkip:
case util.MoveSkip, util.MoveLoose:
return nil
case util.MoveTop:
_, em := c.Move(path, "top", "", nil, nil)
Expand Down
2 changes: 1 addition & 1 deletion namespace/standard.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ func (n *Standard) MoveGroup(pather Pather, lister MoveLister, movement int, rul
return pErr
}

if movement != util.MoveSkip {
if movement != util.MoveSkip && movement != util.MoveLoose {
// Get the list of current security policies.
curList, err := lister()
if err != nil {
Expand Down
22 changes: 19 additions & 3 deletions poli/security/fw.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,25 @@ func (c *Firewall) ConfigureRules(vsys string, rules []Entry, auditComments map[
}
}

// Move the group into place.
if err = c.MoveGroup(vsys, move, oRule, rules...); err != nil {
return err
if move == util.MoveLoose && len(curRules) > 0 {
if len(setRules) > 0 {
// we just move the newly created rules under one of the other rules
// (possibly one we are removing, but we should not really care).
// Goal is to have rules of the security rule group ordered one after another
// but we don't really care about the specific order.
reorderedRules := make([]Entry, 0, len(setRules)+1)
reorderedRules = append(reorderedRules, curRules[0])
reorderedRules = append(reorderedRules, setRules...)

if err = c.MoveGroup(vsys, move, oRule, reorderedRules...); err != nil {
return err
}
}
} else {
// Move the group into place.
if err = c.MoveGroup(vsys, move, oRule, rules...); err != nil {
return err
}
}

// Delete rules removed from the group.
Expand Down
22 changes: 19 additions & 3 deletions poli/security/pano.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,25 @@ func (c *Panorama) ConfigureRules(dg, base string, rules []Entry, auditComments
}
}

// Move the group into place.
if err = c.MoveGroup(dg, base, move, oRule, rules...); err != nil {
return err
if move == util.MoveLoose && len(curRules) > 0 {
if len(setRules) > 0 {
// we just move the newly created rules under one of the other rules
// (possibly one we are removing, but we should not really care).
// Goal is to have rules of the security rule group ordered one after another
// but we don't really care about the specific order.
reorderedRules := make([]Entry, 0, len(setRules)+1)
reorderedRules = append(reorderedRules, curRules[0])
reorderedRules = append(reorderedRules, setRules...)

if err = c.MoveGroup(dg, base, move, oRule, reorderedRules...); err != nil {
return err
}
}
} else {
// Move the group into place.
if err = c.MoveGroup(dg, base, move, oRule, rules...); err != nil {
return err
}
}

// Delete rules removed from the group.
Expand Down
1 change: 1 addition & 0 deletions util/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const (
MoveDirectlyAfter
MoveTop
MoveBottom
MoveLoose
)

// Valid values to use for any function expecting a pango query type `qt`.
Expand Down
2 changes: 1 addition & 1 deletion util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func CleanRawXml(v string) string {
// ValidMovement returns if the movement constant is valid or not.
func ValidMovement(v int) bool {
switch v {
case MoveSkip, MoveBefore, MoveDirectlyBefore, MoveAfter, MoveDirectlyAfter, MoveTop, MoveBottom:
case MoveSkip, MoveBefore, MoveDirectlyBefore, MoveAfter, MoveDirectlyAfter, MoveTop, MoveBottom, MoveLoose:
return true
}

Expand Down

0 comments on commit dd730cf

Please sign in to comment.