search

cycloidio / terracost

Cloud cost estimation for Terraform in your CLI
MIT License
278 stars 29 forks source link

testing: add handling of unknown providers and empty terraform #55

Closed xlr-8 closed 2 years ago

xlr-8 commented 2 years ago

This PR adds the handling of unknown provider (eg. vmware) and also cases of empty terraform plan/module. That allows to give better output when the cost estimated is '0.0'. Now instead of having '0.0' an error will be returned.

In order to improve the testing, some pricing dump got also injected as part of it.

Related to: https://github.com/cycloidio/terracost/issues/21

xlr-8 commented 2 years ago

I feel like the HCL case is not handled and that this is maybe not the best way to handle the case that we do not have 0.0.

Why? Both cases are tested on the PR though?

There are 2 ways to do it:

  • On the functions of the cost.Plan
  • Initialization of the Plan if the prior and planned are totally empty.

HCL doesn't contain prior and plan costs. Plus as we are speaking of plan, you could have some prior queries, but no new ones.

Or following what you did you could also add the if onto the terraform.ExtractQueriesFromHCL

I'm not sure to follow your point there.

xlr-8 commented 2 years ago

I'm currently having issues with the pipeline to properly test it, so I'll remove the need review label.

xescugc commented 2 years ago

Why? Both cases are tested on the PR though?

IDK why I did only see the NoQuereis on the State and not the HCL case

HCL doesn't contain prior and plan costs. Plus as we are speaking of plan, you could have some prior queries, but no new ones.

What I don't like about your solution is that it's the same logic in 2 places, and the Queries itself are of no "use" until you put them into a State. And both of them use the State to calculate so I would move that logic to the State initialization, so then it's not HCL nor Plan but common place, and also it's where the actual calculations are made so it would make sense to be there.

Also as the query returns s []query you could calculate them from different files and then aggregate all the queries into 1 State which would validate if it's empty or not.

xlr-8 commented 2 years ago

Done, pipeline has been fixed too to work with the dump.

xlr-8 commented 2 years ago

Done

xlr-8 commented 2 years ago

I've updated the Makefile/docker-compose file to ease the testing locally/in the CI