diff --git a/x-pack/elastic-agent/CHANGELOG.next.asciidoc b/x-pack/elastic-agent/CHANGELOG.next.asciidoc index f219bc12247..54bf5169960 100644 --- a/x-pack/elastic-agent/CHANGELOG.next.asciidoc +++ b/x-pack/elastic-agent/CHANGELOG.next.asciidoc @@ -94,6 +94,7 @@ - Allow HTTP metrics to run in bootstrap mode. Add ability to adjust timeouts for Fleet Server. {pull}28260[28260] - Fix agent configuration overwritten by default fleet config. {pull}29297[29297] - Allow agent containers to use basic auth to create a service token. {pull}29651[29651] +- Fix issue where a failing artifact verification does not remove the bad artifact. {pull}30281[30281] ==== New features diff --git a/x-pack/elastic-agent/pkg/artifact/download/composed/verifier.go b/x-pack/elastic-agent/pkg/artifact/download/composed/verifier.go index 4dbe5377b79..72ea1d8f3b2 100644 --- a/x-pack/elastic-agent/pkg/artifact/download/composed/verifier.go +++ b/x-pack/elastic-agent/pkg/artifact/download/composed/verifier.go @@ -33,9 +33,8 @@ func NewVerifier(verifiers ...download.Verifier) *Verifier { func (e *Verifier) Verify(spec program.Spec, version string, removeOnFailure bool) (bool, error) { var err error - for i, v := range e.vv { - isLast := (i + 1) == len(e.vv) - b, e := v.Verify(spec, version, isLast && removeOnFailure) + for _, v := range e.vv { + b, e := v.Verify(spec, version, removeOnFailure) if e == nil { return b, nil } diff --git a/x-pack/elastic-agent/pkg/artifact/download/composed/verifier_test.go b/x-pack/elastic-agent/pkg/artifact/download/composed/verifier_test.go new file mode 100644 index 00000000000..a751b32c188 --- /dev/null +++ b/x-pack/elastic-agent/pkg/artifact/download/composed/verifier_test.go @@ -0,0 +1,95 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package composed + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/agent/program" + "github.com/elastic/beats/v7/x-pack/elastic-agent/pkg/artifact/download" +) + +type ErrorVerifier struct { + called bool +} + +func (d *ErrorVerifier) Verify(spec program.Spec, version string, removeOnFailure bool) (bool, error) { + d.called = true + return false, errors.New("failing") +} + +func (d *ErrorVerifier) Called() bool { return d.called } + +type FailVerifier struct { + called bool +} + +func (d *FailVerifier) Verify(spec program.Spec, version string, removeOnFailure bool) (bool, error) { + d.called = true + return false, nil +} + +func (d *FailVerifier) Called() bool { return d.called } + +type SuccVerifier struct { + called bool + removeOnFailure bool +} + +func (d *SuccVerifier) Verify(spec program.Spec, version string, removeOnFailure bool) (bool, error) { + d.called = true + return true, nil +} + +func (d *SuccVerifier) Called() bool { return d.called } + +func TestVerifier(t *testing.T) { + testCases := []verifyTestCase{ + { + verifiers: []CheckableVerifier{&ErrorVerifier{}, &SuccVerifier{}, &FailVerifier{}}, + checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && d[1].Called() && !d[2].Called() }, + expectedResult: true, + }, { + verifiers: []CheckableVerifier{&SuccVerifier{}, &ErrorVerifier{}, &FailVerifier{}}, + checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && !d[1].Called() && !d[2].Called() }, + expectedResult: true, + }, { + verifiers: []CheckableVerifier{&FailVerifier{}, &ErrorVerifier{}, &SuccVerifier{}}, + checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && !d[1].Called() && !d[2].Called() }, + expectedResult: false, + }, { + verifiers: []CheckableVerifier{&ErrorVerifier{}, &FailVerifier{}, &SuccVerifier{}}, + checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && d[1].Called() && !d[2].Called() }, + expectedResult: false, + }, { + verifiers: []CheckableVerifier{&ErrorVerifier{}, &ErrorVerifier{}, &SuccVerifier{}}, + checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && d[1].Called() && d[2].Called() }, + expectedResult: true, + }, + } + + for _, tc := range testCases { + d := NewVerifier(tc.verifiers[0], tc.verifiers[1], tc.verifiers[2]) + r, _ := d.Verify(program.Spec{Name: "a", Cmd: "a", Artifact: "a/a"}, "b", true) + + assert.Equal(t, tc.expectedResult, r) + + assert.True(t, tc.checkFunc(tc.verifiers)) + } +} + +type CheckableVerifier interface { + download.Verifier + Called() bool +} + +type verifyTestCase struct { + verifiers []CheckableVerifier + checkFunc func(verifiers []CheckableVerifier) bool + expectedResult bool +}