From dd730cf471c4797cb55640c783e9bb110d113503 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Rigault?= Date: Fri, 31 Mar 2023 09:25:25 +0200 Subject: [PATCH] adding loose ordering of rules 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. https://github.com/PaloAltoNetworks/terraform-provider-panos/issues/378 --- client.go | 4 ++-- namespace/standard.go | 2 +- poli/security/fw.go | 22 +++++++++++++++++++--- poli/security/pano.go | 22 +++++++++++++++++++--- util/const.go | 1 + util/util.go | 2 +- 6 files changed, 43 insertions(+), 10 deletions(-) diff --git a/client.go b/client.go index 6a200d3a..a60e4113 100644 --- a/client.go +++ b/client.go @@ -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") @@ -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) diff --git a/namespace/standard.go b/namespace/standard.go index 8cddba83..0cb4ba8b 100644 --- a/namespace/standard.go +++ b/namespace/standard.go @@ -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 { diff --git a/poli/security/fw.go b/poli/security/fw.go index 64d0981b..f7732f23 100644 --- a/poli/security/fw.go +++ b/poli/security/fw.go @@ -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. diff --git a/poli/security/pano.go b/poli/security/pano.go index 1424bb39..8af5302c 100644 --- a/poli/security/pano.go +++ b/poli/security/pano.go @@ -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. diff --git a/util/const.go b/util/const.go index c5a6a729..6e520821 100644 --- a/util/const.go +++ b/util/const.go @@ -25,6 +25,7 @@ const ( MoveDirectlyAfter MoveTop MoveBottom + MoveLoose ) // Valid values to use for any function expecting a pango query type `qt`. diff --git a/util/util.go b/util/util.go index 2a21a3e3..b8291015 100644 --- a/util/util.go +++ b/util/util.go @@ -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 }