Skip to content

fix(infracost): sum the cost of every project, not only the first - #418

Open
MahediBuilds wants to merge 1 commit into
StackGuardian:mainfrom
MahediBuilds:fix/infracost-sum-all-projects
Open

MahediBuilds wants to merge 1 commit into
StackGuardian:mainfrom
MahediBuilds:fix/infracost-sum-all-projects

Conversation

@MahediBuilds

Copy link
Copy Markdown

Pull Request Template

Description

  • What changes are being made?

    • The infracost provider now sums projects[].breakdown.resources[] across every project. The return in __get_all_costs and __get_resources_costs was inside the for project in input_data["projects"] loop, so it returned after the first project.
  • Why are these changes necessary?

    • infracost breakdown writes one project per Terraform directory, so a cost limit is only as good as the order of the projects. With a $10 project followed by a $5,000 one, this policy passed on main:

      {"operation_type": "total_monthly_cost", "resource_type": ["*"]}
      {"type": "LessThanEqualTo", "value": 1000}
      [all resources (1 resource)] total_monthly_cost: `10.0` is less than equal to `1000`
      

      Filtering on a resource_type that only exists in a later project reported 0 resources and a cost of 0, which also passes a LessThan limit.

  • Which issues or tickets does this PR close or relate to?

    • Related issue: none filed. documentation/docs/tirith-providers/infracost.md described the behaviour ("only the first project in the projects array is summed"); that note is replaced.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Other (please specify):

Checklist

  • Code has been linted.
  • Documentation has been updated (if needed).
  • Tests have been added or updated (if needed).
  • Any breaking changes have been communicated to the team.

Screenshots or Recordings (if applicable)

N/A

Additional Information

Testing. tests/providers/infracost/test_infracost_multiple_projects.py adds three tests; all three fail on main and pass with this change.

$ pytest tests/providers/infracost -q
18 passed

$ pytest tests/ -q
24 failed, 988 passed

The 24 failures are the same 24 that fail on main on my machine (Windows): test_direct_references, test_discover, test_policyfiles, test_ui_cli, test_archive and one in test_policy_parameterization, all on temp-dir or path-separator handling. None touch infracost. black --check with the pinned 25.1.0 is clean on both changed Python files.

Docs. documentation/scripts/generate-llms-full.py fails on Windows (it compares backslash paths against ORDER), so I applied the same one-line change to static/docs/.../infracost-provider.md and static/llms-full.txt by hand. Re-running the script on Linux should produce no diff, but worth a check.

Behaviour change to be aware of. Policies whose costs were spread over several projects will now see the real total, so some that passed will fail. That is the fix, but it may deserve a changelog line. Two smaller consequences of the same change:

  • A project after the first that lacks breakdown.resources now raises the existing breakdown/resources not found in one of the project error instead of being ignored.
  • "projects": [] now returns a cost of 0 with 0 resources, where it used to crash with TypeError: cannot unpack non-iterable NoneType object.

The `return` in both cost helpers sat inside the loop over `projects`,
so only the first project's resources were ever summed. Infracost writes
one project per Terraform directory, so a cost limit passed whenever the
expensive directory was not listed first, and a `resource_type` living
in a later project was reported as 0 resources costing 0.

This branch has not been deployed

No deployments
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