github/kata-containers
1
0
Fork 1
mirror of https://github.com/kata-containers/kata-containers.git synced 2025-04-29 20:24:31 +00:00
Code Issues Projects Releases Wiki Activity

virtcontainers: validate hypervisor config outside of hypervisor itself

Browse Source 
Depending on the user of it, the hypervisor from hypervisor interface
could have differing view on what is valid or not. To help decouple,
let's instead check the hypervisor config validity as part of the
sandbox creation, rather than as part of the CreateVM call within the
hypervisor interface implementation.

Fixes: #4251

Signed-off-by: Eric Ernst <eric_ernst@apple.com>
This commit is contained in:
Eric Ernst 2022-05-23 15:58:47 -07:00
parent 469e098543
commit bdf5e5229b
10 changed files with 64 additions and 211 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/runtime/virtcontainers/acrn.go
View File

@ -348,10 +348,6 @@ func (a *Acrn) createDummyVirtioBlkDev(ctx context.Context, devices []Device) ([
}
func (a *Acrn) setConfig(config *HypervisorConfig) error {
if err := config.Valid(); err != nil {
return err
}
a.config = *config
return nil

5
src/runtime/virtcontainers/clh.go
View File

@ -268,11 +268,6 @@ var clhDebugKernelParams = []Param{
//###########################################################
func (clh *cloudHypervisor) setConfig(config *HypervisorConfig) error {
err := config.Valid()
if err != nil {
return err
}
clh.config = *config
return nil

5
src/runtime/virtcontainers/fc.go
View File

@ -189,11 +189,6 @@ func (fc *firecracker) truncateID(id string) string {
}
func (fc *firecracker) setConfig(config *HypervisorConfig) error {
err := config.Valid()
if err != nil {
return err
}
fc.config = *config
return nil

55
src/runtime/virtcontainers/hypervisor.go
View File

@ -568,61 +568,6 @@ func (conf *HypervisorConfig) CheckTemplateConfig() error {
return nil
}
func (conf *HypervisorConfig) Valid() error {
// Kata specific checks. Should be done outside the hypervisor
if conf.KernelPath == "" {
return fmt.Errorf("Missing kernel path")
}
if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio {
if conf.ImagePath != "" || conf.InitrdPath != "" {
fmt.Println("yes, failing")
return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution")
}
} else if conf.ImagePath == "" && conf.InitrdPath == "" {
return fmt.Errorf("Missing image and initrd path")
} else if conf.ImagePath != "" && conf.InitrdPath != "" {
return fmt.Errorf("Image and initrd path cannot be both set")
}
if err := conf.CheckTemplateConfig(); err != nil {
return err
}
if conf.NumVCPUs == 0 {
conf.NumVCPUs = defaultVCPUs
}
if conf.MemorySize == 0 {
conf.MemorySize = defaultMemSzMiB
}
if conf.DefaultBridges == 0 {
conf.DefaultBridges = defaultBridges
}
if conf.BlockDeviceDriver == "" {
conf.BlockDeviceDriver = defaultBlockDriver
} else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio {
conf.BlockDeviceDriver = config.VirtioBlockCCW
}
if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs {
conf.DefaultMaxVCPUs = defaultMaxVCPUs
}
if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs {
hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs)
conf.DefaultMaxVCPUs = conf.NumVCPUs
}
if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS {
conf.Msize9p = defaultMsize9p
}
return nil
}
// AddKernelParam allows the addition of new kernel parameters to an existing
// hypervisor configuration.
func (conf *HypervisorConfig) AddKernelParam(p Param) error {

118
src/runtime/virtcontainers/hypervisor_test.go
View File

@ -86,124 +86,6 @@ func TestNewHypervisorFromUnknownHypervisorType(t *testing.T) {
assert.Nil(hy)
}
func testHypervisorConfigValid(t *testing.T, hypervisorConfig *HypervisorConfig, success bool) {
err := hypervisorConfig.Valid()
assert := assert.New(t)
assert.False(success && err != nil)
assert.False(!success && err == nil)
}
func TestHypervisorConfigNoKernelPath(t *testing.T) {
hypervisorConfig := &HypervisorConfig{
KernelPath: "",
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
}
testHypervisorConfigValid(t, hypervisorConfig, false)
}
func TestHypervisorConfigNoImagePath(t *testing.T) {
hypervisorConfig := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: "",
HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
}
testHypervisorConfigValid(t, hypervisorConfig, false)
}
func TestHypervisorConfigNoHypervisorPath(t *testing.T) {
hypervisorConfig := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: "",
}
testHypervisorConfigValid(t, hypervisorConfig, true)
}
func TestHypervisorConfigIsValid(t *testing.T) {
hypervisorConfig := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
}
testHypervisorConfigValid(t, hypervisorConfig, true)
}
func TestHypervisorConfigBothInitrdAndImage(t *testing.T) {
hypervisorConfig := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
InitrdPath: fmt.Sprintf("%s/%s", testDir, testInitrd),
HypervisorPath: "",
}
testHypervisorConfigValid(t, hypervisorConfig, false)
}
func TestHypervisorConfigSecureExecution(t *testing.T) {
hypervisorConfig := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
InitrdPath: fmt.Sprintf("%s/%s", testDir, testInitrd),
ConfidentialGuest: true,
HypervisorMachineType: QemuCCWVirtio,
}
// Secure Execution should only specify a kernel (encrypted image contains all components)
testHypervisorConfigValid(t, hypervisorConfig, false)
}
func TestHypervisorConfigValidTemplateConfig(t *testing.T) {
hypervisorConfig := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
BootToBeTemplate: true,
BootFromTemplate: true,
}
testHypervisorConfigValid(t, hypervisorConfig, false)
hypervisorConfig.BootToBeTemplate = false
testHypervisorConfigValid(t, hypervisorConfig, false)
hypervisorConfig.MemoryPath = "foobar"
testHypervisorConfigValid(t, hypervisorConfig, false)
hypervisorConfig.DevicesStatePath = "foobar"
testHypervisorConfigValid(t, hypervisorConfig, true)
hypervisorConfig.BootFromTemplate = false
hypervisorConfig.BootToBeTemplate = true
testHypervisorConfigValid(t, hypervisorConfig, true)
hypervisorConfig.MemoryPath = ""
testHypervisorConfigValid(t, hypervisorConfig, false)
}
func TestHypervisorConfigDefaults(t *testing.T) {
assert := assert.New(t)
hypervisorConfig := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: "",
}
testHypervisorConfigValid(t, hypervisorConfig, true)
hypervisorConfigDefaultsExpected := &HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: "",
NumVCPUs: defaultVCPUs,
MemorySize: defaultMemSzMiB,
DefaultBridges: defaultBridges,
BlockDeviceDriver: defaultBlockDriver,
DefaultMaxVCPUs: defaultMaxVCPUs,
Msize9p: defaultMsize9p,
}
assert.Exactly(hypervisorConfig, hypervisorConfigDefaultsExpected)
}
func TestAppendParams(t *testing.T) {
assert := assert.New(t)
paramList := []Param{

4
src/runtime/virtcontainers/mock_hypervisor.go
View File

@ -31,10 +31,6 @@ func (m *mockHypervisor) HypervisorConfig() HypervisorConfig {
}
func (m *mockHypervisor) setConfig(config *HypervisorConfig) error {
if err := config.Valid(); err != nil {
return err
}
return nil
}

16
src/runtime/virtcontainers/mock_hypervisor_test.go
View File

@ -21,9 +21,9 @@ func TestMockHypervisorCreateVM(t *testing.T) {
config: &SandboxConfig{
ID: "mock_sandbox",
HypervisorConfig: HypervisorConfig{
KernelPath: "",
ImagePath: "",
HypervisorPath: "",
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
},
},
}
@ -33,16 +33,6 @@ func TestMockHypervisorCreateVM(t *testing.T) {
ctx := context.Background()
// wrong config
err = m.CreateVM(ctx, sandbox.config.ID, network, &sandbox.config.HypervisorConfig)
assert.Error(err)
sandbox.config.HypervisorConfig = HypervisorConfig{
KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel),
ImagePath: fmt.Sprintf("%s/%s", testDir, testImage),
HypervisorPath: fmt.Sprintf("%s/%s", testDir, testHypervisor),
}
err = m.CreateVM(ctx, sandbox.config.ID, network, &sandbox.config.HypervisorConfig)
assert.NoError(err)
}

5
src/runtime/virtcontainers/qemu.go
View File

@ -461,11 +461,6 @@ func (q *qemu) setupFileBackedMem(knobs *govmmQemu.Knobs, memory *govmmQemu.Memo
}
func (q *qemu) setConfig(config *HypervisorConfig) error {
err := config.Valid()
if err != nil {
return err
}
q.config = *config
return nil

60
src/runtime/virtcontainers/sandbox.go
View File

@ -593,6 +593,10 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
s.Logger().WithError(err).Debug("restore sandbox failed")
}
if err := validateHypervisorConfig(&sandboxConfig.HypervisorConfig); err != nil {
return nil, err
}
// store doesn't require hypervisor to be stored immediately
if err = s.hypervisor.CreateVM(ctx, s.id, s.network, &sandboxConfig.HypervisorConfig); err != nil {
return nil, err
@ -605,6 +609,62 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor
return s, nil
}
func validateHypervisorConfig(conf *HypervisorConfig) error {
if conf.KernelPath == "" {
return fmt.Errorf("Missing kernel path")
}
QemuCCWVirtio := ""
if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio {
if conf.ImagePath != "" || conf.InitrdPath != "" {
fmt.Println("yes, failing")
return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution")
}
} else if conf.ImagePath == "" && conf.InitrdPath == "" {
return fmt.Errorf("Missing image and initrd path")
} else if conf.ImagePath != "" && conf.InitrdPath != "" {
return fmt.Errorf("Image and initrd path cannot be both set")
}
if err := conf.CheckTemplateConfig(); err != nil {
return err
}
if conf.NumVCPUs == 0 {
conf.NumVCPUs = defaultVCPUs
}
if conf.MemorySize == 0 {
conf.MemorySize = defaultMemSzMiB
}
if conf.DefaultBridges == 0 {
conf.DefaultBridges = defaultBridges
}
if conf.BlockDeviceDriver == "" {
conf.BlockDeviceDriver = defaultBlockDriver
} else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio {
conf.BlockDeviceDriver = config.VirtioBlockCCW
}
if conf.DefaultMaxVCPUs == 0 || conf.DefaultMaxVCPUs > defaultMaxVCPUs {
conf.DefaultMaxVCPUs = defaultMaxVCPUs
}
if conf.ConfidentialGuest && conf.NumVCPUs != conf.DefaultMaxVCPUs {
hvLogger.Warnf("Confidential guests do not support hotplugging of vCPUs. Setting DefaultMaxVCPUs to NumVCPUs (%d)", conf.NumVCPUs)
conf.DefaultMaxVCPUs = conf.NumVCPUs
}
if conf.Msize9p == 0 && conf.SharedFS != config.VirtioFS {
conf.Msize9p = defaultMsize9p
}
return nil
}
func (s *Sandbox) createResourceController() error {
var err error
cgroupPath := ""

3
src/runtime/virtcontainers/vm.go
View File

@ -42,9 +42,8 @@ type VMConfig struct {
HypervisorConfig HypervisorConfig
}
// Valid Check VMConfig validity.
func (c *VMConfig) Valid() error {
return c.HypervisorConfig.Valid()
return validateHypervisorConfig(&c.HypervisorConfig)
}
// ToGrpc convert VMConfig struct to grpc format pb.GrpcVMConfig.