From 1f7ad0d7dff035464ed914b9a7b78adba9a48d8d Mon Sep 17 00:00:00 2001 From: Wangchong Zhou Date: Thu, 8 Nov 2018 14:46:21 -0800 Subject: [PATCH] fix pluginDeepEqual when integer value is decoded as float64 From #191 --- internal/ingress/controller/kong.go | 64 ++++++++++++-- internal/ingress/controller/kong_test.go | 103 ++++++++++++++++++++++- 2 files changed, 159 insertions(+), 8 deletions(-) diff --git a/internal/ingress/controller/kong.go b/internal/ingress/controller/kong.go index fdb87ba872..2eef562aba 100644 --- a/internal/ingress/controller/kong.go +++ b/internal/ingress/controller/kong.go @@ -18,18 +18,18 @@ package controller import ( "bytes" + "encoding/json" "fmt" "net" "reflect" "sort" "time" - "github.com/imdario/mergo" - "github.com/fatih/structs" "github.com/golang/glog" "github.com/hbagdi/go-kong/kong" "github.com/hbagdi/go-kong/kong/custom" + "github.com/imdario/mergo" configurationv1 "github.com/kong/kubernetes-ingress-controller/internal/apis/configuration/v1" pluginv1 "github.com/kong/kubernetes-ingress-controller/internal/apis/plugin/v1" "github.com/kong/kubernetes-ingress-controller/internal/ingress" @@ -1254,17 +1254,69 @@ func deleteServiceUpstream(host string, client *kong.Client) error { // the persisted state in the Kong database // This is required because a plugin has defaults that could not exists in the CRD. func pluginDeepEqual(config map[string]interface{}, kong *kong.Plugin) bool { - for k, v := range config { - kv, ok := kong.Config[k] - if !ok { + return pluginDeepEqualWrapper(config, kong.Config) +} + +func pluginDeepEqualWrapper(config1 map[string]interface{}, config2 map[string]interface{}) bool { + return interfaceDeepEqual(config1, config2) +} + +func interfaceDeepEqual(i1 interface{}, i2 interface{}) bool { + v1 := reflect.ValueOf(i1) + v2 := reflect.ValueOf(i2) + + k1 := v1.Type().Kind() + k2 := v2.Type().Kind() + if k1 == k2 { + if k1 == reflect.Map { + return mapDeepEqual(v1, v2) + } else if k1 == reflect.Slice || k1 == reflect.Array { + return listUnorderedDeepEqual(v1, v2) + } + } + j1, e1 := json.Marshal(v1.Interface()) + j2, e2 := json.Marshal(v2.Interface()) + return e1 == nil && e2 == nil && string(j1) == string(j2) +} + +func mapDeepEqual(m1 reflect.Value, m2 reflect.Value) bool { + keys1 := m1.MapKeys() + for _, k := range keys1 { + v2 := m2.MapIndex(k) + if !v2.IsValid() { // k not found in m2 return false } + v1 := m1.MapIndex(k) - if !reflect.DeepEqual(v, kv) { + if v1.IsValid() && !interfaceDeepEqual(v1.Interface(), v2.Interface()) { return false } } + return true +} +func listUnorderedDeepEqual(l1 reflect.Value, l2 reflect.Value) bool { + length := l1.Len() + if length != l2.Len() { + return false + } + for i := 0; i < length; i++ { + v1 := l1.Index(i) + if !v1.IsValid() { + return false // this shouldn't happen + } + found := false + for j := 0; j < length; j++ { + v2 := l2.Index(j) + if v2.IsValid() && interfaceDeepEqual(v1.Interface(), v2.Interface()) { + found = true + break + } + } + if !found { + return false + } + } return true } diff --git a/internal/ingress/controller/kong_test.go b/internal/ingress/controller/kong_test.go index adbe2aabf5..6388f76467 100644 --- a/internal/ingress/controller/kong_test.go +++ b/internal/ingress/controller/kong_test.go @@ -73,6 +73,19 @@ func TestPluginDeepEqual(t *testing.T) { t.Errorf("Comparing maps with nested map failed") } + equal = pluginDeepEqual(map[string]interface{}{ + "key1": 1, + "key2": 2, + "key3": 8, + }, &kong.Plugin{Config: map[string]interface{}{ + "key1": 1.0, + "key2": 2.0, + "key3": 8, + }}) + if !equal { + t.Errorf("Comparing maps with numeric values in different type failed") + } + equal = pluginDeepEqual(map[string]interface{}{ "key1": map[string]string{}, "key2": "value2", @@ -116,8 +129,42 @@ func TestPluginDeepEqual(t *testing.T) { "arr2", "arr3", "arr1", }, }}) - if equal { - t.Errorf("Comparing maps with nested array with different order failed") + if !equal { + t.Errorf("Comparing maps with nested string array with different order failed") + } + + equal = pluginDeepEqual(map[string]interface{}{ + "key1": [3]int{ + 1, 2, 3, + }, + "key2": "value2", + "key3": "value3", + }, &kong.Plugin{Config: map[string]interface{}{ + "key2": "value2", + "key3": "value3", + "key1": [3]float64{ + 1.0, 2.0, 3.0, + }, + }}) + if !equal { + t.Errorf("Comparing maps with nested numeric value array failed") + } + + equal = pluginDeepEqual(map[string]interface{}{ + "key1": []interface{}{ + 1, map[string]string{"1": "2"}, "3", + }, + "key2": "value2", + "key3": "value3", + }, &kong.Plugin{Config: map[string]interface{}{ + "key2": "value2", + "key3": "value3", + "key1": []interface{}{ + 1.0, "3", map[string]string{"1": "2"}, + }, + }}) + if !equal { + t.Errorf("Comparing maps with interface value array failed") } equal = pluginDeepEqual(map[string]interface{}{ @@ -165,4 +212,56 @@ func TestPluginDeepEqual(t *testing.T) { if equal { t.Errorf("Comparing maps with unmatched keys failed") } + + equal = pluginDeepEqual(map[string]interface{}{ + "key1": map[string]string{ + "nestedkey1": "nestedvalue1", + "nestedkey2": "nestedvalue2", + }, + "key2": "value2", + "key3": "value3", + }, &kong.Plugin{Config: map[string]interface{}{ + "key3": "value3", + "key1": map[string]string{ + "nestedkey1": "nestedvalue1", + "nestedkey2": "nestedvalue3", + }, + "key2": "value2", + }}) + if equal { + t.Errorf("Comparing maps with unmatched keys in nested structure failed") + } + + equal = pluginDeepEqual(map[string]interface{}{ + "key2": "value2", + "key1": map[string]string{ + "nestedkey1": "nestedvalue1", + }, + }, &kong.Plugin{Config: map[string]interface{}{ + "key2": "value2", + "key1": map[string]string{ + "nestedkey1": "nestedvalue1", + }, + "default3": "value3", + }}) + if !equal { + t.Errorf("Comparing maps with default configs failed") + } + + equal = pluginDeepEqual(map[string]interface{}{ + "key2": "value2", + "key1": map[string]string{ + "nestedkey1": "nestedvalue1", + }, + }, &kong.Plugin{Config: map[string]interface{}{ + "key2": "value2", + "key1": map[string]string{ + "nestedkey1": "nestedvalue1", + "defaultnested2": "nestedvalue2", + }, + "default3": "value3", + }}) + if !equal { + t.Errorf("Comparing maps with nested default configs failed") + } }