Repository: GoogleCloudPlatform/terraform-validator
Pull request #200 (2021-03-18 20:15:37)

returns ErrDuplicateAsset wrapped by a new error so that errors.Cause works
This PR Closes https://github.com/GoogleCloudPlatform/terraform-validator/issues/196

TODO:
- [x] Add unit test for the issue

The converter does a check by the cause of an error and just logs a warning if the error cause is `ErrDuplicateAsset `

https://github.com/GoogleCloudPlatform/terraform-validator/blob/6a9552f3f4c079b6c6abd1a53fc4f51f477beb8e/converters/google/convert.go#L203

```go
if errors.Cause(err) == ErrDuplicateAsset {
	glog.Warningf("adding resource change: %v", err)
} else {
``` 

in previous versions, this error was create as:

```go
return errors.Wrapf(ErrDuplicateAsset, "asset type %s: asset name %s", converted.Type, converted.Name)
```

but now it is using:

```go
return fmt.Errorf("asset type %s: asset name %s %w", converted.Type, converted.Name, ErrDuplicateAsset)
```

which does not wrap the error but creates a new error using only the "error message" from the original error.

this PR reverts the code to its previous version

Result with the new version in the test file from the issue(reformatted for clarity):

` ./bin/terraform-validator convert ../two_project_with_budget/tst.json --project=<REDACTED>`

```console
ERROR: logging before flag.Parse: I0318 16:53:12.402048    5683 convert.go:188] unsupported resource: google_billing_budget
ERROR: logging before flag.Parse: I0318 16:53:12.402104    5683 convert.go:188] unsupported resource: google_billing_budget
ERROR: logging before flag.Parse: I0318 16:53:12.402114    5683 convert.go:182] unknown resource: random_id
ERROR: logging before flag.Parse: W0318 16:53:13.708944    5683 convert.go:204] adding resource change: asset type cloudbilling.googleapis.com/ProjectBillingInfo: asset name //cloudbilling.googleapis.com/projects/<REDACTED>/billingInfo: duplicate asset
[
    {
        "name": "//cloudbilling.googleapis.com/projects/<REDACTED>/billingInfo",
        "asset_type": "cloudbilling.googleapis.com/ProjectBillingInfo",
        "ancestry_path": "organization/<REDACTED>/folder/146035178388/folder/220237755881/project/<REDACTED>",
        "resource": {
            "version": "v1",
            "discovery_document_uri": "https://www.googleapis.com/discovery/v1/apis/cloudbilling/v1/rest",
            "discovery_name": "ProjectBillingInfo",
            "parent": "//cloudresourcemanager.googleapis.com/projects/<REDACTED>",
            "data": {
                "billingAccountName": "billingAccounts/<REDACTED>",
                "name": "projects//billingInfo"
            }
        }
    },
    {
        "name": "//cloudresourcemanager.googleapis.com/projects/<REDACTED>",
        "asset_type": "cloudresourcemanager.googleapis.com/Project",
        "ancestry_path": "organization/<REDACTED>/folder/146035178388/folder/220237755881/project/<REDACTED>",
        "resource": {
            "version": "v1",
            "discovery_document_uri": "https://www.googleapis.com/discovery/v1/apis/compute/v1/rest",
            "discovery_name": "Project",
            "parent": "//cloudresourcemanager.googleapis.com/projects/<REDACTED>",
            "data": {
                "name": "My Project2",
                "parent": {
                    "id": "<REDACTED>",
                    "type": "organization"
                }
            }
        }
    }
]
```


Patch:
diff --git a/converters/google/convert.go b/converters/google/convert.go
index fd850f85..3f8928b5 100644
--- a/converters/google/convert.go
+++ b/converters/google/convert.go
@@ -16,19 +16,20 @@ package google
 
 import (
 	"context"
+	errorssyslib "errors"
 	"fmt"
 	"sort"
 	"time"
 
-	"github.com/hashicorp/terraform-json"
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/golang/glog"
+	tfjson "github.com/hashicorp/terraform-json"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	provider "github.com/hashicorp/terraform-provider-google/v3/google"
 	"github.com/pkg/errors"
 
-	"github.com/GoogleCloudPlatform/terraform-validator/tfplan"
 	converter "github.com/GoogleCloudPlatform/terraform-google-conversion/google"
 	"github.com/GoogleCloudPlatform/terraform-validator/ancestrymanager"
+	"github.com/GoogleCloudPlatform/terraform-validator/tfplan"
 )
 
 var ErrDuplicateAsset = errors.New("duplicate asset")
