cmd: subnet-tags-filter: k=v (and other options) silently fail when provided via ConfigMap

[gandro]

When providing the flags like subnet-tags-filter via ConfigMap, we have assumed and documented that these flags (relying on GetStringMapString) can be populated via a k=v syntax. However, it turns out that when specified via ConfigMap, it needs to be JSON syntax ({"k":"v"}), and not the k=v syntax as documented. The latter syntax only works if directly passed via command-line argument.

As a result, using subnet-tags-filter: foo=bar in the ConfigMap will cause cilium-operator to read the flag as nil and not use any subnet filters at all

Affected flags:

pkg/option/config.go
2771:	if m := viper.GetStringMapString(FixedIdentityMapping); len(m) != 0 {
2777:	if m := viper.GetStringMapString(KVStoreOpt); len(m) != 0 {
2781:	if m := viper.GetStringMapString(LogOpt); len(m) != 0 {
2785:	if m := viper.GetStringMapString(APIRateLimitName); len(m) != 0 {

operator/option/config.go
449:	if m := viper.GetStringMapString(IPAMSubnetsTags); len(m) != 0 {
453:	if m := viper.GetStringMapString(AWSInstanceLimitMapping); len(m) != 0 {
457:	if m := viper.GetStringMapString(ENITags); len(m) != 0 {

Upstream issue: spf13/viper#911

We should replace viper.GetStringMapString by something which behaves the same regardless of wheater the flag has been provided via command-line argument or config-dir (i.e. ConfigMap) entry.

[sayboras]

Let me give it a crack

[sayboras]

Just want to confirm the current behaviour.

We are having below validation, which basically will check the flag value. And for the flag, only k=v format is accepted. So is either format not working at all right now ?

• json format got rejected by below validation
• kv format failed to parse due to upstream issue in viper.

cilium/pkg/option/config.go

Line 3351 in d05dcb5

if err := flag.Value.Set(val); err != nil {

json format

$ cat tmp/subnet-tags-filter 
{"k1":"v1", "k2":"v2"}% 

$ /usr/lib/go-1.17/bin/go build -o /tmp/GoLand/___2go_build_github_com_cilium_cilium_operator github.com/cilium/cilium/operator #gosetup
/tmp/GoLand/___2go_build_github_com_cilium_cilium_operator --config-dir tmp/
level=fatal msg="Incorrect config-map flag subnet-tags-filter" error="{\"k1\":\"v1\", \"k2\":\"v2\"} must be formatted as key=value" subsys=config

kv format

$ cat tmp/subnet-tags-filter
k1=v1,k2=v2% 

/usr/lib/go-1.17/bin/go build -o /tmp/GoLand/___2go_build_github_com_cilium_cilium_operator github.com/cilium/cilium/operator #gosetup
/tmp/GoLand/___2go_build_github_com_cilium_cilium_operator --config-dir tmp/
level=info msg="Started gops server" address="127.0.0.1:9891" subsys=___2go_build_github_com_cilium_cilium_operator
level=warning msg="Running Cilium with \"kvstore\"=\"\" requires identity allocation via CRDs. Changing identity-allocation-mode to \"crd\"" subsys=config
...
level=info msg="  --skip-crd-creation='false'" subsys=___2go_build_github_com_cilium_cilium_operator
level=info msg="  --subnet-ids-filter=''" subsys=___2go_build_github_com_cilium_cilium_operator
level=info msg="  --subnet-ids-filter=''" subsys=___2go_build_github_com_cilium_cilium_operator --> this is empty
level=info msg="  --synchronize-k8s-nodes='true'" subsys=___2go_build_github_com_cilium_cilium_operator
...
level=info msg="Cilium Operator  go version go1.17 linux/amd64" subsys=___2go_build_github_com_cilium_cilium_operator

[gandro]

@sayboras Yes, that looks consistent with my observations. Note that in Cilium v1.10 the former (JSON in ConfigMap) seems to be accepted, but this changed in v1.11.