Repository: cycloidio/terracost (go)

Issue:
estimation: Return a list of skipped resources
## Abstract

Currently, only the resources that had been successfully estimated are returned. The user has no way of knowing which resources from the plan were skipped (due to e.g. not being supported).

The skipped resources should be returned alongside the estimated ones.

## Implementation details

This could be accomplished by including a `SkippedResources []string` field (or similar) in the `cost.State` struct, as well as having a helper method on the `cost.Plan` to extract them.

## Suggestions/ideas

Many Terraform resources are inherently free (such as from the `null` or `local` providers) and shouldn't be included as "skipped." One idea could be to have a list of free resources that should never be estimated and that should never be marked as "skipped." The other would be to ignore all unknown providers and mark as skipped only unsupported resources of supported providers.

Pull request:
estimation: expose a list of skipped resources
This PR adds a new `Skipped` field to `cost.Resource`, as well as a new `SkippedAddresses` method to `cost.Plan` that returns a list of resource addresses that were skipped in the estimation process. Closes #12 

Patch:
diff --git a/cost/plan.go b/cost/plan.go
index f31d588..c715827 100644
--- a/cost/plan.go
+++ b/cost/plan.go
@@ -1,6 +1,8 @@
 package cost
 
 import (
+	"sort"
+
 	"github.com/shopspring/decimal"
 )
 
@@ -52,10 +54,40 @@ func (p Plan) ResourceDifferences() []ResourceDiff {
 	return rds
 }
 
