refactor(arch): unify minion-id validator, drop inline SSH creds, add hypertable compression, beat HA notes (#752 #748 #753 #754)#862
Merged
Conversation
… hypertable compression, beat HA notes (#752 #748 #753 #754) #752 (ARC-8): Extract canonical MINION_ID_RE = ^[a-zA-Z0-9._-]{1,128}$ into fleet_platform/core/validators.py and wire the shared validate_minion_id() function into all input schemas (BootstrapRequest, NodeRegisterRequest, NodeCreateRequest, GrainIngestPayload, ExecutionIngestPayload, ProcessStatsIngestPayload, ImportRow). Closes #752. #748 (ARC-4): Add migration 058 to null ssh_username/ssh_password_enc/ ssh_key_enc/ssh_auth_mode on nodes/groups where credential_id IS NOT NULL (i.e. rows already migrated by migration 055 into the first-class Credential store). Writes were already stopped in #725 via ssh_credential_link.py. Deferred: dropping the inline columns and removing the fallback read path in services/ssh_credential_link.py:owner_secret_flags (out of file-ownership scope; requires confirming all read-paths gone before a DROP COLUMN). Partial. #753 (ARC-9): Add migration 059 with add_compression_policy('node_facts', INTERVAL '7 days'), compress_segmentby='node_id'. No-op on vanilla Postgres (guarded by timescale_enabled()), mirrors the pattern from migrations 024/047. Closes #753. #754 (ARC-10): Enable beat_scheduler='redbeat.RedBeatScheduler' in celery_app.conf (celery-redbeat already declared in pyproject.toml). With two beat replicas, the active leader holds a Redis lock; a standby takes over within one lock-timeout if the primary crashes. Deploy note inlined in config comment. Code change: beat_scheduler set. Doc change: HA replica instructions and Kubernetes guidance in the conf block comment. Closes #754. New tests: 34 in tests/unit/test_validators_752.py (all pass).
github-actions
Bot
added
breaking-change
Coverage Report✅ 85.5% on Gate passed |
…752-748-753-754 # Conflicts: # VERSION # frontend/package.json
Coverage Report✅ 85.9% on Gate passed |
hellodk
pushed a commit
that referenced
this pull request
Jun 23, 2026
…anges - Re-chain must_change_password migration 058 -> 060 (down_revision 059) to resolve collision with #862's 058/059 now on master. - Update test_metrics.py: /metrics now requires a bearer credential (#763), so endpoint tests authenticate via a patched METRICS_TOKEN. - Update test_salt_allowlist.py: cmd.run is intentionally excluded from the default Salt allowlist (RCE defense-in-depth, #758); assert its absence.
hellodk
pushed a commit
that referenced
this pull request
Jun 24, 2026
… reject Four integration regressions from the #861/#862/#863 merge train left master red. Fixes: - Rate-limit headers (#801): #863 rewrote the limiter without re-emitting X-RateLimit-* headers. headers_enabled on the SlowAPI Limiter can't be used (the decorator then tries to inject into dict-returning endpoints and raises), and SlowAPIMiddleware exempts decorated routes. Add a pure-ASGI RateLimitHeadersMiddleware that injects the headers from scope-state view_rate_limit onto every HTTP response (incl. 401 login) and leaves WebSocket scopes untouched. - Import validate (#862): ImportRow carried a hard field_validator on minion_id, so the dry-run validate endpoint 500'd on intentionally-bad rows instead of reporting them. Move format checking into node_import.validate_row (soft, status="invalid") and re-check defensively on the commit path so a bad minion_id can still never be persisted. - Agent reject (#795): the endpoint correctly commits status="rejected" in its own session; the test's module-scoped db_session (expire_on_commit=False) was asserting against a stale identity-mapped instance. Refresh before asserting. Verified against a Postgres-backed fleet_test: the 4 previously-red tests plus their full files (32) and the related rate-limit/import unit tests (45) pass.
5 tasks
hellodk
added a commit
that referenced
this pull request
Jun 24, 2026
… reject (#875) * fix(api): unbreak master — rate-limit headers, import validate, agent reject Four integration regressions from the #861/#862/#863 merge train left master red. Fixes: - Rate-limit headers (#801): #863 rewrote the limiter without re-emitting X-RateLimit-* headers. headers_enabled on the SlowAPI Limiter can't be used (the decorator then tries to inject into dict-returning endpoints and raises), and SlowAPIMiddleware exempts decorated routes. Add a pure-ASGI RateLimitHeadersMiddleware that injects the headers from scope-state view_rate_limit onto every HTTP response (incl. 401 login) and leaves WebSocket scopes untouched. - Import validate (#862): ImportRow carried a hard field_validator on minion_id, so the dry-run validate endpoint 500'd on intentionally-bad rows instead of reporting them. Move format checking into node_import.validate_row (soft, status="invalid") and re-check defensively on the commit path so a bad minion_id can still never be persisted. - Agent reject (#795): the endpoint correctly commits status="rejected" in its own session; the test's module-scoped db_session (expire_on_commit=False) was asserting against a stale identity-mapped instance. Refresh before asserting. Verified against a Postgres-backed fleet_test: the 4 previously-red tests plus their full files (32) and the related rate-limit/import unit tests (45) pass. * fix(test): stop integration fixtures rebinding the limiter The behavioural rate-limit tests failed in the full suite (429 too early / "state shared across IP keys") while passing in isolation. Root cause: four integration app fixtures (conftest.app_with_test_db and the local fixtures in test_auth_endpoints, test_check_minion_id, test_node_import_api) did `limiter_module.limiter = Limiter(...)` before importing the routes. The @limiter.limit decorators bind to whichever limiter object exists at first route import and can't be rebound, so the throwaway test_limiter from the first such module became the decorators' limiter for the whole session — while the behavioural tests reset the *production* limiter and saw stale 429s. Fix: don't swap. Share the real production limiter and just zero its in-memory storage on setup/teardown so counts don't bleed across modules. This keeps app.state.limiter and the decorator limiter as the same object everywhere. Verified: full integration suite (excl. the Redis-only vnc/webssh ws tests that can't run in the sandbox) is 326 passed / 1 xfailed, including all of test_rate_limit_behavioral_801 in full-suite order. * ci: re-trigger checks Co-authored-by: Cursor <cursoragent@cursor.com> * test(unit): align stale assertions with local-tz dates and ImportRow transport Master went red after #877 (local-timezone date formatting) and the ImportRow soft-validation change merged together: - test_ist_timestamps / test_processes_tab_psutil_613 asserted hardcoded "Asia/Kolkata" / " IST" strings; now assert the dynamic getTimezone() + timeZoneName/formatIST contract introduced by #877 (#796). - test_validators_752 TestImportRow expected ValidationError on bad ids, but ImportRow is the validate dry-run transport and must carry invalid rows; assert it accepts the string while MINION_ID_RE rejects it. * ci(mutation): pin mutmut<3.6.0 to fix crashing results step mutmut 3.6.0 renamed the [tool.mutmut] config keys and shipped a config-loader regression that crashes `mutmut results` with "can only concatenate list (not 'str') to list". Since the mutation.yml "Print mutation results" step has no continue-on-error, this failed the check on every PR touching validator/redaction tests (e.g. this branch's test_validators_752.py edit). Pin <3.6.0 so the existing string-valued config keeps working. * test(unit): ConfirmDialog Escape handling moved to useFocusTrap (#878) #878 extracted Escape-to-close + focus trapping into the shared useFocusTrap hook, so ConfirmDialog.tsx no longer contains the literal "Escape". Assert it delegates to useFocusTrap and that the hook handles Escape, instead of grepping the component for the moved string. --------- Co-authored-by: kri-agent <agent@kri> Co-authored-by: Cursor <cursoragent@cursor.com>
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
1 participant
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.
Summary
fleet_platform/core/validators.pywith canonicalMINION_ID_RE = ^[a-zA-Z0-9._-]{1,128}$; all 7 input schemas (BootstrapRequest,NodeRegisterRequest,NodeCreateRequest,GrainIngestPayload,ExecutionIngestPayload,ProcessStatsIngestPayload,ImportRow) now import and apply the sharedvalidate_minion_id()viafield_validator.ssh_username/ssh_password_enc/ssh_key_enc/ssh_auth_modecolumns onnodes/groupswherecredential_id IS NOT NULL(i.e. already migrated by 055). The write path was already stopped in refactor: retire inline ssh_* write surface (redirect to Credential FK) #725; this completes the data clean-up.timescaledb.compress_segmentby='node_id'+add_compression_policy('node_facts', INTERVAL '7 days'). Guards viatimescale_enabled(), no-op on vanilla Postgres.beat_scheduler='redbeat.RedBeatScheduler'activated incelery_app.conf(celery-redbeat was already inpyproject.toml). Config block includes full HA deploy instructions (two replicas, Kubernetes guidance).Issues
Closes #752
Closes #753
Closes #754
Partial #748 — see deferred items below.
What is deferred on #748
ssh_username,ssh_password_enc,ssh_key_enccolumns onnodes/groups): requires first confirming the fallback read-path inservices/ssh_credential_link.py:owner_secret_flagsis removed; that file is out of this agent's file-ownership scope.owner_secret_flagsfallback read from the resolver/service layer: also out of scope (another agent ownsservices/). Once that path is gone, a final migration canALTER TABLE ... DROP COLUMNsafely.New tests
tests/unit/test_validators_752.py— 34 tests covering the bare validator function and thefield_validatorwiring in every affected schema (parametrised valid/invalid inputs). All 47 tests in the related suite pass.Files changed
fleet_platform/core/validators.pyMINION_ID_RE+validate_minion_id()fleet_platform/schemas/ansible.pyBootstrapRequest.minion_idfleet_platform/schemas/fleet.pyNodeCreateRequest.minion_idfleet_platform/schemas/ingest.pyfleet_platform/schemas/node.pyNodeRegisterRequest.minion_idfleet_platform/schemas/node_import.pyImportRow.minion_idfleet_platform/workers/celery_app.pybeat_scheduler+ HA deploy commentfleet_platform/db/migrations/versions/058_null_inline_ssh_creds.pyfleet_platform/db/migrations/versions/059_node_facts_compression.pytests/unit/test_validators_752.pyMade with Cursor