Skip to content

chore(providers): onboarding prep — unblock Azure/GCP/OCI merges#287

Merged
fgogolli merged 10 commits into
mainfrom
chore/provider-onboarding-prep
Jul 14, 2026
Merged

chore(providers): onboarding prep — unblock Azure/GCP/OCI merges#287
fgogolli merged 10 commits into
mainfrom
chore/provider-onboarding-prep

Conversation

@fgogolli

@fgogolli fgogolli commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Description

Cross-cutting prep to remove AWS-shaped leaks from shared code and finalise the ProviderStrategy contract before Azure (PR #258), GCP (PR #257), and OCI (PR #234) provider PRs land. Blocks the multi-provider merge train.

Rebased onto current main and regrouped into 9 thematic commits (down from the original working history), each single-purpose. Highlights:

Shared-code AWS leak removal:

  • Machine.provider_type now required (dropped default="aws" — silent misclassification for non-AWS machines)
  • Bootstrap no longer hard-wires the AWS validator into ProviderValidationService
  • Application handlers no longer fall back to PROVIDER_TYPE_AWS
  • Dashboard aggregates dynamically from the provider registry instead of a hardcoded AWS API list
  • Template generation no longer inserts a hardcoded aws_default_us-east-1 fallback
  • image_id moved from universally required to AWS-specific validator
  • AWS-shaped region regex moved from NamingConfig into AWS provider config

Domain model neutrality (with AliasChoices back-compat + DeprecationWarning):

  • Template.instance_typeTemplate.machine_type
  • Template.instance_profileTemplate.machine_role
  • Template.provider_data deleted; readers migrated to typed subclasses (AWSTemplate, K8sTemplate)

Provider contract additions:

  • get_resource_id_pattern() classmethod on ProviderStrategy (AWS returns ^i-[a-f0-9]{8,17}$, non-AWS returns None)
  • providers/base/exceptions/: shared ProviderError hierarchy with an is_retryable axis and a safe_to_dict() variant that omits underlying_exception to prevent ARN/DSN/secret leakage in HTTP responses

Note: an earlier revision of this branch also introduced a providers/base/scaffolding/ package (registration/handler-base/example). It had zero production adopters, so it was dropped from this PR to avoid shipping unused code. It will be reintroduced with real AWS + k8s adopters once the registry consolidation lands.

Security hardening:

  • IAM admin authorization now uses admin_arns allowlist (frozenset exact-match) instead of substring bypass; :root unconditional grant now respects the allowlist
  • CORS + trusted_hosts defaults tightened to loopback-only (localhost, 127.0.0.1, ::1, testserver)
  • /info no longer discloses auth strategy to unauthenticated callers
  • SQLQueryBuilder abstract-method regression tests to guard against the runtime TypeError that shipped in an earlier release

Storage backfill:

  • Legacy stored Machine records without provider_type are backfilled to "aws" on read

CLI hygiene:

  • --provider-type registered via a shared parent parser (prevents the argparse regression pattern that caused a large test-failure cascade)

API DTO renames with deprecation:

  • TemplateCreateRequest, TemplateUpdateRequest, CreateTemplateInput, UpdateTemplateInput accept both machine_type (primary) and instance_type (deprecated alias with warning log)

Config hygiene:

  • default_config.json (both packaged + repo sample) restored to loopback-only server.host / cors.origins — the packaged JSON is the lowest-precedence base layer and was overriding the hardened schema defaults back to 0.0.0.0 / ["*"], silently defeating the security tightening.
  • Moved AWS-shaped naming defaults (spot_fleet / ec2_fleet / asg patterns, fleet_types, price_types) out of the shared NamingConfig — no shared reader consumes them.

CI / coverage:

  • Coverage is now uploaded to Codecov exactly once, from a coverage-combine fan-in job that merges every leg's .coverage data first. A single deterministic upload replaces the per-leg uploads that produced mid-run partial-merge flapping, and it auto-scales to any number of provider legs (no hardcoded leg count).
  • k8s_legacy, test files, and thin process entrypoints (__main__.py, run.py, server_daemon.py) excluded from the coverage number so it reflects real production source. Combined coverage: ~63%.

Post-review hardening (adversarial review + CodeQL + CI feedback):

  • Three independent adversarial reviews (code-quality, security, architecture) run on the branch; all CRITICAL + HIGH findings fixed inline:
    • ProviderError gained an is_retryable axis (Transient/Permanent was orthogonal to Config/Auth/Quota) + a safe_to_dict() that omits underlying_exception to prevent ARN/DSN/secret leakage into HTTP responses.
    • IAM :root unconditional admin grant now respects the admin_arns allowlist when one is configured.
  • CodeQL findings fixed: dict-subclass equality state (_StickyOverridesDict.__eq__ with __hash__ = None), trusted_hosts asserted by exact list rather than substring membership, unused module-level logger removed.
  • The k8s test suite surfaced 4 pre-existing test-vs-behaviour mismatches (StatefulSet/Job selective-release now refuse and raise rather than warn-and-scale; extension registration moved to an explicit lifecycle hook) — tests aligned to the current semantics. These were never caught on main because its CI matrix was AWS-only; k8s now runs on every PR via provider discovery.
  • Fixed a live-conftest bug that marked the entire collected session as serial (not just its own subtree), which deselected every k8s test under -m "not serial".
  • Retry-classifier tests now raise a real ApiException(status=404) so the classifier recognises the not-found path, instead of a hand-mocked generic exception.

Type of Change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Code cleanup or refactor
  • CI/CD or build process changes

Breaking-change surface (all backwards-compatible via aliases + deprecation warnings):

  • Template.instance_typeTemplate.machine_type (alias preserved)
  • Template.instance_profileTemplate.machine_role (alias preserved)
  • Template.provider_data field removed; readers migrated to typed subclasses
  • Machine.provider_type now required (no default) — legacy storage records are backfilled

How Has This Been Tested?

  • Unit tests added/updated

@fgogolli fgogolli requested a review from a team as a code owner July 7, 2026 15:47
Comment thread tests/e2e/test_critical_path_e2e.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
Comment thread tests/providers/base/scaffolding/test_handler_base.py Fixed
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Test Results Summary

12 612 tests  +251   12 295 ✅ +251   6m 25s ⏱️ -7s
     9 suites ±  0      317 💤 ±  0 
     9 files   ±  0        0 ❌ ±  0 

Results for commit 42688a0. ± Comparison against base commit 542a7de.

This pull request removes 8 and adds 259 tests. Note that renamed tests count towards both.
tests.providers.aws.mocked.test_template_pipeline ‑ test_hf_aws_field_mapping_has_valid_domain_target[instanceProfile-instance_profile]
tests.providers.k8s.unit.domain.template.test_k8s_template ‑ test_explicit_service_account_wins_over_instance_profile
tests.providers.k8s.unit.domain.template.test_k8s_template ‑ test_no_instance_profile_leaves_service_account_unset
tests.providers.k8s.unit.domain.template.test_k8s_template ‑ test_service_account_falls_back_to_instance_profile
tests.providers.k8s.unit.handlers.test_statefulset_handler ‑ test_release_hosts_selective_non_highest_ordinal_refused
tests.providers.k8s.unit.handlers.test_statefulset_handler ‑ test_release_hosts_selective_unparseable_victim_names_refused
tests.providers.k8s.unit.test_coverage_gaps ‑ test_build_pod_spec_falls_back_to_instance_profile
tests.providers.k8s.unit.test_coverage_gaps ‑ test_job_release_logs_selective_release_not_supported
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildBatchInsert ‑ test_raises_on_empty_list
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildBatchInsert ‑ test_returns_query_and_params_list
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildCount ‑ test_returns_count_star_query
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildCreateTable ‑ test_contains_create_table_statement
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildCreateTable ‑ test_includes_all_columns
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildDelete ‑ test_rejects_invalid_id_column
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildDelete ‑ test_returns_query_and_param_name
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildExists ‑ test_returns_limit_1_query
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildInsert ‑ test_filters_unknown_columns
tests.infrastructure.storage.components.test_sql_query_builder.TestBuildInsert ‑ test_raises_when_no_valid_columns
…

♻️ This comment has been updated with latest results.

Comment thread src/orb/providers/base/scaffolding/registration.py Fixed
@fgogolli fgogolli force-pushed the chore/provider-onboarding-prep branch from b8626e9 to f4fd4dd Compare July 8, 2026 11:11
fgogolli added a commit that referenced this pull request Jul 8, 2026
…287)

