fix: preserve optional nested objects on readback

Refs: #1294

Author: dmacvicar

AGENTS.md

@@ -193,6 +193,8 @@ Example:
 
 Terraform state reflects what the user cares about, not the entire API response.
 
+For generated XML -> model conversions, see `internal/codegen/README.md`, especially the "Preserve User Intent For Optional Nested Objects" section. That generator-level rule is authoritative for optional nested object readback behavior.
+
 **Computed** (not Optional) - Always read from API
 - Examples: `id`, `key`, `allocation`, `physical`
 - `model.Key = types.StringValue(xml.Key)`

internal/codegen/README.md

@@ -63,6 +63,26 @@ The generator has two separate responsibilities:
 
 Keep those layers separate. The parser should not accumulate ad hoc Terraform exceptions based on struct names. If a field needs special Terraform behavior, prefer a policy rule after reflection rather than embedding one-off conditionals into the reflector.
 
+### Preserve User Intent For Optional Nested Objects
+
+`Preserve user intent` is a generator-level contract, not a resource-by-resource preference.
+
+For optional nested objects, XML omission and user omission are not the same thing:
+
+- If the user did not configure an optional nested object, state should keep it `null`.
+- If the user configured an optional nested object and libvirt omits that object when reading XML back, the conversion should preserve the planned object unless we have an explicit policy that omission means the field was cleared.
+- Do not silently collapse an explicitly configured nested object to `null` only because the readback XML does not echo it.
+
+This rule exists to prevent Terraform `Provider produced inconsistent result after apply` errors for fields such as `alias`, where libvirt may not round-trip the object exactly even though the user configured it explicitly.
+
+In practice, this means the XML -> model conversion for optional nested objects must distinguish:
+
+- `plan field is null`: user does not care, keep state `null`
+- `plan field is non-null` and `xml field is present`: convert XML back into state
+- `plan field is non-null` and `xml field is absent`: preserve the planned value unless an explicit override says otherwise
+
+When changing converter templates, treat this as an invariant that applies uniformly across generated nested object fields. Do not patch individual resources to compensate for generator behavior unless the field truly needs special semantics.
+
 ### Policy rules
 
 Policy should be applied in an ordered pass over `StructIR` / `FieldIR`:

internal/codegen/templates/convert.go.tmpl

@@ -470,7 +470,14 @@ func {{ .Name }}ToXML(ctx context.Context, model *{{ .Name }}Model) (*libvirtxml
 				model.{{ .GoName }} = types.ObjectNull({{ .NestedStruct.Name }}AttributeTypes())
 			}
 		} else {
-			model.{{ .GoName }} = types.ObjectNull({{ .NestedStruct.Name }}AttributeTypes())
+			// Preserve explicit user intent for optional nested objects when the API omits
+			// the field on readback. This avoids collapsing configured objects such as
+			// aliases to null and triggering "inconsistent result after apply" errors.
+			if plan != nil && !plan.{{ .GoName }}.IsNull() && !plan.{{ .GoName }}.IsUnknown() {
+				model.{{ .GoName }} = plan.{{ .GoName }}
+			} else {
+				model.{{ .GoName }} = types.ObjectNull({{ .NestedStruct.Name }}AttributeTypes())
+			}
 		}
 	} else {
 		model.{{ .GoName }} = types.ObjectNull({{ .NestedStruct.Name }}AttributeTypes())

internal/provider/domain_resource_test.go

@@ -1577,6 +1577,46 @@ func TestAccDomainResource_console(t *testing.T) {
 	})
 }
 
+func TestAccDomainResource_consoleAndSerialAlias(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { testAccPreCheck(t) },
+		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
+		CheckDestroy:             testAccCheckDomainDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccDomainResourceConfigConsoleAndSerialAlias("test-domain-console-serial-alias"),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					resource.TestCheckResourceAttr("libvirt_domain.test", "name", "test-domain-console-serial-alias"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "running", "false"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.#", "1"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.alias.name", "serial0"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.protocol.type", "telnet"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.source.tcp.host", "127.0.0.1"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.source.tcp.mode", "bind"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.source.tcp.service", "12345"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.source.tcp.tls", "no"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.target.type", "isa-serial"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.target.port", "0"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.serials.0.target.model.name", "isa-serial"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.#", "1"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.alias.name", "serial0"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.protocol.type", "telnet"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.source.tcp.host", "127.0.0.1"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.source.tcp.mode", "bind"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.source.tcp.service", "12345"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.source.tcp.tls", "no"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.target.type", "serial"),
+					resource.TestCheckResourceAttr("libvirt_domain.test", "devices.consoles.0.target.port", "0"),
+				),
+			},
+			{
+				Config:   testAccDomainResourceConfigConsoleAndSerialAlias("test-domain-console-serial-alias"),
+				PlanOnly: true,
+			},
+		},
+	})
+}
+
 func testAccDomainResourceConfigConsole(name string) string {
 	return fmt.Sprintf(`
 resource "libvirt_domain" "test" {
@@ -1610,6 +1650,76 @@ resource "libvirt_domain" "test" {
 `, name)
 }
 
+func testAccDomainResourceConfigConsoleAndSerialAlias(name string) string {
+	return fmt.Sprintf(`
+resource "libvirt_domain" "test" {
+  name        = %[1]q
+  running     = false
+  memory      = 512
+  memory_unit = "MiB"
+  vcpu        = 1
+  type        = "kvm"
+
+  os = {
+    type         = "hvm"
+    type_arch    = "x86_64"
+    type_machine = "q35"
+  }
+
+  devices = {
+    serials = [
+      {
+        alias = {
+          name = "serial0"
+        }
+        source = {
+          tcp = {
+            mode    = "bind"
+            host    = "127.0.0.1"
+            service = "12345"
+            tls     = "no"
+          }
+        }
+        protocol = {
+          type = "telnet"
+        }
+        target = {
+          type = "isa-serial"
+          port = 0
+          model = {
+            name = "isa-serial"
+          }
+        }
+      }
+    ]
+
+    consoles = [
+      {
+        alias = {
+          name = "serial0"
+        }
+        source = {
+          tcp = {
+            mode    = "bind"
+            host    = "127.0.0.1"
+            service = "12345"
+            tls     = "no"
+          }
+        }
+        protocol = {
+          type = "telnet"
+        }
+        target = {
+          type = "serial"
+          port = 0
+        }
+      }
+    ]
+  }
+}
+`, name)
+}
+
 func TestAccDomainResource_autostart(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:                 func() { testAccPreCheck(t) },