Skip to content

Commit dc03001

Browse files
kklimonda-clkklimonda-cl
committed
fix(terraform): properly handle object renaming
1 parent ab8055c commit dc03001

File tree

8 files changed

+475
-17
lines changed

8 files changed

+475
-17
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ test/terraform: test/terraform-acc test/terraform-manager
5050

5151
.PHONY: test/terraform-manager
5252
test/terraform-manager: codegen assets
53-
cd $(GENERATED_OUT_PATH)/terraform-provider-panos/ && \
53+
cd $(GENERATED_OUT_PATH)/terraform/ && \
5454
go test -v ./internal/manager/
5555

5656
.PHONY: test/terraform-acc

‎assets/pango/location/types.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
type System struct{}
2+
type Ngfw struct{}
3+
4+
type Shared struct {
5+
Rulebase *string `json:"rulebase,omitempty"`
6+
}
7+
8+
type Vsys struct {
9+
NgfwDevice string `json:"ngfw_device"`
10+
Vsys string `json:"vsys"`
11+
}
12+
13+
type Template struct {
14+
NgwfwDevice string `json:"ngfw_device"`
15+
PanoramaDevice string `json:"panorama_device"`
16+
Name string `json:"template"`
17+
}
18+
19+
type TemplateStack struct {
20+
NgwfwDevice string `json:"ngfw_device"`
21+
PanoramaDevice string `json:"panorama_device"`
22+
Name string `json:"template_stack"`
23+
}
24+
25+
type DeviceGroup struct {
26+
Name string `json:"device_group"`
27+
PanoramaDevice string `json:"panorama_device"`
28+
Rulebase *string `json:"rulebase,omitempty"`
29+
}
30+
31+
func NewDeviceGroupLocation(spec DeviceGroupSpec) *DeviceGroup {
32+
return &Devicegroup{}
33+
}

‎assets/terraform/internal/manager/entry.go‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -253,22 +253,50 @@ func (o *EntryObjectManager[E, T, S]) Delete(ctx context.Context, location T, pa
253253
return nil
254254
}
255255

