diff --git a/virtcontainers/hypervisor_test.go b/virtcontainers/hypervisor_test.go index 6f91287aa1..e4ce65df48 100644 --- a/virtcontainers/hypervisor_test.go +++ b/virtcontainers/hypervisor_test.go @@ -456,3 +456,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/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index dcd8fb23b0..617a1a42ef 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/virtcontainers/pkg/oci/utils.go @@ -330,7 +330,11 @@ func SandboxID(spec specs.Spec) (string, error) { } func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error { - addAssetAnnotations(ocispec, config) + err := addAssetAnnotations(ocispec, config) + if err != nil { + return err + } + if err := addHypervisorConfigOverrides(ocispec, config); err != nil { return err } @@ -345,17 +349,10 @@ func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error { 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 { @@ -366,6 +363,8 @@ func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) { config.Annotations[a] = value } + + return nil } func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig) error { diff --git a/virtcontainers/pkg/oci/utils_test.go b/virtcontainers/pkg/oci/utils_test.go index 0257a05ad0..133c3f5014 100644 --- a/virtcontainers/pkg/oci/utils_test.go +++ b/virtcontainers/pkg/oci/utils_test.go @@ -666,12 +666,26 @@ func TestAddAssetAnnotations(t *testing.T) { assert := assert.New(t) 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: "/some/where", + vcAnnotations.FirmwareHash: "ffff", + + vcAnnotations.HypervisorPath: "/some/where", + vcAnnotations.HypervisorHash: "bbbbb", + + vcAnnotations.HypervisorCtlPath: "/some/where/else", + vcAnnotations.HypervisorCtlHash: "cc", + + vcAnnotations.ImagePath: "/abc/rgb/image", + vcAnnotations.ImageHash: "52ss2550983", + + vcAnnotations.InitrdPath: "/abc/rgb/initrd", + vcAnnotations.InitrdHash: "aaaa", + + vcAnnotations.JailerPath: "/foo/bar", + vcAnnotations.JailerHash: "dddd", + + vcAnnotations.KernelPath: "/abc/rgb/kernel", + vcAnnotations.KernelHash: "3l2353we871g", } config := vc.SandboxConfig{ diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 302a28396b..3b08921aec 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -419,31 +419,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 + if err := sandboxConfig.HypervisorConfig.addCustomAsset(a); err != nil { + return err + } } - initrd, err := types.NewAsset(sandboxConfig.Annotations, types.InitrdAsset) - if err != nil { - return err - } + _, imageErr := sandboxConfig.HypervisorConfig.assetPath(types.ImageAsset) + _, initrdErr := sandboxConfig.HypervisorConfig.assetPath(types.InitrdAsset) - if image != nil && initrd != nil { + if imageErr != nil && initrdErr != 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 - } - } - return nil } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 0e90ed0ef9..4e6f817c15 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -643,51 +643,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, + }, + }, + { + 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.Nil(err) + for i, d := range data { + msg := fmt.Sprintf("test[%d]: %+v", i, d) - a, ok := p.HypervisorConfig.customAssets[types.KernelAsset] - assert.True(ok) - assert.Equal(a.Path(), tmpfile.Name()) + config := &SandboxConfig{ + Annotations: d.annotations, + HypervisorConfig: hc, + } - p = &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentWrongHash, - }, + err = createAssets(context.Background(), config) + assert.NoError(err, msg) - HypervisorConfig: hc, - } + 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(), p) - assert.NotNil(err) + err = createAssets(context.Background(), config) + assert.Error(err, msg) + } } func testFindContainerFailure(t *testing.T, sandbox *Sandbox, cid string) { diff --git a/virtcontainers/types/asset.go b/virtcontainers/types/asset.go index 6f6241e3f0..b95b19aa65 100644 --- a/virtcontainers/types/asset.go +++ b/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/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index 3d10476210..08eef22468 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -29,6 +29,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"