Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

providers/google: Change account_file to expect a JSON string #2839

Merged
merged 9 commits into from
Jul 29, 2015
45 changes: 32 additions & 13 deletions builtin/providers/google/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -36,7 +38,6 @@ 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")
}
Expand All @@ -50,11 +51,33 @@ func (c *Config) loadAndValidate() error {
var client *http.Client

if c.AccountFile != "" {
if err := loadJSON(&account, c.AccountFile); err != nil {
return fmt.Errorf(
"Error loading account file '%s': %s",
c.AccountFile,
err)
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)

if err := parseJSON(&account, contents); err != nil {
return fmt.Errorf(
"Error parsing account file '%s': %s",
contents,
err)
}
}

clientScopes := []string{
Expand Down Expand Up @@ -146,13 +169,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)
}
50 changes: 38 additions & 12 deletions builtin/providers/google/config_test.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,50 @@
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_accountFilePath(t *testing.T) {
config := Config{
AccountFile: testFakeAccountFilePath,
Project: "my-gce-project",
Region: "us-central1",
}

err := config.loadAndValidate()
if err != nil {
t.Fatalf("error: %v", err)
}
}

func TestConfigLoadAndValidate_accountFileJSON(t *testing.T) {
contents, err := ioutil.ReadFile(testFakeAccountFilePath)
if err != nil {
t.Fatalf("error: %v", err)
}
config := Config{
AccountFile: string(contents),
Project: "my-gce-project",
Region: "us-central1",
}

expected := accountFile{
PrivateKeyId: "foo",
PrivateKey: "bar",
ClientEmail: "[email protected]",
ClientId: "[email protected]",
err = config.loadAndValidate()
if err != nil {
t.Fatalf("error: %v", err)
}
}

func TestConfigLoadAndValidate_accountFileJSONInvalid(t *testing.T) {
config := Config{
AccountFile: "{this is not json}",
Project: "my-gce-project",
Region: "us-central1",
}

if !reflect.DeepEqual(actual, expected) {
t.Fatalf("bad: %#v", actual)
if config.loadAndValidate() == nil {
t.Fatalf("expected error, but got nil")
}
}
39 changes: 36 additions & 3 deletions builtin/providers/google/provider.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package google

import (
"encoding/json"
"fmt"
"os"

"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/terraform"
)
Expand All @@ -10,9 +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", nil),
Type: schema.TypeString,
Required: true,
DefaultFunc: schema.EnvDefaultFunc("GOOGLE_ACCOUNT_FILE", nil),
ValidateFunc: validateAccountFile,
},

"project": &schema.Schema{
Expand Down Expand Up @@ -64,3 +69,31 @@ 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to detect if value here is a JSON file, or the contents of a file. If I supply somefile.json or even just the word what, this will throw an error:

invalid character 'w' looking for beginning of value

I'm thinking we shouldn't return in this error block, instead return if there is no error (we found json, yay)

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.`)
} else {
return
}

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
}
12 changes: 6 additions & 6 deletions website/source/docs/providers/google/index.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand All @@ -34,12 +34,12 @@ 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_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
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
Expand Down