The release_hosts provider_data-optional change and the ProviderHandlerBase
scaffolding protocol overlap with PR #287 (provider onboarding prep), which
owns the cross-provider handler contract and rewrites these same handler
files. That work does not belong in this k8s perf/parity PR.

Reverts the handler signature changes to main and removes the orphaned
scaffolding file (which was an incomplete copy of #287's — missing __init__
and siblings). The contract-test adapter is restored to main's
captured-provider_data pattern. The kmock list-semantics fix is unaffected
and retained.
@fgogolli fgogolli force-pushed the chore/provider-onboarding-prep branch from 8368853 to 447140c Compare July 9, 2026 11:13
fgogolli added a commit that referenced this pull request Jul 13, 2026
Rebasing #287 onto main surfaced: inline imports still used the pre-rename
orb.providers.k8s.exceptions.k8s_errors path (main renamed the module to
k8s_exceptions); and CLISpecRegistry test used the old _specs attribute (main
renamed to _store). Correct both so the rebased branch is green.
fgogolli added a commit that referenced this pull request Jul 13, 2026
#287 added scaffolding/exceptions/strategy tests under tests/providers/base/,
but the providers CI leg only runs discovered provider subtrees (aws, k8s via
testconf.mk) and the unit leg runs tests/unit — so tests/providers/base ran in
NO CI leg. Move them into tests/unit/providers/base/ so the unit leg executes
them. Also replace the deprecated asyncio.get_event_loop().run_until_complete()
with asyncio.run() in the handler-base tests — get_event_loop() raises under
Python 3.12 once another test has closed the global loop, which surfaced as
order-dependent failures now that these run in the shared unit leg.
@fgogolli fgogolli force-pushed the chore/provider-onboarding-prep branch from 447140c to bb73c61 Compare July 13, 2026 14:46
@codecov-commenter

