Skip to content

Commit

Permalink
(feat): Add protection clause for non-assigned block device mapping
Browse files Browse the repository at this point in the history
  • Loading branch information
lasith-kg committed Feb 12, 2024
1 parent 802da22 commit 5a8b722
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 23 deletions.
2 changes: 1 addition & 1 deletion internal/config/modifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ func (andm *AwsNitroNVMeModifier) Modify(c *Config) error {
if err != nil {
return err
}
log.Printf("🔵 Nitro NVMe detected: %s -> %s", name, bdm)
cd, exists := c.Devices[bdm]
// We can detect AWS NVMe Devices, but this doesn't neccesarily
// mean they will be managed through configuration
if !exists {
continue
}
log.Printf("🔵 %s: Detected Nitro-based AWS NVMe device => %s", name, bdm)
// Delete the original reference to the device configuration from the
// block device mapping retrieved from the NVMe IoCtl interface and
// replace it with the actual device name
Expand Down
15 changes: 9 additions & 6 deletions internal/service/nvme.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,16 +163,19 @@ func (ns *AwsNitroNVMeService) getBlockDeviceMapping(nir *NVMeIoctlResult) (stri
// 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)`)
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", nir.Name, rebdm.String(), 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)
}
if mbdm[1] == "none" {
return "", fmt.Errorf("🔴 %s: Must provide a device name to the Instance Store NVMe block device mapping", nir.Name)
// 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]
if mbdm[2] == "none" {
bdm = mbdm[1]
} else {
bdm = mbdm[2]
}
bdm = mbdm[1]
}
if len(bdm) == 0 {
return "", fmt.Errorf("🔴 %s is not an AWS-managed NVME device", nir.Name)
Expand Down
40 changes: 24 additions & 16 deletions internal/service/nvme_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const (
)

const (
Space = 0x20
Null = 0x00
SpaceByte = 0x20
NullByte = 0x00
)

func TestGetBlockDeviceMapping(t *testing.T) {
Expand All @@ -24,7 +24,6 @@ func TestGetBlockDeviceMapping(t *testing.T) {
VendorId uint16
ModelNumber string
BlockDevice string
BlockDevicePadding byte
ExpectedOutput string
ExpectedError error
}{
Expand All @@ -34,7 +33,6 @@ func TestGetBlockDeviceMapping(t *testing.T) {
VendorId: AMZN_NVME_VID,
ModelNumber: AMZN_NVME_EBS_MN,
BlockDevice: "sdb",
BlockDevicePadding: Space,
ExpectedOutput: "/dev/sdb",
ExpectedError: nil,
},
Expand All @@ -44,7 +42,6 @@ func TestGetBlockDeviceMapping(t *testing.T) {
VendorId: AMZN_NVME_VID,
ModelNumber: AMZN_NVME_EBS_MN,
BlockDevice: "/dev/sdb",
BlockDevicePadding: Space,
ExpectedOutput: "/dev/sdb",
ExpectedError: nil,
},
Expand All @@ -54,7 +51,6 @@ func TestGetBlockDeviceMapping(t *testing.T) {
VendorId: AMZN_NVME_VID,
ModelNumber: AMZN_NVME_INS_MN,
BlockDevice: "ephemeral0:sdb",
BlockDevicePadding: 0x53,
ExpectedOutput: "/dev/sdb",
ExpectedError: nil,
},
Expand All @@ -64,7 +60,6 @@ func TestGetBlockDeviceMapping(t *testing.T) {
VendorId: AMZN_NVME_VID,
ModelNumber: AMZN_NVME_INS_MN,
BlockDevice: "ephemeral0:sdb\x00a",
BlockDevicePadding: Null,
ExpectedOutput: "/dev/sdb",
ExpectedError: nil,
},
Expand All @@ -74,27 +69,24 @@ func TestGetBlockDeviceMapping(t *testing.T) {
VendorId: AMZN_NVME_VID,
ModelNumber: AMZN_NVME_INS_MN,
BlockDevice: "ephemeral0:none",
BlockDevicePadding: Null,
ExpectedOutput: "",
ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1: Must provide a device name to the Instance Store NVMe block device mapping"),
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",
BlockDevicePadding: Null,
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"),
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: "",
BlockDevicePadding: Null,
ExpectedOutput: "",
ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"),
},
Expand All @@ -104,20 +96,21 @@ func TestGetBlockDeviceMapping(t *testing.T) {
VendorId: AMZN_NVME_VID,
ModelNumber: UNSUPPORTED_NVME_MN,
BlockDevice: "",
BlockDevicePadding: Null,
ExpectedOutput: "",
ExpectedError: fmt.Errorf("🔴 /dev/nvme1n1 is not an AWS-managed NVME device"),
},
}
for _, subtest := range subtests {
t.Run(subtest.Name, func(t *testing.T) {
vsp, err := vendorSpecificPadding(subtest.ModelNumber)
utils.ExpectErr("vendorSpecificPadding()", t, false, err)
nd := &NVMeIoctlResult{
Name: subtest.Device,
IdCtrl: nvmeIdentifyController{
Vid: subtest.VendorId,
Mn: modelNumber(subtest.ModelNumber, Space),
Mn: modelNumber(subtest.ModelNumber, SpaceByte),
Vs: nvmeIdentifyControllerAmznVS{
Bdev: blockDevice(subtest.BlockDevice, subtest.BlockDevicePadding),
Bdev: blockDevice(subtest.BlockDevice, vsp),
},
},
}
Expand Down Expand Up @@ -152,3 +145,18 @@ func blockDevice(input string, padding byte) [32]byte {
}
return bd
}

// The padding byte used for the Vendor Specific section depends on whether the NVMe Block
// device is an EBS (Space) or Instance Store (Null) Volume
func vendorSpecificPadding(modelNumber string) (byte, error) {
switch modelNumber {
case AMZN_NVME_EBS_MN:
return SpaceByte, nil
case AMZN_NVME_INS_MN:
return NullByte, nil
case UNSUPPORTED_NVME_MN:
return NullByte, nil
default:
return NullByte, fmt.Errorf("🔴 %s: Could not determine vendor specific padding", modelNumber)
}
}

0 comments on commit 5a8b722

Please sign in to comment.