Skip to content

Commit

Permalink
Stops admin fetching on first error (#209)
Browse files Browse the repository at this point in the history
Before, we added a dependency on multi-error to allow multiple problems
to accumulate before before returning control to the user. There's
little reason to allow partial failure across admin URLS, especially
when a partial failure still quits with an error status.

This removes the dependency and works normally instead. This also fixes
syntax where formerly we used defer in a for loop.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
  • Loading branch information
codefromthecrypt authored May 11, 2021
1 parent 6a4cf5a commit d5b16dc
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 30 deletions.
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ require (
github.com/StackExchange/wmi v0.0.0-20210224194228-fe8f1750fd46 // indirect
github.com/andybalholm/brotli v1.0.2 // indirect
github.com/go-ole/go-ole v1.2.5 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/klauspost/compress v1.12.2 // indirect
github.com/klauspost/pgzip v1.2.5 // indirect
github.com/mattn/go-runewidth v0.0.12 // indirect
Expand All @@ -22,7 +21,6 @@ require (
github.com/spf13/cobra v1.1.3
github.com/stretchr/testify v1.7.0
github.com/tetratelabs/log v0.0.0-20210422163326-7ba70517903b
github.com/tetratelabs/multierror v1.1.0
github.com/tklauser/go-sysconf v0.3.5 // indirect
github.com/ulikunitz/xz v0.5.10 // indirect
go.uber.org/multierr v1.7.0 // indirect
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t
github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
Expand Down Expand Up @@ -248,8 +246,6 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
github.com/tetratelabs/log v0.0.0-20210422163326-7ba70517903b h1:QjfIzzC2Zoe542WRrJdBymP/CrlwIXD8lYFiXNUTnpA=
github.com/tetratelabs/log v0.0.0-20210422163326-7ba70517903b/go.mod h1:f/tP3FhNTKy6gn0QjYCXViykWjTHceFedy5O4xklKCE=
github.com/tetratelabs/multierror v1.1.0 h1:cKmV/Pbf42K5wp8glxa2YIausbxIraPN8fzru9Pn1Cg=
github.com/tetratelabs/multierror v1.1.0/go.mod h1:kH3SzI/z+FwEbV9bxQDx4GiIgE2djuyb8wiB2DaUBnY=
github.com/tklauser/go-sysconf v0.3.4/go.mod h1:Cl2c8ZRWfHD5IrfHo9VN+FX9kCFjIOyVklgXycLB6ek=
github.com/tklauser/go-sysconf v0.3.5 h1:uu3Xl4nkLzQfXNsWn15rPc/HQCJKObbt1dKJeWp3vU4=
github.com/tklauser/go-sysconf v0.3.5/go.mod h1:MkWzOF4RMCshBAMXuhXJs64Rte09mITnppBXY/rYEFI=
Expand Down
54 changes: 30 additions & 24 deletions pkg/binary/envoy/debug/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"os"
"path/filepath"

"github.com/tetratelabs/multierror"

"github.com/tetratelabs/getenvoy/pkg/binary/envoy"
)

Expand Down Expand Up @@ -54,28 +52,36 @@ func (e *envoyAdminDataCollection) retrieveAdminAPIData() error {
if err != nil {
return fmt.Errorf("unable to capture Envoy configuration and metrics: %w", err)
}
multiErr := &multierror.Error{}
for path, file := range adminAPIPaths {
resp, err := http.Get(fmt.Sprintf("http://%s/%v", adminAddress, path))
if err != nil {
multiErr = multierror.Append(multiErr, err)
continue
}
if resp.StatusCode != http.StatusOK {
multiErr = multierror.Append(multiErr, fmt.Errorf("received %v from /%v ", resp.StatusCode, path))
continue
}
// #nosec -> r.GetWorkingDir() is allowed to be anywhere
f, err := os.OpenFile(filepath.Join(e.workingDir, file), os.O_CREATE|os.O_WRONLY, 0600)
if err != nil {
multiErr = multierror.Append(multiErr, err)
continue
}
defer f.Close() //nolint
defer resp.Body.Close() //nolint
if _, err := io.Copy(f, resp.Body); err != nil {
multiErr = multierror.Append(multiErr, err)
for p, f := range adminAPIPaths {
url := fmt.Sprintf("http://%s/%v", adminAddress, p)
file := filepath.Join(e.workingDir, f)
if e := copyURLToFile(url, file); e != nil {
return e
}
}
return multiErr.ErrorOrNil()
return nil
}

func copyURLToFile(url, fullPath string) error {
// #nosec -> e.workingDir is allowed to be anywhere
f, err := os.OpenFile(fullPath, os.O_CREATE|os.O_WRONLY, 0600)
if err != nil {
return fmt.Errorf("could not open %q: %w", fullPath, err)
}
defer f.Close() //nolint

// #nosec -> adminAddress is written by Envoy and the paths are hard-coded
resp, err := http.Get(url)
if err != nil {
return fmt.Errorf("could not read %v: %w", url, err)
}
if resp.StatusCode != http.StatusOK {
return fmt.Errorf("received %v from %v", resp.StatusCode, url)
}
defer resp.Body.Close() //nolint

if _, err := io.Copy(f, resp.Body); err != nil {
return fmt.Errorf("could not write response body of %v: %w", url, err)
}
return nil
}

0 comments on commit d5b16dc

Please sign in to comment.