From 76064e3e2d562860b6f5fd32fdf20636f08ddf95 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 28 Oct 2020 14:56:02 +0000 Subject: [PATCH 1/4] asset: Formatting, grammar and whitespace Improve formatting, grammar and whitespace. Signed-off-by: James O. D. Hunt --- src/runtime/virtcontainers/types/asset.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/runtime/virtcontainers/types/asset.go b/src/runtime/virtcontainers/types/asset.go index 46a7313a5f..7dc111f831 100644 --- a/src/runtime/virtcontainers/types/asset.go +++ b/src/runtime/virtcontainers/types/asset.go @@ -45,15 +45,16 @@ const ( // ImageAsset is an image asset. ImageAsset AssetType = "image" - // InitrdAsset is an intird asset. + // InitrdAsset is an initrd asset. InitrdAsset AssetType = "initrd" // HypervisorAsset is an hypervisor asset. HypervisorAsset AssetType = "hypervisor" - // HypervisorCtlAsset is an hypervisor control asset. + // HypervisorCtlAsset is a hypervisor control asset. HypervisorCtlAsset AssetType = "hypervisorctl" - // JailerAsset is an jailer asset. + + // JailerAsset is a jailer asset. JailerAsset AssetType = "jailer" // FirmwareAsset is a firmware asset. From 0f26f1cd6f7513ce6c949b4a784c159b9884d847 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 29 Oct 2020 09:23:21 +0000 Subject: [PATCH 2/4] annotations: Add missing hypervisor control annotation Add missing annotation definitions for a hypervisor control binary: - `io.katacontainers.config.hypervisor.ctlpath` - `io.katacontainers.config.hypervisor.hypervisorctl_hash` Signed-off-by: James O. D. Hunt --- src/runtime/virtcontainers/pkg/annotations/annotations.go | 6 ++++++ src/runtime/virtcontainers/types/asset.go | 4 ++++ src/runtime/virtcontainers/types/asset_test.go | 2 ++ 3 files changed, 12 insertions(+) diff --git a/src/runtime/virtcontainers/pkg/annotations/annotations.go b/src/runtime/virtcontainers/pkg/annotations/annotations.go index d044e8891f..8e4306aef6 100644 --- a/src/runtime/virtcontainers/pkg/annotations/annotations.go +++ b/src/runtime/virtcontainers/pkg/annotations/annotations.go @@ -42,6 +42,9 @@ const ( // HypervisorPath is a sandbox annotation for passing a per container path pointing at the hypervisor that will run the container VM. HypervisorPath = kataAnnotHypervisorPrefix + "path" + // HypervisorCtlPath is a sandbox annotation for passing a per container path pointing at the hypervisor control binary that will run the container VM. + HypervisorCtlPath = kataAnnotHypervisorPrefix + "ctlpath" + // JailerPath is a sandbox annotation for passing a per container path pointing at the jailer that will constrain the container VM. JailerPath = kataAnnotHypervisorPrefix + "jailer_path" @@ -63,6 +66,9 @@ const ( // HypervisorHash is an sandbox annotation for passing a container hypervisor binary SHA-512 hash value. HypervisorHash = kataAnnotHypervisorPrefix + "hypervisor_hash" + // HypervisorCtlHash is a sandbox annotation for passing a container hypervisor control binary SHA-512 hash value. + HypervisorCtlHash = kataAnnotHypervisorPrefix + "hypervisorctl_hash" + // JailerHash is an sandbox annotation for passing a jailer binary SHA-512 hash value. JailerHash = kataAnnotHypervisorPrefix + "jailer_hash" diff --git a/src/runtime/virtcontainers/types/asset.go b/src/runtime/virtcontainers/types/asset.go index 7dc111f831..329cd24c4c 100644 --- a/src/runtime/virtcontainers/types/asset.go +++ b/src/runtime/virtcontainers/types/asset.go @@ -29,6 +29,8 @@ func (t AssetType) Annotations() (string, string, error) { return annotations.InitrdPath, annotations.InitrdHash, nil case HypervisorAsset: return annotations.HypervisorPath, annotations.HypervisorHash, nil + case HypervisorCtlAsset: + return annotations.HypervisorCtlPath, annotations.HypervisorCtlHash, nil case JailerAsset: return annotations.JailerPath, annotations.JailerHash, nil case FirmwareAsset: @@ -93,6 +95,8 @@ func (a *Asset) Valid() bool { return true case HypervisorAsset: return true + case HypervisorCtlAsset: + return true case JailerAsset: return true case FirmwareAsset: diff --git a/src/runtime/virtcontainers/types/asset_test.go b/src/runtime/virtcontainers/types/asset_test.go index 4ddbb0e43a..d645173cc5 100644 --- a/src/runtime/virtcontainers/types/asset_test.go +++ b/src/runtime/virtcontainers/types/asset_test.go @@ -116,6 +116,7 @@ func TestAssetNew(t *testing.T) { {annotations.ImagePath, annotations.ImageHash, ImageAsset, assetContentHash, false, false}, {annotations.InitrdPath, annotations.InitrdHash, InitrdAsset, assetContentHash, false, false}, {annotations.HypervisorPath, annotations.HypervisorHash, HypervisorAsset, assetContentHash, false, false}, + {annotations.HypervisorCtlPath, annotations.HypervisorCtlHash, HypervisorCtlAsset, assetContentHash, false, false}, {annotations.JailerPath, annotations.JailerHash, JailerAsset, assetContentHash, false, false}, {annotations.FirmwarePath, annotations.FirmwareHash, FirmwareAsset, assetContentHash, false, false}, @@ -124,6 +125,7 @@ func TestAssetNew(t *testing.T) { {annotations.ImagePath, annotations.ImageHash, ImageAsset, assetContentWrongHash, true, false}, {annotations.InitrdPath, annotations.InitrdHash, InitrdAsset, assetContentWrongHash, true, false}, {annotations.HypervisorPath, annotations.HypervisorHash, HypervisorAsset, assetContentWrongHash, true, false}, + {annotations.HypervisorCtlPath, annotations.HypervisorCtlHash, HypervisorCtlAsset, assetContentWrongHash, true, false}, {annotations.JailerPath, annotations.JailerHash, JailerAsset, assetContentWrongHash, true, false}, {annotations.FirmwarePath, annotations.FirmwareHash, FirmwareAsset, assetContentWrongHash, true, false}, From e82c9daec3bc22a21d3dd242f05343caab9ef10f Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Thu, 5 Nov 2020 12:10:20 +0000 Subject: [PATCH 3/4] annotations: Improve asset annotation handling Make `asset.go` the arbiter of asset annotations by removing all asset annotations lists from other parts of the codebase. This makes the code simpler, easier to maintain, and more robust. Specifically, the previous behaviour was inconsistent as the following ways: - `createAssets()` in `sandbox.go` was not handling the following asset annotations: - firmware: - `io.katacontainers.config.hypervisor.firmware` - `io.katacontainers.config.hypervisor.firmware_hash` - hypervisor: - `io.katacontainers.config.hypervisor.path` - `io.katacontainers.config.hypervisor.hypervisor_hash` - hypervisor control binary: - `io.katacontainers.config.hypervisor.ctlpath` - `io.katacontainers.config.hypervisor.hypervisorctl_hash` - jailer: - `io.katacontainers.config.hypervisor.jailer_path` - `io.katacontainers.config.hypervisor.jailer_hash` - `addAssetAnnotations()` in the `oci` package was not handling the following asset annotations: - hypervisor: - `io.katacontainers.config.hypervisor.path` - `io.katacontainers.config.hypervisor.hypervisor_hash` - hypervisor control binary: - `io.katacontainers.config.hypervisor.ctlpath` - `io.katacontainers.config.hypervisor.hypervisorctl_hash` - jailer: - `io.katacontainers.config.hypervisor.jailer_path` - `io.katacontainers.config.hypervisor.jailer_hash` This change fixes the bug where specifying a custom hypervisor path via an asset annotation was having no effect. Fixes: #1085. Signed-off-by: James O. D. Hunt --- src/runtime/virtcontainers/hypervisor_test.go | 38 ++++++ src/runtime/virtcontainers/pkg/oci/utils.go | 26 ++-- .../virtcontainers/pkg/oci/utils_test.go | 53 +++++-- src/runtime/virtcontainers/sandbox.go | 31 ++--- src/runtime/virtcontainers/sandbox_test.go | 129 ++++++++++++++---- src/runtime/virtcontainers/types/asset.go | 94 ++++++++----- .../virtcontainers/virtcontainers_test.go | 2 + 7 files changed, 271 insertions(+), 102 deletions(-) diff --git a/src/runtime/virtcontainers/hypervisor_test.go b/src/runtime/virtcontainers/hypervisor_test.go index 8c4c160771..a3f3a108a0 100644 --- a/src/runtime/virtcontainers/hypervisor_test.go +++ b/src/runtime/virtcontainers/hypervisor_test.go @@ -447,3 +447,41 @@ func TestGenerateVMSocket(t *testing.T) { assert.NotZero(vsock.ContextID) assert.NotZero(vsock.Port) } + +func TestAssetPath(t *testing.T) { + assert := assert.New(t) + + // Minimal config containing values for all asset annotation options. + // The values are "paths" (start with a slash), but end with the + // annotation name. + cfg := HypervisorConfig{ + HypervisorPath: "/" + "io.katacontainers.config.hypervisor.path", + HypervisorCtlPath: "/" + "io.katacontainers.config.hypervisor.ctlpath", + + KernelPath: "/" + "io.katacontainers.config.hypervisor.kernel", + + ImagePath: "/" + "io.katacontainers.config.hypervisor.image", + InitrdPath: "/" + "io.katacontainers.config.hypervisor.initrd", + + FirmwarePath: "/" + "io.katacontainers.config.hypervisor.firmware", + JailerPath: "/" + "io.katacontainers.config.hypervisor.jailer_path", + } + + for _, asset := range types.AssetTypes() { + msg := fmt.Sprintf("asset: %v", asset) + + annoPath, annoHash, err := asset.Annotations() + assert.NoError(err, msg) + + msg += fmt.Sprintf(", annotation path: %v, annotation hash: %v", annoPath, annoHash) + + p, err := cfg.assetPath(asset) + assert.NoError(err, msg) + + assert.NotEqual(p, annoPath, msg) + assert.NotEqual(p, annoHash, msg) + + expected := fmt.Sprintf("/%s", annoPath) + assert.Equal(expected, p, msg) + } +} diff --git a/src/runtime/virtcontainers/pkg/oci/utils.go b/src/runtime/virtcontainers/pkg/oci/utils.go index 755e3c6b45..28ccd6782a 100644 --- a/src/runtime/virtcontainers/pkg/oci/utils.go +++ b/src/runtime/virtcontainers/pkg/oci/utils.go @@ -210,6 +210,7 @@ func checkPathIsInGlobs(globs []string, path string) bool { } } } + return false } @@ -218,6 +219,7 @@ func checkAnnotationNameIsValid(list []string, name string, prefix string) bool if strings.HasPrefix(name, prefix) { return regexpContains(list, strings.TrimPrefix(name, prefix)) } + return true } @@ -359,7 +361,12 @@ func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig, runtime Runtim return fmt.Errorf("annotation %v is not enabled", key) } } - addAssetAnnotations(ocispec, config) + + err := addAssetAnnotations(ocispec, config) + if err != nil { + return err + } + if err := addHypervisorConfigOverrides(ocispec, config, runtime); err != nil { return err } @@ -374,17 +381,10 @@ func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig, runtime Runtim return nil } -func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) { - assetAnnotations := []string{ - vcAnnotations.KernelPath, - vcAnnotations.ImagePath, - vcAnnotations.InitrdPath, - vcAnnotations.FirmwarePath, - vcAnnotations.KernelHash, - vcAnnotations.ImageHash, - vcAnnotations.InitrdHash, - vcAnnotations.FirmwareHash, - vcAnnotations.AssetHashType, +func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error { + assetAnnotations, err := types.AssetAnnotations() + if err != nil { + return err } for _, a := range assetAnnotations { @@ -393,6 +393,8 @@ func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) { config.Annotations[a] = value } } + + return nil } func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig, runtime RuntimeConfig) error { diff --git a/src/runtime/virtcontainers/pkg/oci/utils_test.go b/src/runtime/virtcontainers/pkg/oci/utils_test.go index a813bfae52..fca2bbbca4 100644 --- a/src/runtime/virtcontainers/pkg/oci/utils_test.go +++ b/src/runtime/virtcontainers/pkg/oci/utils_test.go @@ -659,13 +659,38 @@ func TestMain(m *testing.M) { func TestAddAssetAnnotations(t *testing.T) { assert := assert.New(t) + tmpdir, err := ioutil.TempDir("", "") + assert.NoError(err) + defer os.RemoveAll(tmpdir) + + // Create a pretend asset file + // (required since the existence of binary asset annotations is verified). + fakeAssetFile := filepath.Join(tmpdir, "fake-binary") + + err = ioutil.WriteFile(fakeAssetFile, []byte(""), fileMode) + assert.NoError(err) + expectedAnnotations := map[string]string{ - vcAnnotations.KernelPath: "/abc/rgb/kernel", - vcAnnotations.ImagePath: "/abc/rgb/image", - vcAnnotations.InitrdPath: "/abc/rgb/initrd", - vcAnnotations.KernelHash: "3l2353we871g", - vcAnnotations.ImageHash: "52ss2550983", - vcAnnotations.AssetHashType: "sha", + vcAnnotations.FirmwarePath: fakeAssetFile, + vcAnnotations.FirmwareHash: "ffff", + + vcAnnotations.HypervisorPath: fakeAssetFile, + vcAnnotations.HypervisorHash: "bbbbb", + + vcAnnotations.HypervisorCtlPath: fakeAssetFile, + vcAnnotations.HypervisorCtlHash: "cc", + + vcAnnotations.ImagePath: fakeAssetFile, + vcAnnotations.ImageHash: "52ss2550983", + + vcAnnotations.InitrdPath: fakeAssetFile, + vcAnnotations.InitrdHash: "aaaa", + + vcAnnotations.JailerPath: fakeAssetFile, + vcAnnotations.JailerHash: "dddd", + + vcAnnotations.KernelPath: fakeAssetFile, + vcAnnotations.KernelHash: "3l2353we871g", } config := vc.SandboxConfig{ @@ -682,17 +707,29 @@ func TestAddAssetAnnotations(t *testing.T) { } // Try annotations without enabling them first - err := addAnnotations(ocispec, &config, runtimeConfig) + err = addAnnotations(ocispec, &config, runtimeConfig) + assert.Error(err) assert.Exactly(map[string]string{}, config.Annotations) // Check if annotation not enabled correctly runtimeConfig.HypervisorConfig.EnableAnnotations = []string{"nonexistent"} err = addAnnotations(ocispec, &config, runtimeConfig) + assert.Error(err) - // Check that it works if all annotation are enabled + // Ensure it fails if all annotations enabled but path lists are not set runtimeConfig.HypervisorConfig.EnableAnnotations = []string{".*"} + err = addAnnotations(ocispec, &config, runtimeConfig) + assert.Error(err) + + tmpdirGlob := tmpdir + "/*" + + // Check that it works if all path lists are enabled + runtimeConfig.HypervisorConfig.HypervisorPathList = []string{tmpdirGlob} + runtimeConfig.HypervisorConfig.JailerPathList = []string{tmpdirGlob} + runtimeConfig.HypervisorConfig.HypervisorCtlPathList = []string{tmpdirGlob} + err = addAnnotations(ocispec, &config, runtimeConfig) assert.NoError(err) assert.Exactly(expectedAnnotations, config.Annotations) diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 16db127ea1..ff2c5729dd 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -384,31 +384,24 @@ func createAssets(ctx context.Context, sandboxConfig *SandboxConfig) error { span, _ := trace(ctx, "createAssets") defer span.Finish() - kernel, err := types.NewAsset(sandboxConfig.Annotations, types.KernelAsset) - if err != nil { - return err - } + for _, name := range types.AssetTypes() { + a, err := types.NewAsset(sandboxConfig.Annotations, name) + if err != nil { + return err + } - image, err := types.NewAsset(sandboxConfig.Annotations, types.ImageAsset) - if err != nil { - return err - } - - initrd, err := types.NewAsset(sandboxConfig.Annotations, types.InitrdAsset) - if err != nil { - return err - } - - if image != nil && initrd != nil { - return fmt.Errorf("%s and %s cannot be both set", types.ImageAsset, types.InitrdAsset) - } - - for _, a := range []*types.Asset{kernel, image, initrd} { if err := sandboxConfig.HypervisorConfig.addCustomAsset(a); err != nil { return err } } + _, imageErr := sandboxConfig.HypervisorConfig.assetPath(types.ImageAsset) + _, initrdErr := sandboxConfig.HypervisorConfig.assetPath(types.InitrdAsset) + + if imageErr != nil && initrdErr != nil { + return fmt.Errorf("%s and %s cannot be both set", types.ImageAsset, types.InitrdAsset) + } + return nil } diff --git a/src/runtime/virtcontainers/sandbox_test.go b/src/runtime/virtcontainers/sandbox_test.go index 6e6dd752c1..4adbf78989 100644 --- a/src/runtime/virtcontainers/sandbox_test.go +++ b/src/runtime/virtcontainers/sandbox_test.go @@ -13,6 +13,7 @@ import ( "os/exec" "path" "path/filepath" + "strings" "sync" "syscall" "testing" @@ -641,51 +642,127 @@ var assetContentWrongHash = "92549f8d2018a95a294d28a65e795ed7d1a9d150009a28cea10 func TestSandboxCreateAssets(t *testing.T) { assert := assert.New(t) + type testData struct { + assetType types.AssetType + annotations map[string]string + } + tmpfile, err := ioutil.TempFile("", "virtcontainers-test-") assert.Nil(err) + filename := tmpfile.Name() + defer func() { tmpfile.Close() - os.Remove(tmpfile.Name()) // clean up + os.Remove(filename) // clean up }() _, err = tmpfile.Write(assetContent) assert.Nil(err) originalKernelPath := filepath.Join(testDir, testKernel) + originalImagePath := filepath.Join(testDir, testImage) + originalInitrdPath := filepath.Join(testDir, testInitrd) + originalFirmwarePath := filepath.Join(testDir, testFirmware) + originalHypervisorPath := filepath.Join(testDir, testHypervisor) + originalHypervisorCtlPath := filepath.Join(testDir, testHypervisorCtl) + originalJailerPath := filepath.Join(testDir, testJailer) hc := HypervisorConfig{ - KernelPath: originalKernelPath, - ImagePath: filepath.Join(testDir, testImage), + KernelPath: originalKernelPath, + ImagePath: originalImagePath, + InitrdPath: originalInitrdPath, + FirmwarePath: originalFirmwarePath, + HypervisorPath: originalHypervisorPath, + HypervisorCtlPath: originalHypervisorCtlPath, + JailerPath: originalJailerPath, } - p := &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentHash, + data := []testData{ + { + types.FirmwareAsset, + map[string]string{ + annotations.FirmwarePath: filename, + annotations.FirmwareHash: assetContentHash, + }, }, - - HypervisorConfig: hc, - } - - err = createAssets(context.Background(), p) - assert.Nil(err) - - a, ok := p.HypervisorConfig.customAssets[types.KernelAsset] - assert.True(ok) - assert.Equal(a.Path(), tmpfile.Name()) - - p = &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentWrongHash, + { + types.HypervisorAsset, + map[string]string{ + annotations.HypervisorPath: filename, + annotations.HypervisorHash: assetContentHash, + }, + }, + { + types.HypervisorCtlAsset, + map[string]string{ + annotations.HypervisorCtlPath: filename, + annotations.HypervisorCtlHash: assetContentHash, + }, + }, + { + types.ImageAsset, + map[string]string{ + annotations.ImagePath: filename, + annotations.ImageHash: assetContentHash, + }, + }, + { + types.InitrdAsset, + map[string]string{ + annotations.InitrdPath: filename, + annotations.InitrdHash: assetContentHash, + }, + }, + { + types.JailerAsset, + map[string]string{ + annotations.JailerPath: filename, + annotations.JailerHash: assetContentHash, + }, + }, + { + types.KernelAsset, + map[string]string{ + annotations.KernelPath: filename, + annotations.KernelHash: assetContentHash, + }, }, - - HypervisorConfig: hc, } - err = createAssets(context.Background(), p) - assert.NotNil(err) + for i, d := range data { + msg := fmt.Sprintf("test[%d]: %+v", i, d) + + config := &SandboxConfig{ + Annotations: d.annotations, + HypervisorConfig: hc, + } + + err = createAssets(context.Background(), config) + assert.NoError(err, msg) + + a, ok := config.HypervisorConfig.customAssets[d.assetType] + assert.True(ok, msg) + assert.Equal(a.Path(), filename, msg) + + // Now test with invalid hashes + badHashAnnotations := make(map[string]string) + for k, v := range d.annotations { + if strings.HasSuffix(k, "_hash") { + badHashAnnotations[k] = assetContentWrongHash + } else { + badHashAnnotations[k] = v + } + } + + config = &SandboxConfig{ + Annotations: badHashAnnotations, + HypervisorConfig: hc, + } + + err = createAssets(context.Background(), config) + assert.Error(err, msg) + } } func testFindContainerFailure(t *testing.T, sandbox *Sandbox, cid string) { diff --git a/src/runtime/virtcontainers/types/asset.go b/src/runtime/virtcontainers/types/asset.go index 329cd24c4c..2cbca91f21 100644 --- a/src/runtime/virtcontainers/types/asset.go +++ b/src/runtime/virtcontainers/types/asset.go @@ -18,28 +18,6 @@ import ( // AssetType describe a type of assets. type AssetType string -// Annotations returns the path and hash annotations for a given Asset type. -func (t AssetType) Annotations() (string, string, error) { - switch t { - case KernelAsset: - return annotations.KernelPath, annotations.KernelHash, nil - case ImageAsset: - return annotations.ImagePath, annotations.ImageHash, nil - case InitrdAsset: - return annotations.InitrdPath, annotations.InitrdHash, nil - case HypervisorAsset: - return annotations.HypervisorPath, annotations.HypervisorHash, nil - case HypervisorCtlAsset: - return annotations.HypervisorCtlPath, annotations.HypervisorCtlHash, nil - case JailerAsset: - return annotations.JailerPath, annotations.JailerHash, nil - case FirmwareAsset: - return annotations.FirmwarePath, annotations.FirmwareHash, nil - } - - return "", "", fmt.Errorf("Wrong asset type %s", t) -} - const ( // KernelAsset is a kernel asset. KernelAsset AssetType = "kernel" @@ -63,6 +41,59 @@ const ( FirmwareAsset AssetType = "firmware" ) +// AssetTypes returns a list of all known asset types. +// +// XXX: New asset types *MUST* be added here. +func AssetTypes() []AssetType { + return []AssetType{ + FirmwareAsset, + HypervisorAsset, + HypervisorCtlAsset, + ImageAsset, + InitrdAsset, + JailerAsset, + KernelAsset, + } +} + +// AssetAnnotations returns all annotations for all asset types. +func AssetAnnotations() ([]string, error) { + var result []string + + for _, at := range AssetTypes() { + aPath, aHash, err := at.Annotations() + if err != nil { + return []string{}, err + } + + result = append(result, []string{aPath, aHash}...) + } + + return result, nil +} + +// Annotations returns the path and hash annotations for a given Asset type. +func (t AssetType) Annotations() (string, string, error) { + switch t { + case KernelAsset: + return annotations.KernelPath, annotations.KernelHash, nil + case ImageAsset: + return annotations.ImagePath, annotations.ImageHash, nil + case InitrdAsset: + return annotations.InitrdPath, annotations.InitrdHash, nil + case HypervisorAsset: + return annotations.HypervisorPath, annotations.HypervisorHash, nil + case HypervisorCtlAsset: + return annotations.HypervisorCtlPath, annotations.HypervisorCtlHash, nil + case JailerAsset: + return annotations.JailerPath, annotations.JailerHash, nil + case FirmwareAsset: + return annotations.FirmwarePath, annotations.FirmwareHash, nil + } + + return "", "", fmt.Errorf("Wrong asset type %s", t) +} + // Asset represents a virtcontainers asset. type Asset struct { path string @@ -86,21 +117,10 @@ func (a *Asset) Valid() bool { return false } - switch a.kind { - case KernelAsset: - return true - case ImageAsset: - return true - case InitrdAsset: - return true - case HypervisorAsset: - return true - case HypervisorCtlAsset: - return true - case JailerAsset: - return true - case FirmwareAsset: - return true + for _, at := range AssetTypes() { + if at == a.kind { + return true + } } return false diff --git a/src/runtime/virtcontainers/virtcontainers_test.go b/src/runtime/virtcontainers/virtcontainers_test.go index f955534c42..8a3d2a9854 100644 --- a/src/runtime/virtcontainers/virtcontainers_test.go +++ b/src/runtime/virtcontainers/virtcontainers_test.go @@ -28,6 +28,8 @@ const testKernel = "kernel" const testInitrd = "initrd" const testImage = "image" const testHypervisor = "hypervisor" +const testJailer = "jailer" +const testFirmware = "firmware" const testVirtiofsd = "virtiofsd" const testHypervisorCtl = "hypervisorctl" const testBundle = "bundle" From 5ced96e96d94b64b55163783097b518d668919e4 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 4 Nov 2020 11:37:43 +0000 Subject: [PATCH 4/4] hypervisor: Remove unused methods Deleted `HypervisorConfig`'s unused `CustomFirmwareAsset()` and `JailerAssetPath()` methods. Signed-off-by: James O. D. Hunt --- src/runtime/virtcontainers/hypervisor.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index c67971001a..639808659f 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -612,11 +612,6 @@ func (conf *HypervisorConfig) HypervisorCtlAssetPath() (string, error) { return conf.assetPath(types.HypervisorCtlAsset) } -// JailerAssetPath returns the VM Jailer path -func (conf *HypervisorConfig) JailerAssetPath() (string, error) { - return conf.assetPath(types.JailerAsset) -} - // CustomHypervisorAsset returns true if the hypervisor asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomHypervisorAsset() bool { return conf.isCustomAsset(types.HypervisorAsset) @@ -627,11 +622,6 @@ func (conf *HypervisorConfig) FirmwareAssetPath() (string, error) { return conf.assetPath(types.FirmwareAsset) } -// CustomFirmwareAsset returns true if the firmware asset is a custom one, false otherwise. -func (conf *HypervisorConfig) CustomFirmwareAsset() bool { - return conf.isCustomAsset(types.FirmwareAsset) -} - func appendParam(params []Param, parameter string, value string) []Param { return append(params, Param{parameter, value}) }