Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash on unexpected ova files #1106

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jonner
Copy link
Contributor

@jonner jonner commented Oct 14, 2024

See descriptions on the individual commits. This just makes the ova provider a bit more robust to unexpected inputs.

  • ova: Don't crash on ova files when device is missing 'ElementName'
  • ova: improve handling of extra devices

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 <[email protected]>
When encountering an ova file, we iterate throught the devices listed in
the <VirtualHardwareSection> 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 <[email protected]>
Copy link

sonarcloud bot commented Oct 14, 2024

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 16.19%. Comparing base (bc87852) to head (5a9e04d).

Files with missing lines Patch % Lines
cmd/ova-provider-server/ova-provider-server.go 0.00% 15 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1106      +/-   ##
==========================================
- Coverage   16.20%   16.19%   -0.02%     
==========================================
  Files         112      112              
  Lines       19882    19896      +14     
==========================================
  Hits         3222     3222              
- Misses      16375    16389      +14     
  Partials      285      285              
Flag Coverage Δ
unittests 16.19% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants