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.

Sign up for GitHub

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

Jump to bottom

Read more default envvars for GCP #6114

Merged
merged 1 commit into from
Apr 11, 2016
Merged

Read more default envvars for GCP #6114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions builtin/providers/google/provider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,29 @@ func Provider() terraform.ResourceProvider {
DefaultFunc: schema.MultiEnvDefaultFunc([]string{
"GOOGLE_CREDENTIALS",
"GOOGLE_CLOUD_KEYFILE_JSON",
"GCLOUD_KEYFILE_JSON",
}, nil),
ValidateFunc: validateCredentials,
},

"project": &schema.Schema{
Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.EnvDefaultFunc("GOOGLE_PROJECT", ""),
Type: schema.TypeString,
Optional: true,
DefaultFunc: schema.MultiEnvDefaultFunc([]string{
"GOOGLE_PROJECT",
"GCLOUD_PROJECT",
"CLOUDSDK_CORE_PROJECT",
}, nil),
},

"region": &schema.Schema{
Type: schema.TypeString,
Required: true,
DefaultFunc: schema.EnvDefaultFunc("GOOGLE_REGION", nil),
Type: schema.TypeString,
Required: true,
DefaultFunc: schema.MultiEnvDefaultFunc([]string{
"GOOGLE_REGION",
"GCLOUD_REGION",
"CLOUDSDK_COMPUTE_REGION",
}, nil),
},
},

Expand Down
37 changes: 30 additions & 7 deletions builtin/providers/google/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package google
import (
"io/ioutil"
"os"
"strings"
"testing"

"github.com/hashicorp/terraform/helper/schema"
Expand Down Expand Up @@ -38,18 +39,40 @@ func testAccPreCheck(t *testing.T) {
os.Setenv("GOOGLE_CREDENTIALS", string(creds))
}

if v := os.Getenv("GOOGLE_CREDENTIALS"); v == "" {
if w := os.Getenv("GOOGLE_CLOUD_KEYFILE_JSON"); w == "" {
t.Fatal("GOOGLE_CREDENTIALS or GOOGLE_CLOUD_KEYFILE_JSON must be set for acceptance tests")
multiEnvSearch := func(ks []string) string {
for _, k := range ks {
if v := os.Getenv(k); v != "" {
return v
}
}
return ""
}

if v := os.Getenv("GOOGLE_PROJECT"); v == "" {
t.Fatal("GOOGLE_PROJECT must be set for acceptance tests")
creds := []string{
"GOOGLE_CREDENTIALS",
"GOOGLE_CLOUD_KEYFILE_JSON",
"GCLOUD_KEYFILE_JSON",
}
if v := multiEnvSearch(creds); v == "" {
t.Fatalf("One of %s must be set for acceptance tests", strings.Join(creds, ", "))
}

if v := os.Getenv("GOOGLE_REGION"); v != "us-central1" {
t.Fatal("GOOGLE_REGION must be set to us-central1 for acceptance tests")
projs := []string{
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the difference between projs & creds? can't we set a global constant for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @stack72

  • creds - places to look for credentials json
  • projs - places to look for project
  • regs - places to look for regions

We could make this a constant and share it between the provider and provider_test, but that wasn't a pattern I saw elsewhere in the codebase and didn't want to introduce a new paradigm. This is just the test file searching for the place to pull in creds from, and it arguably could remain unchanged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense not to break the formulae :)

"GOOGLE_PROJECT",
"GCLOUD_PROJECT",
"CLOUDSDK_CORE_PROJECT",
}
if v := multiEnvSearch(projs); v == "" {
t.Fatalf("One of %s must be set for acceptance tests", strings.Join(creds, ", "))
}

regs := []string{
"GOOGLE_REGION",
"GCLOUD_REGION",
"CLOUDSDK_COMPUTE_REGION",
}
if v := multiEnvSearch(regs); v != "us-central-1" {
t.Fatalf("One of %s must be set to us-central-1 for acceptance tests", strings.Join(creds, ", "))
}
}

Expand Down
27 changes: 19 additions & 8 deletions website/source/docs/providers/google/index.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,28 @@ The following keys can be used to configure the provider.
retrieving this file are below. Credentials may be blank 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_CREDENTIALS` or `GOOGLE_CLOUD_KEYFILE_JSON`
shell environment variable, containing the contents of the credentials file.
can also be specified using any of the following environment variables
(listed in order of precedence):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you point to the docs where you found these requirements here? It will make it clearer why Terraform allows you to configure your credentials so many different ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lwander the docs are kinda all over the place:

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, darn, oh well. I guess adding those would just make it more confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, (other cloud) had this problem and eventually someone made an authoritative decision and said "these are the envvars, everything else is wrong!" 😄


* `GOOGLE_CREDENTIALS`
* `GOOGLE_CLOUD_KEYFILE_JSON`
* `GCLOUD_KEYFILE_JSON`

* `project` - (Required) The ID of the project to apply any resources to. This
can be specified using any of the following environment variables (listed in
order of precedence):

* `GOOGLE_PROJECT`
* `GCLOUD_PROJECT`
* `CLOUDSDK_CORE_PROJECT`

* `region` - (Required) The region to operate under. This can also be specified
with the `GOOGLE_REGION` shell environment variable.
using any of the following environment variables (listed in order of
precedence):

* `project` - (Optional) The ID of the project to apply resources in. This
can also be specified with the `GOOGLE_PROJECT` shell environment variable.
If unspecified, users will need to specify the `project` attribute for
all resources. If specified, resources which do not depend on a project will
ignore this value.
* `GOOGLE_REGION`
* `GCLOUD_REGION`
* `CLOUDSDK_COMPUTE_REGION`

The following keys are supported for backwards compatibility, and may be
removed in a future version:
Expand Down