Skip to content

Commit

Permalink
Merge branch 'main' into uninstall-kill-watcher
Browse files Browse the repository at this point in the history
  • Loading branch information
blakerouse committed Sep 20, 2023
2 parents db958b5 + 69cc860 commit a6711b0
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 33 deletions.
2 changes: 1 addition & 1 deletion .buildkite/scripts/steps/integration_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ DEV=true EXTERNAL=true SNAPSHOT=true PLATFORMS=linux/amd64,linux/arm64 PACKAGES=

# Run integration tests
set +e
TEST_INTEG_CLEAN_ON_EXIT=true SNAPSHOT=true mage integration:test
TEST_INTEG_AUTH_ESS_REGION=azure-eastus2 TEST_INTEG_CLEAN_ON_EXIT=true SNAPSHOT=true mage integration:test
TESTS_EXIT_STATUS=$?
set -e

Expand Down
32 changes: 32 additions & 0 deletions changelog/fragments/1694700201-gpg-unreachable-url-fix.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Kind can be one of:
# - breaking-change: a change to previously-documented behavior
# - deprecation: functionality that is being removed in a later release
# - bug-fix: fixes a problem in a previous version
# - enhancement: extends functionality but does not break or fix existing behavior
# - feature: new functionality
# - known-issue: problems that we are aware of in a given version
# - security: impacts on the security of a product or a user’s deployment.
# - upgrade: important information for someone upgrading from a prior version
# - other: does not fit into any of the other categories
kind: bug-fix

# Change summary; a 80ish characters long description of the change.
summary: Fix gpg verification, if one is successful upgrade should continue.

# Long description; in case the summary is not enough to describe the change
# this field accommodate a description without length limits.
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
#description:

# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
component: elastic-agent

# PR URL; optional; the PR number that added the changeset.
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
# Please provide it if you are adding a fragment for a different PR.
pr: https://github.com/elastic/elastic-agent/pull/3426

# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
# If not present is automatically filled by the tooling with the issue linked to the PR number.
issue: https://github.com/elastic/elastic-agent/issues/3368
Original file line number Diff line number Diff line change
Expand Up @@ -10,53 +10,55 @@ import (
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download"
"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
"github.com/elastic/elastic-agent/pkg/core/logger"
)

// Verifier is a verifier with a predefined set of verifiers.
// During each verify call it tries to call the first one and on failure fallbacks to
// the next one.
// Error is returned if all of them fail.
type Verifier struct {
vv []download.Verifier
vv []download.Verifier
log *logger.Logger
}

func (v *Verifier) Name() string {
return "composed.verifier"
}

