From 5b03a4142c6449edbf3383dbf32332e37f41b7da Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 13 Mar 2019 15:41:38 +0100 Subject: [PATCH 1/7] Enable/disable dependencies tab based on spec props Signed-off-by: Pavol Loffay --- pkg/strategy/controller.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/pkg/strategy/controller.go b/pkg/strategy/controller.go index fc2f040e4..a783fe972 100644 --- a/pkg/strategy/controller.go +++ b/pkg/strategy/controller.go @@ -135,21 +135,25 @@ func normalizeRollover(spec *v1.JaegerEsRolloverSpec) { } func normalizeUI(spec *v1.JaegerSpec) { - sOpts := spec.Storage.Options.Map() uiOpts := map[string]interface{}{} if !spec.UI.Options.IsEmpty() { if m, err := spec.UI.Options.GetMap(); err == nil { uiOpts = m } } + sOpts := spec.Storage.Options.Map() // we respect explicit UI config - if _, ok := uiOpts["archiveEnabled"]; ok { - return + if _, ok := uiOpts["archiveEnabled"]; !ok { + if strings.EqualFold(sOpts["es-archive.enabled"], "true") || + strings.EqualFold(sOpts["cassandra-archive.enabled"], "true") { + uiOpts["archiveEnabled"] = true + } } - if strings.EqualFold(sOpts["es-archive.enabled"], "true") || - strings.EqualFold(sOpts["cassandra-archive.enabled"], "true") { - uiOpts["archiveEnabled"] = true + + if _, ok := uiOpts["dependencies"]; !ok { + } + if len(uiOpts) > 0 { spec.UI.Options = v1.NewFreeForm(uiOpts) } From 9c9545849d0f584d660064ed5ae3a11b5bc09f3b Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 13 Mar 2019 18:11:46 +0100 Subject: [PATCH 2/7] Disable dependencies tab Signed-off-by: Pavol Loffay --- pkg/strategy/controller.go | 35 ++++++++++--- pkg/strategy/controller_test.go | 90 ++++++++++++++++++++++++++------- pkg/strategy/production_test.go | 3 +- pkg/strategy/streaming_test.go | 3 +- 4 files changed, 105 insertions(+), 26 deletions(-) diff --git a/pkg/strategy/controller.go b/pkg/strategy/controller.go index a783fe972..647808a90 100644 --- a/pkg/strategy/controller.go +++ b/pkg/strategy/controller.go @@ -141,21 +141,42 @@ func normalizeUI(spec *v1.JaegerSpec) { uiOpts = m } } - sOpts := spec.Storage.Options.Map() - // we respect explicit UI config + enableArchiveButton(uiOpts, spec.Storage.Options.Map()) + disableDependenciesTab(uiOpts, spec.Storage.Type, spec.Storage.SparkDependencies.Enabled) + if len(uiOpts) > 0 { + spec.UI.Options = v1.NewFreeForm(uiOpts) + } +} + +func enableArchiveButton(uiOpts map[string]interface{}, sOpts map[string]string) { + // respect explicit settings if _, ok := uiOpts["archiveEnabled"]; !ok { + // archive tab is by default disabled if strings.EqualFold(sOpts["es-archive.enabled"], "true") || strings.EqualFold(sOpts["cassandra-archive.enabled"], "true") { uiOpts["archiveEnabled"] = true } } +} - if _, ok := uiOpts["dependencies"]; !ok { - +func disableDependenciesTab(uiOpts map[string]interface{}, storage string, depsEnabled *bool) { + // dependency tab is by default enabled and memory storage support it + if strings.EqualFold(storage, "memory") || (depsEnabled != nil && *depsEnabled == false) { + return + } + deps := map[string]interface{}{} + if val, ok := uiOpts["dependencies"]; ok { + if val, ok := val.(map[string]interface{}); ok { + deps = val + } else { + // we return as the type does not match + return + } } - - if len(uiOpts) > 0 { - spec.UI.Options = v1.NewFreeForm(uiOpts) + // respect explicit settings + if _, ok := deps["menuEnabled"]; !ok { + deps["menuEnabled"] = false + uiOpts["dependencies"] = deps } } diff --git a/pkg/strategy/controller_test.go b/pkg/strategy/controller_test.go index 64e6af12f..b5358f720 100644 --- a/pkg/strategy/controller_test.go +++ b/pkg/strategy/controller_test.go @@ -246,38 +246,94 @@ func TestNormalizeUI(t *testing.T) { }{ { j: &v1.JaegerSpec{}, - expected: &v1.JaegerSpec{}, + expected: &v1.JaegerSpec{UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"dependencies": map[string]interface{}{"menuEnabled": false}})}}, }, { - j: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es.archive.enabled": "false"})}}, - expected: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es.archive.enabled": "false"})}}, + j: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "memory"}}, + expected: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "memory"}}, }, { j: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}}, expected: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}, - UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"archiveEnabled": true})}}, + UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"archiveEnabled": true, "dependencies": map[string]interface{}{"menuEnabled": false}})}}, }, + } + for _, test := range tests { + normalizeUI(test.j) + assert.Equal(t, test.expected, test.j) + } +} + +func TestNormalizeUIArchiveButton(t *testing.T) { + tests := []struct { + uiOpts map[string]interface{} + sOpts map[string]string + expected map[string]interface{} + }{ + {}, { - j: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"cassandra-archive.enabled": "true"})}}, - expected: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"cassandra-archive.enabled": "true"})}, - UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"archiveEnabled": true})}}, + uiOpts: map[string]interface{}{}, + sOpts: map[string]string{"es-archive.enabled": "false"}, + expected: map[string]interface{}{}, }, { - j: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}, - UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"other": "foo"})}}, - expected: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}, - UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"other": "foo", "archiveEnabled": true})}}, + uiOpts: map[string]interface{}{}, + sOpts: map[string]string{"es-archive.enabled": "true"}, + expected: map[string]interface{}{"archiveEnabled": true}, }, { - j: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}, - UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"archiveEnabled": "respectThis"})}}, - expected: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}, - UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"archiveEnabled": "respectThis"})}}, + uiOpts: map[string]interface{}{}, + sOpts: map[string]string{"cassandra-archive.enabled": "true"}, + expected: map[string]interface{}{"archiveEnabled": true}, + }, + { + uiOpts: map[string]interface{}{"archiveEnabled": "respectThis"}, + sOpts: map[string]string{"es-archive.enabled": "true"}, + expected: map[string]interface{}{"archiveEnabled": "respectThis"}, }, } for _, test := range tests { - normalizeUI(test.j) - assert.Equal(t, test.expected, test.j) + enableArchiveButton(test.uiOpts, test.sOpts) + assert.Equal(t, test.expected, test.uiOpts) + } +} + +func TestNormalizeUIDependenciesTab(t *testing.T) { + tests := []struct { + uiOpts map[string]interface{} + storage string + enabled *bool + expected map[string]interface{} + }{ + { + uiOpts: map[string]interface{}{}, + storage: "memory", + expected: map[string]interface{}{}, + }, + { + uiOpts: map[string]interface{}{}, + storage: "whateverStorage", + expected: map[string]interface{}{"dependencies": map[string]interface{}{"menuEnabled": false}}, + }, + { + uiOpts: map[string]interface{}{"dependencies": "respectThis"}, + storage: "whateverStorage", + expected: map[string]interface{}{"dependencies": "respectThis"}, + }, + { + uiOpts: map[string]interface{}{"dependencies": map[string]interface{}{"menuEnabled": "respectThis"}}, + storage: "whateverStorage", + expected: map[string]interface{}{"dependencies": map[string]interface{}{"menuEnabled": "respectThis"}}, + }, + { + uiOpts: map[string]interface{}{"dependencies": map[string]interface{}{"foo": "bar"}}, + storage: "whateverStorage", + expected: map[string]interface{}{"dependencies": map[string]interface{}{"foo": "bar", "menuEnabled": false}}, + }, + } + for _, test := range tests { + disableDependenciesTab(test.uiOpts, test.storage, test.enabled) + assert.Equal(t, test.expected, test.uiOpts) } } diff --git a/pkg/strategy/production_test.go b/pkg/strategy/production_test.go index f9280c28b..2cd9b17ca 100644 --- a/pkg/strategy/production_test.go +++ b/pkg/strategy/production_test.go @@ -92,7 +92,8 @@ func TestOptionsArePassed(t *testing.T) { // Including parameter for sampling config assert.Len(t, args, 4) } else { - assert.Len(t, args, 3) + // including UI config + assert.Len(t, args, 4) } var escount int for _, arg := range args { diff --git a/pkg/strategy/streaming_test.go b/pkg/strategy/streaming_test.go index f7bb250cb..fdafb245c 100644 --- a/pkg/strategy/streaming_test.go +++ b/pkg/strategy/streaming_test.go @@ -111,7 +111,8 @@ func TestStreamingOptionsArePassed(t *testing.T) { } else { // Including parameters for ES only - assert.Len(t, args, 3) + // including UI config + assert.Len(t, args, 4) assert.Equal(t, 3, escount) } } From b7ae0aa5523fa4e385e0f4070c1a27b112919bf9 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 13 Mar 2019 19:19:58 +0100 Subject: [PATCH 3/7] Test false Signed-off-by: Pavol Loffay --- pkg/strategy/controller_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/strategy/controller_test.go b/pkg/strategy/controller_test.go index b5358f720..dfa7c744d 100644 --- a/pkg/strategy/controller_test.go +++ b/pkg/strategy/controller_test.go @@ -299,6 +299,7 @@ func TestNormalizeUIArchiveButton(t *testing.T) { } func TestNormalizeUIDependenciesTab(t *testing.T) { + falseVar := false tests := []struct { uiOpts map[string]interface{} storage string @@ -315,6 +316,12 @@ func TestNormalizeUIDependenciesTab(t *testing.T) { storage: "whateverStorage", expected: map[string]interface{}{"dependencies": map[string]interface{}{"menuEnabled": false}}, }, + { + uiOpts: map[string]interface{}{}, + storage: "whateverStorage", + enabled: &falseVar, + expected: map[string]interface{}{}, + }, { uiOpts: map[string]interface{}{"dependencies": "respectThis"}, storage: "whateverStorage", From ed3875ae7efe0116c31f1aaabd5d38ec2b04d3e5 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Wed, 13 Mar 2019 19:35:54 +0100 Subject: [PATCH 4/7] fix fmt Signed-off-by: Pavol Loffay --- pkg/strategy/controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/strategy/controller_test.go b/pkg/strategy/controller_test.go index dfa7c744d..641192ca1 100644 --- a/pkg/strategy/controller_test.go +++ b/pkg/strategy/controller_test.go @@ -319,7 +319,7 @@ func TestNormalizeUIDependenciesTab(t *testing.T) { { uiOpts: map[string]interface{}{}, storage: "whateverStorage", - enabled: &falseVar, + enabled: &falseVar, expected: map[string]interface{}{}, }, { From d8c17a1f4ef8d022b5aad52eb15a80d0a427dac2 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 14 Mar 2019 09:27:11 +0100 Subject: [PATCH 5/7] Fix condition Signed-off-by: Pavol Loffay --- pkg/strategy/controller.go | 2 +- pkg/strategy/controller_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/strategy/controller.go b/pkg/strategy/controller.go index 647808a90..fb40b3e1e 100644 --- a/pkg/strategy/controller.go +++ b/pkg/strategy/controller.go @@ -161,7 +161,7 @@ func enableArchiveButton(uiOpts map[string]interface{}, sOpts map[string]string) func disableDependenciesTab(uiOpts map[string]interface{}, storage string, depsEnabled *bool) { // dependency tab is by default enabled and memory storage support it - if strings.EqualFold(storage, "memory") || (depsEnabled != nil && *depsEnabled == false) { + if strings.EqualFold(storage, "memory") || (depsEnabled != nil && *depsEnabled == true) { return } deps := map[string]interface{}{} diff --git a/pkg/strategy/controller_test.go b/pkg/strategy/controller_test.go index 641192ca1..78032e204 100644 --- a/pkg/strategy/controller_test.go +++ b/pkg/strategy/controller_test.go @@ -320,7 +320,7 @@ func TestNormalizeUIDependenciesTab(t *testing.T) { uiOpts: map[string]interface{}{}, storage: "whateverStorage", enabled: &falseVar, - expected: map[string]interface{}{}, + expected: map[string]interface{}{"dependencies": map[string]interface{}{"menuEnabled": false}}, }, { uiOpts: map[string]interface{}{"dependencies": "respectThis"}, From 720a3d712ac65e888656df75cf4addf58a8416cb Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 14 Mar 2019 09:34:26 +0100 Subject: [PATCH 6/7] Fix tests Signed-off-by: Pavol Loffay --- pkg/strategy/production_test.go | 3 +-- pkg/strategy/streaming_test.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/strategy/production_test.go b/pkg/strategy/production_test.go index 2cd9b17ca..f9280c28b 100644 --- a/pkg/strategy/production_test.go +++ b/pkg/strategy/production_test.go @@ -92,8 +92,7 @@ func TestOptionsArePassed(t *testing.T) { // Including parameter for sampling config assert.Len(t, args, 4) } else { - // including UI config - assert.Len(t, args, 4) + assert.Len(t, args, 3) } var escount int for _, arg := range args { diff --git a/pkg/strategy/streaming_test.go b/pkg/strategy/streaming_test.go index fdafb245c..f7bb250cb 100644 --- a/pkg/strategy/streaming_test.go +++ b/pkg/strategy/streaming_test.go @@ -111,8 +111,7 @@ func TestStreamingOptionsArePassed(t *testing.T) { } else { // Including parameters for ES only - // including UI config - assert.Len(t, args, 4) + assert.Len(t, args, 3) assert.Equal(t, 3, escount) } } From 5c2bbba929f7f535c053aa3a605be9416076d509 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 14 Mar 2019 09:39:40 +0100 Subject: [PATCH 7/7] Add a comment Signed-off-by: Pavol Loffay --- pkg/strategy/controller.go | 1 + pkg/strategy/controller_test.go | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/pkg/strategy/controller.go b/pkg/strategy/controller.go index fb40b3e1e..fcd19a96d 100644 --- a/pkg/strategy/controller.go +++ b/pkg/strategy/controller.go @@ -78,6 +78,7 @@ func normalize(jaeger *v1.Jaeger) { jaeger.Spec.Ingress.Security = v1.IngressSecurityNoneExplicit } + // note that the order normalization matters - UI norm expects all normalized properties normalizeSparkDependencies(&jaeger.Spec.Storage.SparkDependencies, jaeger.Spec.Storage.Type) normalizeIndexCleaner(&jaeger.Spec.Storage.EsIndexCleaner, jaeger.Spec.Storage.Type) normalizeElasticsearch(&jaeger.Spec.Storage.Elasticsearch) diff --git a/pkg/strategy/controller_test.go b/pkg/strategy/controller_test.go index 78032e204..6eb5912b6 100644 --- a/pkg/strategy/controller_test.go +++ b/pkg/strategy/controller_test.go @@ -311,6 +311,12 @@ func TestNormalizeUIDependenciesTab(t *testing.T) { storage: "memory", expected: map[string]interface{}{}, }, + { + uiOpts: map[string]interface{}{}, + storage: "memory", + enabled: &falseVar, + expected: map[string]interface{}{}, + }, { uiOpts: map[string]interface{}{}, storage: "whateverStorage",