chore: drop unbuilt advanced_deliverability + route api reloads via orchestrator#71
Merged
Merged
Conversation
added 2 commits
June 13, 2026 15:17
…e taxonomy
`advanced_deliverability` was carried in EnterpriseFeatures, the wire
contract, and the tier-inference switches, but nothing gates on it: the
/domains/{id}/deliverability endpoint is a basic DNS-readiness check open
to all tiers, and "Advanced deliverability monitoring" is a Roadmap item
on the marketing pricing/features tables — there is no shipped code path.
That meant an Enterprise license advertised an entitlement that doesn't
exist. Remove the constant + every reference (mirroring how multi_tenant
was handled) and leave a NOTE documenting both deferred features and the
condition for re-introducing them. No behaviour change for any real tier.
…docker The api container has no docker socket of its own — only the orchestrator mounts /var/run/docker.sock. So api code calling engine.ExecuteActions (which shells out to `docker exec`/`kill`/`restart`) logged `rspamd reload failed — exec "docker" not found in $PATH` after DKIM and spam-config updates. Benign today (rspamd hot-reloads its maps natively), but the reload should go through the component that actually owns docker. Add a best-effort POST /internal/reload endpoint to the orchestrator that runs ExecuteActions on the caller's behalf, plus a Client.Reload method. The api now delegates all three reload sites (config apply + DKIM/spam regen) via s.reloadServices, which falls back to direct execution when no orchestrator client is configured (host/CLI contexts, unit tests). Reload is independent of the apply state machine.
There was a problem hiding this comment.
Pull request overview
Removes the unshipped advanced_deliverability entitlement from the ValidonX feature taxonomy and updates tier inference/tests accordingly, while also routing API-triggered service reloads through the orchestrator (the only container with Docker access) via a new internal reload endpoint and client method.
Changes:
- Dropped
advanced_deliverabilityfrom feature constants, enterprise feature set, and tier inference; updated wire-contract and integration/unit tests. - Added
POST /internal/reloadto the orchestrator and anorchestrator.Client.Reloadmethod. - Updated API reload call sites (config apply + rspamd DKIM/spam regen) to delegate reload/restart actions via the orchestrator, with a local fallback when the orchestrator client isn’t configured.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/validonx/middleware.go | Removes advanced_deliverability from the feature taxonomy and enterprise tier inference, and documents omitted/unbuilt features. |
| internal/validonx/middleware_test.go | Updates tier inference tests to reflect removed deliverability feature. |
| internal/validonx/feature_wire_contract_test.go | Updates enterprise wire-contract expectations after dropping deliverability. |
| internal/orchestrator/server.go | Adds POST /internal/reload handler to execute service reload/restart actions via orchestrator Docker access. |
| internal/orchestrator/client.go | Adds Client.Reload and refactors JSON helper to support optional request bodies. |
| internal/enterprise_license_gate_integration_test.go | Updates enterprise license feature set used in E2E test after deliverability removal. |
| internal/api/handle_license.go | Updates cached-feature tier inference after deliverability removal. |
| internal/api/handle_license_test.go | Updates tier inference tests after deliverability removal. |
| internal/api/handle_domains.go | Introduces reloadServices helper and switches rspamd reload sites to delegate via orchestrator. |
| internal/api/handle_config.go | Switches config-apply service actions to go through reloadServices (orchestrator delegation). |
Comment on lines
+300
to
+305
| s.respondJSON(w, http.StatusOK, map[string]any{ | ||
| "data": map[string]any{ | ||
| "results": results, | ||
| }, | ||
| }) | ||
| } |
Match the {state, data} envelope used by plan/apply/rollback/status (and
documented in the client) so the internal API contract is consistent.
Addresses Copilot review feedback on #71.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two small follow-ups from the 2026-05-31 audit and the v0.1.26 prod deploy. Independent commits.
1. Drop unbuilt
advanced_deliverabilityfrom the feature taxonomyadvanced_deliverabilitywas carried inEnterpriseFeatures, the wire contract, and both tier-inference switches — but nothing gates on it:/domains/{id}/deliverabilityendpoint is a basic DNS-readiness check (MX/SPF/DKIM/DMARC) open to all tiers.So an Enterprise license advertised an entitlement that doesn't exist. Removed the constant + every reference, mirroring how
multi_tenantwas handled, and left a NOTE documenting both deferred features. No behaviour change for any real tier.2. Route service reloads through the orchestrator
The api container has no docker socket — only the orchestrator mounts
/var/run/docker.sock. So api code callingengine.ExecuteActions(which shells out todocker exec/kill/restart) loggedrspamd reload failed — exec "docker" not found in $PATHafter DKIM/spam-config updates. Benign today (rspamd hot-reloads its maps natively), but the reload should go through the component that owns docker.POST /internal/reloadendpoint on the orchestrator runsExecuteActionson the caller's behalf.Client.Reloadmethod;doJSONrefactored to share a body-aware helper.s.reloadServices, which falls back to direct execution when no orchestrator client is configured (host/CLI contexts, unit tests).Testing
On sa1001 (Go 1.26.4):
go build ./...— cleango test ./internal/validonx/ ./internal/orchestrator/ ./internal/api/— all passgo vet -tags integration ./internal/...— integration-tagged files compile clean