diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 5c2a5aa0..ce7802cd 100644 --- a/CHANGELOG_PENDING.md +++ b/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 diff --git a/cmd/esc/cli/cli_test.go b/cmd/esc/cli/cli_test.go index 274d437d..85dde8f7 100644 --- a/cmd/esc/cli/cli_test.go +++ b/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 } diff --git a/cmd/esc/cli/client/apitype.go b/cmd/esc/cli/client/apitype.go index eb113991..1ee9aea0 100644 --- a/cmd/esc/cli/client/apitype.go +++ b/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 { diff --git a/cmd/esc/cli/client/client.go b/cmd/esc/cli/client/client.go index f24a3ca8..86386adf 100644 --- a/cmd/esc/cli/client/client.go +++ b/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( diff --git a/cmd/esc/cli/env.go b/cmd/esc/cli/env.go index f17e30ac..83a260a0 100644 --- a/cmd/esc/cli/env.go +++ b/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 diff --git a/cmd/esc/cli/env_edit.go b/cmd/esc/cli/env_edit.go index ffbda534..c69e9e20 100644 --- a/cmd/esc/cli/env_edit.go +++ b/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 diff --git a/cmd/esc/cli/env_init.go b/cmd/esc/cli/env_init.go index 9c2ccf69..54dcaf4f 100644 --- a/cmd/esc/cli/env_init.go +++ b/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") diff --git a/cmd/esc/cli/env_version_rollback.go b/cmd/esc/cli/env_version_rollback.go index 1a6645f8..818ad41e 100644 --- a/cmd/esc/cli/env_version_rollback.go +++ b/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") } diff --git a/cmd/esc/cli/testdata/env-edit-editor-invalid-draft.yaml b/cmd/esc/cli/testdata/env-edit-editor-invalid-draft.yaml index e721a457..6ce6c102 100644 --- a/cmd/esc/cli/testdata/env-edit-editor-invalid-draft.yaml +++ b/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 diff --git a/cmd/esc/cli/testdata/env-edit-editor-invalid.yaml b/cmd/esc/cli/testdata/env-edit-editor-invalid.yaml index ebbf2a4f..127039ea 100644 --- a/cmd/esc/cli/testdata/env-edit-editor-invalid.yaml +++ b/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 diff --git a/cmd/esc/cli/testdata/env-edit-file-warnings.yaml b/cmd/esc/cli/testdata/env-edit-file-warnings.yaml new file mode 100644 index 00000000..a9589641 --- /dev/null +++ b/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 diff --git a/cmd/esc/cli/testdata/env-init-diags.yaml b/cmd/esc/cli/testdata/env-init-diags.yaml index c9127f6f..51847249 100644 --- a/cmd/esc/cli/testdata/env-init-diags.yaml +++ b/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 diff --git a/cmd/esc/cli/testdata/env-set-secret.yaml b/cmd/esc/cli/testdata/env-set-secret.yaml index 58301f0b..6bf729e4 100644 --- a/cmd/esc/cli/testdata/env-set-secret.yaml +++ b/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 diff --git a/cmd/esc/cli/testdata/env-version-rollback-draft.yaml b/cmd/esc/cli/testdata/env-version-rollback-draft.yaml index 59eaab9a..986486cf 100644 --- a/cmd/esc/cli/testdata/env-version-rollback-draft.yaml +++ b/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 diff --git a/cmd/esc/cli/testdata/env-version-rollback.yaml b/cmd/esc/cli/testdata/env-version-rollback.yaml index 83f68a56..70e36d37 100644 --- a/cmd/esc/cli/testdata/env-version-rollback.yaml +++ b/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