diff --git a/docs/plans-executions/2026-09-16-1333-kvm2-provider.md b/docs/plans-executions/2026-09-16-1333-kvm2-provider.md index 4c18e34..a500777 100644 --- a/docs/plans-executions/2026-09-16-1333-kvm2-provider.md +++ b/docs/plans-executions/2026-09-16-1333-kvm2-provider.md @@ -5,7 +5,7 @@ Plan: [docs/plans/2026-09-16-1333-kvm2-provider.md](../plans/2026-09-16-1333-kvm - [x] Step 0 — Plan file and branch - [x] Step 1 — The spec, `docs/kvm2-provider-spec.md` - [x] Step 2 — Config wiring, IDs, state mapping, classify -- [ ] Step 3 — Pure builders: domain and volume XML, metadata, seed +- [x] Step 3 — Pure builders: domain and volume XML, metadata, seed - [ ] Step 4 — Transport, seam, decorator, fakes - [ ] Step 5 — Image cache and pool-space precheck - [ ] Step 6 — The four contract methods @@ -56,7 +56,7 @@ re-derive the mapping. ## Step 2 — Config wiring, IDs, state mapping, classify -2026-09-16 16:40. First subagent step. The brief named the allowed files, +2026-09-16 16:38. First subagent step. The brief named the allowed files, the spec entries, the test names and the prohibition on `internal/provider/kvm/`; the agent reported nothing opened there, with one honest caveat: a `golangci-lint` run printed seven single source lines from @@ -94,3 +94,34 @@ because the local toolchain is Go 1.27.0 while `go.mod` says 1.26 and the bundled staticcheck predates it; it reproduces on untouched packages, so lint is deferred to Step 10 with a toolchain note. The `.claude/settings.json` modification is still local and uncommitted. + +## Step 3 — Pure builders: domain and volume XML, metadata, seed + +2026-09-16 16:55. Second subagent step, new files only: `xml.go`, +`metadata.go`, `volume.go`, `seed.go` and their tests (26 tests). The +domain XML is built from `encoding/xml` structs, both disks are +`type="file"` with resolved paths (the AppArmor lesson), UEFI is the default +with the secure-boot feature disabled, the metadata element parses with or +without its namespace declaration, and the seed ISO is staged under +`os.TempDir()` (the library uses `os.MkdirTemp("", "")`, so `TMPDIR` is +honoured; the test points it at a `t.TempDir()` and asserts nothing is left +behind). + +Decisions the agent made, all accepted: `buildDomainXML` defaults an empty +disk format to qcow2 and rejects anything but qcow2 or raw; +`buildVolumeXML` rejects a non-positive capacity; `buildSeedISO` rejects an +empty name; the guest-agent script still runs the enable-and-start step when +the agent is already installed; the multipart boundary is random, so +vendor-data is not byte-deterministic across calls (the seed is rebuilt every +Create anyway). + +Commands: + +```sh +gofmt -l ./internal/provider/kvm2 ; go vet ./internal/provider/kvm2/ && go test -race -count=1 ./internal/provider/kvm2/ +go build ./... && go test -short ./... +``` + +All `ok`. Review greps for a v1 import or v1 identifiers are empty; the only +`type="volume"` in the package is the comment explaining why it is not used. +The agent reported "files read outside the allowed set: none". diff --git a/internal/provider/kvm2/metadata.go b/internal/provider/kvm2/metadata.go new file mode 100644 index 0000000..3d1526d --- /dev/null +++ b/internal/provider/kvm2/metadata.go @@ -0,0 +1,78 @@ +package kvm2 + +import ( + "encoding/xml" + "strings" + "time" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// The GC mark lives in the domain's as one element in kvm2's own +// namespace (spec entry 8). The namespace is what selects it in +// DomainGetMetadata; libvirt 11.6 hands the element back without the xmlns +// declaration, so parsing matches on the local name only. +const ( + metadataURI = "https://gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/kvm2" + metadataLocalName = "instance" +) + +// instanceMetadata is the mark as it is written and read back. The fields +// are the element's text as is; managed and createdAt interpret them. +type instanceMetadata struct { + XMLName xml.Name + Managed string `xml:"managed"` + UID string `xml:"uid"` + CreatedAt string `xml:"createdAt"` +} + +func newInstanceMetadata(uid string, createdAt time.Time) instanceMetadata { + return instanceMetadata{ + XMLName: xml.Name{Space: metadataURI, Local: metadataLocalName}, + Managed: provider.LabelManagedYes, + UID: uid, + CreatedAt: createdAt.UTC().Format(time.RFC3339), + } +} + +func (m instanceMetadata) managed() bool { + return m.Managed == provider.LabelManagedYes +} + +// createdAt returns the zero time when the element carries no parsable +// timestamp; GC treats that as old, which the spec accepts. +func (m instanceMetadata) createdAt() time.Time { + t, err := time.Parse(time.RFC3339, m.CreatedAt) + if err != nil { + return time.Time{} + } + return t +} + +// marshalInstanceMetadata renders the namespaced element that goes under +// the domain's . +func marshalInstanceMetadata(m instanceMetadata) (string, error) { + m.XMLName = xml.Name{Space: metadataURI, Local: metadataLocalName} + out, err := xml.Marshal(m) + if err != nil { + return "", permanentf("marshalling instance metadata: %v", err) + } + return string(out), nil +} + +// parseInstanceMetadata reads the element back, with or without its xmlns +// declaration. Anything that is not an element is ErrPermanent: +// the domain's metadata is corrupt and no retry fixes it. +func parseInstanceMetadata(s string) (instanceMetadata, error) { + var m instanceMetadata + if strings.TrimSpace(s) == "" { + return instanceMetadata{}, permanentf("parsing instance metadata: empty document") + } + if err := xml.Unmarshal([]byte(s), &m); err != nil { + return instanceMetadata{}, permanentf("parsing instance metadata: %v", err) + } + if m.XMLName.Local != metadataLocalName { + return instanceMetadata{}, permanentf("parsing instance metadata: root element is <%s>, want <%s>", m.XMLName.Local, metadataLocalName) + } + return m, nil +} diff --git a/internal/provider/kvm2/metadata_test.go b/internal/provider/kvm2/metadata_test.go new file mode 100644 index 0000000..42c73f2 --- /dev/null +++ b/internal/provider/kvm2/metadata_test.go @@ -0,0 +1,162 @@ +package kvm2 + +import ( + "errors" + "strings" + "testing" + "time" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +func TestMarshalInstanceMetadata_namespacedElement(t *testing.T) { + t.Parallel() + created := time.Date(2026, 9, 16, 10, 30, 0, 0, time.FixedZone("CEST", 2*3600)) + got, err := marshalInstanceMetadata(newInstanceMetadata("uid-1", created)) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + ``, + "true", + "uid-1", + "2026-09-16T08:30:00Z", + "", + } { + if !strings.Contains(got, want) { + t.Errorf("marshalled metadata %q lacks %q", got, want) + } + } +} + +func TestParseMetadata_withNamespace(t *testing.T) { + t.Parallel() + in := `trueabc2026-09-16T08:30:00Z` + m, err := parseInstanceMetadata(in) + if err != nil { + t.Fatal(err) + } + if !m.managed() { + t.Error("managed() = false, want true") + } + if m.UID != "abc" { + t.Errorf("UID = %q, want abc", m.UID) + } + if want := time.Date(2026, 9, 16, 8, 30, 0, 0, time.UTC); !m.createdAt().Equal(want) { + t.Errorf("createdAt() = %v, want %v", m.createdAt(), want) + } +} + +func TestParseMetadata_withoutNamespaceDeclaration(t *testing.T) { + t.Parallel() + in := "\n true\n abc\n 2026-09-16T08:30:00Z\n" + m, err := parseInstanceMetadata(in) + if err != nil { + t.Fatal(err) + } + if !m.managed() || m.UID != "abc" { + t.Errorf("got managed=%v uid=%q, want true/abc", m.managed(), m.UID) + } + if m.createdAt().IsZero() { + t.Error("createdAt() is zero, want parsed timestamp") + } +} + +func TestParseMetadata_absentGivesEmptyUID(t *testing.T) { + t.Parallel() + tests := []struct { + name string + in string + }{ + {name: "empty element", in: ""}, + {name: "empty element with namespace", in: ``}, + {name: "empty children", in: ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + m, err := parseInstanceMetadata(tc.in) + if err != nil { + t.Fatal(err) + } + if m.managed() { + t.Error("managed() = true, want false") + } + if m.UID != "" { + t.Errorf("UID = %q, want empty", m.UID) + } + if !m.createdAt().IsZero() { + t.Errorf("createdAt() = %v, want zero", m.createdAt()) + } + }) + } +} + +func TestParseMetadata_badCreatedAtIsZeroTime(t *testing.T) { + t.Parallel() + tests := []struct { + name string + in string + }{ + {name: "garbage", in: "trueuyesterday"}, + {name: "date only", in: "trueu2026-09-16"}, + {name: "missing", in: "trueu"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + m, err := parseInstanceMetadata(tc.in) + if err != nil { + t.Fatal(err) + } + if !m.createdAt().IsZero() { + t.Errorf("createdAt() = %v, want zero", m.createdAt()) + } + if !m.managed() || m.UID != "u" { + t.Errorf("other fields lost: managed=%v uid=%q", m.managed(), m.UID) + } + }) + } +} + +func TestParseMetadata_notXMLIsPermanent(t *testing.T) { + t.Parallel() + tests := []struct { + name string + in string + }{ + {name: "empty string", in: ""}, + {name: "whitespace", in: " \n"}, + {name: "plain text", in: "managed: true"}, + {name: "unclosed element", in: "true"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := parseInstanceMetadata(tc.in) + if !errors.Is(err, provider.ErrPermanent) { + t.Errorf("err = %v, want ErrPermanent", err) + } + }) + } +} + +func TestParseMetadata_wrongRootIsPermanent(t *testing.T) { + t.Parallel() + tests := []struct { + name string + in string + }{ + {name: "other local name", in: "true"}, + {name: "other local name with our namespace", in: `x`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := parseInstanceMetadata(tc.in) + if !errors.Is(err, provider.ErrPermanent) { + t.Errorf("err = %v, want ErrPermanent", err) + } + }) + } +} diff --git a/internal/provider/kvm2/seed.go b/internal/provider/kvm2/seed.go new file mode 100644 index 0000000..4c81292 --- /dev/null +++ b/internal/provider/kvm2/seed.go @@ -0,0 +1,124 @@ +package kvm2 + +import ( + "bytes" + "errors" + "fmt" + "io" + "mime/multipart" + "net/textproto" + "strings" + + "github.com/kdomanski/iso9660" +) + +// seedLabel is the volume label cloud-init's NoCloud datasource looks for. +const seedLabel = "cidata" + +// guestAgentScript is the vendor-data payload that gets qemu-guest-agent +// running so the provider can read the VM's address through libvirt. It is +// a shell script and not a cloud-config on purpose: cloud-init merges a +// vendor-data cloud-config under the user-data with mergers that keep the +// user-data's value for every top-level key and never append lists, so a +// Proxy with its own packages: would silently lose the agent. Scripts +// bypass the merge. +const guestAgentScript = `#!/bin/sh +# Added by egress-proxies-operator: installs and starts qemu-guest-agent so +# the operator can learn this VM's IP address through libvirt. +# To leave this out, set installGuestAgent: false in the provider's config. +set -e +if command -v qemu-ga >/dev/null 2>&1 || [ -x /usr/sbin/qemu-ga ]; then + echo "egress-proxies-operator: qemu-guest-agent already installed" +elif command -v apt-get >/dev/null 2>&1; then + export DEBIAN_FRONTEND=noninteractive + apt-get update + apt-get install -y qemu-guest-agent +elif command -v dnf >/dev/null 2>&1; then + dnf install -y qemu-guest-agent +elif command -v yum >/dev/null 2>&1; then + yum install -y qemu-guest-agent +elif command -v zypper >/dev/null 2>&1; then + zypper --non-interactive install qemu-guest-agent +elif command -v apk >/dev/null 2>&1; then + apk add qemu-guest-agent +else + echo "egress-proxies-operator: no supported package manager found; install qemu-guest-agent by hand" >&2 + exit 1 +fi +if command -v systemctl >/dev/null 2>&1; then + systemctl enable --now qemu-guest-agent +else + rc-update add qemu-guest-agent default + rc-service qemu-guest-agent start +fi +` + +// buildSeedISO writes a NoCloud seed to w: user-data byte for byte, +// meta-data naming the instance, and vendor-data (an empty file when there +// is none, so cloud-init does not look elsewhere). iso9660.NewWriter stages +// the files in os.MkdirTemp("", ""), that is under os.TempDir(): $TMPDIR +// when set, else /tmp, the manager's 64Mi emptyDir. The staging dir is +// removed before returning. Nothing about the payload is logged here. +func buildSeedISO(name, userData, vendorData string, w io.Writer) (err error) { + if name == "" { + return permanentf("seed ISO: instance name is empty") + } + iw, err := iso9660.NewWriter() + if err != nil { + return fmt.Errorf("creating seed ISO staging dir: %w", err) + } + defer func() { + if cerr := iw.Cleanup(); cerr != nil { + err = errors.Join(err, fmt.Errorf("removing seed ISO staging dir: %w", cerr)) + } + }() + metaData := "instance-id: " + name + "\nlocal-hostname: " + name + "\n" + files := []struct{ name, content string }{ + {"user-data", userData}, + {"meta-data", metaData}, + {"vendor-data", vendorData}, + } + for _, f := range files { + if err := iw.AddFile(strings.NewReader(f.content), f.name); err != nil { + return fmt.Errorf("adding %s to seed ISO: %w", f.name, err) + } + } + if err := iw.WriteTo(w, seedLabel); err != nil { + return fmt.Errorf("writing seed ISO: %w", err) + } + return nil +} + +// buildVendorData composes the seed's vendor-data from the configured +// document and the guest agent script. With both, they travel as separate +// parts of a MIME multipart, so cloud-init handles each by its own type; the +// MIME-Version header comes first because cloud-init detects multipart by +// finding "mime-version:" within the first 4096 bytes. +func buildVendorData(configured string, installAgent bool) (string, error) { + switch { + case !installAgent: + return configured, nil + case strings.TrimSpace(configured) == "": + return guestAgentScript, nil + } + var buf bytes.Buffer + mw := multipart.NewWriter(&buf) + fmt.Fprintf(&buf, "MIME-Version: 1.0\r\nContent-Type: multipart/mixed; boundary=%q\r\n\r\n", mw.Boundary()) + parts := []struct{ contentType, body string }{ + {"text/plain", configured}, + {"text/x-shellscript", guestAgentScript}, + } + for _, p := range parts { + pw, err := mw.CreatePart(textproto.MIMEHeader{"Content-Type": {p.contentType}}) + if err != nil { + return "", fmt.Errorf("creating vendor-data part %s: %w", p.contentType, err) + } + if _, err := io.WriteString(pw, p.body); err != nil { + return "", fmt.Errorf("writing vendor-data part %s: %w", p.contentType, err) + } + } + if err := mw.Close(); err != nil { + return "", fmt.Errorf("closing vendor-data multipart: %w", err) + } + return buf.String(), nil +} diff --git a/internal/provider/kvm2/seed_test.go b/internal/provider/kvm2/seed_test.go new file mode 100644 index 0000000..e0bb40e --- /dev/null +++ b/internal/provider/kvm2/seed_test.go @@ -0,0 +1,264 @@ +package kvm2 + +import ( + "bufio" + "bytes" + "io" + "mime" + "mime/multipart" + "net/textproto" + "os" + "strings" + "testing" + + "github.com/kdomanski/iso9660" +) + +// readSeed builds a seed and returns its files by name plus the label. +func readSeed(t *testing.T, name, userData, vendorData string) (map[string]string, string) { + t.Helper() + var buf bytes.Buffer + if err := buildSeedISO(name, userData, vendorData, &buf); err != nil { + t.Fatalf("buildSeedISO: %v", err) + } + img, err := iso9660.OpenImage(bytes.NewReader(buf.Bytes())) + if err != nil { + t.Fatalf("OpenImage: %v", err) + } + root, err := img.RootDir() + if err != nil { + t.Fatalf("RootDir: %v", err) + } + children, err := root.GetChildren() + if err != nil { + t.Fatalf("GetChildren: %v", err) + } + files := make(map[string]string, len(children)) + for _, c := range children { + if c.IsDir() { + t.Errorf("unexpected directory %q in seed", c.Name()) + continue + } + data, err := io.ReadAll(c.Reader()) + if err != nil { + t.Fatalf("reading %s: %v", c.Name(), err) + } + files[c.Name()] = string(data) + } + label, err := img.Label() + if err != nil { + t.Fatalf("Label: %v", err) + } + return files, label +} + +func TestSeedISO_filesAndLabel(t *testing.T) { + t.Parallel() + userData := "#cloud-config\npackages: [squid]\n" + vendorData := "#!/bin/sh\necho hi\n" + files, label := readSeed(t, "proxy-abc", userData, vendorData) + if label != seedLabel { + t.Errorf("label = %q, want %q", label, seedLabel) + } + want := map[string]string{ + "user-data": userData, + "meta-data": "instance-id: proxy-abc\nlocal-hostname: proxy-abc\n", + "vendor-data": vendorData, + } + if len(files) != len(want) { + t.Errorf("seed has %d files %v, want exactly %d", len(files), keys(files), len(want)) + } + for name, content := range want { + if got, ok := files[name]; !ok { + t.Errorf("seed lacks %s", name) + } else if got != content { + t.Errorf("%s = %q, want %q", name, got, content) + } + } +} + +func keys(m map[string]string) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} + +func TestSeedISO_userDataByteForByte(t *testing.T) { + t.Parallel() + tests := []struct { + name string + userData string + }{ + {name: "mime multipart with CRLF", userData: "MIME-Version: 1.0\r\nContent-Type: multipart/mixed; boundary=\"b\"\r\n\r\n--b\r\nContent-Type: text/cloud-config\r\n\r\n#cloud-config\r\npackages: [squid]\r\n--b--\r\n"}, + {name: "jinja header", userData: "## template: jinja\n#cloud-config\nhostname: {{ v1.local_hostname }}\n"}, + {name: "no trailing newline", userData: "#cloud-config\nruncmd:\n - echo done"}, + {name: "bytes above 0x7f", userData: "#cloud-config\nwrite_files:\n - content: \"caf\xc3\xa9 \xff\xfe\x80\"\n"}, + {name: "empty", userData: ""}, + {name: "trailing whitespace preserved", userData: "#!/bin/sh\necho x \n\n\n"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + files, _ := readSeed(t, "proxy-abc", tc.userData, "") + if got := files["user-data"]; got != tc.userData { + t.Errorf("user-data changed:\n got %q\nwant %q", got, tc.userData) + } + }) + } +} + +// Not parallel: it changes TMPDIR for the process. +func TestSeedISO_tempDirRemoved(t *testing.T) { + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + if got := os.TempDir(); got != tmp { + t.Fatalf("os.TempDir() = %q, want %q", got, tmp) + } + var buf bytes.Buffer + if err := buildSeedISO("proxy-abc", "#cloud-config\n", guestAgentScript, &buf); err != nil { + t.Fatalf("buildSeedISO: %v", err) + } + if buf.Len() == 0 { + t.Fatal("seed ISO is empty") + } + entries, err := os.ReadDir(tmp) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + names := make([]string, 0, len(entries)) + for _, e := range entries { + names = append(names, e.Name()) + } + t.Errorf("staging left behind in TMPDIR: %v", names) + } +} + +func TestSeedISO_emptyVendorDataStillHasFile(t *testing.T) { + t.Parallel() + files, _ := readSeed(t, "proxy-abc", "#cloud-config\n", "") + got, ok := files["vendor-data"] + if !ok { + t.Fatalf("seed lacks vendor-data; files: %v", keys(files)) + } + if got != "" { + t.Errorf("vendor-data = %q, want empty", got) + } +} + +func TestSeedISO_rejectsEmptyName(t *testing.T) { + t.Parallel() + var buf bytes.Buffer + if err := buildSeedISO("", "#cloud-config\n", "", &buf); err == nil { + t.Fatal("buildSeedISO with empty name succeeded") + } + if buf.Len() != 0 { + t.Errorf("wrote %d bytes alongside the error", buf.Len()) + } +} + +func TestVendorData_isShellScriptNotCloudConfig(t *testing.T) { + t.Parallel() + got, err := buildVendorData("", true) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(got, "#!/bin/sh\n") { + t.Errorf("vendor-data starts with %q, want #!/bin/sh", firstLine(got)) + } + if strings.HasPrefix(got, "#cloud-config") { + t.Error("vendor-data is a cloud-config; cloud-init would merge it away") + } + for _, want := range []string{"qemu-guest-agent", "set -e", "apt-get", "dnf", "yum", "zypper", "apk", "systemctl enable --now qemu-guest-agent", "rc-update", "installGuestAgent: false", "DEBIAN_FRONTEND=noninteractive"} { + if !strings.Contains(got, want) { + t.Errorf("script lacks %q", want) + } + } + if strings.Contains(got, "MIME-Version") { + t.Error("script alone should not be wrapped in a multipart") + } +} + +func firstLine(s string) string { + line, _, _ := strings.Cut(s, "\n") + return line +} + +func bufioReader(s string) *bufio.Reader { + return bufio.NewReader(strings.NewReader(s)) +} + +func TestVendorData_multipartWhenExtraProvided(t *testing.T) { + t.Parallel() + configured := "#cloud-config\nntp:\n enabled: true\n" + got, err := buildVendorData(configured, true) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(got, "MIME-Version: 1.0\r\n") { + t.Errorf("vendor-data starts with %q, want MIME-Version: 1.0", firstLine(got)) + } + r := textproto.NewReader(bufioReader(got)) + hdr, err := r.ReadMIMEHeader() + if err != nil { + t.Fatalf("reading top-level headers: %v", err) + } + mediaType, params, err := mime.ParseMediaType(hdr.Get("Content-Type")) + if err != nil { + t.Fatalf("parsing Content-Type %q: %v", hdr.Get("Content-Type"), err) + } + if mediaType != "multipart/mixed" { + t.Errorf("media type = %q, want multipart/mixed", mediaType) + } + mr := multipart.NewReader(r.R, params["boundary"]) + type part struct{ contentType, body string } + var parts []part + for { + p, err := mr.NextPart() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("NextPart: %v", err) + } + body, err := io.ReadAll(p) + if err != nil { + t.Fatalf("reading part: %v", err) + } + parts = append(parts, part{p.Header.Get("Content-Type"), string(body)}) + } + want := []part{{"text/plain", configured}, {"text/x-shellscript", guestAgentScript}} + if len(parts) != len(want) { + t.Fatalf("got %d parts, want %d", len(parts), len(want)) + } + for i := range want { + if parts[i] != want[i] { + t.Errorf("part %d = {%q, %q}, want {%q, %q}", i, parts[i].contentType, firstLine(parts[i].body), want[i].contentType, firstLine(want[i].body)) + } + } +} + +func TestVendorData_offWithoutConfiguredIsEmpty(t *testing.T) { + t.Parallel() + got, err := buildVendorData("", false) + if err != nil { + t.Fatal(err) + } + if got != "" { + t.Errorf("vendor-data = %q, want empty", got) + } +} + +func TestVendorData_offWithConfiguredPassesThrough(t *testing.T) { + t.Parallel() + configured := "#cloud-config\nntp:\n enabled: true" + got, err := buildVendorData(configured, false) + if err != nil { + t.Fatal(err) + } + if got != configured { + t.Errorf("vendor-data = %q, want the configured document unchanged", got) + } +} diff --git a/internal/provider/kvm2/volume.go b/internal/provider/kvm2/volume.go new file mode 100644 index 0000000..ddeaf24 --- /dev/null +++ b/internal/provider/kvm2/volume.go @@ -0,0 +1,95 @@ +package kvm2 + +import ( + "crypto/sha256" + "encoding/hex" + "encoding/xml" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// Volume names in the host's pool (spec entries 4 and 10). The per-VM +// volumes derive from the domain name; the base image from its catalogue +// key or, for a URL image, from a hash of the URL. +func overlayVolumeName(name string) string { return name + ".qcow2" } +func seedVolumeName(name string) string { return name + "-seed.iso" } + +func baseVolumeName(key, format string) string { return "base-" + key + "." + format } +func markerVolumeName(baseName string) string { return baseName + ".sha256" } + +// urlImageKey stands in for the catalogue key of an image given as a URL: +// the first 16 hex characters of sha256(url). +func urlImageKey(url string) string { + sum := sha256.Sum256([]byte(url)) + return hex.EncodeToString(sum[:])[:16] +} + +// volumeSpec is what StorageVolCreateXML needs. BackingPath and +// BackingFormat are set for an overlay only. +type volumeSpec struct { + Name string + CapacityBytes int64 + Format string + BackingPath string + BackingFormat string +} + +type volumeXML struct { + XMLName xml.Name `xml:"volume"` + Type string `xml:"type,attr"` + Name string `xml:"name"` + Capacity volumeCapacityXML `xml:"capacity"` + Target volumeTargetXML `xml:"target"` + BackingStore *backingStoreXML `xml:"backingStore"` +} + +type volumeCapacityXML struct { + Unit string `xml:"unit,attr"` + Value int64 `xml:",chardata"` +} + +type volumeTargetXML struct { + Format volumeFormatXML `xml:"format"` +} + +type volumeFormatXML struct { + Type string `xml:"type,attr"` +} + +type backingStoreXML struct { + Path string `xml:"path"` + Format volumeFormatXML `xml:"format"` +} + +func validVolumeFormat(f string) bool { + return f == provider.KVMFormatQcow2 || f == provider.KVMFormatRaw +} + +func buildVolumeXML(v volumeSpec) (string, error) { + if v.Name == "" { + return "", permanentf("volume XML: name is empty") + } + if v.CapacityBytes <= 0 { + return "", permanentf("volume XML for %q: capacity must be positive, got %d", v.Name, v.CapacityBytes) + } + if !validVolumeFormat(v.Format) { + return "", permanentf("volume XML for %q: unknown format %q (want %s or %s)", v.Name, v.Format, provider.KVMFormatQcow2, provider.KVMFormatRaw) + } + x := volumeXML{ + Type: "file", + Name: v.Name, + Capacity: volumeCapacityXML{Unit: "bytes", Value: v.CapacityBytes}, + Target: volumeTargetXML{Format: volumeFormatXML{Type: v.Format}}, + } + if v.BackingPath != "" { + if !validVolumeFormat(v.BackingFormat) { + return "", permanentf("volume XML for %q: unknown backing format %q (want %s or %s)", v.Name, v.BackingFormat, provider.KVMFormatQcow2, provider.KVMFormatRaw) + } + x.BackingStore = &backingStoreXML{Path: v.BackingPath, Format: volumeFormatXML{Type: v.BackingFormat}} + } + out, err := xml.MarshalIndent(x, "", " ") + if err != nil { + return "", permanentf("volume XML for %q: %v", v.Name, err) + } + return string(out), nil +} diff --git a/internal/provider/kvm2/volume_test.go b/internal/provider/kvm2/volume_test.go new file mode 100644 index 0000000..01030dd --- /dev/null +++ b/internal/provider/kvm2/volume_test.go @@ -0,0 +1,152 @@ +package kvm2 + +import ( + "encoding/xml" + "errors" + "regexp" + "testing" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +type checkedVolume struct { + Type string `xml:"type,attr"` + Name string `xml:"name"` + Capacity struct { + Unit string `xml:"unit,attr"` + Value int64 `xml:",chardata"` + } `xml:"capacity"` + Target struct { + Format struct { + Type string `xml:"type,attr"` + } `xml:"format"` + } `xml:"target"` + BackingStores []struct { + Path string `xml:"path"` + Format struct { + Type string `xml:"type,attr"` + } `xml:"format"` + } `xml:"backingStore"` +} + +func mustBuildVolume(t *testing.T, v volumeSpec) checkedVolume { + t.Helper() + out, err := buildVolumeXML(v) + if err != nil { + t.Fatalf("buildVolumeXML: %v", err) + } + var c checkedVolume + if err := xml.Unmarshal([]byte(out), &c); err != nil { + t.Fatalf("unmarshalling produced XML: %v\n%s", err, out) + } + return c +} + +func TestVolumeXML_overlayBackingStore(t *testing.T) { + t.Parallel() + c := mustBuildVolume(t, volumeSpec{ + Name: "proxy-abc.qcow2", + CapacityBytes: 10 << 30, + Format: provider.KVMFormatQcow2, + BackingPath: "/var/lib/libvirt/images/base-debian-13.qcow2", + BackingFormat: provider.KVMFormatQcow2, + }) + if c.Type != "file" || c.Name != "proxy-abc.qcow2" { + t.Errorf("type=%q name=%q, want file/proxy-abc.qcow2", c.Type, c.Name) + } + if c.Capacity.Unit != "bytes" || c.Capacity.Value != 10<<30 { + t.Errorf("capacity = %d %s, want %d bytes", c.Capacity.Value, c.Capacity.Unit, 10<<30) + } + if c.Target.Format.Type != "qcow2" { + t.Errorf("target format = %q, want qcow2", c.Target.Format.Type) + } + if len(c.BackingStores) != 1 { + t.Fatalf("got %d backingStore elements, want 1", len(c.BackingStores)) + } + bs := c.BackingStores[0] + if bs.Path != "/var/lib/libvirt/images/base-debian-13.qcow2" || bs.Format.Type != "qcow2" { + t.Errorf("backingStore = %+v, want the base path with qcow2 format", bs) + } +} + +func TestVolumeXML_rawWithoutBacking(t *testing.T) { + t.Parallel() + c := mustBuildVolume(t, volumeSpec{Name: "proxy-abc-seed.iso", CapacityBytes: 385024, Format: provider.KVMFormatRaw}) + if c.Target.Format.Type != "raw" { + t.Errorf("target format = %q, want raw", c.Target.Format.Type) + } + if c.Capacity.Value != 385024 || c.Capacity.Unit != "bytes" { + t.Errorf("capacity = %d %s, want 385024 bytes", c.Capacity.Value, c.Capacity.Unit) + } + if len(c.BackingStores) != 0 { + t.Errorf("got backingStore %+v, want none", c.BackingStores) + } +} + +func TestVolumeXML_rejectsBadFormat(t *testing.T) { + t.Parallel() + tests := []struct { + name string + spec volumeSpec + }{ + {name: "vmdk format", spec: volumeSpec{Name: "x", CapacityBytes: 1, Format: "vmdk"}}, + {name: "empty format", spec: volumeSpec{Name: "x", CapacityBytes: 1}}, + {name: "bad backing format", spec: volumeSpec{Name: "x", CapacityBytes: 1, Format: "qcow2", BackingPath: "/b", BackingFormat: "vdi"}}, + {name: "backing path without format", spec: volumeSpec{Name: "x", CapacityBytes: 1, Format: "qcow2", BackingPath: "/b"}}, + {name: "empty name", spec: volumeSpec{CapacityBytes: 1, Format: "qcow2"}}, + {name: "zero capacity", spec: volumeSpec{Name: "x", Format: "qcow2"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + out, err := buildVolumeXML(tc.spec) + if !errors.Is(err, provider.ErrPermanent) { + t.Errorf("err = %v, want ErrPermanent", err) + } + if out != "" { + t.Errorf("got XML %q alongside the error", out) + } + }) + } +} + +func TestVolumeNames(t *testing.T) { + t.Parallel() + tests := []struct { + name string + got string + want string + }{ + {name: "overlay", got: overlayVolumeName("proxy-abc"), want: "proxy-abc.qcow2"}, + {name: "seed", got: seedVolumeName("proxy-abc"), want: "proxy-abc-seed.iso"}, + {name: "base qcow2", got: baseVolumeName("debian-13", provider.KVMFormatQcow2), want: "base-debian-13.qcow2"}, + {name: "base raw", got: baseVolumeName("alpine", provider.KVMFormatRaw), want: "base-alpine.raw"}, + {name: "marker", got: markerVolumeName("base-debian-13.qcow2"), want: "base-debian-13.qcow2.sha256"}, + {name: "marker of url base", got: markerVolumeName(baseVolumeName(urlImageKey("https://x/y.img"), "qcow2")), want: "base-" + urlImageKey("https://x/y.img") + ".qcow2.sha256"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if tc.got != tc.want { + t.Errorf("got %q, want %q", tc.got, tc.want) + } + }) + } + + t.Run("url key", func(t *testing.T) { + t.Parallel() + hex16 := regexp.MustCompile(`^[0-9a-f]{16}$`) + a := urlImageKey("https://cloud.debian.org/images/cloud/trixie/latest/debian-13-generic-amd64.qcow2") + b := urlImageKey("https://cloud.debian.org/images/cloud/trixie/latest/debian-13-generic-amd64.qcow2") + c := urlImageKey("https://cloud.debian.org/images/cloud/bookworm/latest/debian-12-generic-amd64.qcow2") + if !hex16.MatchString(a) { + t.Errorf("urlImageKey = %q, want 16 lowercase hex chars", a) + } + if a != b { + t.Errorf("urlImageKey not deterministic: %q vs %q", a, b) + } + if a == c { + t.Errorf("urlImageKey gives %q for two different URLs", a) + } + }) +} diff --git a/internal/provider/kvm2/xml.go b/internal/provider/kvm2/xml.go new file mode 100644 index 0000000..ad6a1f9 --- /dev/null +++ b/internal/provider/kvm2/xml.go @@ -0,0 +1,256 @@ +package kvm2 + +import ( + "encoding/xml" + "time" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// domainParams is everything buildDomainXML needs, already resolved: the +// paths are what StorageVolGetPath returned for the overlay and the seed. +type domainParams struct { + Name string + UID string + CreatedAt time.Time + VCPUs int32 + MemoryMiB int64 + Firmware string + Bridge string + DiskPath string + SeedPath string + DiskFormat string +} + +const ( + domainArch = "x86_64" + domainMachine = "q35" + guestAgentChannelName = "org.qemu.guest_agent.0" +) + +type domainXML struct { + XMLName xml.Name `xml:"domain"` + Type string `xml:"type,attr"` + Name string `xml:"name"` + Memory domainMemoryXML `xml:"memory"` + VCPU int32 `xml:"vcpu"` + Metadata domainMetadataXML `xml:"metadata"` + OS domainOSXML `xml:"os"` + Features domainFeaturesXML `xml:"features"` + CPU domainCPUXML `xml:"cpu"` + OnPoweroff string `xml:"on_poweroff"` + OnReboot string `xml:"on_reboot"` + OnCrash string `xml:"on_crash"` + Devices domainDevicesXML `xml:"devices"` +} + +type domainMemoryXML struct { + Unit string `xml:"unit,attr"` + Value int64 `xml:",chardata"` +} + +type domainMetadataXML struct { + Instance instanceMetadata `xml:"instance"` +} + +type domainOSXML struct { + // Firmware is the attribute firmware="efi"; FirmwareFeatures is the + // child that turns secure boot off. Both are absent for BIOS. + Firmware string `xml:"firmware,attr,omitempty"` + Type domainOSTypeXML `xml:"type"` + FirmwareFeatures *domainFirmwareXML `xml:"firmware"` +} + +type domainOSTypeXML struct { + Arch string `xml:"arch,attr"` + Machine string `xml:"machine,attr"` + Value string `xml:",chardata"` +} + +type domainFirmwareXML struct { + Features []domainFirmwareFeatureXML `xml:"feature"` +} + +type domainFirmwareFeatureXML struct { + Enabled string `xml:"enabled,attr"` + Name string `xml:"name,attr"` +} + +type domainFeaturesXML struct { + ACPI *struct{} `xml:"acpi"` + APIC *struct{} `xml:"apic"` +} + +type domainCPUXML struct { + Mode string `xml:"mode,attr"` +} + +type domainDevicesXML struct { + Disks []domainDiskXML `xml:"disk"` + Interfaces []domainInterfaceXML `xml:"interface"` + Serials []domainSerialXML `xml:"serial"` + Consoles []domainConsoleXML `xml:"console"` + Channels []domainChannelXML `xml:"channel"` + RNGs []domainRNGXML `xml:"rng"` +} + +type domainDiskXML struct { + Type string `xml:"type,attr"` + Device string `xml:"device,attr"` + Driver domainDiskDriverXML `xml:"driver"` + Source domainDiskSourceXML `xml:"source"` + Target domainDiskTargetXML `xml:"target"` + ReadOnly *struct{} `xml:"readonly"` +} + +type domainDiskDriverXML struct { + Name string `xml:"name,attr"` + Type string `xml:"type,attr"` +} + +type domainDiskSourceXML struct { + File string `xml:"file,attr"` +} + +type domainDiskTargetXML struct { + Dev string `xml:"dev,attr"` + Bus string `xml:"bus,attr"` +} + +type domainInterfaceXML struct { + Type string `xml:"type,attr"` + Source domainInterfaceSourceXML `xml:"source"` + Model domainModelXML `xml:"model"` +} + +type domainInterfaceSourceXML struct { + Bridge string `xml:"bridge,attr"` +} + +type domainModelXML struct { + Type string `xml:"type,attr"` +} + +type domainSerialXML struct { + Type string `xml:"type,attr"` + Target domainPortXML `xml:"target"` +} + +type domainPortXML struct { + Port int `xml:"port,attr"` +} + +type domainConsoleXML struct { + Type string `xml:"type,attr"` + Target domainConsoleTargetXML `xml:"target"` +} + +type domainConsoleTargetXML struct { + Type string `xml:"type,attr"` + Port int `xml:"port,attr"` +} + +type domainChannelXML struct { + Type string `xml:"type,attr"` + Target domainChannelTargetXML `xml:"target"` +} + +type domainChannelTargetXML struct { + Type string `xml:"type,attr"` + Name string `xml:"name,attr"` +} + +type domainRNGXML struct { + Model string `xml:"model,attr"` + Backend domainRNGBackendXML `xml:"backend"` +} + +type domainRNGBackendXML struct { + Model string `xml:"model,attr"` + Value string `xml:",chardata"` +} + +// buildDomainXML renders the XML for DomainDefineXML. Both disks are +// type="file" with : AppArmor derives QEMU's profile from those +// paths, and type="volume" fails with permission denied on stock +// Debian/Ubuntu hosts. UEFI boots with secure boot off; libvirt picks the +// OVMF binary itself. +func buildDomainXML(p domainParams) (string, error) { + if p.Name == "" { + return "", permanentf("domain XML: name is empty") + } + if p.DiskPath == "" { + return "", permanentf("domain XML for %q: disk path is empty", p.Name) + } + if p.SeedPath == "" { + return "", permanentf("domain XML for %q: seed path is empty", p.Name) + } + if p.DiskFormat == "" { + p.DiskFormat = provider.KVMFormatQcow2 + } + if p.DiskFormat != provider.KVMFormatQcow2 && p.DiskFormat != provider.KVMFormatRaw { + return "", permanentf("domain XML for %q: unknown disk format %q", p.Name, p.DiskFormat) + } + osx := domainOSXML{ + Type: domainOSTypeXML{Arch: domainArch, Machine: domainMachine, Value: "hvm"}, + } + switch p.Firmware { + case provider.KVMFirmwareUEFI: + osx.Firmware = "efi" + osx.FirmwareFeatures = &domainFirmwareXML{ + Features: []domainFirmwareFeatureXML{{Enabled: "no", Name: "secure-boot"}}, + } + case provider.KVMFirmwareBIOS: + default: + return "", permanentf("domain XML for %q: unknown firmware %q (want %s or %s)", p.Name, p.Firmware, provider.KVMFirmwareUEFI, provider.KVMFirmwareBIOS) + } + d := domainXML{ + Type: "kvm", + Name: p.Name, + Memory: domainMemoryXML{Unit: "MiB", Value: p.MemoryMiB}, + VCPU: p.VCPUs, + Metadata: domainMetadataXML{Instance: newInstanceMetadata(p.UID, p.CreatedAt)}, + OS: osx, + Features: domainFeaturesXML{ACPI: &struct{}{}, APIC: &struct{}{}}, + CPU: domainCPUXML{Mode: "host-passthrough"}, + OnPoweroff: "destroy", + OnReboot: "restart", + OnCrash: "destroy", + Devices: domainDevicesXML{ + Disks: []domainDiskXML{ + { + Type: "file", + Device: "disk", + Driver: domainDiskDriverXML{Name: "qemu", Type: p.DiskFormat}, + Source: domainDiskSourceXML{File: p.DiskPath}, + Target: domainDiskTargetXML{Dev: "vda", Bus: "virtio"}, + }, + { + Type: "file", + Device: "cdrom", + Driver: domainDiskDriverXML{Name: "qemu", Type: provider.KVMFormatRaw}, + Source: domainDiskSourceXML{File: p.SeedPath}, + Target: domainDiskTargetXML{Dev: "sda", Bus: "sata"}, + ReadOnly: &struct{}{}, + }, + }, + Interfaces: []domainInterfaceXML{{ + Type: "bridge", + Source: domainInterfaceSourceXML{Bridge: p.Bridge}, + Model: domainModelXML{Type: "virtio"}, + }}, + Serials: []domainSerialXML{{Type: "pty", Target: domainPortXML{Port: 0}}}, + Consoles: []domainConsoleXML{{Type: "pty", Target: domainConsoleTargetXML{Type: "serial", Port: 0}}}, + Channels: []domainChannelXML{{ + Type: "unix", + Target: domainChannelTargetXML{Type: "virtio", Name: guestAgentChannelName}, + }}, + RNGs: []domainRNGXML{{Model: "virtio", Backend: domainRNGBackendXML{Model: "random", Value: "/dev/urandom"}}}, + }, + } + out, err := xml.MarshalIndent(d, "", " ") + if err != nil { + return "", permanentf("domain XML for %q: %v", p.Name, err) + } + return string(out), nil +} diff --git a/internal/provider/kvm2/xml_test.go b/internal/provider/kvm2/xml_test.go new file mode 100644 index 0000000..56b67e2 --- /dev/null +++ b/internal/provider/kvm2/xml_test.go @@ -0,0 +1,309 @@ +package kvm2 + +import ( + "encoding/xml" + "errors" + "strings" + "testing" + "time" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// checkedDomain is an independent view of the marshalled XML, so the tests +// assert on what libvirt would see rather than on the builder's own structs. +type checkedDomain struct { + Type string `xml:"type,attr"` + Name string `xml:"name"` + Memory struct { + Unit string `xml:"unit,attr"` + Value int64 `xml:",chardata"` + } `xml:"memory"` + VCPU int32 `xml:"vcpu"` + Metadata struct { + Inner string `xml:",innerxml"` + } `xml:"metadata"` + OS struct { + Firmware string `xml:"firmware,attr"` + Type struct { + Arch string `xml:"arch,attr"` + Machine string `xml:"machine,attr"` + Value string `xml:",chardata"` + } `xml:"type"` + FirmwareElems []struct { + Features []struct { + Enabled string `xml:"enabled,attr"` + Name string `xml:"name,attr"` + } `xml:"feature"` + } `xml:"firmware"` + } `xml:"os"` + Features struct { + ACPI []struct{} `xml:"acpi"` + APIC []struct{} `xml:"apic"` + } `xml:"features"` + CPU struct { + Mode string `xml:"mode,attr"` + } `xml:"cpu"` + OnPoweroff string `xml:"on_poweroff"` + OnReboot string `xml:"on_reboot"` + OnCrash string `xml:"on_crash"` + Devices struct { + Disks []struct { + Type string `xml:"type,attr"` + Device string `xml:"device,attr"` + Driver struct { + Name string `xml:"name,attr"` + Type string `xml:"type,attr"` + } `xml:"driver"` + Source struct { + File string `xml:"file,attr"` + Pool string `xml:"pool,attr"` + Vol string `xml:"volume,attr"` + } `xml:"source"` + Target struct { + Dev string `xml:"dev,attr"` + Bus string `xml:"bus,attr"` + } `xml:"target"` + ReadOnly []struct{} `xml:"readonly"` + } `xml:"disk"` + Interfaces []struct { + Type string `xml:"type,attr"` + Source struct { + Bridge string `xml:"bridge,attr"` + } `xml:"source"` + Model struct { + Type string `xml:"type,attr"` + } `xml:"model"` + } `xml:"interface"` + Serials []struct { + Type string `xml:"type,attr"` + } `xml:"serial"` + Consoles []struct { + Type string `xml:"type,attr"` + Target struct { + Type string `xml:"type,attr"` + } `xml:"target"` + } `xml:"console"` + Channels []struct { + Type string `xml:"type,attr"` + Target struct { + Type string `xml:"type,attr"` + Name string `xml:"name,attr"` + } `xml:"target"` + } `xml:"channel"` + RNGs []struct { + Model string `xml:"model,attr"` + Backend struct { + Model string `xml:"model,attr"` + Value string `xml:",chardata"` + } `xml:"backend"` + } `xml:"rng"` + Graphics []struct{} `xml:"graphics"` + } `xml:"devices"` +} + +var testCreatedAt = time.Date(2026, 9, 16, 12, 0, 0, 0, time.UTC) + +func testDomainParams() domainParams { + return domainParams{ + Name: "proxy-abc", + UID: "0b1f9a2e-uid", + CreatedAt: testCreatedAt, + VCPUs: 2, + MemoryMiB: 2048, + Firmware: provider.KVMFirmwareUEFI, + Bridge: "br0", + DiskPath: "/var/lib/libvirt/images/proxy-abc.qcow2", + SeedPath: "/var/lib/libvirt/images/proxy-abc-seed.iso", + DiskFormat: provider.KVMFormatQcow2, + } +} + +func mustBuildDomain(t *testing.T, p domainParams) (string, checkedDomain) { + t.Helper() + out, err := buildDomainXML(p) + if err != nil { + t.Fatalf("buildDomainXML: %v", err) + } + var d checkedDomain + if err := xml.Unmarshal([]byte(out), &d); err != nil { + t.Fatalf("unmarshalling produced XML: %v\n%s", err, out) + } + return out, d +} + +func TestDomainXML_fileDisksWithResolvedPaths(t *testing.T) { + t.Parallel() + p := testDomainParams() + _, d := mustBuildDomain(t, p) + if d.Type != "kvm" || d.Name != p.Name { + t.Errorf("domain type=%q name=%q, want kvm/%s", d.Type, d.Name, p.Name) + } + if d.Memory.Unit != "MiB" || d.Memory.Value != p.MemoryMiB || d.VCPU != p.VCPUs { + t.Errorf("memory=%d %s vcpu=%d, want %d MiB / %d", d.Memory.Value, d.Memory.Unit, d.VCPU, p.MemoryMiB, p.VCPUs) + } + if len(d.Devices.Disks) != 2 { + t.Fatalf("got %d disks, want 2", len(d.Devices.Disks)) + } + overlay, seed := d.Devices.Disks[0], d.Devices.Disks[1] + if overlay.Type != "file" || overlay.Device != "disk" { + t.Errorf("overlay type=%q device=%q, want file/disk", overlay.Type, overlay.Device) + } + if overlay.Source.File != p.DiskPath || overlay.Source.Pool != "" || overlay.Source.Vol != "" { + t.Errorf("overlay source = %+v, want file=%q only", overlay.Source, p.DiskPath) + } + if overlay.Driver.Name != "qemu" || overlay.Driver.Type != "qcow2" { + t.Errorf("overlay driver = %+v, want qemu/qcow2", overlay.Driver) + } + if overlay.Target.Dev != "vda" || overlay.Target.Bus != "virtio" { + t.Errorf("overlay target = %+v, want vda/virtio", overlay.Target) + } + if len(overlay.ReadOnly) != 0 { + t.Error("overlay is readonly") + } + if seed.Type != "file" || seed.Device != "cdrom" { + t.Errorf("seed type=%q device=%q, want file/cdrom", seed.Type, seed.Device) + } + if seed.Source.File != p.SeedPath || seed.Source.Pool != "" || seed.Source.Vol != "" { + t.Errorf("seed source = %+v, want file=%q only", seed.Source, p.SeedPath) + } + if seed.Driver.Name != "qemu" || seed.Driver.Type != "raw" { + t.Errorf("seed driver = %+v, want qemu/raw", seed.Driver) + } + if seed.Target.Dev != "sda" || seed.Target.Bus != "sata" { + t.Errorf("seed target = %+v, want sda/sata", seed.Target) + } + if len(seed.ReadOnly) != 1 { + t.Error("seed is not readonly") + } + if d.OnPoweroff != "destroy" || d.OnReboot != "restart" || d.OnCrash != "destroy" { + t.Errorf("lifecycle actions = %q/%q/%q, want destroy/restart/destroy", d.OnPoweroff, d.OnReboot, d.OnCrash) + } +} + +func TestDomainXML_uefiDefaultSecureBootOff(t *testing.T) { + t.Parallel() + _, d := mustBuildDomain(t, testDomainParams()) + if d.OS.Firmware != "efi" { + t.Errorf(` = %q, want "efi"`, d.OS.Firmware) + } + if len(d.OS.FirmwareElems) != 1 { + t.Fatalf("got %d elements, want 1", len(d.OS.FirmwareElems)) + } + feats := d.OS.FirmwareElems[0].Features + if len(feats) != 1 || feats[0].Name != "secure-boot" || feats[0].Enabled != "no" { + t.Errorf("firmware features = %+v, want one secure-boot enabled=no", feats) + } + if d.OS.Type.Value != "hvm" { + t.Errorf("os type = %q, want hvm", d.OS.Type.Value) + } +} + +func TestDomainXML_biosEscape(t *testing.T) { + t.Parallel() + p := testDomainParams() + p.Firmware = provider.KVMFirmwareBIOS + out, d := mustBuildDomain(t, p) + if d.OS.Firmware != "" { + t.Errorf(` = %q, want absent`, d.OS.Firmware) + } + if len(d.OS.FirmwareElems) != 0 { + t.Errorf("got %d elements, want none", len(d.OS.FirmwareElems)) + } + if strings.Contains(out, "firmware") { + t.Errorf("BIOS domain XML still mentions firmware:\n%s", out) + } + if d.OS.Type.Value != "hvm" || d.OS.Type.Machine != "q35" { + t.Errorf("os type = %+v, want hvm on q35", d.OS.Type) + } +} + +func TestDomainXML_metadataElement(t *testing.T) { + t.Parallel() + p := testDomainParams() + out, d := mustBuildDomain(t, p) + if !strings.Contains(out, ``) { + t.Errorf("domain XML lacks the namespaced instance element:\n%s", out) + } + m, err := parseInstanceMetadata(d.Metadata.Inner) + if err != nil { + t.Fatalf("parseInstanceMetadata(%q): %v", d.Metadata.Inner, err) + } + if !m.managed() { + t.Error("managed() = false, want true") + } + if m.UID != p.UID { + t.Errorf("UID = %q, want %q", m.UID, p.UID) + } + if m.CreatedAt != "2026-09-16T12:00:00Z" { + t.Errorf("CreatedAt = %q, want RFC3339 UTC", m.CreatedAt) + } + if !m.createdAt().Equal(p.CreatedAt) { + t.Errorf("createdAt() = %v, want %v", m.createdAt(), p.CreatedAt) + } +} + +func TestDomainXML_guestAgentChannel(t *testing.T) { + t.Parallel() + p := testDomainParams() + _, d := mustBuildDomain(t, p) + if len(d.Devices.Channels) != 1 || d.Devices.Channels[0].Type != "unix" || + d.Devices.Channels[0].Target.Type != "virtio" || d.Devices.Channels[0].Target.Name != "org.qemu.guest_agent.0" { + t.Errorf("channels = %+v, want one unix/virtio org.qemu.guest_agent.0", d.Devices.Channels) + } + if len(d.Devices.Interfaces) != 1 || d.Devices.Interfaces[0].Type != "bridge" || + d.Devices.Interfaces[0].Source.Bridge != p.Bridge || d.Devices.Interfaces[0].Model.Type != "virtio" { + t.Errorf("interfaces = %+v, want one bridge %s with virtio model", d.Devices.Interfaces, p.Bridge) + } + if len(d.Devices.Serials) != 1 || d.Devices.Serials[0].Type != "pty" { + t.Errorf("serials = %+v, want one pty", d.Devices.Serials) + } + if len(d.Devices.Consoles) != 1 || d.Devices.Consoles[0].Type != "pty" || d.Devices.Consoles[0].Target.Type != "serial" { + t.Errorf("consoles = %+v, want one pty/serial", d.Devices.Consoles) + } + if len(d.Devices.RNGs) != 1 || d.Devices.RNGs[0].Model != "virtio" || + d.Devices.RNGs[0].Backend.Model != "random" || d.Devices.RNGs[0].Backend.Value != "/dev/urandom" { + t.Errorf("rngs = %+v, want one virtio random /dev/urandom", d.Devices.RNGs) + } + if len(d.Devices.Graphics) != 0 { + t.Error("domain has , want none") + } + if d.OS.Type.Arch != "x86_64" || d.OS.Type.Machine != "q35" { + t.Errorf("os type = %+v, want x86_64/q35", d.OS.Type) + } + if d.CPU.Mode != "host-passthrough" { + t.Errorf("cpu mode = %q, want host-passthrough", d.CPU.Mode) + } + if len(d.Features.ACPI) != 1 || len(d.Features.APIC) != 1 { + t.Errorf("features acpi=%d apic=%d, want one each", len(d.Features.ACPI), len(d.Features.APIC)) + } +} + +func TestDomainXML_rejectsBadInput(t *testing.T) { + t.Parallel() + tests := []struct { + name string + mutate func(*domainParams) + }{ + {name: "unknown firmware", mutate: func(p *domainParams) { p.Firmware = "coreboot" }}, + {name: "empty firmware", mutate: func(p *domainParams) { p.Firmware = "" }}, + {name: "empty name", mutate: func(p *domainParams) { p.Name = "" }}, + {name: "empty disk path", mutate: func(p *domainParams) { p.DiskPath = "" }}, + {name: "empty seed path", mutate: func(p *domainParams) { p.SeedPath = "" }}, + {name: "unknown disk format", mutate: func(p *domainParams) { p.DiskFormat = "vmdk" }}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + p := testDomainParams() + tc.mutate(&p) + out, err := buildDomainXML(p) + if !errors.Is(err, provider.ErrPermanent) { + t.Errorf("err = %v, want ErrPermanent", err) + } + if out != "" { + t.Errorf("got XML %q alongside the error", out) + } + }) + } +}