Fix "bundle summary" to not lose warning messages (#3123)

## Changes
- Merge rendering logic of "bundle validate" and "bundle summary" into
one function.
- Now "bundle summary" logs all warning diagnostics to stderr like
"bundle validate" does.

## Why
Not rendering warnings is a bug IMO. These are not just validation
issues but could be real issues that are only related to summary and
don't show up in "bundle validate".

This is not the first time "bundle summary" differs from "bundle
validate" (#2990) so in this PR I
go beyond fixing and merge two code paths.

Author: denik

NEXT_CHANGELOG.md

@@ -10,5 +10,6 @@
 * Fixed an issue where running `databricks auth login` would remove the `cluster_id` field from profiles in `.databrickscfg`. The login process now preserves the `cluster_id` field. ([#2988](https://github.com/databricks/cli/pull/2988))
 
 ### Bundles
+- "bundle summary" now prints diagnostic warnings to stderr ([#3123](https://github.com/databricks/cli/pull/3123))
 
 ### API Changes

acceptance/bundle/deploy/mlops-stacks/output.txt

@@ -21,6 +21,14 @@ This directory contains an ML project based on the default
 [Databricks MLOps Stacks](https://github.com/databricks/mlops-stacks),
 
 >>> [CLI] bundle summary
+Warning: expected a string value, found null
+  at targets.dev.workspace.host
+  in databricks.yml:34:12
+
+Warning: unknown field: description
+  at resources.experiments.experiment
+  in resources/ml-artifacts-resource.yml:21:7
+
 Name: project_name_[UNIQUE_NAME]
 Target: dev
 Workspace:
@@ -75,14 +83,31 @@ Warning: unknown field: description
 
 
 >>> [CLI] bundle summary -o json
+Warning: expected a string value, found null
+  at targets.dev.workspace.host
+  in databricks.yml:34:12
+
+Warning: unknown field: description
+  at resources.experiments.experiment
+  in resources/ml-artifacts-resource.yml:21:7
+
 {
   "experiment_id": "[NUMID]",
   "model_id": "[dev [USERNAME]] dev-project_name_[UNIQUE_NAME]-model",
   "inference_job_id": "[NUMID]",
   "training_job_id": "[NUMID]"
 }
 
-=== Assert the batch inference job actually exists{
+=== Assert the batch inference job actually exists
+Warning: expected a string value, found null
+  at targets.dev.workspace.host
+  in databricks.yml:34:12
+
+Warning: unknown field: description
+  at resources.experiments.experiment
+  in resources/ml-artifacts-resource.yml:21:7
+
+{
   "name": "[dev [USERNAME]] dev-project_name_[UNIQUE_NAME]-batch-inference-job"
 }
 

acceptance/bundle/deploy/mlops-stacks/script

@@ -31,6 +31,6 @@ trace $CLI bundle deploy
 
 trace $CLI bundle summary -o json | jq -r '{experiment_id: .resources.experiments.experiment.id, model_id: .resources.models.model.id, inference_job_id: .resources.jobs.batch_inference_job.id, training_job_id: .resources.jobs.model_training_job.id}'
 
-title "Assert the batch inference job actually exists"
+title "Assert the batch inference job actually exists\n"
 JOB_ID=$($CLI bundle summary -o json | jq -r '.resources.jobs.batch_inference_job.id')
 $CLI jobs get "${JOB_ID}" | jq '{name: .settings.name}'

acceptance/bundle/python/restricted-execution/output.txt

@@ -16,6 +16,7 @@ value_from_env
 >>> musterr uv run [UV_ARGS] -q [CLI] bundle summary
 Error: Running Python code is not allowed when DATABRICKS_BUNDLE_RESTRICTED_CODE_EXECUTION is set
 
+
 Exit code (musterr): 1
 
 >>> musterr cat envs.txt

acceptance/bundle/variables/host/output.txt

@@ -50,4 +50,17 @@ Workspace:
 
 Found 1 error and 1 warning
 
+Exit code: 1
+
+>>> errcode [CLI] bundle summary
+Warning: Variable interpolation is not supported for fields that configure authentication
+  at workspace.host
+  in [TEST_TMP_DIR]/databricks.yml:10:9
+
+Interpolation is not supported for the field workspace.host. Please set
+the DATABRICKS_HOST environment variable if you wish to configure this field at runtime.
+
+Error: failed during request visitor: parse "https://${var.host}": invalid character "{" in host name
+
+
 Exit code: 1

acceptance/bundle/variables/host/script

@@ -1,2 +1,3 @@
 trace errcode $CLI bundle validate -o json
 trace errcode $CLI bundle validate
+trace errcode $CLI bundle summary

acceptance/bundle/variables/host/test.toml

@@ -0,0 +1,3 @@
+[[Repls]]
+Old = '\\'
+New = '/'

bundle/render/render_text_output.go

@@ -187,6 +187,9 @@ func RenderDiagnostics(out io.Writer, b *bundle.Bundle, diags diag.Diagnostics,
 }
 
 func RenderSummary(ctx context.Context, out io.Writer, b *bundle.Bundle) error {
+	if b == nil {
+		panic("internal error: b must not be nil")
+	}
 	if err := renderSummaryHeaderTemplate(out, b); err != nil {
 		return err
 	}

cmd/bundle/summary.go

@@ -1,21 +1,18 @@
 package bundle
 
 import (
-	"encoding/json"
 	"errors"
-	"fmt"
 	"os"
 	"path/filepath"
 
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/deploy/terraform"
 	"github.com/databricks/cli/bundle/phases"
-	"github.com/databricks/cli/bundle/render"
 	"github.com/databricks/cli/bundle/statemgmt"
 	"github.com/databricks/cli/cmd/bundle/utils"
 	"github.com/databricks/cli/cmd/root"
-	"github.com/databricks/cli/libs/flags"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/spf13/cobra"
 )
 
@@ -33,66 +30,57 @@ func newSummaryCommand() *cobra.Command {
 	cmd.Flags().MarkHidden("include-locations")
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
-		ctx := cmd.Context()
-		b, diags := utils.ConfigureBundleWithVariables(cmd)
-		if err := diags.Error(); err != nil {
-			return diags.Error()
-		}
-
-		diags = phases.Initialize(ctx, b)
-		if err := diags.Error(); err != nil {
-			return err
-		}
+		b, diags := prepareBundleForSummary(cmd, forcePull, includeLocations)
+		return renderBundle(cmd, b, diags, true)
+	}
 
-		cacheDir, err := terraform.Dir(ctx, b)
-		if err != nil {
-			return err
-		}
-		_, stateFileErr := os.Stat(filepath.Join(cacheDir, b.StateFilename()))
-		_, configFileErr := os.Stat(filepath.Join(cacheDir, terraform.TerraformConfigFileName))
-		noCache := errors.Is(stateFileErr, os.ErrNotExist) || errors.Is(configFileErr, os.ErrNotExist)
+	return cmd
+}
 
-		if forcePull || noCache {
-			diags = bundle.ApplySeq(ctx, b,
-				statemgmt.StatePull(),
-				terraform.Interpolate(),
-				terraform.Write(),
-			)
-			if err := diags.Error(); err != nil {
-				return err
-			}
-		}
+func prepareBundleForSummary(cmd *cobra.Command, forcePull, includeLocations bool) (*bundle.Bundle, diag.Diagnostics) {
+	ctx := cmd.Context()
+	b, diags := utils.ConfigureBundleWithVariables(cmd)
+	if err := diags.Error(); err != nil {
+		return nil, diags
+	}
 
-		diags = bundle.ApplySeq(ctx, b,
-			terraform.Load(),
-			mutator.InitializeURLs(),
-		)
+	diags = diags.Extend(phases.Initialize(ctx, b))
+	if err := diags.Error(); err != nil {
+		return nil, diags
+	}
 
-		// Include location information in the output if the flag is set.
-		if includeLocations {
-			diags = diags.Extend(bundle.Apply(ctx, b, mutator.PopulateLocations()))
-		}
+	cacheDir, err := terraform.Dir(ctx, b)
+	if err != nil {
+		return nil, diags
+	}
+	_, stateFileErr := os.Stat(filepath.Join(cacheDir, b.StateFilename()))
+	_, configFileErr := os.Stat(filepath.Join(cacheDir, terraform.TerraformConfigFileName))
+	noCache := errors.Is(stateFileErr, os.ErrNotExist) || errors.Is(configFileErr, os.ErrNotExist)
 
+	if forcePull || noCache {
+		diags = diags.Extend(bundle.ApplySeq(ctx, b,
+			statemgmt.StatePull(),
+			terraform.Interpolate(),
+			terraform.Write(),
+		))
 		if err := diags.Error(); err != nil {
-			return err
+			return nil, diags
 		}
+	}
 
-		switch root.OutputType(cmd) {
-		case flags.OutputText:
-			return render.RenderSummary(ctx, cmd.OutOrStdout(), b)
-		case flags.OutputJSON:
-			buf, err := json.MarshalIndent(b.Config.Value().AsAny(), "", "  ")
-			if err != nil {
-				return err
-			}
-			_, _ = cmd.OutOrStdout().Write(buf)
-			_, _ = cmd.OutOrStdout().Write([]byte{'\n'})
-		default:
-			return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
-		}
+	diags = diags.Extend(bundle.ApplySeq(ctx, b,
+		terraform.Load(),
+		mutator.InitializeURLs(),
+	))
 
-		return nil
+	// Include location information in the output if the flag is set.
+	if includeLocations {
+		diags = diags.Extend(bundle.Apply(ctx, b, mutator.PopulateLocations()))
 	}
 
-	return cmd
+	if err := diags.Error(); err != nil {
+		return nil, diags
+	}
+
+	return b, diags
 }

cmd/bundle/validate.go

@@ -12,6 +12,7 @@ import (
 	"github.com/databricks/cli/bundle/render"
 	"github.com/databricks/cli/cmd/bundle/utils"
 	"github.com/databricks/cli/cmd/root"
+	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/flags"
 	"github.com/spf13/cobra"
 )
@@ -63,41 +64,60 @@ func newValidateCommand() *cobra.Command {
 			diags = diags.Extend(bundle.Apply(ctx, b, mutator.PopulateLocations()))
 		}
 
-		switch root.OutputType(cmd) {
-		case flags.OutputText:
-			renderOpts := render.RenderOptions{RenderSummaryTable: true}
-			err := render.RenderDiagnostics(cmd.OutOrStdout(), b, diags, renderOpts)
-			if err != nil {
-				return fmt.Errorf("failed to render output: %w", err)
-			}
-
-			if diags.HasError() {
-				return root.ErrAlreadyPrinted
-			}
+		return renderBundle(cmd, b, diags, false)
+	}
 
-			return nil
-		case flags.OutputJSON:
-			renderOpts := render.RenderOptions{RenderSummaryTable: false}
-			err1 := render.RenderDiagnostics(cmd.ErrOrStderr(), b, diags, renderOpts)
-			err2 := renderJsonOutput(cmd, b)
+	return cmd
+}
 
+// This function is used to render results both for "bundle validate" and "bundle summary".
+// In JSON mode, there is no difference in rendering between these two (but there is a difference in how we prepare the bundle).
+// In non-JSON mode both "bundle validate" and "bundle summary" will print diagnostics to stderr but "bundle validate"
+// will also print "summary" message via RenderSummaryTable option.
+func renderBundle(cmd *cobra.Command, b *bundle.Bundle, diags diag.Diagnostics, withBundleSummary bool) error {
+	ctx := cmd.Context()
+	switch root.OutputType(cmd) {
+	case flags.OutputText:
+		// Confusingly RenderSummaryTable relates to "Validation OK" and related messages, it has nothing
+		// to do with "bundle summary" command and we don't want to show it in bundle summary command.
+		renderOpts := render.RenderOptions{RenderSummaryTable: !withBundleSummary}
+		err1 := render.RenderDiagnostics(cmd.OutOrStdout(), b, diags, renderOpts)
+		if b != nil && withBundleSummary {
+			// Now RenderSummary actually related to "bundle summary"
+			err2 := render.RenderSummary(ctx, cmd.OutOrStdout(), b)
 			if err2 != nil {
 				return err2
 			}
+		}
 
-			if err1 != nil {
-				return err1
-			}
+		if err1 != nil {
+			return err1
+		}
 
-			if diags.HasError() {
-				return root.ErrAlreadyPrinted
-			}
+		if diags.HasError() {
+			return root.ErrAlreadyPrinted
+		}
+
+		return nil
+	case flags.OutputJSON:
+		renderOpts := render.RenderOptions{RenderSummaryTable: false}
+		err1 := render.RenderDiagnostics(cmd.ErrOrStderr(), b, diags, renderOpts)
+		err2 := renderJsonOutput(cmd, b)
 
-			return nil
-		default:
-			return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
+		if err2 != nil {
+			return err2
 		}
-	}
 
-	return cmd
+		if err1 != nil {
+			return err1
+		}
+
+		if diags.HasError() {
+			return root.ErrAlreadyPrinted
+		}
+
+		return nil
+	default:
+		return fmt.Errorf("unknown output type %s", root.OutputType(cmd))
+	}
 }