From 750e826b9817bfa96f636d610c14f0b23505ecc3 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Mon, 5 Aug 2019 14:39:14 -0500 Subject: [PATCH 1/6] Add query service token propagation support Signed-off-by: Ruben Vargas --- pkg/apis/jaegertracing/v1/jaeger_types.go | 3 +++ pkg/deployment/query.go | 9 ++++++++- pkg/inject/oauth_proxy.go | 14 ++++++++++++-- pkg/storage/elasticsearch.go | 5 +++-- pkg/strategy/production.go | 21 ++++++++++++++++++++- pkg/util/util.go | 16 ++++++++++++++++ 6 files changed, 62 insertions(+), 6 deletions(-) diff --git a/pkg/apis/jaegertracing/v1/jaeger_types.go b/pkg/apis/jaegertracing/v1/jaeger_types.go index 7d0f7c2c0..82c864197 100644 --- a/pkg/apis/jaegertracing/v1/jaeger_types.go +++ b/pkg/apis/jaegertracing/v1/jaeger_types.go @@ -196,6 +196,9 @@ type JaegerQuerySpec struct { // +optional Options Options `json:"options,omitempty"` + // +optional + TokenPropagation bool `json:"tokenPropagation,omitempty"` + // +optional JaegerCommonSpec `json:",inline,omitempty"` } diff --git a/pkg/deployment/query.go b/pkg/deployment/query.go index 5673915e5..0faa1ab30 100644 --- a/pkg/deployment/query.go +++ b/pkg/deployment/query.go @@ -69,7 +69,9 @@ func (q *Query) Get() *appsv1.Deployment { options := allArgs(q.jaeger.Spec.Query.Options, q.jaeger.Spec.Storage.Options.Filter(storage.OptionsPrefix(q.jaeger.Spec.Storage.Type))) - + if q.jaeger.Spec.Query.TokenPropagation { + options = append(options, "--query.bearer-token-propagation=true") + } configmap.Update(q.jaeger, commonSpec, &options) var envFromSource []corev1.EnvFromSource if len(q.jaeger.Spec.Storage.SecretName) > 0 { @@ -194,3 +196,8 @@ func (q *Query) labels() map[string]string { func (q *Query) name() string { return fmt.Sprintf("%s-query", q.jaeger.Name) } + +//TokenPropagation returns true is token propagation is enabled on query service. +func (q *Query) TokenPropagation() bool { + return q.jaeger.Spec.Query.TokenPropagation +} diff --git a/pkg/inject/oauth_proxy.go b/pkg/inject/oauth_proxy.go index b149bd72b..9d915f4a0 100644 --- a/pkg/inject/oauth_proxy.go +++ b/pkg/inject/oauth_proxy.go @@ -9,7 +9,7 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/jaegertracing/jaeger-operator/pkg/account" - v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" + "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" "github.com/jaegertracing/jaeger-operator/pkg/service" "github.com/jaegertracing/jaeger-operator/pkg/util" ) @@ -34,8 +34,9 @@ func OAuthProxy(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment { } func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container { + args := []string{ - "--cookie-secret=SECRET", + fmt.Sprintf("--cookie-secret=%s", util.GenerateProxySecret()), "--https-address=:8443", fmt.Sprintf("--openshift-service-account=%s", account.OAuthProxyAccountNameFor(jaeger)), "--provider=openshift", @@ -44,6 +45,15 @@ func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container { "--upstream=http://localhost:16686", } + if jaeger.Spec.Query.TokenPropagation { + args = append(args, + "--pass-access-token=true", + "--pass-user-bearer-token=true", + "--scope=user:info user:check-access user:list-projects", + "--pass-basic-auth=false", + ) + } + volumeMounts := []corev1.VolumeMount{{ MountPath: "/etc/tls/private", Name: service.GetTLSSecretNameForQueryService(jaeger), diff --git a/pkg/storage/elasticsearch.go b/pkg/storage/elasticsearch.go index 24fcd4c8c..052b89927 100644 --- a/pkg/storage/elasticsearch.go +++ b/pkg/storage/elasticsearch.go @@ -39,7 +39,7 @@ type ElasticsearchDeployment struct { } // InjectStorageConfiguration changes the given spec to include ES-related command line options -func (ed *ElasticsearchDeployment) InjectStorageConfiguration(p *corev1.PodSpec) { +func (ed *ElasticsearchDeployment) InjectStorageConfiguration(p *corev1.PodSpec, tlsAuthentication bool) { p.Volumes = append(p.Volumes, corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{ @@ -52,10 +52,11 @@ func (ed *ElasticsearchDeployment) InjectStorageConfiguration(p *corev1.PodSpec) if len(p.Containers) > 0 { p.Containers[0].Args = append(p.Containers[0].Args, "--es.server-urls="+elasticsearchURL, - "--es.tls=true", + "--es.tls="+strconv.FormatBool(tlsAuthentication), "--es.tls.ca="+caPath, "--es.tls.cert="+certPath, "--es.tls.key="+keyPath) + if util.FindItem("--es.timeout", p.Containers[0].Args) == "" { p.Containers[0].Args = append(p.Containers[0].Args, "--es.timeout=15s") } diff --git a/pkg/strategy/production.go b/pkg/strategy/production.go index 6af0938d5..1f8948155 100644 --- a/pkg/strategy/production.go +++ b/pkg/strategy/production.go @@ -24,7 +24,7 @@ import ( "github.com/jaegertracing/jaeger-operator/pkg/storage" ) -func newProductionStrategy(ctx context.Context, jaeger *v1.Jaeger) S { +func newProductionStrategy(ctx context.Context, jaeger *v1.Jaeger) { tracer := global.TraceProvider().GetTracer(v1.ReconciliationTracer) ctx, span := tracer.Start(ctx, "newProductionStrategy") defer span.End() @@ -115,6 +115,25 @@ func newProductionStrategy(ctx context.Context, jaeger *v1.Jaeger) S { } for i := range esRollover { jobs = append(jobs, &esRollover[i].Spec.JobTemplate.Spec.Template.Spec) + + err := es.CreateCerts() + if err != nil { + jaeger.Logger().WithError(err).Error("failed to create Elasticsearch certificates, Elasticsearch won't be deployed") + } else { + c.secrets = es.ExtractSecrets() + c.elasticsearches = append(c.elasticsearches, *es.Elasticsearch()) + + es.InjectStorageConfiguration(&queryDep.Spec.Template.Spec, !query.TokenPropagation()) + es.InjectStorageConfiguration(&cDep.Spec.Template.Spec, true) + if indexCleaner != nil { + es.InjectSecretsConfiguration(&indexCleaner.Spec.JobTemplate.Spec.Template.Spec) + } + for i := range esRollover { + es.InjectSecretsConfiguration(&esRollover[i].Spec.JobTemplate.Spec.Template.Spec) + } + for i := range c.dependencies { + es.InjectSecretsConfiguration(&c.dependencies[i].Spec.Template.Spec) + } } autoProvisionElasticsearch(&c, jaeger, jobs, []*appsv1.Deployment{queryDep, cDep}) } diff --git a/pkg/util/util.go b/pkg/util/util.go index 7c896f5b3..84b713b41 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -1,6 +1,8 @@ package util import ( + "crypto/rand" + "encoding/base64" "fmt" "strconv" "strings" @@ -242,3 +244,17 @@ func CreateEnvsFromSecret(secretName string) []corev1.EnvFromSource { } return envs } + +// GenerateProxySecret generate random secret key for oauth proxy cookie. +func GenerateProxySecret() string { + const secretLength = 16 + randString := make([]byte, secretLength) + _, err := rand.Read(randString) + if err != nil { + // If we cannot generate random, return fixed. + return "ncNDoqLGrayxXzxTn5ANbOXZp3qXd0LA" + } + base64Secret := base64.StdEncoding.EncodeToString(randString) + return base64Secret + +} From 853b6771725fca2e6e7b509134cce40af58e2efc Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Sun, 13 Oct 2019 23:41:05 -0500 Subject: [PATCH 2/6] E2E Tests for token propagation Signed-off-by: Ruben Vargas --- .ci/run-e2e-tests.sh | 10 + .ci/start-openshift.sh | 5 +- Makefile | 5 + pkg/apis/jaegertracing/v1/jaeger_types.go | 6 +- .../jaegertracing/v1/zz_generated.deepcopy.go | 1 + pkg/deployment/query.go | 9 +- pkg/inject/oauth_proxy.go | 29 +- pkg/storage/elasticsearch.go | 90 ++-- pkg/strategy/production.go | 4 +- pkg/util/util.go | 6 +- .../elasticsearch_token_propagation_test.go | 397 ++++++++++++++++++ 11 files changed, 491 insertions(+), 71 deletions(-) create mode 100644 test/e2e/elasticsearch_token_propagation_test.go diff --git a/.ci/run-e2e-tests.sh b/.ci/run-e2e-tests.sh index c26ecd2ff..96d34353d 100755 --- a/.ci/run-e2e-tests.sh +++ b/.ci/run-e2e-tests.sh @@ -39,6 +39,16 @@ elif [ "${TEST_GROUP}" = "examples2" ] then echo "Running Examples2 Tests" make e2e-tests-examples2 +elif [ "${TEST_GROUP}" = "es-token-propagation" ] +then + echo "Running token propagation tests" + oc create user user-test-token + oc adm policy add-cluster-role-to-user cluster-admin user-test-token + # for ocp 4.2 + htpasswd -c -B -b users.htpasswd user-test-token any + oc delete secret htpass-secret -n openshift-config + oc create secret generic htpass-secret --from-file=htpasswd=./users.htpasswd -n openshift-config + make e2e-tests-token-propagation-es else echo "Unknown TEST_GROUP [${TEST_GROUP}]"; exit 1 fi diff --git a/.ci/start-openshift.sh b/.ci/start-openshift.sh index 4831ebe39..4d198eb72 100755 --- a/.ci/start-openshift.sh +++ b/.ci/start-openshift.sh @@ -9,7 +9,10 @@ export KUBERNETES_VERSION=v1.14.0 echo '{"registry-mirrors": ["https://mirror.gcr.io"], "mtu": 1460, "insecure-registries": ["172.30.0.0/16"] }' | sudo tee /etc/docker/daemon.json sudo service docker restart +HOST_IP=$(hostname -I | awk '{ print $1 }') + # install and run OCP sudo docker cp $(docker create docker.io/openshift/origin:$OPENSHIFT_VERSION):/bin/oc /usr/local/bin/oc -oc cluster up --version=$OPENSHIFT_VERSION + +oc cluster up --version=$OPENSHIFT_VERSION --public-hostname=${HOST_IP}.nip.io oc login -u system:admin diff --git a/Makefile b/Makefile index 8dec88c8a..8b5980158 100644 --- a/Makefile +++ b/Makefile @@ -120,6 +120,11 @@ e2e-tests-self-provisioned-es-kafka: prepare-e2e-tests deploy-kafka-operator dep @echo Running Self provisioned Elasticsearch and Kafka end-to-end tests... @STORAGE_NAMESPACE=$(STORAGE_NAMESPACE) ES_OPERATOR_NAMESPACE=$(ES_OPERATOR_NAMESPACE) ES_OPERATOR_IMAGE=$(ES_OPERATOR_IMAGE) go test -tags=self_provisioned_elasticsearch_kafka ./test/e2e/... $(TEST_OPTIONS) +.PHONY: e2e-tests-token-propagation-es +e2e-tests-token-propagation-es: prepare-e2e-tests deploy-es-operator + @echo Running Token Propagation Elasticsearch end-to-end tests... + @STORAGE_NAMESPACE=$(STORAGE_NAMESPACE) ES_OPERATOR_NAMESPACE=$(ES_OPERATOR_NAMESPACE) ES_OPERATOR_IMAGE=$(ES_OPERATOR_IMAGE) go test -tags=token_propagation_elasticsearch ./test/e2e/... $(TEST_OPTIONS) + .PHONY: e2e-tests-streaming e2e-tests-streaming: prepare-e2e-tests es kafka @echo Running Streaming end-to-end tests... diff --git a/pkg/apis/jaegertracing/v1/jaeger_types.go b/pkg/apis/jaegertracing/v1/jaeger_types.go index 82c864197..1ea2c40c5 100644 --- a/pkg/apis/jaegertracing/v1/jaeger_types.go +++ b/pkg/apis/jaegertracing/v1/jaeger_types.go @@ -196,9 +196,6 @@ type JaegerQuerySpec struct { // +optional Options Options `json:"options,omitempty"` - // +optional - TokenPropagation bool `json:"tokenPropagation,omitempty"` - // +optional JaegerCommonSpec `json:",inline,omitempty"` } @@ -243,6 +240,9 @@ type JaegerIngressSpec struct { // +optional JaegerCommonSpec `json:",inline,omitempty"` + + // +optional + Options Options `json:"options,omitempty"` } // JaegerIngressTLSSpec defines the TLS configuration to be used when deploying the query ingress diff --git a/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go b/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go index 328c56c84..fb4d01f9b 100644 --- a/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go +++ b/pkg/apis/jaegertracing/v1/zz_generated.deepcopy.go @@ -405,6 +405,7 @@ func (in *JaegerIngressSpec) DeepCopyInto(out *JaegerIngressSpec) { } } in.JaegerCommonSpec.DeepCopyInto(&out.JaegerCommonSpec) + in.Options.DeepCopyInto(&out.Options) return } diff --git a/pkg/deployment/query.go b/pkg/deployment/query.go index 0faa1ab30..5673915e5 100644 --- a/pkg/deployment/query.go +++ b/pkg/deployment/query.go @@ -69,9 +69,7 @@ func (q *Query) Get() *appsv1.Deployment { options := allArgs(q.jaeger.Spec.Query.Options, q.jaeger.Spec.Storage.Options.Filter(storage.OptionsPrefix(q.jaeger.Spec.Storage.Type))) - if q.jaeger.Spec.Query.TokenPropagation { - options = append(options, "--query.bearer-token-propagation=true") - } + configmap.Update(q.jaeger, commonSpec, &options) var envFromSource []corev1.EnvFromSource if len(q.jaeger.Spec.Storage.SecretName) > 0 { @@ -196,8 +194,3 @@ func (q *Query) labels() map[string]string { func (q *Query) name() string { return fmt.Sprintf("%s-query", q.jaeger.Name) } - -//TokenPropagation returns true is token propagation is enabled on query service. -func (q *Query) TokenPropagation() bool { - return q.jaeger.Spec.Query.TokenPropagation -} diff --git a/pkg/inject/oauth_proxy.go b/pkg/inject/oauth_proxy.go index 9d915f4a0..4ec26c6b0 100644 --- a/pkg/inject/oauth_proxy.go +++ b/pkg/inject/oauth_proxy.go @@ -9,11 +9,13 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/jaegertracing/jaeger-operator/pkg/account" - "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" + v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" "github.com/jaegertracing/jaeger-operator/pkg/service" "github.com/jaegertracing/jaeger-operator/pkg/util" ) +const defaultProxySecret = "ncNDoqLGrayxXzxTn5ANbOXZp3qXd0LA" + // OAuthProxy injects an appropriate proxy into the given deployment func OAuthProxy(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment { if jaeger.Spec.Ingress.Security != v1.IngressSecurityOAuthProxy { @@ -33,10 +35,14 @@ func OAuthProxy(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment { return dep } -func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container { - +func proxyInitArguments(jaeger *v1.Jaeger) []string { + secret, err := util.GenerateProxySecret() + if err != nil { + jaeger.Logger().Warnf("Error generating secret: %s, fallback to fixed secret", secret) + secret = defaultProxySecret + } args := []string{ - fmt.Sprintf("--cookie-secret=%s", util.GenerateProxySecret()), + fmt.Sprintf("--cookie-secret=%s", secret), "--https-address=:8443", fmt.Sprintf("--openshift-service-account=%s", account.OAuthProxyAccountNameFor(jaeger)), "--provider=openshift", @@ -44,16 +50,11 @@ func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container { "--tls-key=/etc/tls/private/tls.key", "--upstream=http://localhost:16686", } + return args +} - if jaeger.Spec.Query.TokenPropagation { - args = append(args, - "--pass-access-token=true", - "--pass-user-bearer-token=true", - "--scope=user:info user:check-access user:list-projects", - "--pass-basic-auth=false", - ) - } - +func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container { + args := proxyInitArguments(jaeger) volumeMounts := []corev1.VolumeMount{{ MountPath: "/etc/tls/private", Name: service.GetTLSSecretNameForQueryService(jaeger), @@ -75,6 +76,8 @@ func getOAuthProxyContainer(jaeger *v1.Jaeger) corev1.Container { args = append(args, fmt.Sprintf("--openshift-delegate-urls=%s", jaeger.Spec.Ingress.Openshift.DelegateUrls)) } + args = append(args, jaeger.Spec.Ingress.Options.ToArgs()...) + sort.Strings(args) commonSpec := util.Merge([]v1.JaegerCommonSpec{jaeger.Spec.Ingress.JaegerCommonSpec, jaeger.Spec.JaegerCommonSpec}) diff --git a/pkg/storage/elasticsearch.go b/pkg/storage/elasticsearch.go index 052b89927..f1ad3602f 100644 --- a/pkg/storage/elasticsearch.go +++ b/pkg/storage/elasticsearch.go @@ -38,8 +38,55 @@ type ElasticsearchDeployment struct { Secrets []corev1.Secret } +func (ed *ElasticsearchDeployment) injectArguments(container *corev1.Container) { + container.Args = append(container.Args, + "--es.server-urls="+elasticsearchURL) + if util.FindItem("--es.tls=", container.Args) == "" { + container.Args = append(container.Args, "--es.tls=true") + } + + container.Args = append(container.Args, + "--es.tls.ca="+caPath, + "--es.tls.cert="+certPath, + "--es.tls.key="+keyPath) + + if util.FindItem("--es.timeout", container.Args) == "" { + container.Args = append(container.Args, "--es.timeout=15s") + } + if util.FindItem("--es.num-shards", container.Args) == "" { + // taken from https://github.com/openshift/cluster-logging-operator/blob/32b69e8bcf61a805e8f3c45c664a3c08d1ee62d5/vendor/github.com/openshift/elasticsearch-operator/pkg/k8shandler/configmaps.go#L38 + // every ES node is a data node + container.Args = append(container.Args, fmt.Sprintf("--es.num-shards=%d", ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)) + } + if util.FindItem("--es.num-replicas", container.Args) == "" { + container.Args = append(container.Args, fmt.Sprintf("--es.num-replicas=%d", + calculateReplicaShards(ed.Jaeger.Spec.Storage.Elasticsearch.RedundancyPolicy, int(ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)))) + } + if strings.EqualFold(util.FindItem("--es-archive.enabled", container.Args), "--es-archive.enabled=true") { + container.Args = append(container.Args, + "--es-archive.server-urls="+elasticsearchURL, + "--es-archive.tls=true", + "--es-archive.tls.ca="+caPath, + "--es-archive.tls.cert="+certPath, + "--es-archive.tls.key="+keyPath, + ) + if util.FindItem("--es-archive.timeout", container.Args) == "" { + container.Args = append(container.Args, "--es-archive.timeout=15s") + } + if util.FindItem("--es-archive.num-shards", container.Args) == "" { + // taken from https://github.com/openshift/cluster-logging-operator/blob/32b69e8bcf61a805e8f3c45c664a3c08d1ee62d5/vendor/github.com/openshift/elasticsearch-operator/pkg/k8shandler/configmaps.go#L38 + // every ES node is a data node + container.Args = append(container.Args, fmt.Sprintf("--es-archive.num-shards=%d", ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)) + } + if util.FindItem("--es-archive.num-replicas", container.Args) == "" { + container.Args = append(container.Args, fmt.Sprintf("--es-archive.num-replicas=%d", + calculateReplicaShards(ed.Jaeger.Spec.Storage.Elasticsearch.RedundancyPolicy, int(ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)))) + } + } +} + // InjectStorageConfiguration changes the given spec to include ES-related command line options -func (ed *ElasticsearchDeployment) InjectStorageConfiguration(p *corev1.PodSpec, tlsAuthentication bool) { +func (ed *ElasticsearchDeployment) InjectStorageConfiguration(p *corev1.PodSpec) { p.Volumes = append(p.Volumes, corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{ @@ -50,46 +97,7 @@ func (ed *ElasticsearchDeployment) InjectStorageConfiguration(p *corev1.PodSpec, }) // we assume jaeger containers are first if len(p.Containers) > 0 { - p.Containers[0].Args = append(p.Containers[0].Args, - "--es.server-urls="+elasticsearchURL, - "--es.tls="+strconv.FormatBool(tlsAuthentication), - "--es.tls.ca="+caPath, - "--es.tls.cert="+certPath, - "--es.tls.key="+keyPath) - - if util.FindItem("--es.timeout", p.Containers[0].Args) == "" { - p.Containers[0].Args = append(p.Containers[0].Args, "--es.timeout=15s") - } - if util.FindItem("--es.num-shards", p.Containers[0].Args) == "" { - // taken from https://github.com/openshift/cluster-logging-operator/blob/32b69e8bcf61a805e8f3c45c664a3c08d1ee62d5/vendor/github.com/openshift/elasticsearch-operator/pkg/k8shandler/configmaps.go#L38 - // every ES node is a data node - p.Containers[0].Args = append(p.Containers[0].Args, fmt.Sprintf("--es.num-shards=%d", ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)) - } - if util.FindItem("--es.num-replicas", p.Containers[0].Args) == "" { - p.Containers[0].Args = append(p.Containers[0].Args, fmt.Sprintf("--es.num-replicas=%d", - calculateReplicaShards(ed.Jaeger.Spec.Storage.Elasticsearch.RedundancyPolicy, int(ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)))) - } - if strings.EqualFold(util.FindItem("--es-archive.enabled", p.Containers[0].Args), "--es-archive.enabled=true") { - p.Containers[0].Args = append(p.Containers[0].Args, - "--es-archive.server-urls="+elasticsearchURL, - "--es-archive.tls=true", - "--es-archive.tls.ca="+caPath, - "--es-archive.tls.cert="+certPath, - "--es-archive.tls.key="+keyPath, - ) - if util.FindItem("--es-archive.timeout", p.Containers[0].Args) == "" { - p.Containers[0].Args = append(p.Containers[0].Args, "--es-archive.timeout=15s") - } - if util.FindItem("--es-archive.num-shards", p.Containers[0].Args) == "" { - // taken from https://github.com/openshift/cluster-logging-operator/blob/32b69e8bcf61a805e8f3c45c664a3c08d1ee62d5/vendor/github.com/openshift/elasticsearch-operator/pkg/k8shandler/configmaps.go#L38 - // every ES node is a data node - p.Containers[0].Args = append(p.Containers[0].Args, fmt.Sprintf("--es-archive.num-shards=%d", ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)) - } - if util.FindItem("--es-archive.num-replicas", p.Containers[0].Args) == "" { - p.Containers[0].Args = append(p.Containers[0].Args, fmt.Sprintf("--es-archive.num-replicas=%d", - calculateReplicaShards(ed.Jaeger.Spec.Storage.Elasticsearch.RedundancyPolicy, int(ed.Jaeger.Spec.Storage.Elasticsearch.NodeCount)))) - } - } + ed.injectArguments(&p.Containers[0]) p.Containers[0].VolumeMounts = append(p.Containers[0].VolumeMounts, corev1.VolumeMount{ Name: volumeName, ReadOnly: true, diff --git a/pkg/strategy/production.go b/pkg/strategy/production.go index 1f8948155..f91f2161f 100644 --- a/pkg/strategy/production.go +++ b/pkg/strategy/production.go @@ -123,8 +123,8 @@ func newProductionStrategy(ctx context.Context, jaeger *v1.Jaeger) { c.secrets = es.ExtractSecrets() c.elasticsearches = append(c.elasticsearches, *es.Elasticsearch()) - es.InjectStorageConfiguration(&queryDep.Spec.Template.Spec, !query.TokenPropagation()) - es.InjectStorageConfiguration(&cDep.Spec.Template.Spec, true) + es.InjectStorageConfiguration(&queryDep.Spec.Template.Spec) + es.InjectStorageConfiguration(&cDep.Spec.Template.Spec) if indexCleaner != nil { es.InjectSecretsConfiguration(&indexCleaner.Spec.JobTemplate.Spec.Template.Spec) } diff --git a/pkg/util/util.go b/pkg/util/util.go index 84b713b41..6abc3b418 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -246,15 +246,15 @@ func CreateEnvsFromSecret(secretName string) []corev1.EnvFromSource { } // GenerateProxySecret generate random secret key for oauth proxy cookie. -func GenerateProxySecret() string { +func GenerateProxySecret() (string, error) { const secretLength = 16 randString := make([]byte, secretLength) _, err := rand.Read(randString) if err != nil { // If we cannot generate random, return fixed. - return "ncNDoqLGrayxXzxTn5ANbOXZp3qXd0LA" + return "", err } base64Secret := base64.StdEncoding.EncodeToString(randString) - return base64Secret + return base64Secret, nil } diff --git a/test/e2e/elasticsearch_token_propagation_test.go b/test/e2e/elasticsearch_token_propagation_test.go new file mode 100644 index 000000000..f425dabba --- /dev/null +++ b/test/e2e/elasticsearch_token_propagation_test.go @@ -0,0 +1,397 @@ +// +build token_propagation_elasticsearch + +package e2e + +import ( + "bytes" + goctx "context" + "crypto/tls" + "errors" + "fmt" + "io/ioutil" + "net/http" + "net/http/cookiejar" + "net/url" + "strings" + "testing" + "time" + + framework "github.com/operator-framework/operator-sdk/pkg/test" + "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + "github.com/uber/jaeger-client-go/config" + "golang.org/x/net/html" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/jaegertracing/jaeger-operator/pkg/apis" + v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" + esv1 "github.com/jaegertracing/jaeger-operator/pkg/storage/elasticsearch/v1" +) + +// Test parameters +const name = "token-prop" +const username = "user-test-token" +const password = "any" +const collectorPodImageName = "jaeger-collector" +const testServiceName = "token-propagation" + +type TokenPropagationTestSuite struct { + suite.Suite + exampleJaeger *v1.Jaeger + queryName string + collectorName string + queryServiceEndPoint string + host string +} + +func (suite *TokenPropagationTestSuite) SetupSuite() { + t = suite.T() + if !isOpenShift(t) { + t.Skipf("Test %s is currently supported only on OpenShift because es-operator runs only on OpenShift\n", t.Name()) + } + assert.NoError(t, framework.AddToFrameworkScheme(apis.AddToScheme, &v1.JaegerList{ + TypeMeta: metav1.TypeMeta{ + Kind: "Jaeger", + APIVersion: "jaegertracing.io/v1", + }, + })) + assert.NoError(t, framework.AddToFrameworkScheme(apis.AddToScheme, &esv1.ElasticsearchList{ + TypeMeta: metav1.TypeMeta{ + Kind: "Elasticsearch", + APIVersion: "logging.openshift.io/v1", + }, + })) + var err error + ctx, err = prepare(t) + if err != nil { + if ctx != nil { + ctx.Cleanup() + } + require.FailNow(t, "Failed in prepare") + } + fw = framework.Global + namespace, _ = ctx.GetNamespace() + require.NotNil(t, namespace, "GetNamespace failed") + addToFrameworkSchemeForSmokeTests(t) + + suite.deployJaegerWithPropagationEnabled() + +} + +func (suite *TokenPropagationTestSuite) TearDownSuite() { + // undeployJaegerInstance(suite.exampleJaeger) + // handleSuiteTearDown() +} + +func (suite *TokenPropagationTestSuite) TestTokenPropagationNoToken() { + + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + }, + } + + err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { + req, err := http.NewRequest(http.MethodGet, suite.queryServiceEndPoint, nil) + resp, err := client.Do(req) + defer resp.Body.Close() + require.Equal(t, http.StatusForbidden, resp.StatusCode) + return true, nil + }) + require.NoError(t, err, "Token propagation test failed") + +} + +func (suite *TokenPropagationTestSuite) TestTokenPropagationValidToken() { + /* Create an span */ + collectorPort := randomPortNumber() + collectorPorts := []string{collectorPort + ":14268"} + portForwColl, closeChanColl := CreatePortForward(namespace, suite.collectorName, collectorPodImageName, collectorPorts, fw.KubeConfig) + defer portForwColl.Close() + defer close(closeChanColl) + collectorEndpoint := fmt.Sprintf("http://localhost:%s/api/traces", collectorPort) + + cfg := config.Configuration{ + Reporter: &config.ReporterConfig{CollectorEndpoint: collectorEndpoint}, + Sampler: &config.SamplerConfig{Type: "const", Param: 1}, + ServiceName: testServiceName, + } + tracer, closer, err := cfg.NewTracer() + require.NoError(t, err, "Failed to create tracer in token propagation test") + + tStr := time.Now().Format(time.RFC3339Nano) + tracer.StartSpan("TokenTest"). + SetTag("time-RFC3339Nano", tStr). + Finish() + closer.Close() + + /* Get token using oauth OpenShift authorization */ + client, err := oAuthAuthorization(suite.host, username, password) + + /* Try to reach query endpoint */ + err = wait.Poll(retryInterval, timeout, func() (done bool, err error) { + req, err := http.NewRequest(http.MethodGet, suite.queryServiceEndPoint, nil) + resp, err := client.Do(req) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) + if resp.StatusCode != http.StatusOK { + return false, errors.New("Query service returns http code: " + string(resp.StatusCode)) + } + bodyBytes, err := ioutil.ReadAll(resp.Body) + bodyString := string(bodyBytes) + if !strings.Contains(bodyString, "errors\":null") { + return false, errors.New("query service returns errors: " + bodyString) + } + + return strings.Contains(bodyString, tStr), nil + return true, nil + }) + + require.NoError(t, err, "Token propagation test failed") +} + +func getESJaegerInstance() *v1.Jaeger { + exampleJaeger := &v1.Jaeger{ + TypeMeta: metav1.TypeMeta{ + Kind: "Jaeger", + APIVersion: "jaegertracing.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "token-prop", + Namespace: namespace, + }, + Spec: v1.JaegerSpec{ + Strategy: "production", + Storage: v1.JaegerStorageSpec{ + Type: "elasticsearch", + Elasticsearch: v1.ElasticsearchSpec{ + NodeCount: 1, + Resources: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, + }, + }, + }, + Query: v1.JaegerQuerySpec{ + Options: v1.NewOptions(map[string]interface{}{ + "es.version": "5", + "query.bearer-token-propagation": "true", + "es.tls": "false", + }), + }, + Ingress: v1.JaegerIngressSpec{ + Options: v1.NewOptions(map[string]interface{}{ + "pass-access-token": "true", + "pass-user-bearer-token": "true", + "scope": "user:info user:check-access user:list-projects", + "pass-basic-auth": "false", + }), + }, + }, + } + return exampleJaeger +} + +func (suite *TokenPropagationTestSuite) deployJaegerWithPropagationEnabled() { + queryName := fmt.Sprintf("%s-query", name) + collectorName := fmt.Sprintf("%s-collector", name) + + // create jaeger custom resource + suite.exampleJaeger = getESJaegerInstance() + err := fw.Client.Create(goctx.TODO(), + suite.exampleJaeger, + &framework.CleanupOptions{ + TestContext: ctx, + Timeout: timeout, + RetryInterval: retryInterval, + }) + require.NoError(t, err, "Error deploying example Jaeger") + + err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, collectorName, 1, retryInterval, timeout) + require.NoError(t, err, "Error waiting for collector deployment") + + err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, queryName, 1, retryInterval, timeout) + require.NoError(t, err, "Error waiting for query deployment") + + route := findRoute(t, fw, name) + + suite.host = route.Spec.Host + suite.queryServiceEndPoint = fmt.Sprintf("https://%s/api/traces?service=%s", suite.host, testServiceName) + +} + +func TestTokenPropagationSuite(t *testing.T) { + suite.Run(t, new(TokenPropagationTestSuite)) +} + +func newHTTPSClient() *http.Client { + tr := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + client := &http.Client{ + Transport: tr, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + } + return client +} + +func oAuthAuthorization(host, user, pass string) (*http.Client, error) { + /* Setup client*/ + cookieJar, _ := cookiejar.New(nil) + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + }, + Jar: cookieJar, + } + /* Start oauth */ + resp, err := client.Get("https://" + host + "/oauth/start") + defer resp.Body.Close() + if err != nil { + return nil, err + } + responseBytes, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + var req *http.Request + /* Submit form */ + if hasForm(responseBytes) { + req = getLoginFormRequest(responseBytes, resp.Request.URL, user, pass) + } else { + // OCP 4.2 or newer. + // Choose idp + link := getLinkToHtpassIDP(responseBytes) + resp, err := client.Get("https://" + resp.Request.URL.Host + link) + if err != nil { + return nil, err + } + + responseBytes, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() + + if err != nil { + return nil, err + } + req = getLoginFormRequest(responseBytes, resp.Request.URL, user, pass) + } + resp, err = client.Do(req) + defer resp.Body.Close() + if resp.Request.URL.Path == "/oauth/authorize/approve" { + req = submitGrantForm(resp) + resp, err = client.Do(req) + defer resp.Body.Close() + } + return client, nil +} + +func hasForm(responseBytes []byte) bool { + root, _ := html.Parse(bytes.NewBuffer(responseBytes)) + form := false + visit(root, func(n *html.Node) { + if n.Type == html.ElementNode && n.Data == "form" { + form = true + } + }) + return form +} + +func getLinkToHtpassIDP(responseBytes []byte) string { + root, _ := html.Parse(bytes.NewBuffer(responseBytes)) + link := "" + visit(root, func(n *html.Node) { + if n.Type == html.ElementNode && n.Data == "a" { + href := getAttr(n, "href") + url, _ := url.Parse(href) + if url.Query().Get("idp") == "htpasswd_provider" { + link = href + } + } + }) + return link +} + +func getLoginFormRequest(responseBytes []byte, currentURL *url.URL, username, password string) *http.Request { + reqHeader := http.Header{} + action := "" + formData := url.Values{} + root, _ := html.Parse(bytes.NewBuffer(responseBytes)) + visit(root, func(n *html.Node) { + if n.Type == html.ElementNode && n.Data == "input" { + inputType := getAttr(n, "type") + if inputType == "hidden" { + name := getAttr(n, "name") + value := getAttr(n, "value") + formData.Add(name, value) + } + } + if n.Type == html.ElementNode && n.Data == "form" { + action = getAttr(n, "action") + } + }) + + formData.Add("username", username) + formData.Add("password", password) + reqHeader.Set("Content-Type", "application/x-www-form-urlencoded") + reqBody := strings.NewReader(formData.Encode()) + reqURL, _ := currentURL.Parse(action) + req, _ := http.NewRequest("POST", reqURL.String(), reqBody) + req.Header = reqHeader + return req +} + +func submitGrantForm(response *http.Response) *http.Request { + reqHeader := http.Header{} + action := "" + formData := url.Values{} + currentURL := response.Request.URL + responseBytes, err := ioutil.ReadAll(response.Body) + if err != nil { + return nil + } + root, err := html.Parse(bytes.NewBuffer(responseBytes)) + visit(root, func(n *html.Node) { + if n.Type == html.ElementNode && n.Data == "input" { + inputType := getAttr(n, "type") + if inputType == "hidden" || inputType == "checkbox" || inputType == "radio" { + name := getAttr(n, "name") + value := getAttr(n, "value") + formData.Add(name, value) + } + } + if n.Type == html.ElementNode && n.Data == "form" { + action = getAttr(n, "action") + } + }) + formData.Add("approve", "Allow selected permissions") + reqHeader.Set("Content-Type", "application/x-www-form-urlencoded") + reqBody := strings.NewReader(formData.Encode()) + reqURL, _ := currentURL.Parse(action) + req, _ := http.NewRequest("POST", reqURL.String(), reqBody) + req.Header = reqHeader + return req +} + +func getAttr(element *html.Node, attrName string) string { + for _, attr := range element.Attr { + if attr.Key == attrName { + return attr.Val + } + } + return "" +} + +func visit(n *html.Node, visitor func(*html.Node)) { + visitor(n) + for c := n.FirstChild; c != nil; c = c.NextSibling { + visit(c, visitor) + } +} From f100fc9ba7e0388b4e567c0f365c9be917d79529 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Fri, 10 Jan 2020 20:12:16 -0600 Subject: [PATCH 3/6] Use auth delegation to avoid oauth form parsing Signed-off-by: Ruben Vargas Signed-off-by: Ruben Vargas --- .ci/run-e2e-tests.sh | 6 - deploy/crds/jaegertracing.io_jaegers_crd.yaml | 4 + .../jaegertracing/v1/zz_generated.openapi.go | 7 +- pkg/inject/oauth_proxy.go | 1 + pkg/strategy/production.go | 21 +- .../elasticsearch_token_propagation_test.go | 364 ++++++++---------- 6 files changed, 164 insertions(+), 239 deletions(-) diff --git a/.ci/run-e2e-tests.sh b/.ci/run-e2e-tests.sh index 96d34353d..0e95f42f7 100755 --- a/.ci/run-e2e-tests.sh +++ b/.ci/run-e2e-tests.sh @@ -42,12 +42,6 @@ then elif [ "${TEST_GROUP}" = "es-token-propagation" ] then echo "Running token propagation tests" - oc create user user-test-token - oc adm policy add-cluster-role-to-user cluster-admin user-test-token - # for ocp 4.2 - htpasswd -c -B -b users.htpasswd user-test-token any - oc delete secret htpass-secret -n openshift-config - oc create secret generic htpass-secret --from-file=htpasswd=./users.htpasswd -n openshift-config make e2e-tests-token-propagation-es else echo "Unknown TEST_GROUP [${TEST_GROUP}]"; exit 1 diff --git a/deploy/crds/jaegertracing.io_jaegers_crd.yaml b/deploy/crds/jaegertracing.io_jaegers_crd.yaml index 0615e6b65..cf8cec335 100644 --- a/deploy/crds/jaegertracing.io_jaegers_crd.yaml +++ b/deploy/crds/jaegertracing.io_jaegers_crd.yaml @@ -9434,6 +9434,10 @@ spec: sar: type: string type: object + options: + description: Options defines a common options parameter to the different + structs + type: object resources: description: ResourceRequirements describes the compute resource requirements. diff --git a/pkg/apis/jaegertracing/v1/zz_generated.openapi.go b/pkg/apis/jaegertracing/v1/zz_generated.openapi.go index bbf1da80c..bbce3cdc2 100644 --- a/pkg/apis/jaegertracing/v1/zz_generated.openapi.go +++ b/pkg/apis/jaegertracing/v1/zz_generated.openapi.go @@ -1361,11 +1361,16 @@ func schema_pkg_apis_jaegertracing_v1_JaegerIngressSpec(ref common.ReferenceCall Format: "", }, }, + "options": { + SchemaProps: spec.SchemaProps{ + Ref: ref("./pkg/apis/jaegertracing/v1.Options"), + }, + }, }, }, }, Dependencies: []string{ - "./pkg/apis/jaegertracing/v1.JaegerIngressOpenShiftSpec", "./pkg/apis/jaegertracing/v1.JaegerIngressTLSSpec", "k8s.io/api/core/v1.Affinity", "k8s.io/api/core/v1.PodSecurityContext", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.Toleration", "k8s.io/api/core/v1.Volume", "k8s.io/api/core/v1.VolumeMount"}, + "./pkg/apis/jaegertracing/v1.JaegerIngressOpenShiftSpec", "./pkg/apis/jaegertracing/v1.JaegerIngressTLSSpec", "./pkg/apis/jaegertracing/v1.Options", "k8s.io/api/core/v1.Affinity", "k8s.io/api/core/v1.PodSecurityContext", "k8s.io/api/core/v1.ResourceRequirements", "k8s.io/api/core/v1.Toleration", "k8s.io/api/core/v1.Volume", "k8s.io/api/core/v1.VolumeMount"}, } } diff --git a/pkg/inject/oauth_proxy.go b/pkg/inject/oauth_proxy.go index 4ec26c6b0..c77152a88 100644 --- a/pkg/inject/oauth_proxy.go +++ b/pkg/inject/oauth_proxy.go @@ -14,6 +14,7 @@ import ( "github.com/jaegertracing/jaeger-operator/pkg/util" ) +// #nosec G101 (CWE-798): Potential hardcoded credentials const defaultProxySecret = "ncNDoqLGrayxXzxTn5ANbOXZp3qXd0LA" // OAuthProxy injects an appropriate proxy into the given deployment diff --git a/pkg/strategy/production.go b/pkg/strategy/production.go index f91f2161f..6af0938d5 100644 --- a/pkg/strategy/production.go +++ b/pkg/strategy/production.go @@ -24,7 +24,7 @@ import ( "github.com/jaegertracing/jaeger-operator/pkg/storage" ) -func newProductionStrategy(ctx context.Context, jaeger *v1.Jaeger) { +func newProductionStrategy(ctx context.Context, jaeger *v1.Jaeger) S { tracer := global.TraceProvider().GetTracer(v1.ReconciliationTracer) ctx, span := tracer.Start(ctx, "newProductionStrategy") defer span.End() @@ -115,25 +115,6 @@ func newProductionStrategy(ctx context.Context, jaeger *v1.Jaeger) { } for i := range esRollover { jobs = append(jobs, &esRollover[i].Spec.JobTemplate.Spec.Template.Spec) - - err := es.CreateCerts() - if err != nil { - jaeger.Logger().WithError(err).Error("failed to create Elasticsearch certificates, Elasticsearch won't be deployed") - } else { - c.secrets = es.ExtractSecrets() - c.elasticsearches = append(c.elasticsearches, *es.Elasticsearch()) - - es.InjectStorageConfiguration(&queryDep.Spec.Template.Spec) - es.InjectStorageConfiguration(&cDep.Spec.Template.Spec) - if indexCleaner != nil { - es.InjectSecretsConfiguration(&indexCleaner.Spec.JobTemplate.Spec.Template.Spec) - } - for i := range esRollover { - es.InjectSecretsConfiguration(&esRollover[i].Spec.JobTemplate.Spec.Template.Spec) - } - for i := range c.dependencies { - es.InjectSecretsConfiguration(&c.dependencies[i].Spec.Template.Spec) - } } autoProvisionElasticsearch(&c, jaeger, jobs, []*appsv1.Deployment{queryDep, cDep}) } diff --git a/test/e2e/elasticsearch_token_propagation_test.go b/test/e2e/elasticsearch_token_propagation_test.go index f425dabba..12c55f765 100644 --- a/test/e2e/elasticsearch_token_propagation_test.go +++ b/test/e2e/elasticsearch_token_propagation_test.go @@ -3,15 +3,12 @@ package e2e import ( - "bytes" goctx "context" "crypto/tls" "errors" "fmt" "io/ioutil" "net/http" - "net/http/cookiejar" - "net/url" "strings" "testing" "time" @@ -22,12 +19,14 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/uber/jaeger-client-go/config" - "golang.org/x/net/html" corev1 "k8s.io/api/core/v1" + rbac "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apimachinery/pkg/types" + "github.com/jaegertracing/jaeger-operator/pkg/apis" v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1" esv1 "github.com/jaegertracing/jaeger-operator/pkg/storage/elasticsearch/v1" @@ -39,17 +38,19 @@ const username = "user-test-token" const password = "any" const collectorPodImageName = "jaeger-collector" const testServiceName = "token-propagation" +const test_account = "token-test-user" -type TokenPropagationTestSuite struct { +type TokenTestSuite struct { suite.Suite exampleJaeger *v1.Jaeger queryName string collectorName string queryServiceEndPoint string host string + token string } -func (suite *TokenPropagationTestSuite) SetupSuite() { +func (suite *TokenTestSuite) SetupSuite() { t = suite.T() if !isOpenShift(t) { t.Skipf("Test %s is currently supported only on OpenShift because es-operator runs only on OpenShift\n", t.Name()) @@ -78,24 +79,20 @@ func (suite *TokenPropagationTestSuite) SetupSuite() { namespace, _ = ctx.GetNamespace() require.NotNil(t, namespace, "GetNamespace failed") addToFrameworkSchemeForSmokeTests(t) - suite.deployJaegerWithPropagationEnabled() - } -func (suite *TokenPropagationTestSuite) TearDownSuite() { - // undeployJaegerInstance(suite.exampleJaeger) - // handleSuiteTearDown() +func (suite *TokenTestSuite) TearDownSuite() { + undeployJaegerInstance(suite.exampleJaeger) + handleSuiteTearDown() } -func (suite *TokenPropagationTestSuite) TestTokenPropagationNoToken() { - +func (suite *TokenTestSuite) TestTokenPropagationNoToken() { client := &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, }, } - err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { req, err := http.NewRequest(http.MethodGet, suite.queryServiceEndPoint, nil) resp, err := client.Do(req) @@ -104,10 +101,9 @@ func (suite *TokenPropagationTestSuite) TestTokenPropagationNoToken() { return true, nil }) require.NoError(t, err, "Token propagation test failed") - } -func (suite *TokenPropagationTestSuite) TestTokenPropagationValidToken() { +func (suite *TokenTestSuite) TestTokenPropagationValidToken() { /* Create an span */ collectorPort := randomPortNumber() collectorPorts := []string{collectorPort + ":14268"} @@ -115,7 +111,6 @@ func (suite *TokenPropagationTestSuite) TestTokenPropagationValidToken() { defer portForwColl.Close() defer close(closeChanColl) collectorEndpoint := fmt.Sprintf("http://localhost:%s/api/traces", collectorPort) - cfg := config.Configuration{ Reporter: &config.ReporterConfig{CollectorEndpoint: collectorEndpoint}, Sampler: &config.SamplerConfig{Type: "const", Param: 1}, @@ -123,22 +118,18 @@ func (suite *TokenPropagationTestSuite) TestTokenPropagationValidToken() { } tracer, closer, err := cfg.NewTracer() require.NoError(t, err, "Failed to create tracer in token propagation test") - tStr := time.Now().Format(time.RFC3339Nano) tracer.StartSpan("TokenTest"). SetTag("time-RFC3339Nano", tStr). Finish() closer.Close() - - /* Get token using oauth OpenShift authorization */ - client, err := oAuthAuthorization(suite.host, username, password) - + client := newHTTPSClient() /* Try to reach query endpoint */ err = wait.Poll(retryInterval, timeout, func() (done bool, err error) { req, err := http.NewRequest(http.MethodGet, suite.queryServiceEndPoint, nil) + req.Header.Add("Authorization", "Bearer "+suite.token) resp, err := client.Do(req) defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) if resp.StatusCode != http.StatusOK { return false, errors.New("Query service returns http code: " + string(resp.StatusCode)) @@ -148,15 +139,145 @@ func (suite *TokenPropagationTestSuite) TestTokenPropagationValidToken() { if !strings.Contains(bodyString, "errors\":null") { return false, errors.New("query service returns errors: " + bodyString) } - - return strings.Contains(bodyString, tStr), nil return true, nil }) - require.NoError(t, err, "Token propagation test failed") } -func getESJaegerInstance() *v1.Jaeger { +func (suite *TokenTestSuite) deployJaegerWithPropagationEnabled() { + queryName := fmt.Sprintf("%s-query", name) + collectorName := fmt.Sprintf("%s-collector", name) + bindOperatorWithAuthDelegator() + createTestServiceAccount() + suite.token = testAccountToken() + + suite.exampleJaeger = jaegerInstance() + err := fw.Client.Create(goctx.Background(), + suite.exampleJaeger, + &framework.CleanupOptions{ + TestContext: ctx, + Timeout: timeout, + RetryInterval: retryInterval, + }) + require.NoError(t, err, "Error deploying example Jaeger") + + err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, collectorName, 1, retryInterval, timeout) + require.NoError(t, err, "Error waiting for collector deployment") + + err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, queryName, 1, retryInterval, timeout) + require.NoError(t, err, "Error waiting for query deployment") + + route := findRoute(t, fw, name) + + suite.host = route.Spec.Host + suite.queryServiceEndPoint = fmt.Sprintf("https://%s/api/traces?service=%s", suite.host, testServiceName) +} + +func TestTokenSuite(t *testing.T) { + suite.Run(t, new(TokenTestSuite)) +} + +func bindOperatorWithAuthDelegator() { + roleBinding := rbac.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: "jaeger-operator:system:auth-delegator", + Namespace: namespace, + }, + Subjects: []rbac.Subject{{ + Kind: "ServiceAccount", + Name: "jaeger-operator", + Namespace: namespace, + }}, + RoleRef: rbac.RoleRef{ + Kind: "ClusterRole", + Name: "system:auth-delegator", + }, + } + err := fw.Client.Create(goctx.Background(), + &roleBinding, + &framework.CleanupOptions{ + TestContext: ctx, + Timeout: timeout, + RetryInterval: retryInterval, + }) + require.NoError(t, err, "Error binding operator service account with auth-delegator") +} + +func createTestServiceAccount() { + + serviceAccount := corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: test_account, + Namespace: namespace, + }, + } + err := fw.Client.Create(goctx.Background(), + &serviceAccount, + &framework.CleanupOptions{ + TestContext: ctx, + Timeout: timeout, + RetryInterval: retryInterval, + }) + require.NoError(t, err, "Error deploying example Jaeger") + + roleBinding := rbac.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: test_account + "-bind", + Namespace: namespace, + }, + Subjects: []rbac.Subject{{ + Kind: "ServiceAccount", + Name: serviceAccount.Name, + Namespace: namespace, + }}, + RoleRef: rbac.RoleRef{ + Kind: "ClusterRole", + Name: "cluster-admin", + }, + } + + err = fw.Client.Create(goctx.Background(), + &roleBinding, + &framework.CleanupOptions{ + TestContext: ctx, + Timeout: timeout, + RetryInterval: retryInterval, + }) + require.NoError(t, err, "Error deploying example Jaeger") + +} + +func testAccountToken() string { + var secretName string + err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { + serviceAccount := corev1.ServiceAccount{} + e := fw.Client.Get(goctx.Background(), types.NamespacedName{ + Namespace: namespace, + Name: test_account, + }, &serviceAccount) + if e != nil { + return false, e + } + for _, s := range serviceAccount.Secrets { + if strings.HasPrefix(s.Name, test_account+"-token") { + secretName = s.Name + return true, nil + } + } + return false, nil + }) + require.NoError(t, err, "Error getting service account token") + require.NotEmpty(t, secretName, "secret with token not found") + secret := corev1.Secret{} + err = fw.Client.Get(goctx.Background(), types.NamespacedName{ + Namespace: namespace, + Name: secretName, + }, &secret) + require.NoError(t, err, "Error deploying example Jaeger") + return string(secret.Data["token"]) +} + +func jaegerInstance() *v1.Jaeger { exampleJaeger := &v1.Jaeger{ TypeMeta: metav1.TypeMeta{ Kind: "Jaeger", @@ -186,6 +307,10 @@ func getESJaegerInstance() *v1.Jaeger { }), }, Ingress: v1.JaegerIngressSpec{ + Openshift: v1.JaegerIngressOpenShiftSpec{ + SAR: "{\"namespace\": \"default\", \"resource\": \"pods\", \"verb\": \"get\"}", + DelegateUrls: `{"/":{"namespace": "default", "resource": "pods", "verb": "get"}}`, + }, Options: v1.NewOptions(map[string]interface{}{ "pass-access-token": "true", "pass-user-bearer-token": "true", @@ -198,38 +323,6 @@ func getESJaegerInstance() *v1.Jaeger { return exampleJaeger } -func (suite *TokenPropagationTestSuite) deployJaegerWithPropagationEnabled() { - queryName := fmt.Sprintf("%s-query", name) - collectorName := fmt.Sprintf("%s-collector", name) - - // create jaeger custom resource - suite.exampleJaeger = getESJaegerInstance() - err := fw.Client.Create(goctx.TODO(), - suite.exampleJaeger, - &framework.CleanupOptions{ - TestContext: ctx, - Timeout: timeout, - RetryInterval: retryInterval, - }) - require.NoError(t, err, "Error deploying example Jaeger") - - err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, collectorName, 1, retryInterval, timeout) - require.NoError(t, err, "Error waiting for collector deployment") - - err = e2eutil.WaitForDeployment(t, fw.KubeClient, namespace, queryName, 1, retryInterval, timeout) - require.NoError(t, err, "Error waiting for query deployment") - - route := findRoute(t, fw, name) - - suite.host = route.Spec.Host - suite.queryServiceEndPoint = fmt.Sprintf("https://%s/api/traces?service=%s", suite.host, testServiceName) - -} - -func TestTokenPropagationSuite(t *testing.T) { - suite.Run(t, new(TokenPropagationTestSuite)) -} - func newHTTPSClient() *http.Client { tr := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, @@ -242,156 +335,3 @@ func newHTTPSClient() *http.Client { } return client } - -func oAuthAuthorization(host, user, pass string) (*http.Client, error) { - /* Setup client*/ - cookieJar, _ := cookiejar.New(nil) - client := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, - Jar: cookieJar, - } - /* Start oauth */ - resp, err := client.Get("https://" + host + "/oauth/start") - defer resp.Body.Close() - if err != nil { - return nil, err - } - responseBytes, err := ioutil.ReadAll(resp.Body) - if err != nil { - return nil, err - } - var req *http.Request - /* Submit form */ - if hasForm(responseBytes) { - req = getLoginFormRequest(responseBytes, resp.Request.URL, user, pass) - } else { - // OCP 4.2 or newer. - // Choose idp - link := getLinkToHtpassIDP(responseBytes) - resp, err := client.Get("https://" + resp.Request.URL.Host + link) - if err != nil { - return nil, err - } - - responseBytes, err := ioutil.ReadAll(resp.Body) - defer resp.Body.Close() - - if err != nil { - return nil, err - } - req = getLoginFormRequest(responseBytes, resp.Request.URL, user, pass) - } - resp, err = client.Do(req) - defer resp.Body.Close() - if resp.Request.URL.Path == "/oauth/authorize/approve" { - req = submitGrantForm(resp) - resp, err = client.Do(req) - defer resp.Body.Close() - } - return client, nil -} - -func hasForm(responseBytes []byte) bool { - root, _ := html.Parse(bytes.NewBuffer(responseBytes)) - form := false - visit(root, func(n *html.Node) { - if n.Type == html.ElementNode && n.Data == "form" { - form = true - } - }) - return form -} - -func getLinkToHtpassIDP(responseBytes []byte) string { - root, _ := html.Parse(bytes.NewBuffer(responseBytes)) - link := "" - visit(root, func(n *html.Node) { - if n.Type == html.ElementNode && n.Data == "a" { - href := getAttr(n, "href") - url, _ := url.Parse(href) - if url.Query().Get("idp") == "htpasswd_provider" { - link = href - } - } - }) - return link -} - -func getLoginFormRequest(responseBytes []byte, currentURL *url.URL, username, password string) *http.Request { - reqHeader := http.Header{} - action := "" - formData := url.Values{} - root, _ := html.Parse(bytes.NewBuffer(responseBytes)) - visit(root, func(n *html.Node) { - if n.Type == html.ElementNode && n.Data == "input" { - inputType := getAttr(n, "type") - if inputType == "hidden" { - name := getAttr(n, "name") - value := getAttr(n, "value") - formData.Add(name, value) - } - } - if n.Type == html.ElementNode && n.Data == "form" { - action = getAttr(n, "action") - } - }) - - formData.Add("username", username) - formData.Add("password", password) - reqHeader.Set("Content-Type", "application/x-www-form-urlencoded") - reqBody := strings.NewReader(formData.Encode()) - reqURL, _ := currentURL.Parse(action) - req, _ := http.NewRequest("POST", reqURL.String(), reqBody) - req.Header = reqHeader - return req -} - -func submitGrantForm(response *http.Response) *http.Request { - reqHeader := http.Header{} - action := "" - formData := url.Values{} - currentURL := response.Request.URL - responseBytes, err := ioutil.ReadAll(response.Body) - if err != nil { - return nil - } - root, err := html.Parse(bytes.NewBuffer(responseBytes)) - visit(root, func(n *html.Node) { - if n.Type == html.ElementNode && n.Data == "input" { - inputType := getAttr(n, "type") - if inputType == "hidden" || inputType == "checkbox" || inputType == "radio" { - name := getAttr(n, "name") - value := getAttr(n, "value") - formData.Add(name, value) - } - } - if n.Type == html.ElementNode && n.Data == "form" { - action = getAttr(n, "action") - } - }) - formData.Add("approve", "Allow selected permissions") - reqHeader.Set("Content-Type", "application/x-www-form-urlencoded") - reqBody := strings.NewReader(formData.Encode()) - reqURL, _ := currentURL.Parse(action) - req, _ := http.NewRequest("POST", reqURL.String(), reqBody) - req.Header = reqHeader - return req -} - -func getAttr(element *html.Node, attrName string) string { - for _, attr := range element.Attr { - if attr.Key == attrName { - return attr.Val - } - } - return "" -} - -func visit(n *html.Node, visitor func(*html.Node)) { - visitor(n) - for c := n.FirstChild; c != nil; c = c.NextSibling { - visit(c, visitor) - } -} From 233cfd4009ae9050af0f95639bf1d1ff8888abb4 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Thu, 16 Jan 2020 15:10:08 -0600 Subject: [PATCH 4/6] E2E token test more reliable Signed-off-by: Ruben Vargas --- .../elasticsearch_token_propagation_test.go | 111 ++++++++++++------ test/e2e/utils.go | 2 +- 2 files changed, 74 insertions(+), 39 deletions(-) diff --git a/test/e2e/elasticsearch_token_propagation_test.go b/test/e2e/elasticsearch_token_propagation_test.go index 12c55f765..ed7bca5ab 100644 --- a/test/e2e/elasticsearch_token_propagation_test.go +++ b/test/e2e/elasticsearch_token_propagation_test.go @@ -15,7 +15,6 @@ import ( framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/uber/jaeger-client-go/config" @@ -34,20 +33,21 @@ import ( // Test parameters const name = "token-prop" -const username = "user-test-token" -const password = "any" const collectorPodImageName = "jaeger-collector" const testServiceName = "token-propagation" -const test_account = "token-test-user" +const testAccount = "token-test-user" type TokenTestSuite struct { suite.Suite - exampleJaeger *v1.Jaeger - queryName string - collectorName string - queryServiceEndPoint string - host string - token string + exampleJaeger *v1.Jaeger + queryName string + collectorName string + queryServiceEndPoint string + host string + token string + testServiceAccount *corev1.ServiceAccount + testRoleBinding *rbac.ClusterRoleBinding + delegationRoleBinding *rbac.ClusterRoleBinding } func (suite *TokenTestSuite) SetupSuite() { @@ -55,13 +55,13 @@ func (suite *TokenTestSuite) SetupSuite() { if !isOpenShift(t) { t.Skipf("Test %s is currently supported only on OpenShift because es-operator runs only on OpenShift\n", t.Name()) } - assert.NoError(t, framework.AddToFrameworkScheme(apis.AddToScheme, &v1.JaegerList{ + require.NoError(t, framework.AddToFrameworkScheme(apis.AddToScheme, &v1.JaegerList{ TypeMeta: metav1.TypeMeta{ Kind: "Jaeger", APIVersion: "jaegertracing.io/v1", }, })) - assert.NoError(t, framework.AddToFrameworkScheme(apis.AddToScheme, &esv1.ElasticsearchList{ + require.NoError(t, framework.AddToFrameworkScheme(apis.AddToScheme, &esv1.ElasticsearchList{ TypeMeta: metav1.TypeMeta{ Kind: "Elasticsearch", APIVersion: "logging.openshift.io/v1", @@ -82,7 +82,27 @@ func (suite *TokenTestSuite) SetupSuite() { suite.deployJaegerWithPropagationEnabled() } +func (suite *TokenTestSuite) cleanAccountBindings() { + if !debugMode || !t.Failed() { + err := fw.Client.Delete(goctx.Background(), suite.testServiceAccount) + require.NoError(t, err, "Error deleting test service account") + err = e2eutil.WaitForDeletion(t, fw.Client.Client, suite.testServiceAccount, retryInterval, timeout) + require.NoError(t, err) + + err = fw.Client.Delete(goctx.Background(), suite.testRoleBinding) + require.NoError(t, err, "Error deleting test service account bindings") + err = e2eutil.WaitForDeletion(t, fw.Client.Client, suite.testRoleBinding, retryInterval, timeout) + require.NoError(t, err) + + err = fw.Client.Delete(goctx.Background(), suite.delegationRoleBinding) + require.NoError(t, err, "Error deleting delegation bindings") + err = e2eutil.WaitForDeletion(t, fw.Client.Client, suite.delegationRoleBinding, retryInterval, timeout) + require.NoError(t, err) + } +} + func (suite *TokenTestSuite) TearDownSuite() { + suite.cleanAccountBindings() undeployJaegerInstance(suite.exampleJaeger) handleSuiteTearDown() } @@ -95,10 +115,18 @@ func (suite *TokenTestSuite) TestTokenPropagationNoToken() { } err := wait.Poll(retryInterval, timeout, func() (done bool, err error) { req, err := http.NewRequest(http.MethodGet, suite.queryServiceEndPoint, nil) + if err != nil { + return false, err + } resp, err := client.Do(req) defer resp.Body.Close() - require.Equal(t, http.StatusForbidden, resp.StatusCode) - return true, nil + if err != nil { + return false, err + } + if resp.StatusCode == http.StatusForbidden { + return true, nil + } + return false, errors.New(fmt.Sprintf("query service return http code: %d", resp.StatusCode)) }) require.NoError(t, err, "Token propagation test failed") } @@ -127,19 +155,25 @@ func (suite *TokenTestSuite) TestTokenPropagationValidToken() { /* Try to reach query endpoint */ err = wait.Poll(retryInterval, timeout, func() (done bool, err error) { req, err := http.NewRequest(http.MethodGet, suite.queryServiceEndPoint, nil) + if err != nil { + return false, err + } req.Header.Add("Authorization", "Bearer "+suite.token) resp, err := client.Do(req) defer resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) - if resp.StatusCode != http.StatusOK { - return false, errors.New("Query service returns http code: " + string(resp.StatusCode)) + if err != nil { + return false, err } - bodyBytes, err := ioutil.ReadAll(resp.Body) - bodyString := string(bodyBytes) - if !strings.Contains(bodyString, "errors\":null") { - return false, errors.New("query service returns errors: " + bodyString) + if resp.StatusCode == http.StatusOK { + bodyBytes, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + bodyString := string(bodyBytes) + if !strings.Contains(bodyString, "errors\":null") { + return false, errors.New("query service returns errors: " + bodyString) + } + return true, nil } - return true, nil + return false, errors.New(fmt.Sprintf("query service return http code: %d", resp.StatusCode)) }) require.NoError(t, err, "Token propagation test failed") } @@ -147,10 +181,11 @@ func (suite *TokenTestSuite) TestTokenPropagationValidToken() { func (suite *TokenTestSuite) deployJaegerWithPropagationEnabled() { queryName := fmt.Sprintf("%s-query", name) collectorName := fmt.Sprintf("%s-collector", name) - bindOperatorWithAuthDelegator() - createTestServiceAccount() + suite.bindOperatorWithAuthDelegator() + suite.createTestServiceAccount() suite.token = testAccountToken() - + require.NotEmpty(t, suite.token) + println(suite.token) suite.exampleJaeger = jaegerInstance() err := fw.Client.Create(goctx.Background(), suite.exampleJaeger, @@ -177,8 +212,8 @@ func TestTokenSuite(t *testing.T) { suite.Run(t, new(TokenTestSuite)) } -func bindOperatorWithAuthDelegator() { - roleBinding := rbac.ClusterRoleBinding{ +func (suite *TokenTestSuite) bindOperatorWithAuthDelegator() { + suite.delegationRoleBinding = &rbac.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "jaeger-operator:system:auth-delegator", Namespace: namespace, @@ -194,7 +229,7 @@ func bindOperatorWithAuthDelegator() { }, } err := fw.Client.Create(goctx.Background(), - &roleBinding, + suite.delegationRoleBinding, &framework.CleanupOptions{ TestContext: ctx, Timeout: timeout, @@ -203,16 +238,16 @@ func bindOperatorWithAuthDelegator() { require.NoError(t, err, "Error binding operator service account with auth-delegator") } -func createTestServiceAccount() { +func (suite *TokenTestSuite) createTestServiceAccount() { - serviceAccount := corev1.ServiceAccount{ + suite.testServiceAccount = &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ - Name: test_account, + Name: testAccount, Namespace: namespace, }, } err := fw.Client.Create(goctx.Background(), - &serviceAccount, + suite.testServiceAccount, &framework.CleanupOptions{ TestContext: ctx, Timeout: timeout, @@ -220,14 +255,14 @@ func createTestServiceAccount() { }) require.NoError(t, err, "Error deploying example Jaeger") - roleBinding := rbac.ClusterRoleBinding{ + suite.testRoleBinding = &rbac.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ - Name: test_account + "-bind", + Name: testAccount + "-bind", Namespace: namespace, }, Subjects: []rbac.Subject{{ Kind: "ServiceAccount", - Name: serviceAccount.Name, + Name: suite.testServiceAccount.Name, Namespace: namespace, }}, RoleRef: rbac.RoleRef{ @@ -237,7 +272,7 @@ func createTestServiceAccount() { } err = fw.Client.Create(goctx.Background(), - &roleBinding, + suite.testRoleBinding, &framework.CleanupOptions{ TestContext: ctx, Timeout: timeout, @@ -253,13 +288,13 @@ func testAccountToken() string { serviceAccount := corev1.ServiceAccount{} e := fw.Client.Get(goctx.Background(), types.NamespacedName{ Namespace: namespace, - Name: test_account, + Name: testAccount, }, &serviceAccount) if e != nil { return false, e } for _, s := range serviceAccount.Secrets { - if strings.HasPrefix(s.Name, test_account+"-token") { + if strings.HasPrefix(s.Name, testAccount+"-token") { secretName = s.Name return true, nil } diff --git a/test/e2e/utils.go b/test/e2e/utils.go index cf1b70cda..e739c6f6f 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -36,7 +36,7 @@ import ( var ( retryInterval = time.Second * 5 - timeout = time.Minute * 2 + timeout = time.Minute * 5 storageNamespace = os.Getenv("STORAGE_NAMESPACE") kafkaNamespace = os.Getenv("KAFKA_NAMESPACE") debugMode = getBoolEnv("DEBUG_MODE", false) From 93df3af9b49a0c3705222b254ed22025eb1a469a Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Thu, 16 Jan 2020 15:29:35 -0600 Subject: [PATCH 5/6] Allow override es.archive.tls Signed-off-by: Ruben Vargas --- pkg/inject/oauth_proxy.go | 2 +- pkg/storage/elasticsearch.go | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/inject/oauth_proxy.go b/pkg/inject/oauth_proxy.go index c77152a88..2b9e60547 100644 --- a/pkg/inject/oauth_proxy.go +++ b/pkg/inject/oauth_proxy.go @@ -39,7 +39,7 @@ func OAuthProxy(jaeger *v1.Jaeger, dep *appsv1.Deployment) *appsv1.Deployment { func proxyInitArguments(jaeger *v1.Jaeger) []string { secret, err := util.GenerateProxySecret() if err != nil { - jaeger.Logger().Warnf("Error generating secret: %s, fallback to fixed secret", secret) + jaeger.Logger().WithError(err).Warnf("Error generating secret: %s, fallback to fixed secret", secret) secret = defaultProxySecret } args := []string{ diff --git a/pkg/storage/elasticsearch.go b/pkg/storage/elasticsearch.go index f1ad3602f..7d5699031 100644 --- a/pkg/storage/elasticsearch.go +++ b/pkg/storage/elasticsearch.go @@ -65,7 +65,11 @@ func (ed *ElasticsearchDeployment) injectArguments(container *corev1.Container) if strings.EqualFold(util.FindItem("--es-archive.enabled", container.Args), "--es-archive.enabled=true") { container.Args = append(container.Args, "--es-archive.server-urls="+elasticsearchURL, - "--es-archive.tls=true", + ) + if util.FindItem("--es-archive.tls=", container.Args) == "" { + container.Args = append(container.Args, "--es-archive.tls=true") + } + container.Args = append(container.Args, "--es-archive.tls.ca="+caPath, "--es-archive.tls.cert="+certPath, "--es-archive.tls.key="+keyPath, From 0bb2711966ae0179a28a81a1c8b53803584220c2 Mon Sep 17 00:00:00 2001 From: Ruben Vargas Date: Thu, 16 Jan 2020 15:34:04 -0600 Subject: [PATCH 6/6] Address formating comments Signed-off-by: Ruben Vargas --- pkg/storage/elasticsearch.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/storage/elasticsearch.go b/pkg/storage/elasticsearch.go index 7d5699031..fb524a5f8 100644 --- a/pkg/storage/elasticsearch.go +++ b/pkg/storage/elasticsearch.go @@ -39,12 +39,10 @@ type ElasticsearchDeployment struct { } func (ed *ElasticsearchDeployment) injectArguments(container *corev1.Container) { - container.Args = append(container.Args, - "--es.server-urls="+elasticsearchURL) + container.Args = append(container.Args, "--es.server-urls="+elasticsearchURL) if util.FindItem("--es.tls=", container.Args) == "" { container.Args = append(container.Args, "--es.tls=true") } - container.Args = append(container.Args, "--es.tls.ca="+caPath, "--es.tls.cert="+certPath,