Skip to content

Commit

Permalink
fix: properly delete and update resources during UpdateMany() (#290)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kklimonda-cl
kklimonda-cl authored Jan 22, 2025
1 parent 0a4ccc0 commit e7fd331
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 53 deletions.
36 changes: 29 additions & 7 deletions assets/terraform/internal/manager/entry.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (o *EntryObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L
return found, true
}

for _, elt := range planEntries {
for idx, elt := range planEntries {
eltEntryName := elt.EntryName()
var processedEntry *entryObjectWithState[E]

Expand Down Expand Up @@ -257,11 +257,13 @@ func (o *EntryObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L
processedStateEntriesByName[processedEntry.Entry.EntryName()] = *processedEntry
} else {
processedEntry = &entryObjectWithState[E]{
Entry: elt,
State: entryMissing,
Entry: elt,
StateIdx: idx,
}

if !o.matcher(elt, stateElt.Entry) {
if o.matcher(elt, stateElt.Entry) {
processedEntry.State = entryOk
} else {
processedEntry.State = entryOutdated
}
processedStateEntriesByName[elt.EntryName()] = *processedEntry
Expand All @@ -273,6 +275,13 @@ func (o *EntryObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L
return nil, &Error{err: err, message: "failed to get a list of existing entries from the server"}
}

for name, elt := range stateEntriesByName {
if _, processedEntryFound := processedStateEntriesByName[name]; !processedEntryFound {
elt.State = entryDeleted
processedStateEntriesByName[name] = elt
}
}

updates := xmlapi.NewMultiConfig(len(planEntries))

for _, existingEntry := range existing {
Expand Down Expand Up @@ -353,6 +362,12 @@ func (o *EntryObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L
delete(processedStateEntriesByName, elt.Entry.EntryName())
elt.Entry.SetEntryName(elt.NewName)
processedStateEntriesByName[elt.NewName] = elt
case entryDeleted:
updates.Add(&xmlapi.Config{
Action: "delete",
Xpath: util.AsXpath(path),
Target: o.client.GetTarget(),
})
case entryUnknown:
slog.Warn("Entry state is still unknown after reconciliation", "Name", elt.Entry.EntryName())
case entryOk:
Expand All @@ -367,9 +382,16 @@ func (o *EntryObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L
}
}

entries := make([]E, len(processedStateEntriesByName))
for _, elt := range processedStateEntriesByName {
entries[elt.StateIdx] = elt.Entry
existing, err = o.service.List(ctx, location, "get", "", "")
if err != nil {
return nil, fmt.Errorf("Failed to list remote entries: %w", err)
}

entries := make([]E, len(planEntries))
for _, elt := range existing {
if planEntry, found := planEntriesByName[elt.EntryName()]; found {
entries[planEntry.StateIdx] = elt
}
}

return entries, nil
Expand Down
20 changes: 15 additions & 5 deletions assets/terraform/internal/manager/entry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,27 @@ var _ = Describe("Entry", func() {
Context("UpdateMany()", func() {
Context("when entries from the plan are missing from the server", func() {
It("should recreate them, and return back list of all managed entries", func() {
entries := []*MockEntryObject{{Name: "4", Value: "D"}}
processed, err := sdk.UpdateMany(ctx, location, entries, entries)
expected := append(existing, &MockEntryObject{Name: "4", Value: "D"})
processed, err := sdk.UpdateMany(ctx, location, existing, expected)

Expect(err).ToNot(HaveOccurred())
Expect(processed).To(HaveExactElements(entries))

expected := append(existing, entries...)
Expect(processed).To(HaveExactElements(expected))
Expect(client.list()).To(HaveExactElements(expected))
})

})

Context("when some entries are removed from the plan", func() {
It("should properly remove deleted entries from the server and return back updated list", func() {
stateEntries := []*MockEntryObject{{Name: "1", Value: "A"}, {Name: "2", Value: "B"}, {Name: "3", Value: "C"}}
planEntries := []*MockEntryObject{{Name: "1", Value: "A"}, {Name: "3", Value: "C"}}
processed, err := sdk.UpdateMany(ctx, location, stateEntries, planEntries)

Expect(err).ToNot(HaveOccurred())
Expect(processed).To(HaveExactElements(planEntries))
Expect(client.list()).To(HaveExactElements(planEntries))
})
})
})

Context("Delete()", func() {
Expand Down
119 changes: 78 additions & 41 deletions assets/terraform/test/resource_addresses_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,52 +55,89 @@ func TestAccAddresses(t *testing.T) {
statecheck.ExpectKnownValue(
"panos_addresses.addresses",
tfjsonpath.
New("addresses").
AtMapKey(fmt.Sprintf("%s-ip-netmask", prefix)).
AtMapKey("ip_netmask"),
knownvalue.StringExact("172.16.0.1/32"),
),
statecheck.ExpectKnownValue(
"panos_addresses.addresses",
tfjsonpath.
New("addresses").
AtMapKey(fmt.Sprintf("%s-ip-netmask", prefix)).
AtMapKey("tags"),
knownvalue.ListExact([]knownvalue.Check{
knownvalue.StringExact(fmt.Sprintf("%s-tag", prefix)),
New("addresses"),
knownvalue.ObjectExact(map[string]knownvalue.Check{
fmt.Sprintf("%s-ip-netmask", prefix): knownvalue.ObjectExact(map[string]knownvalue.Check{
"tags": knownvalue.ListExact([]knownvalue.Check{
knownvalue.StringExact(fmt.Sprintf("%s-tag", prefix)),
}),
"description": knownvalue.Null(),
"disable_override": knownvalue.StringExact("no"),
"ip_netmask": knownvalue.StringExact("172.16.0.1/32"),
"ip_range": knownvalue.Null(),
"ip_wildcard": knownvalue.Null(),
"fqdn": knownvalue.Null(),
}),
fmt.Sprintf("%s-ip-range", prefix): knownvalue.ObjectExact(map[string]knownvalue.Check{
"tags": knownvalue.Null(),
"description": knownvalue.StringExact("description"),
"disable_override": knownvalue.StringExact("no"),
"ip_netmask": knownvalue.Null(),
"ip_range": knownvalue.StringExact("172.16.0.1-172.16.0.255"),
"ip_wildcard": knownvalue.Null(),
"fqdn": knownvalue.Null(),
}),
fmt.Sprintf("%s-ip-wildcard", prefix): knownvalue.ObjectExact(map[string]knownvalue.Check{
"tags": knownvalue.Null(),
"description": knownvalue.Null(),
"disable_override": knownvalue.StringExact("no"),
"ip_netmask": knownvalue.Null(),
"ip_range": knownvalue.Null(),
"ip_wildcard": knownvalue.StringExact("172.16.0.0/0.0.0.255"),
"fqdn": knownvalue.Null(),
}),
fmt.Sprintf("%s-fqdn", prefix): knownvalue.ObjectExact(map[string]knownvalue.Check{
"tags": knownvalue.Null(),
"description": knownvalue.Null(),
"disable_override": knownvalue.StringExact("no"),
"ip_netmask": knownvalue.Null(),
"ip_range": knownvalue.Null(),
"ip_wildcard": knownvalue.Null(),
"fqdn": knownvalue.StringExact("example.com"),
}),
}),
),
},
},
{
Config: testAccAddressesResourceTmpl,
ConfigVariables: map[string]config.Variable{
"prefix": config.StringVariable(prefix),
"addresses": config.MapVariable(map[string]config.Variable{
fmt.Sprintf("%s-ip-range", prefix): config.ObjectVariable(map[string]config.Variable{
"description": config.StringVariable("description"),
"ip_range": config.StringVariable("172.16.0.1-172.16.0.255"),
}),
fmt.Sprintf("%s-fqdn", prefix): config.ObjectVariable(map[string]config.Variable{
"fqdn": config.StringVariable("example.com"),
}),
}),
},
ConfigStateChecks: []statecheck.StateCheck{
statecheck.ExpectKnownValue(
"panos_addresses.addresses",
tfjsonpath.
New("addresses").
AtMapKey(fmt.Sprintf("%s-ip-range", prefix)).
AtMapKey("description"),
knownvalue.StringExact("description"),
),
statecheck.ExpectKnownValue(
"panos_addresses.addresses",
tfjsonpath.
New("addresses").
AtMapKey(fmt.Sprintf("%s-ip-range", prefix)).
AtMapKey("ip_range"),
knownvalue.StringExact("172.16.0.1-172.16.0.255"),
),
statecheck.ExpectKnownValue(
"panos_addresses.addresses",
tfjsonpath.
New("addresses").
AtMapKey(fmt.Sprintf("%s-ip-wildcard", prefix)).
AtMapKey("ip_wildcard"),
knownvalue.StringExact("172.16.0.0/0.0.0.255"),
),
statecheck.ExpectKnownValue(
"panos_addresses.addresses",
tfjsonpath.
New("addresses").
AtMapKey(fmt.Sprintf("%s-fqdn", prefix)).
AtMapKey("fqdn"),
knownvalue.StringExact("example.com"),
New("addresses"),
knownvalue.ObjectExact(map[string]knownvalue.Check{
fmt.Sprintf("%s-ip-range", prefix): knownvalue.ObjectExact(map[string]knownvalue.Check{
"tags": knownvalue.Null(),
"description": knownvalue.StringExact("description"),
"disable_override": knownvalue.StringExact("no"),
"ip_netmask": knownvalue.Null(),
"ip_range": knownvalue.StringExact("172.16.0.1-172.16.0.255"),
"ip_wildcard": knownvalue.Null(),
"fqdn": knownvalue.Null(),
}),
fmt.Sprintf("%s-fqdn", prefix): knownvalue.ObjectExact(map[string]knownvalue.Check{
"tags": knownvalue.Null(),
"description": knownvalue.Null(),
"disable_override": knownvalue.StringExact("no"),
"ip_netmask": knownvalue.Null(),
"ip_range": knownvalue.Null(),
"ip_wildcard": knownvalue.Null(),
"fqdn": knownvalue.StringExact("example.com"),
}),
}),
),
},
},
Expand Down

0 comments on commit e7fd331

Please sign in to comment.