From 770bff319f27365994a53fd09d97e54aae4b94c6 Mon Sep 17 00:00:00 2001 From: Jonathon Jongsma Date: Fri, 11 Oct 2024 14:31:43 -0500 Subject: [PATCH 1/2] ova: Don't crash on ova files when device is missing 'ElementName' I was testing an ova file generated by virtualbox and it did not satisfy the assumptions made within the ova provider code. In particular, not all devices contained an `ElementName` xml element. Don't crash when encountering images like this. Error encountered was as follows: 2024/10/11 17:38:06 http: panic serving 10.217.0.134:34376: runtime error: slice bounds out of range [:-2] goroutine 7659 [running]: net/http.(*conn).serve.func1() GOROOT/src/net/http/server.go:1868 +0xb9 panic({0x25c640?, 0xc0003070b0?}) GOROOT/src/runtime/panic.go:920 +0x270 main.convertToVmStruct({0xc000223c00, 0x3, 0x4?}, {0xc0002aaec0, 0x3, 0x68943e?}) cmd/ova-provider-server/ova-provider-server.go:470 +0x105c main.vmHandler({0x2f1a48?, 0xc00021c700}, 0xc00014db18?) cmd/ova-provider-server/ova-provider-server.go:229 +0x7a net/http.HandlerFunc.ServeHTTP(0x4a6500?, {0x2f1a48?, 0xc00021c700?}, 0x68711a?) GOROOT/src/net/http/server.go:2136 +0x29 net/http.(*ServeMux).ServeHTTP(0x718080?, {0x2f1a48, 0xc00021c700}, 0xc000200200) GOROOT/src/net/http/server.go:2514 +0x142 net/http.serverHandler.ServeHTTP({0xc000547a70?}, {0x2f1a48?, 0xc00021c700?}, 0x6?) GOROOT/src/net/http/server.go:2938 +0x8e net/http.(*conn).serve(0xc0002021b0, {0x2f2058, 0xc0000ad2c0}) GOROOT/src/net/http/server.go:2009 +0x5f4 created by net/http.(*Server).Serve in goroutine 1 GOROOT/src/net/http/server.go:3086 +0x5cb Signed-off-by: Jonathon Jongsma --- cmd/ova-provider-server/ova-provider-server.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd/ova-provider-server/ova-provider-server.go b/cmd/ova-provider-server/ova-provider-server.go index b62156e4d..3118b9e13 100644 --- a/cmd/ova-provider-server/ova-provider-server.go +++ b/cmd/ova-provider-server/ova-provider-server.go @@ -466,9 +466,11 @@ func convertToVmStruct(envelope []Envelope, ovaPath []string) ([]VM, error) { newVM.MemoryUnits = item.AllocationUnits } else { - newVM.Devices = append(newVM.Devices, Device{ - Kind: item.ElementName[:len(item.ElementName)-2], - }) + if len(item.ElementName) > 2 { + newVM.Devices = append(newVM.Devices, Device{ + Kind: item.ElementName[:len(item.ElementName)-2], + }) + } } } From 5a9e04dd82eea694a585b0d0bdb53fe1284e63f3 Mon Sep 17 00:00:00 2001 From: Jonathon Jongsma Date: Mon, 14 Oct 2024 15:51:45 -0500 Subject: [PATCH 2/2] ova: improve handling of extra devices When encountering an ova file, we iterate throught the devices listed in the of the file and attempt to identify items that are useful to us (notably Network Adapters, memory definition, etc). The remaining items are stored as a list of generic devices with a `Kind` that is a free-form string. It appears that we tried to make the generic devices slightly nicer by stripping off number suffixes from the names of the devices and setting the device `Kind` to e.g. "SCSI Controller" rather than "SCSI Controller 1". But the code assumes that *all* `Items` in this file have the same numeric suffix format for `ElementName`. In reality, the `Items` vary quite a bit. This variation occurs both between different `Items` within the same file, and especially between ova files that are generated by different sources. For example, in an ova file that was generated with vsphere 7.0, the `ElementName` for my video device is simply "Video Card" with no suffix, whereas the Hard Disk has an `ElementName` of "Hard Disk 1". So the ova provider transforms the video card into a generic device with Kind set to the truncated string "Video Ca". In addition, I encountered ova files that were produced by VirtualBox where many of the `Items` in this list did not even contain `ElementName` elements, but did have `Description`s. So to make the ova provider more robust to files that don't follow our current assumptions: - only trim the suffix if the `ElementName` string ends with a space and a digit - if the `ElementName` does not exist at all, use `Description` - If neither of these elements exist, set the `Kind` to "Unknown" Signed-off-by: Jonathon Jongsma --- .../ova-provider-server.go | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/cmd/ova-provider-server/ova-provider-server.go b/cmd/ova-provider-server/ova-provider-server.go index 3118b9e13..318db9248 100644 --- a/cmd/ova-provider-server/ova-provider-server.go +++ b/cmd/ova-provider-server/ova-provider-server.go @@ -15,6 +15,7 @@ import ( "path/filepath" "strconv" "strings" + "unicode" "github.com/konveyor/forklift-controller/pkg/lib/gob" @@ -466,11 +467,28 @@ func convertToVmStruct(envelope []Envelope, ovaPath []string) ([]VM, error) { newVM.MemoryUnits = item.AllocationUnits } else { - if len(item.ElementName) > 2 { - newVM.Devices = append(newVM.Devices, Device{ - Kind: item.ElementName[:len(item.ElementName)-2], - }) + var itemKind string + if len(item.ElementName) == 0 { + if len(item.Description) > 0 { + itemKind = item.Description + } else { + itemKind = "Unknown" + } + } else { + // if the `ElementName` element has a name such as "Hard Disk 1", strip off the + // number suffix + runes := []rune(item.ElementName) + if len(runes) > 2 && + unicode.IsSpace(runes[len(runes)-2]) && + unicode.IsDigit(runes[len(runes)-1]) { + itemKind = item.ElementName[:len(item.ElementName)-2] + } else { + itemKind = item.ElementName + } } + newVM.Devices = append(newVM.Devices, Device{ + Kind: itemKind, + }) } }