adapt tls flag usage for upcoming release (#1891)

Signed-off-by: Benedikt Bongartz <[email protected]>
jaegertracing · May 17, 2022 · d1e725b · d1e725b
1 parent f85df5b
commit d1e725b
Show file tree

Hide file tree

Showing 3 changed files with 168 additions and 0 deletions.
diff --git a/apis/v1/jaeger_webhook.go b/apis/v1/jaeger_webhook.go
@@ -3,6 +3,7 @@ package v1
 import (
 	"context"
 	"fmt"
+	"regexp"
 
 	esv1 "github.com/openshift/elasticsearch-operator/apis/logging/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -36,6 +37,13 @@ func (j *Jaeger) SetupWebhookWithManager(mgr ctrl.Manager) error {
 
 var _ webhook.Defaulter = &Jaeger{}
 
+func (j *Jaeger) objsWithOptions() []*Options {
+	return []*Options{
+		&j.Spec.AllInOne.Options, &j.Spec.Query.Options, &j.Spec.Collector.Options,
+		&j.Spec.Ingester.Options, &j.Spec.Agent.Options, &j.Spec.Storage.Options,
+	}
+}
+
 // Default implements webhook.Defaulter so a webhook will be registered for the type
 func (j *Jaeger) Default() {
 	jaegerlog.Info("default", "name", j.Name)
@@ -56,6 +64,20 @@ func (j *Jaeger) Default() {
 		}
 		j.Spec.Storage.Elasticsearch.NodeCount = OpenShiftElasticsearchNodeCount(es.Spec)
 	}
+
+	for _, opt := range j.objsWithOptions() {
+		optCopy := opt.DeepCopy()
+		if f := getAdditionalTLSFlags(optCopy.ToArgs()); f != nil {
+			newOpts := optCopy.GenericMap()
+			for k, v := range f {
+				newOpts[k] = v
+			}
+
+			if err := opt.parse(newOpts); err != nil {
+				jaegerlog.Error(err, "name", j.Name, "method", "Option.Parse")
+			}
+		}
+	}
 }
 
 // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
@@ -84,6 +106,14 @@ func (j *Jaeger) ValidateUpdate(_ runtime.Object) error {
 			return fmt.Errorf("elasticsearch instance not found: %v", err)
 		}
 	}
+
+	for _, opt := range j.objsWithOptions() {
+		got := opt.DeepCopy().ToArgs()
+		if f := getAdditionalTLSFlags(got); f != nil {
+			return fmt.Errorf("tls flags incomplete, got: %v", got)
+		}
+	}
+
 	return nil
 }
 
@@ -110,3 +140,28 @@ func ShouldInjectOpenShiftElasticsearchConfiguration(s JaegerStorageSpec) bool {
 	_, ok := s.Options.Map()["es.server-urls"]
 	return !ok
 }
