Skip to content

Codebase refactoring and cleanup #1

Merged
davincios merged 1 commit intomainfrom
feature/jan-21-experimentation
Jan 23, 2026
Merged

Codebase refactoring and cleanup #1
davincios merged 1 commit intomainfrom
feature/jan-21-experimentation

Conversation

@davincios
Copy link
Copy Markdown
Contributor

No description provided.

@davincios davincios merged commit 75a6ec7 into main Jan 23, 2026
@davincios davincios deleted the feature/jan-21-experimentation branch January 23, 2026 14:52
VaibhavUpreti pushed a commit that referenced this pull request Mar 13, 2026
devankitjuneja added a commit to devankitjuneja/opensre that referenced this pull request Apr 13, 2026
mazenessam77 pushed a commit to mazenessam77/opensre that referenced this pull request May 5, 2026
Adds six targeted tests that isolate each of the gaps the reviewer
flagged so any regression in one stage fails its own test rather than
bleeding into the end-to-end pipeline test:

Gap Tracer-Cloud#1 — load_env_integrations:
  test_load_env_integrations_skips_rds_when_db_id_missing

Gap Tracer-Cloud#2 — _classify_service_instance:
  test_classify_service_instance_rds_remote_store_returns_flat_shape
  test_classify_service_instance_rds_skips_when_db_id_missing

Gap Tracer-Cloud#3 — detect_sources:
  test_detect_sources_propagates_rds_into_sources
  test_detect_sources_skips_rds_when_not_in_resolved_integrations
  test_detect_sources_skips_rds_when_db_id_blank

The remote-store classifier test asserts the absence of a "credentials"
key in the returned dict, which is the exact failure mode the reviewer
called out (generic fallback nests fields and silently breaks
rds_is_available).

25 RDS tests pass; mypy, lint, format clean.
muddlebee pushed a commit that referenced this pull request May 5, 2026
…1287)

* feat(rds): add RDS integration with describe instance and events tools

- Add app/integrations/rds.py for source detection and param extraction
- Add RDSDescribeInstanceTool for instance status and configuration
- Add RDSEventsTool for failover, maintenance, and parameter events
- Use shared aws_sdk_client read-only allowlist for safety
- Add unit tests for integration helpers and both tools
- Add "rds" to EvidenceSource literal in app/types/evidence.py

Closes #125

* fix(rds): honor RDS_REGION env var in both config and param extraction

Greptile review caught two related issues in app/integrations/rds.py:

- rds_config_from_env: env_str("AWS_REGION", DEFAULT_RDS_REGION) always
  returned a non-empty string, making the RDS_REGION fallback unreachable
  (P1 dead code).
- rds_extract_params: only consulted AWS_REGION, never RDS_REGION,
  diverging from the config path (P2 inconsistency).

Both functions now resolve region as: source-supplied > AWS_REGION >
RDS_REGION > DEFAULT_RDS_REGION. Drops the now-unused os import.

Adds three tests covering the previously-uncovered fallback paths:
RDS_REGION-only in extract, RDS_REGION-only in config-from-env, and
default when neither env var is set.

* chore(rds): apply ruff lint and format fixes

- ruff check --fix reorganized imports in tests/tools/test_rds_tools.py
- ruff format reformatted app/integrations/rds.py

* docs(rds): add RDS integration documentation

- New docs/rds.mdx covering setup, env vars, IAM permissions, the two
  tools, use cases, and troubleshooting. References docs/aws.mdx for
  shared AWS credential setup rather than duplicating it.
- Register the page under "Data and workflow systems" in docs/docs.json.

Documents the RDS_REGION fallback semantics so users understand the
region resolution order: source dict > AWS_REGION > RDS_REGION > default.

* fix(rds): sanitize AWS SDK errors and warn on multiple instances

- Replace raw AWS error forwarding with generic messages in both
  RDSDescribeInstanceTool and RDSEventsTool; full errors are logged
  server-side only to prevent leaking account IDs, ARNs, or regions.
- Add warning log when DescribeDBInstances returns more than one
  instance, since only the first is used.
- Update test assertions to match the new sanitized error messages.

* test(rds): add coverage for multiple-instances warning path

Verifies that when DescribeDBInstances returns >1 record, the tool
uses the first instance and emits a warning log.

* fix(rds): wire RDS into env discovery, classification, and source detection

Reviewer found three real wiring gaps that meant the documented setup
path never reached the planner. The integration helpers were correct but
nothing else in the pipeline knew about RDS:

- app/integrations/_catalog_impl.py::load_env_integrations now
  registers an "rds" record when rds_config_from_env returns a
  configured instance. Previously env-set RDS_DB_INSTANCE_IDENTIFIER
  was a no-op.
- app/integrations/_catalog_impl.py::_classify_service_instance gains
  an "rds" branch that returns a flat shape ({db_instance_identifier,
  region, integration_id}) instead of falling through to the generic
  handler that nests fields under "credentials" — which made
  rds_is_available always return False for store-backed configs.
- app/nodes/plan_actions/detect_sources.py now copies the resolved
  rds integration into sources["rds"], mirroring the rabbitmq/mariadb
  blocks. Without this, even a correctly-resolved integration would
  not be visible to rds_is_available.

Adds tests/integrations/test_rds.py::test_rds_env_discovery_to_sources_pipeline
which walks the full env -> load_env_integrations -> _classify_service_instance
-> detect_sources path end to end and asserts each stage produces the
expected shape, so this regression cannot reappear silently.

* test(rds): add focused regression coverage for the three discovery gaps

Adds six targeted tests that isolate each of the gaps the reviewer
flagged so any regression in one stage fails its own test rather than
bleeding into the end-to-end pipeline test:

Gap #1 — load_env_integrations:
  test_load_env_integrations_skips_rds_when_db_id_missing

Gap #2 — _classify_service_instance:
  test_classify_service_instance_rds_remote_store_returns_flat_shape
  test_classify_service_instance_rds_skips_when_db_id_missing

Gap #3 — detect_sources:
  test_detect_sources_propagates_rds_into_sources
  test_detect_sources_skips_rds_when_not_in_resolved_integrations
  test_detect_sources_skips_rds_when_db_id_blank

The remote-store classifier test asserts the absence of a "credentials"
key in the returned dict, which is the exact failure mode the reviewer
called out (generic fallback nests fields and silently breaks
rds_is_available).

25 RDS tests pass; mypy, lint, format clean.

---------

Co-authored-by: code wizard <[email protected]>
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