Surface warnings when editing with the CLI

CHANGELOG_PENDING.md

@@ -1,5 +1,8 @@
 ### Improvements
 
+- Surface warnings when editing environments with the CLI
+  [#631](https://github.com/pulumi/esc/pull/631)
+
 ### Bug Fixes
 
 ### Breaking changes

cmd/esc/cli/cli_test.go

@@ -25,6 +25,7 @@ import (
 	"time"
 
 	"github.com/gofrs/uuid"
+	"github.com/hashicorp/hcl/v2"
 	"github.com/pgavlin/fx/v2"
 	"github.com/pgavlin/fx/v2/maps"
 	"github.com/pulumi/esc"
@@ -388,10 +389,16 @@ func mapDiags(diags syntax.Diagnostics) []client.EnvironmentDiagnostic {
 			}
 		}
 
+		severity := client.DiagError
+		if d.Severity == hcl.DiagWarning {
+			severity = client.DiagWarning
+		}
+
 		out[i] = client.EnvironmentDiagnostic{
-			Range:   rng,
-			Summary: d.Summary,
-			Detail:  d.Detail,
+			Range:    rng,
+			Summary:  d.Summary,
+			Detail:   d.Detail,
+			Severity: severity,
 		}
 	}
 	return out
@@ -669,12 +676,13 @@ func (c *testPulumiClient) UpdateEnvironmentWithRevision(
 		return nil, 0, errors.New("etag mismatch")
 	}
 
-	_, diags, err := c.checkEnvironment(ctx, orgName, envName, yaml, nil)
-	if err == nil && len(diags) == 0 {
+	envId := projectName + "/" + envName
+	_, diags, err := c.checkEnvironment(ctx, orgName, envId, yaml, nil)
+	if err == nil && !client.DiagnosticsHaveErrors(diags) {
 		h := fnv.New32()
 		h.Write(yaml)
 
-		yaml, err = eval.EncryptSecrets(ctx, envName, yaml, rot128{})
+		yaml, err = eval.EncryptSecrets(ctx, envId, yaml, rot128{})
 		if err != nil {
 			return nil, 0, err
 		}
@@ -713,7 +721,8 @@ func (c *testPulumiClient) CreateEnvironmentDraft(
 		return "", nil, errors.New("etag mismatch")
 	}
 
-	_, diags, err := c.checkEnvironment(ctx, orgName, envName, yaml, nil)
+	envId := projectName + "/" + envName
+	_, diags, err := c.checkEnvironment(ctx, orgName, envId, yaml, nil)
 	if err != nil || len(diags) != 0 {
 		return "", diags, nil
 	}

cmd/esc/cli/client/apitype.go

@@ -22,10 +22,32 @@ import (
 	"github.com/pulumi/esc"
 )
 
+type EnvironmentDiagnosticSeverity string
+
+const (
+	DiagError   EnvironmentDiagnosticSeverity = "error"
+	DiagWarning EnvironmentDiagnosticSeverity = "warning"
+)
+
 type EnvironmentDiagnostic struct {
-	Range   *esc.Range `json:"range,omitempty"`
-	Summary string     `json:"summary,omitempty"`
-	Detail  string     `json:"detail,omitempty"`
+	Range    *esc.Range                    `json:"range,omitempty"`
+	Summary  string                        `json:"summary,omitempty"`
+	Detail   string                        `json:"detail,omitempty"`
+	Severity EnvironmentDiagnosticSeverity `json:"severity,omitempty"`
+}
+
+func (d EnvironmentDiagnostic) IsError() bool {
+	// Empty severity means error for backward compatibility.
+	return d.Severity == DiagError || d.Severity == ""
+}
+
+func DiagnosticsHaveErrors(diags []EnvironmentDiagnostic) bool {
+	for _, d := range diags {
+		if d.IsError() {
+			return true
+		}
+	}
+	return false
 }
 
 type EnvironmentErrorResponse struct {

cmd/esc/cli/client/client.go

@@ -713,13 +713,19 @@ func (pc *client) UpdateEnvironmentWithRevision(
 		}
 		return nil, 0, err
 	}
+	defer contract.IgnoreClose(resp.Body)
 
 	revision, err := strconv.Atoi(resp.Header.Get(revisionHeader))
 	if err != nil {
 		return nil, 0, fmt.Errorf("parsing revision number: %w", err)
 	}
 
-	return nil, revision, nil
+	var updateResp UpdateEnvironmentResponse
+	if err := json.NewDecoder(resp.Body).Decode(&updateResp); err != nil {
+		return nil, revision, nil
+	}
+
+	return updateResp.Diagnostics, revision, nil
 }
 
 func (pc *client) CreateEnvironmentDraft(

cmd/esc/cli/env.go

@@ -370,8 +370,12 @@ func (cmd *envCommand) writeYAMLEnvironmentDiagnostics(
 				},
 			}
 		}
