Allow Fleet Server to be run without TLS. #6020

naemono · 2022-09-20T13:54:18Z

This potential change allows Fleet Server, running as Elastic Agent, to be run without TLS for such scenarios as running within a service mesh which could provide mTLS between applications/services, similar to how we allow Elasticsearch, and Kibana to run without TLS.

This additionally solves a panic when currently attempting to disable TLS on Fleet server:

[elastic-operator-0] {"log.level":"info","@timestamp":"2022-09-14T20:10:55.031Z","log.logger":"manager.eck-operator","message":"Observed a panic in reconciler: runtime error: invalid memory address or nil pointer dereference","service.version":"2.4.0+96282ca9","service.type":"eck","ecs.version":"1.4.0","controller":"agent-controller","object":{"name":"eck-fleet-eck-fleet-server","namespace":"istio-enabled"},"namespace":"istio-enabled","name":"eck-fleet-eck-fleet-server","reconcileID":"c0330ea8-bda6-48d5-8552-d25327c3959e"}
[elastic-operator-0] panic: runtime error: invalid memory address or nil pointer dereference [recovered]
[elastic-operator-0] 	panic: runtime error: invalid memory address or nil pointer dereference
[elastic-operator-0] [signal SIGSEGV: segmentation violation code=0x1 addr=0x120 pc=0x1769409]
[elastic-operator-0]
[elastic-operator-0] goroutine 2739 [running]:
[elastic-operator-0] sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile.func1()
[elastic-operator-0] 	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.2/pkg/internal/controller/controller.go:118 +0x1f4
[elastic-operator-0] panic({0x1a81660, 0x2de94a0})
[elastic-operator-0] 	/usr/local/go/src/runtime/panic.go:838 +0x207
[elastic-operator-0] github.com/elastic/cloud-on-k8s/v2/pkg/controller/agent.internalReconcile({{0x2017448, 0xc00180b110}, {0x201e198, 0xc00020c820}, {0x2016768, 0xc0007c2b80}, {0xc0007bff20, 0xc0007bff80, 0xc0007da000, 0xc0007da060}, ...})
[elastic-operator-0] 	/go/src/github.com/elastic/cloud-on-k8s/pkg/controller/agent/driver.go:130 +0xe89
[elastic-operator-0] github.com/elastic/cloud-on-k8s/v2/pkg/controller/agent.(*ReconcileAgent).doReconcile(_, {_, _}, {{{0x18b3b85, 0x5}, {0xc0017f21a0, 0x1d}}, {{0xc0006eb800, 0x1a}, {0x0, ...}, ...}, ...})
[elastic-operator-0] 	/go/src/github.com/elastic/cloud-on-k8s/pkg/controller/agent/controller.go:181 +0x585
[elastic-operator-0] github.com/elastic/cloud-on-k8s/v2/pkg/controller/agent.(*ReconcileAgent).Reconcile(0xc000772f20, {0x2017448?, 0xc00180b020?}, {{{0xc00076ebb0?, 0x10?}, {0xc000213500?, 0x40d927?}}})
[elastic-operator-0] 	/go/src/github.com/elastic/cloud-on-k8s/pkg/controller/agent/controller.go:147 +0x478
[elastic-operator-0] sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Reconcile(0x20173a0?, {0x2017448?, 0xc00180b020?}, {{{0xc00076ebb0?, 0x1c017c0?}, {0xc000213500?, 0x4041f4?}}})
[elastic-operator-0] 	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.2/pkg/internal/controller/controller.go:121 +0xc8
[elastic-operator-0] sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc0004a2d20, {0x20173a0, 0xc000634a80}, {0x1b01ee0?, 0xc0004d1be0?})
[elastic-operator-0] 	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.2/pkg/internal/controller/controller.go:320 +0x33c
[elastic-operator-0] sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc0004a2d20, {0x20173a0, 0xc000634a80})
[elastic-operator-0] 	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.2/pkg/internal/controller/controller.go:273 +0x1d9
[elastic-operator-0] sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2()
[elastic-operator-0] 	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.2/pkg/internal/controller/controller.go:234 +0x85
[elastic-operator-0] created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2
[elastic-operator-0] 	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.12.2/pkg/internal/controller/controller.go:230 +0x325
^C[1]    43051 terminated  kubetail -n elastic-system -l app.kubernetes.io/instance=elastic-operator  1m

Also, an e2e test was added to ensure the full functionality all the way back to data within Elasticsearch.

E2e test verifying Fleet server + Agent w/o TLS. Unit test for Fleet Server pod template without TLS.

naemono · 2022-09-20T14:23:30Z

run/e2e-tests

naemono · 2022-09-22T14:32:25Z

note. Need to test this against an older version of the stack to ensure that this works against older version, or restrict this to newer versions of the stack if not functional against older versions.

CheyenneForbes · 2022-09-24T19:04:38Z

+1 for this functionality

pebrc

Code looks good, just a few nits. But I have questions about the practicality of running Elastic Agent through a service mesh like Istio. Many of our recipes use hostNetwork: true for the Agents but that is a configuration that is not supported by Istio sidecar injection. Maybe I am missing a clever workaround for this issue?

pebrc · 2022-09-26T09:38:35Z

pkg/controller/agent/pod.go

@@ -507,6 +520,16 @@ func getFleetSetupFleetEnvVars(_ context.Context, agent agentv1alpha1.Agent, cli
 			return nil, err
 		}
 		fleetCfg[FleetURL] = assocConf.GetURL()
+
+		u, err := url.Parse(fleetCfg[FleetURL])


This parsing seems redundant. You could either look at agent.Spec.HTTP.Protocol() once more which is used to construct the Fleet URL IIUC. Or look at agent.Spec.HTTP.TLS.Enabled() Or am I missing something important?

