Prevent memory leak in headers.

[distortedsignal]

Signed-off-by: Tom Kelley distortedsignal@gmail.com

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area outputs

What this PR does / why we need it:

Somebody made a PR that contained a memory leak for most of the outputs in the Falcosidekick. Since the header list was never reset and the output objects are long-lived (they're not recreated for each POST), headers would populate across request boundaries. For most outputs, this would be a major problem. For the Kubeless output, this is a critical problem since an event-id is generated for each event/POST request.

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

I'm sorry that this happened, and I'll try not to let it happen again.

[Issif]

This a a major issue, I'll review it tonight and release 2.23.1 with your fix if it's OK.

[Issif]

I quickly red your fix, and I think we should call BasicAuth() function at client init, and not in Post() and that will avoid to add authentication Headers at each Post, wdyt?

[Issif]

Something like:

	if config.Elasticsearch.HostPort != "" {
		var err error
		elasticsearchClient, err = outputs.NewClient("Elasticsearch", config.Elasticsearch.HostPort+"/"+config.Elasticsearch.Index+"/"+config.Elasticsearch.Type, config.Elasticsearch.MutualTLS, config.Elasticsearch.CheckCert, config, stats, promStats, statsdClient, dogstatsdClient)
		if err != nil {
			config.Elasticsearch.HostPort = ""
		} else {
		    if config.Elasticsearch.Username != "" && config.Elasticsearch.Password != "" {
	      	  elasticsearchClientc.BasicAuth(config.Elasticsearch.Username, config.Elasticsearch.Password)
	        }
			outputs.EnabledOutputs = append(outputs.EnabledOutputs, "Elasticsearch")
		}
	}

[distortedsignal]

I quickly red your fix, and I think we should call BasicAuth() function at client init, and not in Post() and that will avoid to add authentication Headers at each Post, wdyt?

I'm not sure that will work. The reason that the old system worked (and didn't leak memory) is that every time there was a POST, the headers object was recreated. This worked because we were adding all the headers in the client.go Post function. Because we are now adding headers to a list and then persisting that list across POST requests, there are some headers that need to be set on a per-request basis. Not resetting that list will leak memory across Post calls. As I said, this is a moderate problem for most outputs, but this is REALLY a problem for the Kubeless output. Because it sets a new event-id for each issue/POST, these will accumulate across request boundaries. So these headers will have to be cleared after each request.

[Issif]

I quickly red your fix, and I think we should call BasicAuth() function at client init, and not in Post() and that will avoid to add authentication Headers at each Post, wdyt?

I'm not sure that will work. The reason that the old system worked (and didn't leak memory) is that every time there was a POST, the headers object was recreated. This worked because we were adding all the headers in the client.go Post function. Because we are now adding headers to a list and then persisting that list across POST requests, there are some headers that need to be set on a per-request basis. Not resetting that list will leak memory across Post calls. As I said, this is a moderate problem for most outputs, but this is REALLY a problem for the Kubeless output. Because it sets a new event-id for each issue/POST, these will accumulate across request boundaries. So these headers will have to be cleared after each request.

Ok I see, your fix seems the simplest way to do so and without changing too much things

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: distortedsignal, Issif

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Issif]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[poiana]

LGTM label has been added.

Git tree hash: 88e69613d493c0d06ec0a2a78b70db22c959402b