From 4bb1c7a4da6a9a738edc3d0aeec5707715e0a1ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Mathieu?= Date: Mon, 6 May 2024 19:37:52 +0200 Subject: [PATCH] logs/sds: delete existing scanners when an update with 0 configuration is received through RC (#25375) * logs/sds: delete existing scanners when an update with 0 configuration is received through RC. * logs/sds: report the errors from all reconfigured pipelines. * logs/sds: add an unit test for this specific scenario. * inv -e tidy-all * Typo. --- comp/logs/agent/agent.go | 17 +++-- comp/otelcol/logsagentpipeline/go.mod | 2 + comp/otelcol/logsagentpipeline/go.sum | 4 ++ .../logsagentpipelineimpl/go.mod | 2 + .../logsagentpipelineimpl/go.sum | 4 ++ pkg/logs/pipeline/go.mod | 2 + pkg/logs/pipeline/go.sum | 4 ++ pkg/logs/pipeline/provider.go | 3 +- pkg/logs/sds/scanner_test.go | 71 +++++++++++++++++++ 9 files changed, 104 insertions(+), 5 deletions(-) diff --git a/comp/logs/agent/agent.go b/comp/logs/agent/agent.go index f4efc8de3e79b..130551f1439ae 100644 --- a/comp/logs/agent/agent.go +++ b/comp/logs/agent/agent.go @@ -11,6 +11,7 @@ import ( "fmt" "time" + "github.com/hashicorp/go-multierror" "go.uber.org/atomic" "go.uber.org/fx" @@ -308,7 +309,7 @@ func (a *agent) onUpdateSDSRules(updates map[string]state.RawConfig, applyStateC var err error for _, config := range updates { if rerr := a.pipelineProvider.ReconfigureSDSStandardRules(config.Config); rerr != nil { - err = rerr + err = multierror.Append(err, rerr) } } @@ -333,9 +334,17 @@ func (a *agent) onUpdateSDSRules(updates map[string]state.RawConfig, applyStateC func (a *agent) onUpdateSDSAgentConfig(updates map[string]state.RawConfig, applyStateCallback func(string, state.ApplyStatus)) { //nolint:revive var err error - for _, config := range updates { - if rerr := a.pipelineProvider.ReconfigureSDSAgentConfig(config.Config); rerr != nil { - err = rerr + // We received a hit that new updates arrived, but if the list of updates + // is empty, it means we don't have any updates applying to this agent anymore + // Send a reconfiguration with an empty payload, indicating that + // the scanners have to be dropped. + if len(updates) == 0 { + err = a.pipelineProvider.ReconfigureSDSAgentConfig([]byte("{}")) + } else { + for _, config := range updates { + if rerr := a.pipelineProvider.ReconfigureSDSAgentConfig(config.Config); rerr != nil { + err = multierror.Append(err, rerr) + } } } diff --git a/comp/otelcol/logsagentpipeline/go.mod b/comp/otelcol/logsagentpipeline/go.mod index a6879a900e44b..c39bb09065e3b 100644 --- a/comp/otelcol/logsagentpipeline/go.mod +++ b/comp/otelcol/logsagentpipeline/go.mod @@ -109,6 +109,8 @@ require ( github.com/go-ole/go-ole v1.2.6 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/protobuf v1.5.3 // indirect + github.com/hashicorp/errwrap v1.0.0 // indirect + github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/hectane/go-acl v0.0.0-20190604041725-da78bae5fc95 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect diff --git a/comp/otelcol/logsagentpipeline/go.sum b/comp/otelcol/logsagentpipeline/go.sum index 662b88c41f115..aed8e1f83cea5 100644 --- a/comp/otelcol/logsagentpipeline/go.sum +++ b/comp/otelcol/logsagentpipeline/go.sum @@ -191,6 +191,10 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.2.0/go.mod h1:mJzapYve32yjrKlk9G github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.13.0/go.mod h1:8XEsbTttt/W+VvjtQhLACqCisSPWTxCZ7sBRjU6iH9c= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= diff --git a/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.mod b/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.mod index 1d2f6bec9de85..eba8324c96606 100644 --- a/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.mod +++ b/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.mod @@ -120,6 +120,8 @@ require ( github.com/go-ole/go-ole v1.2.6 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/protobuf v1.5.3 // indirect + github.com/hashicorp/errwrap v1.0.0 // indirect + github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/hectane/go-acl v0.0.0-20190604041725-da78bae5fc95 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect diff --git a/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.sum b/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.sum index f0bef09ee7d04..5dbeb9c0fb8af 100644 --- a/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.sum +++ b/comp/otelcol/logsagentpipeline/logsagentpipelineimpl/go.sum @@ -191,6 +191,10 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.2.0/go.mod h1:mJzapYve32yjrKlk9G github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.13.0/go.mod h1:8XEsbTttt/W+VvjtQhLACqCisSPWTxCZ7sBRjU6iH9c= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= diff --git a/pkg/logs/pipeline/go.mod b/pkg/logs/pipeline/go.mod index 17856fd3afd56..b132bf80f67d1 100644 --- a/pkg/logs/pipeline/go.mod +++ b/pkg/logs/pipeline/go.mod @@ -64,6 +64,7 @@ require ( github.com/DataDog/datadog-agent/pkg/status/health v0.54.0-rc.2 github.com/DataDog/datadog-agent/pkg/util/log v0.54.0-rc.2 github.com/DataDog/datadog-agent/pkg/util/startstop v0.54.0-rc.2 + github.com/hashicorp/go-multierror v1.1.1 github.com/stretchr/testify v1.9.0 go.uber.org/atomic v1.11.0 ) @@ -109,6 +110,7 @@ require ( github.com/go-ole/go-ole v1.2.6 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/protobuf v1.5.3 // indirect + github.com/hashicorp/errwrap v1.0.0 // indirect github.com/hashicorp/hcl v1.0.0 // indirect github.com/hectane/go-acl v0.0.0-20190604041725-da78bae5fc95 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect diff --git a/pkg/logs/pipeline/go.sum b/pkg/logs/pipeline/go.sum index 211c65a3c8d76..17ce6ccf840cf 100644 --- a/pkg/logs/pipeline/go.sum +++ b/pkg/logs/pipeline/go.sum @@ -191,6 +191,10 @@ github.com/grpc-ecosystem/go-grpc-middleware v1.2.0/go.mod h1:mJzapYve32yjrKlk9G github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= github.com/grpc-ecosystem/grpc-gateway v1.13.0/go.mod h1:8XEsbTttt/W+VvjtQhLACqCisSPWTxCZ7sBRjU6iH9c= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= diff --git a/pkg/logs/pipeline/provider.go b/pkg/logs/pipeline/provider.go index e4197367997a2..5554843aa342b 100644 --- a/pkg/logs/pipeline/provider.go +++ b/pkg/logs/pipeline/provider.go @@ -8,6 +8,7 @@ package pipeline import ( "context" + "github.com/hashicorp/go-multierror" "go.uber.org/atomic" "github.com/DataDog/datadog-agent/comp/core/hostname/hostnameinterface" @@ -133,7 +134,7 @@ func (p *provider) reconfigureSDS(config []byte, orderType sds.ReconfigureOrderT for _, response := range responses { err := <-response if err != nil { - rerr = err + rerr = multierror.Append(rerr, err) } close(response) } diff --git a/pkg/logs/sds/scanner_test.go b/pkg/logs/sds/scanner_test.go index 3124c05382cf7..9081740a84f3e 100644 --- a/pkg/logs/sds/scanner_test.go +++ b/pkg/logs/sds/scanner_test.go @@ -203,6 +203,77 @@ func TestCreateScanner(t *testing.T) { require.Len(s.configuredRules, 0, "The group is disabled, no rules should be configured.") } +// TestEmptyConfiguration validates that the scanner is destroyed when receiving +// an empty configuration. +func TestEmptyConfiguration(t *testing.T) { + require := require.New(t) + + standardRules := []byte(` + {"priority":1,"is_enabled":true,"rules":[ + { + "id":"zero-0", + "description":"zero desc", + "name":"zero", + "definitions": [{"version":1, "pattern":"zero"}] + },{ + "id":"one-1", + "description":"one desc", + "name":"one", + "definitions": [{"version":1, "pattern":"one"}] + },{ + "id":"two-2", + "description":"two desc", + "name":"two", + "definitions": [{"version":1, "pattern":"two"}] + } + ]} + `) + agentConfig := []byte(` + {"is_enabled":true,"rules":[ + { + "id": "random000", + "name":"zero", + "definition":{"standard_rule_id":"zero-0"}, + "match_action":{"type":"Redact","placeholder":"[redacted]"}, + "is_enabled":true + } + ]} + `) + + s := CreateScanner(0) + + require.NotNil(s, "the scanner should not be nil after a creation") + + err := s.Reconfigure(ReconfigureOrder{ + Type: StandardRules, + Config: standardRules, + }) + + require.NoError(err, "configuring the standard rules should not fail") + + // configure with one rule + + err = s.Reconfigure(ReconfigureOrder{ + Type: AgentConfig, + Config: agentConfig, + }) + + require.NoError(err, "this one should not fail since one rule is enabled") + require.Len(s.configuredRules, 1, "only one rules should be part of this scanner") + require.NotNil(s.Scanner) + + // empty reconfiguration + + err = s.Reconfigure(ReconfigureOrder{ + Type: AgentConfig, + Config: []byte("{}"), + }) + + require.NoError(err) + require.Len(s.configuredRules, 0) + require.Nil(s.Scanner) +} + func TestIsReady(t *testing.T) { require := require.New(t)