Skip to content

Commit 5632899

Browse files
pdziedzic-sumopdziedzic-sumo
authored
Merge pull request #645 from SumoLogic/pdziedzic-large-matchlist-items-test
INVS-2114: Fix match list update function
2 parents 165bcce + 4a8ea3a commit 5632899

File tree

3 files changed

+110
-24
lines changed

3 files changed

+110
-24
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
FEATURES:
33
* resource/sumologic_muting_schedule: Added support for Muting Schedule for an alert group (GH-601)
44

5+
BUG FIXES:
6+
* Speed up match list updates. (GH-645)
7+
58
## 2.29.0 (April 9, 2024)
69

710
FEATURES:

‎sumologic/resource_sumologic_cse_match_list.go‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -242,45 +242,52 @@ func resourceSumologicCSEMatchListUpdate(d *schema.ResourceData, meta interface{
242242
return fmt.Errorf("[ERROR] An error occurred updating match list with id %s, err: %v", d.Id(), err)
243243
}
244244

245+
var deleteItemIds []string
246+
var updateItems []CSEMatchListItemPost
247+
var addItems []CSEMatchListItemPost
248+
245249
itemsData := d.Get("items").(*schema.Set).List()
246-
var newItems []CSEMatchListItemPost
247-
var newItemIds []string
250+
itemsWithIDsMap := make(map[string]CSEMatchListItemPost)
248251
for _, data := range itemsData {
249252
item := resourceToCSEMatchListItem([]interface{}{data})
250-
newItems = append(newItems, item)
251-
newItemIds = append(newItemIds, item.ID)
253+
254+
if item.ID == "" {
255+
// empty ID means item is to be added
256+
addItems = append(addItems, item)
257+
} else {
258+
// item with and ID is already existing/potentially modified
259+
itemsWithIDsMap[item.ID] = item
260+
}
252261
}
253262

254263
CSEMatchListItems, err := c.GetCSEMatchListItemsInMatchList(d.Id())
255264
if err != nil {
256265
return fmt.Errorf("[ERROR] CSE Match List items not found when looking by match list id %s, err: %v", d.Id(), err)
257266
}
258267

259-
var deleteItemIds []string
260-
var updateItemIds []string
261-
262268
// Compare currently existing match list items with the new items to determine if they should be deleted or updated
269+
oldItemsMap := make(map[string]CSEMatchListItemGet)
263270
for _, item := range CSEMatchListItems.CSEMatchListItemsGetObjects {
264-
var oldItemId = item.ID
265-
if contains(newItemIds, oldItemId) {
266-
updateItemIds = append(updateItemIds, oldItemId)
267-
} else {
268-
deleteItemIds = append(deleteItemIds, oldItemId)
269-
}
271+
oldItemsMap[item.ID] = item
270272
}
271273

272-
var updateItems []CSEMatchListItemPost
273-
var addItems []CSEMatchListItemPost
274-
275-
// Any new items that are not updates to existing items should be added instead
276-
for _, newItem := range newItems {
277-
if contains(updateItemIds, newItem.ID) {
278-
updateItems = append(updateItems, newItem)
274+
for oldItemId, oldItem := range oldItemsMap {
275+
newItem, ok := itemsWithIDsMap[oldItemId]
276+
if ok {
277+
if matches(oldItem, newItem) {
278+
// skip (unchanged)
279+
} else {
280+
// same ID, changed contents
281+
updateItems = append(updateItems, newItem)
282+
}
279283
} else {
280-
addItems = append(addItems, newItem)
284+
// match list item doesn't exist any more
285+
deleteItemIds = append(deleteItemIds, oldItemId)
281286
}
282287
}
283288

289+
log.Printf("[DEBUG] Match List update items - to add: %d, to update: %d, to delete: %d", len(addItems), len(updateItems), len(deleteItemIds))
290+
284291
// Delete old items
285292
for _, oldItem := range CSEMatchListItems.CSEMatchListItemsGetObjects {
286293
if contains(deleteItemIds, oldItem.ID) {
@@ -310,14 +317,15 @@ func resourceSumologicCSEMatchListUpdate(d *schema.ResourceData, meta interface{
310317
// Wait for update to finish
311318
updateStateConf := &resource.StateChangeConf{
312319
Target: []string{
313-
fmt.Sprint(len(newItems)),
320+
fmt.Sprint(len(itemsData)),
314321
},
315322
Refresh: func() (interface{}, string, error) {
316323
resp, err := c.GetCSEMatchListItemsInMatchList(d.Id())
317324
if err != nil {
318325
log.Printf("[ERROR] CSE Match List items not found when looking by match list id %s, err: %v", d.Id(), err)
319326
return 0, "", err
320327
}
328+
log.Printf("[DEBUG] Match List update awaiting '%s', got '%s'", fmt.Sprint(len(itemsData)), fmt.Sprint(resp.Total))
321329
return resp, fmt.Sprint(resp.Total), nil
322330
},
323331
Timeout: d.Timeout(schema.TimeoutUpdate),
@@ -334,6 +342,14 @@ func resourceSumologicCSEMatchListUpdate(d *schema.ResourceData, meta interface{
334342
return resourceSumologicCSEMatchListRead(d, meta)
335343
}
336344

345+
func matches(oldItem CSEMatchListItemGet, newItem CSEMatchListItemPost) bool {
346+
return oldItem.ID == newItem.ID &&
347+
oldItem.Expiration == newItem.Expiration &&
348+
oldItem.Active == newItem.Active &&
349+
oldItem.Value == newItem.Value &&
350+
oldItem.Meta.Description == newItem.Description
351+
}
352+
337353
func contains(slice []string, item string) bool {
338354
set := make(map[string]struct{}, len(slice))
339355
for _, s := range slice {

‎sumologic/resource_sumologic_cse_match_list_test.go‎

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ func TestAccSumologicSCEMatchList_createAndUpdate(t *testing.T) {
1717
resourceName := "sumologic_cse_match_list.match_list"
1818

1919
// Create values
20-
nName := fmt.Sprintf("Terraform Test Match List %s", uuid.New())
20+
nName := fmt.Sprintf("Terraform Test Match List Loop %s", uuid.New())
2121
nDefaultTtl := 10800
2222
nDescription := "Match List Description"
2323
nTargetColumn := "SrcIp"
@@ -49,7 +49,7 @@ func TestAccSumologicSCEMatchList_createAndUpdate(t *testing.T) {
4949
resource.TestCheckResourceAttrSet(resourceName, "id"),
5050
),
5151
},
52-
// Updates the match list and its 2 match list items
52+
// Updates the match list description only
5353
{
5454
Config: testCreateCSEMatchListConfig(uDefaultTtl, uDescription, nName, nTargetColumn, liDescription, liExpiration, liValue, liCount),
5555
Check: resource.ComposeTestCheckFunc(
@@ -79,6 +79,73 @@ func TestAccSumologicSCEMatchList_createAndUpdate(t *testing.T) {
7979
})
8080
}
8181

82+
func TestAccSumologicSCEMatchList_createAddRemoveItems(t *testing.T) {
83+
SkipCseTest(t)
84+
85+
var matchList CSEMatchListGet
86+
resourceName := "sumologic_cse_match_list.match_list"
87+
88+
// Create values
89+
nName := fmt.Sprintf("Terraform Test Match List %s", uuid.New())
90+
nDefaultTtl := 10800
91+
nDescription := "Match List Description"
92+
nTargetColumn := "SrcIp"
93+
liDescription := "Match List Item Description"
94+
liExpiration := "2122-02-27T04:00:00"
95+
liValue := "value"
96+
liCount := 3
97+
98+
unDescription := "Updated Match List Description"
99+
uliDescription := "Updated Match List Item Description"
100+
101+
resource.Test(t, resource.TestCase{
102+
PreCheck: func() { testAccPreCheck(t) },
103+
Providers: testAccProviders,
104+
CheckDestroy: testAccCSEMatchListDestroy,
105+
Steps: []resource.TestStep{
106+
// Creates a match list with 3 match list items
107+
{
108+
Config: testCreateCSEMatchListConfig(nDefaultTtl, nDescription, nName, nTargetColumn, liDescription, liExpiration, liValue, liCount),
109+
Check: resource.ComposeTestCheckFunc(
110+
testCheckCSEMatchListExists(resourceName, &matchList),
111+
testCheckMatchListValues(&matchList, nDefaultTtl, nDescription, nName, nTargetColumn),
112+
testCheckMatchListItemsValuesAndCount(resourceName, liDescription, liExpiration, liValue, liCount),
113+
resource.TestCheckResourceAttrSet(resourceName, "id"),
114+
),
115+
},
116+
{ // change match list description only
117+
Config: testCreateCSEMatchListConfig(nDefaultTtl, unDescription, nName, nTargetColumn, liDescription, liExpiration, liValue, liCount),
118+
Check: resource.ComposeTestCheckFunc(
119+
testCheckCSEMatchListExists(resourceName, &matchList),
120+
testCheckMatchListValues(&matchList, nDefaultTtl, unDescription, nName, nTargetColumn),
121+
testCheckMatchListItemsValuesAndCount(resourceName, liDescription, liExpiration, liValue, liCount),
122+
resource.TestCheckResourceAttrSet(resourceName, "id"),
123+
),
124+
},
125+
// Add two match list items
126+
{
127+
Config: testCreateCSEMatchListConfig(nDefaultTtl, unDescription, nName, nTargetColumn, liDescription, liExpiration, liValue, liCount+2),
128+
Check: resource.ComposeTestCheckFunc(
129+
testCheckCSEMatchListExists(resourceName, &matchList),
130+
testCheckMatchListValues(&matchList, nDefaultTtl, unDescription, nName, nTargetColumn),
131+
testCheckMatchListItemsValuesAndCount(resourceName, liDescription, liExpiration, liValue, liCount+2),
132+
resource.TestCheckResourceAttrSet(resourceName, "id"),
133+
),
134+
},
135+
// Remove couple items, change value in remaining two
136+
{
137+
Config: testCreateCSEMatchListConfig(nDefaultTtl, nDescription, nName, nTargetColumn, uliDescription, liExpiration, liValue, liCount-1),
138+
Check: resource.ComposeTestCheckFunc(
139+
testCheckCSEMatchListExists(resourceName, &matchList),
140+
testCheckMatchListValues(&matchList, nDefaultTtl, nDescription, nName, nTargetColumn),
141+
testCheckMatchListItemsValuesAndCount(resourceName, uliDescription, liExpiration, liValue, liCount-1),
142+
resource.TestCheckResourceAttrSet(resourceName, "id"),
143+
),
144+
},
145+
},
146+
})
147+
}
148+
82149
func testAccCSEMatchListDestroy(s *terraform.State) error {
83150
client := testAccProvider.Meta().(*Client)
84151

0 commit comments

Comments
 (0)