Add sysctl allowList

[mmirecki]

The tuning-cni allows to set any sysctl on the created container. The admin might however want to restrict the user to a predefined whitelist of sysctls.
This PR is a POC with a proposal on how this could be implemented.

Tuning-cni would try to read a whitelist file from /tuningwhitelist/sysctlwhitelist (NOTE: temporary location only), and check the incoming sysctls agains it. Cni would return error if an unathorized sysctl is detected.
The tuning-cni would behaves as before is the file is missing.

The POC is meant to only show the concept, the actual implementation will probably differ.

[s1061123]

Hi @mmirecki It seems to be good feature for tuning/sysctl, especially Kubernetes with multus.

• Regarding whitelist/blacklist config file, current PR assumes that the same path of tuning however, I suppose user want to put it in different path from executable binary.
• The path of whitelist/blacklist could be configurable, I suppose. Of course, we could have default path for that, e.g. /etc/cni/tuning/whitelist.conf or some.
• How about to use regexp for the sysctl list, instead of simple list of sysctl?

[mmirecki]

Hi @mmirecki It seems to be good feature for tuning/sysctl, especially Kubernetes with multus.

• Regarding whitelist/blacklist config file, current PR assumes that the same path of tuning however, I suppose user want to put it in different path from executable binary.

/etc/cni/tuning/whitelist.conf is not the default

• The path of whitelist/blacklist could be configurable, I suppose. Of course, we could have default path for that, e.g. /etc/cni/tuning/whitelist.conf or some.

I've set this as the default one. I'll add an override when suggestions appear.

• How about to use regexp for the sysctl list, instead of simple list of sysctl?

Done

[s1061123]

As far as I looked, the code seems good to me. I recommend unit-test for the code, in https://github.com/containernetworking/plugins/blob/master/plugins/meta/tuning/tuning_test.go

[mmirecki]

As far as I looked, the code seems good to me. I recommend unit-test for the code, in https://github.com/containernetworking/plugins/blob/master/plugins/meta/tuning/tuning_test.go

@s1061123 , added tests, replied to other comments
Thanks

[cgoncalves]

Instead of whitelist, consider allow list, permit list or other variations.

[dcbw]

@mmirecki I agree with @cgoncalves ; would you mind whitelist -> allowlist?

[mmirecki]

@mmirecki I agree with @cgoncalves ; would you mind whitelist -> allowlist?

-> allowlist

[s1061123]

It looks good to me. Thank you for the changes!

[dcbw]

/lgtm

[squeed]

Looks good to me. When you have a moment, can you file a PR in https://github.com/containernetworking/cni.dev/ documenting this? Thanks

[mmirecki]

Looks good to me. When you have a moment, can you file a PR in https://github.com/containernetworking/cni.dev/ documenting this? Thanks

@squeed Here's the doc pr: containernetworking/cni.dev#99