+		severity := hcl.DiagError
+		if d.Severity == client.DiagWarning {
+			severity = hcl.DiagWarning
+		}
 		err := writer.WriteDiagnostic(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
+			Severity: severity,
 			Summary:  d.Summary,
 			Subject:  subject,
 		})
@@ -439,7 +443,7 @@ func (esc *escCommand) updateEnvironment(
 		if err != nil {
 			return nil, fmt.Errorf("creating environment draft: %w", err)
 		}
-		if len(diags) == 0 {
+		if !client.DiagnosticsHaveErrors(diags) {
 			fmt.Fprintf(esc.stdout, "Change request created: %v\n", changeRequestID)
 			fmt.Fprintf(esc.stdout, "Change request URL: %v\n", esc.changeRequestURL(ref, changeRequestID))
 
@@ -456,7 +460,7 @@ func (esc *escCommand) updateEnvironment(
 		if err != nil {
 			return nil, fmt.Errorf("updating environment draft: %w", err)
 		}
-		if len(diags) == 0 {
+		if !client.DiagnosticsHaveErrors(diags) {
 			fmt.Fprintln(esc.stdout, "Change request updated")
 			fmt.Fprintf(esc.stdout, "Change request URL: %v\n", esc.changeRequestURL(ref, changeRequestID))
 		}
@@ -467,7 +471,7 @@ func (esc *escCommand) updateEnvironment(
 		if err != nil {
 			return nil, fmt.Errorf("updating environment definition: %w", err)
 		}
-		if len(diags) == 0 && envUpdateSuccessMessage != "" {
+		if !client.DiagnosticsHaveErrors(diags) && envUpdateSuccessMessage != "" {
 			fmt.Fprintln(esc.stdout, envUpdateSuccessMessage)
 		}
 		return diags, nil

cmd/esc/cli/env_edit.go

@@ -81,7 +81,11 @@ func newEnvEditCmd(env *envCommand) *cobra.Command {
 				}
 
 				if len(diags) != 0 {
-					return edit.env.writeYAMLEnvironmentDiagnostics(edit.env.esc.stderr, ref.envName, yaml, diags)
+					err = edit.env.writeYAMLEnvironmentDiagnostics(edit.env.esc.stderr, ref.projectName+"/"+ref.envName, yaml, diags)
+					contract.IgnoreError(err)
+				}
+				if client.DiagnosticsHaveErrors(diags) {
+					return err
 				}
 
 				return nil
@@ -131,13 +135,14 @@ func newEnvEditCmd(env *envCommand) *cobra.Command {
 					return err
 				}
 
-				if len(diags) == 0 {
+				if len(diags) != 0 {
+					err = edit.env.writeYAMLEnvironmentDiagnostics(edit.env.esc.stderr, ref.projectName+"/"+ref.envName, newYAML, diags)
+					contract.IgnoreError(err)
+				}
+				if !client.DiagnosticsHaveErrors(diags) {
 					return nil
 				}
 
-				err = edit.env.writeYAMLEnvironmentDiagnostics(edit.env.esc.stderr, ref.envName, newYAML, diags)
-				contract.IgnoreError(err)
-
 				fmt.Fprintln(edit.env.esc.stderr, "Press ENTER to continue editing or ^D to exit")
 
 				var b [1]byte

cmd/esc/cli/env_init.go

@@ -66,7 +66,7 @@ func newEnvInitCmd(env *envCommand) *cobra.Command {
 					return fmt.Errorf("updating environment definition: %w", err)
 				}
 				if len(diags) != 0 {
-					err = env.writeYAMLEnvironmentDiagnostics(env.esc.stderr, ref.envName, yaml, diags)
+					err = env.writeYAMLEnvironmentDiagnostics(env.esc.stderr, ref.projectName+"/"+ref.envName, yaml, diags)
 					contract.IgnoreError(err)
 
 					return fmt.Errorf("updating environment definition: too many errors")

cmd/esc/cli/env_version_rollback.go

@@ -58,7 +58,7 @@ func newEnvVersionRollbackCmd(env *envCommand) *cobra.Command {
 			}
 
 			if len(diags) != 0 {
-				err = env.writeYAMLEnvironmentDiagnostics(env.esc.stderr, ref.envName, yaml, diags)
+				err = env.writeYAMLEnvironmentDiagnostics(env.esc.stderr, ref.projectName+"/"+ref.envName, yaml, diags)
 				contract.IgnoreError(err)
 				return errors.New("could not roll back: too many errors")
 			}

cmd/esc/cli/testdata/env-edit-editor-invalid-draft.yaml

@@ -17,7 +17,7 @@ environments:
 > esc env edit default/test --draft
 Error: imports must be a list
 
-  on test line 1:
+  on default/test line 1:
    1: imports:
 
 Press ENTER to continue editing or ^D to exit

cmd/esc/cli/testdata/env-edit-editor-invalid.yaml

@@ -17,7 +17,7 @@ environments:
 > esc env edit default/test
 Error: imports must be a list
 
-  on test line 1:
+  on default/test line 1:
    1: imports:
 
 Press ENTER to continue editing or ^D to exit

cmd/esc/cli/testdata/env-edit-file-warnings.yaml

@@ -0,0 +1,36 @@
+run: |
+  printf 'values:\n  x: 1\nvalues:\n  y: 2\n' | esc env edit default/test -f=-
+  esc env get default/test
+environments:
+  test-user/default/test:
+    values:
+      foo: bar
+
+---
+> esc env edit default/test -f=-
+Environment updated.
+> esc env get default/test
+# Value
+```json
+{
+  "y": 2
+}
+```
+# Definition
+```yaml
+values:
+  x: 1
+values:
+  y: 2
+
+```
+
+
+---
+> esc env edit default/test -f=-
+Warning: duplicate key "values"
+
+  on default/test line 3:
+   3: values:
+
+> esc env get default/test

cmd/esc/cli/testdata/env-init-diags.yaml

@@ -11,7 +11,7 @@ Environment created: test-user/default/test-stdin
 > esc env init default/test-stdin -f=-
 Error: unknown property "bar"
 
-  on test-stdin line 1:
+  on default/test-stdin line 1:
    1: {"values":{"foo":"${bar}"}}
 
 Error: updating environment definition: too many errors

cmd/esc/cli/testdata/env-set-secret.yaml

@@ -189,4 +189,4 @@ Error: value looks like a secret; rerun with --secret to mark it as such, or --p
 > esc env set default/test password true --secret
 > esc env get default/test
 > esc env set default/test password [] --secret
-test:3:21: secret values must be string literals
+default/test:3:21: secret values must be string literals

cmd/esc/cli/testdata/env-version-rollback-draft.yaml

@@ -51,7 +51,7 @@ Change request URL: https://app.fake.pulumi.com/test-user/esc/default/test?versi
 > esc env version rollback default/test@2 --draft
 Error: not found
 
-  on test line 2:
+  on default/test line 2:
    2:     - c
 
 Error: could not roll back: too many errors

cmd/esc/cli/testdata/env-version-rollback.yaml

@@ -60,7 +60,7 @@ values:
 > esc env version rollback default/test@2
 Error: not found
 
-  on test line 2:
+  on default/test line 2:
    2:     - c
 
 Error: could not roll back: too many errors