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 <james.o.hunt@intel.com>
This commit is contained in:
James O. D. Hunt
2020-11-05 12:10:20 +00:00
parent 0f26f1cd6f
commit e82c9daec3
7 changed files with 271 additions and 102 deletions

View File

@@ -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)
}
}