Fix: log level and color level should be configurable via the env (#1314

) * Fix: log level and color level should be configurable via the env Signed-off-by: Natalie Arellano <narellano@vmware.com> * Fix version Signed-off-by: Natalie Arellano <narellano@vmware.com> * Update order and comment Signed-off-by: Natalie Arellano <narellano@vmware.com> --------- Signed-off-by: Natalie Arellano <narellano@vmware.com>
buildpacks · Mar 12, 2024 · 435d226 · 435d226
1 parent 3a05fde
commit 435d226
Show file tree

Hide file tree

Showing 11 changed files with 66 additions and 63 deletions.
diff --git a/cmd/lifecycle/analyzer.go b/cmd/lifecycle/analyzer.go
@@ -25,17 +25,14 @@ type analyzeCmd struct {
 
 // DefineFlags defines the flags that are considered valid and reads their values (if provided).
 func (a *analyzeCmd) DefineFlags() {
-	if a.PlatformAPI.LessThan("0.12") {
-		cli.FlagStackPath(&a.StackPath)
-	}
 	switch {
 	case a.PlatformAPI.AtLeast("0.13"):
 		cli.FlagInsecureRegistries(&a.InsecureRegistries)
 		fallthrough
 	case a.PlatformAPI.AtLeast("0.12"):
 		cli.FlagLayoutDir(&a.LayoutDir)
-		cli.FlagUseLayout(&a.UseLayout)
 		cli.FlagRunPath(&a.RunPath)
+		cli.FlagUseLayout(&a.UseLayout)
 		fallthrough
 	case a.PlatformAPI.AtLeast("0.9"):
 		cli.FlagLaunchCacheDir(&a.LaunchCacheDir)
@@ -46,12 +43,18 @@ func (a *analyzeCmd) DefineFlags() {
 		cli.FlagCacheImage(&a.CacheImageRef)
 		cli.FlagGID(&a.GID)
 		cli.FlagLayersDir(&a.LayersDir)
+		cli.FlagLogLevel(&a.LogLevel)
+		cli.FlagNoColor(&a.NoColor)
 		cli.FlagPreviousImage(&a.PreviousImageRef)
 		cli.FlagRunImage(&a.RunImageRef)
 		cli.FlagTags(&a.AdditionalTags)
 		cli.FlagUID(&a.UID)
 		cli.FlagUseDaemon(&a.UseDaemon)
 	}
+	// deprecated
+	if a.PlatformAPI.LessThan("0.12") {
+		cli.FlagStackPath(&a.StackPath)
+	}
 }
 
 // Args validates arguments and flags, and fills in default values.

diff --git a/cmd/lifecycle/builder.go b/cmd/lifecycle/builder.go
@@ -31,6 +31,8 @@ func (b *buildCmd) DefineFlags() {
 		cli.FlagBuildpacksDir(&b.BuildpacksDir)
 		cli.FlagGroupPath(&b.GroupPath)
 		cli.FlagLayersDir(&b.LayersDir)
+		cli.FlagLogLevel(&b.LogLevel)
+		cli.FlagNoColor(&b.NoColor)
 		cli.FlagPlanPath(&b.PlanPath)
 		cli.FlagPlatformDir(&b.PlatformDir)
 	}

diff --git a/cmd/lifecycle/cli/command.go b/cmd/lifecycle/cli/command.go
@@ -14,6 +14,9 @@ type Command interface {
 	// DefineFlags defines the flags that are considered valid and reads their values (if provided)
 	DefineFlags()
 
+	// Inputs returns the platform inputs
+	Inputs() platform.LifecycleInputs
+
 	// Args validates arguments and flags, and fills in default values
 	Args(nargs int, args []string) error
 
@@ -25,16 +28,15 @@ type Command interface {
 }
 
 func Run(c Command, withPhaseName string, asSubcommand bool) {
-	var (
-		printVersion bool
-		logLevel     string
-		noColor      bool
-	)
-
 	log.SetOutput(io.Discard)
+
+	var printVersion bool
 	FlagVersion(&printVersion)
-	FlagLogLevel(&logLevel)
-	FlagNoColor(&noColor)
+
+	// DefineFlags (along with any function FlagXXX) defines the flags that are considered valid,
+	// but does not read the provided values; this is done by `flagSet.Parse`.
+	// The command `c` (e.g., detectCmd) is at this point already populated with platform inputs from the environment and/or default values,
+	// so command-line flags always take precedence.
 	c.DefineFlags()
 	if asSubcommand {
 		if err := flagSet.Parse(os.Args[2:]); err != nil {
@@ -47,22 +49,25 @@ func Run(c Command, withPhaseName string, asSubcommand bool) {
 			cmd.Exit(err)
 		}
 	}
-	cmd.DisableColor(noColor)
 
 	if printVersion {
 		cmd.ExitWithVersion()
 	}
-	if err := cmd.DefaultLogger.SetLevel(logLevel); err != nil {
+
+	cmd.DisableColor(c.Inputs().NoColor)
+	if err := cmd.DefaultLogger.SetLevel(c.Inputs().LogLevel); err != nil {
 		cmd.Exit(err)
 	}
-	cmd.DefaultLogger.Debugf("Starting %s...", withPhaseName)
 
+	// We print a warning here, so we should disable color if needed and set the log level before exercising this logic.
 	for _, arg := range flagSet.Args() {
 		if arg[0:1] == "-" {
 			cmd.DefaultLogger.Warnf("Warning: unconsumed flag-like positional arg: \n\t%s\n\t This will not be interpreted as a flag.\n\t Did you mean to put this before the first positional argument?", arg)
 		}
 	}
 
+	cmd.DefaultLogger.Debugf("Starting %s...", withPhaseName)
+
 	// Warn when CNB_PLATFORM_API is unset
 	if os.Getenv(platform.EnvPlatformAPI) == "" {
 		cmd.DefaultLogger.Warnf("%s is unset; using Platform API version '%s'", platform.EnvPlatformAPI, platform.DefaultPlatformAPI)

diff --git a/cmd/lifecycle/cli/flags.go b/cmd/lifecycle/cli/flags.go
@@ -2,12 +2,9 @@ package cli
 
 import (
 	"flag"
-	"os"
-	"strconv"
 	"time"
 
 	"github.com/buildpacks/lifecycle/internal/str"
-	"github.com/buildpacks/lifecycle/platform"
 )
 
 var flagSet = flag.NewFlagSet("lifecycle", flag.ExitOnError)
@@ -93,11 +90,11 @@ func FlagLayersDir(layersDir *string) {
 }
 
 func FlagLogLevel(logLevel *string) {
-	flagSet.StringVar(logLevel, "log-level", platform.DefaultLogLevel, "logging level")
+	flagSet.StringVar(logLevel, "log-level", *logLevel, "logging level")
 }
 
 func FlagNoColor(noColor *bool) {
-	flagSet.BoolVar(noColor, "no-color", boolEnv(platform.EnvNoColor), "disable color output")
+	flagSet.BoolVar(noColor, "no-color", *noColor, "disable color output")
 }
 
 func FlagOrderPath(orderPath *string) {
@@ -180,17 +177,7 @@ func FlagInsecureRegistries(insecureRegistries *str.Slice) {
 
 // deprecated
 
+// DeprecatedFlagRunImage sets the run image
 func DeprecatedFlagRunImage(deprecatedRunImage *string) {
 	flagSet.StringVar(deprecatedRunImage, "image", "", "[deprecated] reference to run image")
 }
-
-// helpers
-
-func boolEnv(k string) bool {
-	v := os.Getenv(k)
-	b, err := strconv.ParseBool(v)
-	if err != nil {
-		return false
-	}
-	return b
-}
diff --git a/cmd/lifecycle/creator.go b/cmd/lifecycle/creator.go
@@ -29,16 +29,14 @@ type createCmd struct {
 
 // DefineFlags defines the flags that are considered valid and reads their values (if provided).
 func (c *createCmd) DefineFlags() {
+	if c.PlatformAPI.AtLeast("0.13") {
+		cli.FlagInsecureRegistries(&c.InsecureRegistries)
+	}
 	if c.PlatformAPI.AtLeast("0.12") {
 		cli.FlagLayoutDir(&c.LayoutDir)
 		cli.FlagUseLayout(&c.UseLayout)
 		cli.FlagRunPath(&c.RunPath)
 	}
-
-	if c.PlatformAPI.AtLeast("0.13") {
-		cli.FlagInsecureRegistries(&c.InsecureRegistries)
-	}
-
 	if c.PlatformAPI.AtLeast("0.11") {
 		cli.FlagBuildConfigDir(&c.BuildConfigDir)
 		cli.FlagLauncherSBOMDir(&c.LauncherSBOMDir)
@@ -51,6 +49,8 @@ func (c *createCmd) DefineFlags() {
 	cli.FlagLaunchCacheDir(&c.LaunchCacheDir)
 	cli.FlagLauncherPath(&c.LauncherPath)
 	cli.FlagLayersDir(&c.LayersDir)
+	cli.FlagLogLevel(&c.LogLevel)
+	cli.FlagNoColor(&c.NoColor)
 	cli.FlagOrderPath(&c.OrderPath)
 	cli.FlagParallelExport(&c.ParallelExport)
 	cli.FlagPlatformDir(&c.PlatformDir)

diff --git a/cmd/lifecycle/detector.go b/cmd/lifecycle/detector.go
@@ -33,6 +33,8 @@ func (d *detectCmd) DefineFlags() {
 	cli.FlagBuildpacksDir(&d.BuildpacksDir)
 	cli.FlagGroupPath(&d.GroupPath)
 	cli.FlagLayersDir(&d.LayersDir)
+	cli.FlagLogLevel(&d.LogLevel)
+	cli.FlagNoColor(&d.NoColor)
 	cli.FlagOrderPath(&d.OrderPath)
 	cli.FlagPlanPath(&d.PlanPath)
 	cli.FlagPlatformDir(&d.PlatformDir)

diff --git a/cmd/lifecycle/exporter.go b/cmd/lifecycle/exporter.go
@@ -49,19 +49,15 @@ type exportData struct {
 
 // DefineFlags defines the flags that are considered valid and reads their values (if provided).
 func (e *exportCmd) DefineFlags() {
+	if e.PlatformAPI.AtLeast("0.13") {
+		cli.FlagInsecureRegistries(&e.InsecureRegistries)
+	}
 	if e.PlatformAPI.AtLeast("0.12") {
 		cli.FlagExtendedDir(&e.ExtendedDir)
 		cli.FlagLayoutDir(&e.LayoutDir)
 		cli.FlagRunPath(&e.RunPath)
 		cli.FlagUseLayout(&e.UseLayout)
-	} else {
-		cli.FlagStackPath(&e.StackPath)
 	}
-
-	if e.PlatformAPI.AtLeast("0.13") {
-		cli.FlagInsecureRegistries(&e.InsecureRegistries)
-	}
-
 	if e.PlatformAPI.AtLeast("0.11") {
 		cli.FlagLauncherSBOMDir(&e.LauncherSBOMDir)
 	}
@@ -74,6 +70,8 @@ func (e *exportCmd) DefineFlags() {
 	cli.FlagLaunchCacheDir(&e.LaunchCacheDir)
 	cli.FlagLauncherPath(&e.LauncherPath)
 	cli.FlagLayersDir(&e.LayersDir)
+	cli.FlagLogLevel(&e.LogLevel)
+	cli.FlagNoColor(&e.NoColor)
 	cli.FlagParallelExport(&e.ParallelExport)
 	cli.FlagProcessType(&e.DefaultProcessType)
 	cli.FlagProjectMetadataPath(&e.ProjectMetadataPath)
@@ -82,7 +80,11 @@ func (e *exportCmd) DefineFlags() {
 	cli.FlagUID(&e.UID)
 	cli.FlagUseDaemon(&e.UseDaemon)
 
+	// deprecated
 	cli.DeprecatedFlagRunImage(&e.DeprecatedRunImageRef) // FIXME: this flag isn't valid on Platform 0.7 and later
+	if e.PlatformAPI.LessThan("0.12") {
+		cli.FlagStackPath(&e.StackPath)
+	}
 }
 
 // Args validates arguments and flags, and fills in default values.

diff --git a/cmd/lifecycle/extender.go b/cmd/lifecycle/extender.go
@@ -32,6 +32,8 @@ func (e *extendCmd) DefineFlags() {
 	cli.FlagGroupPath(&e.GroupPath)
 	cli.FlagKanikoCacheTTL(&e.KanikoCacheTTL)
 	cli.FlagLayersDir(&e.LayersDir)
+	cli.FlagLogLevel(&e.LogLevel)
+	cli.FlagNoColor(&e.NoColor)
 	cli.FlagPlanPath(&e.PlanPath)
 	cli.FlagPlatformDir(&e.PlatformDir)
 	cli.FlagUID(&e.UID)

diff --git a/cmd/lifecycle/rebaser.go b/cmd/lifecycle/rebaser.go
@@ -32,24 +32,23 @@ type rebaseCmd struct {
 
 // DefineFlags defines the flags that are considered valid and reads their values (if provided).
 func (r *rebaseCmd) DefineFlags() {
-	cli.FlagGID(&r.GID)
-	cli.FlagReportPath(&r.ReportPath)
-	cli.FlagRunImage(&r.RunImageRef)
-	cli.FlagUID(&r.UID)
-	cli.FlagUseDaemon(&r.UseDaemon)
-	cli.DeprecatedFlagRunImage(&r.DeprecatedRunImageRef)
-
-	if r.PlatformAPI.AtLeast("0.11") {
-		cli.FlagPreviousImage(&r.PreviousImageRef)
+	if r.PlatformAPI.AtLeast("0.13") {
+		cli.FlagInsecureRegistries(&r.InsecureRegistries)
 	}
-
 	if r.PlatformAPI.AtLeast("0.12") {
 		cli.FlagForceRebase(&r.ForceRebase)
 	}
-
-	if r.PlatformAPI.AtLeast("0.13") {
-		cli.FlagInsecureRegistries(&r.InsecureRegistries)
+	if r.PlatformAPI.AtLeast("0.11") {
+		cli.FlagPreviousImage(&r.PreviousImageRef)
 	}
+	cli.DeprecatedFlagRunImage(&r.DeprecatedRunImageRef)
+	cli.FlagGID(&r.GID)
+	cli.FlagLogLevel(&r.LogLevel)
+	cli.FlagNoColor(&r.NoColor)
+	cli.FlagReportPath(&r.ReportPath)
+	cli.FlagRunImage(&r.RunImageRef)
+	cli.FlagUID(&r.UID)
+	cli.FlagUseDaemon(&r.UseDaemon)
 }
 
 // Args validates arguments and flags, and fills in default values.

diff --git a/cmd/lifecycle/restorer.go b/cmd/lifecycle/restorer.go
@@ -37,27 +37,26 @@ type restoreCmd struct {
 
 // DefineFlags defines the flags that are considered valid and reads their values (if provided).
 func (r *restoreCmd) DefineFlags() {
+	if r.PlatformAPI.AtLeast("0.13") {
+		cli.FlagInsecureRegistries(&r.InsecureRegistries)
+	}
 	if r.PlatformAPI.AtLeast("0.12") {
 		cli.FlagUseDaemon(&r.UseDaemon)
 		cli.FlagGeneratedDir(&r.GeneratedDir)
 		cli.FlagUseLayout(&r.UseLayout)
 		cli.FlagLayoutDir(&r.LayoutDir)
 	}
-
-	if r.PlatformAPI.AtLeast("0.13") {
-		cli.FlagInsecureRegistries(&r.InsecureRegistries)
-	}
-
 	if r.PlatformAPI.AtLeast("0.10") {
 		cli.FlagBuildImage(&r.BuildImageRef)
 	}
-
 	cli.FlagAnalyzedPath(&r.AnalyzedPath)
 	cli.FlagCacheDir(&r.CacheDir)
 	cli.FlagCacheImage(&r.CacheImageRef)
 	cli.FlagGID(&r.GID)
 	cli.FlagGroupPath(&r.GroupPath)
 	cli.FlagLayersDir(&r.LayersDir)
+	cli.FlagLogLevel(&r.LogLevel)
+	cli.FlagNoColor(&r.NoColor)
 	cli.FlagSkipLayers(&r.SkipLayers)
 	cli.FlagUID(&r.UID)
 }

diff --git a/platform/lifecycle_inputs.go b/platform/lifecycle_inputs.go
@@ -54,8 +54,9 @@ type LifecycleInputs struct {
 	UID                   int
 	GID                   int
 	ForceRebase           bool
-	SkipLayers            bool
+	NoColor               bool
 	ParallelExport        bool
+	SkipLayers            bool
 	UseDaemon             bool
 	UseLayout             bool
 	AdditionalTags        str.Slice // str.Slice satisfies the `Value` interface required by the `flag` package
@@ -88,6 +89,7 @@ func NewLifecycleInputs(platformAPI *api.Version) *LifecycleInputs {
 		// Operator config
 
 		LogLevel:           envOrDefault(EnvLogLevel, DefaultLogLevel),
+		NoColor:            boolEnv(EnvNoColor),
 		PlatformAPI:        platformAPI,
 		ExtendKind:         envOrDefault(EnvExtendKind, DefaultExtendKind),
 		UseDaemon:          boolEnv(EnvUseDaemon),