codecov-commenter commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.37500% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.27%. Comparing base (542a7de) to head (42688a0).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/orb/domain/template/template_aggregate.py 86.36% 2 Missing and 1 partial ⚠️
src/orb/application/commands/provider_handlers.py 50.00% 1 Missing and 1 partial ⚠️
...pplication/services/template_generation_service.py 0.00% 2 Missing ⚠️
...ders/aws/domain/template/aws_template_aggregate.py 50.00% 1 Missing ⚠️
...ders/aws/infrastructure/launch_template/manager.py 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #287      +/-   ##
==========================================
+ Coverage   62.76%   63.27%   +0.50%     
==========================================
  Files         837      840       +3     
  Lines       56277    56378     +101     
  Branches     6891     6903      +12     
==========================================
+ Hits        35325    35671     +346     
+ Misses      19176    19015     -161     
+ Partials     1776     1692      -84     
Flag Coverage Δ
e2e ?
infrastructure ?
integration ?
providers-aws ?
providers-k8s ?
providers-serial-aws ?
providers-serial-k8s ?
ui-unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/orb/api/models/responses.py 96.49% <100.00%> (-0.03%) ⬇️
src/orb/api/routers/templates.py 100.00% <100.00%> (+1.03%) ⬆️
src/orb/api/server.py 63.81% <100.00%> (+3.89%) ⬆️
src/orb/application/commands/template_handlers.py 95.87% <100.00%> (+8.50%) ⬆️
...lication/services/orchestration/create_template.py 100.00% <ø> (ø)
src/orb/application/services/orchestration/dtos.py 97.70% <100.00%> (+0.01%) ⬆️
...lication/services/orchestration/update_template.py 100.00% <ø> (ø)
src/orb/application/template/commands.py 100.00% <100.00%> (ø)
src/orb/bootstrap/domain_services.py 92.10% <ø> (+3.21%) ⬆️
src/orb/cli/args.py 98.19% <100.00%> (+0.26%) ⬆️
... and 31 more

