Skip to content

Commit

Permalink
update(output): complete rework of the output system
Browse files Browse the repository at this point in the history
Old flags "--verbose" and "--disable-styling" have been deprecated.

Two new flags configure the output system:

* --log-level can be one of info, warn debug or trace.
* --log-format can be one of color, text, json.

The output is done using a logger that is used across all commands.
Having a unique logger guarantees a consistent format of the output.

Signed-off-by: Aldo Lacuku <[email protected]>
  • Loading branch information
alacuku authored and poiana committed Oct 25, 2023
1 parent c8359f3 commit b6678c2
Show file tree
Hide file tree
Showing 49 changed files with 1,082 additions and 414 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ test:
.PHONY: gci
gci:
ifeq (, $(shell which gci))
@go install github.com/daixiang0/gci@v0.9.0
@go install github.com/daixiang0/gci@v0.11.1
GCI=$(GOBIN)/gci
else
GCI=$(shell which gci)
Expand Down
26 changes: 11 additions & 15 deletions cmd/artifact/follow/follow.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ Examples:

// RunArtifactFollow executes the business logic for the artifact follow command.
func (o *artifactFollowOptions) RunArtifactFollow(ctx context.Context, args []string) error {
logger := o.Printer.Logger
// Retrieve configuration for follower
configuredFollower, err := config.Follower()
if err != nil {
Expand All @@ -278,15 +279,13 @@ func (o *artifactFollowOptions) RunArtifactFollow(ctx context.Context, args []st
}

var wg sync.WaitGroup
// Disable styling
o.Printer.DisableStylingf()
// For each artifact create a follower.
var followers = make(map[string]*follower.Follower, 0)
for _, a := range args {
if o.cron != "" {
o.Printer.Info.Printfln("Creating follower for %q, with check using cron %s", a, o.cron)
logger.Info("Creating follower", logger.Args("artifact", a, "cron", o.cron))
} else {
o.Printer.Info.Printfln("Creating follower for %q, with check every %s", a, o.every.String())
logger.Info("Creating follower", logger.Args("artifact", a, "check every", o.every.String()))
}
ref, err := o.IndexCache.ResolveReference(a)
if err != nil {
Expand All @@ -305,7 +304,6 @@ func (o *artifactFollowOptions) RunArtifactFollow(ctx context.Context, args []st
PluginsDir: o.pluginsDir,
ArtifactReference: ref,
PlainHTTP: o.PlainHTTP,
Verbose: o.IsVerbose(),
CloseChan: o.closeChan,
TmpDir: o.tmpDir,
FalcoVersions: o.versions,
Expand All @@ -319,19 +317,17 @@ func (o *artifactFollowOptions) RunArtifactFollow(ctx context.Context, args []st
wg.Add(1)
followers[ref] = fol
}
// Enable styling
o.Printer.EnableStyling()

for k, f := range followers {
o.Printer.Info.Printfln("Starting follower for %q", k)
logger.Info("Starting follower", logger.Args("artifact", k))
go f.Follow(ctx)
}

// Wait until we receive a signal to be terminated
<-ctx.Done()

// We are done, shutdown the followers.
o.Printer.DefaultText.Printfln("closing followers...")
logger.Info("Closing followers...")
close(o.closeChan)

// Wait for the followers to shutdown or that the timer expires.
Expand All @@ -344,9 +340,9 @@ func (o *artifactFollowOptions) RunArtifactFollow(ctx context.Context, args []st

select {
case <-doneChan:
o.Printer.DefaultText.Printfln("followers correctly stopped.")
logger.Info("Followers correctly stopped.")
case <-time.After(timeout):
o.Printer.DefaultText.Printfln("Timed out waiting for followers to exit")
logger.Info("Timed out waiting for followers to exit")
}

return nil
Expand Down Expand Up @@ -433,11 +429,11 @@ type backoffTransport struct {
func (bt *backoffTransport) RoundTrip(req *http.Request) (*http.Response, error) {
var err error
var resp *http.Response

logger := bt.Printer.Logger
bt.startTime = time.Now()
bt.attempts = 0

bt.Printer.Verbosef("Retrieving versions from Falco (timeout %s) ...", bt.Config.MaxDelay)
logger.Debug(fmt.Sprintf("Retrieving versions from Falco (timeout %s) ...", bt.Config.MaxDelay))

for {
resp, err = bt.Base.RoundTrip(req)
Expand All @@ -452,10 +448,10 @@ func (bt *backoffTransport) RoundTrip(req *http.Request) (*http.Response, error)
return resp, fmt.Errorf("timeout occurred while retrieving versions from Falco")
}

bt.Printer.Verbosef("error: %s. Trying again in %s", err.Error(), sleep.String())
logger.Debug(fmt.Sprintf("error: %s. Trying again in %s", err.Error(), sleep.String()))
time.Sleep(sleep)
} else {
bt.Printer.Verbosef("Successfully retrieved versions from Falco ...")
logger.Debug("Successfully retrieved versions from Falco")
return resp, err
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/artifact/info/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func NewArtifactInfoCmd(ctx context.Context, opt *options.Common) *cobra.Command

func (o *artifactInfoOptions) RunArtifactInfo(ctx context.Context, args []string) error {
var data [][]string
logger := o.Printer.Logger

client, err := ociutils.Client(true)
if err != nil {
Expand All @@ -75,7 +76,7 @@ func (o *artifactInfoOptions) RunArtifactInfo(ctx context.Context, args []string
if err != nil {
entry, ok := o.IndexCache.MergedIndexes.EntryByName(name)
if !ok {
o.Printer.Warning.Printfln("cannot find %q, skipping", name)
logger.Warn("Cannot find artifact, skipping", logger.Args("name", name))
continue
}
ref = fmt.Sprintf("%s/%s", entry.Registry, entry.Repository)
Expand All @@ -93,7 +94,7 @@ func (o *artifactInfoOptions) RunArtifactInfo(ctx context.Context, args []string

tags, err := repo.Tags(ctx)
if err != nil && !errors.Is(err, context.Canceled) {
o.Printer.Warning.Printfln("cannot retrieve tags from t %q, %v", ref, err)
logger.Warn("Cannot retrieve tags from", logger.Args("ref", ref, "reason", err.Error()))
continue
} else if errors.Is(err, context.Canceled) {
// When the context is canceled we exit, since we receive a termination signal.
Expand Down
27 changes: 19 additions & 8 deletions cmd/artifact/install/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"path/filepath"
"runtime"

"github.com/pterm/pterm"
"github.com/spf13/cobra"
"github.com/spf13/viper"

Expand Down Expand Up @@ -183,6 +184,9 @@ Examples:

// RunArtifactInstall executes the business logic for the artifact install command.
func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []string) error {
var sp *pterm.SpinnerPrinter

logger := o.Printer.Logger
// Retrieve configuration for installer
configuredInstaller, err := config.Installer()
if err != nil {
Expand Down Expand Up @@ -244,7 +248,7 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
var refs []string
if o.resolveDeps {
// Solve dependencies
o.Printer.Info.Println("Resolving dependencies ...")
logger.Info("Resolving dependencies ...")
refs, err = ResolveDeps(resolver, args...)
if err != nil {
return err
Expand All @@ -253,15 +257,15 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
refs = args
}

o.Printer.Info.Printfln("Installing the following artifacts: %v", refs)
logger.Info("Installing artifacts", logger.Args("refs", refs))

for _, ref := range refs {
ref, err = o.IndexCache.ResolveReference(ref)
if err != nil {
return err
}

o.Printer.Info.Printfln("Preparing to pull %q", ref)
logger.Info("Preparing to pull artifact", logger.Args("ref", ref))

if err := puller.CheckAllowedType(ctx, ref, o.allowedTypes.Types); err != nil {
return err
Expand Down Expand Up @@ -290,12 +294,12 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
// the exact digest that we just pulled, even if the tag gets overwritten in the meantime.
digestRef := fmt.Sprintf("%s@%s", repo, result.RootDigest)

o.Printer.Info.Printfln("Verifying signature for %s", digestRef)
logger.Info("Verifying signature for artifact", logger.Args("digest", digestRef))
err = signature.Verify(ctx, digestRef, sig)
if err != nil {
return fmt.Errorf("error while verifying signature for %s: %w", digestRef, err)
}
o.Printer.Info.Printfln("Signature successfully verified!")
logger.Info("Signature successfully verified!")
}

var destDir string
Expand All @@ -312,14 +316,18 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
return fmt.Errorf("cannot use directory %q as install destination: %w", destDir, err)
}

sp, _ := o.Printer.Spinner.Start(fmt.Sprintf("INFO: Extracting and installing %q %q", result.Type, result.Filename))
logger.Info("Extracting and installing artifact", logger.Args("type", result.Type, "file", result.Filename))

if !o.Printer.DisableStyling {
sp, _ = o.Printer.Spinner.Start("Extracting and installing")
}

result.Filename = filepath.Join(tmpDir, result.Filename)

f, err := os.Open(result.Filename)
if err != nil {
return err
}

// Extract artifact and move it to its destination directory
_, err = utils.ExtractTarGz(f, destDir)
if err != nil {
Expand All @@ -331,7 +339,10 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
return err
}

sp.Success(fmt.Sprintf("Artifact successfully installed in %q", destDir))
if sp != nil {
_ = sp.Stop()
}
logger.Info("Artifact successfully installed", logger.Args("name", ref, "type", result.Type, "digest", result.Digest, "directory", destDir))
}

return nil
Expand Down
3 changes: 1 addition & 2 deletions cmd/artifact/install/install_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ var _ = BeforeSuite(func() {
// Create and configure the common options.
opt = commonoptions.NewOptions()
opt.Initialize(commonoptions.WithWriter(output))
opt.Printer.DisableStylingf()

// Create the oras registry.
orasRegistry, err = testutils.NewOrasRegistry(registry, true)
Expand All @@ -98,5 +97,5 @@ var _ = AfterSuite(func() {
func executeRoot(args []string) error {
rootCmd.SetArgs(args)
rootCmd.SetOut(output)
return cmd.Execute(rootCmd, opt.Printer)
return cmd.Execute(rootCmd, opt)
}
24 changes: 12 additions & 12 deletions cmd/artifact/install/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ Flags:
Global Flags:
--config string config file to be used for falcoctl (default "/etc/falcoctl/falcoctl.yaml")
--disable-styling Disable output styling such as spinners, progress bars and colors. Styling is automatically disabled if not attacched to a tty (default false)
-v, --verbose Enable verbose logs (default false)
--log-format string Set formatting for logs (color, text, json) (default "color")
--log-level string Set level for logs (info, warn, debug, trace) (default "info")
`

Expand Down Expand Up @@ -170,7 +170,7 @@ var artifactInstallTests = Describe("install", func() {
args = []string{artifactCmd, installCmd, "--config", configFile}
})
installAssertFailedBehavior(artifactInstallUsage,
"ERRO: no artifacts to install, please configure artifacts or pass them as arguments to this command")
"ERROR no artifacts to install, please configure artifacts or pass them as arguments to this command")
})

When("unreachable registry", func() {
Expand All @@ -181,7 +181,7 @@ var artifactInstallTests = Describe("install", func() {
Expect(err).To(BeNil())
args = []string{artifactCmd, installCmd, "noregistry/testrules", "--plain-http", "--config", configFile}
})
installAssertFailedBehavior(artifactInstallUsage, `ERRO: unable to fetch reference`)
installAssertFailedBehavior(artifactInstallUsage, `ERROR unable to fetch reference`)
})

When("invalid repository", func() {
Expand All @@ -193,7 +193,7 @@ var artifactInstallTests = Describe("install", func() {
Expect(err).To(BeNil())
args = []string{artifactCmd, installCmd, newReg, "--plain-http", "--config", configFile}
})
installAssertFailedBehavior(artifactInstallUsage, fmt.Sprintf("ERRO: unable to fetch reference %q", newReg))
installAssertFailedBehavior(artifactInstallUsage, fmt.Sprintf("ERROR unable to fetch reference %q", newReg))
})

When("with disallowed types (rulesfile)", func() {
Expand Down Expand Up @@ -222,7 +222,7 @@ var artifactInstallTests = Describe("install", func() {
"--config", configFilePath, "--allowed-types", "rulesfile"}
})

installAssertFailedBehavior(artifactInstallUsage, "ERRO: cannot download artifact of type \"plugin\": type not permitted")
installAssertFailedBehavior(artifactInstallUsage, "ERROR cannot download artifact of type \"plugin\": type not permitted")
})

When("with disallowed types (plugin)", func() {
Expand Down Expand Up @@ -251,7 +251,7 @@ var artifactInstallTests = Describe("install", func() {
"--config", configFilePath, "--allowed-types", "plugin"}
})

installAssertFailedBehavior(artifactInstallUsage, "ERRO: cannot download artifact of type \"rulesfile\": type not permitted")
installAssertFailedBehavior(artifactInstallUsage, "ERROR cannot download artifact of type \"rulesfile\": type not permitted")
})

When("an unknown type is used", func() {
Expand Down Expand Up @@ -281,7 +281,7 @@ var artifactInstallTests = Describe("install", func() {
"--config", configFilePath, "--allowed-types", "plugin," + wrongType}
})

installAssertFailedBehavior(artifactInstallUsage, fmt.Sprintf("ERRO: invalid argument \"plugin,%s\" for \"--allowed-types\" flag: "+
installAssertFailedBehavior(artifactInstallUsage, fmt.Sprintf("ERROR invalid argument \"plugin,%s\" for \"--allowed-types\" flag: "+
"not valid token %q: must be one of \"rulesfile\", \"plugin\"", wrongType, wrongType))
})

Expand Down Expand Up @@ -315,7 +315,7 @@ var artifactInstallTests = Describe("install", func() {
})

It("check that fails and the usage is not printed", func() {
expectedError := fmt.Sprintf("ERRO: cannot use directory %q "+
expectedError := fmt.Sprintf("ERROR cannot use directory %q "+
"as install destination: %s is not writable", destDir, destDir)
Expect(err).To(HaveOccurred())
Expect(output).ShouldNot(gbytes.Say(regexp.QuoteMeta(artifactInstallUsage)))
Expand Down Expand Up @@ -353,7 +353,7 @@ var artifactInstallTests = Describe("install", func() {
})

It("check that fails and the usage is not printed", func() {
expectedError := fmt.Sprintf("ERRO: cannot use directory %q "+
expectedError := fmt.Sprintf("ERROR cannot use directory %q "+
"as install destination: %s doesn't exists", destDir, destDir)
Expect(err).To(HaveOccurred())
Expect(output).ShouldNot(gbytes.Say(regexp.QuoteMeta(artifactInstallUsage)))
Expand Down Expand Up @@ -391,7 +391,7 @@ var artifactInstallTests = Describe("install", func() {
})

It("check that fails and the usage is not printed", func() {
expectedError := fmt.Sprintf("ERRO: cannot use directory %q "+
expectedError := fmt.Sprintf("ERROR cannot use directory %q "+
"as install destination: %s is not writable", destDir, destDir)
Expect(err).To(HaveOccurred())
Expect(output).ShouldNot(gbytes.Say(regexp.QuoteMeta(artifactInstallUsage)))
Expand Down Expand Up @@ -429,7 +429,7 @@ var artifactInstallTests = Describe("install", func() {
})

It("check that fails and the usage is not printed", func() {
expectedError := fmt.Sprintf("ERRO: cannot use directory %q "+
expectedError := fmt.Sprintf("ERROR cannot use directory %q "+
"as install destination: %s doesn't exists", destDir, destDir)
Expect(err).To(HaveOccurred())
Expect(output).ShouldNot(gbytes.Say(regexp.QuoteMeta(artifactInstallUsage)))
Expand Down
2 changes: 1 addition & 1 deletion cmd/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ var tests = []testCase{

func run(t *testing.T, test *testCase) {
// Setup
c := New(context.Background(), &options.Common{})
c := New(context.Background(), options.NewOptions())
o := bytes.NewBufferString("")
c.SetOut(o)
c.SetErr(o)
Expand Down
11 changes: 6 additions & 5 deletions cmd/index/add/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func NewIndexAddCmd(ctx context.Context, opt *options.Common) *cobra.Command {
// RunIndexAdd implements the index add command.
func (o *IndexAddOptions) RunIndexAdd(ctx context.Context, args []string) error {
var err error
logger := o.Printer.Logger

name := args[0]
url := args[1]
Expand All @@ -64,24 +65,24 @@ func (o *IndexAddOptions) RunIndexAdd(ctx context.Context, args []string) error
backend = args[2]
}

o.Printer.Verbosef("Creating in-memory cache using indexes file %q and indexes directory %q", config.IndexesFile, config.IndexesDir)
logger.Debug("Creating in-memory cache using", logger.Args("indexes file", config.IndexesFile, "indexes directory", config.IndexesDir))
indexCache, err := cache.New(ctx, config.IndexesFile, config.IndexesDir)
if err != nil {
return fmt.Errorf("unable to create index cache: %w", err)
}

o.Printer.Info.Printfln("Adding index")
logger.Info("Adding index", logger.Args("name", name, "path", url))

if err = indexCache.Add(ctx, name, backend, url); err != nil {
return fmt.Errorf("unable to add index: %w", err)
}

o.Printer.Verbosef("Writing cache to disk")
logger.Debug("Writing cache to disk")
if _, err = indexCache.Write(); err != nil {
return fmt.Errorf("unable to write cache to disk: %w", err)
}

o.Printer.Verbosef("Adding new index entry to configuration file %q", o.ConfigFile)
logger.Debug("Adding new index entry to configuration", logger.Args("file", o.ConfigFile))
if err = config.AddIndexes([]config.Index{{
Name: name,
URL: url,
Expand All @@ -90,7 +91,7 @@ func (o *IndexAddOptions) RunIndexAdd(ctx context.Context, args []string) error
return fmt.Errorf("index entry %q: %w", name, err)
}

o.Printer.Success.Printfln("Index %q successfully added", name)
logger.Info("Index successfully added")

return nil
}
Loading

0 comments on commit b6678c2

Please sign in to comment.