+// SkippedAddresses returns the addresses of resources that were excluded from the estimation process.
+// The order of the elements in the slice is undefined and unstable.
+func (p Plan) SkippedAddresses() []string {
+	skippedMap := make(map[string]struct{})
+	if p.Prior != nil {
+		for addr, res := range p.Prior.Resources {
+			if res.Skipped {
+				skippedMap[addr] = struct{}{}
+			}
+		}
+	}
+	if p.Planned != nil {
+		for addr, res := range p.Planned.Resources {
+			if res.Skipped {
+				skippedMap[addr] = struct{}{}
+			}
+		}
+	}
+	skippedList := make([]string, 0, len(skippedMap))
+	for addr := range skippedMap {
+		skippedList = append(skippedList, addr)
+	}
+	sort.Strings(skippedList)
+	return skippedList
+}
+
 // mergeResourceDiffsFromState adds all the resources from the State to the provided ResourceDiff map. Each component
 // of every resource is then placed into an appropriate ComponentDiff field based on the value of the `planned` argument.
 func mergeResourceDiffsFromState(rdmap map[string]ResourceDiff, state *State, planned bool) {
 	for address, res := range state.Resources {
+		if res.Skipped {
+			continue
+		}
+
 		if _, ok := rdmap[address]; !ok {
 			rdmap[address] = ResourceDiff{
 				Address:        address,
diff --git a/cost/resource.go b/cost/resource.go
index 000266f..587cb11 100644
--- a/cost/resource.go
+++ b/cost/resource.go
@@ -8,6 +8,7 @@ import (
 // by the label.
 type Resource struct {
 	Components map[string]Component
+	Skipped    bool
 }
 
 // Cost returns the sum of costs of every Component of this Resource.
diff --git a/cost/state.go b/cost/state.go
index da34602..50a567c 100644
--- a/cost/state.go
+++ b/cost/state.go
@@ -40,6 +40,11 @@ func NewState(ctx context.Context, backend Backend, queries []query.Resource) (*
 	state := &State{Resources: make(map[string]Resource)}
 
 	for _, res := range queries {
+		// Mark the Resource as skipped if there are no valid Components.
+		if len(res.Components) == 0 {
+			state.Resources[res.Address] = Resource{Skipped: true}
+		}
+
 		for _, comp := range res.Components {
 			prods, err := backend.Product().Filter(ctx, comp.ProductFilter)
 			if err != nil {
diff --git a/query/query.go b/query/query.go
index 841c284..bdcc27a 100644
--- a/query/query.go
+++ b/query/query.go
@@ -13,7 +13,8 @@ type Resource struct {
 	// Address uniquely identifies this cloud Resource.
 	Address string
 
-	// Components is a list of price components that make up this Resource.
+	// Components is a list of price components that make up this Resource. If it is empty, the resource
+	// is considered to be skipped.
 	Components []Component
 }
 
diff --git a/terraform/plan.go b/terraform/plan.go
index 70445b1..cceab22 100644
--- a/terraform/plan.go
+++ b/terraform/plan.go
@@ -92,6 +92,10 @@ func (p *Plan) extractQueries(modules map[string]Module, providers map[string]Pr
 	result := make([]query.Resource, 0)
 	for _, module := range modules {
 		for _, tfres := range module.Resources {
+			if tfres.Mode != "managed" {
+				continue
+			}
+
 			providerKey, ok := resToProvKey[tfres.Address]
 			if !ok {
 				providerKey = tfres.ProviderName
@@ -99,13 +103,11 @@ func (p *Plan) extractQueries(modules map[string]Module, providers map[string]Pr
 
 			if provider, ok := providers[providerKey]; ok {
 				comps := provider.ResourceComponents(tfres)
-				if comps != nil {
-					q := query.Resource{
-						Address:    tfres.Address,
-						Components: comps,
-					}
-					result = append(result, q)
+				q := query.Resource{
+					Address:    tfres.Address,
+					Components: comps,
 				}
+				result = append(result, q)
 			}
 		}
 	}
diff --git a/terraform/schema.go b/terraform/schema.go
index 6d3bb4d..4f8e94e 100644
--- a/terraform/schema.go
+++ b/terraform/schema.go
@@ -22,6 +22,7 @@ type State struct {
 type Resource struct {
 	Address      string                 `json:"address"`
 	Index        int                    `json:"index"`
+	Mode         string                 `json:"mode"`
 	Type         string                 `json:"type"`
 	Name         string                 `json:"name"`
 	ProviderName string                 `json:"provider_name"`


Test patch:
diff --git a/cost/plan_test.go b/cost/plan_test.go
index d0fae4a..5fac6fc 100644
--- a/cost/plan_test.go
+++ b/cost/plan_test.go
@@ -167,3 +167,90 @@ func TestPlan_ResourceDifferences(t *testing.T) {
 		})
 	})
 }
+
+func TestPlan_SkippedAddresses(t *testing.T) {
+	t.Run("OnlyPrior", func(t *testing.T) {
+		prior := &cost.State{
+			Resources: map[string]cost.Resource{
+				"aws_instance.test_valid": {
+					Components: map[string]cost.Component{
+						"EC2 instance hours": {
+							Quantity: decimal.NewFromInt(730),
+							Unit:     "Hrs",
+							Rate:     decimal.NewFromFloat(2.50),
+						},
+					},
+				},
+				"aws_invalid_resource.test_skipped": {
+					Skipped: true,
+				},
+			},
+		}
+		plan := cost.NewPlan(prior, nil)
+		skipped := plan.SkippedAddresses()
+		require.Len(t, skipped, 1)
+		assert.Contains(t, skipped, "aws_invalid_resource.test_skipped")
+	})
+
+	t.Run("OnlyPlanned", func(t *testing.T) {
+		planned := &cost.State{
+			Resources: map[string]cost.Resource{
+				"aws_instance.test_valid": {
+					Components: map[string]cost.Component{
+						"EC2 instance hours": {
+							Quantity: decimal.NewFromInt(730),
+							Unit:     "Hrs",
+							Rate:     decimal.NewFromFloat(2.50),
+						},
+					},
+				},
+				"aws_invalid_resource.test_skipped": {
+					Skipped: true,
+				},
+			},
+		}
+		plan := cost.NewPlan(nil, planned)
+		skipped := plan.SkippedAddresses()
+		require.Len(t, skipped, 1)
+		assert.Contains(t, skipped, "aws_invalid_resource.test_skipped")
+	})
+
+	t.Run("PriorAndPlanned", func(t *testing.T) {
+		prior := &cost.State{
+			Resources: map[string]cost.Resource{
+				"aws_instance.test_prior": {
+					Components: map[string]cost.Component{
+						"EC2 instance hours": {
+							Quantity: decimal.NewFromInt(730),
+							Unit:     "Hrs",
+							Rate:     decimal.NewFromFloat(2.50),
+						},
+					},
+				},
+				"aws_invalid_resource.skipped_prior_planned": {Skipped: true},
+				"aws_invalid_resource.skipped_prior":         {Skipped: true},
+			},
+		}
+		planned := &cost.State{
+			Resources: map[string]cost.Resource{
+				"aws_instance.test_prior": {
+					Components: map[string]cost.Component{
+						"EC2 instance hours": {
+							Quantity: decimal.NewFromInt(730),
+							Unit:     "Hrs",
+							Rate:     decimal.NewFromFloat(2.50),
+						},
+					},
+				},
+				"aws_invalid_resource.skipped_prior_planned": {Skipped: true},
+				"aws_invalid_resource.skipped_planned":       {Skipped: true},
+			},
+		}
+		plan := cost.NewPlan(prior, planned)
+		skipped := plan.SkippedAddresses()
+		require.Len(t, skipped, 3)
+		assert.Contains(t, skipped, "aws_invalid_resource.skipped_prior_planned")
+		assert.Contains(t, skipped, "aws_invalid_resource.skipped_prior")
+		assert.Contains(t, skipped, "aws_invalid_resource.skipped_planned")
+	})
+}
diff --git a/cost/state_test.go b/cost/state_test.go
index 68245de..df32d95 100644
--- a/cost/state_test.go
+++ b/cost/state_test.go
@@ -38,6 +38,10 @@ func TestNewState(t *testing.T) {
 				},
 			},
 		},
+		{
+			Address:    "aws_invalid_resource.skipped",
+			Components: nil,
+		},
 	}
 
 	t.Run("Success", func(t *testing.T) {
@@ -66,6 +70,9 @@ func TestNewState(t *testing.T) {
 						},
 					},
 				},
+				"aws_invalid_resource.skipped": {
+					Skipped: true,
+				},
 			},
 		}
 
diff --git a/terraform/plan_test.go b/terraform/plan_test.go
index 55046a5..512701e 100644
--- a/terraform/plan_test.go
+++ b/terraform/plan_test.go
@@ -99,7 +99,8 @@ func TestPlan_ExtractPlannedQueries(t *testing.T) {
 
 		queries, err := plan.ExtractPlannedQueries()
 		require.NoError(t, err)
-		require.Len(t, queries, 0)
+		require.Len(t, queries, 2)
+		assert.Contains(t, queries, query.Resource{Address: "aws_instance.example"})
 	})
 }
 
@@ -183,6 +184,7 @@ func TestPlan_ExtractPriorQueries(t *testing.T) {
 
 		queries, err := plan.ExtractPriorQueries()
 		require.NoError(t, err)
-		require.Len(t, queries, 0)
+		require.Len(t, queries, 1)
+		assert.Contains(t, queries, query.Resource{Address: "aws_instance.example"})
 	})
 }