Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: render tables and yaml on stdout #3226

Merged
merged 12 commits into from
Dec 5, 2024
9 changes: 7 additions & 2 deletions src/cmd/common/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,19 @@ package common
import (
"context"
"fmt"
"os"
"path/filepath"

"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/fatih/color"

"github.com/zarf-dev/zarf/src/pkg/lint"
"github.com/zarf-dev/zarf/src/pkg/logger"
"github.com/zarf-dev/zarf/src/pkg/message"
)

// OutputWriter provides a writer to stdout for user-focused output
var OutputWriter = os.Stdout

// PrintFindings prints the findings in the LintError as a table.
func PrintFindings(ctx context.Context, lintErr *lint.LintError) {
mapOfFindingsByPath := lint.GroupFindingsByPath(lintErr.Findings, lintErr.PackageName)
Expand All @@ -42,9 +45,11 @@ func PrintFindings(ctx context.Context, lintErr *lint.LintError) {
} else {
packagePathFromUser = filepath.Join(lintErr.BaseDir, findings[0].PackagePathOverride)
}

// Print table to our OutputWriter
message.Notef("Linting package %q at %s", findings[0].PackageNameOverride, packagePathFromUser)
logger.From(ctx).Info("linting package", "name", findings[0].PackageNameOverride, "path", packagePathFromUser)
message.Table([]string{"Type", "Path", "Message"}, lintData)
message.TableWithWriter(OutputWriter, []string{"Type", "Path", "Message"}, lintData)
}
}

Expand Down
3 changes: 0 additions & 3 deletions src/cmd/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,6 @@ var connectListCmd = &cobra.Command{
if err != nil {
return err
}
// HACK: Re-initializing PTerm with a stderr writer isn't great, but it lets us render these
// tables for backwards compatibility
message.InitializePTerm(logger.DestinationDefault)
message.PrintConnectStringTable(connections)
return nil
},
Expand Down
10 changes: 0 additions & 10 deletions src/cmd/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,6 @@ var devFindImagesCmd = &cobra.Command{

var lintErr *lint.LintError
if errors.As(err, &lintErr) {
// HACK(mkcp): Re-initializing PTerm with a stderr writer isn't great, but it lets us render these lint
// tables below for backwards compatibility
if logger.Enabled(ctx) {
message.InitializePTerm(logger.DestinationDefault)
}
common.PrintFindings(ctx, lintErr)
}
if err != nil {
Expand Down Expand Up @@ -314,11 +309,6 @@ var devLintCmd = &cobra.Command{
err := lint.Validate(ctx, pkgConfig.CreateOpts.BaseDir, pkgConfig.CreateOpts.Flavor, pkgConfig.CreateOpts.SetVariables)
var lintErr *lint.LintError
if errors.As(err, &lintErr) {
// HACK(mkcp): Re-initializing PTerm with a stderr writer isn't great, but it lets us render these lint
// tables below for backwards compatibility
if logger.Enabled(ctx) {
message.InitializePTerm(logger.DestinationDefault)
}
common.PrintFindings(ctx, lintErr)
// Do not return an error if the findings are all warnings.
if lintErr.OnlyWarnings() {
Expand Down
12 changes: 2 additions & 10 deletions src/cmd/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"runtime"
"strings"

"github.com/zarf-dev/zarf/src/pkg/logger"

"github.com/AlecAivazis/survey/v2"
"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/spf13/cobra"
Expand All @@ -29,6 +27,7 @@ import (
"github.com/zarf-dev/zarf/src/internal/packager2"
"github.com/zarf-dev/zarf/src/pkg/cluster"
"github.com/zarf-dev/zarf/src/pkg/lint"
"github.com/zarf-dev/zarf/src/pkg/logger"
"github.com/zarf-dev/zarf/src/pkg/message"
"github.com/zarf-dev/zarf/src/pkg/packager"
"github.com/zarf-dev/zarf/src/pkg/packager/filters"
Expand Down Expand Up @@ -237,9 +236,6 @@ var packageInspectCmd = &cobra.Command{
if err != nil {
return fmt.Errorf("failed to inspect package: %w", err)
}
// HACK(mkcp): This init call ensures we still can still print Yaml when message is disabled. Remove when we
// release structured logged and don't have to disable message globally in pre-run.
message.InitializePTerm(logger.DestinationDefault)
err = utils.ColorPrintYAML(output, nil, false)
if err != nil {
return err
Expand Down Expand Up @@ -281,12 +277,8 @@ var packageListCmd = &cobra.Command{
})
}

// NOTE(mkcp): Renders table with message.
header := []string{"Package", "Version", "Components"}
// HACK(mkcp): Similar to `package inspect`, we do want to use message here but we have to make sure our feature
// flagging doesn't disable this. Nothing happens after this so it's safe, but still very hacky.
message.InitializePTerm(logger.DestinationDefault)
message.Table(header, packageData)
message.TableWithWriter(message.OutputWriter, header, packageData)

// Print out any unmarshalling errors
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions src/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ var (
SkipLogFile bool
// NoColor is a flag to disable colors in output
NoColor bool
// OutputWriter provides a default writer to Stdout for user-facing command output
OutputWriter = os.Stdout
)

var rootCmd = &cobra.Command{
Expand Down
2 changes: 1 addition & 1 deletion src/pkg/message/connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ func PrintConnectStringTable(connectStrings types.ConnectStrings) {

// Create the table output with the data
header := []string{"Connect Command", "Description"}
Table(header, connectData)
TableWithWriter(OutputWriter, header, connectData)
}
}
2 changes: 1 addition & 1 deletion src/pkg/message/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func PrintCredentialTable(state *types.ZarfState, componentsToDeploy []types.Dep

if len(loginData) > 0 {
header := []string{"Application", "Username", "Password", "Connect", "Get-Creds Key"}
Table(header, loginData)
TableWithWriter(OutputWriter, header, loginData)
}
}

Expand Down
36 changes: 23 additions & 13 deletions src/pkg/message/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,18 @@ const (
TermWidth = 100
)

// NoProgress tracks whether spinner/progress bars show updates.
var NoProgress bool

// RuleLine creates a line of ━ as wide as the terminal
var RuleLine = strings.Repeat("━", TermWidth)

// logLevel holds the pterm compatible log level integer
var logLevel = InfoLevel

// logFile acts as a buffer for logFile generation
var logFile *PausableWriter
var (
// NoProgress tracks whether spinner/progress bars show updates.
NoProgress bool
// RuleLine creates a line of ━ as wide as the terminal
RuleLine = strings.Repeat("━", TermWidth)
// OutputWriter provides a default writer to Stdout for user-focused output like tables and yaml
OutputWriter = os.Stdout
// logLevel holds the pterm compatible log level integer
logLevel = InfoLevel
// logFile acts as a buffer for logFile generation
logFile *PausableWriter
)

// DebugWriter represents a writer interface that writes to message.Debug
type DebugWriter struct{}
Expand Down Expand Up @@ -249,6 +250,11 @@ func Paragraphn(n int, format string, a ...any) string {

// Table prints a padded table containing the specified header and data
func Table(header []string, data [][]string) {
TableWithWriter(nil, header, data)
}

// TableWithWriter prints a padded table containing the specified header and data to the optional writer.
func TableWithWriter(writer io.Writer, header []string, data [][]string) {
pterm.Println()

// To avoid side effects make copies of the header and data before adding padding
Expand All @@ -271,8 +277,12 @@ func Table(header []string, data [][]string) {
table = append(table, pterm.TableData{row}...)
}

//nolint:errcheck // never returns an error
pterm.DefaultTable.WithHasHeader().WithData(table).Render()
// Use DefaultTable writer if none is provided
tPrinter := pterm.DefaultTable
if writer != nil {
tPrinter.Writer = writer
}
_ = tPrinter.WithHasHeader().WithData(table).Render() //nolint:errcheck
}

func debugPrinter(offset int, a ...any) {
Expand Down
4 changes: 2 additions & 2 deletions src/pkg/utils/yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ func ColorPrintYAML(data any, hints map[string]string, spaceRootLists bool) erro
outputYAML = ansiRegex.ReplaceAllString(outputYAML, "")
}

pterm.Println()
pterm.Println(outputYAML)
content := strings.Join([]string{"\n", outputYAML}, "")
pterm.Fprintln(message.OutputWriter, content)
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/e2e/06_create_sbom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestCreateSBOM(t *testing.T) {

stdOut, _, err := e2e.Zarf(t, "package", "inspect", tarPath, "--list-images")
require.NoError(t, err)
require.Equal(t, "- ghcr.io/zarf-dev/doom-game:0.0.1\n", stdOut)
require.Contains(t, stdOut, "- ghcr.io/zarf-dev/doom-game:0.0.1\n")

// Pull the current zarf binary version to find the corresponding init package
version, _, err := e2e.Zarf(t, "version")
Expand Down
29 changes: 15 additions & 14 deletions src/test/e2e/12_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,31 +31,32 @@ func TestLint(t *testing.T) {
configPath := filepath.Join(testPackagePath, "zarf-config.toml")
osSetErr := os.Setenv("ZARF_CONFIG", configPath)
require.NoError(t, osSetErr, "Unable to set ZARF_CONFIG")
_, stderr, err := e2e.Zarf(t, "dev", "lint", testPackagePath, "-f", "good-flavor")
stdOut, stdErr, err := e2e.Zarf(t, "dev", "lint", testPackagePath, "-f", "good-flavor")
osUnsetErr := os.Unsetenv("ZARF_CONFIG")
require.NoError(t, osUnsetErr, "Unable to cleanup ZARF_CONFIG")
require.Error(t, err, "Require an exit code since there was warnings / errors")
strippedStderr := e2e.StripMessageFormatting(stderr)
strippedStdOut := e2e.StripMessageFormatting(stdOut)
strippedStdErr := e2e.StripMessageFormatting(stdErr)

key := "WHATEVER_IMAGE"
require.Contains(t, strippedStderr, lang.UnsetVarLintWarning)
require.Contains(t, strippedStderr, fmt.Sprintf(lang.PkgValidateTemplateDeprecation, key, key, key))
require.Contains(t, strippedStderr, ".components.[2].repos.[0] | Unpinned repository")
require.Contains(t, strippedStderr, ".metadata | Additional property description1 is not allowed")
require.Contains(t, strippedStderr, ".components.[0].import | Additional property not-path is not allowed")
require.Contains(t, strippedStdOut, lang.UnsetVarLintWarning)
require.Contains(t, strippedStdOut, fmt.Sprintf(lang.PkgValidateTemplateDeprecation, key, key, key))
require.Contains(t, strippedStdOut, ".components.[2].repos.[0] | Unpinned repository")
require.Contains(t, strippedStdOut, ".metadata | Additional property description1 is not allowed")
require.Contains(t, strippedStdOut, ".components.[0].import | Additional property not-path is not allowed")
// Testing the import / compose on lint is working
require.Contains(t, strippedStderr, ".components.[1].images.[0] | Image not pinned with digest - registry.com:9001/whatever/image:latest")
require.Contains(t, strippedStdOut, ".components.[1].images.[0] | Image not pinned with digest - registry.com:9001/whatever/image:latest")
// Testing import / compose + variables are working
require.Contains(t, strippedStderr, ".components.[2].images.[3] | Image not pinned with digest - busybox:latest")
require.Contains(t, strippedStdOut, ".components.[2].images.[3] | Image not pinned with digest - busybox:latest")
// Testing OCI imports get linted
require.Contains(t, strippedStderr, ".components.[0].images.[0] | Image not pinned with digest - ghcr.io/zarf-dev/doom-game:0.0.1")
require.Contains(t, strippedStdOut, ".components.[0].images.[0] | Image not pinned with digest - ghcr.io/zarf-dev/doom-game:0.0.1")

// Check flavors
require.NotContains(t, strippedStderr, "image-in-bad-flavor-component:unpinned")
require.Contains(t, strippedStderr, "image-in-good-flavor-component:unpinned")
require.NotContains(t, strippedStdOut, "image-in-bad-flavor-component:unpinned")
require.Contains(t, strippedStdOut, "image-in-good-flavor-component:unpinned")

// Check reported filepaths
require.Contains(t, strippedStderr, "Linting package \"dos-games\" at oci://ghcr.io/zarf-dev/packages/dos-games:1.1.0")
require.Contains(t, strippedStderr, fmt.Sprintf("Linting package \"lint\" at %s", testPackagePath))
require.Contains(t, strippedStdErr, "Linting package \"dos-games\" at oci://ghcr.io/zarf-dev/packages/dos-games:1.1.0")
require.Contains(t, strippedStdErr, fmt.Sprintf("Linting package \"lint\" at %s", testPackagePath))
})
}
4 changes: 2 additions & 2 deletions src/test/e2e/14_oci_compose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (suite *PublishCopySkeletonSuite) Test_1_Compose_Everything_Inception() {
_, _, err = e2e.Zarf(suite.T(), "package", "create", importception, "-o", "build", "--plain-http", "--confirm")
suite.NoError(err)

_, stdErr, err := e2e.Zarf(suite.T(), "package", "inspect", importEverythingPath)
stdOut, _, err := e2e.Zarf(suite.T(), "package", "inspect", importEverythingPath)
suite.NoError(err)

targets := []string{
Expand All @@ -109,7 +109,7 @@ func (suite *PublishCopySkeletonSuite) Test_1_Compose_Everything_Inception() {
}

for _, target := range targets {
suite.Contains(stdErr, target)
suite.Contains(stdOut, target)
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/test/e2e/24_variables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ func TestVariables(t *testing.T) {
stdOut, stdErr, err = e2e.Zarf(t, "package", "deploy", path, "--confirm", "--set", "SITE_NAME=Lula Web", "--set", "AWS_REGION=unicorn-land", "-l", "trace")
require.NoError(t, err, stdOut, stdErr)
// Verify that the variables were shown to the user in the formats we expect
require.Contains(t, stdErr, "currently set to 'Defense Unicorns' (default)")
require.Contains(t, stdErr, "currently set to 'Lula Web'")
require.Contains(t, stdErr, "currently set to '**sanitized**'")
require.Contains(t, stdOut, "currently set to 'Defense Unicorns' (default)")
require.Contains(t, stdOut, "currently set to 'Lula Web'")
require.Contains(t, stdOut, "currently set to '**sanitized**'")
// Verify that the sensitive variable 'unicorn-land' was not printed to the screen
require.NotContains(t, stdErr, "unicorn-land")
require.NotContains(t, stdOut, "unicorn-land")

logText := e2e.GetLogFileContents(t, e2e.StripMessageFormatting(stdErr))
// Verify that the sensitive variable 'unicorn-land' was not included in the log
Expand Down
4 changes: 2 additions & 2 deletions src/test/e2e/25_helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ func testHelmChartsExample(t *testing.T) {
helmChartsPkg := filepath.Join("build", fmt.Sprintf("zarf-package-helm-charts-%s-0.0.1.tar.zst", e2e.Arch))
stdOut, stdErr, err = e2e.Zarf(t, "package", "deploy", helmChartsPkg, "--confirm")
require.NoError(t, err, stdOut, stdErr)
require.Contains(t, string(stdErr), "registryOverrides", "registry overrides was not saved to build data")
require.Contains(t, string(stdErr), "docker.io", "docker.io not found in registry overrides")
require.Contains(t, stdOut, "registryOverrides", "registry overrides was not saved to build data")
require.Contains(t, stdOut, "docker.io", "docker.io not found in registry overrides")

// Remove the example package.
stdOut, stdErr, err = e2e.Zarf(t, "package", "remove", "helm-charts", "--confirm")
Expand Down
Loading