refactor command package to remove globals and add dependency injection

[spiffcs]

Summary

This PR attempts to update the command package so that Syft moves away from global package variables and instead is architected around command constructors with functional options.

Changes

• package cmd separated into cli with commands.go organizing cli construction. Individual commands have constructors along with new packages named as command name.
  EX:import github.com/anchore/syft/cmd/syft/cli/attest includes attest.Run as the execution path.
• package options built to describe options that can be decorated onto built commands
• eventloop given its own package
• remove cli options and elevate to top of config.Application struct
• update application configuration to use dependency injection of viper and config object

TODO:

• Update tests in new packages to cover constructors
• Review removed code and make commits that restore parity with main
• Review changes and add summary of changes to config package
• Review completion changes and fix generation for Broken shell completion - Bash #962

Signed-off-by: Christopher Phillips christopher.phillips@anchore.com

[github-actions]

Benchmark Test Results

Benchmark results from the latest changes vs base branch

name                                                       old time/op    new time/op    delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2              1.23ms ± 5%    1.13ms ± 1%   -7.78%  (p=0.008 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2            3.23ms ± 6%    2.97ms ± 9%     ~     (p=0.056 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2    1.04ms ± 6%    0.93ms ± 0%  -10.40%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         712µs ± 6%     651µs ± 4%   -8.54%  (p=0.016 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     788µs ± 1%     752µs ± 2%   -4.67%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      726µs ± 3%     668µs ± 0%   -8.10%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      13.8ms ± 4%    13.4ms ± 1%   -2.90%  (p=0.016 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.22ms ± 4%    1.17ms ± 1%     ~     (p=0.056 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2          2.09µs ± 2%    2.04µs ± 1%   -2.63%  (p=0.032 n=5+5)

name                                                       old alloc/op   new alloc/op   delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               184kB ± 0%     184kB ± 0%   +0.23%  (p=0.008 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2             895kB ± 0%     896kB ± 0%     ~     (p=0.056 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     196kB ± 0%     196kB ± 0%   +0.22%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         140kB ± 0%     140kB ± 0%   -0.08%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                     175kB ± 0%     176kB ± 0%   +0.50%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      163kB ± 0%     163kB ± 0%   +0.19%  (p=0.016 n=5+5)
ImagePackageCatalogers/java-cataloger-2                      3.29MB ± 0%    3.30MB ± 0%     ~     (p=0.222 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                     1.24MB ± 0%    1.24MB ± 0%   +0.08%  (p=0.008 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2            672B ± 0%      672B ± 0%     ~     (all equal)

name                                                       old allocs/op  new allocs/op  delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2               3.66k ± 0%     3.66k ± 0%     ~     (all equal)
ImagePackageCatalogers/python-package-cataloger-2             14.8k ± 0%     14.8k ± 0%     ~     (p=0.373 n=5+5)
ImagePackageCatalogers/php-composer-installed-cataloger-2     4.94k ± 0%     4.94k ± 0%     ~     (p=0.683 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2         2.72k ± 0%     2.72k ± 0%     ~     (all equal)
ImagePackageCatalogers/dpkgdb-cataloger-2                     3.93k ± 0%     3.95k ± 0%   +0.43%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                      4.01k ± 0%     4.02k ± 0%   +0.17%  (p=0.008 n=5+5)
ImagePackageCatalogers/java-cataloger-2                       52.2k ± 0%     52.2k ± 0%     ~     (p=0.452 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                      4.82k ± 0%     4.83k ± 0%   +0.31%  (p=0.029 n=4+4)
ImagePackageCatalogers/go-module-binary-cataloger-2            15.0 ± 0%      15.0 ± 0%     ~     (all equal)

[spiffcs]

@wagoodman if I provide the path to the main package does the bouncer check things at the top level like internal or syft?

[spiffcs]

Self Review Comments

[spiffcs]

Because poweruser is being decorated with the packages flags output is now a valid option.

TODO: update the writer function so poweruser can do multiple formats. This test becomes invalid.

[luhring]

I want to make sure I follow what's happening here... I know we plan to remove the power-user command in the future. And IMHO it's not so bad if we support -o json for the command in the meantime (curious for your thoughts on this!).

Should we remove this test case if we don't plan to use it either now or later?

[spiffcs]

Good callout here as I did not circle back on this comment. The solution here is to just make -o json the ONLY valid format for power-user. We can then remove this test.

It's currently hard coded here:

syft/cmd/power_user.go

Lines 76 to 81 in c270ee2

	writer, err := sbom.NewWriter(
	sbom.NewWriterOption(
	syftjson.Format(),
	appConfig.File,
	),
	)

[luhring]

This is fantastic @spiffcs — what an upgrade for the code... 🚀

Approving this! ✅

It looks like there's still some follow-up needed here. I asked a couple of questions, and I saw there were a few existing questions/nits that aren't answered yet. But nothing that's left seems like a big lift at this point.

I also have two overall questions re: this PR:

1. Since main.go has been moved to another package, this breaks installation for users that build from source using go install github.com/anchore/syft@latest. We don't list this installation method in our README documentation, so no change is needed here. But it is a nice feature of Go 1.16+, and I've used it several times. Could we do a cursory check of uses of this installation method just so we have a heads up on potential broken workflows? (My guess is we're not using this internally.)

2. I see some PR comments that have "TODO", and the comments are resolved, and the code hasn't changed. I want to make sure I parse that correctly. Does that mean that no code change is needed at this point? Such as it was just a thing to look into, but you've done that outside of the code?

[spiffcs]

This is fantastic @spiffcs — what an upgrade for the code... 🚀

Approving this! ✅

It looks like there's still some follow-up needed here. I asked a couple of questions, and I saw there were a few existing questions/nits that aren't answered yet. But nothing that's left seems like a big lift at this point.

I also have two overall questions re: this PR:

1. Since main.go has been moved to another package, this breaks installation for users that build from source using go install github.com/anchore/syft@latest. We don't list this installation method in our README documentation, so no change is needed here. But it is a nice feature of Go 1.16+, and I've used it several times. Could we do a cursory check of uses of this installation method just so we have a heads up on potential broken workflows? (My guess is we're not using this internally.)

2. I see some PR comments that have "TODO", and the comments are resolved, and the code hasn't changed. I want to make sure I parse that correctly. Does that mean that no code change is needed at this point? Such as it was just a thing to look into, but you've done that outside of the code?

There are some I reconciled outside the code and realized they were not as big of a deal as when I made the comment. I think I responded to each conversation you commented on so we can follow up on those threads.

TLDR: you're right and thanks for pointing those out. I think they got resolved accidentally as I was going through some of my previous self review.

I also checked and we do not use the command you listed for install. When this merges I think we can update that to be

go install github.com/anchore/syft/cmd/syft@latest