+
+var (
+	tlsFlag          = regexp.MustCompile("--.*tls.*=")
+	tlsFlagIdx       = regexp.MustCompile("--.*tls")
+	tlsEnabledExists = regexp.MustCompile("--.*tls.enabled")
+)
+
+// getAdditionalTLSFlags returns additional tls arguments based on the argument
+// list. If no additional argument is needed, nil is returned.
+func getAdditionalTLSFlags(args []string) map[string]interface{} {
+	var res map[string]interface{}
+	for _, arg := range args {
+		a := []byte(arg)
+		if tlsEnabledExists.Match(a) {
+			// NOTE: if flag exists, we are done.
+			return nil
+		}
+		if tlsFlag.Match(a) && res == nil {
+			idx := tlsFlagIdx.FindIndex(a)
+			res = make(map[string]interface{})
+			res[arg[idx[0]+2:idx[1]]+".enabled"] = "true"
+		}
+	}
+	return res
+}
diff --git a/apis/v1/jaeger_webhook_test.go b/apis/v1/jaeger_webhook_test.go
@@ -11,6 +11,7 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -127,6 +128,39 @@ func TestDefault(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "missing tls enable flag",
+			j: &Jaeger{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "project1",
+				},
+				Spec: JaegerSpec{
+					Storage: JaegerStorageSpec{
+						Type:    JaegerMemoryStorage,
+						Options: NewOptions(map[string]interface{}{"stuff.tls.test": "something"}),
+					},
+				},
+			},
+			expected: &Jaeger{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "project1",
+				},
+				Spec: JaegerSpec{
+					Storage: JaegerStorageSpec{
+						Type: JaegerMemoryStorage,
+						Options: NewOptions(
+							map[string]interface{}{
+								"stuff.tls.test":    "something",
+								"stuff.tls.enabled": "true",
+							},
+						),
+						Elasticsearch: ElasticsearchSpec{
+							Name: defaultElasticsearchName,
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -142,6 +176,10 @@ func TestDefault(t *testing.T) {
 	}
 }
 
+func TestValidateDelete(t *testing.T) {
+	assert.Nil(t, new(Jaeger).ValidateDelete())
+}
+
 func TestValidate(t *testing.T) {
 	tests := []struct {
 		name         string
@@ -211,6 +249,23 @@ func TestValidate(t *testing.T) {
 			},
 			err: `elasticsearch instance not found: elasticsearchs.logging.openshift.io "my-es" not found`,
 		},
+		{
+			name: "missing tls options",
+			current: &Jaeger{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "project1",
+				},
+				Spec: JaegerSpec{
+					Storage: JaegerStorageSpec{
+						Options: NewOptions(map[string]interface{}{
+							"something.tls.else": "fails",
+						}),
+						Type: JaegerMemoryStorage,
+					},
+				},
+			},
+			err: `tls flags incomplete, got: [--something.tls.else=fails]`,
+		},
 	}
 
 	for _, test := range tests {
@@ -222,6 +277,7 @@ func TestValidate(t *testing.T) {
 
 			err := test.current.ValidateCreate()
 			if test.err != "" {
+				assert.NotNil(t, err)
 				assert.Equal(t, test.err, err.Error())
 			} else {
 				assert.Nil(t, err)
@@ -246,3 +302,59 @@ func TestShouldDeployElasticsearch(t *testing.T) {
 		})
 	}
 }
+
+func TestGetAdditionalTLSFlags(t *testing.T) {
+	tt := []struct {
+		name   string
+		args   []string
+		expect map[string]interface{}
+	}{
+		{
+			name:   "no tls flag",
+			args:   []string{"--something.else"},
+			expect: nil,
+		},
+		{
+			name:   "already enabled",
+			args:   []string{"--something.tls.enabled=true", "--something.tls.else=abc"},
+			expect: nil,
+		},
+		{
+			name:   "is disabled",
+			args:   []string{"--tls.enabled=false", "--something.else", "--something.tls.else=abc"},
+			expect: nil,
+		},
+		{
+			name: "must be enabled",
+			args: []string{"--something.tls.else=abc"},
+			expect: map[string]interface{}{
+				"something.tls.enabled": "true",
+			},
+		},
+		{
+			// NOTE: we want to avoid something like:
+			// --kafka.consumer.authentication=tls.enabled=true
+			name: "enable consumer tls",
+			args: []string{
+				"--es.server-urls=http://elasticsearch:9200",
+				"--kafka.consumer.authentication=tls",
+				"--kafka.consumer.brokers=my-cluster-kafka-bootstrap:9093",
+				"--kafka.consumer.tls.ca=/var/run/secrets/cluster-ca/ca.crt",
+				"--kafka.consumer.tls.cert=/var/run/secrets/kafkauser/user.crt",
+				"--kafka.consumer.tls.key=/var/run/secrets/kafkauser/user.key",
+			},
+			expect: map[string]interface{}{
+				"kafka.consumer.tls.enabled": "true",
+			},
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			got := getAdditionalTLSFlags(tc.args)
+			if !cmp.Equal(tc.expect, got) {
+				t.Error("err:", cmp.Diff(tc.expect, got))
+			}
+		})
+	}
+}
diff --git a/apis/v1/options.go b/apis/v1/options.go
@@ -88,6 +88,7 @@ func (o Options) MarshalJSON() ([]byte, error) {
 }
 
 func (o *Options) parse(entries map[string]interface{}) error {
+	o.json = nil
 	o.opts = make(map[string]interface{})
 	var err error
 	for k, v := range entries {