diff --git a/cmd/spinifex/cmd/admin.go b/cmd/spinifex/cmd/admin.go index 34bef4be4..f55c35a03 100644 --- a/cmd/spinifex/cmd/admin.go +++ b/cmd/spinifex/cmd/admin.go @@ -522,6 +522,8 @@ func runimagesImportCmd(cmd *cobra.Command, args []string) { manifest.AMIMetadata.ImageOwnerAlias = "system" manifest.AMIMetadata.VolumeSizeGiB = utils.SafeInt64ToUint64(imageStat.Size() / 1024 / 1024 / 1024) manifest.AMIMetadata.BootMode = image.BootMode + manifest.AMIMetadata.Distro = image.Distro + manifest.AMIMetadata.DistroFamily = utils.DistroFamily(image.Distro) // Copy catalog-provided tags (e.g. spinifex:managed-by for system AMIs) // onto the imported AMI so the UI can filter them out. diff --git a/go.mod b/go.mod index 41f6f5750..7295b00fe 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/kdomanski/iso9660 v0.4.0 github.com/klauspost/cpuid/v2 v2.3.0 github.com/mulgadc/predastore v1.4.0 - github.com/mulgadc/viperblock v1.4.1 + github.com/mulgadc/viperblock v1.4.2 github.com/nats-io/nats-server/v2 v2.14.0 github.com/nats-io/nats.go v1.52.0 github.com/ovn-kubernetes/libovsdb v0.8.1 diff --git a/go.sum b/go.sum index ec8279986..a86a0c447 100644 --- a/go.sum +++ b/go.sum @@ -288,8 +288,8 @@ github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= github.com/mulgadc/predastore v1.4.0 h1:txPZCaVT1cDpBotx8zHPXHZD3Mz62+n207zXISkK4uU= github.com/mulgadc/predastore v1.4.0/go.mod h1:fXIGz7yCKcHh68ybcw7aigdOdzIfyt19h/0diPF+JCU= -github.com/mulgadc/viperblock v1.4.1 h1:DCnZctlmY0clw4wV+3TohBWuivwATi0+/5yA4Jdaezw= -github.com/mulgadc/viperblock v1.4.1/go.mod h1:hHdfo02iJgCdNktiPAq+x+VQE0ZNYOLJXw0abJ81LeE= +github.com/mulgadc/viperblock v1.4.2 h1:05mp5kCIar70pxkYx/xFA0lGK9PfR87WGNxUSn34Dbg= +github.com/mulgadc/viperblock v1.4.2/go.mod h1:hHdfo02iJgCdNktiPAq+x+VQE0ZNYOLJXw0abJ81LeE= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= diff --git a/spinifex/handlers/ec2/instance/service_impl.go b/spinifex/handlers/ec2/instance/service_impl.go index 16255ce51..55ec26892 100644 --- a/spinifex/handlers/ec2/instance/service_impl.go +++ b/spinifex/handlers/ec2/instance/service_impl.go @@ -39,7 +39,7 @@ users: - name: {{.Username}} shell: /bin/sh groups: - - sudo + - {{.SudoGroup}} sudo: "ALL=(ALL) NOPASSWD:ALL" ssh_authorized_keys: - {{.SSHKey}} @@ -60,16 +60,22 @@ ca_certs: {{.UserDataCloudConfig}} {{end}} -{{if .UserDataScript}} +{{if or .RHELWriteFiles .UserDataScript}} write_files: +{{- if .RHELWriteFiles}} +{{.RHELWriteFiles}}{{end}}{{if .UserDataScript}} - path: /tmp/cloud-init-startup.sh permissions: '0755' content: | -{{.UserDataScript}} - +{{.UserDataScript}}{{end}} +{{end}} +{{if or .RHELRunCmd .UserDataScript}} runcmd: +{{- if .RHELRunCmd}} +{{.RHELRunCmd}}{{end}}{{if .UserDataScript}} - [ "/bin/bash", "/tmp/cloud-init-startup.sh" ] {{end}} +{{end}} ` const cloudInitMetaTemplate = `# meta-data @@ -91,6 +97,26 @@ const cloudInitNetworkConfigWildcard = `network: dhcp-identifier: mac ` +// cloudInitNetworkConfigDisabled tells cloud-init to do no network rendering +// at all. Used on RHEL-family guests where Spinifex writes NM keyfiles via +// user-data write_files; without this stub cloud-init's RHEL network module +// falls back to autodetect-DHCP and drops a second keyfile next to ours. +const cloudInitNetworkConfigDisabled = `network: + config: disabled +` + +// selectNetworkConfigForFamily returns the cloud-init network-config payload +// for the given distro family. RHEL-family guests get an explicit +// "config: disabled" stub because their NM keyfiles are written via user-data +// write_files; shipping a netplan config alongside would let cloud-init's RHEL +// network module drop a competing cloud-init-enpXsY.nmconnection. +func selectNetworkConfigForFamily(distroFamily, eniMAC, devMAC, mgmtMAC, mgmtIP string, extraENIMACs []string) string { + if distroFamily == "rhel" { + return cloudInitNetworkConfigDisabled + } + return generateNetworkConfig(eniMAC, devMAC, mgmtMAC, mgmtIP, extraENIMACs) +} + // generateNetworkConfig produces the cloud-init network-config for the instance. // // Per-interface config is generated when eniMAC is present (VPC NIC). This allows @@ -163,6 +189,116 @@ type CloudInitData struct { UserDataCloudConfig string UserDataScript string CACertPEM string + // DistroFamily selects per-distro branches in the cloud-init template. + // "debian" | "rhel" | "alpine". Empty falls through to debian rendering + // (legacy AMIs registered before the field existed). + DistroFamily string + // SudoGroup is the OS group cloud-init adds the user to for passwordless + // sudo. "sudo" on debian/ubuntu/alpine, "wheel" on RHEL-family. + SudoGroup string + // RHELWriteFiles is the pre-indented YAML block of NM keyfile entries + // (one per NIC) appended under the merged write_files: key. Empty on + // non-RHEL guests; the network-config ISO file carries the YAML netplan + // equivalent instead. + RHELWriteFiles string + // RHELRunCmd is the pre-indented YAML block of runcmd entries (restorecon + // + nmcli reload/up) needed to load and bring up the keyfiles. Empty on + // non-RHEL guests. + RHELRunCmd string +} + +// buildRHELCloudInit produces the cloud-init write_files (NM keyfiles) and +// runcmd (restorecon + nmcli) blocks needed to bring up networking on a +// RHEL-family guest. Returns empty strings when eniMAC is empty (wildcard / +// non-VPC path) so NM's default autoconnect handles the interface. +// +// Mirrors generateNetworkConfig's per-interface structure: vpc0 + optional +// vpc1..N for extra ENIs, dev0 (DHCP with route/DNS suppression), mgmt0 +// (static). Owning the keyfile bytes directly avoids relying on cloud-init's +// v2-to-NM renderer round-tripping dhcp-identifier: mac → dhcp-client-id=mac, +// which OVN's MAC-keyed DHCP requires. +// +// File mode 0600 root:root is load-bearing — NM ignores keyfiles otherwise. +// restorecon defers to host SELinux policy (no-op on non-SELinux systems); +// nmcli reload forces NM to rescan system-connections after write_files +// completes in cloud-init's config stage, and nmcli up brings each interface +// online without waiting on autoconnect heuristics. +func buildRHELCloudInit(eniMAC, devMAC, mgmtMAC, mgmtIP string, extraENIMACs []string) (writeFiles, runCmd string) { + if eniMAC == "" { + return "", "" + } + + var wf, rc strings.Builder + + rc.WriteString(" - [ restorecon, -R, /etc/NetworkManager/system-connections/ ]\n") + rc.WriteString(" - [ nmcli, connection, reload ]\n") + + appendRHELDHCPKeyfile(&wf, "vpc0", eniMAC, false) + rc.WriteString(" - [ nmcli, connection, up, vpc0 ]\n") + + for i, mac := range extraENIMACs { + if mac == "" { + continue + } + name := fmt.Sprintf("vpc%d", i+1) + appendRHELDHCPKeyfile(&wf, name, mac, false) + fmt.Fprintf(&rc, " - [ nmcli, connection, up, %s ]\n", name) + } + + if devMAC != "" { + appendRHELDHCPKeyfile(&wf, "dev0", devMAC, true) + rc.WriteString(" - [ nmcli, connection, up, dev0 ]\n") + } + + if mgmtMAC != "" && mgmtIP != "" { + appendRHELStaticKeyfile(&wf, "mgmt0", mgmtMAC, mgmtIP) + rc.WriteString(" - [ nmcli, connection, up, mgmt0 ]\n") + } + + // Trim trailing newline so the template's {{.RHELWriteFiles}} doesn't + // produce a blank line before the user-script write_files entry on + // instances that have both. + return strings.TrimRight(wf.String(), "\n"), strings.TrimRight(rc.String(), "\n") +} + +func appendRHELDHCPKeyfile(b *strings.Builder, name, mac string, suppressDefaults bool) { + fmt.Fprintf(b, " - path: /etc/NetworkManager/system-connections/%s.nmconnection\n", name) + b.WriteString(" owner: root:root\n") + b.WriteString(" permissions: '0600'\n") + b.WriteString(" content: |\n") + b.WriteString(" [connection]\n") + fmt.Fprintf(b, " id=%s\n", name) + b.WriteString(" type=ethernet\n") + b.WriteString(" [ethernet]\n") + fmt.Fprintf(b, " mac-address=%s\n", mac) + b.WriteString(" [ipv4]\n") + b.WriteString(" method=auto\n") + b.WriteString(" dhcp-client-id=mac\n") + if suppressDefaults { + // Equivalent to netplan dhcp4-overrides {use-routes: false, use-dns: false}: + // dev NIC gets an IP for hostfwd but must not install a default route or DNS. + b.WriteString(" never-default=true\n") + b.WriteString(" ignore-auto-dns=true\n") + } + b.WriteString(" [ipv6]\n") + b.WriteString(" method=disabled\n") +} + +func appendRHELStaticKeyfile(b *strings.Builder, name, mac, ip string) { + fmt.Fprintf(b, " - path: /etc/NetworkManager/system-connections/%s.nmconnection\n", name) + b.WriteString(" owner: root:root\n") + b.WriteString(" permissions: '0600'\n") + b.WriteString(" content: |\n") + b.WriteString(" [connection]\n") + fmt.Fprintf(b, " id=%s\n", name) + b.WriteString(" type=ethernet\n") + b.WriteString(" [ethernet]\n") + fmt.Fprintf(b, " mac-address=%s\n", mac) + b.WriteString(" [ipv4]\n") + b.WriteString(" method=manual\n") + fmt.Fprintf(b, " addresses=%s/24\n", ip) + b.WriteString(" [ipv6]\n") + b.WriteString(" method=disabled\n") } type CloudInitMetaData struct { @@ -190,6 +326,33 @@ type volumeParams struct { deleteOnTermination bool } +// floorVolumeSizeToAMI ensures requested is at least the AMI's snapshot size. +// Cloud images embed a fixed-size partition table; truncating below it (e.g. +// a 10 GiB Rocky image requested at the 4 GiB default) hangs the guest in +// dracut waiting on a root UUID that lives past the cut-off. AWS rejects +// under-sized requests with InvalidParameterValue — spinifex silently rounds +// up since we don't surface that error code today and the user-visible +// failure (dracut hang) is worse than a slightly bigger volume. +func floorVolumeSizeToAMI(loader AMIMetaLoader, imageID string, requested int) int { + if loader == nil || !strings.HasPrefix(imageID, "ami-") { + return requested + } + amiMeta, err := loader.GetAMIConfig(imageID) + if err != nil { + slog.Warn("floorVolumeSizeToAMI: AMI metadata fetch failed; using requested size — guest may dracut-hang if requested is smaller than the AMI snapshot", "imageId", imageID, "err", err) + return requested + } + if amiMeta.VolumeSizeGiB == 0 { + return requested + } + amiSize := utils.SafeUint64ToInt64(amiMeta.VolumeSizeGiB) * 1024 * 1024 * 1024 + if amiSize <= int64(requested) { + return requested + } + slog.Info("floorVolumeSizeToAMI: rounding up to AMI snapshot size", "imageId", imageID, "requestedBytes", requested, "amiBytes", amiSize) + return int(amiSize) +} + // parseVolumeParams extracts volume parameters from RunInstancesInput, // applying defaults and resolving AMI-based image IDs. func parseVolumeParams(input *ec2.RunInstancesInput) volumeParams { @@ -802,6 +965,9 @@ func (s *InstanceServiceImpl) StopOrTerminateInstance(instance *vm.VM, command s func (s *InstanceServiceImpl) GenerateVolumes(input *ec2.RunInstancesInput, instance *vm.VM) ([]VolumeInfo, error) { p := parseVolumeParams(input) + if input.ImageId != nil { + p.size = floorVolumeSizeToAMI(s.amiLoader, *input.ImageId, p.size) + } // Capture attach time for the root volume attachTime := time.Now() @@ -1253,11 +1419,40 @@ func (s *InstanceServiceImpl) createCloudInitISO(input *ec2.RunInstancesInput, i slog.Error("failed to read CA cert for guest cloud-init injection", "path", caCertPath, "error", err) } + // Re-read AMI metadata for DistroFamily rather than persisting it on + // vm.VM — cloud-init renders once at first launch and the ISO is sealed, + // so no consumer needs DistroFamily after this point. + var distroFamily string + if s.amiLoader != nil && input.ImageId != nil && *input.ImageId != "" { + amiMeta, err := s.amiLoader.GetAMIConfig(*input.ImageId) + if err != nil { + slog.Warn("createCloudInitISO: AMI metadata fetch failed; cloud-init will render with debian-family defaults (RHEL guests will boot without networking and with wrong sudo group)", "imageId", *input.ImageId, "err", err) + } else { + distroFamily = amiMeta.DistroFamily + } + } + + extraMACs := make([]string, 0, len(instance.ExtraENIs)) + for _, extra := range instance.ExtraENIs { + extraMACs = append(extraMACs, extra.ENIMac) + } + + sudoGroup := "sudo" + var rhelWriteFiles, rhelRunCmd string + if distroFamily == "rhel" { + sudoGroup = "wheel" + rhelWriteFiles, rhelRunCmd = buildRHELCloudInit(instance.ENIMac, instance.DevMAC, instance.MgmtMAC, instance.MgmtIP, extraMACs) + } + userData := CloudInitData{ - Username: "ec2-user", - SSHKey: string(sshKey), - Hostname: hostname, - CACertPEM: caCertPEM, + Username: "ec2-user", + SSHKey: string(sshKey), + Hostname: hostname, + CACertPEM: caCertPEM, + DistroFamily: distroFamily, + SudoGroup: sudoGroup, + RHELWriteFiles: rhelWriteFiles, + RHELRunCmd: rhelRunCmd, } // Decode and classify user-data from RunInstances (base64-encoded). @@ -1319,12 +1514,10 @@ func (s *InstanceServiceImpl) createCloudInitISO(input *ec2.RunInstancesInput, i // Add network-config: per-interface when VPC+dev (suppresses dev default route), // wildcard DHCP otherwise. Extra ENI MACs produce additional DHCP NICs for - // multi-subnet system VMs (multi-AZ ALBs). - extraMACs := make([]string, 0, len(instance.ExtraENIs)) - for _, extra := range instance.ExtraENIs { - extraMACs = append(extraMACs, extra.ENIMac) - } - networkConfig := generateNetworkConfig(instance.ENIMac, instance.DevMAC, instance.MgmtMAC, instance.MgmtIP, extraMACs) + // multi-subnet system VMs (multi-AZ ALBs). RHEL-family guests get a + // "config: disabled" stub here so cloud-init doesn't drop a second NM + // keyfile alongside the ones we wrote via user-data write_files. + networkConfig := selectNetworkConfigForFamily(distroFamily, instance.ENIMac, instance.DevMAC, instance.MgmtMAC, instance.MgmtIP, extraMACs) err = writer.AddFile(strings.NewReader(networkConfig), "network-config") if err != nil { slog.Error("failed to add network-config file", "err", err) diff --git a/spinifex/handlers/ec2/instance/service_impl_test.go b/spinifex/handlers/ec2/instance/service_impl_test.go index 586db1db7..64af21784 100644 --- a/spinifex/handlers/ec2/instance/service_impl_test.go +++ b/spinifex/handlers/ec2/instance/service_impl_test.go @@ -272,6 +272,187 @@ func TestCloudInitTemplateRendering(t *testing.T) { } } +func TestFloorVolumeSizeToAMI(t *testing.T) { + loader := &fakeAMILoader{byID: map[string]viperblock.AMIMetadata{ + "ami-rocky": {VolumeSizeGiB: 10}, + "ami-debian": {VolumeSizeGiB: 3}, + "ami-no-size": {VolumeSizeGiB: 0}, + }} + const fourGiB = 4 * 1024 * 1024 * 1024 + const tenGiB = 10 * 1024 * 1024 * 1024 + const twentyGiB = 20 * 1024 * 1024 * 1024 + + t.Run("ami larger than requested rounds up", func(t *testing.T) { + assert.Equal(t, tenGiB, floorVolumeSizeToAMI(loader, "ami-rocky", fourGiB)) + }) + t.Run("ami smaller than requested keeps requested", func(t *testing.T) { + assert.Equal(t, fourGiB, floorVolumeSizeToAMI(loader, "ami-debian", fourGiB)) + }) + t.Run("requested larger than ami keeps requested", func(t *testing.T) { + assert.Equal(t, twentyGiB, floorVolumeSizeToAMI(loader, "ami-rocky", twentyGiB)) + }) + t.Run("missing VolumeSizeGiB keeps requested (legacy AMI)", func(t *testing.T) { + assert.Equal(t, fourGiB, floorVolumeSizeToAMI(loader, "ami-no-size", fourGiB)) + }) + t.Run("unknown AMI keeps requested", func(t *testing.T) { + assert.Equal(t, fourGiB, floorVolumeSizeToAMI(loader, "ami-unknown", fourGiB)) + }) + t.Run("non-ami image id keeps requested", func(t *testing.T) { + assert.Equal(t, fourGiB, floorVolumeSizeToAMI(loader, "vol-123", fourGiB)) + }) + t.Run("nil loader keeps requested", func(t *testing.T) { + assert.Equal(t, fourGiB, floorVolumeSizeToAMI(nil, "ami-rocky", fourGiB)) + }) +} + +func TestBuildRHELCloudInit(t *testing.T) { + t.Run("empty eniMAC returns empty (wildcard path)", func(t *testing.T) { + wf, rc := buildRHELCloudInit("", "", "", "", nil) + assert.Empty(t, wf) + assert.Empty(t, rc) + }) + + t.Run("vpc0 only", func(t *testing.T) { + wf, rc := buildRHELCloudInit("02:00:00:00:00:01", "", "", "", nil) + // File mode + ownership are load-bearing — NM ignores anything else. + assert.Contains(t, wf, "/etc/NetworkManager/system-connections/vpc0.nmconnection") + assert.Contains(t, wf, "owner: root:root") + assert.Contains(t, wf, "permissions: '0600'") + assert.Contains(t, wf, "mac-address=02:00:00:00:00:01") + assert.Contains(t, wf, "dhcp-client-id=mac") + assert.Contains(t, wf, "method=auto") + assert.NotContains(t, wf, "never-default") + assert.Contains(t, rc, " - [ restorecon, -R, /etc/NetworkManager/system-connections/ ]") + assert.Contains(t, rc, " - [ nmcli, connection, reload ]") + assert.Contains(t, rc, " - [ nmcli, connection, up, vpc0 ]") + }) + + t.Run("vpc0 + extra ENIs + dev + mgmt", func(t *testing.T) { + wf, rc := buildRHELCloudInit( + "02:00:00:00:00:01", + "02:00:00:00:00:99", + "02:00:00:00:00:aa", "10.250.0.5", + []string{"02:00:00:00:00:02", "02:00:00:00:00:03"}, + ) + for _, name := range []string{"vpc0", "vpc1", "vpc2", "dev0", "mgmt0"} { + assert.Contains(t, wf, "/etc/NetworkManager/system-connections/"+name+".nmconnection", name) + assert.Contains(t, rc, " - [ nmcli, connection, up, "+name+" ]", name) + } + // dev NIC must not install default route or DNS. + assert.Contains(t, wf, "never-default=true") + assert.Contains(t, wf, "ignore-auto-dns=true") + // mgmt NIC is static with the supplied IP. + assert.Contains(t, wf, "method=manual") + assert.Contains(t, wf, "addresses=10.250.0.5/24") + }) + + t.Run("empty extra MAC skipped", func(t *testing.T) { + wf, _ := buildRHELCloudInit( + "02:00:00:00:00:01", "", "", "", + []string{"", "02:00:00:00:00:02"}, + ) + // Empty slot is skipped, but index advances — second valid MAC is vpc2. + assert.NotContains(t, wf, "vpc1.nmconnection") + assert.Contains(t, wf, "vpc2.nmconnection") + }) +} + +func TestCloudInitTemplate_FamilyBranching(t *testing.T) { + rhelWF, rhelRC := buildRHELCloudInit("02:00:00:11:22:33", "", "", "", nil) + + t.Run("debian family: sudo group, no NM keyfiles", func(t *testing.T) { + var buf bytes.Buffer + tmpl := template.Must(template.New("c").Parse(cloudInitUserDataTemplate)) + require.NoError(t, tmpl.Execute(&buf, CloudInitData{ + Username: "ec2-user", SSHKey: "ssh-rsa AAA", Hostname: "spinifex-vm-deb", + DistroFamily: "debian", SudoGroup: "sudo", + })) + out := buf.String() + assert.Contains(t, out, "- sudo") + assert.NotContains(t, out, "- wheel") + assert.NotContains(t, out, "NetworkManager/system-connections") + assert.NotContains(t, out, "nmcli") + assert.NotContains(t, out, "restorecon") + }) + + t.Run("rhel family: wheel group, NM keyfile write_files + runcmd", func(t *testing.T) { + var buf bytes.Buffer + tmpl := template.Must(template.New("c").Parse(cloudInitUserDataTemplate)) + require.NoError(t, tmpl.Execute(&buf, CloudInitData{ + Username: "ec2-user", SSHKey: "ssh-rsa AAA", Hostname: "spinifex-vm-rhel", + DistroFamily: "rhel", SudoGroup: "wheel", + RHELWriteFiles: rhelWF, RHELRunCmd: rhelRC, + })) + out := buf.String() + assert.Contains(t, out, "- wheel") + assert.NotContains(t, out, "- sudo\n") + assert.Contains(t, out, "write_files:") + assert.Contains(t, out, "/etc/NetworkManager/system-connections/vpc0.nmconnection") + assert.Contains(t, out, "permissions: '0600'") + assert.Contains(t, out, "dhcp-client-id=mac") + assert.Contains(t, out, "runcmd:") + assert.Contains(t, out, "restorecon") + assert.Contains(t, out, "nmcli, connection, reload") + assert.Contains(t, out, "nmcli, connection, up, vpc0") + }) + + t.Run("rhel + user script: both blocks merged under single write_files/runcmd", func(t *testing.T) { + var buf bytes.Buffer + tmpl := template.Must(template.New("c").Parse(cloudInitUserDataTemplate)) + require.NoError(t, tmpl.Execute(&buf, CloudInitData{ + Username: "ec2-user", SSHKey: "ssh-rsa AAA", Hostname: "spinifex-vm-rhel", + DistroFamily: "rhel", SudoGroup: "wheel", + RHELWriteFiles: rhelWF, RHELRunCmd: rhelRC, + UserDataScript: " #!/bin/bash\n echo hi\n", + })) + out := buf.String() + // YAML disallows duplicate top-level keys; assert each appears exactly once. + assert.Equal(t, 1, strings.Count(out, "\nwrite_files:")) + assert.Equal(t, 1, strings.Count(out, "\nruncmd:")) + assert.Contains(t, out, "/etc/NetworkManager/system-connections/vpc0.nmconnection") + assert.Contains(t, out, "/tmp/cloud-init-startup.sh") + assert.Contains(t, out, "nmcli, connection, up, vpc0") + assert.Contains(t, out, `"/bin/bash", "/tmp/cloud-init-startup.sh"`) + }) +} + +func TestSelectNetworkConfigForFamily(t *testing.T) { + const eniMAC = "02:00:00:00:00:01" + + t.Run("rhel returns disabled stub", func(t *testing.T) { + out := selectNetworkConfigForFamily("rhel", eniMAC, "", "", "", nil) + assert.Equal(t, cloudInitNetworkConfigDisabled, out) + assert.Contains(t, out, "config: disabled") + // Must not emit any netplan v2 keys — those would render alongside + // our NM keyfile and produce a competing cloud-init-enpXsY connection. + assert.NotContains(t, out, "ethernets:") + assert.NotContains(t, out, "version: 2") + }) + + t.Run("debian returns per-interface netplan", func(t *testing.T) { + out := selectNetworkConfigForFamily("debian", eniMAC, "", "", "", nil) + assert.Contains(t, out, "version: 2") + assert.Contains(t, out, "ethernets:") + assert.Contains(t, out, "vpc0:") + assert.Contains(t, out, "dhcp-identifier: mac") + assert.NotContains(t, out, "config: disabled") + }) + + t.Run("empty family falls through to netplan", func(t *testing.T) { + // Legacy AMIs without DistroFamily populated must keep today's behaviour. + out := selectNetworkConfigForFamily("", eniMAC, "", "", "", nil) + assert.Contains(t, out, "version: 2") + assert.NotContains(t, out, "config: disabled") + }) + + t.Run("alpine falls through to netplan", func(t *testing.T) { + // Alpine has no RHEL-style NM keyfile path; reuses netplan. + out := selectNetworkConfigForFamily("alpine", eniMAC, "", "", "", nil) + assert.Contains(t, out, "version: 2") + assert.NotContains(t, out, "config: disabled") + }) +} + func TestCloudInitMetaTemplateRendering(t *testing.T) { data := CloudInitMetaData{ InstanceID: "i-0123456789abcdef0", diff --git a/spinifex/utils/images.go b/spinifex/utils/images.go index 799349dea..ac1b56d7d 100644 --- a/spinifex/utils/images.go +++ b/spinifex/utils/images.go @@ -291,6 +291,34 @@ type Images struct { Tags map[string]string `json:"tags,omitempty"` } +// distroFamilies maps a distro name to its cloud-init family. Family selects +// the per-distro branches in the cloud-init template (sudoers group, +// NetworkManager keyfile vs netplan). Keep keys lowercase; callers normalise. +var distroFamilies = map[string]string{ + "debian": "debian", + "ubuntu": "debian", + "rocky": "rhel", + "rhel": "rhel", + "alma": "rhel", + "fedora": "rhel", + "centos": "rhel", + "alpine": "alpine", +} + +// DistroFamily returns the cloud-init family for distro. Unknown or empty +// distro maps to "debian" (today's default rendering) and logs a warning so +// operators using --file imports for custom appliances aren't broken by a +// missing --distro flag; explicit RHEL-family rendering requires +// --distro rocky|rhel|alma|fedora|centos. +func DistroFamily(distro string) string { + d := strings.ToLower(strings.TrimSpace(distro)) + if family, ok := distroFamilies[d]; ok { + return family + } + slog.Warn("unknown distro, defaulting to debian-family cloud-init", "distro", distro) + return "debian" +} + var AvailableImages = map[string]Images{ "debian-13-x86_64": { @@ -376,6 +404,34 @@ var AvailableImages = map[string]Images{ ChecksumType: "sha512", BootMode: "uefi", }, + + "rocky-10-x86_64": { + Name: "rocky-10-x86_64", + Description: "Rocky Linux 10 x86_64 cloud image", + Distro: "rocky", + Version: "10", + Arch: "x86_64", + Platform: "Linux/UNIX", + CreatedAt: time.Date(2025, 11, 16, 0, 0, 0, 0, time.UTC), + URL: "https://dl.rockylinux.org/pub/rocky/10/images/x86_64/Rocky-10-GenericCloud-Base-10.1-20251116.0.x86_64.qcow2", + Checksum: "https://dl.rockylinux.org/pub/rocky/10/images/x86_64/Rocky-10-GenericCloud-Base-10.1-20251116.0.x86_64.qcow2.CHECKSUM", + ChecksumType: "sha256", + BootMode: "uefi", + }, + + "rocky-10-arm64": { + Name: "rocky-10-arm64", + Description: "Rocky Linux 10 arm64 cloud image", + Distro: "rocky", + Version: "10", + Arch: "arm64", + Platform: "Linux/UNIX", + CreatedAt: time.Date(2025, 11, 16, 0, 0, 0, 0, time.UTC), + URL: "https://dl.rockylinux.org/pub/rocky/10/images/aarch64/Rocky-10-GenericCloud-Base-10.1-20251116.0.aarch64.qcow2", + Checksum: "https://dl.rockylinux.org/pub/rocky/10/images/aarch64/Rocky-10-GenericCloud-Base-10.1-20251116.0.aarch64.qcow2.CHECKSUM", + ChecksumType: "sha256", + BootMode: "uefi", + }, } // AMI / image extraction utils diff --git a/spinifex/utils/utils_test.go b/spinifex/utils/utils_test.go index 0e33bee4b..8cc7465cf 100644 --- a/spinifex/utils/utils_test.go +++ b/spinifex/utils/utils_test.go @@ -1583,3 +1583,63 @@ iQIzBAEBCAAdFiEEnK6Ehq8eQ0z6yqv7tQAaIQAaIQAAaIQFAmJabcdEFGhijklm assert.Equal(t, targetHex, got) }) } + +func TestAvailableImages_Rocky(t *testing.T) { + cases := []struct { + name string + arch string + filenameSub string + }{ + {"rocky-10-x86_64", "x86_64", "x86_64.qcow2"}, + {"rocky-10-arm64", "arm64", "aarch64.qcow2"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + img, ok := AvailableImages[tc.name] + require.True(t, ok, "catalog must contain %q", tc.name) + assert.Equal(t, tc.name, img.Name) + assert.Equal(t, "rocky", img.Distro) + assert.Equal(t, "10", img.Version) + assert.Equal(t, tc.arch, img.Arch) + // Rocky 10 cloud images are UEFI-only on both architectures. + assert.Equal(t, "uefi", img.BootMode) + assert.Equal(t, "sha256", img.ChecksumType) + // URL must be pinned to a dated build (no moving "latest" symlink) + // and the checksum URL must be the BSD-style .CHECKSUM companion. + assert.NotContains(t, img.URL, "latest") + assert.Contains(t, img.URL, tc.filenameSub) + assert.True(t, strings.HasSuffix(img.Checksum, ".CHECKSUM"), + "Rocky publishes BSD-style .CHECKSUM files; got %q", img.Checksum) + // Catalog Distro must resolve to the rhel family — typo here would + // silently render Rocky guests with netplan/sudo group. + assert.Equal(t, "rhel", DistroFamily(img.Distro)) + }) + } +} + +func TestDistroFamily(t *testing.T) { + cases := []struct { + distro string + want string + }{ + {"debian", "debian"}, + {"ubuntu", "debian"}, + {"rocky", "rhel"}, + {"rhel", "rhel"}, + {"alma", "rhel"}, + {"fedora", "rhel"}, + {"centos", "rhel"}, + {"alpine", "alpine"}, + // Case + whitespace normalisation + {" Rocky ", "rhel"}, + {"UBUNTU", "debian"}, + // Unknown and empty fall through to debian with a warning logged. + {"", "debian"}, + {"plan9", "debian"}, + } + for _, tc := range cases { + t.Run(tc.distro, func(t *testing.T) { + assert.Equal(t, tc.want, DistroFamily(tc.distro)) + }) + } +}