From 67e696bf624582342ee796d604aad4d1390b46a2 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Fri, 11 Jan 2019 16:46:48 +0100 Subject: [PATCH] virtcontainers: Add Asset to the types package In order to move the hypervisor implementations into their own package, we need to put the asset type into the types package and break the hypervisor->asset->virtcontainers->hypervisor cyclic dependency. Fixes: #1119 Signed-off-by: Samuel Ortiz --- virtcontainers/hypervisor.go | 53 +++++++------- virtcontainers/sandbox.go | 10 +-- virtcontainers/sandbox_test.go | 8 ++- virtcontainers/{ => types}/asset.go | 88 +++++++++++++++--------- virtcontainers/{ => types}/asset_test.go | 33 ++++----- 5 files changed, 106 insertions(+), 86 deletions(-) rename virtcontainers/{ => types}/asset.go (58%) rename virtcontainers/{ => types}/asset_test.go (77%) diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 6d5335917c..4df16803a4 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/kata-containers/runtime/virtcontainers/device/config" + "github.com/kata-containers/runtime/virtcontainers/types" ) // HypervisorType describes an hypervisor type. @@ -221,7 +222,7 @@ type HypervisorConfig struct { // Each value in that map takes precedence over the configured assets. // For example, if there is a value for the "kernel" key in this map, // it will be used for the sandbox's kernel path instead of KernelPath. - customAssets map[assetType]*asset + customAssets map[types.AssetType]*types.Asset // BlockDeviceCacheSet specifies cache-related options will be set to block devices or not. BlockDeviceCacheSet bool @@ -357,53 +358,53 @@ func (conf *HypervisorConfig) AddKernelParam(p Param) error { return nil } -func (conf *HypervisorConfig) addCustomAsset(a *asset) error { - if a == nil || a.path == "" { +func (conf *HypervisorConfig) addCustomAsset(a *types.Asset) error { + if a == nil || a.Path() == "" { // We did not get a custom asset, we will use the default one. return nil } - if !a.valid() { - return fmt.Errorf("Invalid %s at %s", a.kind, a.path) + if !a.Valid() { + return fmt.Errorf("Invalid %s at %s", a.Type(), a.Path()) } - virtLog.Debugf("Using custom %v asset %s", a.kind, a.path) + virtLog.Debugf("Using custom %v asset %s", a.Type(), a.Path()) if conf.customAssets == nil { - conf.customAssets = make(map[assetType]*asset) + conf.customAssets = make(map[types.AssetType]*types.Asset) } - conf.customAssets[a.kind] = a + conf.customAssets[a.Type()] = a return nil } -func (conf *HypervisorConfig) assetPath(t assetType) (string, error) { +func (conf *HypervisorConfig) assetPath(t types.AssetType) (string, error) { // Custom assets take precedence over the configured ones a, ok := conf.customAssets[t] if ok { - return a.path, nil + return a.Path(), nil } // We could not find a custom asset for the given type, let's // fall back to the configured ones. switch t { - case kernelAsset: + case types.KernelAsset: return conf.KernelPath, nil - case imageAsset: + case types.ImageAsset: return conf.ImagePath, nil - case initrdAsset: + case types.InitrdAsset: return conf.InitrdPath, nil - case hypervisorAsset: + case types.HypervisorAsset: return conf.HypervisorPath, nil - case firmwareAsset: + case types.FirmwareAsset: return conf.FirmwarePath, nil default: return "", fmt.Errorf("Unknown asset type %v", t) } } -func (conf *HypervisorConfig) isCustomAsset(t assetType) bool { +func (conf *HypervisorConfig) isCustomAsset(t types.AssetType) bool { _, ok := conf.customAssets[t] if ok { return true @@ -414,52 +415,52 @@ func (conf *HypervisorConfig) isCustomAsset(t assetType) bool { // KernelAssetPath returns the guest kernel path func (conf *HypervisorConfig) KernelAssetPath() (string, error) { - return conf.assetPath(kernelAsset) + return conf.assetPath(types.KernelAsset) } // CustomKernelAsset returns true if the kernel asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomKernelAsset() bool { - return conf.isCustomAsset(kernelAsset) + return conf.isCustomAsset(types.KernelAsset) } // ImageAssetPath returns the guest image path func (conf *HypervisorConfig) ImageAssetPath() (string, error) { - return conf.assetPath(imageAsset) + return conf.assetPath(types.ImageAsset) } // CustomImageAsset returns true if the image asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomImageAsset() bool { - return conf.isCustomAsset(imageAsset) + return conf.isCustomAsset(types.ImageAsset) } // InitrdAssetPath returns the guest initrd path func (conf *HypervisorConfig) InitrdAssetPath() (string, error) { - return conf.assetPath(initrdAsset) + return conf.assetPath(types.InitrdAsset) } // CustomInitrdAsset returns true if the initrd asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomInitrdAsset() bool { - return conf.isCustomAsset(initrdAsset) + return conf.isCustomAsset(types.InitrdAsset) } // HypervisorAssetPath returns the VM hypervisor path func (conf *HypervisorConfig) HypervisorAssetPath() (string, error) { - return conf.assetPath(hypervisorAsset) + return conf.assetPath(types.HypervisorAsset) } // CustomHypervisorAsset returns true if the hypervisor asset is a custom one, false otherwise. func (conf *HypervisorConfig) CustomHypervisorAsset() bool { - return conf.isCustomAsset(hypervisorAsset) + return conf.isCustomAsset(types.HypervisorAsset) } // FirmwareAssetPath returns the guest firmware path func (conf *HypervisorConfig) FirmwareAssetPath() (string, error) { - return conf.assetPath(firmwareAsset) + 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(firmwareAsset) + return conf.isCustomAsset(types.FirmwareAsset) } func appendParam(params []Param, parameter string, value string) []Param { diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 51a3a7f745..a3794ab90f 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -435,26 +435,26 @@ func createAssets(ctx context.Context, sandboxConfig *SandboxConfig) error { span, _ := trace(ctx, "createAssets") defer span.Finish() - kernel, err := newAsset(sandboxConfig, kernelAsset) + kernel, err := types.NewAsset(sandboxConfig.Annotations, types.KernelAsset) if err != nil { return err } - image, err := newAsset(sandboxConfig, imageAsset) + image, err := types.NewAsset(sandboxConfig.Annotations, types.ImageAsset) if err != nil { return err } - initrd, err := newAsset(sandboxConfig, initrdAsset) + 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", imageAsset, initrdAsset) + return fmt.Errorf("%s and %s cannot be both set", types.ImageAsset, types.InitrdAsset) } - for _, a := range []*asset{kernel, image, initrd} { + for _, a := range []*types.Asset{kernel, image, initrd} { if err := sandboxConfig.HypervisorConfig.addCustomAsset(a); err != nil { return err } diff --git a/virtcontainers/sandbox_test.go b/virtcontainers/sandbox_test.go index 125e354220..84ee50ebb0 100644 --- a/virtcontainers/sandbox_test.go +++ b/virtcontainers/sandbox_test.go @@ -1201,6 +1201,10 @@ func TestSandboxAttachDevicesVFIO(t *testing.T) { assert.Nil(t, err, "Error while detaching devices %s", err) } +var assetContent = []byte("FakeAsset fake asset FAKE ASSET") +var assetContentHash = "92549f8d2018a95a294d28a65e795ed7d1a9d150009a28cea108ae10101178676f04ab82a6950d0099e4924f9c5e41dcba8ece56b75fc8b4e0a7492cb2a8c880" +var assetContentWrongHash = "92549f8d2018a95a294d28a65e795ed7d1a9d150009a28cea108ae10101178676f04ab82a6950d0099e4924f9c5e41dcba8ece56b75fc8b4e0a7492cb2a8c881" + func TestSandboxCreateAssets(t *testing.T) { assert := assert.New(t) @@ -1234,9 +1238,9 @@ func TestSandboxCreateAssets(t *testing.T) { err = createAssets(context.Background(), p) assert.Nil(err) - a, ok := p.HypervisorConfig.customAssets[kernelAsset] + a, ok := p.HypervisorConfig.customAssets[types.KernelAsset] assert.True(ok) - assert.Equal(a.path, tmpfile.Name()) + assert.Equal(a.Path(), tmpfile.Name()) p = &SandboxConfig{ Annotations: map[string]string{ diff --git a/virtcontainers/asset.go b/virtcontainers/types/asset.go similarity index 58% rename from virtcontainers/asset.go rename to virtcontainers/types/asset.go index 8294fc5f23..dd9bab3b7c 100644 --- a/virtcontainers/asset.go +++ b/virtcontainers/types/asset.go @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -package virtcontainers +package types import ( "crypto/sha512" @@ -15,19 +15,21 @@ import ( "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" ) -type assetType string +// AssetType describe a type of assets. +type AssetType string -func (t assetType) annotations() (string, string, error) { +// Annotations returns the path and hash annotations for a given Asset type. +func (t AssetType) Annotations() (string, string, error) { switch t { - case kernelAsset: + case KernelAsset: return annotations.KernelPath, annotations.KernelHash, nil - case imageAsset: + case ImageAsset: return annotations.ImagePath, annotations.ImageHash, nil - case initrdAsset: + case InitrdAsset: return annotations.InitrdPath, annotations.InitrdHash, nil - case hypervisorAsset: + case HypervisorAsset: return annotations.HypervisorPath, annotations.HypervisorHash, nil - case firmwareAsset: + case FirmwareAsset: return annotations.FirmwarePath, annotations.FirmwareHash, nil } @@ -35,42 +37,63 @@ func (t assetType) annotations() (string, string, error) { } const ( - kernelAsset assetType = "kernel" - imageAsset assetType = "image" - initrdAsset assetType = "initrd" - hypervisorAsset assetType = "hypervisor" - firmwareAsset assetType = "firmware" + // KernelAsset is a kernel asset. + KernelAsset AssetType = "kernel" + + // ImageAsset is an image asset. + ImageAsset AssetType = "image" + + // InitrdAsset is an intird asset. + InitrdAsset AssetType = "initrd" + + // HypervisorAsset is an hypervisor asset. + HypervisorAsset AssetType = "hypervisor" + + // FirmwareAsset is a firmware asset. + FirmwareAsset AssetType = "firmware" ) -type asset struct { +// Asset represents a virtcontainers asset. +type Asset struct { path string computedHash string - kind assetType + kind AssetType } -func (a *asset) valid() bool { +// Path returns an asset path. +func (a Asset) Path() string { + return a.path +} + +// Type returns an asset type. +func (a Asset) Type() AssetType { + return a.kind +} + +// Valid checks if an asset is valid or not. +func (a *Asset) Valid() bool { if !filepath.IsAbs(a.path) { return false } switch a.kind { - case kernelAsset: + case KernelAsset: return true - case imageAsset: + case ImageAsset: return true - case initrdAsset: + case InitrdAsset: return true - case hypervisorAsset: + case HypervisorAsset: return true - case firmwareAsset: + case FirmwareAsset: return true } return false } -// hash returns the hex encoded string for the asset hash -func (a *asset) hash(hashType string) (string, error) { +// Hash returns the hex encoded string for the asset hash +func (a *Asset) Hash(hashType string) (string, error) { var hashEncodedLen int var hash string @@ -88,13 +111,11 @@ func (a *asset) hash(hashType string) (string, error) { // We only support SHA512 for now. switch hashType { case annotations.SHA512: - virtLog.Debugf("Computing %v hash", a.path) hashComputed := sha512.Sum512(bytes) hashEncodedLen = hex.EncodedLen(len(hashComputed)) hashEncoded := make([]byte, hashEncodedLen) hex.Encode(hashEncoded, hashComputed[:]) hash = string(hashEncoded[:]) - virtLog.Debugf("%v hash: %s", a.path, hash) default: return "", fmt.Errorf("Invalid hash type %s", hashType) } @@ -104,9 +125,9 @@ func (a *asset) hash(hashType string) (string, error) { return hash, nil } -// newAsset returns a new asset from the sandbox annotations. -func newAsset(sandboxConfig *SandboxConfig, t assetType) (*asset, error) { - pathAnnotation, hashAnnotation, err := t.annotations() +// NewAsset returns a new asset from a slice of annotations. +func NewAsset(anno map[string]string, t AssetType) (*Asset, error) { + pathAnnotation, hashAnnotation, err := t.Annotations() if err != nil { return nil, err } @@ -115,7 +136,7 @@ func newAsset(sandboxConfig *SandboxConfig, t assetType) (*asset, error) { return nil, fmt.Errorf("Missing annotation paths for %s", t) } - path, ok := sandboxConfig.Annotations[pathAnnotation] + path, ok := anno[pathAnnotation] if !ok || path == "" { return nil, nil } @@ -124,21 +145,20 @@ func newAsset(sandboxConfig *SandboxConfig, t assetType) (*asset, error) { return nil, fmt.Errorf("%s is not an absolute path", path) } - a := &asset{path: path, kind: t} + a := &Asset{path: path, kind: t} - hash, ok := sandboxConfig.Annotations[hashAnnotation] + hash, ok := anno[hashAnnotation] if !ok || hash == "" { return a, nil } // We have a hash annotation, we need to verify the asset against it. - hashType, ok := sandboxConfig.Annotations[annotations.AssetHashType] + hashType, ok := anno[annotations.AssetHashType] if !ok { - virtLog.Warningf("Unrecognized hash type: %s, switching to %s", hashType, annotations.SHA512) hashType = annotations.SHA512 } - hashComputed, err := a.hash(hashType) + hashComputed, err := a.Hash(hashType) if err != nil { return a, err } diff --git a/virtcontainers/asset_test.go b/virtcontainers/types/asset_test.go similarity index 77% rename from virtcontainers/asset_test.go rename to virtcontainers/types/asset_test.go index dd414fec2d..b025fe0e03 100644 --- a/virtcontainers/asset_test.go +++ b/virtcontainers/types/asset_test.go @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 // -package virtcontainers +package types import ( "io/ioutil" @@ -32,11 +32,11 @@ func TestAssetWrongHashType(t *testing.T) { _, err = tmpfile.Write(assetContent) assert.Nil(err) - a := &asset{ + a := &Asset{ path: tmpfile.Name(), } - h, err := a.hash("shafoo") + h, err := a.Hash("shafoo") assert.Equal(h, "") assert.NotNil(err) } @@ -55,11 +55,11 @@ func TestAssetHash(t *testing.T) { _, err = tmpfile.Write(assetContent) assert.Nil(err) - a := &asset{ + a := &Asset{ path: tmpfile.Name(), } - hash, err := a.hash(annotations.SHA512) + hash, err := a.Hash(annotations.SHA512) assert.Nil(err) assert.Equal(assetContentHash, hash) assert.Equal(assetContentHash, a.computedHash) @@ -79,28 +79,23 @@ func TestAssetNew(t *testing.T) { _, err = tmpfile.Write(assetContent) assert.Nil(err) - p := &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentHash, - }, + anno := map[string]string{ + annotations.KernelPath: tmpfile.Name(), + annotations.KernelHash: assetContentHash, } - - a, err := newAsset(p, imageAsset) + a, err := NewAsset(anno, ImageAsset) assert.Nil(err) assert.Nil(a) - a, err = newAsset(p, kernelAsset) + a, err = NewAsset(anno, KernelAsset) assert.Nil(err) assert.Equal(assetContentHash, a.computedHash) - p = &SandboxConfig{ - Annotations: map[string]string{ - annotations.KernelPath: tmpfile.Name(), - annotations.KernelHash: assetContentWrongHash, - }, + anno = map[string]string{ + annotations.KernelPath: tmpfile.Name(), + annotations.KernelHash: assetContentWrongHash, } - _, err = newAsset(p, kernelAsset) + _, err = NewAsset(anno, KernelAsset) assert.NotNil(err) }