Skip to content

[SG-4917] feat(infracost): name the cost being measured and what it covers - #362

Merged
refeed merged 1 commit into
mainfrom
feat/infracost-message-context
Sep 4, 2026
Merged

refeed merged 1 commit into
mainfrom
feat/infracost-message-context

Conversation

@refeed

@refeed refeed commented Sep 4, 2026

Copy link
Copy Markdown
Member

What

A cost message was the comparison and nothing else:

1. FAILED: `300.1` is not less than or equal to `20`
2. FAILED: `100.1` is not less than or equal to `-1`

It says neither which cost was measured nor what it covered — a monthly and an hourly figure of the same size read identically, and you have to go open the policy to tell which one you are looking at. Now:

1. FAILED: [all resources (2 resources)] total_monthly_cost: `300.1` is not less than or equal to `20`
2. FAILED: [aws_eks_cluster, aws_s3_bucket (1 resource)] total_monthly_cost: `100.1` is not less than or equal to `-1`

Same provider context mechanism terraform_plan uses, so the detail is also structured:

{
  "operation_type": "total_monthly_cost",
  "label": "aws_s3_bucket",
  "attribute": "total_monthly_cost",
  "resource_type": ["aws_s3_bucket"],
  "matched_resources": 1,
  "qualifier": "1 resource",
  "currency": "USD"
}

The resource count is there for a specific trap

resource_type is matched against the infracost breakdown, so a typo produces a cost of 0 — and LessThan 20 passes, while measuring nothing at all:

before:  1. PASSED: `0` is less than `20`
after:   1. PASSED: [aws_instances (0 resources)] total_monthly_cost: `0` is less than `20`

The verdict is deliberately unchanged — a policy that passed still passes, since changing that would turn existing green pipelines red. But the reason is now visible instead of a green check on an empty measurement. test_a_resource_type_matching_nothing_says_so_instead_of_reporting_a_bare_zero pins both halves of that.

Also in here

  • format_context_prefix gained a qualifier key, rendering in the same parenthetical as action, for providers that have no notion of a planned action. terraform_plan keeps using action; nothing about its output changes.
  • The core's non-ProviderError error branch (added in 1.1.0, core.py:118) was the one message path still not getting the context prefix. Now it does, so [all resources] total_monthly_cost: 'projects not found in input_data' names the evaluator it belongs to.
  • _is_every_resource() collects the three spellings of "all resources" (None, "*", ["*"]) that provide already accepted, so the message agrees with what was actually measured on every path.

Verification

  • pytest: 787 passed. The 11 failures are pre-existing and need a terraform binary on PATH.
  • 9 new tests in tests/providers/infracost/test_infracost_message_context.py, including one asserting the two cost walks still return the same totals (they now return (total, matched_count)).
  • black --check clean under both 25.9.0 and 26.5.1, and this branch adds zero files to the set main already flags under the newer one.
  • pydocstyle clean.
  • E2E on QA to follow — the terraform step template runs infracost, so a real run in demo-org exercises this.

Two things found while doing this, not fixed here

  • tests/providers/infracost/tests_infracost.py is not collected and is vacuous. The filename is tests_ rather than test_, so pytest skips it. Its 6 tests would pass regardless: they pass costType instead of operation_type, so provide raises KeyError, returns {"value": None}, and every assertion is None != 0. Left alone — renaming it would surface 6 failures unrelated to this PR.
  • The new test file is test_infracost_message_context.py rather than test_message_context.py because pytest cannot collect two same-named test modules without __init__.py in the test dirs.

Related: SG-4917, tirith#243

@notion-workspace

Copy link
Copy Markdown

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.72727% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/tirith/providers/infracost/handler.py 97.36% 0 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
src/tirith/core/core.py 85.77% <100.00%> (+0.26%) ⬆️
src/tirith/providers/common.py 98.48% <100.00%> (ø)
src/tirith/providers/infracost/handler.py 86.25% <97.36%> (+16.43%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

A cost message was the comparison and nothing else:

    `300.1` is not less than or equal to `20`

which says neither which cost was measured nor what it covered. A monthly and
an hourly figure of the same size read identically, and the reader has to go
and open the policy to find out which one they are looking at. Now:

    [all resources (2 resources)] total_monthly_cost: `300.1` is not less than or equal to `20`
    [aws_eks_cluster, aws_s3_bucket (1 resource)] total_monthly_cost: `100.1` is not less than or equal to `-1`

This uses the same provider `context` mechanism terraform_plan uses, so it is
also carried as structured fields — `operation_type`, `resource_type`,
`matched_resources`, `currency`.

The resource count is there for a specific trap. `resource_type` is matched
against the breakdown, so a typo produces a cost of 0, and `LessThan 20` passes
while measuring nothing at all. The verdict is unchanged — a policy that passed
still passes — but `[aws_instances (0 resources)]` makes the reason visible
rather than leaving a green check on an empty measurement.

Two smaller things come with it: `format_context_prefix` gained a `qualifier`
key, which renders in the same parenthetical as `action` for providers that have
no planned action, and the core's non-`ProviderError` error branch now gets the
context prefix like every other message.
@refeed
refeed force-pushed the feat/infracost-message-context branch from 8110b6f to a8fb5fb Compare September 4, 2026 15:24
@sonarqubecloud

sonarqubecloud Bot commented Sep 4, 2026

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@refeed
refeed merged commit 0ff8751 into main Sep 4, 2026
20 of 21 checks passed
@refeed
refeed deleted the feat/infracost-message-context branch September 4, 2026 23:09
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.

2 participants