k8snetworkplumbingwg / multus-cni

A CNI meta-plugin for multi-homed pods in Kubernetes
Apache License 2.0
2.36k stars 585 forks source link

Change Validation of interface name #1243

Closed adrianchiris closed 4 months ago

adrianchiris commented 6 months ago

interface name should not be limited to DNS-1123 label format. instead validate interface name if provided in pod network annotation in a similar manner as iproute2[1].

this will allow to request interface names such as: "uplink_p0"

[1]https://github.com/iproute2/iproute2/blob/11740815bfe69d6ee2cad7c608a8edc70147209a/lib/utils.c#L832

coveralls commented 6 months ago

Coverage Status

coverage: 62.829% (+0.05%) from 62.778% when pulling d625d482311f591afee8d7aef9008419e8c640aa on adrianchiris:allow_undersocre_in_ifname into 0fd3fa79191f9e570df74568cf0ea61b2cd12820 on k8snetworkplumbingwg:master.

dougbtv commented 6 months ago

Hey Adrian -- this one is bigger than a breadbox :) Would you mind joining the NPWG or Multus maintainer's call to chat us about it?

I think there's some subtlety here between "attachment name" and "interface name" and the nuance there in the spec as well, we should chat it out. Thanks!

adrianchiris commented 6 months ago

sure, will raise it in one of the meetings. ATM on PTO (paternity leave) so will bring it up once im back. (~3 weeks )

dougbtv commented 5 months ago

I think my readthrough was wrong, and that this is totally reasonable given teh spec:

4.1.2.1.5 "interface" This optional key with value of type string requires the implementation to use the given name for the pod interface resulting from this network attachment. This key’s value must be a valid Linux kernel interface name.

killianmuldoon commented 5 months ago

@Eoghan1232 - wondering if I could get your eyes on this one

Eoghan1232 commented 5 months ago

lgtm - apologies I have been away for some time and only back now.

killianmuldoon commented 4 months ago

@adrianchiris @dougbtv WDYT about merging this one?

dougbtv commented 4 months ago

thanks for the submission, after considerable discussion: we couldn't find a reason why underscores would be invalid