// NewVerifier creates a verifier composed out of predefined set of verifiers.
// During each verify call it tries to call the first one and on failure fallbacks to
// the next one.
// Error is returned if all of them fail.
func NewVerifier(verifiers ...download.Verifier) *Verifier {
func NewVerifier(log *logger.Logger, verifiers ...download.Verifier) *Verifier {
return &Verifier{
vv: verifiers,
log: log,
vv: verifiers,
}
}

// Verify checks the package from configured source.
func (e *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error {
func (v *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error {
var err error
var checksumMismatchErr *download.ChecksumMismatchError
var invalidSignatureErr *download.InvalidSignatureError

for _, v := range e.vv {
e := v.Verify(a, version, skipDefaultPgp, pgpBytes...)
for _, verifier := range v.vv {
e := verifier.Verify(a, version, skipDefaultPgp, pgpBytes...)
if e == nil {
// Success
return nil
}

err = multierror.Append(err, e)

if errors.As(e, &checksumMismatchErr) || errors.As(err, &invalidSignatureErr) {
// Stop verification chain on checksum/signature errors.
break
}
v.log.Warnw("Verifier failed!", "verifier", verifier.Name(), "error", e)
}

return err
}

func (e *Verifier) Reload(c *artifact.Config) error {
for _, v := range e.vv {
reloadable, ok := v.(download.Reloader)
func (v *Verifier) Reload(c *artifact.Config) error {
for _, verifier := range v.vv {
reloadable, ok := verifier.(download.Reloader)
if !ok {
continue
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact"
"github.com/elastic/elastic-agent/internal/pkg/agent/application/upgrade/artifact/download"
"github.com/elastic/elastic-agent/pkg/core/logger"

"github.com/stretchr/testify/assert"
)
Expand All @@ -18,6 +19,10 @@ type ErrorVerifier struct {
called bool
}

func (d *ErrorVerifier) Name() string {
return "error"
}

func (d *ErrorVerifier) Verify(a artifact.Artifact, version string, _ bool, _ ...string) error {
d.called = true
return errors.New("failing")
Expand All @@ -29,9 +34,13 @@ type FailVerifier struct {
called bool
}

func (d *FailVerifier) Name() string {
return "fail"
}

func (d *FailVerifier) Verify(a artifact.Artifact, version string, _ bool, _ ...string) error {
d.called = true
return &download.InvalidSignatureError{}
return &download.InvalidSignatureError{File: "", Err: errors.New("invalid signature")}
}

func (d *FailVerifier) Called() bool { return d.called }
Expand All @@ -40,6 +49,10 @@ type SuccVerifier struct {
called bool
}

func (d *SuccVerifier) Name() string {
return "succ"
}

func (d *SuccVerifier) Verify(a artifact.Artifact, version string, _ bool, _ ...string) error {
d.called = true
return nil
Expand All @@ -48,6 +61,7 @@ func (d *SuccVerifier) Verify(a artifact.Artifact, version string, _ bool, _ ...
func (d *SuccVerifier) Called() bool { return d.called }

func TestVerifier(t *testing.T) {
log, _ := logger.New("", false)
testCases := []verifyTestCase{
{
verifiers: []CheckableVerifier{&ErrorVerifier{}, &SuccVerifier{}, &FailVerifier{}},
Expand All @@ -59,21 +73,21 @@ func TestVerifier(t *testing.T) {
expectedResult: true,
}, {
verifiers: []CheckableVerifier{&FailVerifier{}, &ErrorVerifier{}, &SuccVerifier{}},
checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && !d[1].Called() && !d[2].Called() },
expectedResult: false,
checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && d[1].Called() && d[2].Called() },
expectedResult: true,
}, {
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,
}, {
verifiers: []CheckableVerifier{&ErrorVerifier{}, &ErrorVerifier{}, &FailVerifier{}},
checkFunc: func(d []CheckableVerifier) bool { return d[0].Called() && d[1].Called() && d[2].Called() },
expectedResult: false,
},
}

for _, tc := range testCases {
d := NewVerifier(tc.verifiers[0], tc.verifiers[1], tc.verifiers[2])
d := NewVerifier(log, tc.verifiers[0], tc.verifiers[1], tc.verifiers[2])
err := d.Verify(artifact.Artifact{Name: "a", Cmd: "a", Artifact: "a/a"}, "b", false)

assert.Equal(t, tc.expectedResult, err == nil)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ type Verifier struct {
log *logger.Logger
}

func (v *Verifier) Name() string {
return "fs.verifier"
}

// NewVerifier creates a verifier checking downloaded package on preconfigured
// location against a key stored on elastic.co website.
func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool, pgp []byte) (*Verifier, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ type Verifier struct {
log progressLogger
}

func (v *Verifier) Name() string {
return "http.verifier"
}

// NewVerifier create a verifier checking downloaded package on preconfigured
// location against a key stored on elastic.co website.
func NewVerifier(log progressLogger, config *artifact.Config, allowEmptyPgp bool, pgp []byte) (*Verifier, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool
}
verifiers = append(verifiers, remoteVer)

return composed.NewVerifier(verifiers...), nil
return composed.NewVerifier(log, verifiers...), nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ type Verifier struct {
versionOverride *agtversion.ParsedSemVer
}

func (v *Verifier) Name() string {
return "snapshot.verifier"
}

// NewVerifier creates a downloader which first checks local directory
// and then fallbacks to remote if configured.
func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool, pgp []byte, versionOverride *agtversion.ParsedSemVer) (download.Verifier, error) {
Expand All @@ -37,17 +41,17 @@ func NewVerifier(log *logger.Logger, config *artifact.Config, allowEmptyPgp bool
}

// Verify checks the package from configured source.
func (e *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error {
return e.verifier.Verify(a, version, skipDefaultPgp, pgpBytes...)
func (v *Verifier) Verify(a artifact.Artifact, version string, skipDefaultPgp bool, pgpBytes ...string) error {
return v.verifier.Verify(a, version, skipDefaultPgp, pgpBytes...)
}

func (e *Verifier) Reload(c *artifact.Config) error {
reloader, ok := e.verifier.(artifact.ConfigReloader)
func (v *Verifier) Reload(c *artifact.Config) error {
reloader, ok := v.verifier.(artifact.ConfigReloader)
if !ok {
return nil
}

cfg, err := snapshotConfig(c, e.versionOverride)
cfg, err := snapshotConfig(c, v.versionOverride)
if err != nil {
return errors.New(err, "snapshot.downloader: failed to generate snapshot config")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ func (e *InvalidSignatureError) Unwrap() error { return e.Err }
// Verifier is an interface verifying the SHA512 checksum and GPG signature and
// of a downloaded artifact.
type Verifier interface {
// Verify should verify the artifact and return an error if any checks fail.
Name() string
// Verify should verify the artifact and return if succeed status (true|false) and an error if any checks fail.
// If the checksum does no match Verify returns a
// *download.ChecksumMismatchError. And if the GPG signature is invalid then
// Verify returns a *download.InvalidSignatureError. Use errors.As() to
Expand Down
3 changes: 1 addition & 2 deletions internal/pkg/agent/application/upgrade/step_download.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ func (u *Upgrader) downloadArtifact(ctx context.Context, version, sourceURI stri
if err := verifier.Verify(agentArtifact, parsedVersion.VersionWithPrerelease(), skipDefaultPgp, pgpBytes...); err != nil {
return "", errors.New(err, "failed verification of agent binary")
}

return path, nil
}

Expand Down Expand Up @@ -141,7 +140,7 @@ func newVerifier(version *agtversion.ParsedSemVer, log *logger.Logger, settings
return nil, err
}

return composed.NewVerifier(fsVerifier, snapshotVerifier, remoteVerifier), nil
return composed.NewVerifier(log, fsVerifier, snapshotVerifier, remoteVerifier), nil
}

func (u *Upgrader) downloadWithRetries(
Expand Down

0 comments on commit a6711b0

Please sign in to comment.