From 9fa858c225c655df6df5fc73c79b18193f2a5755 Mon Sep 17 00:00:00 2001 From: Caleb Piekstra Date: Wed, 21 Oct 2026 10:39:47 -0400 Subject: [PATCH 1/2] feat: note on stderr before writes to resources commonly managed as code Dashboards, alert policies and conditions, and synthetic monitors are often managed as code, so a direct change can be overwritten by the next deploy. dashboards and synthetics create/update/delete, and nerdgraph query mutations that write those resources, now print a one-time note to stderr before the write. Stdout and exit codes are unchanged. NEWRELIC_CLI_NO_AS_CODE_NOTICE=1 hides it. --- CHANGELOG.md | 6 + README.md | 52 ++- cmd/nrq/ascode_notice_test.go | 242 +++++++++++++ .../testdata/ascode/dashboard_created.json | 20 ++ cmd/nrq/testdata/ascode/dashboard_get.json | 13 + cmd/nrq/testdata/ascode/dashboard_input.json | 9 + .../ascode/nerdgraph_current_user.json | 10 + .../ascode/nerdgraph_dashboard_deleted.json | 8 + cmd/nrq/testdata/ascode/nerdgraph_error.json | 7 + .../ascode/synthetics_monitor_created.json | 15 + .../ascode/synthetics_monitor_input.json | 7 + docs/development.md | 15 + internal/ascode/ascode.go | 94 ++++++ internal/ascode/ascode_test.go | 219 ++++++++++++ internal/ascode/graphql.go | 318 ++++++++++++++++++ internal/cmd/dashboards/dashboards.go | 10 + internal/cmd/nerdgraph/nerdgraph.go | 10 +- internal/cmd/root/root.go | 3 + internal/cmd/synthetics/synthetics.go | 11 +- 19 files changed, 1066 insertions(+), 3 deletions(-) create mode 100644 cmd/nrq/ascode_notice_test.go create mode 100644 cmd/nrq/testdata/ascode/dashboard_created.json create mode 100644 cmd/nrq/testdata/ascode/dashboard_get.json create mode 100644 cmd/nrq/testdata/ascode/dashboard_input.json create mode 100644 cmd/nrq/testdata/ascode/nerdgraph_current_user.json create mode 100644 cmd/nrq/testdata/ascode/nerdgraph_dashboard_deleted.json create mode 100644 cmd/nrq/testdata/ascode/nerdgraph_error.json create mode 100644 cmd/nrq/testdata/ascode/synthetics_monitor_created.json create mode 100644 cmd/nrq/testdata/ascode/synthetics_monitor_input.json create mode 100644 internal/ascode/ascode.go create mode 100644 internal/ascode/ascode_test.go create mode 100644 internal/ascode/graphql.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 299454c..d343ad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Writes to resources that teams commonly manage as code print a one-time note + to stderr before the write: `dashboards create/update/delete`, + `synthetics create/update/delete`, and `nerdgraph query` mutations that + create, update or delete dashboards, alert policies, NRQL alert conditions + or synthetic monitors. Stdout and exit codes are unchanged. Set + `NEWRELIC_CLI_NO_AS_CODE_NOTICE=1` to hide it. - Linux distribution support via Snap, APT, and RPM packages ([#67](https://github.com/open-cli-collective/newrelic-cli/pull/67)) - `nrq init` command for guided API key setup ([#60](https://github.com/open-cli-collective/newrelic-cli/pull/60)) - `nrq config test` and `config clear` subcommands ([#60](https://github.com/open-cli-collective/newrelic-cli/pull/60)) diff --git a/README.md b/README.md index 700984c..c7ca2ca 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,7 @@ nrq apps list | `NEWRELIC_API_KEY` | New Relic User API key (`NRAK-`) | **Setup ingress only** — accepted by `nrq init --api-key-from-env` / `nrq set-credential --from-env`; **not** read at runtime | | `NEWRELIC_ACCOUNT_ID` | New Relic account ID (non-secret) | Runtime override; precedence **env > config.yml** | | `NEWRELIC_REGION` | API region: `US` (default) or `EU` (non-secret) | Runtime override; precedence **env > config.yml** | +| `NEWRELIC_CLI_NO_AS_CODE_NOTICE` | Set to `1` or `true` to hide the [managed-as-code note](#resources-managed-as-code) | Runtime; stderr only | ### CLI Configuration Commands @@ -226,6 +227,37 @@ fail loudly rather than silently picking a winner. - **API key:** OS keyring only at runtime (env is setup-ingress only). - **account_id / region:** environment variable > `config.yml` > built-in default. +### Resources managed as code + +Dashboards, alert policies, NRQL alert conditions and synthetic monitors are +often managed as code (Terraform, a repository or similar). A direct change +can be overwritten by the next deploy from that source, so before nrq writes +one of these it prints a note to stderr: + +``` +Note: dashboards are often managed as code (Terraform, a repository or similar), and the next deploy from that source can overwrite a direct change. +Check for a source of truth before making this change. Set NEWRELIC_CLI_NO_AS_CODE_NOTICE=1 to hide this note. +``` + +The note is printed once per invocation, before the write is sent, by: + +- `dashboards create`, `update` and `delete` +- `synthetics create`, `update` and `delete` +- `nerdgraph query` when the document is a mutation whose top-level fields + create, update or delete dashboards (`dashboardCreate`, `dashboardUpdate`, + `dashboardUpdatePage`, `dashboardUpdateWidgetsInPage`, + `dashboardAddWidgetsToPage`, `dashboardDelete`, `dashboardUndelete`), alert + policies (`alertsPolicyCreate`, `alertsPolicyUpdate`, `alertsPolicyDelete`), + alert conditions (`alertsNrqlCondition*`, `alertsConditionDelete`) or + synthetic monitors (`syntheticsCreate*Monitor`, `syntheticsUpdate*Monitor`, + `syntheticsDeleteMonitor`, `syntheticsUpdateScript`). One note names every + kind the mutation touches. Read-only queries and other mutations print + nothing. + +The note never prompts or blocks, and never changes stdout or the exit code, +so JSON output stays machine-parseable. Set `NEWRELIC_CLI_NO_AS_CODE_NOTICE=1` +to hide it. + ### Shell Completion Generate shell completions for tab completion support: @@ -400,6 +432,18 @@ nrq dashboards get nrq dashboards get "ABC123..." ``` +#### dashboards create / update / delete + +Create and update dashboards from a JSON definition, or delete one by GUID. +See `nrq dashboards create --help` for the JSON schema. Each prints the +[managed-as-code note](#resources-managed-as-code) to stderr before the write. + +```bash +nrq dashboards create --from-file dashboard.json +nrq dashboards update --from-file dashboard.json +nrq dashboards delete +``` + --- ### deployments @@ -719,6 +763,11 @@ nrq nerdgraph query '{ }' ``` +A mutation that writes dashboards, alert policies, alert conditions or +synthetic monitors prints the +[managed-as-code note](#resources-managed-as-code) to stderr before it is +sent; the JSON on stdout is unchanged. + --- ### nrql @@ -795,7 +844,8 @@ Create, update, and delete monitors from a JSON definition. Monitors are created via NerdGraph on the current synthetics runtimes; scripted monitors (`SCRIPT_API`, `SCRIPT_BROWSER`) take `script` and optional `runtime` fields. See `nrq synthetics create --help` for the JSON schema, valid frequencies, -and locations. +and locations. Each prints the +[managed-as-code note](#resources-managed-as-code) to stderr before the write. ```bash nrq synthetics create --from-file monitor.json diff --git a/cmd/nrq/ascode_notice_test.go b/cmd/nrq/ascode_notice_test.go new file mode 100644 index 0000000..f74f09a --- /dev/null +++ b/cmd/nrq/ascode_notice_test.go @@ -0,0 +1,242 @@ +package main + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/open-cli-collective/newrelic-cli/internal/ascode" + "github.com/open-cli-collective/newrelic-cli/internal/keychain" + "github.com/open-cli-collective/newrelic-cli/internal/testutil" +) + +const ( + dashboardGUID = "MTIzNDU2N3xWSVp8REFTSEJPQVJEfDEyMzQ1Njc4OQ==" + deleteMutation = `mutation { dashboardDelete(guid: "` + dashboardGUID + `") { status errors { type } } }` + currentUserRead = `{ actor { user { email name } } }` +) + +// nerdGraphRoute answers a NerdGraph request whose query contains match +// with the given fixture from testdata/ascode. +type nerdGraphRoute struct { + match string + fixture string +} + +// sentRequest records one NerdGraph request and what stderr held when it +// was sent, so tests can prove the note precedes the write. +type sentRequest struct { + query string + stderrAtSend string +} + +// noticeHarness runs the real nrq command tree against a stubbed NerdGraph +// endpoint (http.DefaultTransport is swapped for the test's duration) with an +// API key planted in the hermetic file-backend keyring. +type noticeHarness struct { + t *testing.T + routes []nerdGraphRoute + stderr *bytes.Buffer + sent []sentRequest +} + +func newNoticeHarness(t *testing.T, routes ...nerdGraphRoute) *noticeHarness { + t.Helper() + testutil.Setup(t) + t.Setenv("NEWRELIC_ACCOUNT_ID", "1234567") + t.Setenv(ascode.DisableEnv, "") + + st, err := keychain.OpenNoMigrate() + require.NoError(t, err) + require.NoError(t, st.SetAPIKey("NRAK-TEST-PLACEHOLDER")) + require.NoError(t, st.Close()) + + h := ¬iceHarness{t: t, routes: routes} + orig := http.DefaultTransport + http.DefaultTransport = h + t.Cleanup(func() { http.DefaultTransport = orig }) + return h +} + +func (h *noticeHarness) RoundTrip(req *http.Request) (*http.Response, error) { + var body struct { + Query string `json:"query"` + } + raw, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + if err := json.Unmarshal(raw, &body); err != nil { + return nil, err + } + h.sent = append(h.sent, sentRequest{query: body.Query, stderrAtSend: h.stderr.String()}) + + for _, r := range h.routes { + if strings.Contains(body.Query, r.match) { + payload, err := os.ReadFile(filepath.Join("testdata", "ascode", r.fixture)) + if err != nil { + return nil, err + } + return &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{"Content-Type": []string{"application/json"}}, + Body: io.NopCloser(bytes.NewReader(payload)), + Request: req, + }, nil + } + } + h.t.Fatalf("unexpected NerdGraph request: %s", body.Query) + return nil, nil +} + +// run executes nrq with args and returns stdout, stderr and the error. +func (h *noticeHarness) run(args ...string) (string, string, error) { + h.t.Helper() + h.sent = nil + rootCmd, opts := buildRootCommand() + var stdout, stderr bytes.Buffer + h.stderr = &stderr + opts.Stdout = &stdout + opts.Stderr = &stderr + opts.Stdin = strings.NewReader("") + rootCmd.SetErr(&stderr) + rootCmd.SetArgs(append([]string{"--no-color"}, args...)) + err := rootCmd.Execute() + return stdout.String(), stderr.String(), err +} + +// sentWith returns the requests whose query contains match. +func (h *noticeHarness) sentWith(match string) []sentRequest { + var out []sentRequest + for _, s := range h.sent { + if strings.Contains(s.query, match) { + out = append(out, s) + } + } + return out +} + +func fixtureData(t *testing.T, name string) string { + t.Helper() + raw, err := os.ReadFile(filepath.Join("testdata", "ascode", name)) + require.NoError(t, err) + var resp struct { + Data json.RawMessage `json:"data"` + } + require.NoError(t, json.Unmarshal(raw, &resp)) + var data interface{} + require.NoError(t, json.Unmarshal(resp.Data, &data)) + pretty, err := json.MarshalIndent(data, "", " ") + require.NoError(t, err) + return string(pretty) + "\n" +} + +func TestNerdGraphMutation_NoteOnStderrOnly(t *testing.T) { + want := fixtureData(t, "nerdgraph_dashboard_deleted.json") + for _, alias := range []string{"nerdgraph", "ng", "graphql"} { + t.Run(alias, func(t *testing.T) { + h := newNoticeHarness(t, nerdGraphRoute{"dashboardDelete", "nerdgraph_dashboard_deleted.json"}) + stdout, stderr, err := h.run(alias, "query", deleteMutation) + require.NoError(t, err) + + assert.Equal(t, ascode.Message(ascode.Dashboard), stderr, "exactly one note, nothing else") + assert.Equal(t, want, stdout, "stdout is the unchanged JSON passthrough") + assert.True(t, json.Valid([]byte(stdout))) + + sent := h.sentWith("dashboardDelete") + require.Len(t, sent, 1) + assert.Equal(t, ascode.Message(ascode.Dashboard), sent[0].stderrAtSend, "note precedes the write") + }) + } +} + +func TestNerdGraphMutation_MultipleKindsOneNote(t *testing.T) { + h := newNoticeHarness(t, nerdGraphRoute{"dashboardDelete", "nerdgraph_dashboard_deleted.json"}) + doc := `mutation { + dashboardDelete(guid: "` + dashboardGUID + `") { status } + alertsPolicyDelete(accountId: 1234567, id: 123456789) { id } + }` + _, stderr, err := h.run("ng", "query", doc) + require.NoError(t, err) + assert.Equal(t, ascode.Message(ascode.Dashboard, ascode.AlertPolicy), stderr) + assert.Equal(t, 1, strings.Count(stderr, "Note:")) +} + +func TestNerdGraphReadOnlyQuery_NoNote(t *testing.T) { + h := newNoticeHarness(t, nerdGraphRoute{"actor", "nerdgraph_current_user.json"}) + stdout, stderr, err := h.run("nerdgraph", "query", currentUserRead) + require.NoError(t, err) + assert.Empty(t, stderr) + assert.Equal(t, fixtureData(t, "nerdgraph_current_user.json"), stdout) +} + +func TestNerdGraphMutation_Silenced(t *testing.T) { + h := newNoticeHarness(t, nerdGraphRoute{"dashboardDelete", "nerdgraph_dashboard_deleted.json"}) + t.Setenv(ascode.DisableEnv, "1") + stdout, stderr, err := h.run("ng", "query", deleteMutation) + require.NoError(t, err) + assert.Empty(t, stderr) + assert.Equal(t, fixtureData(t, "nerdgraph_dashboard_deleted.json"), stdout) +} + +func TestNerdGraphMutation_NoteShownWhenWriteFails(t *testing.T) { + h := newNoticeHarness(t, nerdGraphRoute{"dashboardDelete", "nerdgraph_error.json"}) + stdout, stderr, err := h.run("ng", "query", deleteMutation) + require.Error(t, err) + assert.Empty(t, stdout) + assert.True(t, strings.HasPrefix(stderr, ascode.Message(ascode.Dashboard)), stderr) +} + +func TestDashboardsCreate_Note(t *testing.T) { + input := filepath.Join("testdata", "ascode", "dashboard_input.json") + route := nerdGraphRoute{"dashboardCreate", "dashboard_created.json"} + + h := newNoticeHarness(t, route) + stdout, stderr, err := h.run("dash", "create", "-f", input, "-o", "plain") + require.NoError(t, err) + assert.Equal(t, ascode.Message(ascode.Dashboard), stderr) + sent := h.sentWith("dashboardCreate") + require.Len(t, sent, 1) + assert.Equal(t, ascode.Message(ascode.Dashboard), sent[0].stderrAtSend, "note precedes the write") + + t.Setenv(ascode.DisableEnv, "true") + silencedStdout, silencedStderr, err := h.run("dash", "create", "-f", input, "-o", "plain") + require.NoError(t, err) + assert.Empty(t, silencedStderr) + assert.Equal(t, silencedStdout, stdout, "the note never changes stdout") +} + +func TestDashboardsDelete_NoteOnce(t *testing.T) { + h := newNoticeHarness(t, + nerdGraphRoute{"dashboardDelete", "nerdgraph_dashboard_deleted.json"}, + nerdGraphRoute{"entity(guid", "dashboard_get.json"}, + ) + stdout, stderr, err := h.run("dashboards", "delete", dashboardGUID, "--force") + require.NoError(t, err) + assert.Empty(t, stdout) + assert.Equal(t, 1, strings.Count(stderr, "Note:")) + assert.True(t, strings.HasPrefix(stderr, ascode.Message(ascode.Dashboard)), stderr) + sent := h.sentWith("dashboardDelete") + require.Len(t, sent, 1) + assert.Equal(t, ascode.Message(ascode.Dashboard), sent[0].stderrAtSend, "note precedes the write") +} + +func TestSyntheticsCreate_Note(t *testing.T) { + h := newNoticeHarness(t, nerdGraphRoute{"syntheticsCreateSimpleMonitor", "synthetics_monitor_created.json"}) + input := filepath.Join("testdata", "ascode", "synthetics_monitor_input.json") + stdout, stderr, err := h.run("syn", "create", "-f", input, "-o", "plain") + require.NoError(t, err) + assert.Equal(t, ascode.Message(ascode.SyntheticMonitor), stderr) + assert.Contains(t, stdout, "checkout-service health") + sent := h.sentWith("syntheticsCreateSimpleMonitor") + require.Len(t, sent, 1) + assert.Equal(t, ascode.Message(ascode.SyntheticMonitor), sent[0].stderrAtSend, "note precedes the write") +} diff --git a/cmd/nrq/testdata/ascode/dashboard_created.json b/cmd/nrq/testdata/ascode/dashboard_created.json new file mode 100644 index 0000000..38ca441 --- /dev/null +++ b/cmd/nrq/testdata/ascode/dashboard_created.json @@ -0,0 +1,20 @@ +{ + "data": { + "dashboardCreate": { + "entityResult": { + "guid": "MTIzNDU2N3xWSVp8REFTSEJPQVJEfDEyMzQ1Njc4OQ==", + "name": "Checkout Overview", + "description": "", + "permissions": "PUBLIC_READ_WRITE", + "pages": [ + { + "guid": "MTIzNDU2N3xWSVp8REFTSEJPQVJEfDEyMzQ1Njc5MA==", + "name": "Overview", + "widgets": [] + } + ] + }, + "errors": [] + } + } +} diff --git a/cmd/nrq/testdata/ascode/dashboard_get.json b/cmd/nrq/testdata/ascode/dashboard_get.json new file mode 100644 index 0000000..8bbe577 --- /dev/null +++ b/cmd/nrq/testdata/ascode/dashboard_get.json @@ -0,0 +1,13 @@ +{ + "data": { + "actor": { + "entity": { + "guid": "MTIzNDU2N3xWSVp8REFTSEJPQVJEfDEyMzQ1Njc4OQ==", + "name": "Checkout Overview", + "description": "", + "permissions": "PUBLIC_READ_WRITE", + "pages": [] + } + } + } +} diff --git a/cmd/nrq/testdata/ascode/dashboard_input.json b/cmd/nrq/testdata/ascode/dashboard_input.json new file mode 100644 index 0000000..94a623a --- /dev/null +++ b/cmd/nrq/testdata/ascode/dashboard_input.json @@ -0,0 +1,9 @@ +{ + "name": "Checkout Overview", + "pages": [ + { + "name": "Overview", + "widgets": [] + } + ] +} diff --git a/cmd/nrq/testdata/ascode/nerdgraph_current_user.json b/cmd/nrq/testdata/ascode/nerdgraph_current_user.json new file mode 100644 index 0000000..b1da9de --- /dev/null +++ b/cmd/nrq/testdata/ascode/nerdgraph_current_user.json @@ -0,0 +1,10 @@ +{ + "data": { + "actor": { + "user": { + "email": "user@example.com", + "name": "Example User" + } + } + } +} diff --git a/cmd/nrq/testdata/ascode/nerdgraph_dashboard_deleted.json b/cmd/nrq/testdata/ascode/nerdgraph_dashboard_deleted.json new file mode 100644 index 0000000..b62251b --- /dev/null +++ b/cmd/nrq/testdata/ascode/nerdgraph_dashboard_deleted.json @@ -0,0 +1,8 @@ +{ + "data": { + "dashboardDelete": { + "errors": [], + "status": "SUCCESS" + } + } +} diff --git a/cmd/nrq/testdata/ascode/nerdgraph_error.json b/cmd/nrq/testdata/ascode/nerdgraph_error.json new file mode 100644 index 0000000..60471ab --- /dev/null +++ b/cmd/nrq/testdata/ascode/nerdgraph_error.json @@ -0,0 +1,7 @@ +{ + "errors": [ + { + "message": "Not authorized to delete this dashboard" + } + ] +} diff --git a/cmd/nrq/testdata/ascode/synthetics_monitor_created.json b/cmd/nrq/testdata/ascode/synthetics_monitor_created.json new file mode 100644 index 0000000..891953c --- /dev/null +++ b/cmd/nrq/testdata/ascode/synthetics_monitor_created.json @@ -0,0 +1,15 @@ +{ + "data": { + "result": { + "errors": [], + "monitor": { + "guid": "MTIzNDU2N3xTWU5USHxNT05JVE9SfDEyMzQ1Njc4OQ==", + "id": "00000000-0000-4000-8000-000000000001", + "name": "checkout-service health", + "period": "EVERY_10_MINUTES", + "status": "ENABLED", + "uri": "https://example.com/health" + } + } + } +} diff --git a/cmd/nrq/testdata/ascode/synthetics_monitor_input.json b/cmd/nrq/testdata/ascode/synthetics_monitor_input.json new file mode 100644 index 0000000..97fa12e --- /dev/null +++ b/cmd/nrq/testdata/ascode/synthetics_monitor_input.json @@ -0,0 +1,7 @@ +{ + "name": "checkout-service health", + "type": "SIMPLE", + "frequency": 10, + "uri": "https://example.com/health", + "locations": ["AWS_US_EAST_1"] +} diff --git a/docs/development.md b/docs/development.md index 3e964c8..bf9c722 100644 --- a/docs/development.md +++ b/docs/development.md @@ -32,6 +32,7 @@ newrelic-cli/ ├── cmd/nrq/main.go ├── api/ # public New Relic API client package ├── internal/ +│ ├── ascode/ # stderr note before writes to resources managed as code │ ├── cmd/ # Cobra commands by resource │ ├── config/ # non-secret config │ ├── keychain/ # cli-common credstore adapter and legacy migration @@ -100,6 +101,20 @@ Local convenience copy, if present: `../cli-common/docs/scriptability.md` - `NEWRELIC_REGION` - non-secret runtime override. - `NEWRELIC_CLI_KEYRING_BACKEND` - backend selector. - `NEWRELIC_CLI_KEYRING_PASSPHRASE` - file-backend passphrase for headless use. +- `NEWRELIC_CLI_NO_AS_CODE_NOTICE` - `1`/`true` hides the stderr note printed + before writes to resources commonly managed as code (`internal/ascode`). + +## Managed-As-Code Note + +Commands that write dashboards, alert policies or conditions, or synthetic +monitors call `ascode.Notify` once, immediately before the write (before a +delete's confirmation prompt), so the note is visible even when the write +fails. `nerdgraph query` derives the kinds from the document with +`ascode.MutationKinds`, which matches top-level mutation fields only, including +those reached through fragments. A new write command for one of these +resources calls `ascode.Notify`; a new resource kind goes in `ascode.Kind` and +`classifyMutationField`. The note goes to stderr only and never changes stdout +or the exit code. ## Shared Repo Standards diff --git a/internal/ascode/ascode.go b/internal/ascode/ascode.go new file mode 100644 index 0000000..8128d59 --- /dev/null +++ b/internal/ascode/ascode.go @@ -0,0 +1,94 @@ +// Package ascode prints a one-time stderr note when nrq is about to write a +// New Relic resource that teams commonly manage as code (Terraform, a +// dashboards repository or similar). A direct write to such a resource is +// silently reverted by the next deploy from its source, so the note asks the +// caller to check for a source of truth first. +// +// The note never blocks, prompts, or touches stdout or the exit code: per +// cli-common docs/output-and-rendering.md §5 it is side-channel chatter, and +// JSON passthrough output (`nrq nerdgraph query`) must stay machine-parseable. +package ascode + +import ( + "fmt" + "io" + "os" + "strconv" + "strings" +) + +// DisableEnv silences the note when set to a true value ("1", "true"). +const DisableEnv = "NEWRELIC_CLI_NO_AS_CODE_NOTICE" + +// Kind is a resource kind that teams commonly manage as code. +type Kind int + +// Kinds in the order the note names them. +const ( + Dashboard Kind = iota + 1 + AlertPolicy + AlertCondition + SyntheticMonitor +) + +func (k Kind) plural() string { + switch k { + case Dashboard: + return "dashboards" + case AlertPolicy: + return "alert policies" + case AlertCondition: + return "alert conditions" + case SyntheticMonitor: + return "synthetic monitors" + default: + return "resources" + } +} + +// Disabled reports whether DisableEnv silences the note. +func Disabled() bool { + on, err := strconv.ParseBool(strings.TrimSpace(os.Getenv(DisableEnv))) + return err == nil && on +} + +// Notify writes the note for kinds to w, unless kinds is empty or the note is +// disabled. Callers invoke it once per command, before the write is sent, so +// the note is visible even when the write fails. +func Notify(w io.Writer, kinds ...Kind) { + if len(kinds) == 0 || Disabled() { + return + } + fmt.Fprint(w, Message(kinds...)) +} + +// Message returns the note text for kinds, deduplicated and in Kind order. +func Message(kinds ...Kind) string { + seen := make(map[Kind]bool, len(kinds)) + for _, k := range kinds { + seen[k] = true + } + var names []string + for _, k := range []Kind{Dashboard, AlertPolicy, AlertCondition, SyntheticMonitor} { + if seen[k] { + names = append(names, k.plural()) + } + } + return fmt.Sprintf("Note: %s are often managed as code (Terraform, a repository or similar), "+ + "and the next deploy from that source can overwrite a direct change.\n"+ + "Check for a source of truth before making this change. Set %s=1 to hide this note.\n", + joinNames(names), DisableEnv) +} + +func joinNames(names []string) string { + switch len(names) { + case 0: + return Kind(0).plural() + case 1: + return names[0] + case 2: + return names[0] + " and " + names[1] + default: + return strings.Join(names[:len(names)-1], ", ") + " and " + names[len(names)-1] + } +} diff --git a/internal/ascode/ascode_test.go b/internal/ascode/ascode_test.go new file mode 100644 index 0000000..7c58a9c --- /dev/null +++ b/internal/ascode/ascode_test.go @@ -0,0 +1,219 @@ +package ascode + +import ( + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMutationKinds(t *testing.T) { + tests := []struct { + name string + doc string + want []Kind + }{ + { + name: "dashboard create", + doc: `mutation { dashboardCreate(accountId: 1, dashboard: {name: "x"}) { entityResult { guid } } }`, + want: []Kind{Dashboard}, + }, + { + name: "named mutation with variables and directives", + doc: `mutation UpdateIt($guid: EntityGuid!, $d: DashboardInput!) @trace { + dashboardUpdate(guid: $guid, dashboard: $d) { errors { description } } + }`, + want: []Kind{Dashboard}, + }, + { + name: "every dashboard write", + doc: `mutation { + a: dashboardUpdateWidgetsInPage(guid: "g", widgets: []) { errors { type } } + b: dashboardAddWidgetsToPage(guid: "g", widgets: []) { errors { type } } + c: dashboardDelete(guid: "g") { status } + d: dashboardUndelete(guid: "g") { errors { type } } + e: dashboardUpdatePage(guid: "g", page: {}) { errors { type } } + }`, + want: []Kind{Dashboard}, + }, + { + name: "alias resolves to the real field", + doc: `mutation { snapshot: dashboardCreateSnapshotUrl(guid: "g") ok: dashboardDelete(guid: "g") { status } }`, + want: []Kind{Dashboard}, + }, + { + name: "alias named like a write does not match", + doc: `mutation { dashboardCreate: dashboardCreateSnapshotUrl(guid: "g") }`, + want: nil, + }, + { + name: "multiple kinds in one document", + doc: `mutation { + syntheticsDeleteMonitor(guid: "g") { deletedGuid } + alertsNrqlConditionStaticUpdate(accountId: 1, id: 2, condition: {}) { id } + alertsPolicyCreate(accountId: 1, policy: {name: "p", incidentPreference: PER_POLICY}) { id } + dashboardCreate(accountId: 1, dashboard: {}) { entityResult { guid } } + }`, + want: []Kind{Dashboard, AlertPolicy, AlertCondition, SyntheticMonitor}, + }, + { + name: "alert policy update and delete", + doc: `mutation { alertsPolicyUpdate(accountId: 1, id: 2, policy: {}) { id } alertsPolicyDelete(accountId: 1, id: 2) { id } }`, + want: []Kind{AlertPolicy}, + }, + { + name: "alert condition delete and baseline create", + doc: `mutation { alertsConditionDelete(accountId: 1, id: 2) { id } alertsNrqlConditionBaselineCreate(accountId: 1, policyId: 3, condition: {}) { id } }`, + want: []Kind{AlertCondition}, + }, + { + name: "synthetics monitor writes", + doc: `mutation { + syntheticsCreateSimpleMonitor(accountId: 1, monitor: {}) { errors { type } } + syntheticsUpdateScriptApiMonitor(guid: "g", monitor: {}) { errors { type } } + syntheticsUpdateScript(guid: "g", script: {}) { errors { type } } + }`, + want: []Kind{SyntheticMonitor}, + }, + { + name: "fragment spread on Mutation", + doc: `mutation { ...Writes } + fragment Writes on Mutation { dashboardDelete(guid: "g") { status } }`, + want: []Kind{Dashboard}, + }, + { + name: "fragment defined before use and spread twice", + doc: `fragment W on Mutation { alertsPolicyDelete(accountId: 1, id: 2) { id } ...W } + mutation { ...W ...W }`, + want: []Kind{AlertPolicy}, + }, + { + name: "inline fragment with and without type condition", + doc: `mutation { ... on Mutation { dashboardCreate(accountId: 1, dashboard: {}) { entityResult { guid } } } ... @include(if: true) { syntheticsDeleteMonitor(guid: "g") { deletedGuid } } }`, + want: []Kind{Dashboard, SyntheticMonitor}, + }, + { + name: "comments, commas and block strings around a write", + doc: `# a comment mentioning query { } + mutation { # trailing comment } + dashboardDelete(guid: "g",) { status, errors { type } } + description: x(text: """ a "quoted" } block """) + }`, + want: []Kind{Dashboard}, + }, + { + name: "read-only query", + doc: `{ actor { entitySearch(query: "type = 'DASHBOARD'") { results { entities { guid } } } } }`, + want: nil, + }, + { + name: "named query whose fields share write names", + doc: `query dashboardCreate { actor { dashboardCreate: user { name } } }`, + want: nil, + }, + { + name: "write name only in a comment", + doc: `# mutation { dashboardDelete(guid: "g") } + { actor { user { email } } }`, + want: nil, + }, + { + name: "write name only in a string literal", + doc: `{ actor { account(id: 1) { nrql(query: "mutation { dashboardDelete(guid: \"g\") }") { results } } } }`, + want: nil, + }, + { + name: "write name only in a block string", + doc: `{ actor { account(id: 1) { nrql(query: """ mutation { alertsPolicyDelete } """) { results } } } }`, + want: nil, + }, + { + name: "write name nested below the top level of a mutation", + doc: `mutation { taggingAddTagsToEntity(guid: "g", tags: []) { errors { dashboardCreate } } }`, + want: nil, + }, + { + name: "other dashboard and alert mutations", + doc: `mutation { + dashboardCreateSnapshotUrl(guid: "g") + alertsPolicyAddNotificationChannels(accountId: 1, id: 2) { errors { type } } + alertsMutingRuleCreate(accountId: 1, rule: {}) { id } + syntheticsCreateSecureCredential(accountId: 1, key: "K", value: "v") { errors { type } } + syntheticsStartAutomatedTest { batchId } + }`, + want: nil, + }, + { + name: "fragment with write fields that no mutation spreads", + doc: `query { actor { user { name } } } + fragment W on Mutation { dashboardDelete(guid: "g") { status } }`, + want: nil, + }, + { + name: "subscription is not a mutation", + doc: `subscription { dashboardCreate }`, + want: nil, + }, + { + name: "empty document", + doc: ``, + want: nil, + }, + { + name: "unterminated document still finds the write", + doc: `mutation { dashboardDelete(guid: "g") { status `, + want: []Kind{Dashboard}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, MutationKinds(tt.doc)) + }) + } +} + +func TestMessage(t *testing.T) { + one := Message(Dashboard) + assert.True(t, strings.HasPrefix(one, "Note: dashboards are often managed as code"), one) + assert.Contains(t, one, DisableEnv+"=1") + assert.Equal(t, 2, strings.Count(one, "\n"), "the note is two lines") + + assert.Contains(t, Message(SyntheticMonitor, AlertPolicy), "Note: alert policies and synthetic monitors are") + assert.Contains(t, Message(SyntheticMonitor, Dashboard, AlertCondition, AlertPolicy, Dashboard), + "Note: dashboards, alert policies, alert conditions and synthetic monitors are") + + assert.NotContains(t, Message(Dashboard, AlertPolicy, AlertCondition, SyntheticMonitor), "—", + "the note must not contain an em dash") +} + +func TestNotify(t *testing.T) { + t.Run("writes the note", func(t *testing.T) { + t.Setenv(DisableEnv, "") + var buf bytes.Buffer + Notify(&buf, Dashboard) + assert.Equal(t, Message(Dashboard), buf.String()) + }) + t.Run("no kinds writes nothing", func(t *testing.T) { + t.Setenv(DisableEnv, "") + var buf bytes.Buffer + Notify(&buf) + assert.Empty(t, buf.String()) + }) + for _, v := range []string{"1", "true", "TRUE", " 1 "} { + t.Run("silenced by "+v, func(t *testing.T) { + t.Setenv(DisableEnv, v) + var buf bytes.Buffer + Notify(&buf, Dashboard) + assert.Empty(t, buf.String()) + }) + } + for _, v := range []string{"0", "false", "no"} { + t.Run("not silenced by "+v, func(t *testing.T) { + t.Setenv(DisableEnv, v) + var buf bytes.Buffer + Notify(&buf, Dashboard) + assert.NotEmpty(t, buf.String()) + }) + } +} diff --git a/internal/ascode/graphql.go b/internal/ascode/graphql.go new file mode 100644 index 0000000..24dbcfe --- /dev/null +++ b/internal/ascode/graphql.go @@ -0,0 +1,318 @@ +package ascode + +import "strings" + +// MutationKinds returns the managed-as-code kinds that a GraphQL document's +// mutation operations create, update or delete, in Kind order. Read-only +// operations, other mutations, and text inside comments or string literals +// never match. Top-level fields reached through inline fragments and named +// fragment spreads count. The scan is lenient: a malformed document yields +// whatever it could recognize, and NerdGraph reports the syntax error. +func MutationKinds(document string) []Kind { + p := &gqlParser{toks: lexGraphQL(document)} + fragments, mutations := p.parseDocument() + + seen := map[Kind]bool{} + visited := map[string]bool{} + var walk func([]gqlSelection) + walk = func(sels []gqlSelection) { + for _, s := range sels { + switch { + case s.spread != "": + if !visited[s.spread] { + visited[s.spread] = true + walk(fragments[s.spread]) + } + case s.inline != nil: + walk(s.inline) + default: + if k, ok := classifyMutationField(s.field); ok { + seen[k] = true + } + } + } + } + for _, m := range mutations { + walk(m) + } + + var kinds []Kind + for _, k := range []Kind{Dashboard, AlertPolicy, AlertCondition, SyntheticMonitor} { + if seen[k] { + kinds = append(kinds, k) + } + } + return kinds +} + +var dashboardMutations = map[string]bool{ + "dashboardCreate": true, + "dashboardUpdate": true, + "dashboardUpdatePage": true, + "dashboardUpdateWidgetsInPage": true, + "dashboardAddWidgetsToPage": true, + "dashboardDelete": true, + "dashboardUndelete": true, +} + +var alertPolicyMutations = map[string]bool{ + "alertsPolicyCreate": true, + "alertsPolicyUpdate": true, + "alertsPolicyDelete": true, +} + +// classifyMutationField maps a NerdGraph mutation field name to the kind it +// writes. Dashboard and policy fields are an explicit list because sibling +// mutations (dashboardCreateSnapshotUrl, alertsPolicyAddNotificationChannels) +// do not change the resource definition. +func classifyMutationField(name string) (Kind, bool) { + switch { + case dashboardMutations[name]: + return Dashboard, true + case alertPolicyMutations[name]: + return AlertPolicy, true + case strings.HasPrefix(name, "alertsNrqlCondition"), name == "alertsConditionDelete": + return AlertCondition, true + case isSyntheticsMonitorWrite(name): + return SyntheticMonitor, true + } + return 0, false +} + +func isSyntheticsMonitorWrite(name string) bool { + if name == "syntheticsUpdateScript" { + return true + } + for _, verb := range []string{"syntheticsCreate", "syntheticsUpdate", "syntheticsDelete"} { + if strings.HasPrefix(name, verb) && strings.HasSuffix(name, "Monitor") { + return true + } + } + return false +} + +type gqlTokenKind int + +const ( + tokName gqlTokenKind = iota + 1 + tokPunct + tokSpread + tokValue // string or number literal; its content is irrelevant here +) + +type gqlToken struct { + kind gqlTokenKind + text string +} + +func isNameStart(c byte) bool { + return c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') +} + +func isNameChar(c byte) bool { + return isNameStart(c) || (c >= '0' && c <= '9') +} + +// lexGraphQL tokenizes a GraphQL document, dropping whitespace, commas, +// comments and the contents of string literals. +func lexGraphQL(src string) []gqlToken { + var toks []gqlToken + for i := 0; i < len(src); { + c := src[i] + switch { + case c == '#': + for i < len(src) && src[i] != '\n' && src[i] != '\r' { + i++ + } + case c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == ',': + i++ + case strings.HasPrefix(src[i:], `"""`): + i += 3 + for i < len(src) && !strings.HasPrefix(src[i:], `"""`) { + if strings.HasPrefix(src[i:], `\"""`) { + i += 4 + continue + } + i++ + } + i += 3 + toks = append(toks, gqlToken{kind: tokValue}) + case c == '"': + i++ + for i < len(src) && src[i] != '"' && src[i] != '\n' { + if src[i] == '\\' { + i++ + } + i++ + } + i++ + toks = append(toks, gqlToken{kind: tokValue}) + case strings.HasPrefix(src[i:], "..."): + i += 3 + toks = append(toks, gqlToken{kind: tokSpread, text: "..."}) + case isNameStart(c): + start := i + for i < len(src) && isNameChar(src[i]) { + i++ + } + toks = append(toks, gqlToken{kind: tokName, text: src[start:i]}) + case c == '-' || (c >= '0' && c <= '9'): + i++ + for i < len(src) && (isNameChar(src[i]) || src[i] == '.' || src[i] == '+' || src[i] == '-') { + i++ + } + toks = append(toks, gqlToken{kind: tokValue}) + default: + toks = append(toks, gqlToken{kind: tokPunct, text: string(c)}) + i++ + } + } + return toks +} + +// gqlSelection is one top-level selection: a field name, a named fragment +// spread, or an inline fragment's own selections. +type gqlSelection struct { + field string + spread string + inline []gqlSelection +} + +type gqlParser struct { + toks []gqlToken + pos int +} + +func (p *gqlParser) peek(offset int) gqlToken { + if p.pos+offset < len(p.toks) { + return p.toks[p.pos+offset] + } + return gqlToken{} +} + +func (p *gqlParser) isPunct(text string) bool { + t := p.peek(0) + return t.kind == tokPunct && t.text == text +} + +func (p *gqlParser) done() bool { return p.pos >= len(p.toks) } + +// parseDocument returns named fragment selections and the top-level +// selections of each mutation operation. +func (p *gqlParser) parseDocument() (map[string][]gqlSelection, [][]gqlSelection) { + fragments := map[string][]gqlSelection{} + var mutations [][]gqlSelection + for !p.done() { + t := p.peek(0) + switch { + case t.kind == tokPunct && t.text == "{": + p.parseSelectionSet() + case t.kind == tokName && (t.text == "query" || t.text == "mutation" || t.text == "subscription"): + p.pos++ + if p.peek(0).kind == tokName { + p.pos++ + } + p.skipBalanced("(", ")") + p.skipDirectives() + if !p.isPunct("{") { + continue + } + sels := p.parseSelectionSet() + if t.text == "mutation" { + mutations = append(mutations, sels) + } + case t.kind == tokName && t.text == "fragment": + p.pos++ + name := p.peek(0).text + p.pos++ + if p.peek(0).kind == tokName && p.peek(0).text == "on" { + p.pos += 2 + } + p.skipDirectives() + if p.isPunct("{") { + fragments[name] = p.parseSelectionSet() + } + default: + p.pos++ + } + } + return fragments, mutations +} + +// parseSelectionSet consumes a `{ ... }` block and returns its top-level +// selections. Nested sub-selections are skipped. +func (p *gqlParser) parseSelectionSet() []gqlSelection { + var sels []gqlSelection + p.pos++ // "{" + for !p.done() && !p.isPunct("}") { + t := p.peek(0) + switch t.kind { + case tokSpread: + p.pos++ + next := p.peek(0) + if next.kind == tokName && next.text != "on" { + p.pos++ + p.skipDirectives() + sels = append(sels, gqlSelection{spread: next.text}) + continue + } + if next.kind == tokName && next.text == "on" { + p.pos += 2 + } + p.skipDirectives() + if p.isPunct("{") { + sels = append(sels, gqlSelection{inline: p.parseSelectionSet()}) + } + case tokName: + name := t.text + p.pos++ + if p.isPunct(":") && p.peek(1).kind == tokName { + name = p.peek(1).text + p.pos += 2 + } + p.skipBalanced("(", ")") + p.skipDirectives() + p.skipBalanced("{", "}") + sels = append(sels, gqlSelection{field: name}) + default: + p.pos++ + } + } + p.pos++ // "}" + return sels +} + +func (p *gqlParser) skipDirectives() { + for p.isPunct("@") { + p.pos++ + if p.peek(0).kind == tokName { + p.pos++ + } + p.skipBalanced("(", ")") + } +} + +// skipBalanced consumes a balanced open/close group when the next token is +// open; otherwise it does nothing. +func (p *gqlParser) skipBalanced(open, closer string) { + if !p.isPunct(open) { + return + } + depth := 0 + for !p.done() { + t := p.peek(0) + p.pos++ + if t.kind != tokPunct { + continue + } + switch t.text { + case open: + depth++ + case closer: + depth-- + if depth == 0 { + return + } + } + } +} diff --git a/internal/cmd/dashboards/dashboards.go b/internal/cmd/dashboards/dashboards.go index f11426c..adf1d81 100644 --- a/internal/cmd/dashboards/dashboards.go +++ b/internal/cmd/dashboards/dashboards.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/newrelic-cli/api" + "github.com/open-cli-collective/newrelic-cli/internal/ascode" "github.com/open-cli-collective/newrelic-cli/internal/cmd/root" "github.com/open-cli-collective/newrelic-cli/internal/confirm" "github.com/open-cli-collective/newrelic-cli/internal/view" @@ -19,6 +20,12 @@ func Register(rootCmd *cobra.Command, opts *root.Options) { Use: "dashboards", Aliases: []string{"dashboard", "dash"}, Short: "Manage New Relic dashboards", + Long: `Manage New Relic dashboards. + +create, update and delete print a note to stderr before the write, because +dashboards are often managed as code and a direct change can be overwritten by +the next deploy from that source. Stdout and exit codes are unaffected. Set +NEWRELIC_CLI_NO_AS_CODE_NOTICE=1 to hide the note.`, } dashboardsCmd.AddCommand(newListCmd(opts)) @@ -217,6 +224,7 @@ func runCreate(opts *createOptions) error { return err } + ascode.Notify(opts.Stderr, ascode.Dashboard) dashboard, err := client.CreateDashboard(&input) if err != nil { return fmt.Errorf("failed to create dashboard: %w", err) @@ -292,6 +300,7 @@ func runUpdate(opts *updateOptions, guid api.EntityGUID) error { return err } + ascode.Notify(opts.Stderr, ascode.Dashboard) dashboard, err := client.UpdateDashboard(guid, &input) if err != nil { return fmt.Errorf("failed to update dashboard: %w", err) @@ -358,6 +367,7 @@ func runDelete(opts *deleteOptions, guid api.EntityGUID) error { return fmt.Errorf("failed to get dashboard: %w", err) } + ascode.Notify(opts.Stderr, ascode.Dashboard) if !opts.force { p := &confirm.Prompter{ In: opts.Stdin, diff --git a/internal/cmd/nerdgraph/nerdgraph.go b/internal/cmd/nerdgraph/nerdgraph.go index 2dc1e2e..22ad787 100644 --- a/internal/cmd/nerdgraph/nerdgraph.go +++ b/internal/cmd/nerdgraph/nerdgraph.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/cobra" + "github.com/open-cli-collective/newrelic-cli/internal/ascode" "github.com/open-cli-collective/newrelic-cli/internal/cmd/root" ) @@ -44,7 +45,13 @@ data and functionality. Use the NerdGraph API explorer to discover available queries and mutations: https://api.newrelic.com/graphiql -Output is always JSON format.`, +Output is always JSON format. + +A mutation that creates, updates or deletes dashboards, alert policies, NRQL +alert conditions or synthetic monitors prints one note to stderr before it is +sent, because those resources are often managed as code. Stdout (the JSON +response) and exit codes are unaffected. Set NEWRELIC_CLI_NO_AS_CODE_NOTICE=1 +to hide the note.`, Example: ` # Get current user info nrq nerdgraph query '{ actor { user { email name } } }' @@ -99,6 +106,7 @@ func runQuery(opts *root.Options, query string) error { return err } + ascode.Notify(opts.Stderr, ascode.MutationKinds(query)...) result, err := client.NerdGraphQuery(query, nil) if err != nil { return err diff --git a/internal/cmd/root/root.go b/internal/cmd/root/root.go index de4850a..50d5884 100644 --- a/internal/cmd/root/root.go +++ b/internal/cmd/root/root.go @@ -103,6 +103,9 @@ Set the non-secret account ID / region (written to config.yml): Non-secret runtime overrides (precedence: env > config.yml): NEWRELIC_ACCOUNT_ID, NEWRELIC_REGION +Writes to dashboards, alerts and synthetic monitors print a stderr note that +these are often managed as code; NEWRELIC_CLI_NO_AS_CODE_NOTICE=1 hides it. + NEWRELIC_API_KEY is accepted ONLY as setup ingress (init/set-credential --from-env); it is no longer read at runtime (§1.11).` diff --git a/internal/cmd/synthetics/synthetics.go b/internal/cmd/synthetics/synthetics.go index 131b028..9698fdd 100644 --- a/internal/cmd/synthetics/synthetics.go +++ b/internal/cmd/synthetics/synthetics.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" "github.com/open-cli-collective/newrelic-cli/api" + "github.com/open-cli-collective/newrelic-cli/internal/ascode" "github.com/open-cli-collective/newrelic-cli/internal/cmd/root" "github.com/open-cli-collective/newrelic-cli/internal/confirm" "github.com/open-cli-collective/newrelic-cli/internal/view" @@ -30,7 +31,12 @@ and https://docs.newrelic.com/docs/apis/nerdgraph/examples/synthetics-api/overview/. Note: 'list' and 'get' read the entity index, which can lag create/update -by a minute or so; create and update print the applied state directly.`, +by a minute or so; create and update print the applied state directly. + +create, update and delete print a note to stderr before the write, because +monitors are often managed as code and a direct change can be overwritten by +the next deploy from that source. Stdout and exit codes are unaffected. Set +NEWRELIC_CLI_NO_AS_CODE_NOTICE=1 to hide the note.`, } syntheticsCmd.AddCommand(newListCmd(opts)) @@ -251,6 +257,7 @@ func runCreate(opts *createOptions) error { return err } + ascode.Notify(opts.Stderr, ascode.SyntheticMonitor) monitor, err := client.CreateSyntheticMonitor(&input) if err != nil { return fmt.Errorf("failed to create monitor: %w", err) @@ -328,6 +335,7 @@ func runUpdate(opts *updateOptions, monitorID string) error { return err } + ascode.Notify(opts.Stderr, ascode.SyntheticMonitor) monitor, err := client.UpdateSyntheticMonitor(monitorID, &input) if err != nil { return fmt.Errorf("failed to update monitor: %w", err) @@ -403,6 +411,7 @@ func runDelete(opts *deleteOptions, monitorID string) error { return fmt.Errorf("failed to get monitor: %w", err) } + ascode.Notify(opts.Stderr, ascode.SyntheticMonitor) if !opts.force { p := &confirm.Prompter{ In: opts.Stdin, From df8cabf53166e5957e570725d8f641862184900d Mon Sep 17 00:00:00 2001 From: Caleb Piekstra Date: Wed, 21 Oct 2026 10:50:02 -0400 Subject: [PATCH 2/2] test: cover every notice call site and keep one kind order Adds command cases for update and synthetics delete, a declined delete that shows the note before the prompt and sends no write, one canonical kind list, and a pointer to the NerdGraph schema as the source of the detected mutation fields. --- cmd/nrq/ascode_notice_test.go | 169 ++++++++++++++---- .../testdata/ascode/dashboard_updated.json | 20 +++ .../ascode/synthetics_monitor_deleted.json | 7 + .../ascode/synthetics_monitor_get.json | 17 ++ .../ascode/synthetics_monitor_search.json | 24 +++ .../ascode/synthetics_monitor_updated.json | 15 ++ docs/development.md | 11 +- internal/ascode/ascode.go | 8 +- internal/ascode/ascode_test.go | 10 ++ internal/ascode/graphql.go | 12 +- 10 files changed, 248 insertions(+), 45 deletions(-) create mode 100644 cmd/nrq/testdata/ascode/dashboard_updated.json create mode 100644 cmd/nrq/testdata/ascode/synthetics_monitor_deleted.json create mode 100644 cmd/nrq/testdata/ascode/synthetics_monitor_get.json create mode 100644 cmd/nrq/testdata/ascode/synthetics_monitor_search.json create mode 100644 cmd/nrq/testdata/ascode/synthetics_monitor_updated.json diff --git a/cmd/nrq/ascode_notice_test.go b/cmd/nrq/ascode_notice_test.go index f74f09a..d22ef36 100644 --- a/cmd/nrq/ascode_notice_test.go +++ b/cmd/nrq/ascode_notice_test.go @@ -46,6 +46,8 @@ type noticeHarness struct { routes []nerdGraphRoute stderr *bytes.Buffer sent []sentRequest + // stdin is what the command reads, e.g. a delete confirmation answer. + stdin string } func newNoticeHarness(t *testing.T, routes ...nerdGraphRoute) *noticeHarness { @@ -106,7 +108,7 @@ func (h *noticeHarness) run(args ...string) (string, string, error) { h.stderr = &stderr opts.Stdout = &stdout opts.Stderr = &stderr - opts.Stdin = strings.NewReader("") + opts.Stdin = strings.NewReader(h.stdin) rootCmd.SetErr(&stderr) rootCmd.SetArgs(append([]string{"--no-color"}, args...)) err := rootCmd.Execute() @@ -195,48 +197,141 @@ func TestNerdGraphMutation_NoteShownWhenWriteFails(t *testing.T) { assert.True(t, strings.HasPrefix(stderr, ascode.Message(ascode.Dashboard)), stderr) } -func TestDashboardsCreate_Note(t *testing.T) { - input := filepath.Join("testdata", "ascode", "dashboard_input.json") - route := nerdGraphRoute{"dashboardCreate", "dashboard_created.json"} +const ( + monitorID = "00000000-0000-4000-8000-000000000001" + monitorInput = "testdata/ascode/synthetics_monitor_input.json" + dashInput = "testdata/ascode/dashboard_input.json" +) + +var ( + dashboardGetRoute = nerdGraphRoute{"entity(guid", "dashboard_get.json"} + monitorFindRoute = nerdGraphRoute{"entitySearch", "synthetics_monitor_search.json"} +) + +func TestWriteCommands_NoteOnceBeforeWrite(t *testing.T) { + tests := []struct { + name string + args []string + routes []nerdGraphRoute + mutation string + kind ascode.Kind + }{ + { + name: "dashboards create", + args: []string{"dash", "create", "-f", dashInput}, + routes: []nerdGraphRoute{{"dashboardCreate", "dashboard_created.json"}}, + mutation: "dashboardCreate", + kind: ascode.Dashboard, + }, + { + name: "dashboards update", + args: []string{"dashboard", "update", dashboardGUID, "-f", dashInput}, + routes: []nerdGraphRoute{{"dashboardUpdate", "dashboard_updated.json"}}, + mutation: "dashboardUpdate", + kind: ascode.Dashboard, + }, + { + name: "dashboards delete --force", + args: []string{"dashboards", "delete", dashboardGUID, "--force"}, + routes: []nerdGraphRoute{{"dashboardDelete", "nerdgraph_dashboard_deleted.json"}, dashboardGetRoute}, + mutation: "dashboardDelete", + kind: ascode.Dashboard, + }, + { + name: "synthetics create", + args: []string{"syn", "create", "-f", monitorInput}, + routes: []nerdGraphRoute{{"syntheticsCreateSimpleMonitor", "synthetics_monitor_created.json"}}, + mutation: "syntheticsCreateSimpleMonitor", + kind: ascode.SyntheticMonitor, + }, + { + name: "synthetics update", + args: []string{"synthetic", "update", monitorID, "-f", monitorInput}, + routes: []nerdGraphRoute{{"syntheticsUpdateSimpleMonitor", "synthetics_monitor_updated.json"}, monitorFindRoute}, + mutation: "syntheticsUpdateSimpleMonitor", + kind: ascode.SyntheticMonitor, + }, + { + name: "synthetics delete --force", + args: []string{"synthetics", "delete", monitorID, "--force"}, + // delete resolves the monitor by ID, then again by its GUID. + routes: []nerdGraphRoute{ + {"syntheticsDeleteMonitor", "synthetics_monitor_deleted.json"}, + monitorFindRoute, + {"entity(guid", "synthetics_monitor_get.json"}, + }, + mutation: "syntheticsDeleteMonitor", + kind: ascode.SyntheticMonitor, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := newNoticeHarness(t, tt.routes...) + _, stderr, err := h.run(tt.args...) + require.NoError(t, err) + + note := ascode.Message(tt.kind) + assert.True(t, strings.HasPrefix(stderr, note), stderr) + assert.Equal(t, 1, strings.Count(stderr, "Note:"), "note printed once") + sent := h.sentWith(tt.mutation) + require.Len(t, sent, 1) + assert.Equal(t, note, sent[0].stderrAtSend, "note precedes the write") + }) + } +} + +func TestDeleteDeclined_NoteBeforePromptAndNoWrite(t *testing.T) { + tests := []struct { + name string + args []string + routes []nerdGraphRoute + mutation string + kind ascode.Kind + prompt string + }{ + { + name: "dashboards delete", + args: []string{"dashboards", "delete", dashboardGUID}, + routes: []nerdGraphRoute{dashboardGetRoute}, + mutation: "dashboardDelete", + kind: ascode.Dashboard, + prompt: `Delete dashboard "Checkout Overview"`, + }, + { + name: "synthetics delete", + args: []string{"synthetics", "delete", monitorID}, + routes: []nerdGraphRoute{monitorFindRoute}, + mutation: "syntheticsDeleteMonitor", + kind: ascode.SyntheticMonitor, + prompt: `Delete synthetic monitor "checkout-service health"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := newNoticeHarness(t, tt.routes...) + h.stdin = "n\n" + stdout, stderr, err := h.run(tt.args...) + require.NoError(t, err) + assert.Empty(t, stdout) - h := newNoticeHarness(t, route) - stdout, stderr, err := h.run("dash", "create", "-f", input, "-o", "plain") + note := ascode.Message(tt.kind) + require.True(t, strings.HasPrefix(stderr, note), stderr) + assert.True(t, strings.HasPrefix(strings.TrimPrefix(stderr, note), tt.prompt), + "prompt follows the note: %q", stderr) + assert.Empty(t, h.sentWith(tt.mutation), "a declined delete sends no mutation") + }) + } +} + +func TestDashboardsCreate_SilencedStdoutUnchanged(t *testing.T) { + h := newNoticeHarness(t, nerdGraphRoute{"dashboardCreate", "dashboard_created.json"}) + stdout, stderr, err := h.run("dash", "create", "-f", dashInput, "-o", "plain") require.NoError(t, err) assert.Equal(t, ascode.Message(ascode.Dashboard), stderr) - sent := h.sentWith("dashboardCreate") - require.Len(t, sent, 1) - assert.Equal(t, ascode.Message(ascode.Dashboard), sent[0].stderrAtSend, "note precedes the write") t.Setenv(ascode.DisableEnv, "true") - silencedStdout, silencedStderr, err := h.run("dash", "create", "-f", input, "-o", "plain") + silencedStdout, silencedStderr, err := h.run("dash", "create", "-f", dashInput, "-o", "plain") require.NoError(t, err) assert.Empty(t, silencedStderr) assert.Equal(t, silencedStdout, stdout, "the note never changes stdout") } - -func TestDashboardsDelete_NoteOnce(t *testing.T) { - h := newNoticeHarness(t, - nerdGraphRoute{"dashboardDelete", "nerdgraph_dashboard_deleted.json"}, - nerdGraphRoute{"entity(guid", "dashboard_get.json"}, - ) - stdout, stderr, err := h.run("dashboards", "delete", dashboardGUID, "--force") - require.NoError(t, err) - assert.Empty(t, stdout) - assert.Equal(t, 1, strings.Count(stderr, "Note:")) - assert.True(t, strings.HasPrefix(stderr, ascode.Message(ascode.Dashboard)), stderr) - sent := h.sentWith("dashboardDelete") - require.Len(t, sent, 1) - assert.Equal(t, ascode.Message(ascode.Dashboard), sent[0].stderrAtSend, "note precedes the write") -} - -func TestSyntheticsCreate_Note(t *testing.T) { - h := newNoticeHarness(t, nerdGraphRoute{"syntheticsCreateSimpleMonitor", "synthetics_monitor_created.json"}) - input := filepath.Join("testdata", "ascode", "synthetics_monitor_input.json") - stdout, stderr, err := h.run("syn", "create", "-f", input, "-o", "plain") - require.NoError(t, err) - assert.Equal(t, ascode.Message(ascode.SyntheticMonitor), stderr) - assert.Contains(t, stdout, "checkout-service health") - sent := h.sentWith("syntheticsCreateSimpleMonitor") - require.Len(t, sent, 1) - assert.Equal(t, ascode.Message(ascode.SyntheticMonitor), sent[0].stderrAtSend, "note precedes the write") -} diff --git a/cmd/nrq/testdata/ascode/dashboard_updated.json b/cmd/nrq/testdata/ascode/dashboard_updated.json new file mode 100644 index 0000000..909fb97 --- /dev/null +++ b/cmd/nrq/testdata/ascode/dashboard_updated.json @@ -0,0 +1,20 @@ +{ + "data": { + "dashboardUpdate": { + "entityResult": { + "guid": "MTIzNDU2N3xWSVp8REFTSEJPQVJEfDEyMzQ1Njc4OQ==", + "name": "Checkout Overview", + "description": "", + "permissions": "PUBLIC_READ_WRITE", + "pages": [ + { + "guid": "MTIzNDU2N3xWSVp8REFTSEJPQVJEfDEyMzQ1Njc5MA==", + "name": "Overview", + "widgets": [] + } + ] + }, + "errors": [] + } + } +} diff --git a/cmd/nrq/testdata/ascode/synthetics_monitor_deleted.json b/cmd/nrq/testdata/ascode/synthetics_monitor_deleted.json new file mode 100644 index 0000000..3f77f4a --- /dev/null +++ b/cmd/nrq/testdata/ascode/synthetics_monitor_deleted.json @@ -0,0 +1,7 @@ +{ + "data": { + "syntheticsDeleteMonitor": { + "deletedGuid": "MTIzNDU2N3xTWU5USHxNT05JVE9SfDEyMzQ1Njc4OQ==" + } + } +} diff --git a/cmd/nrq/testdata/ascode/synthetics_monitor_get.json b/cmd/nrq/testdata/ascode/synthetics_monitor_get.json new file mode 100644 index 0000000..869521d --- /dev/null +++ b/cmd/nrq/testdata/ascode/synthetics_monitor_get.json @@ -0,0 +1,17 @@ +{ + "data": { + "actor": { + "entity": { + "guid": "MTIzNDU2N3xTWU5USHxNT05JVE9SfDEyMzQ1Njc4OQ==", + "name": "checkout-service health", + "monitorId": "00000000-0000-4000-8000-000000000001", + "monitorType": "SIMPLE", + "period": 10, + "monitoredUrl": "https://example.com/health", + "monitorSummary": { + "status": "ENABLED" + } + } + } + } +} diff --git a/cmd/nrq/testdata/ascode/synthetics_monitor_search.json b/cmd/nrq/testdata/ascode/synthetics_monitor_search.json new file mode 100644 index 0000000..4ffc25b --- /dev/null +++ b/cmd/nrq/testdata/ascode/synthetics_monitor_search.json @@ -0,0 +1,24 @@ +{ + "data": { + "actor": { + "entitySearch": { + "results": { + "nextCursor": null, + "entities": [ + { + "guid": "MTIzNDU2N3xTWU5USHxNT05JVE9SfDEyMzQ1Njc4OQ==", + "name": "checkout-service health", + "monitorId": "00000000-0000-4000-8000-000000000001", + "monitorType": "SIMPLE", + "period": 10, + "monitoredUrl": "https://example.com/health", + "monitorSummary": { + "status": "ENABLED" + } + } + ] + } + } + } + } +} diff --git a/cmd/nrq/testdata/ascode/synthetics_monitor_updated.json b/cmd/nrq/testdata/ascode/synthetics_monitor_updated.json new file mode 100644 index 0000000..891953c --- /dev/null +++ b/cmd/nrq/testdata/ascode/synthetics_monitor_updated.json @@ -0,0 +1,15 @@ +{ + "data": { + "result": { + "errors": [], + "monitor": { + "guid": "MTIzNDU2N3xTWU5USHxNT05JVE9SfDEyMzQ1Njc4OQ==", + "id": "00000000-0000-4000-8000-000000000001", + "name": "checkout-service health", + "period": "EVERY_10_MINUTES", + "status": "ENABLED", + "uri": "https://example.com/health" + } + } + } +} diff --git a/docs/development.md b/docs/development.md index bf9c722..966de67 100644 --- a/docs/development.md +++ b/docs/development.md @@ -112,8 +112,15 @@ delete's confirmation prompt), so the note is visible even when the write fails. `nerdgraph query` derives the kinds from the document with `ascode.MutationKinds`, which matches top-level mutation fields only, including those reached through fragments. A new write command for one of these -resources calls `ascode.Notify`; a new resource kind goes in `ascode.Kind` and -`classifyMutationField`. The note goes to stderr only and never changes stdout +resources calls `ascode.Notify`; a new resource kind is a constant in +`ascode.Kind`, an entry in `allKinds`, its plural, and a case in +`classifyMutationField`. + +The detected field names come from the `Mutation` type of the NerdGraph schema +(browse it in the [NerdGraph API explorer](https://api.newrelic.com/graphiql)). +When changing `internal/ascode`, check that type for new `dashboard*`, +`alerts*` and `synthetics*` fields that create, update or delete these +resources, and add each to `classifyMutationField` with a test case. The note goes to stderr only and never changes stdout or the exit code. ## Shared Repo Standards diff --git a/internal/ascode/ascode.go b/internal/ascode/ascode.go index 8128d59..6ebd2ce 100644 --- a/internal/ascode/ascode.go +++ b/internal/ascode/ascode.go @@ -23,7 +23,8 @@ const DisableEnv = "NEWRELIC_CLI_NO_AS_CODE_NOTICE" // Kind is a resource kind that teams commonly manage as code. type Kind int -// Kinds in the order the note names them. +// Kinds. A new kind is a constant here, an entry in allKinds and a case in +// plural. const ( Dashboard Kind = iota + 1 AlertPolicy @@ -31,6 +32,9 @@ const ( SyntheticMonitor ) +// allKinds is the canonical order in which notes and MutationKinds list kinds. +var allKinds = []Kind{Dashboard, AlertPolicy, AlertCondition, SyntheticMonitor} + func (k Kind) plural() string { switch k { case Dashboard: @@ -69,7 +73,7 @@ func Message(kinds ...Kind) string { seen[k] = true } var names []string - for _, k := range []Kind{Dashboard, AlertPolicy, AlertCondition, SyntheticMonitor} { + for _, k := range allKinds { if seen[k] { names = append(names, k.plural()) } diff --git a/internal/ascode/ascode_test.go b/internal/ascode/ascode_test.go index 7c58a9c..b59b273 100644 --- a/internal/ascode/ascode_test.go +++ b/internal/ascode/ascode_test.go @@ -217,3 +217,13 @@ func TestNotify(t *testing.T) { }) } } + +func TestAllKindsHavePlurals(t *testing.T) { + seen := map[string]bool{} + for _, k := range allKinds { + p := k.plural() + assert.NotEqual(t, Kind(0).plural(), p, "kind %d needs a case in plural", k) + assert.False(t, seen[p], "duplicate plural %q", p) + seen[p] = true + } +} diff --git a/internal/ascode/graphql.go b/internal/ascode/graphql.go index 24dbcfe..d0860f4 100644 --- a/internal/ascode/graphql.go +++ b/internal/ascode/graphql.go @@ -37,7 +37,7 @@ func MutationKinds(document string) []Kind { } var kinds []Kind - for _, k := range []Kind{Dashboard, AlertPolicy, AlertCondition, SyntheticMonitor} { + for _, k := range allKinds { if seen[k] { kinds = append(kinds, k) } @@ -62,9 +62,13 @@ var alertPolicyMutations = map[string]bool{ } // classifyMutationField maps a NerdGraph mutation field name to the kind it -// writes. Dashboard and policy fields are an explicit list because sibling -// mutations (dashboardCreateSnapshotUrl, alertsPolicyAddNotificationChannels) -// do not change the resource definition. +// writes. The field names come from the Mutation type of the NerdGraph schema +// (https://api.newrelic.com/graphiql). When changing this package, check that +// type for new dashboard*, alerts* and synthetics* fields that create, update +// or delete these resources and add them here with a test case. Dashboard and +// policy fields are an explicit list because sibling mutations +// (dashboardCreateSnapshotUrl, alertsPolicyAddNotificationChannels) do not +// change the resource definition. func classifyMutationField(name string) (Kind, bool) { switch { case dashboardMutations[name]: