Repository: GoogleCloudPlatform/terraform-validator
Pull request #285 (2021-08-05 17:24:20)

Updated add_new_resource.md for mappers.go move
mappers.go was moved to terraform-google-conversion; this updates the documentation accordingly.


Patch:
diff --git a/ancestrymanager/ancestrymanager.go b/ancestrymanager/ancestrymanager.go
index 55f6e834..9835f956 100644
--- a/ancestrymanager/ancestrymanager.go
+++ b/ancestrymanager/ancestrymanager.go
@@ -2,14 +2,11 @@
 package ancestrymanager
 
 import (
-	"context"
 	"fmt"
 	"log"
 	"strings"
 
-	"github.com/pkg/errors"
 	"google.golang.org/api/cloudresourcemanager/v1"
-	"google.golang.org/api/option"
 
 	converter "github.com/GoogleCloudPlatform/terraform-google-conversion/google"
 )
@@ -22,6 +19,12 @@ type AncestryManager interface {
 	GetAncestryWithResource(project string, tfData converter.TerraformResourceData, cai converter.Asset) (string, error)
 }
 
+// ClientRetriever is the interface that returns an instance of various clients.
+type ClientRetriever interface {
+	// NewResourceManagerClient returns an initialized *cloudresourcemanager.Service
+	NewResourceManagerClient(userAgent string) *cloudresourcemanager.Service
+}
+
 // resourceAncestryManager provides common methods for retrieving ancestry from resources
 type resourceAncestryManager struct {
 }
@@ -180,7 +183,7 @@ func (m *offlineAncestryManager) GetAncestryWithResource(project string, tfData
 }
 
 // New returns AncestryManager that can be used to fetch ancestry information for a project.
