Rethink runtime annotations

[jodh-intel]

PR #902 added the ability to restrict the set of annotations the runtime will honour, based on a set of regular expressions. However, with the advent of the useful PR #2156, I think we need to do more.

Current design

• Set DEFENABLEANNOTATIONS in src/runtime/Makefile to a set of regular expressions. If an annotation matches a regex, it is allowed.
• This list is added to the auto-generated configuration file (such as configuration-qemu.toml) as enable_annotations =.
• When the runtime parses the OCI config file, it calls addAnnotations() (in src/runtime/virtcontainers/pkg/oci/utils.go) to check that the annotations specified in the OCI config file have been enabled. If any specified annotation is not enabled, the function returns and error.

Problems

• There is currently no way to list available annotations: you need to go and view the source file src/runtime/virtcontainers/pkg/annotations/annotations.go or look at the docs (which we have to remember to update...).
• As highlighted on runtime: Enable all the "non dangerous" annotations by default #2156, we arguably have an "annotation management issue": we have a lot of annotations and need to be able to create an allow or deny list to manage the default set. But currently we either have a huge allow list (safer) or a smaller deny list. The latter is easier to manage but less safe: if we use a deny list and add a new annotation but forget to add it to the deny list (even if it's "dangerous"), that new annotation will be allowed.

Proposals

Create a new runtime command to list annotations:

$ kata-runtime annotations list

This command will list atleast the following details (potentially in various output formats (csv, json, blah):

• The name of the annotation.

• The type of the annotation (int, bool, string, etc).

• The human readable textual description of the annotation.

• Whether the annotation would be enabled at runtime.
  (based on the current values speciifed in the enable_annotations = setting in the configuration file).

• Whether the annotation is "dangerous"

  It may be better to encode one or more "types" for each annotation, one of which could be "security"?

All fields would be mandatory.

Advantages

• Guaranteed correct annotation details: Admins wouldn't need to look at the source / potentiall outdated docs to find out what an annotation does.

• Simpler docs: we could auto-generate docs/how-to/how-to-set-sandbox-config-kata.md from the output of kata-runtime annotations list (or just tell admins the command to run themselves).

• Safer updates: If a new annotation is added to the runtime, it would need to specify the details above, meaning everyone would always be able to determine which annotations where "dangerous".

• We could generate a guaranteed safe set of "non-dangerous" annotations as part of the packaging by running something equivalent to:

  $ kata-runtime annotations list | grep -iv dangerous > configuration-annotations.toml

• We could "seed" the CI by creating a data file in the tests repo.

  Ever PR could then run a set of simple annotation tests that ran kata-runtime annotations list.
  If the output contains annotations not listed in the test repos data file, the CI would immediately fail
  (since it indicates we added an annotation without updating the tests config file). The correct way to deal with a new annotation would be to update the tests config file before testing a PR that added a new annotation.

Refactoring work

Remove the "bare const" annotation variables and instead create a function that returns a slice of all available annotations. There will be some additional rework (and unit tests) for this, but by making the annotation code more cohesive, we get the benefits outlined above.

The work would in fact be similar to #1086 (and would require rework of the asset types package).

See also

• runtime: Enable all the "non dangerous" annotations by default #2156
• Add swap support #2202

/cc @fidencio, @c3d, @amshinde, @egernst, @sameo

[eadamsintel]

I think this is a wonderful enhancement idea. Are you also suggesting to move the annotations enabled out of the Kata configuration.toml file and into a configuration-annotations.toml file? I personally would try to keep a single Kata configuration.toml for all configuration items for simplicity. I didn't catch it in the proposal but could you enable/disable annotations directly from kata-runtime. That would be a nice feature as well. I could see something like the following. I put multiple ones in a list since I assume people would want multiple annotations turned on and off by default. You could extend this idea with the dangerous annotations as well so you could enable everything but the dangerous ones or just enable every annotation with a single command.

$ sudo kata-runtime annotations enable kernel image kernel_params

I can see benefits to having the annotations configured in a separate file as well. In the case of kata-deploy or even a Kata operator you could install Kata but then drop in the supported annotation file to your cluster to ensure you set the right level of security. If the kata-deploy pod got restarted it might reinstall the kata artifacts over writing any custom config which is where a separate approved annotations file could make sense. I had this actually happen to me when I enabled all the annotations and 20 min later my configuration.toml got reverted.

[fidencio]

@eadamsintel, partially related to your comment, but I've been going back and forth with the idea of supporting drop-in snippets, something that could be read from /etc/kata-containers/configuration.d/00-annotations, for instance, so admins could simply drop-in a file to augment the default configuration with whatever they need.

This is something that's been on my long-term todo list for a while already, and I think that sooner or later (depending on my employer's wishlist) I'll end up taking the bullet and implementing it. While on this, I should track it. /o.

[eadamsintel]

I ran into what I think is a bug with the current implementation. I was just using kata-deploy and the configuration-qemu.toml has it set to enable_annotations = []. When I changed this to enable_annotations = [".*"] I was able to change things like the PCIe root port and other items. However, when I rebooted my computer the kata-deploy script ran again and overwrote my installation and reset the configuration-qemu.toml back to its original. That means that my pod couldn't start back up after rebooting.

Failed to create pod sandbox: rpc error: code = Unknown desc = CreateContainer failed: annotation io.katacontainers.config.hypervisor.kernel_params is not enabled: unknown

Probably this is a bug with kata-deploy to not install over itself, but it is the annotations that really makes this issue manifest.