From 84dce0462eb13c1c0bce828a556fa04b9fc7bbb9 Mon Sep 17 00:00:00 2001 From: kacerr Date: Fri, 4 Sep 2026 14:25:06 +0000 Subject: [PATCH] Add KVM provider config: KVMConfig structs and validation Adds the kvm block to the providers config (SSH access, images map, machine-type profiles, hosts list) with validation and table tests. validate() now rejects mismatched type-specific blocks generically via setBlocks() instead of pairwise checks. Step 1 of docs/plans/2026-09-04-1404-kvm-provider.md. Co-Authored-By: Claude Fable 5.1 --- .claude/settings.json | 8 +- .../2026-09-04-1404-kvm-provider.md | 54 +++++ internal/provider/config.go | 49 +++-- internal/provider/config_kvm.go | 138 ++++++++++++ internal/provider/config_kvm_test.go | 208 ++++++++++++++++++ 5 files changed, 443 insertions(+), 14 deletions(-) create mode 100644 docs/plans-executions/2026-09-04-1404-kvm-provider.md create mode 100644 internal/provider/config_kvm.go create mode 100644 internal/provider/config_kvm_test.go diff --git a/.claude/settings.json b/.claude/settings.json index ec2dbda..e95a782 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -93,7 +93,13 @@ "Bash(git -C /Users/jan.novak/srv/go/egress-proxies-operator tag 0.01)", "Bash(git -C /Users/jan.novak/srv/go/egress-proxies-operator push origin 0.01)", "Bash(chmod +x docs/demo/run-demo.sh)", - "Bash(bash -n docs/demo/run-demo.sh)" + "Bash(bash -n docs/demo/run-demo.sh)", + "Bash(git add *)", + "Bash(git commit -q -m 'Add plan: KVM/libvirt provider *)", + "Bash(go get *)", + "Bash(go build *)", + "Bash(go test *)", + "Bash(git commit *)" ], "additionalDirectories": [ "/Users/jan.novak/srv/go/egress-proxies-operator/.claude", diff --git a/docs/plans-executions/2026-09-04-1404-kvm-provider.md b/docs/plans-executions/2026-09-04-1404-kvm-provider.md new file mode 100644 index 0000000..656015c --- /dev/null +++ b/docs/plans-executions/2026-09-04-1404-kvm-provider.md @@ -0,0 +1,54 @@ +# Execution: KVM/libvirt provider + +Plan: [2026-09-04-1404-kvm-provider.md](../plans/2026-09-04-1404-kvm-provider.md) + +- [x] Step 0 — Save plan + branch `feat/kvm-provider` +- [x] Step 1 — Config (`KVMConfig` & friends, `validate()` case, tests) +- [ ] Step 2 — Pure builders (`domainxml.go`, `volume.go`, `seed.go`, `id.go`, `metadata.go`) +- [ ] Step 3 — Transport + errors (`conn.go`, `errors.go`) +- [ ] Step 3b — Image cache (`image.go`) +- [ ] Step 4 — Provider methods (`kvm.go`) over a fake API +- [ ] Step 5 — Wiring, manifests, samples, docs, CHANGELOG +- [ ] Step 6 — Integration test against a real hypervisor +- [ ] Step 7 — Manual validation + MR + +## Step 0 — Save plan + branch + +Plan committed alone on `feat/proxy-operator`, then branched: + +```bash +git checkout -b feat/kvm-provider +``` + +## Step 1 — Config + +The new structs (`KVMConfig`, `KVMHost`, `KVMMachineType`, `KVMImage`) and +`validateKVM` went into their own file, `internal/provider/config_kvm.go`, +rather than being appended to `config.go` as the plan said: the KVM block has +far more fields and rules than the other two providers, and keeping it +separate leaves `config.go` readable. Tests likewise live in +`config_kvm_test.go` instead of extending the table in `config_test.go`; the +invalid-config table covers every required field, duplicate host names, image +URL scheme, sha256 shape, and the disk format enum. + +While adding the `kvm` case to `validate()`, the pairwise "type is X but a Y +block is set" checks (which would have needed six branches with three types) +were replaced by a `setBlocks()` helper that lists the non-nil blocks and +rejects any that don't match `type`. The error strings are unchanged, so the +existing GCP/kubernetes tests still pass. + +Dependencies were fetched up front so the API signatures in the plan could be +checked with `go doc`; they stay `// indirect` in `go.mod` until Step 2 imports +them: + +```bash +go get github.com/digitalocean/go-libvirt@latest github.com/kdomanski/iso9660@v0.4.0 +# go-libvirt resolved to v0.0.0-20260814190004-1a83157e1858 (no tagged release) +# side effect: golang.org/x/crypto 0.54.0 → 0.56.0, x/mod, x/text, x/tools bumped +``` + +Worth noting: `go doc` showed that go-libvirt's `New(net.Conn)` is deprecated +in favour of `NewWithDialer(socket.Dialer)`, where `Dialer` is a one-method +`Dial() (net.Conn, error)` interface. Step 3 should wrap the SSH client's unix +forward in that interface instead of using `New`; the plan's transport design +is otherwise unaffected. diff --git a/internal/provider/config.go b/internal/provider/config.go index 6a4d4b7..f9b81d8 100644 --- a/internal/provider/config.go +++ b/internal/provider/config.go @@ -21,6 +21,23 @@ type ProviderConfig struct { Type string `json:"type"` Kubernetes *KubernetesConfig `json:"kubernetes,omitempty"` GCP *GCPConfig `json:"gcp,omitempty"` + KVM *KVMConfig `json:"kvm,omitempty"` +} + +// setBlocks names the type-specific blocks that are non-nil, so validate can +// reject a block that doesn't match Type without a pairwise check per type. +func (p ProviderConfig) setBlocks() []string { + var out []string + if p.Kubernetes != nil { + out = append(out, "kubernetes") + } + if p.GCP != nil { + out = append(out, "gcp") + } + if p.KVM != nil { + out = append(out, "kvm") + } + return out } // KubernetesConfig configures the kubernetes-pod provider, which creates @@ -81,22 +98,28 @@ func (c *Config) validate() error { } seen[p.Name] = true + prefix := fmt.Sprintf("providers[%d] %q", i, p.Name) switch p.Type { - case "kubernetes": - if p.GCP != nil { - return fmt.Errorf("providers[%d] %q: type is kubernetes but a gcp block is set", i, p.Name) - } - case "gcp": - if p.Kubernetes != nil { - return fmt.Errorf("providers[%d] %q: type is gcp but a kubernetes block is set", i, p.Name) - } - if p.GCP == nil || p.GCP.Project == "" { - return fmt.Errorf("providers[%d] %q: gcp.project is required", i, p.Name) - } + case "kubernetes", "gcp", "kvm": case "": - return fmt.Errorf("providers[%d] %q: type is required", i, p.Name) + return fmt.Errorf("%s: type is required", prefix) default: - return fmt.Errorf("providers[%d] %q: unknown provider type %q", i, p.Name, p.Type) + return fmt.Errorf("%s: unknown provider type %q", prefix, p.Type) + } + for _, block := range p.setBlocks() { + if block != p.Type { + return fmt.Errorf("%s: type is %s but a %s block is set", prefix, p.Type, block) + } + } + switch p.Type { + case "gcp": + if p.GCP == nil || p.GCP.Project == "" { + return fmt.Errorf("%s: gcp.project is required", prefix) + } + case "kvm": + if err := validateKVM(prefix, p.KVM); err != nil { + return err + } } } return nil diff --git a/internal/provider/config_kvm.go b/internal/provider/config_kvm.go new file mode 100644 index 0000000..606d643 --- /dev/null +++ b/internal/provider/config_kvm.go @@ -0,0 +1,138 @@ +package provider + +import ( + "fmt" + "net/url" + "regexp" +) + +// KVMConfig configures a kvm provider instance: one or more libvirt/KVM +// hypervisors reached over SSH (the libvirt unix socket is forwarded through +// the SSH connection, so hosts need no libvirtd TCP/TLS listener). +// placement.zone on a Proxy selects a host by name; with a single host it may +// be omitted. +type KVMConfig struct { + // SSHKeyFile is the path to the private key used for every host. Required. + SSHKeyFile string `json:"sshKeyFile"` + // KnownHostsFile is an OpenSSH known_hosts file used to verify host keys. + // Required unless InsecureIgnoreHostKey is set. + KnownHostsFile string `json:"knownHostsFile,omitempty"` + // InsecureIgnoreHostKey skips host-key verification entirely. + InsecureIgnoreHostKey bool `json:"insecureIgnoreHostKey,omitempty"` + // DiskSizeGB is the capacity of each VM's overlay disk. Default 10. The + // guest's cloud-init growpart expands the root filesystem into it. + DiskSizeGB int64 `json:"diskSizeGb,omitempty"` + // VendorData is cloud-init vendor-data placed on every seed next to the + // Proxy's own user-data. Default: installs and starts qemu-guest-agent so + // the provider can read the VM's IP even before it shows up in ARP. + VendorData string `json:"vendorData,omitempty"` + // Images are named base images; placement.image names a key here (or is + // an http(s) URL directly). The provider downloads each image into a + // host's storage pool on first use and caches it there. + Images map[string]KVMImage `json:"images,omitempty"` + // MachineTypes are named size profiles; placement.machineType names a + // key here. Required, at least one. + MachineTypes map[string]KVMMachineType `json:"machineTypes"` + // Hosts are the hypervisors. Required, at least one, unique names. + Hosts []KVMHost `json:"hosts"` +} + +// KVMImage is one downloadable base image (a distro cloud image with +// cloud-init preinstalled, e.g. Debian's generic qcow2). +type KVMImage struct { + // URL is where to fetch the image from. Required, http or https. + URL string `json:"url"` + // SHA256 is the expected hex digest of the file. Optional; when set it is + // verified after download and a later change triggers a re-download. + SHA256 string `json:"sha256,omitempty"` + // Format is the image's disk format, "qcow2" (default) or "raw". + Format string `json:"format,omitempty"` +} + +// KVMMachineType is a VM size profile. +type KVMMachineType struct { + VCPUs int32 `json:"vcpus"` + MemoryMiB int64 `json:"memoryMiB"` +} + +// KVMHost is one hypervisor. +type KVMHost struct { + // Name is the placement.zone value that selects this host. Required. + Name string `json:"name"` + // Address is the SSH endpoint, host:port. Required. + Address string `json:"address"` + // User is the SSH user; it must be allowed to use the libvirt socket + // (typically a member of the libvirt group). Required. + User string `json:"user"` + // Socket is the libvirt unix socket path on the host. + // Default /var/run/libvirt/libvirt-sock. + Socket string `json:"socket,omitempty"` + // Pool is the libvirt storage pool for base images, overlay disks and + // seed ISOs. Default "default". + Pool string `json:"pool,omitempty"` + // Bridge is the host bridge the VM's NIC attaches to; it must lead to a + // network the cluster can reach and that hands out DHCP leases. Required. + Bridge string `json:"bridge"` +} + +var sha256Hex = regexp.MustCompile(`^[0-9a-fA-F]{64}$`) + +func validateKVM(prefix string, c *KVMConfig) error { + if c == nil { + return fmt.Errorf("%s: kvm block is required", prefix) + } + if c.SSHKeyFile == "" { + return fmt.Errorf("%s: kvm.sshKeyFile is required", prefix) + } + if c.KnownHostsFile == "" && !c.InsecureIgnoreHostKey { + return fmt.Errorf("%s: kvm.knownHostsFile is required unless kvm.insecureIgnoreHostKey is true", prefix) + } + if c.DiskSizeGB < 0 { + return fmt.Errorf("%s: kvm.diskSizeGb must not be negative", prefix) + } + for name, img := range c.Images { + p := fmt.Sprintf("%s: kvm.images[%q]", prefix, name) + u, err := url.Parse(img.URL) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" { + return fmt.Errorf("%s.url must be an http(s) URL (got %q)", p, img.URL) + } + if img.SHA256 != "" && !sha256Hex.MatchString(img.SHA256) { + return fmt.Errorf("%s.sha256 must be 64 hex characters", p) + } + if img.Format != "" && img.Format != "qcow2" && img.Format != "raw" { + return fmt.Errorf("%s.format must be qcow2 or raw (got %q)", p, img.Format) + } + } + if len(c.MachineTypes) == 0 { + return fmt.Errorf("%s: kvm.machineTypes must define at least one profile", prefix) + } + for name, mt := range c.MachineTypes { + if mt.VCPUs <= 0 || mt.MemoryMiB <= 0 { + return fmt.Errorf("%s: kvm.machineTypes[%q] needs vcpus > 0 and memoryMiB > 0", prefix, name) + } + } + if len(c.Hosts) == 0 { + return fmt.Errorf("%s: kvm.hosts must list at least one host", prefix) + } + seen := make(map[string]bool, len(c.Hosts)) + for i, h := range c.Hosts { + p := fmt.Sprintf("%s: kvm.hosts[%d]", prefix, i) + if h.Name == "" { + return fmt.Errorf("%s: name is required", p) + } + if seen[h.Name] { + return fmt.Errorf("%s: duplicate host name %q", p, h.Name) + } + seen[h.Name] = true + if h.Address == "" { + return fmt.Errorf("%s %q: address is required", p, h.Name) + } + if h.User == "" { + return fmt.Errorf("%s %q: user is required", p, h.Name) + } + if h.Bridge == "" { + return fmt.Errorf("%s %q: bridge is required", p, h.Name) + } + } + return nil +} diff --git a/internal/provider/config_kvm_test.go b/internal/provider/config_kvm_test.go new file mode 100644 index 0000000..6a00afb --- /dev/null +++ b/internal/provider/config_kvm_test.go @@ -0,0 +1,208 @@ +package provider + +import ( + "strings" + "testing" +) + +const validKVMYAML = ` +providers: + - name: kvm-home + type: kvm + kvm: + sshKeyFile: /etc/proxy-operator/kvm/id_ed25519 + knownHostsFile: /etc/proxy-operator/kvm/known_hosts + diskSizeGb: 20 + images: + debian-13: + url: https://cloud.debian.org/images/cloud/trixie/latest/debian-13-generic-amd64.qcow2 + sha256: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef + ubuntu: + url: http://mirror.local/noble.img + format: raw + machineTypes: + small: {vcpus: 1, memoryMiB: 1024} + medium: {vcpus: 2, memoryMiB: 2048} + hosts: + - name: nas1 + address: nas1.home.lan:22 + user: virt + bridge: br0 + - name: lab2 + address: 10.0.0.2:2222 + user: root + socket: /run/libvirt/virtqemud-sock + pool: fast + bridge: br1 +` + +func TestLoadConfig_kvmValid(t *testing.T) { + t.Parallel() + cfg, err := LoadConfig([]byte(validKVMYAML)) + if err != nil { + t.Fatalf("LoadConfig() error = %v, want nil", err) + } + k := cfg.Providers[0].KVM + if k == nil { + t.Fatal("providers[0].kvm = nil") + } + if k.SSHKeyFile != "/etc/proxy-operator/kvm/id_ed25519" || k.DiskSizeGB != 20 { + t.Errorf("kvm = %+v, want sshKeyFile and diskSizeGb parsed", k) + } + if got := k.Images["debian-13"].SHA256; !strings.HasPrefix(got, "0123") { + t.Errorf("images[debian-13].sha256 = %q", got) + } + if k.Images["ubuntu"].Format != "raw" { + t.Errorf("images[ubuntu].format = %q, want raw", k.Images["ubuntu"].Format) + } + if mt := k.MachineTypes["medium"]; mt.VCPUs != 2 || mt.MemoryMiB != 2048 { + t.Errorf("machineTypes[medium] = %+v, want {2 2048}", mt) + } + if len(k.Hosts) != 2 || k.Hosts[1].Socket != "/run/libvirt/virtqemud-sock" || k.Hosts[1].Pool != "fast" { + t.Errorf("hosts = %+v, want two hosts with lab2's socket and pool parsed", k.Hosts) + } +} + +func TestLoadConfig_kvmInsecureHostKeyNeedsNoKnownHosts(t *testing.T) { + t.Parallel() + yaml := strings.Replace(validKVMYAML, + "knownHostsFile: /etc/proxy-operator/kvm/known_hosts", + "insecureIgnoreHostKey: true", 1) + if _, err := LoadConfig([]byte(yaml)); err != nil { + t.Fatalf("LoadConfig() error = %v, want nil", err) + } +} + +func TestLoadConfig_kvmInvalid(t *testing.T) { + t.Parallel() + minimal := func(kvmBody string) string { + return "providers:\n - name: kvm-home\n type: kvm\n kvm:\n" + indent(kvmBody, 6) + } + hostsAndTypes := ` +machineTypes: + small: {vcpus: 1, memoryMiB: 1024} +hosts: + - {name: nas1, address: nas1:22, user: virt, bridge: br0} +` + tests := []struct { + name string + yaml string + wantErrSub string + }{ + { + name: "missing kvm block", + yaml: "providers:\n - name: kvm-home\n type: kvm\n", + wantErrSub: "kvm block is required", + }, + { + name: "missing sshKeyFile", + yaml: minimal("knownHostsFile: /kh\n" + hostsAndTypes), + wantErrSub: "kvm.sshKeyFile is required", + }, + { + name: "missing knownHostsFile without insecure flag", + yaml: minimal("sshKeyFile: /key\n" + hostsAndTypes), + wantErrSub: "kvm.knownHostsFile is required unless", + }, + { + name: "negative disk size", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\ndiskSizeGb: -1\n" + hostsAndTypes), + wantErrSub: "diskSizeGb must not be negative", + }, + { + name: "no machine types", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nhosts:\n - {name: nas1, address: nas1:22, user: virt, bridge: br0}\n"), + wantErrSub: "machineTypes must define at least one", + }, + { + name: "machine type with zero memory", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n small: {vcpus: 1}\nhosts:\n - {name: nas1, address: nas1:22, user: virt, bridge: br0}\n"), + wantErrSub: `machineTypes["small"] needs vcpus > 0 and memoryMiB > 0`, + }, + { + name: "no hosts", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n small: {vcpus: 1, memoryMiB: 1024}\n"), + wantErrSub: "kvm.hosts must list at least one host", + }, + { + name: "duplicate host name", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n small: {vcpus: 1, memoryMiB: 1024}\nhosts:\n - {name: nas1, address: a:22, user: virt, bridge: br0}\n - {name: nas1, address: b:22, user: virt, bridge: br0}\n"), + wantErrSub: `duplicate host name "nas1"`, + }, + { + name: "host missing address", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n small: {vcpus: 1, memoryMiB: 1024}\nhosts:\n - {name: nas1, user: virt, bridge: br0}\n"), + wantErrSub: "address is required", + }, + { + name: "host missing user", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n small: {vcpus: 1, memoryMiB: 1024}\nhosts:\n - {name: nas1, address: a:22, bridge: br0}\n"), + wantErrSub: "user is required", + }, + { + name: "host missing bridge", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nmachineTypes:\n small: {vcpus: 1, memoryMiB: 1024}\nhosts:\n - {name: nas1, address: a:22, user: virt}\n"), + wantErrSub: "bridge is required", + }, + { + name: "image with non-http url", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nimages:\n d: {url: file:///tmp/x.qcow2}\n" + hostsAndTypes), + wantErrSub: `images["d"].url must be an http(s) URL`, + }, + { + name: "image with bad sha256", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nimages:\n d: {url: https://x/y.qcow2, sha256: abc}\n" + hostsAndTypes), + wantErrSub: `images["d"].sha256 must be 64 hex characters`, + }, + { + name: "image with unknown format", + yaml: minimal("sshKeyFile: /key\nknownHostsFile: /kh\nimages:\n d: {url: https://x/y.vmdk, format: vmdk}\n" + hostsAndTypes), + wantErrSub: `images["d"].format must be qcow2 or raw`, + }, + { + name: "kvm type with gcp block", + yaml: ` +providers: + - name: p1 + type: kvm + gcp: + project: my-project`, + wantErrSub: "type is kvm but a gcp block is set", + }, + { + name: "gcp type with kvm block", + yaml: ` +providers: + - name: p1 + type: gcp + gcp: + project: my-project + kvm: + sshKeyFile: /key`, + wantErrSub: "type is gcp but a kvm block is set", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := LoadConfig([]byte(tc.yaml)) + if err == nil { + t.Fatalf("LoadConfig() error = nil, want error containing %q", tc.wantErrSub) + } + if !strings.Contains(err.Error(), tc.wantErrSub) { + t.Errorf("LoadConfig() error = %q, want substring %q", err.Error(), tc.wantErrSub) + } + }) + } +} + +func indent(s string, n int) string { + pad := strings.Repeat(" ", n) + lines := strings.Split(strings.TrimLeft(s, "\n"), "\n") + for i, l := range lines { + if l != "" { + lines[i] = pad + l + } + } + return strings.Join(lines, "\n") +}