Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: VALIDATE Operation #1132

Open
LionelJouin opened this issue Oct 30, 2024 · 2 comments
Open

Proposal: VALIDATE Operation #1132

LionelJouin opened this issue Oct 30, 2024 · 2 comments

Comments

@LionelJouin
Copy link
Member

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:

  • CNI config: CNI config format is correct (done by cnilib).
  • Runtime config: The interface name, arguments, and capabilities data are correct (done by cnilib?).
  • Plugin specific data: Each plugin would be responsible for validating its specific fields (ipam: submets ; vlan: vlan ID...).

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:

  • The vlan ID is within 0 and 4095.
  • The master interface name is a correct Linux interface name.
  • The host-local IPAM plugin checks that the subnet format is valid (CIDR Format).

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
Copy link
Contributor

@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:

  • Performance / Scalability
  • More validation requirements (i.e. a case that needs something in K8s to validate)

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:

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

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
Copy link
Contributor

bleggett commented Nov 4, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants