From 707335e9172e94ccc4f006df39065c980960b92c Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Tue, 29 Oct 2024 13:48:33 -0700 Subject: [PATCH 1/5] ensure we print out a lint table even when logger is enabled Signed-off-by: Kit Patella --- src/cmd/dev.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 7540c2b365..8242771f7b 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -23,6 +23,7 @@ import ( "github.com/zarf-dev/zarf/src/config" "github.com/zarf-dev/zarf/src/config/lang" "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/transform" @@ -299,6 +300,11 @@ var devLintCmd = &cobra.Command{ err := lint.Validate(cmd.Context(), 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(cmd.Context()) { + message.InitializePTerm(logger.DestinationDefault) + } common.PrintFindings(lintErr) // Do not return an error if the findings are all warnings. if lintErr.OnlyWarnings() { From 62e08b66c36badae1fe011f3241aea52df3ca436 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Tue, 29 Oct 2024 15:08:02 -0700 Subject: [PATCH 2/5] add logger to dev find-images Signed-off-by: Kit Patella --- src/cmd/dev.go | 24 +++++++++++++++++++++--- src/pkg/packager/prepare.go | 25 +++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 8242771f7b..da3e2293c8 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -41,6 +41,7 @@ var devCmd = &cobra.Command{ Short: lang.CmdDevShort, } +// FIXME(mkcp): Lots of message var devDeployCmd = &cobra.Command{ Use: "deploy", Args: cobra.MaximumNArgs(1), @@ -74,6 +75,7 @@ var devDeployCmd = &cobra.Command{ }, } +// FIXME(mkcp): Lots of message var devGenerateCmd = &cobra.Command{ Use: "generate NAME", Aliases: []string{"g"}, @@ -105,8 +107,9 @@ var devTransformGitLinksCmd = &cobra.Command{ Aliases: []string{"p"}, Short: lang.CmdDevPatchGitShort, Args: cobra.ExactArgs(2), - RunE: func(_ *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, args []string) error { host, fileName := args[0], args[1] + ctx := cmd.Context() // Read the contents of the given file content, err := os.ReadFile(fileName) @@ -114,11 +117,20 @@ var devTransformGitLinksCmd = &cobra.Command{ return fmt.Errorf("unable to read the file %s: %w", fileName, err) } - pkgConfig.InitOpts.GitServer.Address = host + gitServer := pkgConfig.InitOpts.GitServer + gitServer.Address = host // Perform git url transformation via regex text := string(content) - processedText := transform.MutateGitURLsInText(message.Warnf, pkgConfig.InitOpts.GitServer.Address, text, pkgConfig.InitOpts.GitServer.PushUsername) + + // Set log func for transform + var logFn func(string, ...any) + logFn = message.Warnf + // Use logger if we can + if logger.Enabled(ctx) { + logFn = logger.From(ctx).Warn + } + processedText := transform.MutateGitURLsInText(logFn, gitServer.Address, text, gitServer.PushUsername) // Print the differences dmp := diffmatchpatch.New() @@ -252,8 +264,14 @@ var devFindImagesCmd = &cobra.Command{ defer pkgClient.ClearTempPaths() _, err = pkgClient.FindImages(cmd.Context()) + 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(cmd.Context()) { + message.InitializePTerm(logger.DestinationDefault) + } common.PrintFindings(lintErr) } if err != nil { diff --git a/src/pkg/packager/prepare.go b/src/pkg/packager/prepare.go index 1e43335898..9661c1ceb8 100644 --- a/src/pkg/packager/prepare.go +++ b/src/pkg/packager/prepare.go @@ -8,11 +8,13 @@ import ( "context" "errors" "fmt" + "github.com/zarf-dev/zarf/src/pkg/logger" "os" "path/filepath" "regexp" "sort" "strings" + "time" "github.com/defenseunicorns/pkg/helpers/v2" "github.com/goccy/go-yaml" @@ -40,6 +42,7 @@ var imageFuzzyCheck = regexp.MustCompile(`(?mi)["|=]([a-z0-9\-.\/:]+:[\w.\-]*[a- // FindImages iterates over a Zarf.yaml and attempts to parse any images. func (p *Packager) FindImages(ctx context.Context) (map[string][]string, error) { + l := logger.From(ctx) cwd, err := os.Getwd() if err != nil { return nil, err @@ -48,12 +51,14 @@ func (p *Packager) FindImages(ctx context.Context) (map[string][]string, error) // Return to the original working directory if err := os.Chdir(cwd); err != nil { message.Warnf("Unable to return to the original working directory: %s", err.Error()) + l.Warn("unable to return to the original working directory", "error", err) } }() if err := os.Chdir(p.cfg.CreateOpts.BaseDir); err != nil { return nil, fmt.Errorf("unable to access directory %q: %w", p.cfg.CreateOpts.BaseDir, err) } message.Note(fmt.Sprintf("Using build directory %s", p.cfg.CreateOpts.BaseDir)) + l.Info("using build directory", "path", p.cfg.CreateOpts.BaseDir) c := creator.NewPackageCreator(p.cfg.CreateOpts, cwd) @@ -67,6 +72,7 @@ func (p *Packager) FindImages(ctx context.Context) (map[string][]string, error) } for _, warning := range warnings { message.Warn(warning) + l.Warn(warning) } p.cfg.Pkg = pkg @@ -75,12 +81,15 @@ func (p *Packager) FindImages(ctx context.Context) (map[string][]string, error) // TODO: Refactor to return output string instead of printing inside of function. func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) { + l := logger.From(ctx) for _, component := range p.cfg.Pkg.Components { if len(component.Repos) > 0 && p.cfg.FindImagesOpts.RepoHelmChartPath == "" { - message.Note("This Zarf package contains git repositories, " + + msg := "This Zarf package contains git repositories, " + "if any repos contain helm charts you want to template and " + "search for images, make sure to specify the helm chart path " + - "via the --repo-chart-path flag") + "via the --repo-chart-path flag" + message.Note(msg) + l.Info(msg) break } } @@ -260,6 +269,8 @@ func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) } } + imgCompStart := time.Now() + l.Info("looking for image in component", "name", component.Name, "resourcesCount", len(resources)) spinner := message.NewProgressSpinner("Looking for images in component %q across %d resources", component.Name, len(resources)) defer spinner.Stop() @@ -288,9 +299,11 @@ func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) if descriptor, err := crane.Head(image, images.WithGlobalInsecureFlag()...); err != nil { // Test if this is a real image, if not just quiet log to debug, this is normal message.Debugf("Suspected image does not appear to be valid: %#v", err) + l.Debug("suspected image does not appear to be valid", "error", err) } else { // Otherwise, add to the list of images message.Debugf("Imaged digest found: %s", descriptor.Digest) + l.Debug("imaged digest found", "digest", descriptor.Digest) validImages = append(validImages, image) } } @@ -305,16 +318,23 @@ func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) } spinner.Success() + l.Debug("done looking for image in component", + "name", component.Name, + "resourcesCount", len(resources), + "duration", time.Since(imgCompStart)) if !p.cfg.FindImagesOpts.SkipCosign { // Handle cosign artifact lookups if len(imagesMap[component.Name]) > 0 { var cosignArtifactList []string + imgStart := time.Now() spinner := message.NewProgressSpinner("Looking up cosign artifacts for discovered images (0/%d)", len(imagesMap[component.Name])) defer spinner.Stop() + l.Info("looking up cosign artifacts for discovered images", "count", len(imagesMap[component.Name])) for idx, image := range imagesMap[component.Name] { spinner.Updatef("Looking up cosign artifacts for discovered images (%d/%d)", idx+1, len(imagesMap[component.Name])) + l.Debug("looking up cosign artifacts for image", "name", imagesMap[component.Name]) cosignArtifacts, err := utils.GetCosignArtifacts(image) if err != nil { return nil, fmt.Errorf("could not lookup the cosing artifacts for image %s: %w", image, err) @@ -323,6 +343,7 @@ func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) } spinner.Success() + l.Debug("done looking up cosign artifacts for discovered images", "count", len(imagesMap[component.Name]), "duration", time.Since(imgStart)) if len(cosignArtifactList) > 0 { imagesMap[component.Name] = append(imagesMap[component.Name], cosignArtifactList...) From 32d455bc4557d75d3a736276e55abb290cff9801 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Tue, 29 Oct 2024 15:12:57 -0700 Subject: [PATCH 3/5] remove some fixmes and add logger to dev sha256sum Signed-off-by: Kit Patella --- src/cmd/dev.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cmd/dev.go b/src/cmd/dev.go index da3e2293c8..0b500c4ea3 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -41,7 +41,6 @@ var devCmd = &cobra.Command{ Short: lang.CmdDevShort, } -// FIXME(mkcp): Lots of message var devDeployCmd = &cobra.Command{ Use: "deploy", Args: cobra.MaximumNArgs(1), @@ -75,7 +74,6 @@ var devDeployCmd = &cobra.Command{ }, } -// FIXME(mkcp): Lots of message var devGenerateCmd = &cobra.Command{ Use: "generate NAME", Aliases: []string{"g"}, @@ -173,6 +171,7 @@ var devSha256SumCmd = &cobra.Command{ if helpers.IsURL(fileName) { message.Warn(lang.CmdDevSha256sumRemoteWarning) + logger.From(cmd.Context()).Warn("this is a remote source. If a published checksum is available you should use that rather than calculating it directly from the remote link") fileBase, err := helpers.ExtractBasePathFromURL(fileName) if err != nil { From 71eae1c2ba51174e7f9f92651272a9cca83cd32d Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 30 Oct 2024 09:50:21 -0700 Subject: [PATCH 4/5] pluralize log messages dealing with images Signed-off-by: Kit Patella --- src/pkg/packager/prepare.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pkg/packager/prepare.go b/src/pkg/packager/prepare.go index 9661c1ceb8..784b3bedbd 100644 --- a/src/pkg/packager/prepare.go +++ b/src/pkg/packager/prepare.go @@ -270,7 +270,7 @@ func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) } imgCompStart := time.Now() - l.Info("looking for image in component", "name", component.Name, "resourcesCount", len(resources)) + l.Info("looking for images in component", "name", component.Name, "resourcesCount", len(resources)) spinner := message.NewProgressSpinner("Looking for images in component %q across %d resources", component.Name, len(resources)) defer spinner.Stop() @@ -318,7 +318,7 @@ func (p *Packager) findImages(ctx context.Context) (map[string][]string, error) } spinner.Success() - l.Debug("done looking for image in component", + l.Debug("done looking for images in component", "name", component.Name, "resourcesCount", len(resources), "duration", time.Since(imgCompStart)) From d7b2f0d2e50028fd53c07c98c6fcebf180976fc8 Mon Sep 17 00:00:00 2001 From: Kit Patella Date: Wed, 30 Oct 2024 09:59:52 -0700 Subject: [PATCH 5/5] fix transform log breaking on log fn pass-in Signed-off-by: Kit Patella --- src/cmd/dev.go | 14 ++++---------- src/pkg/transform/types.go | 1 + 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/cmd/dev.go b/src/cmd/dev.go index 0b500c4ea3..e8bffd57c0 100644 --- a/src/cmd/dev.go +++ b/src/cmd/dev.go @@ -105,9 +105,8 @@ var devTransformGitLinksCmd = &cobra.Command{ Aliases: []string{"p"}, Short: lang.CmdDevPatchGitShort, Args: cobra.ExactArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(_ *cobra.Command, args []string) error { host, fileName := args[0], args[1] - ctx := cmd.Context() // Read the contents of the given file content, err := os.ReadFile(fileName) @@ -121,16 +120,11 @@ var devTransformGitLinksCmd = &cobra.Command{ // Perform git url transformation via regex text := string(content) - // Set log func for transform - var logFn func(string, ...any) - logFn = message.Warnf - // Use logger if we can - if logger.Enabled(ctx) { - logFn = logger.From(ctx).Warn - } - processedText := transform.MutateGitURLsInText(logFn, gitServer.Address, text, gitServer.PushUsername) + // TODO(mkcp): Currently uses message for its log fn. Migrate to ctx and slog + processedText := transform.MutateGitURLsInText(message.Warnf, gitServer.Address, text, gitServer.PushUsername) // Print the differences + // TODO(mkcp): Uses pterm to print text diffs. Decouple from pterm after we release logger. dmp := diffmatchpatch.New() diffs := dmp.DiffMain(text, processedText, true) diffs = dmp.DiffCleanupSemantic(diffs) diff --git a/src/pkg/transform/types.go b/src/pkg/transform/types.go index dbf4922a0f..89cfaa391e 100644 --- a/src/pkg/transform/types.go +++ b/src/pkg/transform/types.go @@ -4,4 +4,5 @@ package transform // Log is a function that logs a message. +// TODO(mkcp): Remove Log and port over to logger once we remove message. type Log func(string, ...any)