fix(terraform): make sure unsupported XML nodes are properly preserved on updates

Author: kklimonda-cl

assets/terraform/test/resource_template_stack_test.go

@@ -1,14 +1,21 @@
 package provider_test
 
 import (
+	"context"
+	"encoding/xml"
 	"fmt"
 	"testing"
 
+	"github.com/PaloAltoNetworks/pango/generic"
+	"github.com/PaloAltoNetworks/pango/panorama/template_stack"
+	"github.com/PaloAltoNetworks/pango/util"
+	"github.com/PaloAltoNetworks/pango/xmlapi"
 	"github.com/hashicorp/terraform-plugin-testing/config"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
 	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
 	"github.com/hashicorp/terraform-plugin-testing/statecheck"
+	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
 )
 
@@ -399,3 +406,230 @@ resource "panos_template_stack" "example" {
   default_vsys = "vsys1"
 }
 `
+
+// TestAccTemplateStack_DeviceVariablePreservation verifies that updating a
+// template stack (e.g. changing description) does not delete per-device
+// variable overrides stored as sub-elements of device entries.
+//
+// This reproduces a reported bug where the PUT request sends device entries
+// without their child elements, causing PAN-OS to delete per-device template
+// variable overrides.
+func TestAccTemplateStack_DeviceVariablePreservation(t *testing.T) {
+	t.Parallel()
+
+	nameSuffix := acctest.RandStringFromCharSet(6, acctest.CharSetAlphaNum)
+	prefix := fmt.Sprintf("test-acc-%s", nameSuffix)
+
+	suffix := acctest.RandStringFromCharSet(13, "0123456789")
+	serialNumber := fmt.Sprintf("00%s", suffix)
+
+	stackName := fmt.Sprintf("%s-stack", prefix)
+
+	location := config.ObjectVariable(map[string]config.Variable{
+		"panorama": config.ObjectVariable(map[string]config.Variable{}),
+	})
+
+	configVars := map[string]config.Variable{
+		"prefix":        config.StringVariable(prefix),
+		"location":      location,
+		"serial_number": config.StringVariable(serialNumber),
+	}
+
+	// Build the xpath for direct SDK access since Read/Update have a bug
+	// with name formatting. Use ReadWithXpath/UpdateWithXpath instead.
+	loc := template_stack.Location{
+		Panorama: &template_stack.PanoramaLocation{
+			PanoramaDevice: "localhost.localdomain",
+		},
+	}
+	stackXpathParts, err := loc.XpathWithComponents(sdkClient.Versioning(), util.AsEntryXpath(stackName))
+	if err != nil {
+		t.Fatalf("Failed to build template stack xpath: %v", err)
+	}
+	stackXpath := util.AsXpath(stackXpathParts)
+
+	// injectDeviceVariable uses a direct API call to add a per-device variable
+	// override to the device entry, simulating a user setting local values
+	// via the Panorama UI.
+	//
+	// We use sdkClient.Communicate directly because the SDK's UpdateWithXpath
+	// uses SpecMatches which doesn't compare Misc fields, so it would skip
+	// the update.
+	injectDeviceVariable := func() {
+		// Build the xpath to the specific device entry within the template stack.
+		deviceXpath := fmt.Sprintf("%s/devices/entry[@name='%s']", stackXpath, serialNumber)
+
+		// Override the stack-level template variable with a per-device value.
+		// The variable name must match the existing template variable.
+		varName := fmt.Sprintf("$%s-var", prefix)
+		variableXml := generic.Xml{
+			XMLName: xml.Name{Local: "variable"},
+			Nodes: []generic.Xml{
+				{
+					XMLName: xml.Name{Local: "entry"},
+					Name:    &varName,
+					Nodes: []generic.Xml{
+						{
+							XMLName: xml.Name{Local: "type"},
+							Nodes: []generic.Xml{
+								{
+									XMLName: xml.Name{Local: "ip-netmask"},
+									Text:    []byte("10.0.0.1/24"),
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		cmd := &xmlapi.Config{
+			Action:  "set",
+			Xpath:   deviceXpath,
+			Element: variableXml,
+			Target:  sdkClient.GetTarget(),
+		}
+
+		if _, _, err := sdkClient.Communicate(context.TODO(), cmd, false, nil); err != nil {
+			t.Fatalf("Failed to inject per-device variable: %v", err)
+		}
+	}
+
+	// checkDeviceVariableExists verifies that the per-device variable data
+	// survived the Terraform update by reading the template stack via the
+	// pango SDK and checking the device entry's Misc field.
+	checkDeviceVariableExists := func(s *terraform.State) error {
+		svc := template_stack.NewService(sdkClient)
+
+		entry, err := svc.ReadWithXpath(context.TODO(), stackXpath, "get")
+		if err != nil {
+			return fmt.Errorf("failed to read template stack: %v", err)
+		}
+
+		for _, device := range entry.Devices {
+			if device.Name == serialNumber {
+				if len(device.Misc) == 0 {
+					return fmt.Errorf(
+						"device %s lost its Misc data: per-device variable overrides were deleted during template stack update",
+						serialNumber,
+					)
+				}
+				return nil
+			}
+		}
+		return fmt.Errorf("device %s not found in template stack after update", serialNumber)
+	}
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:          templateStack_DeviceVarPreservation_Step1_Tmpl,
+				ConfigVariables: configVars,
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(
+						"panos_template_stack.test",
+						tfjsonpath.New("description"),
+						knownvalue.StringExact("Original description"),
+					),
+					statecheck.ExpectKnownValue(
+						"panos_template_stack.test",
+						tfjsonpath.New("devices"),
+						knownvalue.ListExact([]knownvalue.Check{
+							knownvalue.ObjectExact(map[string]knownvalue.Check{
+								"name": knownvalue.StringExact(serialNumber),
+							}),
+						}),
+					),
+				},
+			},
+			{
+				PreConfig:       injectDeviceVariable,
+				Config:          templateStack_DeviceVarPreservation_Step2_Tmpl,
+				ConfigVariables: configVars,
+				Check:           checkDeviceVariableExists,
+				ConfigStateChecks: []statecheck.StateCheck{
+					statecheck.ExpectKnownValue(
+						"panos_template_stack.test",
+						tfjsonpath.New("description"),
+						knownvalue.StringExact("Updated description"),
+					),
+					statecheck.ExpectKnownValue(
+						"panos_template_stack.test",
+						tfjsonpath.New("devices"),
+						knownvalue.ListExact([]knownvalue.Check{
+							knownvalue.ObjectExact(map[string]knownvalue.Check{
+								"name": knownvalue.StringExact(serialNumber),
+							}),
+						}),
+					),
+				},
+			},
+		},
+	})
+}
+
+const templateStack_DeviceVarPreservation_Step1_Tmpl = `
+variable "prefix" { type = string }
+variable "location" { type = any }
+variable "serial_number" { type = string }
+
+resource "panos_template" "test" {
+  location = var.location
+  name = "${var.prefix}-template"
+}
+
+resource "panos_firewall_device" "test" {
+  location = var.location
+  name = var.serial_number
+  hostname = "fw-devvar.example.com"
+  ip = "192.0.2.10"
+}
+
+resource "panos_template_stack" "test" {
+  location = var.location
+  name = "${var.prefix}-stack"
+  description = "Original description"
+  templates = [panos_template.test.name]
+  devices = [{ name = panos_firewall_device.test.name }]
+}
+
+resource "panos_template_variable" "test" {
+  location = { template_stack = { name = panos_template_stack.test.name } }
+  name = format("$%s-var", var.prefix)
+  type = { ip_netmask = "10.0.0.0/24" }
+}
+`
+
+const templateStack_DeviceVarPreservation_Step2_Tmpl = `
+variable "prefix" { type = string }
+variable "location" { type = any }
+variable "serial_number" { type = string }
+
+resource "panos_template" "test" {
+  location = var.location
+  name = "${var.prefix}-template"
+}
+
+resource "panos_firewall_device" "test" {
+  location = var.location
+  name = var.serial_number
+  hostname = "fw-devvar.example.com"
+  ip = "192.0.2.10"
+}
+
+resource "panos_template_stack" "test" {
+  location = var.location
+  name = "${var.prefix}-stack"
+  description = "Updated description"
+  templates = [panos_template.test.name]
+  devices = [{ name = panos_firewall_device.test.name }]
+}
+
+resource "panos_template_variable" "test" {
+  location = { template_stack = { name = panos_template_stack.test.name } }
+  name = format("$%s-var", var.prefix)
+  type = { ip_netmask = "10.0.0.0/24" }
+}
+`

templates/terraform-provider/conversion/copy_to_pango.tmpl

@@ -34,6 +34,7 @@
     {{- $terraformType := printf "%s%sObject" $.Spec.TerraformType .TerraformName.CamelCase }}
     {{- $pangoEntries := printf "%s_pango_entries" .TerraformName.LowerCamelCase }}
     {{- $tfEntries := printf "%s_tf_entries" .TerraformName.LowerCamelCase }}
+    {{- $existingEntries := printf "%s_existing_entries" .TerraformName.LowerCamelCase }}
     {{- if eq .ItemsType "entry" }}
 		var {{ $tfEntries }} []{{ $terraformType }}
 		var {{ $pangoEntries }} []{{ $pangoType }}
@@ -43,8 +44,14 @@
 		if diags.HasError() {
 			return diags
 		}
+		{{ $existingEntries }} := make(map[string]*{{ $pangoType }})
+		if *obj != nil {
+			for idx := range (*obj).{{ .PangoName.CamelCase }} {
+				{{ $existingEntries }}[(*obj).{{ .PangoName.CamelCase }}[idx].Name] = &(*obj).{{ .PangoName.CamelCase }}[idx]
+			}
+		}
 		for _, elt := range {{ $tfEntries }} {
-			var entry *{{ $pangoType }}
+			entry := {{ $existingEntries }}[elt.Name.ValueString()]
 			diags.Append(elt.CopyToPango(ctx, client, append(ancestors, elt), &entry, ev)...)
 			if diags.HasError() {
 				return diags