diff --git a/internal/config/validator.go b/internal/config/validator.go index 42e0f0f..d7a115c 100644 --- a/internal/config/validator.go +++ b/internal/config/validator.go @@ -9,6 +9,10 @@ import ( "github.com/reecetech/ebs-bootstrap/internal/service" ) +const ( + LvmWikiDocumentationUrl = "https://github.com/reecetech/ebs-bootstrap/wiki/LVM" +) + type Validator interface { Validate(c *Config) error } @@ -48,6 +52,9 @@ func (fsv *FileSystemValidator) Validate(c *Config) error { if fs == model.Unformatted { return fmt.Errorf("🔴 %s: Must provide a supported file system", name) } + if fs == model.Lvm { + return fmt.Errorf("🔴 %s: Refer to %s on how to manage LVM file systems", name, LvmWikiDocumentationUrl) + } } return nil } diff --git a/internal/config/validator_test.go b/internal/config/validator_test.go index 20493f7..c108975 100644 --- a/internal/config/validator_test.go +++ b/internal/config/validator_test.go @@ -207,6 +207,17 @@ func TestFileSystemValidator(t *testing.T) { }, ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Must provide a supported file system"), }, + { + Name: "LVM File System", + Config: &Config{ + Devices: map[string]Device{ + "/dev/xvdf": { + Fs: model.Lvm, + }, + }, + }, + ExpectedError: fmt.Errorf("🔴 /dev/xvdf: Refer to %s on how to manage LVM file systems", LvmWikiDocumentationUrl), + }, } for _, subtest := range subtests { fsv := NewFileSystemValidator() diff --git a/internal/service/device.go b/internal/service/device.go index 962abbf..b239441 100644 --- a/internal/service/device.go +++ b/internal/service/device.go @@ -1,14 +1,18 @@ package service import ( - "encoding/json" "fmt" + "regexp" "strconv" + "strings" "github.com/reecetech/ebs-bootstrap/internal/model" "github.com/reecetech/ebs-bootstrap/internal/utils" ) +var deviceNameRegex = regexp.MustCompile(`^NAME="(.*)"`) +var devicePropertiesRegex = regexp.MustCompile(`^LABEL="(.*)" FSTYPE="(.*)" MOUNTPOINT="(.*)"`) + type DeviceService interface { GetSize(name string) (uint64, error) // bytes GetBlockDevices() ([]string, error) @@ -21,15 +25,6 @@ type LinuxDeviceService struct { runnerFactory utils.RunnerFactory } -type LsblkBlockDeviceResponse struct { - BlockDevices []struct { - Name *string `json:"name"` - Label *string `json:"label"` - FsType *string `json:"fstype"` - MountPoint *string `json:"mountpoint"` - } `json:"blockdevices"` -} - func NewLinuxDeviceService(rc utils.RunnerFactory) *LinuxDeviceService { return &LinuxDeviceService{ runnerFactory: rc, @@ -51,46 +46,45 @@ func (du *LinuxDeviceService) GetSize(name string) (uint64, error) { func (du *LinuxDeviceService) GetBlockDevices() ([]string, error) { r := du.runnerFactory.Select(utils.Lsblk) - output, err := r.Command("--nodeps", "-o", "NAME", "-J") + output, err := r.Command("--nodeps", "-o", "NAME", "-P") if err != nil { return nil, err } - lbd := &LsblkBlockDeviceResponse{} - err = json.Unmarshal([]byte(output), lbd) - if err != nil { - return nil, fmt.Errorf("🔴 Failed to decode lsblk response: %v", err) - } - d := make([]string, len(lbd.BlockDevices)) - for i := range d { - d[i] = "/dev/" + utils.Safe(lbd.BlockDevices[i].Name) + + lines := strings.Split(strings.TrimSuffix(output, "\n"), "\n") + d := make([]string, len(lines)) + + for i, line := range lines { + matches := deviceNameRegex.FindStringSubmatch(line) + if len(matches) != 2 { + return nil, fmt.Errorf("🔴 Failed to decode lsblk response") + } + d[i] = "/dev/" + matches[1] } return d, nil } func (du *LinuxDeviceService) GetBlockDevice(name string) (*model.BlockDevice, error) { r := du.runnerFactory.Select(utils.Lsblk) - output, err := r.Command("--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", name) + output, err := r.Command("--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", name) if err != nil { return nil, err } - lbd := &LsblkBlockDeviceResponse{} - err = json.Unmarshal([]byte(output), lbd) - if err != nil { - return nil, fmt.Errorf("🔴 Failed to decode lsblk response: %v", err) - } - if len(lbd.BlockDevices) != 1 { - return nil, fmt.Errorf("🔴 %s: An unexpected number of block devices were returned: Expected=1 Actual=%d", name, len(lbd.BlockDevices)) + + matches := devicePropertiesRegex.FindStringSubmatch(output) + if len(matches) != 4 { + return nil, fmt.Errorf("🔴 Failed to decode lsblk response") } - fst := utils.Safe(lbd.BlockDevices[0].FsType) - fs, err := model.ParseFileSystem(fst) + + fs, err := model.ParseFileSystem(matches[2]) if err != nil { return nil, fmt.Errorf("🔴 %s: %s", name, err) } return &model.BlockDevice{ Name: name, - Label: utils.Safe(lbd.BlockDevices[0].Label), + Label: matches[1], FileSystem: fs, - MountPoint: utils.Safe(lbd.BlockDevices[0].MountPoint), + MountPoint: matches[3], }, nil } diff --git a/internal/service/device_test.go b/internal/service/device_test.go index a6906c5..47c4100 100644 --- a/internal/service/device_test.go +++ b/internal/service/device_test.go @@ -72,34 +72,32 @@ func TestGetBlockDevices(t *testing.T) { ExpectedError error }{ { - Name: "lsblk=success + json=valid", + Name: "lsblk=success", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "NAME", "-J"}, - RunnerOutput: `{"blockdevices": [ - {"name":"nvme1n1"}, - {"name":"nvme0n1"} - ]}`, + RunnerArgs: []string{"--nodeps", "-o", "NAME", "-P"}, + RunnerOutput: `NAME="nvme1n1" +NAME="nvme0n1"`, RunnerError: nil, ExpectedOutput: []string{"/dev/nvme1n1", "/dev/nvme0n1"}, ExpectedError: nil, }, { - Name: "lsblk=success + json=invalid", + Name: "lsblk=success + malformed response", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "NAME", "-J"}, - RunnerOutput: `{"invalid_json"}`, + RunnerArgs: []string{"--nodeps", "-o", "NAME", "-P"}, + RunnerOutput: `malformed`, RunnerError: nil, ExpectedOutput: nil, - ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response: *"), + ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response"), }, { Name: "lsblk=failure", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "NAME", "-J"}, + RunnerArgs: []string{"--nodeps", "-o", "NAME", "-P"}, RunnerOutput: "", - RunnerError: fmt.Errorf("🔴 lsblk: invalid option -- 'J'"), + RunnerError: fmt.Errorf("🔴 lsblk: invalid option -- 'P'"), ExpectedOutput: nil, - ExpectedError: fmt.Errorf("🔴 lsblk: invalid option -- 'J'"), + ExpectedError: fmt.Errorf("🔴 lsblk: invalid option -- 'P'"), }, } for _, subtest := range subtests { @@ -128,11 +126,9 @@ func TestGetBlockDevice(t *testing.T) { Name: "lsblk=success", Device: "/dev/nvme1n1", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/nvme1n1"}, - RunnerOutput: `{"blockdevices": [ - {"name":"nvme1n1", "label":"external-vol", "fstype":"xfs", "mountpoint":"/mnt/app"} - ]}`, - RunnerError: nil, + RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/nvme1n1"}, + RunnerOutput: `LABEL="external-vol" FSTYPE="xfs" MOUNTPOINT="/mnt/app"`, + RunnerError: nil, ExpectedOutput: &model.BlockDevice{ Name: "/dev/nvme1n1", Label: "external-vol", @@ -145,11 +141,9 @@ func TestGetBlockDevice(t *testing.T) { Name: "lsblk=success + device=unformatted", Device: "/dev/nvme1n1", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/nvme1n1"}, - RunnerOutput: `{"blockdevices": [ - {"name":"nvme1n1", "label":null, "fstype":null, "mountpoint":null} - ]}`, - RunnerError: nil, + RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/nvme1n1"}, + RunnerOutput: `LABEL="" FSTYPE="" MOUNTPOINT=""`, + RunnerError: nil, ExpectedOutput: &model.BlockDevice{ Name: "/dev/nvme1n1", Label: "", @@ -158,75 +152,31 @@ func TestGetBlockDevice(t *testing.T) { }, ExpectedError: nil, }, - /* Reference [https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/device_naming.html] - --- - For example, if /dev/sdb is renamed /dev/xvdf, then /dev/sdc is renamed /dev/xvdg. - Amazon Linux creates a symbolic link for the name you specified to the renamed device. - Other operating systems could behave differently. - -- - With this behaviour established on Amazon Linux, it is possible for lsblk to return a - device name that might not reflect the one that was provided to lsblk. To simplify the - output of ebs-bootstrap, lets force the BlockDevice to house the name that was provided - to the lsblk call - */ { - Name: "lsblk=success + symlink=true", - Device: "/dev/sdb", - RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"}, - RunnerOutput: `{"blockdevices": [ - {"name":"xvdf", "label":"external-vol", "fstype":"xfs", "mountpoint":"/mnt/app"} - ]}`, - RunnerError: nil, - ExpectedOutput: &model.BlockDevice{ - Name: "/dev/sdb", - Label: "external-vol", - FileSystem: model.Xfs, - MountPoint: "/mnt/app", - }, - ExpectedError: nil, - }, - { - Name: "lsblk=success + json=invalid", + Name: "lsblk=success + malformed response", Device: "/dev/sdb", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"}, - RunnerOutput: `{"invalid_json"}`, - RunnerError: nil, - ExpectedOutput: nil, - ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response: *"), - }, - { - Name: "lsblk=success + filesystem=unsupported", - Device: "/dev/sdb", - RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"}, - RunnerOutput: `{"blockdevices": [ - {"name":"sdb", "label":null, "fstype":"jfs", "mountpoint":"/mnt/app"} - ]}`, + RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/sdb"}, + RunnerOutput: "malformed", RunnerError: nil, ExpectedOutput: nil, - ExpectedError: fmt.Errorf("🔴 /dev/sdb: File system 'jfs' is not supported"), + ExpectedError: fmt.Errorf("🔴 Failed to decode lsblk response"), }, - /* I haven't encountered a scenario where lsblk successfully returns an empty array - Typically, if it cannot find a specific block device, it would produce an error. - Nevertheless, lets test this scenario... - */ { - Name: "lsblk=success + len(devices)==0", + Name: "lsblk=success + filesystem=unsupported", Device: "/dev/sdb", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdb"}, - RunnerOutput: `{"blockdevices": []}`, + RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/sdb"}, + RunnerOutput: `LABEL="" FSTYPE="jfs" MOUNTPOINT="/mnt/app"`, RunnerError: nil, ExpectedOutput: nil, - ExpectedError: fmt.Errorf("🔴 /dev/sdb: An unexpected number of block devices were returned: Expected=1 Actual=0"), + ExpectedError: fmt.Errorf("🔴 /dev/sdb: File system 'jfs' is not supported"), }, { Name: "lsblk=failure", Device: "/dev/sdc", RunnerBinary: utils.Lsblk, - RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-J", "/dev/sdc"}, + RunnerArgs: []string{"--nodeps", "-o", "LABEL,FSTYPE,MOUNTPOINT", "-P", "/dev/sdc"}, RunnerOutput: "", RunnerError: fmt.Errorf("🔴 lsblk: /dev/sdc: not a block device"), ExpectedOutput: nil, diff --git a/internal/service/filesystem.go b/internal/service/filesystem.go index d68e6e0..77ceb1a 100644 --- a/internal/service/filesystem.go +++ b/internal/service/filesystem.go @@ -42,7 +42,7 @@ func (fsf *LinuxFileSystemServiceFactory) Select(fs model.FileSystem) (FileSyste return NewXfsService(fsf.RunnerFactory), nil case model.Lvm: //lint:ignore ST1005 This Error Message Is Supposed to Be Prepended With A Device Name - return nil, fmt.Errorf("Refer to https://github.com/reecetech/ebs-bootstrap/wiki/LVM on how to manage LVM file systems") + return nil, fmt.Errorf("A Physical Volume cannot be queried/modified") case model.Unformatted: //lint:ignore ST1005 This Error Message Is Supposed to Be Prepended With A Device Name return nil, fmt.Errorf("An unformatted file system can not be queried/modified") diff --git a/internal/service/filesystem_test.go b/internal/service/filesystem_test.go index b1d0784..9dbc243 100644 --- a/internal/service/filesystem_test.go +++ b/internal/service/filesystem_test.go @@ -48,6 +48,13 @@ func TestFileSystemFactory(t *testing.T) { ExpectedOutput: nil, ExpectedError: fmt.Errorf("An unformatted file system can not be queried/modified"), }, + { + Name: "LVM", + FileSystem: model.Lvm, + CmpOption: cmp.AllowUnexported(), + ExpectedOutput: nil, + ExpectedError: fmt.Errorf("A Physical Volume cannot be queried/modified"), + }, } for _, subtest := range subtests { t.Run(subtest.Name, func(t *testing.T) { diff --git a/internal/service/nvme.go b/internal/service/nvme.go index a4be66b..e50f3fd 100644 --- a/internal/service/nvme.go +++ b/internal/service/nvme.go @@ -90,6 +90,8 @@ type nvmeIdentifyController struct { Vs nvmeIdentifyControllerAmznVS } +var instanceStoreRegex = regexp.MustCompile(`^(ephemeral[0-9]):(sd[a-z]|none)`) + type NVMeIoctlResult struct { Name string IdCtrl nvmeIdentifyController @@ -162,12 +164,11 @@ func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nir *NVMeIoctlResult) (stri if ns.isInstanceStoreVolume(nir) { // Vendor Specfic (vs) vs := strings.TrimRightFunc(string(nir.IdCtrl.Vs.Bdev[:]), ns.trimBlockDevice) - // Regex Block device Mapping - rebdm := regexp.MustCompile(`^(ephemeral[0-9]):(sd[a-z]|none)`) + // Match Block Device Mapping - mbdm := rebdm.FindStringSubmatch(vs) + mbdm := instanceStoreRegex.FindStringSubmatch(vs) if len(mbdm) != 3 { - return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern. Pattern=%s, Actual=%s", nir.Name, rebdm.String(), vs) + return "", fmt.Errorf("🔴 %s: Instance-store vendor specific metadata did not match pattern. Pattern=%s, Actual=%s", nir.Name, instanceStoreRegex.String(), vs) } // If the block device mapping is "none", then lets default to assigning the // the block device mapping to the match result from ephemeral[0-9] diff --git a/internal/service/nvme_test.go b/internal/service/nvme_test.go index 48f0e65..5fcc241 100644 --- a/internal/service/nvme_test.go +++ b/internal/service/nvme_test.go @@ -19,85 +19,85 @@ const ( func TestGetBlockDeviceMapping(t *testing.T) { subtests := []struct { - Name string - Device string - VendorId uint16 - ModelNumber string - BlockDevice string - ExpectedOutput string - ExpectedError error + Name string + Device string + VendorId uint16 + ModelNumber string + BlockDevice string + ExpectedOutput string + ExpectedError error }{ { - Name: "EBS NVMe Device + Pre-launch", - Device: "/dev/nvme1n1", - VendorId: AMZN_NVME_VID, - ModelNumber: AMZN_NVME_EBS_MN, - BlockDevice: "sdb", - ExpectedOutput: "/dev/sdb", - ExpectedError: nil, + Name: "EBS NVMe Device + Pre-launch", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_EBS_MN, + BlockDevice: "sdb", + ExpectedOutput: "/dev/sdb", + ExpectedError: nil, }, { - Name: "EBS NVMe Device + Post-launch", - Device: "/dev/nvme1n1", - VendorId: AMZN_NVME_VID, - ModelNumber: AMZN_NVME_EBS_MN, - BlockDevice: "/dev/sdb", - ExpectedOutput: "/dev/sdb", - ExpectedError: nil, + Name: "EBS NVMe Device + Post-launch", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_EBS_MN, + BlockDevice: "/dev/sdb", + ExpectedOutput: "/dev/sdb", + ExpectedError: nil, }, { - Name: "Instance Store NVMe Device", - Device: "/dev/nvme1n1", - VendorId: AMZN_NVME_VID, - ModelNumber: AMZN_NVME_INS_MN, - BlockDevice: "ephemeral0:sdb", - ExpectedOutput: "/dev/sdb", - ExpectedError: nil, + Name: "Instance Store NVMe Device", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_INS_MN, + BlockDevice: "ephemeral0:sdb", + ExpectedOutput: "/dev/sdb", + ExpectedError: nil, }, { - Name: "Instance Store NVMe Device + Null Byte", - Device: "/dev/nvme1n1", - VendorId: AMZN_NVME_VID, - ModelNumber: AMZN_NVME_INS_MN, - BlockDevice: "ephemeral0:sdb\x00a", - ExpectedOutput: "/dev/sdb", - ExpectedError: nil, + Name: "Instance Store NVMe Device + Null Byte", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_INS_MN, + BlockDevice: "ephemeral0:sdb\x00a", + ExpectedOutput: "/dev/sdb", + ExpectedError: 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", - ExpectedOutput: "/dev/ephemeral0", - ExpectedError: 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", + ExpectedOutput: "/dev/ephemeral0", + ExpectedError: nil, }, { - Name: "Instance Store NVMe Device + Pattern Mismatch", - Device: "/dev/nvme1n1", - VendorId: AMZN_NVME_VID, - ModelNumber: AMZN_NVME_INS_MN, - BlockDevice: "ephemeral0:vdb", - ExpectedOutput: "", - ExpectedError: 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: "Instance Store NVMe Device + Pattern Mismatch", + Device: "/dev/nvme1n1", + VendorId: AMZN_NVME_VID, + ModelNumber: AMZN_NVME_INS_MN, + BlockDevice: "ephemeral0:vdb", + ExpectedOutput: "", + ExpectedError: 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: "", - ExpectedOutput: "", - ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"), + Name: "Invalid NVMe Device (Unsupported Vendor ID)", + Device: "/dev/nvme1n1", + VendorId: UNSUPPORTED_NVME_VID, + ModelNumber: AMZN_NVME_EBS_MN, + BlockDevice: "", + ExpectedOutput: "", + ExpectedError: 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: "", - ExpectedOutput: "", - ExpectedError: 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: "", + ExpectedOutput: "", + ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"), }, } for _, subtest := range subtests {