feat(sdk): add out of the box controls - part 1 - #246
feat(sdk): add out of the box controls - part 1#246namrataghadi-galileo wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| template: OutOfBoxControlTemplate, | ||
| ) -> str: | ||
| control_service = ControlService(session) | ||
| if await control_service.active_control_name_exists(template.name, namespace_key=namespace_key): |
There was a problem hiding this comment.
Using an active, mutable name as the seed identity resurrects deleted controls and duplicates renamed ones on the next standalone startup. Could we persist an immutable seed/source ID plus an explicit opt-out tombstone, and cover delete/rename followed by reseeding?
There was a problem hiding this comment.
Implemented. Out-of-box controls now persist an immutable, namespace-scoped source_id, and deleting a seeded control records an explicit opt-out tombstone. Reseeding resolves by source ID, so renamed controls are preserved and deleted controls are not resurrected. Added regression coverage for both rename → reseed and delete → reseed scenarios.
| logger.info(f"Evaluator discovery complete. Available evaluators: {available}") | ||
|
|
||
| try: | ||
| seed_result = await seed_out_of_box_controls( |
There was a problem hiding this comment.
The try/except is fail-open only after this await returns. A database lock wait can hold startup before lifespan yields, especially because statement timeouts may be disabled. Please bound the whole bootstrap, or set a local lock timeout and retry later.
There was a problem hiding this comment.
Addressed. The full OOTB bootstrap await is now bounded by a configurable timeout (10 seconds by default). If it blocks, the operation is cancelled and startup continues fail-open with a warning. Added lifespan coverage using a seed coroutine that never returns.
| def __post_init__(self) -> None: | ||
| object.__setattr__(self, "source_id", _SLUG_NAME_ADAPTER.validate_python(self.source_id)) | ||
| object.__setattr__(self, "name", _SLUG_NAME_ADAPTER.validate_python(self.name)) | ||
| if not self.required_evaluators: |
There was a problem hiding this comment.
[P2] Do not let explicit requirements replace condition dependencies
When required_evaluators is non-empty, this skips derivation from the control leaves, so a regex control with an extra Luna requirement can seed on a pod that has Luna but not regex. Always union explicit requirements with evaluator names derived from the condition, and add a mixed-requirement test.
| "controls", | ||
| sa.Column("seed_opted_out_at", sa.DateTime(timezone=True), nullable=True), | ||
| ) | ||
| op.create_index( |
There was a problem hiding this comment.
[P2] Build the seed index without blocking writes
This emits a transactional CREATE UNIQUE INDEX, which holds a write-blocking table lock for the scan; the preceding clone-lineage migration already uses CREATE INDEX CONCURRENTLY for this rollout pattern. Use the same autocommit/concurrent approach for upgrade and downgrade.
There was a problem hiding this comment.
[P2] Make the concurrent migration retry-safe
Entering autocommit_block() commits both unguarded ADD COLUMN statements before the index build and revision stamp. If the concurrent build or stamp fails, rerunning stops on DuplicateColumn; a failed concurrent build can also leave an invalid index that IF NOT EXISTS skips by name. Split the column and index changes into retry-safe revisions and recreate any invalid existing index.
Summary
Scope
agent_control_server.bootstrapmodule, startup seeding hook, and bootstrap tests.Risk and Rollout
seed_out_of_box_controlsand/or revert the bootstrap module and related tests.Testing
make check(not run becauseuvdependency resolution hit private index 401s).venvChecklist
.mdfile