diff --git a/builder/hyperv/common/step_set_first_boot_device.go b/builder/hyperv/common/step_set_first_boot_device.go index f17e9a178..397b76669 100644 --- a/builder/hyperv/common/step_set_first_boot_device.go +++ b/builder/hyperv/common/step_set_first_boot_device.go @@ -18,66 +18,86 @@ type StepSetFirstBootDevice struct { func ParseBootDeviceIdentifier(deviceIdentifier string, generation uint) (string, uint, uint, error) { - captureExpression := "^(FLOPPY|IDE|NET)|(CD|DVD)$" - if generation > 1 { - captureExpression = "^((IDE|SCSI):(\\d+):(\\d+))|(DVD|CD)|(NET)$" - } + // all input strings are forced to upperCase for comparison, I believe this is + // safe as all of our values are 7bit ASCII clean. - r, err := regexp.Compile(captureExpression) - if err != nil { - return "", 0, 0, err + lookupDeviceIdentifier := strings.ToUpper(deviceIdentifier) + + if (generation == 1) { + + // Gen1 values are a simple set of if/then/else values, which we coalesce into a map + // here for simplicity + + lookupTable := map[string]string { + "FLOPPY": "FLOPPY", + "IDE": "IDE", + "NET": "NET", + "CD": "CD", + "DVD": "CD", + } + + controllerType, isDefined := lookupTable[lookupDeviceIdentifier] + if (!isDefined) { + + return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device group identifier.", deviceIdentifier) + + } + + // success + return controllerType, 0, 0, nil } - // match against the appropriate set of values.. we force to uppercase to ensure that - // all devices are always in the same case + // everything else is treated as generation 2... the first set of lookups covers + // the simple options.. - identifierMatches := r.FindStringSubmatch(strings.ToUpper(deviceIdentifier)) - if identifierMatches == nil { - return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device or device group identifier.", deviceIdentifier) + lookupTable := map[string]string { + "CD": "CD", + "DVD": "CD", + "NET": "NET", } - switch { + controllerType, isDefined := lookupTable[lookupDeviceIdentifier] + if (isDefined) { - // CD or DVD are always returned as "CD" - case ((generation == 1) && (identifierMatches[2] != "")) || ((generation > 1) && (identifierMatches[5] != "")): - return "CD", 0, 0, nil + // these types do not require controllerNumber or controllerLocation + return controllerType, 0, 0, nil - // generation 1 only has FLOPPY, IDE or NET remaining.. - case (generation == 1): - return identifierMatches[0], 0, 0, nil + } - // generation 2, check for IDE or SCSI and parse location and number - case (identifierMatches[2] != ""): - { + // not a simple option, check for a controllerType:controllerNumber:controllerLocation formatted + // device.. - var controllerLocation int64 - var controllerNumber int64 + r, err := regexp.Compile("^(IDE|SCSI):(\\d+):(\\d+)$") + if err != nil { + return "", 0, 0, err + } - // NOTE: controllerNumber and controllerLocation cannot be negative, the regex expression - // would not have matched if either number was signed + controllerMatch := r.FindStringSubmatch(lookupDeviceIdentifier) + if controllerMatch != nil { - controllerNumber, err = strconv.ParseInt(identifierMatches[3], 10, 8) - if err == nil { + var controllerLocation int64 + var controllerNumber int64 - controllerLocation, err = strconv.ParseInt(identifierMatches[4], 10, 8) - if err == nil { + // NOTE: controllerNumber and controllerLocation cannot be negative, the regex expression + // would not have matched if either number was signed - return identifierMatches[2], uint(controllerNumber), uint(controllerLocation), nil + controllerNumber, err = strconv.ParseInt(controllerMatch[2], 10, 8) + if err == nil { - } + controllerLocation, err = strconv.ParseInt(controllerMatch[3], 10, 8) + if err == nil { - } + return controllerMatch[1], uint(controllerNumber), uint(controllerLocation), nil - return "", 0, 0, err + } } - // only "NET" left on generation 2 - default: - return "NET", 0, 0, nil + return "", 0, 0, err } + return "", 0, 0, fmt.Errorf("The value %q is not a properly formatted device identifier.", deviceIdentifier) } func (s *StepSetFirstBootDevice) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { diff --git a/builder/hyperv/common/step_set_first_boot_device_test.go b/builder/hyperv/common/step_set_first_boot_device_test.go new file mode 100644 index 000000000..fd663aeb6 --- /dev/null +++ b/builder/hyperv/common/step_set_first_boot_device_test.go @@ -0,0 +1,74 @@ +package common + +import ( + "testing" + + "github.com/hashicorp/packer/helper/multistep" +) + +func TestStepSetFirstBootDevice_impl(t *testing.T) { + var _ multistep.Step = new(StepSetFirstBootDevice) +} + +func TestStepSetFirstBootDevice(t *testing.T) { +// t.Fatal("Fail IT!") +} + +type parseBootDeviceIdentifierTest struct { + generation uint + deviceIdentifier string + controllerType string + controllerNumber uint + controllerLocation uint + shouldError bool +} + +func TestStepSetFirstBootDevice_ParseIdentifier(t *testing.T) { + + identifierTests := [...]parseBootDeviceIdentifierTest{ + {1, "IDE", "IDE", 0, 0, false}, + {1, "idE", "IDE", 0, 0, false}, + {1, "CD", "CD", 0, 0, false}, + {1, "cD", "CD", 0, 0, false}, + {1, "DVD", "CD", 0, 0, false}, + {1, "Dvd", "CD", 0, 0, false}, + {1, "FLOPPY", "FLOPPY", 0, 0, false}, + {1, "FloppY", "FLOPPY", 0, 0, false}, + {1, "NET", "NET", 0, 0, false}, + {1, "net", "NET", 0, 0, false}, + {1, "", "", 0, 0, true}, + {1, "bad", "", 0, 0, true}, + {1, "IDE:0:0", "", 0, 0, true}, + {1, "SCSI:0:0", "", 0, 0, true}, + } + + for _, identifierTest := range identifierTests { + + controllerType, controllerNumber, controllerLocation, err := ParseBootDeviceIdentifier( + identifierTest.deviceIdentifier, + identifierTest.generation) + + if (err != nil) != identifierTest.shouldError { + + t.Fatalf("Test %q (gen %v): shouldError: %v but err: %v", identifierTest.deviceIdentifier, + identifierTest.generation, identifierTest.shouldError, err) + + } + + switch { + + case controllerType != identifierTest.controllerType: + t.Fatalf("Test %q (gen %v): controllerType: %q != %q", identifierTest.deviceIdentifier, identifierTest.generation, + identifierTest.controllerType, controllerType) + + case controllerNumber != identifierTest.controllerNumber: + t.Fatalf("Test %q (gen %v): controllerNumber: %v != %v", identifierTest.deviceIdentifier, identifierTest.generation, + identifierTest.controllerNumber, controllerNumber) + + case controllerLocation != identifierTest.controllerLocation: + t.Fatalf("Test %q (gen %v): controllerLocation: %v != %v", identifierTest.deviceIdentifier, identifierTest.generation, + identifierTest.controllerLocation, controllerLocation) + + } + } +} \ No newline at end of file