Skip to content
This repository has been archived by the owner on May 15, 2023. It is now read-only.

Commit

Permalink
Merge pull request #200 from daniel-cit/bugfix/fix-error-cause-handling
Browse files Browse the repository at this point in the history
returns ErrDuplicateAsset wrapped by a new error so that errors.Cause works
  • Loading branch information
melinath committed Mar 22, 2021
2 parents 6cc8af9 + 4e59e14 commit 816a8dd
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 9 deletions.
12 changes: 6 additions & 6 deletions converters/google/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
138 changes: 135 additions & 3 deletions converters/google/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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",
Expand All @@ -156,7 +156,7 @@ func TestAddResourceChanges_deleteProcessed(t *testing.T) {
},
"physical_block_size_bytes": 4096,
},
After: nil,
After: nil,
},
}
c, err := newTestConverter()
Expand Down Expand Up @@ -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)
}

0 comments on commit 816a8dd

Please sign in to comment.