update ancestry manager to use cfg (GoogleCloudPlatform#283)

* update ancestry manager to use cfg

* fix tests, fork function

* move credential test

* remove split functionality, change original interface

* abstract config out to retriever class

* align coments with names

* clean up

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
 }

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)
 	}

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
+}

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)
+				}
+			}
+		})
+	}
+}

converters/google/utils.go → 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"

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