-func New(ctx context.Context, project, ancestry string, offline bool, opts ...option.ClientOption) (AncestryManager, error) {
+func New(retriever ClientRetriever, project, ancestry, userAgent string, offline bool) (AncestryManager, error) {
 	if ancestry != "" {
 		ancestry = fmt.Sprintf("%s/project/%s", ancestry, project)
 	}
@@ -189,10 +192,7 @@ func New(ctx context.Context, project, ancestry string, offline bool, opts ...op
 	}
 	am := &onlineAncestryManager{ancestryCache: map[string]string{}}
 	am.store(project, ancestry)
-	rm, err := cloudresourcemanager.NewService(ctx, opts...)
-	if err != nil {
-		return nil, errors.Wrap(err, "constructing resource manager client")
-	}
+	rm := retriever.NewResourceManagerClient(userAgent)
 	am.resourceManager = rm
 	return am, nil
 }
diff --git a/cnvconfig/getconfig.go b/cnvconfig/getconfig.go
new file mode 100644
index 00000000..70f029e4
--- /dev/null
+++ b/cnvconfig/getconfig.go
@@ -0,0 +1,48 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cnvconfig
+
+import (
+	"context"
+
+	converter "github.com/GoogleCloudPlatform/terraform-google-conversion/google"
+	"github.com/pkg/errors"
+)
+
+func GetConfig(ctx context.Context, project string, offline bool) (*converter.Config, error) {
+	cfg := &converter.Config{
+		Project: project,
+	}
+
+	// Search for default credentials
+	cfg.Credentials = multiEnvSearch([]string{
+		"GOOGLE_CREDENTIALS",
+		"GOOGLE_CLOUD_KEYFILE_JSON",
+		"GCLOUD_KEYFILE_JSON",
+	})
+
+	cfg.AccessToken = multiEnvSearch([]string{
+		"GOOGLE_OAUTH_ACCESS_TOKEN",
+	})
+
+	if !offline {
+		converter.ConfigureBasePaths(cfg)
+		if err := cfg.LoadAndValidate(ctx); err != nil {
+			return nil, errors.Wrap(err, "load and validate config")
+		}
+	}
+
+	return cfg, nil
+}
diff --git a/converters/google/utils.go b/cnvconfig/utils.go
similarity index 97%
rename from converters/google/utils.go
rename to cnvconfig/utils.go
index 26631e20..5640e5ef 100644
--- a/converters/google/utils.go
+++ b/cnvconfig/utils.go
@@ -11,7 +11,7 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-package google
+package cnvconfig
 
 import (
 	"os"
diff --git a/converters/google/convert.go b/converters/google/convert.go
index cb8980f2..2b335292 100644
--- a/converters/google/convert.go
+++ b/converters/google/convert.go
@@ -15,7 +15,6 @@
 package google
 
 import (
-	"context"
 	errorssyslib "errors"
 	"fmt"
 	"sort"
@@ -127,27 +126,7 @@ type RestoreDefault struct {
 }
 
 // NewConverter is a factory function for Converter.
-func NewConverter(ctx context.Context, ancestryManager ancestrymanager.AncestryManager, project string, offline, convertUnchanged bool) (*Converter, error) {
-	cfg := &converter.Config{
-		Project: project,
-	}
-	// Search for default credentials
-	cfg.Credentials = multiEnvSearch([]string{
-		"GOOGLE_CREDENTIALS",
-		"GOOGLE_CLOUD_KEYFILE_JSON",
-		"GCLOUD_KEYFILE_JSON",
-	})
-
-	cfg.AccessToken = multiEnvSearch([]string{
-		"GOOGLE_OAUTH_ACCESS_TOKEN",
-	})
-	if !offline {
-		converter.ConfigureBasePaths(cfg)
-		if err := cfg.LoadAndValidate(ctx); err != nil {
-			return nil, errors.Wrap(err, "load and validate config")
-		}
-	}
-
+func NewConverter(cfg *converter.Config, ancestryManager ancestrymanager.AncestryManager, offline bool, convertUnchanged bool) *Converter {
 	return &Converter{
 		schema:           provider.Provider(),
 		mapperFuncs:      converter.Mappers(),
@@ -156,7 +135,7 @@ func NewConverter(ctx context.Context, ancestryManager ancestrymanager.AncestryM
 		ancestryManager:  ancestryManager,
 		assets:           make(map[string]Asset),
 		convertUnchanged: convertUnchanged,
-	}, nil
+	}
 }
 
 // Converter knows how to convert terraform resources to their
diff --git a/docs/contributing/add_new_resource.md b/docs/contributing/add_new_resource.md
index bc52dacd..d051a4d4 100644
--- a/docs/contributing/add_new_resource.md
+++ b/docs/contributing/add_new_resource.md
@@ -18,29 +18,26 @@ The first step in determining if a GCP resource is supported is to figure out th
 
 ## How to add support for a new resource
 
-A resource is "supported" by terraform-validator if it has an entry in [mappers.go](../converters/google/mappers.go). For example, you could search mappers.go for [`google_compute_disk`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_disk) to see if that resource is supported. Each entry in mappers.go has the following keys:
+A resource is "supported" by terraform-validator if it has an entry in [mappers.go](https://github.com/GoogleCloudPlatform/terraform-google-conversion/blob/master/google/mappers.go). For example, you could search mappers.go for [`google_compute_disk`](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_disk) to see if that resource is supported.
 
-- `convert`: Required. This function does basic conversion of a Terraform resource to a CAI Asset, including converting nested structures and specifying what the [CAI Asset Type](https://cloud.google.com/asset-inventory/docs/supported-asset-types) is.
-- `fetch`, `mergeCreateUpdate`, `mergeDelete`: Optional. Some assets, like [IAM Members and Bindings](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam), have to be merged with remote data prior to validation in order to properly check whether policies are being followed. If you're not sure whether you need this, you probably don't.
-
-The code referenced by the mappers comes from [terraform-google-conversion](https://github.com/GoogleCloudPlatform/terraform-google-conversion/tree/master/google), but that repository is generated using [Magic Modules](https://github.com/googleCloudPlatform/magic-modules/). Magic Modules uses a shared code base to generate terraform-google-conversion and the [google](https://github.com/hashicorp/terraform-provider-google) and [google-beta](https://github.com/hashicorp/terraform-provider-google-beta) Terraform providers.
-
-So, adding support for a resource means:
+Adding support for a resource has two steps:
 
-1. Make a PR for Magic Modules to add the necessary code to terraform-google-conversion. Once your PR is merged, the code will be automatically copied into terraform-google-conversion.
-2. Make a PR for terraform-validator that updates the version of terraform-google-conversion and adds a new mapper to mappers.go for the new resource, as well as new tests.
+1. Make a PR for [Magic Modules](https://github.com/GoogleCloudPlatform/magic-modules) to add the necessary code to terraform-google-conversion. Once your PR is merged, the code will be automatically copied into terraform-google-conversion.
+2. Make a PR for terraform-validator that updates the version of terraform-google-conversion and adds tests for the new resource.
 
-Each of these steps is discussed in more detail below.
+Each of these is discussed in more detail below.
 
 **Note**: terraform-validator can only support resources that are supported by the GA terraform provider, not beta resources.
 
 ### 1. Magic Modules
 
-The goal of Magic Modules is to auto-generate code targeting specific API endpoints using [yaml files](https://github.com/GoogleCloudPlatform/magic-modules/tree/master/mmv1/products) which are grouped by product.
-By default, those yaml files will also be used to generate code for terraform-google-conversion.
+Magic Modules uses a shared code base to generate terraform-google-conversion and the [google](https://github.com/hashicorp/terraform-provider-google) and [google-beta](https://github.com/hashicorp/terraform-provider-google-beta) Terraform providers.
+Most Terraform resources are represented as [yaml files which are grouped by product](https://github.com/GoogleCloudPlatform/magic-modules/tree/master/mmv1/products).
+Each product has an `api.yaml` file (which defines the basic API schema) and a `terraform.yaml` file (which defines any terraform-specific overrides.)
 A `terraform.yaml` file can specify `exclude_validator: true` on a resource to skip terraform-google-conversion autogeneration, or `exclude_resource: true` to skip autogeneration for both terraform-google-conversion and the providers.
 
-If terraform-google-conversion code can't be auto-generated based on a yaml file, you can instead place a handwritten file in the [`magic-modules/mmv1/third_party/validator` folder](https://github.com/GoogleCloudPlatform/magic-modules/tree/master/mmv1/third_party/validator). Most resources will only need a conversion func, which should look something like:
+Auto-generating terraform-google-conversion code based on yaml files is strongly preferred; however, if that is not possible, you can instead place a handwritten file in the [`magic-modules/mmv1/third_party/validator` folder](https://github.com/GoogleCloudPlatform/magic-modules/tree/master/mmv1/third_party/validator).
+Most resources will only need a conversion func, which should look something like:
 
 ```golang
 func GetWhateverResourceCaiObject(d TerraformResourceData, config *Config) ([]Asset, error) {
@@ -79,6 +76,11 @@ func GetWhateverResourceApiObject(d TerraformResourceData, config *Config) (map[
 
 ```
 
+For handwritten conversion code, you will also need to add an entry to [`mappers.go.erb`](https://github.com/GoogleCloudPlatform/magic-modules/blob/master/mmv1/templates/validator/mappers/mappers.go.erb), which is used to generate the mappers.go file in terraform-google-conversion. Each entry in `mappers.go.erb` has the following keys:
+
+- `convert`: Required. This function does basic conversion of a Terraform resource to a CAI Asset, including converting nested structures and specifying what the [CAI Asset Type](https://cloud.google.com/asset-inventory/docs/supported-asset-types) is.
+- `fetch`, `mergeCreateUpdate`, `mergeDelete`: Optional. Some assets, like [IAM Members and Bindings](https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_project_iam), have to be merged with remote data prior to validation in order to properly check whether policies are being followed. If you're not sure whether you need this, you probably don't.
+
 To generate terraform-google-conversion code locally, run the following from the root of the `magic-modules` repository:
 
 ```
@@ -99,4 +101,4 @@ Be sure to add test cases to [test/cli_test.go](https://github.com/GoogleCloudPl
    - A .tfplan.json file.
    - A .json file (representing the output of `terraform-validator convert`)
 
-See [Getting started](./getting_started.md) for details on running tests.
\ No newline at end of file
+See [Getting started](./getting_started.md) for details on running tests.
diff --git a/tfgcv/planned_assets.go b/tfgcv/planned_assets.go
index b07b4f99..31d9b868 100644
--- a/tfgcv/planned_assets.go
+++ b/tfgcv/planned_assets.go
@@ -20,9 +20,8 @@ import (
 	"io/ioutil"
 	"path/filepath"
 
-	"google.golang.org/api/option"
-
 	"github.com/GoogleCloudPlatform/terraform-validator/ancestrymanager"
+	"github.com/GoogleCloudPlatform/terraform-validator/cnvconfig"
 	"github.com/GoogleCloudPlatform/terraform-validator/converters/google"
 	"github.com/GoogleCloudPlatform/terraform-validator/tfplan"
 	"github.com/pkg/errors"
@@ -60,15 +59,16 @@ func ReadPlannedAssets(ctx context.Context, path, project, ancestry string, offl
 }
 
 func newConverter(ctx context.Context, path, project, ancestry string, offline, convertUnchanged bool) (*google.Converter, error) {
-	ua := option.WithUserAgent(fmt.Sprintf("config-validator-tf/%s", BuildVersion()))
-	ancestryManager, err := ancestrymanager.New(context.Background(), project, ancestry, offline, ua)
+	cfg, err := cnvconfig.GetConfig(ctx, project, offline)
 	if err != nil {
-		return nil, errors.Wrap(err, "constructing resource manager client")
+		return nil, errors.Wrap(err, "building google configuration")
 	}
-	converter, err := google.NewConverter(ctx, ancestryManager, project, offline, convertUnchanged)
+	ua := fmt.Sprintf("config-validator-tf/%s", BuildVersion())
+	ancestryManager, err := ancestrymanager.New(cfg, project, ancestry, ua, offline)
 	if err != nil {
-		return nil, errors.Wrap(err, "building google converter")
+		return nil, errors.Wrap(err, "building google ancestry manager")
 	}
+	converter := google.NewConverter(cfg, ancestryManager, offline, convertUnchanged)
 	return converter, nil
 }
 


Test patch:
diff --git a/ancestrymanager/ancestrymanager_test.go b/ancestrymanager/ancestrymanager_test.go
index 382c8ba5..1720d4aa 100644
--- a/ancestrymanager/ancestrymanager_test.go
+++ b/ancestrymanager/ancestrymanager_test.go
@@ -11,6 +11,24 @@ import (
 	"google.golang.org/api/option"
 )
 
+type testRetriever struct {
+	online bool
+	opts   []option.ClientOption
+}
+
+func (tr *testRetriever) NewResourceManagerClient(userAgent string) *cloudresourcemanager.Service {
+	ctx := context.Background()
+
+	if tr.online {
+		rm, err := cloudresourcemanager.NewService(ctx, tr.opts...)
+		if err != nil {
+			panic(err)
+		}
+		return rm
+	}
+	return nil
+}
+
 func TestAncestryPath(t *testing.T) {
 	cases := []struct {
 		name           string
@@ -53,7 +71,6 @@ func TestAncestryPath(t *testing.T) {
 }
 
 func TestGetAncestry(t *testing.T) {
-	ctx := context.Background()
 	ownerProject := "foo"
 	ownerAncestry := "organization/qux/folder/bar"
 	ownerAncestryPath := "organization/qux/folder/bar/project/foo"
@@ -75,11 +92,15 @@ func TestGetAncestry(t *testing.T) {
 	ts := newAncestryManagerMockServer(t, cache)
 	defer ts.Close()
 
-	amOnline, err := New(ctx, ownerProject, "", false, option.WithEndpoint(ts.URL), option.WithoutAuthentication())
+	// option.WithEndpoint(ts.URL), option.WithoutAuthentication()
+	trOnline := &testRetriever{online: true, opts: []option.ClientOption{option.WithEndpoint(ts.URL), option.WithoutAuthentication()}}
+	amOnline, err := New(trOnline, ownerProject, ownerAncestry, "", false)
 	if err != nil {
 		t.Fatalf("failed to create online ancestry manager: %s", err)
 	}
-	amOffline, err := New(ctx, ownerProject, ownerAncestry, true)
+
+	trOffline := &testRetriever{online: false}
+	amOffline, err := New(trOffline, ownerProject, ownerAncestry, "", true)
 	if err != nil {
 		t.Fatalf("failed to create offline ancestry manager: %s", err)
 	}
diff --git a/cnvconfig/getconfig_test.go b/cnvconfig/getconfig_test.go
new file mode 100644
index 00000000..68888ad5
--- /dev/null
+++ b/cnvconfig/getconfig_test.go
@@ -0,0 +1,84 @@
+package cnvconfig
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	converter "github.com/GoogleCloudPlatform/terraform-google-conversion/google"
+	"github.com/stretchr/testify/assert"
+)
+
+type configAttrGetter func(cfg *converter.Config) string
+
+func getCredentials(cfg *converter.Config) string {
+	return cfg.Credentials
+}
+func getAccessToken(cfg *converter.Config) string {
+	return cfg.AccessToken
+}
+
+func TestNewConverterCredentials(t *testing.T) {
+	ctx := context.Background()
+	offline := true
+	cases := []struct {
+		name           string
+		envKey         string
+		envValue       string
+		getConfigValue configAttrGetter
+	}{
+		{
+			name:           "GOOGLE_CREDENTIALS",
+			envKey:         "GOOGLE_CREDENTIALS",
+			envValue:       "whatever",
+			getConfigValue: getCredentials,
+		},
+		{
+			name:           "GOOGLE_CLOUD_KEYFILE_JSON",
+			envKey:         "GOOGLE_CLOUD_KEYFILE_JSON",
+			envValue:       "whatever",
+			getConfigValue: getCredentials,
+		},
+		{
+			name:           "GCLOUD_KEYFILE_JSON",
+			envKey:         "GCLOUD_KEYFILE_JSON",
+			envValue:       "whatever",
+			getConfigValue: getCredentials,
+		},
+		{
+			name:           "GOOGLE_OAUTH_ACCESS_TOKEN",
+			envKey:         "GOOGLE_OAUTH_ACCESS_TOKEN",
+			envValue:       "whatever",
+			getConfigValue: getAccessToken,
+		},
+	}
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			originalValue, isSet := os.LookupEnv(c.envKey)
+			err := os.Setenv(c.envKey, c.envValue)
+			if err != nil {
+				t.Fatalf("error setting env var %s=%s: %s", c.envKey, c.envValue, err)
+			}
+
+			cfg, err := GetConfig(ctx, "project", offline)
+			if err != nil {
+				t.Fatalf("error building converter: %s", err)
+			}
+
+			assert.EqualValues(t, c.getConfigValue(cfg), c.envValue)
+
+			if isSet {
+				err = os.Setenv(c.envKey, originalValue)
+				if err != nil {
+					t.Fatalf("error setting env var %s=%s: %s", c.envKey, originalValue, err)
+				}
+			} else {
+				err = os.Unsetenv(c.envKey)
+				if err != nil {
+					t.Fatalf("error unsetting env var %s: %s", c.envKey, err)
+				}
+			}
+		})
+	}
+}
diff --git a/converters/google/convert_test.go b/converters/google/convert_test.go
index 594f11e6..0e337816 100644
--- a/converters/google/convert_test.go
+++ b/converters/google/convert_test.go
@@ -16,13 +16,12 @@ package google
 
 import (
 	"context"
-	"os"
 	"sort"
 	"testing"
 	"time"
 
-	converter "github.com/GoogleCloudPlatform/terraform-google-conversion/google"
 	"github.com/GoogleCloudPlatform/terraform-validator/ancestrymanager"
+	"github.com/GoogleCloudPlatform/terraform-validator/cnvconfig"
 	tfjson "github.com/hashicorp/terraform-json"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -33,91 +32,21 @@ const testProject = "test-project"
 func newTestConverter(convertUnchanged bool) (*Converter, error) {
 	ctx := context.Background()
 	ancestry := ""
+	ua := ""
 	project := testProject
 	offline := true
-	ancestryManager, err := ancestrymanager.New(context.Background(), project, ancestry, offline)
+	cfg, err := cnvconfig.GetConfig(ctx, project, offline)
 	if err != nil {
-		return nil, errors.Wrap(err, "constructing resource manager client")
+		return nil, errors.Wrap(err, "constructing configuration")
 	}
-	c, err := NewConverter(ctx, ancestryManager, project, offline, convertUnchanged)
+	ancestryManager, err := ancestrymanager.New(cfg, project, ancestry, ua, offline)
 	if err != nil {
-		return nil, errors.Wrap(err, "building converter")
+		return nil, errors.Wrap(err, "constructing resource ancestryManager")
 	}
+	c := NewConverter(cfg, ancestryManager, offline, convertUnchanged)
 	return c, nil
 }
 
-type configAttrGetter func(cfg *converter.Config) string
-
-func getCredentials(cfg *converter.Config) string {
-	return cfg.Credentials
-}
-func getAccessToken(cfg *converter.Config) string {
-	return cfg.AccessToken
-}
-
-func TestNewConverterCredentials(t *testing.T) {
-	cases := []struct {
-		name           string
-		envKey         string
-		envValue       string
-		getConfigValue configAttrGetter
-	}{
-		{
-			name:           "GOOGLE_CREDENTIALS",
-			envKey:         "GOOGLE_CREDENTIALS",
-			envValue:       "whatever",
-			getConfigValue: getCredentials,
-		},
-		{
-			name:           "GOOGLE_CLOUD_KEYFILE_JSON",
-			envKey:         "GOOGLE_CLOUD_KEYFILE_JSON",
-			envValue:       "whatever",
-			getConfigValue: getCredentials,
-		},
-		{
-			name:           "GCLOUD_KEYFILE_JSON",
-			envKey:         "GCLOUD_KEYFILE_JSON",
-			envValue:       "whatever",
-			getConfigValue: getCredentials,
-		},
-		{
-			name:           "GOOGLE_OAUTH_ACCESS_TOKEN",
-			envKey:         "GOOGLE_OAUTH_ACCESS_TOKEN",
-			envValue:       "whatever",
-			getConfigValue: getAccessToken,
-		},
-	}
-
-	for _, c := range cases {
-		t.Run(c.name, func(t *testing.T) {
-			originalValue, isSet := os.LookupEnv(c.envKey)
-			err := os.Setenv(c.envKey, c.envValue)
-			if err != nil {
-				t.Fatalf("error setting env var %s=%s: %s", c.envKey, c.envValue, err)
-			}
-
-			converter, err := newTestConverter(false)
-			if err != nil {
-				t.Fatalf("error building converter: %s", err)
-			}
-
-			assert.EqualValues(t, c.getConfigValue(converter.cfg), c.envValue)
-
-			if isSet {
-				err = os.Setenv(c.envKey, originalValue)
-				if err != nil {
-					t.Fatalf("error setting env var %s=%s: %s", c.envKey, originalValue, err)
-				}
-			} else {
-				err = os.Unsetenv(c.envKey)
-				if err != nil {
-					t.Fatalf("error unsetting env var %s: %s", c.envKey, err)
-				}
-			}
-		})
-	}
-}
-
 func TestSortByName(t *testing.T) {
 	cases := []struct {
 		name           string