Very true. I'd forgotten about his method on the spec. Fixed.

Turns out that it's the associated agent that we need to inspect (the fleet agent). I updated this to just strings.Contains(url, "https://") to test if the associated fleet is running with/without tls. Lmk your thoughts.

You are right I missed this. I think the downside of deriving this from the URL is that we effectively disable the safeguard the Fleet team has built in here. They could have just done the same thing and look at the URL to decide whether they allow Agent to talk to an insecure Fleet server. But they opted to add another configuration setting to make this an explicit decision.

pebrc · 2022-09-26T18:58:35Z

pkg/controller/agent/pod.go

+				))
+		} else {
+			// Force FLEET_SERVER_HOST environment variable to Pod IP, as without this, Fleet Server only binds to localhost.
+			builder = builder.WithEnv(corev1.EnvVar{Name: "FLEET_SERVER_HOST", Value: "", ValueFrom: &corev1.EnvVarSource{


Should FLEET_SERVER_HOST be a constant?

Also the need for this only in HTTP mode confuses me. In my experiments I did not seem to need this even with Istio enabled.

Should this be inside the existing applyEnvVars function?

Yes it should. Fixed.

Absolutely. I've moved that into the function.

Longer answer. Let me test this one additional time. IIRC, everything worked up until I started trying to use an actual agent attached to fleet. Let me re-verify.

After testing 3: It turns out that environment variable isn't needed after all. I've removed it, and updated our agent tests to use actual URLs in the tests instead of "url" or similar.

Don't use url.Parse, just use agent.Spec.HTTP.Protocol(). Move envvar addition to applyEnvVars function. Fix wording.

…unneeded.

Add insecure flag when agent is running with associated fleet running without tls

pkg/controller/agent/pod.go

pebrc · 2022-09-30T08:03:25Z

pkg/controller/agent/pod.go

@@ -507,6 +520,16 @@ func getFleetSetupFleetEnvVars(_ context.Context, agent agentv1alpha1.Agent, cli
 			return nil, err
 		}
 		fleetCfg[FleetURL] = assocConf.GetURL()
+
+		u, err := url.Parse(fleetCfg[FleetURL])


You are right I missed this. I think the downside of deriving this from the URL is that we effectively disable the safeguard the Fleet team has built in here. They could have just done the same thing and look at the URL to decide whether they allow Agent to talk to an insecure Fleet server. But they opted to add another configuration setting to make this an explicit decision.

Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>

Allow Fleet Server to be run without TLS.

faafb89

E2e test verifying Fleet server + Agent w/o TLS. Unit test for Fleet Server pod template without TLS.

naemono added >bug Something isn't working >enhancement Enhancement of existing functionality labels Sep 20, 2022

pebrc reviewed Sep 26, 2022

View reviewed changes

naemono added 5 commits September 28, 2022 13:46

Make FLEET_SERVER_HOST a constant.

6138a95

Don't use url.Parse, just use agent.Spec.HTTP.Protocol(). Move envvar addition to applyEnvVars function. Fix wording.

Test to see if it works without FLEET_SERVER_HOST set.

5a7fab8

strings.Contains on fleet URL to set insecure.

77e3a40

Remove FleetServerHost and associated env settings, and was, indeed, …

7ebd030

…unneeded.

Test with actual urls in agent+fleet.

9260d3c

Add insecure flag when agent is running with associated fleet running without tls

pebrc reviewed Sep 30, 2022

View reviewed changes

pebrc approved these changes Sep 30, 2022

View reviewed changes

Use HasPrefix instead of Contains

af4726e

Co-authored-by: Peter Brachwitz <peter.brachwitz@gmail.com>

pebrc mentioned this pull request Oct 3, 2022

Crash of ECK Operator after disabling TLS on elastic agent in fleet mode #6062

Closed

naemono merged commit 90b8499 into elastic:main Oct 4, 2022

pebrc added the v2.6.0 label Oct 5, 2022

thbkrkr mentioned this pull request Jun 26, 2023

TestFleetAgentWithoutTLS/ES_data_should_pass_validations flaky in 8-0-1 #6956

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Fleet Server to be run without TLS. #6020

Allow Fleet Server to be run without TLS. #6020

naemono commented Sep 20, 2022

naemono commented Sep 20, 2022

naemono commented Sep 22, 2022

CheyenneForbes commented Sep 24, 2022

pebrc left a comment •

edited

Loading

pebrc Sep 26, 2022

naemono Sep 28, 2022

naemono Sep 29, 2022

pebrc Sep 30, 2022

pebrc Sep 26, 2022 •

edited

Loading

naemono Sep 28, 2022

naemono Sep 29, 2022

pebrc Sep 30, 2022

Allow Fleet Server to be run without TLS. #6020

Allow Fleet Server to be run without TLS. #6020

Conversation

naemono commented Sep 20, 2022

naemono commented Sep 20, 2022

naemono commented Sep 22, 2022

CheyenneForbes commented Sep 24, 2022

pebrc left a comment • edited Loading

Choose a reason for hiding this comment

pebrc Sep 26, 2022

Choose a reason for hiding this comment

naemono Sep 28, 2022

Choose a reason for hiding this comment

naemono Sep 29, 2022

Choose a reason for hiding this comment

pebrc Sep 30, 2022

Choose a reason for hiding this comment

pebrc Sep 26, 2022 • edited Loading

Choose a reason for hiding this comment

naemono Sep 28, 2022

Choose a reason for hiding this comment

naemono Sep 29, 2022

Choose a reason for hiding this comment

pebrc Sep 30, 2022

Choose a reason for hiding this comment

pebrc left a comment •

edited

Loading

pebrc Sep 26, 2022 •

edited

Loading