search

k8snetworkplumbingwg / net-attach-def-admission-controller

An admission controller to check resources as defined by the NPWG spec
Apache License 2.0
4 stars 15 forks source link

Change 'isolate' validation as optional in deployment #54

Closed opstalj closed 1 year ago

opstalj commented 2 years ago

(original subject: networks annotations must not refer to namespaced values?) @s1061123 updated: As of the thread conversation, we clarified that we can improve as https://github.com/k8snetworkplumbingwg/net-attach-def-admission-controller/issues/54#issuecomment-1282577843

The net-attach-def-admission-controller is rejecting the creation of our replica set due to the following:

65s Warning FailedCreate replicaset/loam-b-v1-54b6459f86 Error creating: admission webhook "net-attach-def-admission-controller-isolating-config.k8s.io" denied the request: k8s.v1.cni.cncf.io/networks annotations must not refer to namespaced values (must use local namespace, i.e. must not contain a /), rejected: my-ns/loam-b-mgmt (namespace: my-ns)

Looking at other implementations (eg: multus-cni), it is allowed to have a namespace in the network annotation. So why is the net-attach-def-admission-controller rejecting it?

s1061123 commented 2 years ago

Thank you for the report. Could you share your rejected yaml file to take a look into the issue?

opstalj commented 2 years ago

Hi, Thanks for your fast reply. However, I can't share the yaml as it is being used on a customer's platform. The part which results in the rejection is the following:

Annotations: k8s.v1.cni.cncf.io/networks: my-ns/nrd-oam

The net-attach-def-admission-controller does not like the namespace in the annotation. As soon as we edit the replica set and change the annotation as follows:

Annotations: k8s.v1.cni.cncf.io/networks: nrd-oam

then everything works fine.

To me, it looks like the net-attach-def-admission-controller is a bit too strict in its checking... Regards.

opstalj commented 2 years ago

Hi, Could it be that you should not refer to a namespace in the annotation if that namespace is in fact your own local namespace? (so only refer to a namespace if the resource is in a different namespace than where you are deploying) Regs.

s1061123 commented 2 years ago

Hi, I understood that the issue is happen but now I'm look for feasible fix for that because this namespace mechanism is also related to multus feature, 'namespace isolation', too.

So let me think how to fix (and how makes the fix feasible to the various users, too).

s1061123 commented 2 years ago

Hi @opstalj

So I understand the root cause of the issue. Currently admission controller have two verification, validate for net-attach-def and isolate for pod annotation.

validate is to validate a net-attach-def's CNI json config (just json syntax check), and isolate is to check a pod network annotation is aligned to 'namespaceIsolation' config (see https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/configuration.md for the details).

Your warning message mention that your pod annotation is not aligned with 'namespaceIsolation' config of multus.

So if you don't use 'namespaceIsolation' then, this validation can be invalidated because this validation is for 'namespaceIsonation'. You can remove isolation validation by following kubectl command.

#  kubectl delete validatingwebhookconfigurations.admissionregistration.k8s.io net-attach-def-admission-controller-isolating-config

So that could be a fix for your issue. I guess. But I also feels that we should

If you don't mind it, I will change the title of the issue and use this issue to track these TODO.

opstalj commented 2 years ago

Hi, Thanks for your investigation! Yes please, you can set the issue to TODO. Kind regards.

opstalj commented 2 years ago

Sorry, looking once more to the warning message: it mentions:

k8s.v1.cni.cncf.io/networks annotations must not refer to namespaced values (must use local namespace, i.e. must not contain a /)

which is quite strange, as https://github.com/k8snetworkplumbingwg/multus-cni/blob/master/docs/configuration.md shows examples how to set the networks annotation, eg:

annotations: k8s.v1.cni.cncf.io/networks: privileged/macvlan-conf

where privileged is the namespace.

So why would that be OK and not:

Annotations: k8s.v1.cni.cncf.io/networks: my-ns/nrd-oam

where my-ns is the namespace?

-> could this be because my-ns is the local namespace for the resource and should not be referenced at all here?

Kind regards.

s1061123 commented 2 years ago

This webhook and error message is only for 'namespaceIsolation' feature of multus. This feature limits user to configure net-attach-def annotation in pod to specific namespace (e.g. pod's namespace).

As you told, multus-cni without 'namespaceIsolation' can configure any namespace for net-attach-def annotation in pod. On the otherside, however, sometimes Kubernetes administrator want to set the limits due to the security reason. 'namespaceIsolation' is for the purpose.

Currently default deployment script of net-attach-def-admission-controller expects that user configures 'namespaceIsolation' in multus and do verification for that. That is the background.

opstalj commented 2 years ago

Ok, understood! Thanks.