Skip to content

Commit

Permalink
feat: distinguish between stdout and stderr output (#730)
Browse files Browse the repository at this point in the history
  • Loading branch information
UncleGedd authored Jun 26, 2024
1 parent 0e64f8b commit ef32a0a
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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
Expand Down
26 changes: 26 additions & 0 deletions adr/0008-cli-output.md
Original file line number Diff line number Diff line change
@@ -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.
```
2 changes: 1 addition & 1 deletion src/cmd/monitor/pepr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion src/pkg/bundle/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package bundle
import (
"context"
"fmt"
"io"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
}
Expand Down
6 changes: 5 additions & 1 deletion src/test/e2e/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
12 changes: 6 additions & 6 deletions src/test/e2e/monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}

0 comments on commit ef32a0a

Please sign in to comment.