Skip to content

feat: mattermost target with operator alerts, per-group format rules, autocomplete fixes (includes #70) - #73

Open
christopherfickess wants to merge 20 commits into
mainfrom
feat/mattermost-operator-pack
Open

christopherfickess wants to merge 20 commits into
mainfrom
feat/mattermost-operator-pack

Conversation

@christopherfickess

@christopherfickess christopherfickess commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Builds on #70 (this branch contains all of its commits) and adds:

  • mattermost target with mm-namespace=<ns> / mm-name=<cr> (the rules match namespace plus the installation.mattermost.com/installation pod label, so they work whichever Service is scraped; the ServiceMonitor needs podTargetLabels for it): the 5 runbook receivers plus one mattermost-server receiver for the 13 Mattermost Operator alerts, and one merged PrometheusRule (mattermost-alertmanager-rules-mattermost, 18 alerts).
  • format rules <group|runbook|all> [crd|standard]: DM just one group's rules, named mattermost-alertmanager-rules-<group>, the same object config add sends.
  • Autocomplete: one option at a time after config add; help typeahead; format rules offers the group, then mm-namespace, then the format.
  • Removed the license-expiry alert: there is no such Mattermost metric. Stale metric names in the mattermost runbooks are fixed.
  • Deps: goldmark 1.8.7 in build/render-docs.

Review guide

The branch has three commits after the feature work. Review them in this order:

  1. feat(security): central command registry, receiver kinds, channel-scoped silences is the behaviour change and the part that needs a security read:
    • command_registry.go: every verb is authorized before it runs. Check each verb's access level against docs/SLASH_COMMANDS.md.
    • cmd_silence.go, channel_rules.go: silences are recorded per channel and Alertmanager. expire and list must not cross channels or Alertmanagers.
    • am_errors.go, alertmanager/ssrf.go: the blocked address and suggested CIDR reach system admins only.
    • templates.go, templates_crd.go: runbook_url is limited to http(s):// and has no whitespace, quotes or angle brackets.
    • cmd_rules.go, cmd_allowlist.go: image-registry allowlist entries appear in generated rules for team admins only.
    • plugin.go: requireChannelTeamAdmin rejects channels with no team.
  2. refactor: split cmd_scaffold, cmd_lifecycle and admin_inventory by concern moves code between files and changes nothing else. Skip it in review, or diff the function bodies.
  3. refactor: build handleAdd and the inventory page from stage functions splits two long functions. The inventory page output was compared byte for byte against the old code for seven queries, and inventory_page_test.go covers the CL-20 POST-only gate for webhook-test and end-to-end.

Supersedes

Merging this makes these redundant, so they can be closed:

Test plan

  • go test -race ./...
  • golangci-lint run ./server/ ./server/alertmanager/
  • make dist builds
  • promtool and kubectl apply --dry-run=server on the generated per-group PrometheusRules
  • Inventory page output unchanged by the refactor (golden diff, seven queries)
  • Live on a Flux-managed cluster: label on up, 8 scrape-based rules healthy and matching, no match for another install or namespace
  • Install the bundle and run config add <url> mattermost crd mm-namespace=<ns> and format rules security crd

Made with Cursor

christopher.fickess and others added 20 commits September 21, 2026 17:22
Redesigns /alertmanager show diff to scope output to the current channel
only instead of showing the full AM config as context. Eliminates noise
from other channels' receivers and routes.

Key changes:
- detectCRDMode: auto-detects Prometheus Operator environments by looking
  for namespaced receiver names (<ns>/mattermost-alertmanager-<hash>/<name>)
  matching this channel's receivers in AM's loaded config
- receiverLoadedInAM: CRD-aware already-loaded check — matches on the
  third path segment in namespaced names, not just the full string
- extractReceiverBlocks / extractRouteItems: extract only this channel's
  multi-line YAML blocks from the loaded config
- buildChannelScopedDiff: produces the channel-scoped diff with context
  lines (  prefix) for what's loaded and + lines for what would be added
- Fixed receiverNameLineRegex to use \s* instead of \s+ so it matches
  indent-0 receivers in raw YAML (was a latent bug in redactOtherChannels
  too, affecting configs where receivers: items start at column 0)
- Adds 19 new table-driven tests covering all new helpers

Also includes Mattermost runbook set (6 runbooks), updated catalog/docs,
/alertmanager show positional keyword dispatch (show export/diff/<name>),
and several other field-feedback improvements accumulated on this branch.

Co-Authored-By: christopher.fickess <christopher.fickess@mattermost.com>
Go dependencies (root):
- google.golang.org/grpc 1.82.1 → 1.83.2 (security fixes in xDS RBAC
  and HTTP/2 frame flood handling)
- github.com/prometheus/alertmanager 0.33.1 → 0.34.0
- github.com/mattermost/mattermost/server/public 0.4.3 → 0.4.4
- github.com/prometheus/common 0.70.1 → 0.71.0
- github.com/go-openapi/strfmt 0.27.0 → 0.27.2
- github.com/stretchr/testify 1.12.0 → 1.12.1
- transitively updated indirect deps

Go dependencies (/build):
- google.golang.org/grpc 1.82.1 → 1.83.2

Go dependencies (/build/render-docs):
- github.com/yuin/goldmark 1.8.5 → 1.8.6

GitHub Actions:
- actions/deploy-pages v5.0.0 → v5.0.1
- softprops/action-gh-release v3.0.2 → v3.0.3
- anchore/scan-action, github/codeql-action/* → v3.30.0 SHA refresh

Security workflow: split CodeQL gate by branch
- main + scheduled: continue-on-error: false (hard fail, blocks release)
- PR / branch push: continue-on-error: true (warn, finding visible in
  Security tab via SARIF upload but job stays green)
- Grype: unchanged (fail-build: false on all triggers)

Co-Authored-By: christopher.fickess <christopher.fickess@mattermost.com>
A shorter receiver name was treated as already loaded when a longer one existed, so the diff dropped the add. RFC1918 allowlist hints stay a /16; every other blocked address is now the exact host.

Co-authored-by: Cursor <cursoragent@cursor.com>
Clears the two SSH connection deadlocks Grype reported against v0.55.0. That version was already selected on main; the findings showed up here because go.sum changed.

Co-authored-by: Cursor <cursoragent@cursor.com>
…eleases

Clears the critical and high x/net advisories Grype flagged on PR 70.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…nnel-scoped silence expire

- Add /alertmanager format (export, diff, rules); config export|diff and rules reply with a moved message.
- Replace every -- flag with bare words (force, prune, overdue, end-to-end, ...) and key=value (namespace=, webhook-host=, severity=). Old -- spellings are rejected with a rewrite hint.
- config add, add-custom and format export send the config and the Prometheus rules as attachments on one DM post.
- validate typeahead lists only the groups present in the current channel (membership-checked endpoint).
- Typeahead is alphabetical; silence TTLs go by duration.
- silence expire only acts on silences created from the calling channel with silence create (per-silence KV record), so a team admin cannot end another team's silence on a shared Alertmanager.

Co-authored-by: Cursor <cursoragent@cursor.com>
… scoped silence expire

Co-authored-by: Cursor <cursoragent@cursor.com>
PersistentVolumeFull failed to evaluate when a claim is mounted by more than one pod (many-to-many matching). The claim is now the one side of a group_right join over per-pod series.

UnexpectedContainerImage allowed docker.io/bitnamilabs/, which flags the real docker.io/bitnamilegacy/postgresql image and allowlists a registry org anyone could register. Allow bitnamilegacy instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
…tedContainerImage

check rules only parses the PromQL, so the many-to-many join error passed CI. The new tests evaluate it with two pods on one claim. CI runs them after check rules.

Co-authored-by: Cursor <cursoragent@cursor.com>
…l selector gotcha

Explain why stock etcd, scheduler, controller-manager and kube-proxy alerts fire on managed clusters and RKE2, give the plain chart values that turn them off, and document how to check that Prometheus actually selected a rule or ServiceMonitor.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ad metric names

The mattermost target now covers both the runbook alerts and, with
mm-namespace=<ns>, the 13 alerts the Mattermost Operator generates. There is no
separate mattermost-operator target.

- config add <am-url> mattermost mm-namespace=<ns> [mm-name=<cr>] adds one extra
  receiver (mattermost-server--<team>-<channel>) that routes by alertname, and
  the DM carries one merged PrometheusRule (group rules + operator rules).
- format rules mattermost [mm-namespace=<ns>] returns the group's rules, or the
  merged file with mm-namespace.
- config remove mattermost force also removes the operator receiver, including
  legacy mattermost-operator-- entries.
- Autocomplete offers mm-namespace= only after the word mattermost.
- CRD export numbers only runbook groups, so adding the operator alerts never
  renames the runbook group's objects. The operator objects end in
  -mattermost-server.
- samples/mattermost-operator-rules.yaml is a verbatim copy of the operator's
  rules; CI runs promtool on the rendered and merged files.

Rules fixes:
- Correct stale Mattermost metric names (mattermost_api_time_*,
  mattermost_db_store_time_bucket, login and HTTP counters) in the sample rules,
  runbooks and requirements doc.
- Remove MattermostLicenseExpiringSoon and its runbook. Mattermost exports no
  license-expiry metric (checked in the server source and on a live /metrics
  endpoint), so the alert could never fire. 36 runbooks become 35 and 37 rules
  become 36.

Known limits: MattermostHighErrorRate and MattermostHighLoginFailures overlap the
operator's HTTP error and login alerts; silence and validate cover the runbook
alerts only; format export omits the operator rules.

Co-authored-by: Cursor <cursoragent@cursor.com>
…peahead

Mattermost merges the suggestions of every optional argument that follows, so
`config add <url> mattermost ` listed crd, standard, mm-namespace= and on in one
hover.

- config add and add-custom now end in required dynamic lists that all ask one
  handler (/autocomplete/add-options). It returns the next option only: format,
  then mm-namespace= (mattermost target only), then on. Once the user types a
  word, every unused option is returned so they can skip one or reorder.
  A free-typed runbook slug and finished flags such as namespace= are consumed so
  the later options still show.
- `help` now offers the command path (help config add), one level at a time.
- Leaf commands offer a trailing `help` (config add help), which the server
  already accepted. Duration lists are left alone so they stay sorted.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ime hover

format rules <group|runbook|all> [crd|standard] now DMs just that group's
PrometheusRule, named mattermost-alertmanager-rules-<group> like config add.
The hover offers the group first, then mm-namespace for mattermost, then the
format. Replaces the mattermost-pack hover handler.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Bump plugin.json and the release-please manifest to 1.5.0 and write the
changelog by hand, since release-please is not producing release PRs.
The operator-rules promtool step wrote 0600 files that the promtool
container could not read.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ped silences

Review fixes for the operator-pack PR.

- Command registry: every verb is parsed, authorized and run through one
  path. Access levels are declared per verb instead of checked ad hoc.
- Receiver kinds (runbook/custom/pack) replace the Custom/Pack flag pair.
  Stored entries are read through resolvedKind(), so no migration.
- silence: expire only ends silences created from the calling channel,
  list is filtered to the channel, create is limited to the channel's
  alert set (shipped runbooks plus pack alert names).
- runbook_url in the Alertmanager template only follows http(s) URLs.
- SSRF: BlockedDestinationError keeps the blocked address and suggested
  CIDR out of the generic message; only system admins see the detail.
- Image-registry allowlist entries go into generated rules for team
  admins only. Autocomplete requests are size-capped.
- requireChannelTeamAdmin rejects channels with no team.
- mm-service flag and service=~ placeholder fix for operators without a
  <name>-metrics Service.
- Docs: receiver kinds, self-hosted Kubernetes flow, slash commands.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ncern

No behaviour change. Declarations were moved by script and the pre/post
sources compared line for line; only the stale file header on
cmd_lifecycle.go was dropped. The inventory page HTML moves into
inventory.html, loaded with go:embed, byte-identical to the old string.

- cmd_scaffold.go -> cmd_add.go, cmd_add_custom.go, cmd_add_destination.go,
  runbook_catalog.go, receiver_names.go, routes_assemble.go
- cmd_lifecycle.go -> cmd_remove.go, cmd_rotate.go, cmd_config_view.go,
  receiver_store.go
- admin_inventory.go -> inventory_rows.go, inventory_actions.go,
  inventory_template.go, inventory.html

Co-authored-by: Cursor <cursoragent@cursor.com>
handleAdd becomes parseAddRequest -> createAddReceivers ->
persistAddEntries -> renderAddSummary/deliverAddBundle. The config write
lock still spans create and persist. Messages, log calls and ordering are
unchanged.

renderInventoryHTML becomes a composition of probeInventoryAMs,
buildInventoryRows, amOnlyReceivers, parseInventoryForm and
runInventoryForm, with the page data in a named inventoryPageData type.
The CL-20 POST-only gate for webhook-test and end-to-end is unchanged and
now has a test. Rendered output for seven representative queries was
compared byte for byte against the previous implementation.

Co-authored-by: Cursor <cursoragent@cursor.com>
…lation, drop mm-service

The operator alerts filtered on the scraped Service name, which only holds when
Prometheus scrapes that exact Service. They now match namespace plus the
installation.mattermost.com/installation pod label (exposed by Prometheus as
installation_mattermost_com_installation), so they hold whichever Service is
scraped. The ServiceMonitor must copy that label with podTargetLabels.

- samples/mattermost-operator-rules.yaml: __SERVICE__ becomes __INSTALLATION__,
  by-clauses and annotations use the installation label.
- mm-service is removed from config add and format rules. It was never released.
- Tests: no rule depends on the Service name, and every series selector carries
  the namespace.
- Docs: how to add podTargetLabels to a ServiceMonitor, and how to verify it.

Co-authored-by: Cursor <cursoragent@cursor.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant