From d36543d6d55d016ce62f3361faf44ca51a6de501 Mon Sep 17 00:00:00 2001 From: lasith-kg Date: Fri, 29 Dec 2023 05:43:34 +0000 Subject: [PATCH] (feat): Add test coverage for service.NvmeService() --- coverage.html | 321 ++++++++++++---------------- internal/service/nvme.go | 55 +++-- internal/service/nvme_test.go | 152 +++++++++++++ internal/service/testdata/empty.txt | 0 4 files changed, 313 insertions(+), 215 deletions(-) create mode 100644 internal/service/nvme_test.go delete mode 100644 internal/service/testdata/empty.txt diff --git a/coverage.html b/coverage.html index fb22cc9..584ccdc 100644 --- a/coverage.html +++ b/coverage.html @@ -77,13 +77,13 @@ - + - + - + @@ -403,15 +403,15 @@ } func (a *LabelDeviceAction) Prompt() string { - return fmt.Sprintf("Would you like to label device %s to %s", a.device, a.label) + return fmt.Sprintf("Would you like to label device %s to '%s'", a.device, a.label) } func (a *LabelDeviceAction) Refuse() string { - return fmt.Sprintf("Refused to label to %s", a.label) + return fmt.Sprintf("Refused to label to '%s'", a.label) } func (a *LabelDeviceAction) Success() string { - return fmt.Sprintf("Successfully labelled to %s", a.label) + return fmt.Sprintf("Successfully labelled to '%s'", a.label) } @@ -844,7 +844,7 @@ for name := range c.Devices { _, err := dv.deviceService.GetBlockDevice(name) if err != nil { - return fmt.Errorf("🔴 %s is not a block device", name) + return err } } return nil @@ -1026,7 +1026,7 @@ } type LinuxDeviceService struct { - RunnerFactory utils.RunnerFactory + runnerFactory utils.RunnerFactory } type LsblkBlockDeviceResponse struct { @@ -1042,12 +1042,12 @@ func NewLinuxDeviceService(rc utils.RunnerFactory) *LinuxDeviceService { return &LinuxDeviceService{ - RunnerFactory: rc, + runnerFactory: rc, } } func (du *LinuxDeviceService) GetSize(name string) (uint64, error) { - r := du.RunnerFactory.Select(utils.BlockDev) + r := du.runnerFactory.Select(utils.BlockDev) output, err := r.Command("--getsize64", name) if err != nil { return 0, err @@ -1060,7 +1060,7 @@ } func (du *LinuxDeviceService) GetBlockDevices() ([]string, error) { - r := du.RunnerFactory.Select(utils.Lsblk) + r := du.runnerFactory.Select(utils.Lsblk) output, err := r.Command("--nodeps", "-o", "NAME", "-J") if err != nil { return nil, err @@ -1078,7 +1078,7 @@ } func (du *LinuxDeviceService) GetBlockDevice(name string) (*model.BlockDevice, error) { - r := du.RunnerFactory.Select(utils.Lsblk) + r := du.runnerFactory.Select(utils.Lsblk) output, err := r.Command("--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", name) if err != nil { return nil, err @@ -1105,22 +1105,16 @@ } func (du *LinuxDeviceService) Mount(source string, target string, fs model.FileSystem, options model.MountOptions) error { - r := du.RunnerFactory.Select(utils.Mount) + r := du.runnerFactory.Select(utils.Mount) _, err := r.Command(source, "-t", string(fs), "-o", string(options), target) - if err != nil { - return err - } - return nil -} + return err +} func (du *LinuxDeviceService) Umount(source string, target string) error { - r := du.RunnerFactory.Select(utils.Umount) + r := du.runnerFactory.Select(utils.Umount) _, err := r.Command(target) - if err != nil { - return err - } - return nil -} + return err +} @@ -1605,20 +1587,16 @@ Vs nvmeIdentifyControllerAmznVS } -type NVMeDevice struct { +type NVMeIoctlResult struct { Name string IdCtrl nvmeIdentifyController } -func NewNVMeDevice(name string) (*NVMeDevice, error) { - d := &NVMeDevice{Name: name} - if err := d.NVMeIOctl(); err != nil { - return nil, err - } - return d, nil -} +func NewNVMeIoctlResult(name string) *NVMeIoctlResult { + return &NVMeIoctlResult{Name: name} +} -func (d *NVMeDevice) NVMeIOctl() error { +func (d *NVMeIoctlResult) Syscall() error { idResponse := uintptr(unsafe.Pointer(&d.IdCtrl)) idLen := unsafe.Sizeof(d.IdCtrl) @@ -1643,75 +1621,72 @@ return nil } -// NVMe Service [Start] type NVMeService interface { GetBlockDeviceMapping(device string) (string, error) } -// NVMe Service [END] - type AwsNitroNVMeService struct{} -func NewAwsNitroNVMeService() *AwsNitroNVMeService { +func NewAwsNitroNVMeService() *AwsNitroNVMeService { return &AwsNitroNVMeService{} } func (ns *AwsNitroNVMeService) GetBlockDeviceMapping(device string) (string, error) { - nd, err := NewNVMeDevice(device) - if err != nil { + nir := NewNVMeIoctlResult(device) + if err := nir.Syscall(); err != nil { return "", err } - return ns.getBlockDeviceMapping(nd) + return ns.getBlockDeviceMapping(nir) } -func (ns *AwsNitroNVMeService) isEBSVolume(nd *NVMeDevice) bool { - vid := nd.IdCtrl.Vid - mn := strings.TrimRightFunc(string(nd.IdCtrl.Mn[:]), ns.TrimModelNumber) +func (ns *AwsNitroNVMeService) isEBSVolume(nir *NVMeIoctlResult) bool { + vid := nir.IdCtrl.Vid + mn := strings.TrimRightFunc(string(nir.IdCtrl.Mn[:]), ns.trimModelNumber) return vid == AMZN_NVME_VID && mn == AMZN_NVME_EBS_MN } -func (ns *AwsNitroNVMeService) isInstanceStoreVolume(nd *NVMeDevice) bool { - vid := nd.IdCtrl.Vid - mn := strings.TrimRightFunc(string(nd.IdCtrl.Mn[:]), ns.TrimModelNumber) +func (ns *AwsNitroNVMeService) isInstanceStoreVolume(nir *NVMeIoctlResult) bool { + vid := nir.IdCtrl.Vid + mn := strings.TrimRightFunc(string(nir.IdCtrl.Mn[:]), ns.trimModelNumber) return vid == AMZN_NVME_VID && mn == AMZN_NVME_INS_MN } -func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nd *NVMeDevice) (string, error) { +func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nir *NVMeIoctlResult) (string, error) { var bdm string - if ns.isEBSVolume(nd) { - bdm = strings.TrimRightFunc(string(nd.IdCtrl.Vs.Bdev[:]), ns.TrimVendorSpecfic) + if ns.isEBSVolume(nir) { + bdm = strings.TrimRightFunc(string(nir.IdCtrl.Vs.Bdev[:]), ns.trimBlockDevice) } - if ns.isInstanceStoreVolume(nd) { + if ns.isInstanceStoreVolume(nir) { // Vendor Specfic (vs) - vs := strings.TrimRightFunc(string(nd.IdCtrl.Vs.Bdev[:]), ns.TrimVendorSpecfic) + vs := strings.TrimRightFunc(string(nir.IdCtrl.Vs.Bdev[:]), ns.trimBlockDevice) // Regex Block device Mapping - rebdm := regexp.MustCompile(`ephemeral[0-9]:(sd[a-z]|none)`) + rebdm := regexp.MustCompile(`^ephemeral[0-9]:(sd[a-z]|none)`) // Match Block Device Mapping mbdm := rebdm.FindStringSubmatch(vs) - if len(mbdm) != 2 { - return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern . Pattern=%s, Actual=%s", nd.Name, rebdm.String(), vs) + if len(mbdm) != 2 { + return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern . Pattern=%s, Actual=%s", nir.Name, rebdm.String(), vs) } - if mbdm[1] == "none" { - return "", fmt.Errorf("🔴 %s: Must provide a device name to the Instance Store NVMe block device mapping", nd.Name) + if mbdm[1] == "none" { + return "", fmt.Errorf("🔴 %s: Must provide a device name to the Instance Store NVMe block device mapping", nir.Name) } - bdm = mbdm[1] + bdm = mbdm[1] } - if len(bdm) == 0 { - return "", fmt.Errorf("🔴 %s is not an AWS-managed NVME device", nd.Name) + if len(bdm) == 0 { + return "", fmt.Errorf("🔴 %s is not an AWS-managed NVME device", nir.Name) } - if !strings.HasPrefix(bdm, "/dev/") { + if !strings.HasPrefix(bdm, "/dev/") { bdm = "/dev/" + bdm } - return bdm, nil + return bdm, nil } -func (ns *AwsNitroNVMeService) TrimModelNumber(r rune) bool { +func (ns *AwsNitroNVMeService) trimModelNumber(r rune) bool { // Explanation: // - Both the AWS EC2 and EBS team use the 0x20 (space) byte to pad out the Model Number return r == 0x20 } -func (ns *AwsNitroNVMeService) TrimVendorSpecfic(r rune) bool { +func (ns *AwsNitroNVMeService) trimBlockDevice(r rune) bool { // Explanation: // - The AWS EC2 team uses the 0x00 (null) byte to pad out the Vendor Specific allocation // - The AWS EBS team uses the 0x20 (space) byte to pad out the Vendor Specific allocation @@ -1741,76 +1716,54 @@ ) type OwnerService interface { - GetCurrentUser() (*model.User, error) GetUser(usr string) (*model.User, error) GetGroup(grp string) (*model.Group, error) } type UnixOwnerService struct{} -func NewUnixOwnerService() *UnixOwnerService { +func NewUnixOwnerService() *UnixOwnerService { return &UnixOwnerService{} } -func (uos *UnixOwnerService) GetCurrentUser() (*model.User, error) { - u, err := user.Current() - if err != nil { - return nil, fmt.Errorf("🔴 Failed to get current user") - } - uid, err := strconv.ParseUint(u.Uid, 10, 32) - if err != nil { - return nil, fmt.Errorf("🔴 Failed to cast user (id) to unsigned 32-bit integer") - } - return &model.User{ - Name: u.Name, - Id: model.UserId(uid), - }, nil -} - -func (uos *UnixOwnerService) GetUser(usr string) (*model.User, error) { +func (uos *UnixOwnerService) GetUser(usr string) (*model.User, error) { var u *user.User - if _, err := strconv.Atoi(usr); err != nil { + if _, err := strconv.Atoi(usr); err != nil { // If not a valid integer, try to look up by username u, err = user.Lookup(usr) - if err != nil { + if err != nil { return nil, fmt.Errorf("🔴 User (name=%s) does not exist", usr) } - } else { + } else { u, err = user.LookupId(usr) - if err != nil { + if err != nil { return nil, fmt.Errorf("🔴 User (id=%s) does not exist", usr) } } - - uid, err := strconv.ParseUint(u.Uid, 10, 32) - if err != nil { - return nil, fmt.Errorf("🔴 Failed to cast user (id=%s) to unsigned 32-bit integer", u.Uid) - } - - return &model.User{Name: u.Username, Id: model.UserId(uid)}, nil + // We realistically can expect that u.Uid will always be a able + // to be cast into an unsigned 32-bit integer. Error handling would be dead code + uid, _ := strconv.ParseUint(u.Uid, 10, 32) + return &model.User{Name: u.Username, Id: model.UserId(uid)}, nil } -func (uos *UnixOwnerService) GetGroup(grp string) (*model.Group, error) { +func (uos *UnixOwnerService) GetGroup(grp string) (*model.Group, error) { var g *user.Group - if _, err := strconv.Atoi(grp); err != nil { + if _, err := strconv.Atoi(grp); err != nil { // If not a valid integer, try to look up by group name g, err = user.LookupGroup(grp) - if err != nil { + if err != nil { return nil, fmt.Errorf("🔴 Group (name=%s) does not exist", grp) } - } else { + } else { g, err = user.LookupGroupId(grp) - if err != nil { + if err != nil { return nil, fmt.Errorf("🔴 Group (id=%s) does not exist", grp) } } - - gid, err := strconv.ParseUint(g.Gid, 10, 32) - if err != nil { - return nil, fmt.Errorf("🔴 Failed to cast group (id=%s) to integer", g.Gid) - } - - return &model.Group{Name: g.Name, Id: model.GroupId(gid)}, nil + // We realistically can expect that g.Gid will always be a able + // to be cast into an unsigned 32-bit integer. Error handling would be dead code + gid, _ := strconv.ParseUint(g.Gid, 10, 32) + return &model.Group{Name: g.Name, Id: model.GroupId(gid)}, nil } diff --git a/internal/service/nvme.go b/internal/service/nvme.go index c92e248..e0ba04c 100644 --- a/internal/service/nvme.go +++ b/internal/service/nvme.go @@ -90,20 +90,16 @@ type nvmeIdentifyController struct { Vs nvmeIdentifyControllerAmznVS } -type NVMeDevice struct { +type NVMeIoctlResult struct { Name string IdCtrl nvmeIdentifyController } -func NewNVMeDevice(name string) (*NVMeDevice, error) { - d := &NVMeDevice{Name: name} - if err := d.NVMeIOctl(); err != nil { - return nil, err - } - return d, nil +func NewNVMeIoctlResult(name string) *NVMeIoctlResult { + return &NVMeIoctlResult{Name: name} } -func (d *NVMeDevice) NVMeIOctl() error { +func (d *NVMeIoctlResult) Syscall() error { idResponse := uintptr(unsafe.Pointer(&d.IdCtrl)) idLen := unsafe.Sizeof(d.IdCtrl) @@ -128,13 +124,10 @@ func (d *NVMeDevice) NVMeIOctl() error { return nil } -// NVMe Service [Start] type NVMeService interface { GetBlockDeviceMapping(device string) (string, error) } -// NVMe Service [END] - type AwsNitroNVMeService struct{} func NewAwsNitroNVMeService() *AwsNitroNVMeService { @@ -142,47 +135,47 @@ func NewAwsNitroNVMeService() *AwsNitroNVMeService { } func (ns *AwsNitroNVMeService) GetBlockDeviceMapping(device string) (string, error) { - nd, err := NewNVMeDevice(device) - if err != nil { + nir := NewNVMeIoctlResult(device) + if err := nir.Syscall(); err != nil { return "", err } - return ns.getBlockDeviceMapping(nd) + return ns.getBlockDeviceMapping(nir) } -func (ns *AwsNitroNVMeService) isEBSVolume(nd *NVMeDevice) bool { - vid := nd.IdCtrl.Vid - mn := strings.TrimRightFunc(string(nd.IdCtrl.Mn[:]), ns.TrimModelNumber) +func (ns *AwsNitroNVMeService) isEBSVolume(nir *NVMeIoctlResult) bool { + vid := nir.IdCtrl.Vid + mn := strings.TrimRightFunc(string(nir.IdCtrl.Mn[:]), ns.trimModelNumber) return vid == AMZN_NVME_VID && mn == AMZN_NVME_EBS_MN } -func (ns *AwsNitroNVMeService) isInstanceStoreVolume(nd *NVMeDevice) bool { - vid := nd.IdCtrl.Vid - mn := strings.TrimRightFunc(string(nd.IdCtrl.Mn[:]), ns.TrimModelNumber) +func (ns *AwsNitroNVMeService) isInstanceStoreVolume(nir *NVMeIoctlResult) bool { + vid := nir.IdCtrl.Vid + mn := strings.TrimRightFunc(string(nir.IdCtrl.Mn[:]), ns.trimModelNumber) return vid == AMZN_NVME_VID && mn == AMZN_NVME_INS_MN } -func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nd *NVMeDevice) (string, error) { +func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nir *NVMeIoctlResult) (string, error) { var bdm string - if ns.isEBSVolume(nd) { - bdm = strings.TrimRightFunc(string(nd.IdCtrl.Vs.Bdev[:]), ns.TrimVendorSpecfic) + if ns.isEBSVolume(nir) { + bdm = strings.TrimRightFunc(string(nir.IdCtrl.Vs.Bdev[:]), ns.trimBlockDevice) } - if ns.isInstanceStoreVolume(nd) { + if ns.isInstanceStoreVolume(nir) { // Vendor Specfic (vs) - vs := strings.TrimRightFunc(string(nd.IdCtrl.Vs.Bdev[:]), ns.TrimVendorSpecfic) + vs := strings.TrimRightFunc(string(nir.IdCtrl.Vs.Bdev[:]), ns.trimBlockDevice) // Regex Block device Mapping - rebdm := regexp.MustCompile(`ephemeral[0-9]:(sd[a-z]|none)`) + rebdm := regexp.MustCompile(`^ephemeral[0-9]:(sd[a-z]|none)`) // Match Block Device Mapping mbdm := rebdm.FindStringSubmatch(vs) if len(mbdm) != 2 { - return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern . Pattern=%s, Actual=%s", nd.Name, rebdm.String(), vs) + return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern . Pattern=%s, Actual=%s", nir.Name, rebdm.String(), vs) } if mbdm[1] == "none" { - return "", fmt.Errorf("🔴 %s: Must provide a device name to the Instance Store NVMe block device mapping", nd.Name) + return "", fmt.Errorf("🔴 %s: Must provide a device name to the Instance Store NVMe block device mapping", nir.Name) } bdm = mbdm[1] } if len(bdm) == 0 { - return "", fmt.Errorf("🔴 %s is not an AWS-managed NVME device", nd.Name) + return "", fmt.Errorf("🔴 %s is not an AWS-managed NVME device", nir.Name) } if !strings.HasPrefix(bdm, "/dev/") { bdm = "/dev/" + bdm @@ -190,13 +183,13 @@ func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nd *NVMeDevice) (string, er return bdm, nil } -func (ns *AwsNitroNVMeService) TrimModelNumber(r rune) bool { +func (ns *AwsNitroNVMeService) trimModelNumber(r rune) bool { // Explanation: // - Both the AWS EC2 and EBS team use the 0x20 (space) byte to pad out the Model Number return r == 0x20 } -func (ns *AwsNitroNVMeService) TrimVendorSpecfic(r rune) bool { +func (ns *AwsNitroNVMeService) trimBlockDevice(r rune) bool { // Explanation: // - The AWS EC2 team uses the 0x00 (null) byte to pad out the Vendor Specific allocation // - The AWS EBS team uses the 0x20 (space) byte to pad out the Vendor Specific allocation diff --git a/internal/service/nvme_test.go b/internal/service/nvme_test.go new file mode 100644 index 0000000..acd6aca --- /dev/null +++ b/internal/service/nvme_test.go @@ -0,0 +1,152 @@ +package service + +import ( + "fmt" + "testing" + + "github.com/reecetech/ebs-bootstrap/internal/utils" +) + +const ( + UNSUPPORTED_NVME_VID = 0xFFFF + UNSUPPORTED_NVME_MN = "External NVME Manufacturer" +) + +const ( + Space = 0x20 + Null = 0x00 +) + +func TestGetBlockDeviceMapping(t *testing.T) { + subtests := []struct { + Name string + Device string + VendorId uint16 + ModelNumber string + BlockDevice string + BlockDevicePadding byte + ExpectedOutput string + ExpectedErr error + }{ + { + Name: "EBS NVMe Device + Pre-launch", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_EBS_MN, + BlockDevice: "sdb", + BlockDevicePadding: Space, + ExpectedOutput: "/dev/sdb", + ExpectedErr: nil, + }, + { + Name: "EBS NVMe Device + Post-launch", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_EBS_MN, + BlockDevice: "/dev/sdb", + BlockDevicePadding: Space, + ExpectedOutput: "/dev/sdb", + ExpectedErr: nil, + }, + { + Name: "Instance Store NVMe Device", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_INS_MN, + BlockDevice: "ephemeral0:sdb", + BlockDevicePadding: 0x53, + ExpectedOutput: "/dev/sdb", + ExpectedErr: nil, + }, + { + Name: "Instance Store NVMe Device + Null Byte", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_INS_MN, + BlockDevice: "ephemeral0:sdb\x00a", + BlockDevicePadding: Null, + ExpectedOutput: "/dev/sdb", + ExpectedErr: nil, + }, + { + Name: "Instance Store NVMe Device + Missing Block Device Mapping", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_INS_MN, + BlockDevice: "ephemeral0:none", + BlockDevicePadding: Null, + ExpectedOutput: "", + ExpectedErr: fmt.Errorf("🔴 /dev/nvme1n1: Must provide a device name to the Instance Store NVMe block device mapping"), + }, + { + Name: "Instance Store NVMe Device + Pattern Mismatch", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_INS_MN, + BlockDevice: "ephemeral0:vdb", + BlockDevicePadding: Null, + ExpectedOutput: "", + ExpectedErr: fmt.Errorf("🔴 /dev/nvme1n1: Instance-store vendor specific metadata did not match pattern . Pattern=^ephemeral[0-9]:(sd[a-z]|none), Actual=ephemeral0:vdb"), + }, + { + Name: "Invalid NVMe Device (Unsupported Vendor ID)", + Device: "/dev/nvme1n1", + VendorId: UNSUPPORTED_NVME_VID, + ModelNumber: AMZN_NVME_EBS_MN, + BlockDevice: "", + BlockDevicePadding: Null, + ExpectedOutput: "", + ExpectedErr: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"), + }, + { + Name: "Invalid NVMe Device (Unsupported Model Number)", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: UNSUPPORTED_NVME_MN, + BlockDevice: "", + BlockDevicePadding: Null, + ExpectedOutput: "", + ExpectedErr: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"), + }, + } + for _, subtest := range subtests { + t.Run(subtest.Name, func(t *testing.T) { + nd := &NVMeIoctlResult{ + Name: subtest.Device, + IdCtrl: nvmeIdentifyController{ + Vid: subtest.VendorId, + Mn: modelNumber(subtest.ModelNumber, Space), + Vs: nvmeIdentifyControllerAmznVS{ + Bdev: blockDevice(subtest.BlockDevice, subtest.BlockDevicePadding), + }, + }, + } + ns := NewAwsNitroNVMeService() + bdm, err := ns.getBlockDeviceMapping(nd) + utils.CheckError("getBlockDeviceMapping()", t, subtest.ExpectedErr, err) + utils.CheckOutput("getBlockDeviceMapping()", t, subtest.ExpectedOutput, bdm) + }) + } +} + +func modelNumber(input string, padding byte) [40]byte { + var mn [40]byte + copy(mn[:], input) + if len(input) < 40 { + for i := len(input); i < 40; i++ { + mn[i] = padding + } + } + return mn +} + +func blockDevice(input string, padding byte) [32]byte { + var bd [32]byte + copy(bd[:], input) + if len(input) < 32 { + for i := len(input); i < 32; i++ { + bd[i] = padding + } + } + return bd +} diff --git a/internal/service/testdata/empty.txt b/internal/service/testdata/empty.txt deleted file mode 100644 index e69de29..0000000