diff --git a/pkg/apis/jaegertracing/v1/options.go b/pkg/apis/jaegertracing/v1/options.go index a7355f79b..1631741ea 100644 --- a/pkg/apis/jaegertracing/v1/options.go +++ b/pkg/apis/jaegertracing/v1/options.go @@ -7,10 +7,28 @@ import ( "strings" ) +// Values hold a map, with string as the key and either a string or a slice of strings as the value +type Values map[string]interface{} + +// DeepCopy indicate how to do a deep copy of Values type +func (v *Values) DeepCopy() *Values { + out := make(Values, len(*v)) + for key, val := range *v { + switch val.(type) { + case string: + out[key] = val + + case []string: + out[key] = append([]string(nil), val.([]string)...) + } + } + return &out +} + // Options defines a common options parameter to the different structs type Options struct { - opts map[string]string `json:"-"` - json *[]byte `json:"-"` + opts Values `json:"-"` + json *[]byte `json:"-"` } // NewOptions build a new Options object based on the given map @@ -23,7 +41,7 @@ func NewOptions(o map[string]interface{}) Options { // Filter creates a new Options object with just the elements identified by the supplied prefix func (o *Options) Filter(prefix string) Options { options := Options{} - options.opts = make(map[string]string) + options.opts = make(map[string]interface{}) archivePrefix := prefix + "-archive." prefix += "." @@ -45,7 +63,6 @@ func (o *Options) UnmarshalJSON(b []byte) error { if err := d.Decode(&entries); err != nil { return err } - o.parse(entries) o.json = &b return nil @@ -69,18 +86,24 @@ func (o Options) MarshalJSON() ([]byte, error) { } func (o *Options) parse(entries map[string]interface{}) { - o.opts = make(map[string]string) + o.opts = make(map[string]interface{}) for k, v := range entries { o.opts = entry(o.opts, k, v) } } -func entry(entries map[string]string, key string, value interface{}) map[string]string { +func entry(entries map[string]interface{}, key string, value interface{}) map[string]interface{} { switch value.(type) { case map[string]interface{}: for k, v := range value.(map[string]interface{}) { entries = entry(entries, fmt.Sprintf("%s.%v", key, k), v) } + case []interface{}: + values := make([]string, 0, len(value.([]interface{}))) + for _, v := range value.([]interface{}) { + values = append(values, v.(string)) + } + entries[key] = values case interface{}: entries[key] = fmt.Sprintf("%v", value) } @@ -91,24 +114,41 @@ func entry(entries map[string]string, key string, value interface{}) map[string] // ToArgs converts the options to a value suitable for the Container.Args field func (o *Options) ToArgs() []string { if len(o.opts) > 0 { - i := 0 - args := make([]string, len(o.opts)) + args := make([]string, 0, len(o.opts)) for k, v := range o.opts { - args[i] = fmt.Sprintf("--%s=%v", k, v) - i++ + switch v.(type) { + case string: + args = append(args, fmt.Sprintf("--%s=%v", k, v)) + case []string: + for _, vv := range v.([]string) { + args = append(args, fmt.Sprintf("--%s=%v", k, vv)) + } + } } return args } - return nil } // Map returns a map representing the option entries. Items are flattened, with dots as separators. For instance // an option "cassandra" with a nested "servers" object becomes an entry with the key "cassandra.servers" -func (o *Options) Map() map[string]string { +func (o *Options) Map() map[string]interface{} { return o.opts } +// StringMap returns a map representing the option entries,excluding entries that have multiple values. +// Items are flattened, with dots as separators in the same way as Map does. +func (o *Options) StringMap() map[string]string { + smap := make(map[string]string) + for k, v := range o.opts { + switch v.(type) { + case string: + smap[k] = v.(string) + } + } + return smap +} + // GenericMap returns the map representing the option entries as interface{}, suitable for usage with NewOptions() func (o *Options) GenericMap() map[string]interface{} { out := make(map[string]interface{}) diff --git a/pkg/apis/jaegertracing/v1/options_test.go b/pkg/apis/jaegertracing/v1/options_test.go index fc74ba981..bf3f008f5 100644 --- a/pkg/apis/jaegertracing/v1/options_test.go +++ b/pkg/apis/jaegertracing/v1/options_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSimpleOption(t *testing.T) { @@ -148,3 +149,38 @@ func TestUpdate(t *testing.T) { // verify assert.Equal(t, o.opts["key"], "new") } + +func TestStringMap(t *testing.T) { + o := NewOptions(nil) + err := o.UnmarshalJSON([]byte(`{"firstsarg":"v1", "additional-headers":["whatever:thing", "access-control-allow-origin:blerg"]}`)) + require.NoError(t, err) + expected := map[string]string{"firstsarg": "v1"} + strMap := o.StringMap() + assert.Len(t, strMap, 1) + assert.Equal(t, expected, strMap) +} + +func TestDeepCopy(t *testing.T) { + o1 := NewOptions(nil) + err := o1.UnmarshalJSON([]byte(`{"firstsarg":"v1", "additional-headers":["whatever:thing", "access-control-allow-origin:blerg"]}`)) + require.NoError(t, err) + copy := o1.opts.DeepCopy() + + assert.Equal(t, copy, &(o1.opts)) +} + +func TestRepetitiveArguments(t *testing.T) { + o := NewOptions(nil) + err := o.UnmarshalJSON([]byte(`{"firstsarg":"v1", "additional-headers":["whatever:thing", "access-control-allow-origin:blerg"]}`)) + require.NoError(t, err) + expected := []string{"--additional-headers=access-control-allow-origin:blerg", "--additional-headers=whatever:thing", "--firstsarg=v1"} + + args := o.ToArgs() + sort.SliceStable(args, func(i, j int) bool { + return args[i] < args[j] + }) + + assert.Len(t, args, 3) + assert.Equal(t, expected, args) + +} diff --git a/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go b/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go index 89fa24435..97a3e8594 100644 --- a/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go +++ b/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go @@ -729,13 +729,7 @@ func (in *JaegerUISpec) DeepCopy() *JaegerUISpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Options) DeepCopyInto(out *Options) { *out = *in - if in.opts != nil { - in, out := &in.opts, &out.opts - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } + in.opts.DeepCopyInto(&out.opts) if in.json != nil { in, out := &in.json, &out.json *out = new([]byte) @@ -757,3 +751,13 @@ func (in *Options) DeepCopy() *Options { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in Values) DeepCopyInto(out *Values) { + { + in := &in + clone := in.DeepCopy() + *out = *clone + return + } +} diff --git a/pkg/cronjob/es_index_cleaner.go b/pkg/cronjob/es_index_cleaner.go index f902009b7..f46298412 100644 --- a/pkg/cronjob/es_index_cleaner.go +++ b/pkg/cronjob/es_index_cleaner.go @@ -28,7 +28,7 @@ func CreateEsIndexCleaner(jaeger *v1.Jaeger) *batchv1beta1.CronJob { envFromSource := util.CreateEnvsFromSecret(jaeger.Spec.Storage.SecretName) envs := EsScriptEnvVars(jaeger.Spec.Storage.Options) - if val, ok := jaeger.Spec.Storage.Options.Map()["es.use-aliases"]; ok && strings.EqualFold(val, "true") { + if val, ok := jaeger.Spec.Storage.Options.StringMap()["es.use-aliases"]; ok && strings.EqualFold(val, "true") { envs = append(envs, corev1.EnvVar{Name: "ROLLOVER", Value: "true"}) } diff --git a/pkg/cronjob/es_rollover.go b/pkg/cronjob/es_rollover.go index cb2a04e0a..95506db55 100644 --- a/pkg/cronjob/es_rollover.go +++ b/pkg/cronjob/es_rollover.go @@ -155,7 +155,7 @@ func EsScriptEnvVars(opts v1.Options) []corev1.EnvVar { {flag: "es.tls.key", envVar: "ES_TLS_KEY"}, {flag: "es.tls.skip-host-verify", envVar: "ES_TLS_SKIP_HOST_VERIFY"}, } - options := opts.Map() + options := opts.StringMap() var envs []corev1.EnvVar for _, x := range scriptEnvVars { if val, ok := options[x.flag]; ok { diff --git a/pkg/cronjob/spark_dependencies.go b/pkg/cronjob/spark_dependencies.go index 301d233bb..d333b1653 100644 --- a/pkg/cronjob/spark_dependencies.go +++ b/pkg/cronjob/spark_dependencies.go @@ -119,7 +119,7 @@ func CreateSparkDependencies(jaeger *v1.Jaeger) *batchv1beta1.CronJob { } func getStorageEnvs(s v1.JaegerStorageSpec) []corev1.EnvVar { - sFlagsMap := s.Options.Map() + sFlagsMap := s.Options.StringMap() switch s.Type { case v1.JaegerCassandraStorage: keyspace := sFlagsMap["cassandra.keyspace"] @@ -156,7 +156,7 @@ func getStorageEnvs(s v1.JaegerStorageSpec) []corev1.EnvVar { } func logTLSNotSupported(j *v1.Jaeger) { - sFlagsMap := j.Spec.Storage.Options.Map() + sFlagsMap := j.Spec.Storage.Options.StringMap() if strings.EqualFold(sFlagsMap["es.tls.enabled"], "true") || strings.EqualFold(sFlagsMap["es.tls"], "true") { j.Logger().Warn("Spark dependencies does not support TLS with Elasticsearch, consider disabling dependencies") } diff --git a/pkg/ingress/query.go b/pkg/ingress/query.go index 74e4c4d56..1da3a9a5a 100644 --- a/pkg/ingress/query.go +++ b/pkg/ingress/query.go @@ -77,9 +77,9 @@ func (i *QueryIngress) Get() *networkingv1.Ingress { func (i *QueryIngress) addRulesSpec(spec *networkingv1.IngressSpec, backend *networkingv1.IngressBackend) { path := "" - if allInOneQueryBasePath, ok := i.jaeger.Spec.AllInOne.Options.Map()["query.base-path"]; ok && i.jaeger.Spec.Strategy == v1.DeploymentStrategyAllInOne { + if allInOneQueryBasePath, ok := i.jaeger.Spec.AllInOne.Options.StringMap()["query.base-path"]; ok && i.jaeger.Spec.Strategy == v1.DeploymentStrategyAllInOne { path = allInOneQueryBasePath - } else if queryBasePath, ok := i.jaeger.Spec.Query.Options.Map()["query.base-path"]; ok && i.jaeger.Spec.Strategy == v1.DeploymentStrategyProduction { + } else if queryBasePath, ok := i.jaeger.Spec.Query.Options.StringMap()["query.base-path"]; ok && i.jaeger.Spec.Strategy == v1.DeploymentStrategyProduction { path = queryBasePath } diff --git a/pkg/service/collector.go b/pkg/service/collector.go index 146407b58..16e9511ac 100644 --- a/pkg/service/collector.go +++ b/pkg/service/collector.go @@ -107,7 +107,7 @@ func GetPortNameForGRPC(jaeger *v1.Jaeger) string { // perhaps the user has provisioned the certs and configured the CR manually? // for that, we check whether the CLI option `collector.grpc.tls.enabled` was set for the collector - if val, ok := jaeger.Spec.Collector.Options.Map()["collector.grpc.tls.enabled"]; ok { + if val, ok := jaeger.Spec.Collector.Options.StringMap()["collector.grpc.tls.enabled"]; ok { enabled, err := strconv.ParseBool(val) if err != nil { return "grpc-http" // not "true", defaults to false diff --git a/pkg/storage/cassandra_dependencies.go b/pkg/storage/cassandra_dependencies.go index f6ba129cc..688c9f37f 100644 --- a/pkg/storage/cassandra_dependencies.go +++ b/pkg/storage/cassandra_dependencies.go @@ -44,7 +44,7 @@ func cassandraDeps(jaeger *v1.Jaeger) []batchv1.Job { Value: jaeger.Spec.Storage.CassandraCreateSchema.Datacenter, }} - host := jaeger.Spec.Storage.Options.Map()["cassandra.servers"] + host := jaeger.Spec.Storage.Options.StringMap()["cassandra.servers"] if host == "" { jaeger.Logger().Info("Cassandra hostname not specified. Using 'cassandra' for the cassandra-create-schema job.") host = "cassandra" // this is the default in the image @@ -54,7 +54,7 @@ func cassandraDeps(jaeger *v1.Jaeger) []batchv1.Job { Value: host, }) - port := jaeger.Spec.Storage.Options.Map()["cassandra.port"] + port := jaeger.Spec.Storage.Options.StringMap()["cassandra.port"] if port == "" { jaeger.Logger().Info("Cassandra port not specified. Using '9042' for the cassandra-create-schema job.") port = "9042" // this is the default in the image @@ -64,7 +64,7 @@ func cassandraDeps(jaeger *v1.Jaeger) []batchv1.Job { Value: port, }) - keyspace := jaeger.Spec.Storage.Options.Map()["cassandra.keyspace"] + keyspace := jaeger.Spec.Storage.Options.StringMap()["cassandra.keyspace"] if keyspace == "" { jaeger.Logger().Info("Cassandra keyspace not specified. Using 'jaeger_v1_test' for the cassandra-create-schema job.") keyspace = "jaeger_v1_test" // this is default in the image @@ -74,16 +74,18 @@ func cassandraDeps(jaeger *v1.Jaeger) []batchv1.Job { Name: "KEYSPACE", Value: keyspace, }) + username := jaeger.Spec.Storage.Options.StringMap()["cassandra.username"] + password := jaeger.Spec.Storage.Options.StringMap()["cassandra.password"] envFromSource := util.CreateEnvsFromSecret(jaeger.Spec.Storage.SecretName) if len(envFromSource) == 0 { envVars = append(envVars, corev1.EnvVar{ Name: "CASSANDRA_USERNAME", - Value: jaeger.Spec.Storage.Options.Map()["cassandra.username"], + Value: username, }) envVars = append(envVars, corev1.EnvVar{ Name: "CASSANDRA_PASSWORD", - Value: jaeger.Spec.Storage.Options.Map()["cassandra.password"], + Value: password, }) } diff --git a/pkg/storage/elasticsearch_dependencies.go b/pkg/storage/elasticsearch_dependencies.go index 3e60cbc00..41fd4985d 100644 --- a/pkg/storage/elasticsearch_dependencies.go +++ b/pkg/storage/elasticsearch_dependencies.go @@ -15,7 +15,7 @@ import ( // EnableRollover returns true if rollover should be enabled func EnableRollover(spec v1.JaegerStorageSpec) bool { - useAliases := spec.Options.Map()["es.use-aliases"] + useAliases := spec.Options.StringMap()["es.use-aliases"] return (spec.Type == v1.JaegerESStorage) && strings.EqualFold(useAliases, "true") } @@ -78,7 +78,7 @@ func envVars(opts v1.Options) []corev1.EnvVar { {flag: "es.num-shards", envVar: "SHARDS"}, {flag: "es.num-replicas", envVar: "REPLICAS"}, } - options := opts.Map() + options := opts.StringMap() for _, x := range scriptEnvVars { if val, ok := options[x.flag]; ok { envs = append(envs, corev1.EnvVar{Name: x.envVar, Value: val}) diff --git a/pkg/strategy/controller.go b/pkg/strategy/controller.go index 5e03b1685..f8ae098f5 100644 --- a/pkg/strategy/controller.go +++ b/pkg/strategy/controller.go @@ -129,7 +129,7 @@ func distributedStorage(storage v1.JaegerStorageType) bool { } func normalizeSparkDependencies(spec *v1.JaegerStorageSpec) { - sFlagsMap := spec.Options.Map() + sFlagsMap := spec.Options.StringMap() tlsEnabled := sFlagsMap["es.tls"] tlsSkipHost := sFlagsMap["es.tls.skip-host-verify"] tlsCa := sFlagsMap["es.tls.ca"] @@ -201,7 +201,7 @@ func normalizeUI(spec *v1.JaegerSpec) { uiOpts = m } } - enableArchiveButton(uiOpts, spec.Storage.Options.Map()) + enableArchiveButton(uiOpts, spec.Storage.Options.StringMap()) disableDependenciesTab(uiOpts, spec.Storage.Type, spec.Storage.Dependencies.Enabled) enableDocumentationLink(uiOpts, spec) enableLogOut(uiOpts, spec) diff --git a/pkg/upgrade/v1_22_0_test.go b/pkg/upgrade/v1_22_0_test.go index 16df373a3..4bb1e5e6e 100644 --- a/pkg/upgrade/v1_22_0_test.go +++ b/pkg/upgrade/v1_22_0_test.go @@ -252,7 +252,7 @@ func TestMigrateQueryHostPortFlagsv1_22_0(t *testing.T) { persisted := &v1.Jaeger{} assert.NoError(t, cl.Get(context.Background(), nsn, persisted)) assert.Equal(t, latestVersion, persisted.Status.Version) - assert.Equal(t, tt.expectedOps, persisted.Spec.Query.Options.Map()) + assert.Equal(t, tt.expectedOps, persisted.Spec.Query.Options.StringMap()) } diff --git a/pkg/util/util.go b/pkg/util/util.go index d1b5bd4cf..dd0949d4c 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -177,12 +177,16 @@ func Labels(name, component string, jaeger v1.Jaeger) map[string]string { } // GetEsHostname return first ES hostname from options map -func GetEsHostname(opts map[string]string) string { +func GetEsHostname(opts map[string]interface{}) string { urls, ok := opts["es.server-urls"] if !ok { return "" } - urlArr := strings.Split(urls, ",") + urlsString, isString := urls.(string) + if !isString { + return "" + } + urlArr := strings.Split(urlsString, ",") return urlArr[0] } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 9f87d5bde..6ce62b6c2 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -288,14 +288,14 @@ func TestMergeTolerations(t *testing.T) { func TestGetEsHostname(t *testing.T) { tests := []struct { - underTest map[string]string + underTest map[string]interface{} hostname string }{ {hostname: ""}, - {underTest: map[string]string{"": ""}, hostname: ""}, - {underTest: map[string]string{"es.server-urls": ""}, hostname: ""}, - {underTest: map[string]string{"es.server-urls": "goo:tar"}, hostname: "goo:tar"}, - {underTest: map[string]string{"es.server-urls": "http://es:9000,https://es2:9200"}, hostname: "http://es:9000"}, + {underTest: map[string]interface{}{"": ""}, hostname: ""}, + {underTest: map[string]interface{}{"es.server-urls": ""}, hostname: ""}, + {underTest: map[string]interface{}{"es.server-urls": "goo:tar"}, hostname: "goo:tar"}, + {underTest: map[string]interface{}{"es.server-urls": "http://es:9000,https://es2:9200"}, hostname: "http://es:9000"}, } for _, test := range tests { assert.Equal(t, test.hostname, GetEsHostname(test.underTest))