Race condition with services instantiation in modules

[tpaschalis]

What's wrong?

While config blocks are not permitted to be used inside of a module definition, this is not the case for service blocks.

This means, that for example an http block can be instantiated and evaluated within a module, but it still refers to the original shared service of the root module. This can lead to inconsistent behavior, depending on the order of evaluation.

This also means that a module loader can accidentally impact the functionality of the root controller, or other module loaders.

A potential solution would be to restrict the definition of service blocks to the root configuration only; another solution would be to allow modules to instantiate and use their own instance of a service if they provide a block.

cc @rfratto as the original author of services and @wildum as part of his work on #5968.

Steps to reproduce

Use the configuration like the following, and try to reload the configuration.

Depending on the order of evaluation, the reload might either go through or fail with Client sent an HTTP request to an HTTPS server.

System information

macOS

Software version

main (2e41841)

Configuration

/* http { */
	/* tls { */
	/* 	cert_file = "/tmp/cert" */
	/* 	key_file = "/tmp/key" */
	/* } */
/* } */


module.string "foo" {
	content = `
http {
	tls {
		cert_file = "/tmp/cert2"
		key_file = "/tmp/key2"
	}
}
`
}

Logs

Logs show the node_id=http evaluation order from different controller_ids

## Run the agent with the provided config

ts=2024-01-17T10:00:59.392539Z level=info msg="starting complete graph evaluation" controller_id="" trace_id=c77e20c4346d984cc815bf9dc0f5f695
ts=2024-01-17T10:00:59.392604Z level=info msg="starting complete graph evaluation" controller_id=module.string.foo trace_id=12c3b45044ec82895d488cacc8def5ac
ts=2024-01-17T10:00:59.392639Z level=info msg="applying TLS config to HTTP server" service=http
ts=2024-01-17T10:00:59.392642Z level=info msg="finished node evaluation" controller_id=module.string.foo trace_id=12c3b45044ec82895d488cacc8def5ac node_id=http duration=21.625µs
ts=2024-01-17T10:00:59.392707Z level=info msg="finished complete graph evaluation" controller_id=module.string.foo trace_id=12c3b45044ec82895d488cacc8def5ac duration=118.458µs
ts=2024-01-17T10:00:59.392715Z level=info msg="finished node evaluation" controller_id="" trace_id=c77e20c4346d984cc815bf9dc0f5f695 node_id=module.string.foo duration=162.75µs
ts=2024-01-17T10:00:59.392722Z level=info msg="finished node evaluation" controller_id="" trace_id=c77e20c4346d984cc815bf9dc0f5f695 node_id=logging duration=1.833µs
ts=2024-01-17T10:00:59.39274Z level=info msg="finished node evaluation" controller_id="" trace_id=c77e20c4346d984cc815bf9dc0f5f695 node_id=tracing duration=2.041µs
ts=2024-01-17T10:00:59.392759Z level=info msg="applying non-TLS config to HTTP server" service=http
ts=2024-01-17T10:00:59.392762Z level=info msg="finished node evaluation" controller_id="" trace_id=c77e20c4346d984cc815bf9dc0f5f695 node_id=http duration=3.959µs

$ curl localhost:12345/-/reload
config reloaded

## Close and re-run the agent with the same config

ts=2024-01-17T10:00:52.595619Z level=info msg="starting complete graph evaluation" controller_id="" trace_id=d0f6b0f968fb2ea32940d65c40728962
ts=2024-01-17T10:00:52.596152Z level=info msg="applying non-TLS config to HTTP server" service=http
ts=2024-01-17T10:00:52.596155Z level=info msg="finished node evaluation" controller_id="" trace_id=d0f6b0f968fb2ea32940d65c40728962 node_id=http duration=4.917µs
ts=2024-01-17T10:00:52.596168Z level=info msg="finished node evaluation" controller_id="" trace_id=d0f6b0f968fb2ea32940d65c40728962 node_id=tracing duration=1.167µs
ts=2024-01-17T10:00:52.596263Z level=info msg="starting complete graph evaluation" controller_id=module.string.foo trace_id=32803262f546b779c0afea4727177d19
ts=2024-01-17T10:00:52.596295Z level=info msg="applying TLS config to HTTP server" service=http
ts=2024-01-17T10:00:52.596301Z level=info msg="finished node evaluation" controller_id=module.string.foo trace_id=32803262f546b779c0afea4727177d19 node_id=http duration=19.333µs

$ curl localhost:12345/-/reload
Client sent an HTTP request to an HTTPS server.

[rfratto]

It's definitely not intentional that modules can configure services. They're supposed to act like any other config block and only be configurable from the main config.

[wildum]

The fix should be easy, I can take care of it