diff --git a/cmd/main.go b/cmd/main.go index 4bacd2f..5a0ff77 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -61,6 +61,7 @@ import ( "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/kubernetes" "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/kvm" "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/kvm2" + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/proxmox" "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider/registry" "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/providerhealth" "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/tracing" @@ -211,8 +212,9 @@ func main() { "gcp": func(ctx context.Context, pc provider.ProviderConfig) (provider.Provider, error) { return gcp.NewWithWireOptions(ctx, pc, gcp.WireLogOptions{FullPayloads: gcpWireFullPayloads}) }, - "kvm": kvm.New, - "kvm2": kvm2.New, + "kvm": kvm.New, + "kvm2": kvm2.New, + "proxmox": proxmox.New, }) if err != nil { setupLog.Error(err, "Failed to build providers") diff --git a/docs/plans-executions/2026-09-18-1502-proxmox-provider.md b/docs/plans-executions/2026-09-18-1502-proxmox-provider.md index ec9cc10..24d7fcb 100644 --- a/docs/plans-executions/2026-09-18-1502-proxmox-provider.md +++ b/docs/plans-executions/2026-09-18-1502-proxmox-provider.md @@ -6,7 +6,7 @@ Pairs with [docs/plans/2026-09-18-1502-proxmox-provider.md](../plans/2026-09-18- - [x] Phase 0 — Plan file, branch, host recon, fixtures - [x] Phase 1 — The spec (`docs/proxmox-provider-spec.md`) -- [ ] Phase 2 — Config block, registration, package skeleton +- [x] Phase 2 — Config block, registration, package skeleton - [ ] Phase 3 — Extract `nocloud` and `keyedlock` from kvm2 - [ ] Phase 4 — Seam, HTTP client, decorator, fake - [ ] Phase 5 — Get, ListByTag, task helpers @@ -329,3 +329,55 @@ listed as `unknown` is Proxmox's documented design, unverifiable on a single node; entry 13 now says so. And the matcher for `VM is not running` is a whole-message regexp, because ` is not running` alone also matches `QEMU guest agent is not running`. + +## Phase 2 — Config block, registration, package skeleton + +Added the `proxmox:` block (`internal/provider/config_proxmox.go`), wired it +into `config.go` (the field, `setBlocks`, the allow-list and the dispatch: +three `"proxmox"` sites) and registered `"proxmox": proxmox.New` in +`cmd/main.go`, so the two type lists changed in one commit. The package +`internal/provider/proxmox` now holds the seam interface and its data types +(`api.go`), `id.go`, `mark.go`, `state.go`, `errors.go` and the constructor +(`proxmox.go`). The four contract methods are stubs that answer +`ErrTransient`, so a Proxy on a configured `proxmox` entry retries instead of +latching until Phases 5 to 7 land. `New` passes a nil `api` until Phase 4 +builds the HTTP client. + +Every test that depends on Proxmox's wording or JSON shape reads the Phase 0 +captures through `fixtures_test.go` (`readCapture`, `loadAPIError`, +`decodeData`), so no message string is repeated in a test. The one exception +is `TestClassify_table`'s synthetic rows (a 404, 502, 595, 596, a storage +message), for statuses recon never produced. + +Deviations from the plan's Phase 2 list, both making it smaller: + +- **No temporary `locks.go`.** Only Create and Delete take the per-name lock, + and they arrive after Phase 3 extracts `internal/provider/keyedlock`, so + copying kvm2's lock now would only be deleted next phase. +- **No `provider.Checker` assertion yet.** `cmd/main.go` type-asserts the + Checker and would call a stub; the assertion comes with the real `Check` + in Phase 7. + +Added beyond the plan: `api_test.go` decodes the captured responses into the +seam's types, so a misspelled JSON tag fails now rather than in Phase 5, and +`TestParseDescription_fromClone` reads the mark back from the description of +the VM cloned in Phase 0. + +A mutation check confirmed the suite can fail. Matching `VM is not +running` as a plain suffix (the first spec draft's mistake) failed +`TestAPIError_matchers` on the agent-not-running capture; treating a paused +VM as running failed `TestMapState_table` and `TestMapState_fromCaptures`. +Both mutations were reverted and the files compared identical to backups. + +```bash +gofmt -w cmd/main.go # realigned the constructor map for the longer key +grep -c '"proxmox"' internal/provider/config.go # 3 +go build ./... && go vet ./... +go test -race -count=1 ./internal/provider/... ./cmd/... # all ok +go test -short -count=1 ./... # all ok +go test -count=1 -v ./internal/provider/proxmox/ | grep -c '^=== RUN' # 152 +``` + +Worth noting: the `validProxmoxYAML` constant is the shape Phase 9's commented +sample entry should take, with the user's values (`192.168.0.8`, `pv1-1`, +`proxy-seeds`, `vlanTag: 4`, template 9001). diff --git a/internal/provider/config.go b/internal/provider/config.go index 07fe0cd..64c7260 100644 --- a/internal/provider/config.go +++ b/internal/provider/config.go @@ -25,6 +25,8 @@ type ProviderConfig struct { // KVM2 is the same block shape as KVM, for the kvm2 provider type: one // or more libvirt/KVM hypervisors reached over SSH. KVM2 *KVMConfig `json:"kvm2,omitempty"` + // Proxmox is a Proxmox VE cluster reached through its REST API. + Proxmox *ProxmoxConfig `json:"proxmox,omitempty"` } // setBlocks names the type-specific blocks that are non-nil, so validate can @@ -43,6 +45,9 @@ func (p ProviderConfig) setBlocks() []string { if p.KVM2 != nil { out = append(out, "kvm2") } + if p.Proxmox != nil { + out = append(out, "proxmox") + } return out } @@ -106,7 +111,7 @@ func (c *Config) validate() error { prefix := fmt.Sprintf("providers[%d] %q", i, p.Name) switch p.Type { - case "kubernetes", "gcp", "kvm", "kvm2": + case "kubernetes", "gcp", "kvm", "kvm2", "proxmox": case "": return fmt.Errorf("%s: type is required", prefix) default: @@ -130,6 +135,10 @@ func (c *Config) validate() error { if err := validateKVMBlock(prefix, "kvm2", p.KVM2); err != nil { return err } + case "proxmox": + if err := validateProxmox(prefix, p.Proxmox); err != nil { + return err + } } } return nil diff --git a/internal/provider/config_proxmox.go b/internal/provider/config_proxmox.go new file mode 100644 index 0000000..18c877b --- /dev/null +++ b/internal/provider/config_proxmox.go @@ -0,0 +1,179 @@ +package provider + +import ( + "fmt" + "net/url" + "regexp" +) + +// ProxmoxConfig configures a proxmox provider instance: one Proxmox VE +// cluster reached through its REST API with an API token. VMs are full +// clones of prepared templates, seeded with a NoCloud ISO uploaded to +// ISOStorage. The design is in docs/proxmox-provider-spec.md. +type ProxmoxConfig struct { + // URL is the API endpoint, https://:8006. The host must be a name + // or address in the node certificate. Required. + URL string `json:"url"` + // TokenFile holds one line, @!=, read on + // every request. Required. + TokenFile string `json:"tokenFile"` + // CAFile is a PEM bundle trusted for the API's certificate, typically the + // node's /etc/pve/pve-root-ca.pem. Empty means the system roots. + CAFile string `json:"caFile,omitempty"` + // InsecureSkipTLSVerify disables certificate verification. + InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"` + // Nodes are the Proxmox node names placement.zone selects from. + // Required, at least one, unique. + Nodes []string `json:"nodes"` + // ISOStorage is the storage seed ISOs are uploaded to and deleted from; + // the token needs Datastore.Allocate on it. Required. + ISOStorage string `json:"isoStorage"` + // Storage is the target storage of the full clone. Empty clones onto the + // template's own disk storage. + Storage string `json:"storage,omitempty"` + // Bridge replaces the bridge of the cloned net0. Empty keeps it. + Bridge string `json:"bridge,omitempty"` + // VLANTag sets the VLAN tag of the cloned net0, 1 to 4094. Zero keeps + // whatever the template has. + VLANTag int `json:"vlanTag,omitempty"` + // VMIDRange is where VMIDs are allocated. Default 100000 to 199999. + VMIDRange *ProxmoxVMIDRange `json:"vmidRange,omitempty"` + // Templates are the clone sources placement.image names. Required, at + // least one. + Templates map[string]ProxmoxTemplate `json:"templates"` + // MachineTypes are the size profiles placement.machineType names. + // Required, at least one. + MachineTypes map[string]ProxmoxMachineType `json:"machineTypes"` + // RequestTimeoutSeconds bounds one API request. Default 30. + RequestTimeoutSeconds int `json:"requestTimeoutSeconds,omitempty"` + // VendorData is optional cloud-init vendor-data placed on every seed next + // to the Proxy's own user-data, as for the kvm providers. + VendorData string `json:"vendorData,omitempty"` + // InstallGuestAgent adds a vendor-data script that installs and starts + // qemu-guest-agent, which is how the provider learns a VM's address. + // Default true. Turn it off for templates that ship the agent. + InstallGuestAgent *bool `json:"installGuestAgent,omitempty"` +} + +// ProxmoxVMIDRange bounds the VMIDs the provider allocates, both inclusive. +type ProxmoxVMIDRange struct { + Min int `json:"min"` + Max int `json:"max"` +} + +// ProxmoxTemplate is one clone source: a VM converted to a template. +type ProxmoxTemplate struct { + VMID int `json:"vmid"` +} + +// ProxmoxMachineType is a VM size profile. DiskGiB grows the cloned boot +// disk when larger than the template's; zero keeps the template's size. +type ProxmoxMachineType struct { + Cores int32 `json:"cores"` + MemoryMiB int64 `json:"memoryMiB"` + DiskGiB int64 `json:"diskGiB,omitempty"` +} + +// Proxmox's own limits on VMIDs and VLAN tags. +const ( + ProxmoxMinVMID = 100 + ProxmoxMaxVMID = 999999999 + proxmoxMaxVLANTag = 4094 +) + +var ( + // proxmoxNodeName is a Proxmox node name: a DNS label, as pvecm allows. + proxmoxNodeName = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?$`) + // proxmoxStorageID is a Proxmox storage identifier. + proxmoxStorageID = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9_.-]*$`) +) + +func validateProxmox(prefix string, c *ProxmoxConfig) error { + if c == nil { + return fmt.Errorf("%s: proxmox block is required", prefix) + } + if err := validateProxmoxURL(c.URL); err != nil { + return fmt.Errorf("%s: proxmox.url %w", prefix, err) + } + if c.TokenFile == "" { + return fmt.Errorf("%s: proxmox.tokenFile is required", prefix) + } + if len(c.Nodes) == 0 { + return fmt.Errorf("%s: proxmox.nodes must list at least one node", prefix) + } + seen := make(map[string]bool, len(c.Nodes)) + for i, n := range c.Nodes { + if !proxmoxNodeName.MatchString(n) { + return fmt.Errorf("%s: proxmox.nodes[%d] %q is not a Proxmox node name (lowercase letters, digits and dashes)", prefix, i, n) + } + if seen[n] { + return fmt.Errorf("%s: proxmox.nodes[%d]: duplicate node %q", prefix, i, n) + } + seen[n] = true + } + if c.ISOStorage == "" { + return fmt.Errorf("%s: proxmox.isoStorage is required: name a storage with iso content where the token holds Datastore.Allocate", prefix) + } + if !proxmoxStorageID.MatchString(c.ISOStorage) { + return fmt.Errorf("%s: proxmox.isoStorage %q is not a Proxmox storage id", prefix, c.ISOStorage) + } + if c.Storage != "" && !proxmoxStorageID.MatchString(c.Storage) { + return fmt.Errorf("%s: proxmox.storage %q is not a Proxmox storage id", prefix, c.Storage) + } + if c.VLANTag < 0 || c.VLANTag > proxmoxMaxVLANTag { + return fmt.Errorf("%s: proxmox.vlanTag must be between 1 and %d (got %d)", prefix, proxmoxMaxVLANTag, c.VLANTag) + } + if r := c.VMIDRange; r != nil { + if r.Min < ProxmoxMinVMID || r.Max > ProxmoxMaxVMID || r.Min >= r.Max { + return fmt.Errorf("%s: proxmox.vmidRange must satisfy %d <= min < max <= %d (got %d to %d)", prefix, ProxmoxMinVMID, ProxmoxMaxVMID, r.Min, r.Max) + } + } + if len(c.Templates) == 0 { + return fmt.Errorf("%s: proxmox.templates must define at least one template", prefix) + } + for key, t := range c.Templates { + if key == "" { + return fmt.Errorf("%s: proxmox.templates has an empty key", prefix) + } + if t.VMID < ProxmoxMinVMID { + return fmt.Errorf("%s: proxmox.templates[%q].vmid must be at least %d (got %d)", prefix, key, ProxmoxMinVMID, t.VMID) + } + } + if len(c.MachineTypes) == 0 { + return fmt.Errorf("%s: proxmox.machineTypes must define at least one profile", prefix) + } + for key, mt := range c.MachineTypes { + if key == "" { + return fmt.Errorf("%s: proxmox.machineTypes has an empty key", prefix) + } + if mt.Cores <= 0 || mt.MemoryMiB <= 0 { + return fmt.Errorf("%s: proxmox.machineTypes[%q] needs cores > 0 and memoryMiB > 0", prefix, key) + } + if mt.DiskGiB < 0 { + return fmt.Errorf("%s: proxmox.machineTypes[%q].diskGiB must not be negative", prefix, key) + } + } + if c.RequestTimeoutSeconds < 0 { + return fmt.Errorf("%s: proxmox.requestTimeoutSeconds must not be negative", prefix) + } + return nil +} + +// validateProxmoxURL accepts only an https URL with a host and no path, so +// the client can append /api2/json itself. +func validateProxmoxURL(s string) error { + if s == "" { + return fmt.Errorf("is required") + } + u, err := url.Parse(s) + if err != nil { + return fmt.Errorf("does not parse: %w", err) + } + if u.Scheme != "https" || u.Host == "" { + return fmt.Errorf("must be an https URL with a host (got %q)", s) + } + if u.Path != "" && u.Path != "/" || u.RawQuery != "" || u.Fragment != "" { + return fmt.Errorf("must have no path, query or fragment; the client appends /api2/json (got %q)", s) + } + return nil +} diff --git a/internal/provider/config_proxmox_test.go b/internal/provider/config_proxmox_test.go new file mode 100644 index 0000000..d623928 --- /dev/null +++ b/internal/provider/config_proxmox_test.go @@ -0,0 +1,132 @@ +package provider + +import ( + "strings" + "testing" +) + +const validProxmoxYAML = ` +providers: + - name: proxmox-home + type: proxmox + proxmox: + url: https://192.168.0.8:8006 + tokenFile: /etc/proxy-operator/proxmox/token + caFile: /etc/proxy-operator/proxmox/ca.pem + nodes: [pv1-1, pv1-2] + isoStorage: proxy-seeds + storage: local-lvm + bridge: vmbr0 + vlanTag: 4 + vmidRange: {min: 150000, max: 150999} + templates: + debian-13: {vmid: 9001} + machineTypes: + small: {cores: 1, memoryMiB: 1024} + large: {cores: 4, memoryMiB: 4096, diskGiB: 20} + requestTimeoutSeconds: 45 + installGuestAgent: false +` + +func TestLoadConfig_proxmoxValid(t *testing.T) { + t.Parallel() + cfg, err := LoadConfig([]byte(validProxmoxYAML)) + if err != nil { + t.Fatalf("LoadConfig() error = %v, want nil", err) + } + c := cfg.Providers[0].Proxmox + if c == nil { + t.Fatal("providers[0].proxmox = nil") + } + if c.URL != "https://192.168.0.8:8006" || c.ISOStorage != "proxy-seeds" || c.VLANTag != 4 { + t.Errorf("proxmox = %+v, want url, isoStorage and vlanTag parsed", c) + } + if len(c.Nodes) != 2 || c.Nodes[1] != "pv1-2" { + t.Errorf("nodes = %v, want [pv1-1 pv1-2]", c.Nodes) + } + if c.VMIDRange == nil || c.VMIDRange.Min != 150000 || c.VMIDRange.Max != 150999 { + t.Errorf("vmidRange = %+v, want 150000 to 150999", c.VMIDRange) + } + if c.Templates["debian-13"].VMID != 9001 { + t.Errorf("templates[debian-13] = %+v, want vmid 9001", c.Templates["debian-13"]) + } + if mt := c.MachineTypes["large"]; mt.Cores != 4 || mt.MemoryMiB != 4096 || mt.DiskGiB != 20 { + t.Errorf("machineTypes[large] = %+v, want {4 4096 20}", mt) + } + if c.InstallGuestAgent == nil || *c.InstallGuestAgent { + t.Errorf("installGuestAgent = %v, want explicit false", c.InstallGuestAgent) + } +} + +func TestValidateProxmox_table(t *testing.T) { + t.Parallel() + block := func(body string) string { + return "providers:\n - name: proxmox-home\n type: proxmox\n proxmox:\n" + indent(body, 6) + } + const base = ` +url: https://192.168.0.8:8006 +tokenFile: /t +nodes: [pv1-1] +isoStorage: proxy-seeds +templates: {debian-13: {vmid: 9001}} +machineTypes: {small: {cores: 1, memoryMiB: 1024}} +` + with := func(old, repl string) string { + if !strings.Contains(base, old) { + t.Fatalf("base has no %q", old) + } + return block(strings.Replace(base, old, repl, 1)) + } + tests := []struct { + name string + yaml string + wantErrSub string + }{ + {name: "valid minimal", yaml: block(base)}, + {name: "valid full", yaml: validProxmoxYAML}, + {name: "valid with trailing slash", yaml: with("url: https://192.168.0.8:8006", "url: https://192.168.0.8:8006/")}, + {name: "missing block", yaml: "providers:\n - name: proxmox-home\n type: proxmox\n", wantErrSub: "proxmox block is required"}, + {name: "missing url", yaml: with("url: https://192.168.0.8:8006\n", ""), wantErrSub: "proxmox.url is required"}, + {name: "http url", yaml: with("https://", "http://"), wantErrSub: "must be an https URL"}, + {name: "url with api path", yaml: with("8006", "8006/api2/json"), wantErrSub: "must have no path"}, + {name: "missing tokenFile", yaml: with("tokenFile: /t\n", ""), wantErrSub: "proxmox.tokenFile is required"}, + {name: "no nodes", yaml: with("nodes: [pv1-1]", "nodes: []"), wantErrSub: "proxmox.nodes must list at least one node"}, + {name: "duplicate node", yaml: with("nodes: [pv1-1]", "nodes: [pv1-1, pv1-1]"), wantErrSub: `duplicate node "pv1-1"`}, + {name: "node with a slash", yaml: with("nodes: [pv1-1]", "nodes: [pv1/1]"), wantErrSub: "is not a Proxmox node name"}, + {name: "node in uppercase", yaml: with("nodes: [pv1-1]", "nodes: [PV1]"), wantErrSub: "is not a Proxmox node name"}, + {name: "missing isoStorage", yaml: with("isoStorage: proxy-seeds\n", ""), wantErrSub: "proxmox.isoStorage is required"}, + {name: "bad isoStorage id", yaml: with("isoStorage: proxy-seeds", "isoStorage: 1seeds"), wantErrSub: "proxmox.isoStorage \"1seeds\" is not a Proxmox storage id"}, + {name: "bad storage id", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nstorage: a/b"), wantErrSub: "proxmox.storage \"a/b\""}, + {name: "vlanTag too large", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nvlanTag: 4095"), wantErrSub: "proxmox.vlanTag must be between 1 and 4094"}, + {name: "vlanTag negative", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nvlanTag: -1"), wantErrSub: "proxmox.vlanTag"}, + {name: "vmidRange min below 100", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nvmidRange: {min: 50, max: 500}"), wantErrSub: "proxmox.vmidRange must satisfy"}, + {name: "vmidRange min above max", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nvmidRange: {min: 2000, max: 1000}"), wantErrSub: "proxmox.vmidRange must satisfy"}, + {name: "vmidRange max too large", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nvmidRange: {min: 100, max: 1000000000}"), wantErrSub: "proxmox.vmidRange must satisfy"}, + {name: "no templates", yaml: with("templates: {debian-13: {vmid: 9001}}", "templates: {}"), wantErrSub: "proxmox.templates must define at least one template"}, + {name: "template vmid below 100", yaml: with("vmid: 9001", "vmid: 99"), wantErrSub: `proxmox.templates["debian-13"].vmid must be at least 100`}, + {name: "no machineTypes", yaml: with("machineTypes: {small: {cores: 1, memoryMiB: 1024}}", "machineTypes: {}"), wantErrSub: "proxmox.machineTypes must define at least one profile"}, + {name: "cores zero", yaml: with("cores: 1", "cores: 0"), wantErrSub: `proxmox.machineTypes["small"] needs cores > 0`}, + {name: "memory zero", yaml: with("memoryMiB: 1024", "memoryMiB: 0"), wantErrSub: "memoryMiB > 0"}, + {name: "negative diskGiB", yaml: with("memoryMiB: 1024", "memoryMiB: 1024, diskGiB: -5"), wantErrSub: "diskGiB must not be negative"}, + {name: "negative timeout", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nrequestTimeoutSeconds: -1"), wantErrSub: "proxmox.requestTimeoutSeconds must not be negative"}, + {name: "unknown key is rejected", yaml: with("isoStorage: proxy-seeds", "isoStorage: proxy-seeds\nsshKeyFile: /k"), wantErrSub: "parsing providers config"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, err := LoadConfig([]byte(tc.yaml)) + if tc.wantErrSub == "" { + if err != nil { + t.Fatalf("LoadConfig() error = %v, want nil", err) + } + return + } + 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) + } + }) + } +} diff --git a/internal/provider/config_test.go b/internal/provider/config_test.go index f82999c..4b81ddd 100644 --- a/internal/provider/config_test.go +++ b/internal/provider/config_test.go @@ -138,6 +138,26 @@ providers: sshKeyFile: /key`, wantErrSub: "type is kvm2 but a kvm block is set", }, + { + name: "proxmox type with kvm block", + yaml: ` +providers: + - name: p1 + type: proxmox + kvm: + sshKeyFile: /key`, + wantErrSub: "type is proxmox but a kvm block is set", + }, + { + name: "kvm type with proxmox block", + yaml: ` +providers: + - name: p1 + type: kvm + proxmox: + url: https://pve:8006`, + wantErrSub: "type is kvm but a proxmox block is set", + }, { name: "strict mode rejects unknown top-level key", yaml: ` diff --git a/internal/provider/proxmox/api.go b/internal/provider/proxmox/api.go new file mode 100644 index 0000000..e5e479f --- /dev/null +++ b/internal/provider/proxmox/api.go @@ -0,0 +1,119 @@ +package proxmox + +import ( + "context" + "io" +) + +// api is the seam over the Proxmox VE REST API: one method per endpoint the +// provider uses, ctx first (spec entry 17). Calls that start a Proxmox task +// return its UPID; the caller waits on it with TaskStatus. +type api interface { + Version(ctx context.Context) (versionInfo, error) + NodeStatus(ctx context.Context, node string) (nodeStatus, error) + StorageStatus(ctx context.Context, node, storage string) (storageStatus, error) + ClusterResources(ctx context.Context) ([]vmResource, error) + VMConfig(ctx context.Context, node string, vmid int) (vmConfig, error) + SetVMConfig(ctx context.Context, node string, vmid int, params map[string]string) error + Clone(ctx context.Context, node string, vmid int, p cloneParams) (upid string, err error) + Resize(ctx context.Context, node string, vmid int, disk, size string) (upid string, err error) + Start(ctx context.Context, node string, vmid int) (upid string, err error) + Stop(ctx context.Context, node string, vmid int) (upid string, err error) + VMStatus(ctx context.Context, node string, vmid int) (vmStatus, error) + Destroy(ctx context.Context, node string, vmid int) (upid string, err error) + TaskStatus(ctx context.Context, node, upid string) (taskStatus, error) + AgentNetworkInterfaces(ctx context.Context, node string, vmid int) ([]agentInterface, error) + UploadISO(ctx context.Context, node, storage, filename string, r io.Reader, size int64) (upid string, err error) + DeleteVolume(ctx context.Context, node, storage, volid string) (upid string, err error) +} + +// versionInfo is GET /version. +type versionInfo struct { + Version string `json:"version"` + Release string `json:"release"` + RepoID string `json:"repoid"` +} + +// nodeStatus is the part of GET /nodes/{node}/status the Checker reports. +type nodeStatus struct { + PVEVersion string `json:"pveversion"` + CurrentKernel struct { + Release string `json:"release"` + } `json:"current-kernel"` +} + +// storageStatus is GET /nodes/{node}/storage/{storage}/status. Content is a +// comma-separated list such as "iso,import,backup,vztmpl". +type storageStatus struct { + Type string `json:"type"` + Content string `json:"content"` + Active int `json:"active"` + Enabled int `json:"enabled"` + Shared int `json:"shared"` + Total int64 `json:"total"` + Used int64 `json:"used"` + Avail int64 `json:"avail"` +} + +// vmResource is one entry of GET /cluster/resources?type=vm. A VM being +// cloned is listed without a name, with lock "clone" and status "unknown" +// (spec entry 9). Tags are separated by ";". +type vmResource struct { + VMID int `json:"vmid"` + Name string `json:"name"` + Node string `json:"node"` + Type string `json:"type"` + Status string `json:"status"` + Lock string `json:"lock"` + Tags string `json:"tags"` + Template int `json:"template"` +} + +// vmConfig is GET /nodes/{node}/qemu/{vmid}/config with every value +// rendered as a string: Proxmox sends some scalars as JSON numbers (cores, +// template) and others as strings (memory). +type vmConfig map[string]string + +// vmStatus is GET /nodes/{node}/qemu/{vmid}/status/current. QMPStatus tells +// a paused VM from a running one; the resources listing cannot. +type vmStatus struct { + Status string `json:"status"` + QMPStatus string `json:"qmpstatus"` + Name string `json:"name"` + Lock string `json:"lock"` + Uptime int64 `json:"uptime"` +} + +// taskStatus is GET /nodes/{node}/tasks/{upid}/status. ExitStatus is "OK" +// or the failure's text once Status is "stopped". +type taskStatus struct { + UPID string `json:"upid"` + Type string `json:"type"` + ID string `json:"id"` + Node string `json:"node"` + Status string `json:"status"` + ExitStatus string `json:"exitstatus"` +} + +// agentInterface is one entry of the guest agent's network-get-interfaces +// result. +type agentInterface struct { + Name string `json:"name"` + IPAddresses []agentIP `json:"ip-addresses"` +} + +type agentIP struct { + Address string `json:"ip-address"` + Type string `json:"ip-address-type"` + Prefix int `json:"prefix"` +} + +// cloneParams are the parameters of POST .../clone the provider sets. +// Storage and Target are omitted when empty. +type cloneParams struct { + NewID int + Name string + Description string + Storage string + Target string +} diff --git a/internal/provider/proxmox/api_test.go b/internal/provider/proxmox/api_test.go new file mode 100644 index 0000000..b91d2b1 --- /dev/null +++ b/internal/provider/proxmox/api_test.go @@ -0,0 +1,97 @@ +package proxmox + +import ( + "strings" + "testing" +) + +// TestAPITypes_decodeCaptures decodes the Phase 0 responses into the seam's +// types, so a misspelled JSON tag fails here and not in a later phase. +func TestAPITypes_decodeCaptures(t *testing.T) { + t.Parallel() + + t.Run("version", func(t *testing.T) { + t.Parallel() + if v := decodeData[versionInfo](t, "version"); v.Version != "9.2.20" || v.Release != "9.2" || v.RepoID == "" { + t.Errorf("version = %+v", v) + } + }) + t.Run("node status", func(t *testing.T) { + t.Parallel() + n := decodeData[nodeStatus](t, "node_status") + if !strings.HasPrefix(n.PVEVersion, "pve-manager/9.2.20") || n.CurrentKernel.Release != "7.0.14-17-pve" { + t.Errorf("node status = %+v", n) + } + }) + t.Run("storage status", func(t *testing.T) { + t.Parallel() + s := decodeData[storageStatus](t, "storage_status") + if s.Type != "dir" || !strings.Contains(s.Content, "iso") || s.Active != 1 || s.Avail <= 0 || s.Total < s.Avail { + t.Errorf("storage status = %+v", s) + } + }) + t.Run("cluster resources", func(t *testing.T) { + t.Parallel() + rs := decodeData[[]vmResource](t, "cluster_resources") + byID := map[int]vmResource{} + for _, r := range rs { + byID[r.VMID] = r + } + if r := byID[9001]; r.Name != "debian-13-template" || r.Template != 1 || r.Node != "pv1-1" || r.Type != "qemu" || r.Status != "stopped" { + t.Errorf("resources[9001] = %+v", r) + } + if r := byID[100]; r.Template != 0 || r.Name != "ubuntu-vm-1" { + t.Errorf("resources[100] = %+v", r) + } + }) + t.Run("cluster resources during clone", func(t *testing.T) { + t.Parallel() + for _, r := range decodeData[[]vmResource](t, "cluster_resources_during_clone") { + if r.VMID != 199999 { + continue + } + if r.Name != "" || r.Lock != "clone" || r.Status != "unknown" { + t.Errorf("mid-clone entry = %+v, want no name, lock clone, status unknown", r) + } + return + } + t.Error("no entry for VMID 199999 in the mid-clone capture") + }) + t.Run("cluster resources tags", func(t *testing.T) { + t.Parallel() + for _, r := range decodeData[[]vmResource](t, "cluster_resources_during_destroy") { + if r.VMID == 199999 && r.Tags != "proxy_operator" { + t.Errorf("tags = %q, want proxy_operator", r.Tags) + } + } + }) + t.Run("status current", func(t *testing.T) { + t.Parallel() + if s := decodeData[vmStatus](t, "status_current_paused"); s.Status != "running" || s.QMPStatus != "paused" { + t.Errorf("paused status = %+v", s) + } + if s := decodeData[vmStatus](t, "status_current_stopped"); s.Status != "stopped" || s.QMPStatus != "stopped" { + t.Errorf("stopped status = %+v", s) + } + }) + t.Run("task status", func(t *testing.T) { + t.Parallel() + if s := decodeData[taskStatus](t, "task_status_stopped_error"); s.Status != "stopped" || s.ExitStatus != "shrinking disks is not supported" || s.Type != "resize" { + t.Errorf("failed task = %+v", s) + } + if s := decodeData[taskStatus](t, "task_status_running"); s.Status != "running" || s.ExitStatus != "" || s.Node != "pv1-1" { + t.Errorf("running task = %+v", s) + } + if s := decodeData[taskStatus](t, "task_status_volume_missing"); s.ExitStatus != "OK" { + t.Errorf("deleting a missing volume = %+v, want exitstatus OK", s) + } + }) + t.Run("UPID responses", func(t *testing.T) { + t.Parallel() + for _, c := range []string{"clone_response", "start_response", "stop_response", "destroy_response", "resize_response", "upload_response", "content_delete_response"} { + if upid := decodeData[string](t, c); !strings.HasPrefix(upid, "UPID:pv1-1:") { + t.Errorf("%s = %q, want a UPID of node pv1-1", c, upid) + } + } + }) +} diff --git a/internal/provider/proxmox/errors.go b/internal/provider/proxmox/errors.go new file mode 100644 index 0000000..3dbf074 --- /dev/null +++ b/internal/provider/proxmox/errors.go @@ -0,0 +1,192 @@ +package proxmox + +import ( + "crypto/tls" + "crypto/x509" + "encoding/json" + "errors" + "fmt" + "maps" + "net/http" + "regexp" + "slices" + "strings" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// permanentf and transientf classify an error at the source, so classify +// keeps that class no matter what wraps it later. +func permanentf(format string, args ...any) error { + return fmt.Errorf("%w: %s", provider.ErrPermanent, fmt.Sprintf(format, args...)) +} + +func transientf(format string, args ...any) error { + return fmt.Errorf("%w: %s", provider.ErrTransient, fmt.Sprintf(format, args...)) +} + +// apiError is a non-2xx answer from the API. It carries the status and the +// message only, never headers or the request URL, so no credential can leak +// through it (spec entry 14). +type apiError struct { + Status int + Message string + // Errors is a 400's per-parameter map. + Errors map[string]string +} + +func (e *apiError) Error() string { + msg := fmt.Sprintf("proxmox API %d: %s", e.Status, e.Message) + if len(e.Errors) == 0 { + return msg + } + fields := make([]string, 0, len(e.Errors)) + for _, k := range slices.Sorted(maps.Keys(e.Errors)) { + fields = append(fields, k+": "+e.Errors[k]) + } + return msg + " (" + strings.Join(fields, "; ") + ")" +} + +// newAPIError builds an apiError from a response: the body's message with +// its trailing newline trimmed, or the status reason phrase when the body +// has none, as for a 401 (spec entry 13, fact F1). +func newAPIError(status int, reason string, body []byte) *apiError { + var b struct { + Message string `json:"message"` + Errors map[string]string `json:"errors"` + } + _ = json.Unmarshal(body, &b) + e := &apiError{Status: status, Message: strings.TrimSpace(b.Message)} + if e.Message == "" { + e.Message = strings.TrimSpace(reason) + } + if e.Message == "" { + e.Message = http.StatusText(status) + } + if len(b.Errors) > 0 { + e.Errors = make(map[string]string, len(b.Errors)) + for k, v := range b.Errors { + e.Errors[k] = strings.TrimSpace(v) + } + } + return e +} + +// The 500 messages the provider acts on, each backed by a captured response +// under testdata/. +var ( + vmConfigMissing = regexp.MustCompile(`^Configuration file '[^']*' does not exist$`) + vmNotRunning = regexp.MustCompile(`^VM [0-9]+ is not running$`) +) + +const ( + msgLocked = "VM is locked" + msgAlreadyExists = "config file already exists" + msgAgentNotRunning = "QEMU guest agent is not running" + msgAgentNotConfigured = "No QEMU guest agent configured" +) + +// server500 returns the message of err when it is, or wraps, an apiError +// with status 500. +func server500(err error) (string, bool) { + var ae *apiError + if !errors.As(err, &ae) || ae.Status != http.StatusInternalServerError { + return "", false + } + return ae.Message, true +} + +// isNotExist reports the missing-VM answer: the VM's config file is gone. +func isNotExist(err error) bool { + msg, ok := server500(err) + return ok && vmConfigMissing.MatchString(msg) +} + +// isLocked reports a VM held by a running task ("VM is locked (clone)"). +func isLocked(err error) bool { + msg, ok := server500(err) + return ok && strings.HasPrefix(msg, msgLocked) +} + +// isAlreadyExists reports a clone onto a VMID that is taken. +func isAlreadyExists(err error) bool { + msg, ok := server500(err) + return ok && strings.HasSuffix(msg, msgAlreadyExists) +} + +// isAgentNotRunning reports a VM with the agent enabled whose agent does not +// answer yet. +func isAgentNotRunning(err error) bool { + msg, ok := server500(err) + return ok && msg == msgAgentNotRunning +} + +// isAgentNotConfigured reports a VM whose config no longer enables the agent. +func isAgentNotConfigured(err error) bool { + msg, ok := server500(err) + return ok && msg == msgAgentNotConfigured +} + +// isVMNotRunning reports an agent call on a VM that is stopped. It is +// matched on the whole message because " is not running" alone is also a +// suffix of the agent-not-running answer. +func isVMNotRunning(err error) bool { + msg, ok := server500(err) + return ok && vmNotRunning.MatchString(msg) +} + +// classify maps any error reaching the provider's boundary to one of the +// four contract sentinels, per spec entry 13. op is the contract operation: +// a vanished VM inside Create is Transient, because the contract forbids +// NotFound from Create. +func classify(op string, err error) error { + if err == nil { + return provider.ErrTransient + } + for _, class := range []error{provider.ErrNotFound, provider.ErrQuotaExceeded, provider.ErrPermanent, provider.ErrTransient} { + if errors.Is(err, class) { + return class + } + } + var ae *apiError + if errors.As(err, &ae) { + return classifyAPI(op, err, ae) + } + if isTLSVerification(err) { + return provider.ErrPermanent + } + // Everything else retries: context cancellation and deadlines, + // net.OpError, io.EOF, and anything unrecognised. + return provider.ErrTransient +} + +func classifyAPI(op string, err error, ae *apiError) error { + switch { + case ae.Status >= 400 && ae.Status < 500: + // 401 and 403 are credentials or privileges, 400 a request Proxmox + // will never accept: all fixed by an admin, not by retrying. + return provider.ErrPermanent + case isNotExist(err): + if op == "create" { + return provider.ErrTransient + } + return provider.ErrNotFound + default: + // Other 500s (a lock, a taken VMID escaping the allocator, agent + // states escaping Get), pveproxy's 595 and 596, and any other status. + return provider.ErrTransient + } +} + +// isTLSVerification reports a certificate the client refused: the URL does +// not match the certificate, or the CA is wrong. An admin fixes the URL or +// caFile; retrying cannot. +func isTLSVerification(err error) bool { + var ( + verr *tls.CertificateVerificationError + unknown x509.UnknownAuthorityError + hostname x509.HostnameError + invalid x509.CertificateInvalidError + ) + return errors.As(err, &verr) || errors.As(err, &unknown) || errors.As(err, &hostname) || errors.As(err, &invalid) +} diff --git a/internal/provider/proxmox/errors_test.go b/internal/provider/proxmox/errors_test.go new file mode 100644 index 0000000..f3e8fc3 --- /dev/null +++ b/internal/provider/proxmox/errors_test.go @@ -0,0 +1,197 @@ +package proxmox + +import ( + "context" + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "io" + "net" + "net/url" + "strings" + "testing" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +func TestClassify_table(t *testing.T) { + t.Parallel() + fixture := func(name string) error { return loadAPIError(t, name) } + tests := []struct { + name string + op string + err error + want error + }{ + {name: "nil is transient", op: "get", err: nil, want: provider.ErrTransient}, + + {name: "missing VM config on get", op: "get", err: fixture("err_vm_missing"), want: provider.ErrNotFound}, + {name: "missing VM status on get", op: "get", err: fixture("err_vm_missing_status"), want: provider.ErrNotFound}, + {name: "missing VM on delete", op: "delete", err: fixture("err_destroy_missing"), want: provider.ErrNotFound}, + {name: "missing VM on list", op: "list", err: fixture("err_vm_missing"), want: provider.ErrNotFound}, + {name: "missing VM inside create is transient", op: "create", err: fixture("err_vm_missing"), want: provider.ErrTransient}, + {name: "wrapped missing VM keeps its class", op: "get", err: fmt.Errorf("reading config: %w", fixture("err_vm_missing")), want: provider.ErrNotFound}, + + {name: "locked by a clone", op: "create", err: fixture("err_config_while_clone_locked"), want: provider.ErrTransient}, + {name: "VMID taken escaping the allocator", op: "create", err: fixture("err_newid_in_use"), want: provider.ErrTransient}, + {name: "agent not running escaping Get", op: "get", err: fixture("err_agent_not_running"), want: provider.ErrTransient}, + {name: "agent not configured escaping Get", op: "get", err: fixture("err_agent_vm_stopped"), want: provider.ErrTransient}, + {name: "VM not running escaping Get", op: "get", err: fixture("err_agent_vm_stopped_agent_enabled"), want: provider.ErrTransient}, + + {name: "wrong secret", op: "get", err: fixture("err_unauthorized"), want: provider.ErrPermanent}, + {name: "unknown token id", op: "list", err: fixture("err_unauthorized_unknown_token"), want: provider.ErrPermanent}, + {name: "no credentials", op: "list", err: fixture("err_no_credentials"), want: provider.ErrPermanent}, + {name: "missing privilege", op: "get", err: fixture("err_forbidden"), want: provider.ErrPermanent}, + {name: "missing Datastore.Allocate", op: "delete", err: fixture("err_forbidden_volume_delete"), want: provider.ErrPermanent}, + {name: "parameter verification", op: "create", err: fixture("err_bad_param"), want: provider.ErrPermanent}, + {name: "invalid tag", op: "create", err: fixture("err_tag_invalid"), want: provider.ErrPermanent}, + {name: "upload without .iso", op: "create", err: fixture("err_upload_bad_name"), want: provider.ErrPermanent}, + {name: "clone onto a storage without images", op: "create", err: fixture("err_clone_bad_storage"), want: provider.ErrPermanent}, + {name: "any other 4xx", op: "get", err: &apiError{Status: 404, Message: "Not Found"}, want: provider.ErrPermanent}, + + {name: "other 500", op: "get", err: &apiError{Status: 500, Message: "got timeout"}, want: provider.ErrTransient}, + {name: "storage does not exist is not a missing VM", op: "delete", err: &apiError{Status: 500, Message: "storage 'x' does not exist"}, want: provider.ErrTransient}, + {name: "502", op: "get", err: &apiError{Status: 502, Message: "Bad Gateway"}, want: provider.ErrTransient}, + {name: "pveproxy 595", op: "get", err: &apiError{Status: 595, Message: "No route to host"}, want: provider.ErrTransient}, + {name: "pveproxy 596", op: "list", err: &apiError{Status: 596, Message: "Connection timed out"}, want: provider.ErrTransient}, + + {name: "unknown authority", op: "get", err: x509.UnknownAuthorityError{}, want: provider.ErrPermanent}, + {name: "hostname mismatch", op: "create", err: x509.HostnameError{Certificate: &x509.Certificate{}, Host: "192.168.2.8"}, want: provider.ErrPermanent}, + {name: "expired certificate", op: "list", err: x509.CertificateInvalidError{Reason: x509.Expired}, want: provider.ErrPermanent}, + {name: "tls verification wrapped in url.Error", op: "get", err: &url.Error{Op: "Get", URL: "https://pve", Err: &tls.CertificateVerificationError{Err: x509.UnknownAuthorityError{}}}, want: provider.ErrPermanent}, + + {name: "context canceled", op: "get", err: context.Canceled, want: provider.ErrTransient}, + {name: "context deadline", op: "get", err: context.DeadlineExceeded, want: provider.ErrTransient}, + {name: "net.OpError", op: "get", err: &net.OpError{Op: "dial", Net: "tcp", Err: errors.New("connection refused")}, want: provider.ErrTransient}, + {name: "io.EOF", op: "list", err: io.EOF, want: provider.ErrTransient}, + {name: "io.ErrUnexpectedEOF", op: "list", err: io.ErrUnexpectedEOF, want: provider.ErrTransient}, + {name: "anything else is transient", op: "get", err: errors.New("anything"), want: provider.ErrTransient}, + + {name: "pre-classified NotFound stays", op: "create", err: fmt.Errorf("x: %w", provider.ErrNotFound), want: provider.ErrNotFound}, + {name: "pre-classified Quota stays", op: "create", err: fmt.Errorf("x: %w", provider.ErrQuotaExceeded), want: provider.ErrQuotaExceeded}, + {name: "pre-classified Permanent stays", op: "get", err: permanentf("bad %s", "thing"), want: provider.ErrPermanent}, + {name: "pre-classified Transient beats a 4xx underneath", op: "get", err: fmt.Errorf("%w: %w", provider.ErrTransient, fixture("err_forbidden")), want: provider.ErrTransient}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := classify(tc.op, tc.err); !errors.Is(got, tc.want) { + t.Errorf("classify(%q, %v) = %v, want %v", tc.op, tc.err, got, tc.want) + } + }) + } +} + +// TestAPIError_matchers runs every matcher over every captured error, so a +// matcher that also fires on another message fails here. +func TestAPIError_matchers(t *testing.T) { + t.Parallel() + matchers := map[string]func(error) bool{ + "isNotExist": isNotExist, + "isLocked": isLocked, + "isAlreadyExists": isAlreadyExists, + "isAgentNotRunning": isAgentNotRunning, + "isAgentNotConfigured": isAgentNotConfigured, + "isVMNotRunning": isVMNotRunning, + } + tests := []struct { + capture string + want string // the one matcher that fires, or "" for none + }{ + {"err_vm_missing", "isNotExist"}, + {"err_vm_missing_status", "isNotExist"}, + {"err_destroy_missing", "isNotExist"}, + {"err_config_while_clone_locked", "isLocked"}, + {"err_newid_in_use", "isAlreadyExists"}, + {"err_agent_not_running", "isAgentNotRunning"}, + {"err_agent_vm_stopped", "isAgentNotConfigured"}, + {"err_agent_vm_stopped_agent_enabled", "isVMNotRunning"}, + {"err_unauthorized", ""}, + {"err_forbidden", ""}, + {"err_bad_param", ""}, + {"err_clone_bad_storage", ""}, + } + for _, tc := range tests { + t.Run(tc.capture, func(t *testing.T) { + t.Parallel() + err := fmt.Errorf("wrapped: %w", loadAPIError(t, tc.capture)) + for name, match := range matchers { + if got, want := match(err), name == tc.want; got != want { + t.Errorf("%s(%v) = %v, want %v", name, err, got, want) + } + } + }) + } + if isNotExist(errors.New("Configuration file 'x' does not exist")) { + t.Error("isNotExist matched an error that is not an apiError") + } +} + +func TestNewAPIError_fromCaptures(t *testing.T) { + t.Parallel() + tests := []struct { + capture string + wantStatus int + wantMessage string + wantErrors map[string]string + }{ + {capture: "err_vm_missing", wantStatus: 500, wantMessage: "Configuration file 'nodes/pv1-1/qemu-server/999999.conf' does not exist"}, + {capture: "err_unauthorized", wantStatus: 401, wantMessage: "Authentication failed!"}, + {capture: "err_no_credentials", wantStatus: 401, wantMessage: "No ticket"}, + {capture: "err_forbidden", wantStatus: 403, wantMessage: "Permission check failed (/nodes/pv1-1, Sys.Syslog)"}, + {capture: "err_bad_param", wantStatus: 400, wantMessage: "Parameter verification failed.", wantErrors: map[string]string{"newid": "type check ('integer') failed - got 'abc'"}}, + } + for _, tc := range tests { + t.Run(tc.capture, func(t *testing.T) { + t.Parallel() + got := loadAPIError(t, tc.capture) + if got.Status != tc.wantStatus || got.Message != tc.wantMessage { + t.Errorf("newAPIError = {%d %q}, want {%d %q}", got.Status, got.Message, tc.wantStatus, tc.wantMessage) + } + if len(got.Errors) != len(tc.wantErrors) { + t.Fatalf("Errors = %v, want %v", got.Errors, tc.wantErrors) + } + for k, v := range tc.wantErrors { + if got.Errors[k] != v { + t.Errorf("Errors[%q] = %q, want %q", k, got.Errors[k], v) + } + } + }) + } +} + +func TestNewAPIError_fallbacks(t *testing.T) { + t.Parallel() + if got := newAPIError(503, "", []byte("")); got.Message != "Service Unavailable" { + t.Errorf("no reason and no JSON body: Message = %q, want the status text", got.Message) + } + if got := newAPIError(500, "reason text", []byte(`{"data":null}`)); got.Message != "reason text" { + t.Errorf("body without message: Message = %q, want the reason phrase", got.Message) + } +} + +func TestAPIError_errorString(t *testing.T) { + t.Parallel() + got := loadAPIError(t, "err_bad_param").Error() + want := "proxmox API 400: Parameter verification failed. (newid: type check ('integer') failed - got 'abc')" + if got != want { + t.Errorf("Error() = %q, want %q", got, want) + } + e := &apiError{Status: 400, Message: "m", Errors: map[string]string{"b": "2", "a": "1"}} + if got := e.Error(); !strings.HasSuffix(got, "(a: 1; b: 2)") { + t.Errorf("Error() = %q, want field errors sorted by key", got) + } +} + +func TestPermanentfAndTransientf(t *testing.T) { + t.Parallel() + perm := permanentf("node %q missing", "x") + if !errors.Is(perm, provider.ErrPermanent) || perm.Error() != `provider: permanent error: node "x" missing` { + t.Errorf("permanentf() = %q, want ErrPermanent with the message", perm) + } + trans := transientf("retry %d", 3) + if !errors.Is(trans, provider.ErrTransient) || trans.Error() != "provider: transient error: retry 3" { + t.Errorf("transientf() = %q, want ErrTransient with the message", trans) + } +} diff --git a/internal/provider/proxmox/fixtures_test.go b/internal/provider/proxmox/fixtures_test.go new file mode 100644 index 0000000..a49bc32 --- /dev/null +++ b/internal/provider/proxmox/fixtures_test.go @@ -0,0 +1,57 @@ +package proxmox + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +// capture is one response recorded from the real host in Phase 0: the +// status, the reason phrase after the status code, and the raw body. +type capture struct { + Status int `json:"status"` + Reason string `json:"reason"` + Body string `json:"body"` +} + +// readCapture loads testdata/.json and fails the test when it is +// missing, so every string a test relies on is backed by a real response. +func readCapture(t *testing.T, name string) capture { + t.Helper() + raw, err := os.ReadFile(filepath.Join("testdata", name+".json")) + if err != nil { + t.Fatalf("reading capture %s: %v", name, err) + } + var c capture + if err := json.Unmarshal(raw, &c); err != nil { + t.Fatalf("parsing capture %s: %v", name, err) + } + return c +} + +// loadAPIError turns an error capture into the apiError the client builds. +func loadAPIError(t *testing.T, name string) *apiError { + t.Helper() + c := readCapture(t, name) + if c.Status < 400 { + t.Fatalf("capture %s has status %d, want an error status", name, c.Status) + } + return newAPIError(c.Status, c.Reason, []byte(c.Body)) +} + +// decodeData decodes the "data" member of a successful capture into T. +func decodeData[T any](t *testing.T, name string) T { + t.Helper() + c := readCapture(t, name) + if c.Status != 200 { + t.Fatalf("capture %s has status %d, want 200", name, c.Status) + } + var env struct { + Data T `json:"data"` + } + if err := json.Unmarshal([]byte(c.Body), &env); err != nil { + t.Fatalf("decoding capture %s: %v", name, err) + } + return env.Data +} diff --git a/internal/provider/proxmox/id.go b/internal/provider/proxmox/id.go new file mode 100644 index 0000000..f0c48d3 --- /dev/null +++ b/internal/provider/proxmox/id.go @@ -0,0 +1,42 @@ +package proxmox + +import ( + "hash/fnv" + "strconv" + "strings" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +const idPrefix = "vms" + +// formatProviderID gives vms//: location-free, because VMs can +// migrate between nodes (spec entry 1). +func formatProviderID(vmid int, name string) string { + return idPrefix + "/" + strconv.Itoa(vmid) + "/" + name +} + +// parseProviderID is strict: exactly three segments, the literal "vms", a +// decimal VMID without leading zeros in Proxmox's range, and a non-empty +// name. Anything else is ErrPermanent. +func parseProviderID(id string) (int, string, error) { + parts := strings.Split(id, "/") + if len(parts) != 3 || parts[0] != idPrefix || parts[2] == "" { + return 0, "", permanentf("malformed providerID %q, want vms//", id) + } + vmid, err := strconv.Atoi(parts[1]) + if err != nil || strconv.Itoa(vmid) != parts[1] || vmid < provider.ProxmoxMinVMID || vmid > provider.ProxmoxMaxVMID { + return 0, "", permanentf("malformed providerID %q: %q is not a VMID", id, parts[1]) + } + return vmid, parts[2], nil +} + +// vmidFor is the attempt-th VMID candidate for a Proxy UID: a stable hash of +// the UID placed in [min, max], moving forward and wrapping as attempt grows, +// so a retry of the same Create lands on the same VMID (spec entry 2). +func vmidFor(uid string, min, max, attempt int) int { + h := fnv.New64a() + _, _ = h.Write([]byte(uid)) + span := uint64(max - min + 1) + return min + int((h.Sum64()%span+uint64(attempt)%span)%span) +} diff --git a/internal/provider/proxmox/id_test.go b/internal/provider/proxmox/id_test.go new file mode 100644 index 0000000..e86a84a --- /dev/null +++ b/internal/provider/proxmox/id_test.go @@ -0,0 +1,89 @@ +package proxmox + +import ( + "errors" + "testing" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +func TestFormatProviderID_roundTrip(t *testing.T) { + t.Parallel() + for _, vmid := range []int{100, 150042, 999999999} { + id := formatProviderID(vmid, "proxy-abcdefghijklmnop") + gotVMID, gotName, err := parseProviderID(id) + if err != nil || gotVMID != vmid || gotName != "proxy-abcdefghijklmnop" { + t.Errorf("parseProviderID(%q) = %d, %q, %v; want %d, proxy-abcdefghijklmnop, nil", id, gotVMID, gotName, err, vmid) + } + } + if got := formatProviderID(150042, "proxy-x"); got != "vms/150042/proxy-x" { + t.Errorf("formatProviderID() = %q, want vms/150042/proxy-x", got) + } +} + +func TestParseProviderID_malformed(t *testing.T) { + t.Parallel() + for _, id := range []string{ + "", + "vms", + "vms/150042", + "vms/150042/", + "vms//proxy-x", + "vm/150042/proxy-x", + "hosts/beelink/domains/proxy-x", + "vms/150042/proxy-x/extra", + "vms/abc/proxy-x", + "vms/0150042/proxy-x", + "vms/+150042/proxy-x", + "vms/-5/proxy-x", + "vms/99/proxy-x", + "vms/1000000000/proxy-x", + } { + t.Run(id, func(t *testing.T) { + t.Parallel() + _, _, err := parseProviderID(id) + if !errors.Is(err, provider.ErrPermanent) { + t.Errorf("parseProviderID(%q) error = %v, want ErrPermanent", id, err) + } + }) + } +} + +func TestVMIDFor_deterministicAndInRange(t *testing.T) { + t.Parallel() + uids := []string{"3f1c2d7e-8a44-4f1b-9a6c-1d2e3f4a5b6c", "00000000-0000-0000-0000-000000000000", "x"} + for _, uid := range uids { + first := vmidFor(uid, 100000, 199999, 0) + if first < 100000 || first > 199999 { + t.Errorf("vmidFor(%q) = %d, want within 100000..199999", uid, first) + } + if again := vmidFor(uid, 100000, 199999, 0); again != first { + t.Errorf("vmidFor(%q) = %d then %d, want the same VMID every time", uid, first, again) + } + } + if vmidFor(uids[0], 100000, 199999, 0) == vmidFor(uids[1], 100000, 199999, 0) { + t.Error("two different UIDs got the same first VMID; the hash does not spread") + } +} + +func TestVMIDFor_probesForwardAndWraps(t *testing.T) { + t.Parallel() + const min, max = 500, 509 + uid := "3f1c2d7e-8a44-4f1b-9a6c-1d2e3f4a5b6c" + start := vmidFor(uid, min, max, 0) + seen := map[int]bool{} + for attempt := range 10 { + got := vmidFor(uid, min, max, attempt) + want := min + (start-min+attempt)%10 + if got != want { + t.Errorf("attempt %d: vmidFor = %d, want %d (one step forward, wrapping at max)", attempt, got, want) + } + seen[got] = true + } + if len(seen) != 10 { + t.Errorf("10 attempts over a range of 10 visited %d VMIDs, want all 10", len(seen)) + } + if got := vmidFor(uid, min, max, 10); got != start { + t.Errorf("attempt 10 = %d, want to wrap back to %d", got, start) + } +} diff --git a/internal/provider/proxmox/mark.go b/internal/provider/proxmox/mark.go new file mode 100644 index 0000000..76954f0 --- /dev/null +++ b/internal/provider/proxmox/mark.go @@ -0,0 +1,54 @@ +package proxmox + +import ( + "strings" + "time" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// createdAtKey is the third line of the mark; the first two use the +// contract's label names. +const createdAtKey = "proxy-operator-created-at" + +// instanceMark is the GC mark as read back from a VM's description (spec +// entry 8). +type instanceMark struct { + Managed bool + UID string + CreatedAt time.Time +} + +// formatDescription renders the mark written in the clone call. +func formatDescription(uid string, createdAt time.Time) string { + return provider.LabelManaged + "=" + provider.LabelManagedYes + "\n" + + provider.LabelUID + "=" + uid + "\n" + + createdAtKey + "=" + createdAt.UTC().Format(time.RFC3339) +} + +// parseDescription reads key=value lines, tolerating CRLF, blank lines and +// lines an admin added. The first occurrence of a key wins. Managed only +// when the value is exactly LabelManagedYes; an unparsable created-at gives +// the zero time, which GC treats as old. +func parseDescription(desc string) instanceMark { + var m instanceMark + seen := map[string]bool{} + for line := range strings.SplitSeq(desc, "\n") { + key, value, ok := strings.Cut(strings.TrimSpace(line), "=") + if !ok || seen[key] { + continue + } + seen[key] = true + switch key { + case provider.LabelManaged: + m.Managed = value == provider.LabelManagedYes + case provider.LabelUID: + m.UID = value + case createdAtKey: + if t, err := time.Parse(time.RFC3339, value); err == nil { + m.CreatedAt = t + } + } + } + return m +} diff --git a/internal/provider/proxmox/mark_test.go b/internal/provider/proxmox/mark_test.go new file mode 100644 index 0000000..56004e3 --- /dev/null +++ b/internal/provider/proxmox/mark_test.go @@ -0,0 +1,64 @@ +package proxmox + +import ( + "testing" + "time" +) + +func TestFormatDescription_keys(t *testing.T) { + t.Parallel() + created := time.Date(2026, 9, 23, 8, 0, 0, 0, time.FixedZone("CEST", 2*3600)) + got := formatDescription("3f1c2d7e-uid", created) + want := "proxy-operator-managed=true\nproxy-operator-uid=3f1c2d7e-uid\nproxy-operator-created-at=2026-09-23T06:00:00Z" + if got != want { + t.Errorf("formatDescription() =\n%s\nwant\n%s", got, want) + } + m := parseDescription(got) + if !m.Managed || m.UID != "3f1c2d7e-uid" || !m.CreatedAt.Equal(created) { + t.Errorf("parseDescription(formatDescription()) = %+v, want managed, the UID and the time back", m) + } +} + +// TestParseDescription_fromClone reads the mark back from the config of the +// VM cloned in Phase 0, as Proxmox stored it. +func TestParseDescription_fromClone(t *testing.T) { + t.Parallel() + cfg := decodeData[map[string]any](t, "vm_config_clone") + desc, _ := cfg["description"].(string) + m := parseDescription(desc) + want := time.Date(2026, 9, 23, 7, 0, 0, 0, time.UTC) + if !m.Managed || m.UID != "00000000-0000-0000-0000-00000000recon" || !m.CreatedAt.Equal(want) { + t.Errorf("parseDescription(captured) = %+v, want the recon mark", m) + } +} + +func TestParseDescription_table(t *testing.T) { + t.Parallel() + ts := time.Date(2026, 9, 23, 7, 0, 0, 0, time.UTC) + tests := []struct { + name string + desc string + want instanceMark + }{ + {name: "empty", desc: "", want: instanceMark{}}, + {name: "unmarked free text", desc: "Ubuntu VM for testing", want: instanceMark{}}, + {name: "managed with UID", desc: "proxy-operator-managed=true\nproxy-operator-uid=u1\nproxy-operator-created-at=2026-09-23T07:00:00Z", want: instanceMark{Managed: true, UID: "u1", CreatedAt: ts}}, + {name: "managed with empty UID", desc: "proxy-operator-managed=true\nproxy-operator-uid=", want: instanceMark{Managed: true}}, + {name: "managed without UID line", desc: "proxy-operator-managed=true", want: instanceMark{Managed: true}}, + {name: "managed value must be exactly true", desc: "proxy-operator-managed=yes\nproxy-operator-uid=u1", want: instanceMark{UID: "u1"}}, + {name: "unparsable created-at gives zero time", desc: "proxy-operator-managed=true\nproxy-operator-created-at=yesterday", want: instanceMark{Managed: true}}, + {name: "CRLF line endings", desc: "proxy-operator-managed=true\r\nproxy-operator-uid=u1\r\n", want: instanceMark{Managed: true, UID: "u1"}}, + {name: "admin lines kept around the mark", desc: "owner: team-crawl\n\nproxy-operator-managed=true\nnote=do not touch\nproxy-operator-uid=u1", want: instanceMark{Managed: true, UID: "u1"}}, + {name: "first occurrence of a key wins", desc: "proxy-operator-uid=u1\nproxy-operator-uid=u2\nproxy-operator-managed=true", want: instanceMark{Managed: true, UID: "u1"}}, + {name: "surrounding spaces trimmed", desc: " proxy-operator-managed=true \n proxy-operator-uid=u1", want: instanceMark{Managed: true, UID: "u1"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := parseDescription(tc.desc) + if got.Managed != tc.want.Managed || got.UID != tc.want.UID || !got.CreatedAt.Equal(tc.want.CreatedAt) { + t.Errorf("parseDescription(%q) = %+v, want %+v", tc.desc, got, tc.want) + } + }) + } +} diff --git a/internal/provider/proxmox/proxmox.go b/internal/provider/proxmox/proxmox.go new file mode 100644 index 0000000..d3875b5 --- /dev/null +++ b/internal/provider/proxmox/proxmox.go @@ -0,0 +1,173 @@ +// Package proxmox implements the provider contract on a Proxmox VE cluster +// through its REST API: VMs are full clones of prepared templates, seeded +// with a NoCloud ISO uploaded to a storage and addressed through the guest +// agent. The design is in docs/proxmox-provider-spec.md. +package proxmox + +import ( + "context" + "os" + "time" + + "github.com/go-logr/logr" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +const ( + defaultVMIDMin = 100000 + defaultVMIDMax = 199999 + defaultRequestTimeoutSeconds = 30 +) + +// budgets bounds every wait of the provider (spec entry 18); tests shrink +// them. +type budgets struct { + clone time.Duration // the clone task, and waiting for a VM's lock + upload time.Duration + resize time.Duration + start time.Duration + stop time.Duration + destroy time.Duration + seedDelete time.Duration + running time.Duration // from the start task's end to status running + poll time.Duration // task and status poll interval +} + +func defaultBudgets() budgets { + return budgets{ + clone: 10 * time.Minute, + upload: 2 * time.Minute, + resize: 2 * time.Minute, + start: 2 * time.Minute, + stop: 2 * time.Minute, + destroy: 5 * time.Minute, + seedDelete: time.Minute, + running: 60 * time.Second, + poll: time.Second, + } +} + +// Provider is one configured proxmox entry: one cluster reached through one +// API URL with one token. +type Provider struct { + name string + cfg provider.ProxmoxConfig + api api + now func() time.Time + budgets budgets +} + +var _ provider.Provider = (*Provider)(nil) + +// New builds a Provider from pc.Proxmox. It never makes a request: the token +// file is only checked for existence, and a missing one is a warning, not an +// error, because the Secret mount is optional and may land later. +func New(ctx context.Context, pc provider.ProviderConfig) (provider.Provider, error) { + cfg := provider.ProxmoxConfig{} + if pc.Proxmox != nil { + cfg = *pc.Proxmox + } + // The HTTP client behind the api seam arrives in Phase 4 of the plan; + // until then every method is a stub that makes no request. + p := newProvider(pc.Name, cfg, nil) + log := p.logger(ctx) + if _, err := os.Stat(p.cfg.TokenFile); err != nil { + log.Info("WARNING: token file is not readable; every call fails until it is", "path", p.cfg.TokenFile, "reason", err.Error()) + } + if p.cfg.InsecureSkipTLSVerify { + log.Info("WARNING: insecureSkipTLSVerify is set; the API's certificate is not verified") + } + return p, nil +} + +// newProvider is the constructor tests use, with a in-memory api. +func newProvider(name string, cfg provider.ProxmoxConfig, a api) *Provider { + return &Provider{ + name: name, + cfg: withDefaults(cfg), + api: a, + now: time.Now, + budgets: defaultBudgets(), + } +} + +// withDefaults returns cfg with every optional field filled in, so the rest +// of the package never re-checks for an empty value. Slices, maps and +// pointers are copied, not shared with the caller. +func withDefaults(cfg provider.ProxmoxConfig) provider.ProxmoxConfig { + cfg.Nodes = append([]string(nil), cfg.Nodes...) + if cfg.VMIDRange == nil { + cfg.VMIDRange = &provider.ProxmoxVMIDRange{Min: defaultVMIDMin, Max: defaultVMIDMax} + } else { + r := *cfg.VMIDRange + cfg.VMIDRange = &r + } + if cfg.RequestTimeoutSeconds == 0 { + cfg.RequestTimeoutSeconds = defaultRequestTimeoutSeconds + } + install := true + if cfg.InstallGuestAgent != nil { + install = *cfg.InstallGuestAgent + } + cfg.InstallGuestAgent = &install + templates := make(map[string]provider.ProxmoxTemplate, len(cfg.Templates)) + for k, v := range cfg.Templates { + templates[k] = v + } + cfg.Templates = templates + machineTypes := make(map[string]provider.ProxmoxMachineType, len(cfg.MachineTypes)) + for k, v := range cfg.MachineTypes { + machineTypes[k] = v + } + cfg.MachineTypes = machineTypes + return cfg +} + +func (p *Provider) logger(ctx context.Context) logr.Logger { + return logf.FromContext(ctx).WithName("proxmox").WithValues("provider", p.name) +} + +// node resolves placement.zone to a configured node. An empty zone is +// allowed only when exactly one node is configured. +func (p *Provider) node(zone string) (string, error) { + if zone == "" { + if len(p.cfg.Nodes) == 1 { + return p.cfg.Nodes[0], nil + } + return "", permanentf("placement.zone is required when proxmox.nodes lists more than one node") + } + for _, n := range p.cfg.Nodes { + if n == zone { + return n, nil + } + } + return "", permanentf("placement.zone %q is not in proxmox.nodes", zone) +} + +// wrapErr is the single exit for errors: it classifies err and wraps it with +// the configured provider name, op and id. +func (p *Provider) wrapErr(op, id string, err error) error { + return provider.Wrap(classify(op, err), op, p.name, id, err) +} + +// The four contract methods are stubs until the phases that implement them +// (Get and ListByTag in Phase 5, Create in 6, Delete in 7). Each answers +// Transient, so a Proxy on this provider retries rather than latching. + +func (p *Provider) Create(_ context.Context, req provider.CreateRequest) (string, error) { + return "", p.wrapErr("create", "", transientf("proxmox Create is not implemented yet (request %s)", req.Name)) +} + +func (p *Provider) Get(_ context.Context, id string) (*provider.Instance, error) { + return nil, p.wrapErr("get", id, transientf("proxmox Get is not implemented yet")) +} + +func (p *Provider) Delete(_ context.Context, id string) error { + return p.wrapErr("delete", id, transientf("proxmox Delete is not implemented yet")) +} + +func (p *Provider) ListByTag(_ context.Context) ([]provider.Instance, error) { + return nil, p.wrapErr("list", "", transientf("proxmox ListByTag is not implemented yet")) +} diff --git a/internal/provider/proxmox/proxmox_test.go b/internal/provider/proxmox/proxmox_test.go new file mode 100644 index 0000000..1b25f70 --- /dev/null +++ b/internal/provider/proxmox/proxmox_test.go @@ -0,0 +1,208 @@ +package proxmox + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + "time" + + "github.com/go-logr/logr/funcr" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// captureContext collects funcr output at V(2). +func captureContext() (context.Context, *[]string) { + lines := &[]string{} + var mu sync.Mutex + log := funcr.New(func(prefix, args string) { + mu.Lock() + defer mu.Unlock() + *lines = append(*lines, prefix+" "+args) + }, funcr.Options{Verbosity: 2}) + return logf.IntoContext(context.Background(), log), lines +} + +func testConfig() provider.ProxmoxConfig { + return provider.ProxmoxConfig{ + URL: "https://192.0.2.8:8006", + TokenFile: "/nonexistent/proxmox/token", + Nodes: []string{"pv1-1"}, + ISOStorage: "proxy-seeds", + Templates: map[string]provider.ProxmoxTemplate{"debian-13": {VMID: 9001}}, + MachineTypes: map[string]provider.ProxmoxMachineType{"small": {Cores: 1, MemoryMiB: 1024}}, + } +} + +func TestNew_doesNotDialAndWarnsOnMissingToken(t *testing.T) { + t.Parallel() + ctx, lines := captureContext() + cfg := testConfig() + start := time.Now() + p, err := New(ctx, provider.ProviderConfig{Name: "proxmox-home", Type: "proxmox", Proxmox: &cfg}) + if err != nil || p == nil { + t.Fatalf("New() = %v, %v; want a provider", p, err) + } + if elapsed := time.Since(start); elapsed > 100*time.Millisecond { + t.Errorf("New() took %v, want under 100ms (it must not make a request)", elapsed) + } + var warnings int + for _, l := range *lines { + if strings.Contains(l, "WARNING:") && strings.Contains(l, "token file") { + warnings++ + } + if !strings.Contains(l, `"provider"="proxmox-home"`) { + t.Errorf("log line lacks the provider value: %s", l) + } + } + if warnings != 1 { + t.Errorf("got %d token WARNING: lines, want 1:\n%s", warnings, strings.Join(*lines, "\n")) + } +} + +func TestNew_insecureSkipTLSVerifyWarns(t *testing.T) { + t.Parallel() + ctx, lines := captureContext() + cfg := testConfig() + cfg.InsecureSkipTLSVerify = true + if _, err := New(ctx, provider.ProviderConfig{Name: "proxmox-home", Type: "proxmox", Proxmox: &cfg}); err != nil { + t.Fatalf("New() error = %v, want nil", err) + } + for _, l := range *lines { + if strings.Contains(l, "WARNING:") && strings.Contains(l, "not verified") { + return + } + } + t.Errorf("no insecureSkipTLSVerify warning in:\n%s", strings.Join(*lines, "\n")) +} + +func TestNew_nilBlockDoesNotPanic(t *testing.T) { + t.Parallel() + ctx, _ := captureContext() + p, err := New(ctx, provider.ProviderConfig{Name: "proxmox-empty", Type: "proxmox"}) + if err != nil { + t.Fatalf("New() error = %v, want nil", err) + } + pp, ok := p.(*Provider) + if !ok { + t.Fatalf("New() = %T, want *Provider", p) + } + if pp.name != "proxmox-empty" || pp.cfg.VMIDRange == nil || pp.now == nil { + t.Errorf("provider = %+v, want its name, defaults and a clock", pp) + } +} + +func TestWithDefaults_table(t *testing.T) { + t.Parallel() + off := false + tests := []struct { + name string + in provider.ProxmoxConfig + wantRange provider.ProxmoxVMIDRange + wantTimeout int + wantAgent bool + }{ + {name: "empty fills every default", wantRange: provider.ProxmoxVMIDRange{Min: 100000, Max: 199999}, wantTimeout: 30, wantAgent: true}, + { + name: "explicit values kept", + in: provider.ProxmoxConfig{VMIDRange: &provider.ProxmoxVMIDRange{Min: 500, Max: 600}, RequestTimeoutSeconds: 45, InstallGuestAgent: &off}, + wantRange: provider.ProxmoxVMIDRange{Min: 500, Max: 600}, + wantTimeout: 45, + wantAgent: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := withDefaults(tc.in) + if *got.VMIDRange != tc.wantRange || got.RequestTimeoutSeconds != tc.wantTimeout || *got.InstallGuestAgent != tc.wantAgent { + t.Errorf("withDefaults() = range %+v timeout %d agent %v; want %+v %d %v", + *got.VMIDRange, got.RequestTimeoutSeconds, *got.InstallGuestAgent, tc.wantRange, tc.wantTimeout, tc.wantAgent) + } + }) + } +} + +func TestWithDefaults_doesNotShareWithCaller(t *testing.T) { + t.Parallel() + in := testConfig() + in.VMIDRange = &provider.ProxmoxVMIDRange{Min: 500, Max: 600} + got := withDefaults(in) + got.Nodes[0] = "changed" + got.VMIDRange.Min = 1 + got.Templates["debian-13"] = provider.ProxmoxTemplate{VMID: 1} + got.MachineTypes["small"] = provider.ProxmoxMachineType{} + if in.Nodes[0] != "pv1-1" || in.VMIDRange.Min != 500 || in.Templates["debian-13"].VMID != 9001 || in.MachineTypes["small"].Cores != 1 { + t.Errorf("mutating the result changed the caller's config: %+v", in) + } +} + +func TestNode_table(t *testing.T) { + t.Parallel() + tests := []struct { + name string + nodes []string + zone string + want string + wantErr string + }{ + {name: "one node and empty zone selects it", nodes: []string{"pv1-1"}, zone: "", want: "pv1-1"}, + {name: "several nodes and empty zone is permanent", nodes: []string{"pv1-1", "pv1-2"}, zone: "", wantErr: "placement.zone is required"}, + {name: "known node", nodes: []string{"pv1-1", "pv1-2"}, zone: "pv1-2", want: "pv1-2"}, + {name: "unknown node is permanent", nodes: []string{"pv1-1"}, zone: "pv9", wantErr: `placement.zone "pv9" is not in proxmox.nodes`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cfg := testConfig() + cfg.Nodes = tc.nodes + got, err := newProvider("proxmox-home", cfg, nil).node(tc.zone) + if tc.wantErr == "" { + if err != nil || got != tc.want { + t.Errorf("node(%q) = %q, %v; want %q, nil", tc.zone, got, err, tc.want) + } + return + } + if !errors.Is(err, provider.ErrPermanent) || !strings.Contains(err.Error(), tc.wantErr) { + t.Errorf("node(%q) error = %v, want ErrPermanent containing %q", tc.zone, err, tc.wantErr) + } + }) + } +} + +func TestWrapErr_carriesProviderNameOpAndID(t *testing.T) { + t.Parallel() + p := newProvider("proxmox-home", testConfig(), nil) + err := p.wrapErr("get", "vms/150042/proxy-x", loadAPIError(t, "err_vm_missing")) + var pe *provider.Error + if !errors.As(err, &pe) { + t.Fatalf("wrapErr() = %T, want *provider.Error", err) + } + if pe.Provider != "proxmox-home" || pe.Op != "get" || pe.ID != "vms/150042/proxy-x" || !errors.Is(err, provider.ErrNotFound) { + t.Errorf("wrapErr() = %+v, want configured name, op, id and class NotFound", pe) + } + var ae *apiError + if !errors.As(err, &ae) || ae.Status != 500 { + t.Errorf("wrapErr() lost the underlying apiError: %v", err) + } +} + +// TestStubs_answerTransient keeps a Proxy on this provider retrying, not +// latched, until the methods are implemented in later phases. +func TestStubs_answerTransient(t *testing.T) { + t.Parallel() + ctx := context.Background() + p := newProvider("proxmox-home", testConfig(), nil) + _, createErr := p.Create(ctx, provider.CreateRequest{Name: "proxy-x"}) + _, getErr := p.Get(ctx, "vms/150042/proxy-x") + deleteErr := p.Delete(ctx, "vms/150042/proxy-x") + _, listErr := p.ListByTag(ctx) + for op, err := range map[string]error{"create": createErr, "get": getErr, "delete": deleteErr, "list": listErr} { + if !errors.Is(err, provider.ErrTransient) { + t.Errorf("%s error = %v, want ErrTransient", op, err) + } + } +} diff --git a/internal/provider/proxmox/state.go b/internal/provider/proxmox/state.go new file mode 100644 index 0000000..4807d7a --- /dev/null +++ b/internal/provider/proxmox/state.go @@ -0,0 +1,52 @@ +package proxmox + +import ( + "net/netip" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +// mapState turns status/current's status and qmpstatus plus the discovered +// address into the contract's state (spec entry 9). A VM is Running only +// with an address; prelaunch is still starting; any other qmpstatus of a +// running VM (paused, suspended, io-error, …) and any status not recognised +// is Stopped, which the reconciler answers by replacing the VM. An empty +// qmpstatus is taken as running: the field is missing, not a state. +func mapState(status, qmpStatus, ip string) provider.InstanceState { + if status != "running" { + return provider.StateStopped + } + switch qmpStatus { + case "running", "": + if ip == "" { + return provider.StateProvisioning + } + return provider.StateRunning + case "prelaunch": + return provider.StateProvisioning + default: + return provider.StateStopped + } +} + +// firstIPv4 picks the first usable IPv4 from the guest agent's interfaces, +// in the agent's order: not on the loopback interface, not loopback, +// link-local or unspecified. Empty means no address yet. +func firstIPv4(ifaces []agentInterface) string { + for _, iface := range ifaces { + if iface.Name == "lo" { + continue + } + for _, a := range iface.IPAddresses { + if a.Type != "ipv4" { + continue + } + addr, err := netip.ParseAddr(a.Address) + if err != nil || !addr.Is4() || addr.IsLoopback() || addr.IsLinkLocalUnicast() || addr.IsUnspecified() { + continue + } + return addr.String() + } + } + return "" +} diff --git a/internal/provider/proxmox/state_test.go b/internal/provider/proxmox/state_test.go new file mode 100644 index 0000000..55de709 --- /dev/null +++ b/internal/provider/proxmox/state_test.go @@ -0,0 +1,114 @@ +package proxmox + +import ( + "testing" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/provider" +) + +func TestMapState_table(t *testing.T) { + t.Parallel() + tests := []struct { + status, qmp, ip string + want provider.InstanceState + }{ + {"running", "running", "192.168.4.147", provider.StateRunning}, + {"running", "running", "", provider.StateProvisioning}, + {"running", "", "192.168.4.147", provider.StateRunning}, + {"running", "", "", provider.StateProvisioning}, + {"running", "prelaunch", "", provider.StateProvisioning}, + {"running", "paused", "192.168.4.147", provider.StateStopped}, + {"running", "suspended", "", provider.StateStopped}, + {"running", "io-error", "", provider.StateStopped}, + {"running", "something-new", "", provider.StateStopped}, + {"stopped", "stopped", "", provider.StateStopped}, + {"stopped", "", "", provider.StateStopped}, + {"unknown", "", "", provider.StateStopped}, + {"", "", "", provider.StateStopped}, + } + for _, tc := range tests { + t.Run(tc.status+"/"+tc.qmp+"/"+tc.ip, func(t *testing.T) { + t.Parallel() + if got := mapState(tc.status, tc.qmp, tc.ip); got != tc.want { + t.Errorf("mapState(%q, %q, %q) = %q, want %q", tc.status, tc.qmp, tc.ip, got, tc.want) + } + }) + } +} + +// TestMapState_fromCaptures feeds status/current as Proxmox reported it for +// a running, a stopped and a paused VM. +func TestMapState_fromCaptures(t *testing.T) { + t.Parallel() + tests := []struct { + capture string + ip string + want provider.InstanceState + }{ + {"status_current_running", "192.168.0.150", provider.StateRunning}, + {"status_current_running", "", provider.StateProvisioning}, + {"status_current_stopped", "", provider.StateStopped}, + {"status_current_paused", "192.168.0.150", provider.StateStopped}, + } + for _, tc := range tests { + t.Run(tc.capture+"/"+tc.ip, func(t *testing.T) { + t.Parallel() + s := decodeData[vmStatus](t, tc.capture) + if got := mapState(s.Status, s.QMPStatus, tc.ip); got != tc.want { + t.Errorf("mapState(%q, %q, %q) = %q, want %q", s.Status, s.QMPStatus, tc.ip, got, tc.want) + } + }) + } +} + +func TestFirstIPv4_filters(t *testing.T) { + t.Parallel() + ip := func(addr, typ string) agentIP { return agentIP{Address: addr, Type: typ} } + tests := []struct { + name string + ifaces []agentInterface + want string + }{ + {name: "none", want: ""}, + {name: "loopback interface only", ifaces: []agentInterface{{Name: "lo", IPAddresses: []agentIP{ip("127.0.0.1", "ipv4")}}}, want: ""}, + {name: "ipv6 only", ifaces: []agentInterface{{Name: "ens18", IPAddresses: []agentIP{ip("fe80::1", "ipv6"), ip("2001:db8::5", "ipv6")}}}, want: ""}, + {name: "link-local skipped", ifaces: []agentInterface{{Name: "ens18", IPAddresses: []agentIP{ip("169.254.3.4", "ipv4"), ip("192.168.4.20", "ipv4")}}}, want: "192.168.4.20"}, + {name: "unspecified skipped", ifaces: []agentInterface{{Name: "ens18", IPAddresses: []agentIP{ip("0.0.0.0", "ipv4")}}}, want: ""}, + {name: "loopback address on another interface skipped", ifaces: []agentInterface{{Name: "dummy0", IPAddresses: []agentIP{ip("127.0.0.2", "ipv4")}}}, want: ""}, + {name: "type and literal disagree", ifaces: []agentInterface{{Name: "ens18", IPAddresses: []agentIP{ip("fe80::1", "ipv4"), ip("10.0.0.9", "ipv6")}}}, want: ""}, + {name: "unparsable skipped", ifaces: []agentInterface{{Name: "ens18", IPAddresses: []agentIP{ip("not-an-ip", "ipv4"), ip("10.0.0.9", "ipv4")}}}, want: "10.0.0.9"}, + {name: "agent order decides", ifaces: []agentInterface{ + {Name: "ens18", IPAddresses: []agentIP{ip("192.168.4.20", "ipv4")}}, + {Name: "ens19", IPAddresses: []agentIP{ip("10.0.0.9", "ipv4")}}, + }, want: "192.168.4.20"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := firstIPv4(tc.ifaces); got != tc.want { + t.Errorf("firstIPv4() = %q, want %q", got, tc.want) + } + }) + } +} + +// TestFirstIPv4_fromCaptures reads the guest agent's answer from the two +// Phase 0 runs, untagged and on VLAN 4. +func TestFirstIPv4_fromCaptures(t *testing.T) { + t.Parallel() + tests := map[string]string{ + "agent_network_get_interfaces": "192.168.0.150", + "agent_network_get_interfaces_vlan": "192.168.4.147", + } + for capture, want := range tests { + t.Run(capture, func(t *testing.T) { + t.Parallel() + res := decodeData[struct { + Result []agentInterface `json:"result"` + }](t, capture) + if got := firstIPv4(res.Result); got != want { + t.Errorf("firstIPv4(%s) = %q, want %q", capture, got, want) + } + }) + } +}