From ef32a0a42e53e6249eeb788274c6fd9a0bac6a16 Mon Sep 17 00:00:00 2001 From: UncleGedd <42304551+UncleGedd@users.noreply.github.com> Date: Wed, 26 Jun 2024 12:13:03 -0500 Subject: [PATCH] feat: distinguish between stdout and stderr output (#730) --- CONTRIBUTING.md | 6 ++++++ adr/0008-cli-output.md | 26 ++++++++++++++++++++++++++ src/cmd/monitor/pepr.go | 2 +- src/pkg/bundle/inspect.go | 7 ++++++- src/test/e2e/bundle_test.go | 6 +++++- src/test/e2e/monitor_test.go | 12 ++++++------ 6 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 adr/0008-cli-output.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 364e87aa..aea63673 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -21,6 +21,9 @@ Fundamentally, software engineering is a communication problem; we write code fo - **User experience is paramount**: UDS CLI doesn't have a pretty UI (yet), but the core user-centered design principles that apply when building a frontend also apply to this CLI tool. First and foremost, features in UDS CLI should enhance workflows and make life easier for end users; if a feature doesn't accomplish this, it will be dropped. + +- **Design Decision**: We use [Architectural Decision Records](https://adr.github.io/) to document the design decisions that we make. These records live in the `adr` directory. We highly recommend reading through the existing ADRs to understand the context and decisions that have been made in the past, and to inform current development. + ### Continuous Delivery Continuous Delivery is core to our development philosophy. Check out [https://minimumcd.org](https://minimumcd.org/) for a good baseline agreement on what that means. @@ -34,6 +37,9 @@ Specifically: ## How to Contribute Please ensure there is a Gitub issue for your proposed change, this helps the UDS CLI team to understand the context of the change and to track the progress of the work. If there isn't an issue for your change, please create one before starting work. The recommended workflow for contributing is as follows: + +*Before starting development, we highly recommend reading through the UDS CLI [documentation](https://uds.defenseunicorns.com/cli/) and our [ADRs](./adr). + 1. **Fork this repo** and clone it locally 1. **Create a branch** for your changes 1. **Create, [test](#testing)** your changes diff --git a/adr/0008-cli-output.md b/adr/0008-cli-output.md new file mode 100644 index 00000000..97333ebd --- /dev/null +++ b/adr/0008-cli-output.md @@ -0,0 +1,26 @@ +# 8. UDS CLI Output + +Date: 26 June 2024 + +## Status + +Accepted + +## Context + +Today, UDS CLI outputs virtually all CLI output to `stderr`. However, as the team begins to implement CLI commands that are designed to be consumed by other tools or automation, we need to norm on where logs are sent + +## Alternatives + +1. **`stderr`**: Continue to output all logs to `stderr`. This is the current behavior and is the simplest to implement. +2. **`stdout`**: Output all logs to `stdout`. This is the most common behavior for CLI tools and is the most likely to be consumed by other tools or automation. +3. **`stdout` and `stderr`**: Strategically output logs to both `stdout` and `stderr`. This is the most flexible option, but is slightly more code to implement. + +## Decision + +We will strategically output CLI messages to both `stdout` and `stderr`. Typical log output such as progress messages, spinners, etc, will be sent to `stderr`, and output that can be acted upon or is designed to be consumed by other tools will be sent to `stdout`. + +## Consequences + +The team needs to identify and refactor log output that is meant to be consumed by other tools and ensure it is sent to `stdout`. We will also need to ensure future CLI output adheres to this standard. +``` diff --git a/src/cmd/monitor/pepr.go b/src/cmd/monitor/pepr.go index 517b6681..4081ad92 100644 --- a/src/cmd/monitor/pepr.go +++ b/src/cmd/monitor/pepr.go @@ -65,7 +65,7 @@ var peprCmd = &cobra.Command{ // Create a new stream for the Pepr logs peprReader := pepr.NewStreamReader(namespace, "") - peprStream := stream.NewStream(os.Stderr, peprReader, "pepr-system") + peprStream := stream.NewStream(os.Stdout, peprReader, "pepr-system") // Set the stream flags peprReader.JSON = json diff --git a/src/pkg/bundle/inspect.go b/src/pkg/bundle/inspect.go index 6598b7e6..4a9f0d01 100644 --- a/src/pkg/bundle/inspect.go +++ b/src/pkg/bundle/inspect.go @@ -7,6 +7,7 @@ package bundle import ( "context" "fmt" + "io" "os" "path/filepath" "strings" @@ -40,7 +41,7 @@ func (b *Bundle) Inspect() error { // Check that provided oci source path is valid, and update it if it's missing the full path source, err := CheckOCISourcePath(b.cfg.InspectOpts.Source) if err != nil { - return err + return fmt.Errorf("source %s is either invalid or doesn't exist", b.cfg.InspectOpts.Source) } b.cfg.InspectOpts.Source = source @@ -95,6 +96,10 @@ func (b *Bundle) listImages() error { } formattedImgs := pterm.Color(color.FgHiMagenta).Sprintf(strings.Join(imgs, "\n")) + + // print to stdout to enable users to easily grab the output + // (and stderr for backwards compatibility) + pterm.SetDefaultOutput(io.MultiWriter(os.Stderr, os.Stdout)) pterm.Printfln("\n%s\n", formattedImgs) return nil } diff --git a/src/test/e2e/bundle_test.go b/src/test/e2e/bundle_test.go index add6ce3e..67c835f8 100644 --- a/src/test/e2e/bundle_test.go +++ b/src/test/e2e/bundle_test.go @@ -660,12 +660,16 @@ func TestListImages(t *testing.T) { t.Run("list images on bundle YAML only", func(t *testing.T) { cmd := strings.Split(fmt.Sprintf("inspect %s --list-images --insecure", filepath.Join(bundleDir, config.BundleYAML)), " ") - _, stderr, err := e2e.UDS(cmd...) + stdout, stderr, err := e2e.UDS(cmd...) require.NoError(t, err) require.Contains(t, stderr, "library/registry") require.Contains(t, stderr, "ghcr.io/defenseunicorns/zarf/agent") require.Contains(t, stderr, "nginx") require.Contains(t, stderr, "quay.io/prometheus/node-exporter") + require.Contains(t, stdout, "library/registry") + require.Contains(t, stdout, "ghcr.io/defenseunicorns/zarf/agent") + require.Contains(t, stdout, "nginx") + require.Contains(t, stdout, "quay.io/prometheus/node-exporter") // ensure non-req'd components got filtered require.NotContains(t, stderr, "grafana") diff --git a/src/test/e2e/monitor_test.go b/src/test/e2e/monitor_test.go index 75c5b6ab..199f7813 100644 --- a/src/test/e2e/monitor_test.go +++ b/src/test/e2e/monitor_test.go @@ -24,17 +24,17 @@ func TestMonitor(t *testing.T) { require.Error(t, err) t.Run("test mutated policies", func(t *testing.T) { - _, stderr, _ := e2e.UDS("monitor", "pepr", "mutated") - require.Contains(t, stderr, "✎ MUTATED podinfo") + stdout, _, _ := e2e.UDS("monitor", "pepr", "mutated") + require.Contains(t, stdout, "✎ MUTATED podinfo") }) t.Run("test allowed policies", func(t *testing.T) { - _, stderr, _ := e2e.UDS("monitor", "pepr", "allowed") - require.Contains(t, stderr, "✓ ALLOWED podinfo") + stdout, _, _ := e2e.UDS("monitor", "pepr", "allowed") + require.Contains(t, stdout, "✓ ALLOWED podinfo") }) t.Run("test denied policies", func(t *testing.T) { - _, stderr, _ := e2e.UDS("monitor", "pepr", "denied") - require.Contains(t, stderr, "✗ DENIED podinfo") + stdout, _, _ := e2e.UDS("monitor", "pepr", "denied") + require.Contains(t, stdout, "✗ DENIED podinfo") }) }