feat: provision mapping rules from a directory at startup#6544
Conversation
|
|
manota01
force-pushed
the
feat/4487-provision-mapping-rules
branch
from
b1b864f to
560006f
Compare
There was a problem hiding this comment.
Pull request overview
Adds GitOps-style provisioning for Mapping Rules by loading YAML manifests from a directory at backend startup (via KEEP_MAPPINGS_DIRECTORY), aligning mappings with the existing provisioning flows for workflows/providers/dedup rules.
Changes:
- Add provisioning metadata to
MappingRule(is_provisioned,provisioned_file) plus an Alembic migration. - Introduce
provision_mapping_rules_from_env()to upsert mapping rules from YAML manifests and deprovision removed manifests. - Wire mapping provisioning into backend startup and add a dedicated test suite + fixture manifests.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
keep/api/models/db/mapping.py |
Adds provisioning-tracking columns to the MappingRule model. |
keep/api/models/db/migrations/versions/2026-05-28-16-50_67ff7efffed4.py |
Creates DB columns for mapping rule provisioning metadata. |
keep/api/bl/mapping_rules_provisioning.py |
Implements directory-based mapping rule provisioning + deprovisioning. |
keep/api/config.py |
Calls mapping provisioning during provision_resources() startup flow. |
tests/test_mapping_rules_provisioning.py |
Adds coverage for create/update/adopt/deprovision behaviors and error handling. |
tests/provision/mapping_rules_1/* |
Fixture manifests for single-rule provisioning scenarios. |
tests/provision/mapping_rules_2/* |
Fixture manifests for multi-rule provisioning scenarios. |
tests/provision/mapping_rules_invalid/* |
Fixture manifests for partial-failure provisioning scenarios. |
tests/provision/mapping_rules_empty/.gitkeep |
Fixture directory for “empty dir deprovisions existing” behavior. |
.gitignore |
Ensures mapping provisioning fixtures remain tracked under tests/provision/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _provision_one(session, tenant_id, path) | ||
| except Exception as exc: | ||
| logger.error( | ||
| "Failed to provision mapping rule from %s", | ||
| path, | ||
| extra={"exception": exc}, | ||
| ) | ||
| session.rollback() |
There was a problem hiding this comment.
Addressed in an earlier fix on this branch — the loop now commits per-manifest (current L105), so a late failure only rolls back its own work, not previously-applied manifests.
| def _collect_manifest_paths(mappings_dir: str) -> list[str]: | ||
| """Return sorted absolute paths of YAML manifests in the directory.""" | ||
| paths = [] | ||
| for filename in sorted(os.listdir(mappings_dir)): | ||
| if filename.endswith((".yaml", ".yml")): | ||
| paths.append(os.path.join(mappings_dir, filename)) | ||
| else: | ||
| logger.info("Skipping non-YAML file %s in mappings directory", filename) | ||
| return paths |
There was a problem hiding this comment.
Fixed in 78e74c3 — _collect_manifest_paths now normalizes via os.path.abspath(), and _provision_one stores that same path on provisioned_file. Set-membership comparison stays stable across cwd / env-var-form changes between runs. Added test_provisioned_file_is_absolute_and_stable_across_dir_form_changes to cover the relative-then-absolute case.
| logger.error( | ||
| "Failed to provision mapping rule from %s", | ||
| path, | ||
| extra={"exception": exc}, |
There was a problem hiding this comment.
Fixed in 78e74c3 — switched to logger.exception(...) so the traceback is captured. logger.exception is the dominant pattern across keep/api/bl/* (enrichments, incidents, maintenance_windows, etc.); aligning with that.
| prefix_to_remove: Optional[str] = Field(max_length=255) | ||
| # Provisioning fields (set when the rule is loaded from KEEP_MAPPINGS_DIRECTORY at startup) | ||
| is_provisioned: bool = Field(default=False) | ||
| provisioned_file: Optional[str] = Field(max_length=255, default=None) |
There was a problem hiding this comment.
Addressed in an earlier fix on this branch — current model is provisioned_file: Optional[str] = Field(default=None) with no max_length, matching the migration's AutoString() (no length). Mirrors Workflow.provisioned_file since GitOps mount paths can exceed 255 chars.
| assert len(rules) == 1 | ||
| assert rules[0].name == "valid-mapping" | ||
|
|
||
|
|
There was a problem hiding this comment.
Addressed in an earlier fix — the invalid fixture is named zzz-bad-missing-rows.yaml (paired with good.yaml), so under sorted(os.listdir(...)) the invalid manifest is processed AFTER the valid one. The test does exercise the late-failure ordering you described.
manota01
force-pushed
the
feat/4487-provision-mapping-rules
branch
2 times, most recently
from
aae4abf to
3fb12bc
Compare
manota01
changed the title
Adds KEEP_MAPPINGS_DIRECTORY support, mirroring the existing
KEEP_WORKFLOWS_DIRECTORY / KEEP_PROVIDERS_DIRECTORY / KEEP_DEDUPLICATION_RULES
provisioning. Closes the last GitOps gap — mapping rules can now be
maintained as YAML files in a repo and loaded by Keep on every backend start.
Each YAML manifest in the directory describes one mapping rule:
name: example-prometheus-mapping
description: optional
priority: 0
type: csv
matchers:
- [namespace]
rows:
- { namespace: monitoring, team: platform }
Behavior on startup (mirrors provision_workflows + provision_deduplication_rules):
- For each .yaml in KEEP_MAPPINGS_DIRECTORY: upsert by `name`. An existing
rule with the same name (whether UI-created or previously provisioned)
is adopted: is_provisioned=True, provisioned_file=<path>, contents
overwritten from the manifest.
- For DB rows with is_provisioned=True whose provisioned_file no longer
exists or is outside the directory: delete (deprovision).
- UI-created mapping rules (is_provisioned=False) whose name does not
appear in any manifest are untouched.
Changes:
- Add `is_provisioned` and `provisioned_file` columns to MappingRule
(with alembic migration)
- New module keep/api/bl/mapping_rules_provisioning.py implementing
provision_mapping_rules_from_env
- Wire into provision_resources() in keep/api/config.py after
provision_deduplication_rules_from_env
- Tests in tests/test_mapping_rules_provisioning.py covering: create,
multiple-manifest, idempotency, adoption of UI rules, update of
provisioned rules, deprovision on file disappear, deprovision on
env unset, leave-unrelated-UI-rules-untouched, invalid dir raises,
malformed-manifest-doesn't-break-others, no-op when nothing to do,
empty-dir-deprovisions
Closes keephq#4487
DELETE /mapping/{id} and PUT /mapping/{id} now return 409 when the
target rule has is_provisioned=True, mirroring the dedupe pattern at
alert_deduplicator.py:555,602.
Without this, a user can mutate or delete a GitOps-managed rule via
the UI/API; the next backend restart silently re-applies (or for
delete, the rule disappears until restart). The manifest in
KEEP_MAPPINGS_DIRECTORY must remain the source of truth.
Adds two route-level tests that seed a provisioned rule directly and
assert the guards short-circuit before the underlying mutation.
Two issues caught in Copilot review:
1. _collect_manifest_paths returned os.path.join(dir, filename) where dir
could be relative. Between two runs (cwd change, env var switched from
./foo to /abs/foo) the set-membership comparison against stored
provisioned_file would string-fail and spuriously deprovision a still-
present rule. Normalize via os.path.abspath up-front so both collected
and stored paths use the same form.
2. Per-manifest failure was logged with logger.error(...,
extra={"exception": exc}) which drops the traceback. Switch to
logger.exception(...) — the dominant pattern across keep/api/bl/* —
so failures are debuggable.
Adds test_provisioned_file_is_absolute_and_stable_across_dir_form_changes
asserting (a) stored provisioned_file is absolute and (b) switching the
env var from relative to absolute form between runs does not deprovision.
manota01
force-pushed
the
feat/4487-provision-mapping-rules
branch
from
78e74c3 to
80de72e
Compare
dosubot
Bot
added
size:L
|
💪 Fantastic work @manota01! Your very first PR to keep has been merged! 🎉🥳 You've just taken your first step into open-source, and we couldn't be happier to have you onboard. 🙌 For any support, feel free to reach out on the community: https://slack.keephq.dev. Happy coding! 👩💻👨💻 |
Closes #4487.
What
Adds
KEEP_MAPPINGS_DIRECTORYsupport so mapping rules can be GitOps-managed (loaded from YAML files on every backend start), mirroring the existingKEEP_WORKFLOWS_DIRECTORY/KEEP_PROVIDERS_DIRECTORY/KEEP_DEDUPLICATION_RULESpatterns. Closes the last config gap that required UI-only management.Manifest format:
Behavior
Mirrors
provision_workflowsandprovision_deduplication_rules:.yaml/.ymlinKEEP_MAPPINGS_DIRECTORY: upsert byname. An existing rule with the same name (whether UI-created or previously provisioned) is adopted:is_provisioned=True,provisioned_file=<path>, contents overwritten from the manifest. Fields not inMappingRuleDtoIn(disabled,override,condition) are reset to model defaults on adoption — the manifest is the source of truth. DB id is preserved across re-runs.is_provisioned=Truewhoseprovisioned_fileno longer exists or is outside the directory: delete (deprovision).is_provisioned=False) whose name does not appear in any manifest are untouched.KEEP_MAPPINGS_DIRECTORYis unset and any provisioned rules exist in DB → all deprovisioned.FileNotFoundError.Changes
keep/api/models/db/mapping.pyis_provisioned: bool+provisioned_file: Optional[str]toMappingRule(nomax_lengthon the path — real GitOps mount paths can exceed 255 chars, mirrorsWorkflow.provisioned_file)keep/api/models/db/migrations/versions/2026-05-28-16-50_67ff7efffed4.pybatch_alter_tablefor SQLite compatibility)keep/api/bl/mapping_rules_provisioning.pyprovision_mapping_rules_from_env. Validates each manifest with the existingMappingRuleDtoInDTO (no new validator).keep/api/config.pyprovision_mapping_rules_from_env(SINGLE_TENANT_UUID)intoprovision_resources()afterprovision_deduplication_rules_from_envtests/test_mapping_rules_provisioning.pydb_sessionfixture pattern fromtests/test_workflowstore.pytests/provision/mapping_rules_{1,2,invalid,empty,same_name,with_noise}/...example-prometheus-mappingetc.)docs/deployment/provision/mapping.mdxdocs/deployment/provision/overview.mdxKEEP_MAPPINGS_DIRECTORYrow to the env-var table + a 4th list item linking to the new page.gitignore!tests/provision/mapping_rules*(parallel to existing!tests/provision/workflows*exception)Test coverage
Design choices worth a maintainer sanity-check
These are the bits where I made a call without explicit guidance — happy to switch:
rows(vs. metadata YAML + sibling CSV). Reasoning: file-per-mapping is the simplest GitOps shape, mirrorsKEEP_WORKFLOWS_DIRECTORYergonomics, and validation reuses the existingMappingRuleDtoIn(no new validator).is_provisioned+provisioned_filecolumns (workflow-style) vs. justis_provisioned(dedupe-style). Theprovisioned_fileenables clean "manifest file disappeared → deprovision" detection, which dedupe doesn't need because dedupe rules live nested inKEEP_PROVIDERSenv (no file-tracking).KEEP_MAPPINGSsingle-env-var mode (workflows have bothKEEP_WORKFLOWS_DIRECTORYandKEEP_WORKFLOW). Skipped because the directory mode covers every GitOps use case; a single inline mapping in an env var seems vanishingly rare. Easy to add later if needed.disabled/override/conditionon adoption (manifest is source of truth) rather than carrying UI state over. A UI rule that was disabled via the UI will be re-enabled when adopted. Documented inmapping.mdx. Could carry over instead — would require new fields onMappingRuleDtoIn.Out of scope
provisionedindicator to the Keep UI for mapping rules (workflows show this; mappings could too). Separate UI concern.Context
Production users of Keep adopting GitOps for providers, workflows, and team-routing configs have hit the same gap that motivated #4487 — mapping rules were the one config type that still required manual UI upload. Filing this so the same "I forgot to upload after merge" failure mode is structurally impossible going forward.