diff --git a/assets/terraform/test/resource_template_stack_test.go b/assets/terraform/test/resource_template_stack_test.go index a7a69680..b799ca08 100644 --- a/assets/terraform/test/resource_template_stack_test.go +++ b/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" } +} +` diff --git a/templates/terraform-provider/conversion/copy_to_pango.tmpl b/templates/terraform-provider/conversion/copy_to_pango.tmpl index ff4f599f..87d35bce 100644 --- a/templates/terraform-provider/conversion/copy_to_pango.tmpl +++ b/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