@@ -143,7 +144,7 @@ type Converter struct {
 	mapperFuncs map[string][]mapper
 
 	offline bool
-	cfg *converter.Config
+	cfg     *converter.Config
 
 	// ancestryManager provides a manager to find the ancestry information for a project.
 	ancestryManager ancestrymanager.AncestryManager
@@ -200,7 +201,7 @@ func (c *Converter) AddResourceChanges(changes []*tfjson.ResourceChange) error {
 
 	for _, rc := range createOrUpdates {
 		if err := c.addCreateOrUpdate(rc); err != nil {
-			if errors.Cause(err) == ErrDuplicateAsset {
+			if errorssyslib.Is(err, ErrDuplicateAsset) {
 				glog.Warningf("adding resource change: %v", err)
 			} else {
 				return fmt.Errorf("adding resource create or update %w", err)
@@ -211,7 +212,6 @@ func (c *Converter) AddResourceChanges(changes []*tfjson.ResourceChange) error {
 	return nil
 }
 
-
 // For deletions, we only need to handle mappers that support
 // both fetch and mergeDelete. Supporting just one doesn't
 // make sense, and supporting neither means that the deletion


Test patch:
diff --git a/converters/google/convert_test.go b/converters/google/convert_test.go
index bd3a53e6..89327acb 100644
--- a/converters/google/convert_test.go
+++ b/converters/google/convert_test.go
@@ -19,8 +19,8 @@ import (
 	"sort"
 	"testing"
 
-	"github.com/hashicorp/terraform-json"
 	"github.com/GoogleCloudPlatform/terraform-validator/ancestrymanager"
+	tfjson "github.com/hashicorp/terraform-json"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 )
@@ -145,7 +145,7 @@ func TestAddResourceChanges_deleteProcessed(t *testing.T) {
 		ProviderName: "google",
 		Change: &tfjson.Change{
 			Actions: tfjson.Actions{"delete"},
-			Before:  map[string]interface{}{
+			Before: map[string]interface{}{
 				"project": testProject,
 				"name":    "test-disk",
 				"type":    "pd-ssd",
@@ -156,7 +156,7 @@ func TestAddResourceChanges_deleteProcessed(t *testing.T) {
 				},
 				"physical_block_size_bytes": 4096,
 			},
-			After:   nil,
+			After: nil,
 		},
 	}
 	c, err := newTestConverter()
@@ -220,3 +220,135 @@ func TestAddResourceChanges_createOrUpdateOrDeleteCreateProcessed(t *testing.T)
 		})
 	}
 }
+
+func TestAddDuplicatedResources(t *testing.T) {
+	rcb1 := tfjson.ResourceChange{
+		Address:      "google_billing_budget.budget1",
+		Mode:         "managed",
+		Type:         "google_billing_budget",
+		Name:         "budget1",
+		ProviderName: "google",
+		Change: &tfjson.Change{
+			Actions: tfjson.Actions{"create"},
+			Before:  nil,
+			After: map[string]interface{}{
+				"all_updates_rule": []map[string]interface{}{},
+				"amount": []map[string]interface{}{
+					{
+						"last_period_amount": nil,
+						"specified_amount": []map[string]interface{}{
+							{
+								"currency_code": "USD",
+								"nanos":         nil,
+								"units":         "100",
+							},
+						},
+					},
+				},
+				"billing_account": "000000-000000-000000",
+				"budget_filter": []map[string]interface{}{
+					{
+						"credit_types_treatment": "INCLUDE_ALL_CREDITS",
+					},
+				},
+				"display_name": "Example Billing Budget 1",
+				"threshold_rules": []map[string]interface{}{
+					{
+						"spend_basis":       "CURRENT_SPEND",
+						"threshold_percent": 0.5,
+					},
+				},
+				"timeouts": nil,
+			},
+		},
+	}
+	rcb2 := tfjson.ResourceChange{
+		Address:      "google_billing_budget.budget2",
+		Mode:         "managed",
+		Type:         "google_billing_budget",
+		Name:         "budget2",
+		ProviderName: "google",
+		Change: &tfjson.Change{
+			Actions: tfjson.Actions{"create"},
+			Before:  nil,
+			After: map[string]interface{}{
+				"all_updates_rule": []map[string]interface{}{},
+				"amount": []map[string]interface{}{
+					{
+						"last_period_amount": nil,
+						"specified_amount": []map[string]interface{}{
+							{
+								"currency_code": "USD",
+								"nanos":         nil,
+								"units":         "100",
+							},
+						},
+					},
+				},
+				"billing_account": "000000-000000-000000",
+				"budget_filter": []map[string]interface{}{
+					{
+						"credit_types_treatment": "INCLUDE_ALL_CREDITS",
+					},
+				},
+				"display_name": "Example Billing Budget 2",
+				"threshold_rules": []map[string]interface{}{
+					{
+						"spend_basis":       "CURRENT_SPEND",
+						"threshold_percent": 0.5,
+					},
+				},
+				"timeouts": nil,
+			},
+		},
+	}
+	rcp1 := tfjson.ResourceChange{
+		Address:      "google_project.my_project1",
+		Mode:         "managed",
+		Type:         "google_project",
+		Name:         "my_project1",
+		ProviderName: "google",
+		Change: &tfjson.Change{
+			Actions: tfjson.Actions{"create"},
+			Before:  nil,
+			After: map[string]interface{}{
+				"auto_create_network": true,
+				"billing_account":     "000000-000000-000000",
+				"labels":              nil,
+				"name":                "My Project1",
+				"org_id":              "00000000000000",
+				"timeouts":            nil,
+			},
+		},
+	}
+	rcp2 := tfjson.ResourceChange{
+		Address:      "google_project.my_project2",
+		Mode:         "managed",
+		Type:         "google_project",
+		Name:         "my_project2",
+		ProviderName: "google",
+		Change: &tfjson.Change{
+			Actions: tfjson.Actions{"create"},
+			Before:  nil,
+			After: map[string]interface{}{
+				"auto_create_network": true,
+				"billing_account":     "000000-000000-000000",
+				"labels":              nil,
+				"name":                "My Project2",
+				"org_id":              "00000000000000",
+				"timeouts":            nil,
+			},
+		},
+	}
+	c, err := newTestConverter()
+	assert.Nil(t, err)
+
+	err = c.AddResourceChanges([]*tfjson.ResourceChange{&rcb1, &rcb2, &rcp1, &rcp2})
+	assert.Nil(t, err)
+
+	caiKeyBilling := "cloudbilling.googleapis.com/ProjectBillingInfo//cloudbilling.googleapis.com/projects/test-project/billingInfo"
+	assert.Contains(t, c.assets, caiKeyBilling)
+
+	caiKeyProject := "cloudresourcemanager.googleapis.com/Project//cloudresourcemanager.googleapis.com/projects/test-project"
+	assert.Contains(t, c.assets, caiKeyProject)
+}