From 09c7ca8f8ffcf9ea1fe93a6494e0162de5d6dd2d Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Thu, 21 May 2020 16:53:17 -0400 Subject: [PATCH] replace panics with logging --- cmd/{config.go => init.go} | 15 +++++++++++++-- cmd/root.go | 19 ++++++------------- imgbom/analyzer/analyzer.go | 1 + imgbom/analyzer/controller.go | 9 ++++++++- imgbom/analyzer/dpkg/analyzer.go | 26 ++++++++++++++++++++++---- imgbom/analyzer/dpkg/parser_test.go | 4 ++-- imgbom/constants.go | 4 ++++ imgbom/log.go | 6 ++++++ imgbom/presenter/json/presenter.go | 15 +++++++++++---- 9 files changed, 73 insertions(+), 26 deletions(-) rename cmd/{config.go => init.go} (68%) create mode 100644 imgbom/constants.go diff --git a/cmd/config.go b/cmd/init.go similarity index 68% rename from cmd/config.go rename to cmd/init.go index 730810d1f..3a9bd9f57 100644 --- a/cmd/config.go +++ b/cmd/init.go @@ -1,18 +1,20 @@ package cmd import ( + "encoding/json" "fmt" "os" "github.com/anchore/imgbom/imgbom" "github.com/anchore/imgbom/internal/config" + "github.com/anchore/imgbom/internal/log" "github.com/anchore/imgbom/internal/logger" "github.com/spf13/viper" ) var appConfig *config.Application -func loadAppConfig() { +func initAppConfig() { cfg, err := config.LoadConfigFromFile(viper.GetViper(), &cliOpts) if err != nil { fmt.Printf("failed to load application config: \n\t%+v\n", err) @@ -21,7 +23,7 @@ func loadAppConfig() { appConfig = cfg } -func setupLoggingFromAppConfig() { +func initLogging() { config := logger.LogConfig{ EnableConsole: appConfig.Log.FileLocation == "" && !appConfig.Quiet, EnableFile: appConfig.Log.FileLocation != "", @@ -32,3 +34,12 @@ func setupLoggingFromAppConfig() { imgbom.SetLogger(logger.NewZapLogger(config)) } + +func logAppConfig() { + appCfgStr, err := json.MarshalIndent(&appConfig, " ", " ") + if err != nil { + log.Debugf("Could not display application config: %+v", err) + } else { + log.Debugf("Application config:\n%+v", string(appCfgStr)) + } +} diff --git a/cmd/root.go b/cmd/root.go index 830c979ec..29246a8c2 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,7 +1,6 @@ package cmd import ( - "encoding/json" "fmt" "os" @@ -31,8 +30,9 @@ Supports the following image sources: func init() { setCliOptions() - cobra.OnInitialize(loadAppConfig) - cobra.OnInitialize(setupLoggingFromAppConfig) + cobra.OnInitialize(initAppConfig) + cobra.OnInitialize(initLogging) + cobra.OnInitialize(logAppConfig) } func Execute() { @@ -43,15 +43,8 @@ func Execute() { } func doRunCmd(cmd *cobra.Command, args []string) { - appCfgStr, err := json.MarshalIndent(&appConfig, " ", " ") - if err != nil { - log.Debugf("could not display application config: %+v", err) - } else { - log.Debugf("application config:\n%+v", string(appCfgStr)) - } - userImageStr := args[0] - log.Infof("fetching image %s...", userImageStr) + log.Infof("Fetching image '%s'", userImageStr) img, err := stereoscope.GetImage(userImageStr) if err != nil { log.Errorf("could not fetch image '%s': %w", userImageStr, err) @@ -59,14 +52,14 @@ func doRunCmd(cmd *cobra.Command, args []string) { } defer stereoscope.Cleanup() - log.Info("cataloging image...") + log.Info("Cataloging image") catalog, err := imgbom.CatalogImage(img, appConfig.ScopeOpt) if err != nil { log.Errorf("could not catalog image: %w", err) os.Exit(1) } - log.Info("done!") + log.Info("Complete!") err = presenter.GetPresenter(appConfig.PresenterOpt).Present(os.Stdout, img, catalog) if err != nil { log.Errorf("could not format catalog results: %w", err) diff --git a/imgbom/analyzer/analyzer.go b/imgbom/analyzer/analyzer.go index 4eb9f6955..23760fa07 100644 --- a/imgbom/analyzer/analyzer.go +++ b/imgbom/analyzer/analyzer.go @@ -7,6 +7,7 @@ import ( ) type Analyzer interface { + Name() string // TODO: add ID / Name for analyze for uniquely identifying this analyzer type SelectFiles([]*tree.FileTree) []file.Reference // NOTE: one of the errors which is returned is "IterationNeeded", which indicates to the driver to diff --git a/imgbom/analyzer/controller.go b/imgbom/analyzer/controller.go index 21431c8c4..ed9c4c99f 100644 --- a/imgbom/analyzer/controller.go +++ b/imgbom/analyzer/controller.go @@ -4,6 +4,7 @@ import ( "github.com/anchore/imgbom/imgbom/analyzer/dpkg" "github.com/anchore/imgbom/imgbom/pkg" "github.com/anchore/imgbom/imgbom/scope" + "github.com/anchore/imgbom/internal/log" "github.com/anchore/stereoscope/pkg/file" "github.com/hashicorp/go-multierror" ) @@ -26,6 +27,7 @@ type controller struct { } func (c *controller) add(a Analyzer) { + log.Debugf("adding analyzer: %s", a.Name()) c.analyzers = append(c.analyzers, a) } @@ -36,6 +38,7 @@ func (c *controller) analyze(s scope.Scope) (pkg.Catalog, error) { // ask analyzers for files to extract from the image tar for _, a := range c.analyzers { fileSelection = append(fileSelection, a.SelectFiles(s.Trees)...) + log.Debugf("analyzer '%s' selected '%d' files", a.Name(), len(fileSelection)) } // fetch contents for requested selection by analyzers @@ -47,11 +50,15 @@ func (c *controller) analyze(s scope.Scope) (pkg.Catalog, error) { // perform analysis, accumulating errors for each failed analysis var errs error for _, a := range c.analyzers { - packages, err := a.Analyze(contents) // TODO: check for multiple rounds of analyses by Iterate error + packages, err := a.Analyze(contents) if err != nil { errs = multierror.Append(errs, err) + continue } + + log.Debugf("analyzer '%s' discovered '%d' packages", a.Name(), len(packages)) + for _, p := range packages { catalog.Add(p) } diff --git a/imgbom/analyzer/dpkg/analyzer.go b/imgbom/analyzer/dpkg/analyzer.go index 380c86e4c..26cf69f77 100644 --- a/imgbom/analyzer/dpkg/analyzer.go +++ b/imgbom/analyzer/dpkg/analyzer.go @@ -4,6 +4,7 @@ import ( "strings" "github.com/anchore/imgbom/imgbom/pkg" + "github.com/anchore/imgbom/internal/log" "github.com/anchore/stereoscope/pkg/file" "github.com/anchore/stereoscope/pkg/tree" ) @@ -16,6 +17,10 @@ func NewAnalyzer() *Analyzer { return &Analyzer{} } +func (a *Analyzer) Name() string { + return "dpkg-analyzer" +} + func (a *Analyzer) SelectFiles(trees []*tree.FileTree) []file.Reference { files := make([]file.Reference, 0) for _, tree := range trees { @@ -36,14 +41,26 @@ func (a *Analyzer) Analyze(contents map[file.Reference]string) ([]pkg.Package, e for _, reference := range a.selectedFiles { content, ok := contents[reference] if !ok { - // TODO: this needs handling - panic(reference) + // TODO: test case + log.WithFields(map[string]interface{}{ + "path": reference.Path, + "id": reference.ID(), + "analyzer": a.Name(), + }).Errorf("analyzer file content missing") + + continue } entries, err := ParseEntries(strings.NewReader(content)) if err != nil { - // TODO: punt for now, we need to handle this - panic(err) + // TODO: test case + log.WithFields(map[string]interface{}{ + "path": reference.Path, + "id": reference.ID(), + "analyzer": a.Name(), + }).Errorf("analyzer failed to parse entries: %w", err) + + continue } for _, entry := range entries { packages = append(packages, pkg.Package{ @@ -55,5 +72,6 @@ func (a *Analyzer) Analyze(contents map[file.Reference]string) ([]pkg.Package, e }) } } + return packages, nil } diff --git a/imgbom/analyzer/dpkg/parser_test.go b/imgbom/analyzer/dpkg/parser_test.go index 0704c54ae..1cc8ec133 100644 --- a/imgbom/analyzer/dpkg/parser_test.go +++ b/imgbom/analyzer/dpkg/parser_test.go @@ -54,7 +54,7 @@ func TestSinglePackage(t *testing.T) { defer func() { err := file.Close() if err != nil { - panic(err) + t.Fatal("closing file failed:", err) } }() @@ -123,7 +123,7 @@ func TestMultiplePackages(t *testing.T) { defer func() { err := file.Close() if err != nil { - panic(err) + t.Fatal("closing file failed:", err) } }() diff --git a/imgbom/constants.go b/imgbom/constants.go new file mode 100644 index 000000000..1e64632ee --- /dev/null +++ b/imgbom/constants.go @@ -0,0 +1,4 @@ +package imgbom + +// note: must be a single word, all lowercase +const LibraryName = "imgbom" diff --git a/imgbom/log.go b/imgbom/log.go index ac2d5d4d1..42a51bd36 100644 --- a/imgbom/log.go +++ b/imgbom/log.go @@ -8,3 +8,9 @@ import ( func SetLogger(logger logger.Logger) { log.Log = logger } + +func SetLoggerWithTags(logger logger.Logger, tags map[string]interface{}) { + log.Log = logger.WithFields(map[string]interface{}{ + "source": LibraryName, + }) +} diff --git a/imgbom/presenter/json/presenter.go b/imgbom/presenter/json/presenter.go index 6d06c4ecb..785acec03 100644 --- a/imgbom/presenter/json/presenter.go +++ b/imgbom/presenter/json/presenter.go @@ -5,6 +5,7 @@ import ( "io" "github.com/anchore/imgbom/imgbom/pkg" + "github.com/anchore/imgbom/internal/log" stereoscopeImg "github.com/anchore/stereoscope/pkg/image" ) @@ -88,8 +89,13 @@ func (pres *Presenter) Present(output io.Writer, img *stereoscopeImg.Image, cata for idx, src := range p.Source { fileMetadata, err := img.FileCatalog.Get(src) if err != nil { - // TODO: replace - panic(err) + // TODO: test case + log.WithFields(map[string]interface{}{ + "path": src.Path, + "id": src.ID(), + "presenter": "json", + }).Errorf("could not get metadata from catalog") + } srcObj := source{ @@ -105,8 +111,9 @@ func (pres *Presenter) Present(output io.Writer, img *stereoscopeImg.Image, cata bytes, err := json.Marshal(&doc) if err != nil { - // TODO: replace - panic(err) + log.WithFields(map[string]interface{}{ + "presenter": "json", + }).Errorf("failed to marshal json: %w", err) } _, err = output.Write(bytes)