containernetworking / cni

Container Network Interface - networking for Linux containers
https://cni.dev
Apache License 2.0
5.59k stars 1.08k forks source link

Proposal: VALIDATE Operation #1132

Open LionelJouin opened 1 month ago

LionelJouin commented 1 month ago

Overview

This proposal introduces a new VALIDATE operation which could be helpful in Multus-like projects.

The VALIDATE operation would accept the same input as the ADD operation and perform a validation of it. This would be verifying:

As of now, ValidateNetworkList/ValidateNetwork exits, but checks only if all the specified plugins exist on disk and if every plugin supports the desired version. This function could be extended with the VALIDATE operation.

Example

In the following configuration:

{
  "cniVersion": "1.0.0",
  "name": "vlan-eth0",
  "plugins": [
    {
      "type": "vlan",
      "master": "eth0",
      "vlanId": 2000,
      "ipam": {
        "type": "host-local",
        "ranges": [
          [
            {
              "subnet": "10.10.1.0/24"
            }
          ]
        ]
      }
    }
  ]
}

The VALIDATE operation would ensure that:

VALIDATE is intended for configuration verification only, so the plugin should not check if, for example, the master interface eth0 exists or not.

Benefits

The VALIDATE operation would allow Multus-like projects to create enhanced Kubernetes validation-webhook and prevent the creation of object (e.g. NetworkAttachementDefinition) which would never pass the configuration stage (as they are valid), thus, reducing the likelihood of errors during the actual ADD stage.

s1061123 commented 1 month ago

@LionelJouin , thank you for the proposal. I love to have validate functionality in CNI, but I l'm also wondering should we implement it as a part of 'Verb' (i.e. new commands into CNI plugin).

I suppose we should think how to implmenet based on following topics, at least:

In addition, as you mentioned above, it does not have to be tied into container runtime (i.e. could implement in validation-webhook), there should be several way to implement, such as:

From my point of view, I suppose we should implement out of 'plugin' go code (even in 'containernetworking/pluign' repo). So let's discuss about it in weekly call.

bleggett commented 4 weeks ago

VALIDATE is intended for configuration verification only, so the plugin should not check if, for example, the master interface eth0 exists or not.

If this is only really checking static config, and doing zero runtime validation, are we just reinventing JSON-Schema for plugin values?

I.E. should we just allow plugins to declare a JSON schema, rather than add a new verb?

EDIT: Yeah Tomo mentioned this already:

Additional JSON Schema component (i.e. create another directory called in '/etc/cni/schemas')

I'd at least like to investigate why we shouldn't do something like that (it may turn out that there are good reasons to add a verb and not use something more standard, but it's not clear ATM to me at least what those are).

We could let plugins inline a schema, we could have a field that points to a schema, we could have a by-convention directory with schemas sharing names, etc etc.