256-
func (o *EntryObjectManager[E, L, S]) Update(ctx context.Context, location L, components []string, entry E, name string) (E, error) {
257-
xpath, err := location.XpathWithComponents(o.client.Versioning(), append(components, util.AsEntryXpath(name))...)
256+
func (o *EntryObjectManager[E, L, S]) Update(ctx context.Context, location L, components []string, entry E, newName string) (E, error) {
257+
updates := xmlapi.NewMultiConfig(2)
258+
259+
var xpath, renamedXpath []string
260+
var err error
261+
262+
spec, err := o.specifier(entry)
258263
if err != nil {
259264
return *new(E), err
260265
}
261266

262-
err = o.service.UpdateWithXpath(ctx, util.AsXpath(xpath), entry, name)
263-
if err != nil {
264-
if sdkerrors.IsObjectNotFound(err) {
265-
return *new(E), ErrObjectNotFound
266-
} else {
267-
return *new(E), &Error{err: err, message: "sdk error while updating"}
267+
xpath, err = location.XpathWithComponents(o.client.Versioning(), append(components, util.AsEntryXpath(entry.EntryName()))...)
268+
updates.Add(&xmlapi.Config{
269+
Action: "edit",
270+
Xpath: util.AsXpath(xpath),
271+
Element: spec,
272+
Target: o.client.GetTarget(),
273+
})
274+
275+
if newName != "" {
276+
renamedXpath, err = location.XpathWithComponents(o.client.Versioning(), append(components, util.AsEntryXpath(newName))...)
277+
_, err = o.service.ReadWithXpath(ctx, util.AsXpath(renamedXpath), "get")
278+
if err == nil {
279+
return *new(E), &Error{err: ErrConflict, message: fmt.Sprintf("entry '%s' already exists", newName)}
268280
}
281+
282+
updates.Add(&xmlapi.Config{
283+
Action: "rename",
284+
Xpath: util.AsXpath(xpath),
285+
NewName: newName,
286+
Target: o.client.GetTarget(),
287+
})
288+
}
289+
290+
_, _, _, err = o.client.MultiConfig(ctx, updates, false, nil)
291+
if err != nil {
292+
return *new(E), err
269293
}
270294

271-
return o.service.ReadWithXpath(ctx, util.AsXpath(xpath), "get")
295+
if renamedXpath != nil {
296+
return o.service.ReadWithXpath(ctx, util.AsXpath(renamedXpath), "get")
297+
} else {
298+
return o.service.ReadWithXpath(ctx, util.AsXpath(xpath), "get")
299+
}
272300
}
273301

274302
func (o *EntryObjectManager[E, L, S]) UpdateMany(ctx context.Context, location L, components []string, stateEntries []E, planEntries []E) ([]E, error) {

‎assets/terraform/internal/manager/entry_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,50 @@ var _ = Describe("Entry", func() {
177177
})
178178

179179
Context("Update()", func() {
180+
BeforeEach(func() {
181+
existing = []*MockEntryObject{{Name: "entry-to-rename", Value: "original"}}
182+
})
183+
Context("when just updating an entry", func() {
184+
It("should update entry on the server without executing 'rename' command", func() {
185+
updatedContent := &MockEntryObject{Name: "entry-to-rename", Value: "updated"}
186+
updatedEntry, err := sdk.Update(ctx, location, []string{}, updatedContent, "")
187+
188+
Expect(err).ToNot(HaveOccurred())
189+
190+
Expect(updatedEntry).ToNot(BeNil())
191+
Expect(updatedEntry.EntryName()).To(Equal("entry-to-rename"))
192+
Expect(updatedEntry.Value).To(Equal("updated"))
193+
194+
Expect(client.MultiConfigOpers[0]).To(HaveExactElements([]MultiConfigOper{
195+
{Operation: MultiConfigOperEdit, EntryName: "entry-to-rename"},
196+
}))
197+
198+
expectedAfterUpdate := []*MockEntryObject{{Name: "entry-to-rename", Value: "updated"}}
199+
Expect(client.list()).To(MatchEntries(expectedAfterUpdate))
200+
})
201+
})
202+
Context("when renaming an entry", func() {
180203

204+
It("should rename the entry on the server", func() {
205+
updatedContent := &MockEntryObject{Name: "entry-to-rename", Value: "updated"}
206+
newName := "renamed-entry"
207+
updatedEntry, err := sdk.Update(ctx, location, []string{}, updatedContent, newName)
208+
209+
Expect(err).ToNot(HaveOccurred())
210+
211+
Expect(updatedEntry).ToNot(BeNil())
212+
Expect(updatedEntry.EntryName()).To(Equal(newName))
213+
Expect(updatedEntry.Value).To(Equal("updated"))
214+
215+
Expect(client.MultiConfigOpers[0]).To(HaveExactElements([]MultiConfigOper{
216+
{Operation: MultiConfigOperEdit, EntryName: "entry-to-rename"},
217+
{Operation: MultiConfigOperRename, EntryName: "entry-to-rename", NewName: "renamed-entry"},
218+
}))
219+
220+
expectedAfterUpdate := []*MockEntryObject{{Name: newName, Value: "updated"}}
221+
Expect(client.list()).To(MatchEntries(expectedAfterUpdate))
222+
})
223+
})
181224
})
182225

183226
Context("UpdateMany()", func() {

‎assets/terraform/internal/manager/entry_utils_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ func (o *MockEntryObject) DeepCopy() any {
6565
type MockEntryClient[E manager.UuidObject] struct {
6666
Initial *list.List
6767
Current *list.List
68-
MultiConfigOpers []MultiConfigOper
68+
MultiConfigOpers [][]MultiConfigOper
6969
}
7070

7171
func NewMockEntryClient[E manager.UuidObject](initial []E) *MockEntryClient[E] {
@@ -110,7 +110,8 @@ func (o *MockEntryClient[E]) ChunkedMultiConfig(ctx context.Context, updates *xm
110110
}
111111

112112
func (o *MockEntryClient[E]) MultiConfig(ctx context.Context, updates *xmlapi.MultiConfig, arg1 bool, arg2 url.Values) ([]byte, *http.Response, *xmlapi.MultiConfigResponse, error) {
113-
o.MultiConfigOpers, _ = MultiConfig[E](updates, &o.Current, multiConfigEntry, 0)
113+
opers, _ := MultiConfig[E](updates, &o.Current, multiConfigEntry, 0)
114+
o.MultiConfigOpers = append(o.MultiConfigOpers, opers)
114115

115116
return nil, nil, nil, nil
116117
}
@@ -136,12 +137,12 @@ func (o *MockEntryService[E, L]) CreateWithXpath(ctx context.Context, xpath stri
136137
}
137138

138139
func (o *MockEntryService[E, L]) Create(ctx context.Context, location L, entry E) (E, error) {
139-
o.client.Initial.PushBack(entry)
140+
o.client.Current.PushBack(entry)
140141

141142
return entry, nil
142143
}
143144

144-
func (o *MockEntryService[E, L]) UpdateWithXpath(ctx context.Context, xpath string, entry E, name string) error {
145+
func (o *MockEntryService[E, L]) UpdateWithXpath(ctx context.Context, xpath string, entry E, renamedXpath string) error {
145146
for e := o.client.Initial.Front(); e != nil; e = e.Next() {
146147
eltEntry := e.Value.(E)
147148
if entry.EntryName() == eltEntry.EntryName() {
@@ -166,7 +167,7 @@ func (o *MockEntryService[E, L]) ReadWithXpath(ctx context.Context, xpath string
166167
}
167168

168169
func (o *MockEntryService[E, L]) Read(ctx context.Context, location L, name string, action string) (E, error) {
169-
for e := o.client.Initial.Front(); e != nil; e = e.Next() {
170+
for e := o.client.Current.Front(); e != nil; e = e.Next() {
170171
entry := e.Value.(E)
171172
if entry.EntryName() == name {
172173
return entry, nil

‎assets/terraform/internal/manager/utils_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ type MultiConfigOper struct {
6868
Operation MultiConfigOperType
6969
EntryName string
7070
EntryUuid *string
71+
NewName string
72+
Value string
7173
}
7274

7375
func entryNameFromXpath(xpath string) string {
@@ -133,6 +135,7 @@ func MultiConfig[E sdkmanager.UuidObject](updates *xmlapi.MultiConfig, existingP
133135
operEntry := MultiConfigOper{
134136
Operation: MultiConfigOperType(op),
135137
EntryName: entryName,
138+
NewName: oper.NewName,
136139
}
137140

138141
slog.Debug("MultiConfig", "operEntry", operEntry)
@@ -146,6 +149,7 @@ func MultiConfig[E sdkmanager.UuidObject](updates *xmlapi.MultiConfig, existingP
146149
entry.SetEntryUuid(existing.Entry.EntryUuid())
147150
}
148151
existing.Entry = entry
152+
entriesByName[entryName] = existing
149153
} else {
150154
entryUuid := fmt.Sprintf("%05d", uuid)
151155
if objectType == multiConfigUuid {
@@ -169,6 +173,19 @@ func MultiConfig[E sdkmanager.UuidObject](updates *xmlapi.MultiConfig, existingP
169173
fixIndices(entry.Idx)
170174
delete(entriesByName, entryName)
171175
idx -= 1
176+
case MultiConfigOperRename:
177+
if _, found := entriesByName[oper.NewName]; found {
178+
panic(fmt.Sprintf("FIXME: should propagate back error from MultiConfig"))
179+
}
180+
181+
existing, found := entriesByName[entryName]
182+
if !found {
183+
panic(fmt.Sprintf("FIXME: should propagate back erro from MultiConfig"))
184+
}
185+
186+
delete(entriesByName, entryName)
187+
existing.Entry.SetEntryName(oper.NewName)
188+
entriesByName[oper.NewName] = existing
172189
case MultiConfigOperMove:
173190
pivot, found := entriesByName[oper.Destination]
174191
if !found && oper.Destination != "top" && oper.Destination != "bottom" {

0 commit comments

Comments
 (0)