search

bluebosh / helm-update-config

Helm plugin that allows to update config values of existing release.
MIT License
0 stars 3 forks source link

update-config -f yaml --set-value returned "panic" when the same multi-layer value is set #3

Open EmilyEmily opened 5 years ago

EmilyEmily commented 5 years ago

This seems a bug from go package strvals. To simplify the issue, I wrote test code for this package: 

import  "k8s.io/helm/pkg/strvals"

    m1 := make(map[string]interface{}) 
    m2 := make (map[string]map[string]int)

    m2["key2"]["key1"] = 1
    m2["key2"]["key12"] = 12
    m1["m2"] = m2
    m1["age"] = "12"
        m1["name"] = "simon"
    testStr := string("m2.key2.key1=200")

    fmt.Println("testStr:",testStr)
    fmt.Println("map1:", m1)

    strvals.ParseInto(testStr,m1)
    fmt.Println("after parse:", m1)

Output is

testStr: m2.key2.key1=200
map1: map[age:12 name:simon m2:map[key2:map[key1:1 key12:12]]]
panic: interface conversion: interface {} is map[string]map[string]int, not map[string]interface {}

goroutine 1 [running]:
k8s.io/helm/pkg/strvals.(*parser).key(0xc0000b2860, 0xc00009b950, 0x120917d, 0x14568a0)
    /Users/liujing/go/src/k8s.io/helm/pkg/strvals/parser.go:211 +0xedf
k8s.io/helm/pkg/strvals.(*parser).parse(0xc0000b2860, 0xc00009b950, 0x0)
    /Users/liujing/go/src/k8s.io/helm/pkg/strvals/parser.go:133 +0x38
k8s.io/helm/pkg/strvals.ParseInto(0x149e4ca, 0x10, 0xc00009b950, 0x4a, 0x0)
    /Users/liujing/go/src/k8s.io/helm/pkg/strvals/parser.go:85 +0xbe
cobratest/cmd.funcMapInterface()
    /Users/liujing/go/src/cobratest/cmd/hello.go:416 +0x9b9
cobratest/cmd.glob..func2(0x17f1e00, 0x1817fa8, 0x0, 0x0)
    /Users/liujing/go/src/cobratest/cmd/hello.go:52 +0x62
github.com/spf13/cobra.(*Command).execute(0x17f1e00, 0x1817fa8, 0x0, 0x0, 0x17f1e00, 0x1817fa8)
    /Users/liujing/go/src/github.com/spf13/cobra/command.go:766 +0x2cc
github.com/spf13/cobra.(*Command).ExecuteC(0x17f2060, 0xc000131f88, 0x1007220, 0xc000086058)
    /Users/liujing/go/src/github.com/spf13/cobra/command.go:852 +0x2fd
github.com/spf13/cobra.(*Command).Execute(0x17f2060, 0x0, 0x0)
    /Users/liujing/go/src/github.com/spf13/cobra/command.go:800 +0x2b
cobratest/cmd.Execute()
    /Users/liujing/go/src/cobratest/cmd/root.go:66 +0x2d
main.main()
    /Users/liujing/go/src/cobratest/main.go:20 +0x20
exit status 2
EmilyEmily commented 5 years ago

On the other hand, if teststr value has different keys from the map, then there is no error: 

    testStr := string("m2new.key2.key1=200")

Output is:

testStr: m2new.key2.key1=200
map1: map[addr:China age:12 name:simon m2:map[key2:map[key1:1 key12:12]]]
after parse: map[addr:China age:12 name:simon m2:map[key2:map[key1:1 key12:12]] m2new:map[key2:map[key1:200]]]
EmilyEmily commented 5 years ago

My third test is to verify that if the testStr has simple key/value pair but not multi-layer key, then strvals.ParseInto(testStr,m1) will merge the testStr and m1 together without error: 

    testStr := string("age=300,name=newname")

Output is:

testStr: age=300, name=newname
map1: map[age:12 name:simon m2:map[key2:map[key1:1 key12:12]]]
after parse: map[age:300 name:newname m2:map[key2:map[key1:1 key12:12]]  ]
EmilyEmily commented 5 years ago

In one word, from above test, strvals.ParseInto() can't merge multi-layer key/value from the first parameter string to the second parameter map.

Since bcf deployer doesn't rely on this function, we can set this issue to low priority.