... and 5 files with indirect coverage changes

Comment thread tests/unit/config/test_server_schema.py Fixed
fgogolli added a commit that referenced this pull request Jul 13, 2026
Rebasing #287 onto main surfaced: inline imports still used the pre-rename
orb.providers.k8s.exceptions.k8s_errors path (main renamed the module to
k8s_exceptions); and CLISpecRegistry test used the old _specs attribute (main
renamed to _store). Correct both so the rebased branch is green.
fgogolli added a commit that referenced this pull request Jul 13, 2026
#287 added scaffolding/exceptions/strategy tests under tests/providers/base/,
but the providers CI leg only runs discovered provider subtrees (aws, k8s via
testconf.mk) and the unit leg runs tests/unit — so tests/providers/base ran in
NO CI leg. Move them into tests/unit/providers/base/ so the unit leg executes
them. Also replace the deprecated asyncio.get_event_loop().run_until_complete()
with asyncio.run() in the handler-base tests — get_event_loop() raises under
Python 3.12 once another test has closed the global loop, which surfaced as
order-dependent failures now that these run in the shared unit leg.
@fgogolli fgogolli force-pushed the chore/provider-onboarding-prep branch 2 times, most recently from 9204bea to 1034b72 Compare July 13, 2026 15:42
fgogolli added 8 commits July 13, 2026 17:21
Remove AWS-specific defaults from shared domain/config; rename fields
to provider-neutral equivalents so non-AWS providers can onboard cleanly.
Accept machine_type alongside deprecated instance_type on Template DTOs;
emit operator-visible warnings for the old field and migrate AWS handlers.
…hardening

Tighten IAM ARN validation to an allowlist, restrict CORS/trusted_hosts
to loopback, harden the /info endpoint, and lock down test assertions.
Add ProviderError base exception and get_resource_id_pattern() classmethod
for provider-scoped ID validation; scaffolding stubs added then removed.
Register --provider-type on the shared parent parser so all sub-commands
inherit it consistently rather than each registering it independently.
Add regression tests covering abstract-method contracts on SQLQueryBuilder
to prevent silent interface drift as the storage layer evolves.
…ixes

Align k8s tests with the refuse-and-raise release semantics and fix stale
import paths and registry names introduced by the earlier rebase.
Finish the instance_profile->machine_role rename in base context flags
that was missed in the earlier AWS handler migration.
@fgogolli fgogolli force-pushed the chore/provider-onboarding-prep branch 2 times, most recently from 821f9de to dc29142 Compare July 14, 2026 08:19
…nts from the number

Consolidate coverage uploads into one combined report with after_n_builds
synchronisation; exclude legacy/entrypoint modules from the coverage metric.
@fgogolli fgogolli force-pushed the chore/provider-onboarding-prep branch 5 times, most recently from ab739df to f61c541 Compare July 14, 2026 08:59
The scorecard workflow already publishes results (publish_results: true);
this surfaces the score in the README badge row.

Badge image: api.securityscorecards.dev; click-through: scorecard.dev viewer.
@fgogolli fgogolli force-pushed the chore/provider-onboarding-prep branch from f61c541 to 42688a0 Compare July 14, 2026 09:02
@canonicalname canonicalname self-requested a review July 14, 2026 09:19
@fgogolli fgogolli requested a review from a team July 14, 2026 09:19
@fgogolli fgogolli merged commit 36e9847 into main Jul 14, 2026
118 checks passed
@fgogolli fgogolli deleted the chore/provider-onboarding-prep branch July 14, 2026 09:19
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.

4 participants