From a8d0a70c0389de763ab77a8778d7c7148b2ace4b Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Thu, 23 Jul 2015 16:53:44 -0400 Subject: [PATCH 1/9] providers/google: Add account_file_contents to provider --- builtin/providers/google/config.go | 10 +++++++--- builtin/providers/google/provider.go | 6 ++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/builtin/providers/google/config.go b/builtin/providers/google/config.go index 6af5fbd61b..99e693fd44 100644 --- a/builtin/providers/google/config.go +++ b/builtin/providers/google/config.go @@ -23,9 +23,10 @@ import ( // Config is the configuration structure used to instantiate the Google // provider. type Config struct { - AccountFile string - Project string - Region string + AccountFile string + AccountFileContents string + Project string + Region string clientCompute *compute.Service clientContainer *container.Service @@ -40,6 +41,9 @@ func (c *Config) loadAndValidate() error { if c.AccountFile == "" { c.AccountFile = os.Getenv("GOOGLE_ACCOUNT_FILE") } + if c.AccountFileContents == "" { + c.AccountFileContents = os.Getenv("GOOGLE_ACCOUNT_FILE_CONTENTS") + } if c.Project == "" { c.Project = os.Getenv("GOOGLE_PROJECT") } diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index 30cef8c1bd..c93812d5a0 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -15,6 +15,12 @@ func Provider() terraform.ResourceProvider { DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", nil), }, + "account_file_contents": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE_CONTENTS", nil), + }, + "project": &schema.Schema{ Type: schema.TypeString, Required: true, From 4ce776d252c29b26466b81dfd2de1c062f9e94fa Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Thu, 23 Jul 2015 17:56:32 -0400 Subject: [PATCH 2/9] providers/google: Use account_file_contents if provided --- builtin/providers/google/config.go | 33 ++++++++---- builtin/providers/google/config_test.go | 67 +++++++++++++++++++------ builtin/providers/google/provider.go | 7 +-- 3 files changed, 80 insertions(+), 27 deletions(-) diff --git a/builtin/providers/google/config.go b/builtin/providers/google/config.go index 99e693fd44..853b5228a4 100644 --- a/builtin/providers/google/config.go +++ b/builtin/providers/google/config.go @@ -3,10 +3,12 @@ package google import ( "encoding/json" "fmt" + "io/ioutil" "log" "net/http" "os" "runtime" + "strings" // TODO(dcunnin): Use version code from version.go @@ -54,10 +56,25 @@ func (c *Config) loadAndValidate() error { var client *http.Client if c.AccountFile != "" { - if err := loadJSON(&account, c.AccountFile); err != nil { + if c.AccountFileContents != "" { return fmt.Errorf( - "Error loading account file '%s': %s", - c.AccountFile, + "Cannot provide both account_file and account_file_contents", + ) + } + + b, err := ioutil.ReadFile(c.AccountFile) + if err != nil { + return err + } + + c.AccountFileContents = string(b) + } + + if c.AccountFileContents != "" { + if err := parseJSON(&account, c.AccountFileContents); err != nil { + return fmt.Errorf( + "Error parsing account file contents '%s': %s", + c.AccountFileContents, err) } @@ -150,13 +167,9 @@ type accountFile struct { ClientId string `json:"client_id"` } -func loadJSON(result interface{}, path string) error { - f, err := os.Open(path) - if err != nil { - return err - } - defer f.Close() +func parseJSON(result interface{}, contents string) error { + r := strings.NewReader(contents) + dec := json.NewDecoder(r) - dec := json.NewDecoder(f) return dec.Decode(result) } diff --git a/builtin/providers/google/config_test.go b/builtin/providers/google/config_test.go index b4ee585209..3ec20376a9 100644 --- a/builtin/providers/google/config_test.go +++ b/builtin/providers/google/config_test.go @@ -1,24 +1,63 @@ package google import ( - "reflect" + "io/ioutil" "testing" ) -func TestConfigLoadJSON_account(t *testing.T) { - var actual accountFile - if err := loadJSON(&actual, "./test-fixtures/fake_account.json"); err != nil { - t.Fatalf("err: %s", err) +const testFakeAccountFilePath = "./test-fixtures/fake_account.json" + +func TestConfigLoadAndValidate_accountFile(t *testing.T) { + config := Config{ + AccountFile: testFakeAccountFilePath, + Project: "my-gce-project", + Region: "us-central1", } - expected := accountFile{ - PrivateKeyId: "foo", - PrivateKey: "bar", - ClientEmail: "foo@bar.com", - ClientId: "id@foo.com", - } - - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("bad: %#v", actual) + err := config.loadAndValidate() + if err != nil { + t.Fatalf("error: %v", err) + } +} + +func TestConfigLoadAndValidate_accountFileContents(t *testing.T) { + contents, err := ioutil.ReadFile(testFakeAccountFilePath) + if err != nil { + t.Fatalf("error: %v", err) + } + config := Config{ + AccountFileContents: string(contents), + Project: "my-gce-project", + Region: "us-central1", + } + + err = config.loadAndValidate() + if err != nil { + t.Fatalf("error: %v", err) + } +} + +func TestConfigLoadAndValidate_none(t *testing.T) { + config := Config{ + Project: "my-gce-project", + Region: "us-central1", + } + + err := config.loadAndValidate() + if err != nil { + t.Fatalf("error: %v", err) + } +} + +func TestConfigLoadAndValidate_both(t *testing.T) { + config := Config{ + AccountFile: testFakeAccountFilePath, + AccountFileContents: "{}", + Project: "my-gce-project", + Region: "us-central1", + } + + if config.loadAndValidate() == nil { + t.Fatalf("expected error, but got nil") } } diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index c93812d5a0..969895d177 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -59,9 +59,10 @@ func Provider() terraform.ResourceProvider { func providerConfigure(d *schema.ResourceData) (interface{}, error) { config := Config{ - AccountFile: d.Get("account_file").(string), - Project: d.Get("project").(string), - Region: d.Get("region").(string), + AccountFile: d.Get("account_file").(string), + AccountFileContents: d.Get("account_file_contents").(string), + Project: d.Get("project").(string), + Region: d.Get("region").(string), } if err := config.loadAndValidate(); err != nil { From c7954dbf745baf6fb34ce14cbf90b3b057f510e7 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Fri, 24 Jul 2015 10:09:19 -0400 Subject: [PATCH 3/9] providers/google: Document account_file_contents --- .../docs/providers/google/index.html.markdown | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/website/source/docs/providers/google/index.html.markdown b/website/source/docs/providers/google/index.html.markdown index 8adb9ed908..86745b8afb 100644 --- a/website/source/docs/providers/google/index.html.markdown +++ b/website/source/docs/providers/google/index.html.markdown @@ -34,13 +34,19 @@ resource "google_compute_instance" "default" { The following keys can be used to configure the provider. -* `account_file` - (Required) Path to the JSON file used to describe your - account credentials, downloaded from Google Cloud Console. More details on - retrieving this file are below. The _account file_ can be "" if you - are running terraform from a GCE instance with a properly-configured [Compute - Engine Service Account](https://cloud.google.com/compute/docs/authentication). - This can also be specified with the `GOOGLE_ACCOUNT_FILE` shell environment - variable. +* `account_file` - (Required, unless `account_file_contents` is present) Path + to the JSON file used to describe your account credentials, downloaded from + Google Cloud Console. More details on retrieving this file are below. The + _account file_ can be "" if you are running terraform from a GCE instance with + a properly-configured [Compute Engine Service + Account](https://cloud.google.com/compute/docs/authentication). This can also + be specified with the `GOOGLE_ACCOUNT_FILE` shell environment variable. + +* `account_file_contents` - (Required, unless `account_file` is present) The + contents of `account_file`. This can be used to pass the account credentials + with a Terraform var or environment variable if the account file is not + accessible. This can also be specified with the `GOOGLE_ACCOUNT_FILE_CONTENTS` + shell environment variable. * `project` - (Required) The ID of the project to apply any resources to. This can also be specified with the `GOOGLE_PROJECT` shell environment variable. From a7ca7e0b36fcfab21d5c678514d62b3c865a0b4a Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Fri, 24 Jul 2015 10:20:08 -0400 Subject: [PATCH 4/9] providers/google: Add account_file/account_file_contents ConflictsWith --- builtin/providers/google/provider.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index 969895d177..8d26bc627d 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -10,15 +10,17 @@ func Provider() terraform.ResourceProvider { return &schema.Provider{ Schema: map[string]*schema.Schema{ "account_file": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", nil), + Type: schema.TypeString, + Optional: true, + ConflictsWith: []string{"account_file_contents"}, + DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", nil), }, "account_file_contents": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE_CONTENTS", nil), + Type: schema.TypeString, + Optional: true, + ConflictsWith: []string{"account_file"}, + DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE_CONTENTS", nil), }, "project": &schema.Schema{ From 2a04708d664ef9b4800cf216577993b0c5af8303 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Fri, 24 Jul 2015 14:25:27 -0400 Subject: [PATCH 5/9] providers/google: Default account_file* to empty Prevents prompting for input --- builtin/providers/google/provider.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index 8d26bc627d..c1b2316fd5 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -10,17 +10,15 @@ func Provider() terraform.ResourceProvider { return &schema.Provider{ Schema: map[string]*schema.Schema{ "account_file": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ConflictsWith: []string{"account_file_contents"}, - DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", nil), + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", ""), }, "account_file_contents": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ConflictsWith: []string{"account_file"}, - DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE_CONTENTS", nil), + Type: schema.TypeString, + Optional: true, + DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE_CONTENTS", ""), }, "project": &schema.Schema{ From 773852e2d5fe7e70afb034c54dfd4f319c479c14 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Mon, 27 Jul 2015 15:35:52 -0400 Subject: [PATCH 6/9] providers/google: Change account_file to JSON If JSON fails to parse, treat it as a file path --- builtin/providers/google/config.go | 50 ++++++++++--------- builtin/providers/google/config_test.go | 31 ++++-------- builtin/providers/google/provider.go | 49 +++++++++++++----- .../docs/providers/google/index.html.markdown | 22 +++----- 4 files changed, 79 insertions(+), 73 deletions(-) diff --git a/builtin/providers/google/config.go b/builtin/providers/google/config.go index 853b5228a4..409443fdba 100644 --- a/builtin/providers/google/config.go +++ b/builtin/providers/google/config.go @@ -25,10 +25,9 @@ import ( // Config is the configuration structure used to instantiate the Google // provider. type Config struct { - AccountFile string - AccountFileContents string - Project string - Region string + AccountFile string + Project string + Region string clientCompute *compute.Service clientContainer *container.Service @@ -39,13 +38,9 @@ type Config struct { func (c *Config) loadAndValidate() error { var account accountFile - // TODO: validation that it isn't blank if c.AccountFile == "" { c.AccountFile = os.Getenv("GOOGLE_ACCOUNT_FILE") } - if c.AccountFileContents == "" { - c.AccountFileContents = os.Getenv("GOOGLE_ACCOUNT_FILE_CONTENTS") - } if c.Project == "" { c.Project = os.Getenv("GOOGLE_PROJECT") } @@ -56,25 +51,32 @@ func (c *Config) loadAndValidate() error { var client *http.Client if c.AccountFile != "" { - if c.AccountFileContents != "" { - return fmt.Errorf( - "Cannot provide both account_file and account_file_contents", - ) + contents := c.AccountFile + + // Assume account_file is a JSON string + if err := parseJSON(&account, contents); err != nil { + // If account_file was not JSON, assume it is a file path instead + if _, err := os.Stat(c.AccountFile); os.IsNotExist(err) { + return fmt.Errorf( + "account_file path does not exist: %s", + c.AccountFile) + } + + b, err := ioutil.ReadFile(c.AccountFile) + if err != nil { + return fmt.Errorf( + "Error reading account_file from path '%s': %s", + c.AccountFile, + err) + } + + contents = string(b) } - b, err := ioutil.ReadFile(c.AccountFile) - if err != nil { - return err - } - - c.AccountFileContents = string(b) - } - - if c.AccountFileContents != "" { - if err := parseJSON(&account, c.AccountFileContents); err != nil { + if err := parseJSON(&account, contents); err != nil { return fmt.Errorf( - "Error parsing account file contents '%s': %s", - c.AccountFileContents, + "Error parsing account file '%s': %s", + contents, err) } diff --git a/builtin/providers/google/config_test.go b/builtin/providers/google/config_test.go index 3ec20376a9..cc1b6213fa 100644 --- a/builtin/providers/google/config_test.go +++ b/builtin/providers/google/config_test.go @@ -7,7 +7,7 @@ import ( const testFakeAccountFilePath = "./test-fixtures/fake_account.json" -func TestConfigLoadAndValidate_accountFile(t *testing.T) { +func TestConfigLoadAndValidate_accountFilePath(t *testing.T) { config := Config{ AccountFile: testFakeAccountFilePath, Project: "my-gce-project", @@ -20,15 +20,15 @@ func TestConfigLoadAndValidate_accountFile(t *testing.T) { } } -func TestConfigLoadAndValidate_accountFileContents(t *testing.T) { +func TestConfigLoadAndValidate_accountFileJSON(t *testing.T) { contents, err := ioutil.ReadFile(testFakeAccountFilePath) if err != nil { t.Fatalf("error: %v", err) } config := Config{ - AccountFileContents: string(contents), - Project: "my-gce-project", - Region: "us-central1", + AccountFile: string(contents), + Project: "my-gce-project", + Region: "us-central1", } err = config.loadAndValidate() @@ -37,24 +37,11 @@ func TestConfigLoadAndValidate_accountFileContents(t *testing.T) { } } -func TestConfigLoadAndValidate_none(t *testing.T) { +func TestConfigLoadAndValidate_accountFileJSONInvalid(t *testing.T) { config := Config{ - Project: "my-gce-project", - Region: "us-central1", - } - - err := config.loadAndValidate() - if err != nil { - t.Fatalf("error: %v", err) - } -} - -func TestConfigLoadAndValidate_both(t *testing.T) { - config := Config{ - AccountFile: testFakeAccountFilePath, - AccountFileContents: "{}", - Project: "my-gce-project", - Region: "us-central1", + AccountFile: "{this is not json}", + Project: "my-gce-project", + Region: "us-central1", } if config.loadAndValidate() == nil { diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index c1b2316fd5..b99ac44b3d 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -1,6 +1,10 @@ package google import ( + "encoding/json" + "fmt" + "os" + "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) @@ -10,15 +14,10 @@ func Provider() terraform.ResourceProvider { return &schema.Provider{ Schema: map[string]*schema.Schema{ "account_file": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", ""), - }, - - "account_file_contents": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE_CONTENTS", ""), + Type: schema.TypeString, + Required: true, + DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", nil), + ValidateFunc: validateAccountFile, }, "project": &schema.Schema{ @@ -59,10 +58,9 @@ func Provider() terraform.ResourceProvider { func providerConfigure(d *schema.ResourceData) (interface{}, error) { config := Config{ - AccountFile: d.Get("account_file").(string), - AccountFileContents: d.Get("account_file_contents").(string), - Project: d.Get("project").(string), - Region: d.Get("region").(string), + AccountFile: d.Get("account_file").(string), + Project: d.Get("project").(string), + Region: d.Get("region").(string), } if err := config.loadAndValidate(); err != nil { @@ -71,3 +69,28 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { return &config, nil } + +func validateAccountFile(v interface{}, k string) (warnings []string, errors []error) { + value := v.(string) + + if value == "" { + return + } + + var account accountFile + if err := json.Unmarshal([]byte(value), &account); err != nil { + warnings = append(warnings, ` +account_file is not valid JSON, so we are assuming it is a file path. This +support will be removed in the future. Please update your configuration to use +${file("filename.json")} instead.`) + + return + } + + if _, err := os.Stat(value); os.IsNotExist(err) { + errors = append(errors, err) + fmt.Errorf("account_file path does not exist: %s", value) + } + + return +} diff --git a/website/source/docs/providers/google/index.html.markdown b/website/source/docs/providers/google/index.html.markdown index 86745b8afb..3bbef84c60 100644 --- a/website/source/docs/providers/google/index.html.markdown +++ b/website/source/docs/providers/google/index.html.markdown @@ -19,7 +19,7 @@ Use the navigation to the left to read about the available resources. ``` # Configure the Google Cloud provider provider "google" { - account_file = "account.json" + account_file = "${file("account.json")}" project = "my-gce-project" region = "us-central1" } @@ -34,19 +34,13 @@ resource "google_compute_instance" "default" { The following keys can be used to configure the provider. -* `account_file` - (Required, unless `account_file_contents` is present) Path - to the JSON file used to describe your account credentials, downloaded from - Google Cloud Console. More details on retrieving this file are below. The - _account file_ can be "" if you are running terraform from a GCE instance with - a properly-configured [Compute Engine Service - Account](https://cloud.google.com/compute/docs/authentication). This can also - be specified with the `GOOGLE_ACCOUNT_FILE` shell environment variable. - -* `account_file_contents` - (Required, unless `account_file` is present) The - contents of `account_file`. This can be used to pass the account credentials - with a Terraform var or environment variable if the account file is not - accessible. This can also be specified with the `GOOGLE_ACCOUNT_FILE_CONTENTS` - shell environment variable. +* `account_file` - (Required) Contents of the JSON file used to describe your + account credentials, downloaded from Google Cloud Console. More details on + retrieving this file are below. The `account file` can be "" if you are running + terraform from a GCE instance with a properly-configured [Compute Engine + Service Account](https://cloud.google.com/compute/docs/authentication). This + can also be specified with the `GOOGLE_ACCOUNT_FILE` shell environment + variable. * `project` - (Required) The ID of the project to apply any resources to. This can also be specified with the `GOOGLE_PROJECT` shell environment variable. From 8faa11115625d024400d27b16616d108906ea69a Mon Sep 17 00:00:00 2001 From: Clint Shryock Date: Mon, 27 Jul 2015 17:06:48 -0400 Subject: [PATCH 7/9] providers/google: Return if we could parse JSON --- builtin/providers/google/provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index b99ac44b3d..af5ae03ce7 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -83,7 +83,7 @@ func validateAccountFile(v interface{}, k string) (warnings []string, errors []e account_file is not valid JSON, so we are assuming it is a file path. This support will be removed in the future. Please update your configuration to use ${file("filename.json")} instead.`) - + } else { return } From 4764a556c0258322acf4bd396198d0eb0d2f64bb Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Mon, 27 Jul 2015 17:07:38 -0400 Subject: [PATCH 8/9] providers/google: Fix error appending --- builtin/providers/google/provider.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/builtin/providers/google/provider.go b/builtin/providers/google/provider.go index af5ae03ce7..6dade8deaa 100644 --- a/builtin/providers/google/provider.go +++ b/builtin/providers/google/provider.go @@ -87,9 +87,12 @@ ${file("filename.json")} instead.`) return } - if _, err := os.Stat(value); os.IsNotExist(err) { - errors = append(errors, err) - fmt.Errorf("account_file path does not exist: %s", value) + if _, err := os.Stat(value); err != nil { + errors = append(errors, + fmt.Errorf( + "account_file path could not be read from '%s': %s", + value, + err)) } return From 7884456c4b8352bd5fa786f230002c4c1a3c5ae4 Mon Sep 17 00:00:00 2001 From: Justin Campbell Date: Tue, 28 Jul 2015 09:36:05 -0400 Subject: [PATCH 9/9] providers/google: Fix reading account_file path --- builtin/providers/google/config.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/providers/google/config.go b/builtin/providers/google/config.go index 409443fdba..838966528f 100644 --- a/builtin/providers/google/config.go +++ b/builtin/providers/google/config.go @@ -71,13 +71,13 @@ func (c *Config) loadAndValidate() error { } contents = string(b) - } - if err := parseJSON(&account, contents); err != nil { - return fmt.Errorf( - "Error parsing account file '%s': %s", - contents, - err) + if err := parseJSON(&account, contents); err != nil { + return fmt.Errorf( + "Error parsing account file '%s': %s", + contents, + err) + } } clientScopes := []string{