-
Notifications
You must be signed in to change notification settings - Fork 77
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
Config var data #109
Config var data #109
Changes from 5 commits
1fd31dc
86d6b8b
24bb822
aa96d42
44203d5
f3ef6e0
dfcc392
d7f9d87
2b9ccc1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ website/node_modules | |
*.iml | ||
*.test | ||
*.iml | ||
|
||
.vscode | ||
website/vendor | ||
|
||
# Test exclusions | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
package heroku | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/hashicorp/terraform/helper/schema" | ||
"github.com/heroku/heroku-go/v3" | ||
) | ||
|
||
func dataSourceHerokuAppConfigVars() *schema.Resource { | ||
return &schema.Resource{ | ||
Read: dataSourceHerokuAppConfigVarsRead, | ||
Schema: map[string]*schema.Schema{ | ||
"app": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
}, | ||
"all_config_vars": { | ||
Type: schema.TypeMap, | ||
Computed: true, | ||
}, | ||
"depends": { | ||
Type: schema.TypeList, | ||
Optional: true, | ||
Elem: &schema.Schema{ | ||
Type: schema.TypeString, | ||
}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func dataSourceHerokuAppConfigVarsRead(d *schema.ResourceData, m interface{}) error { | ||
client := m.(*heroku.Service) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If m doesn't hold a value of type *heroku.Service it will panic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its a valid point, that being said we aren't checking the type conversion anywhere else in the code base, so I wouldn't necessarily ding this PR for that. |
||
appName := d.Get("app").(string) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. https://github.com/terraform-providers/terraform-provider-heroku/blob/master/heroku/helper.go#L12 could use this helper here. |
||
d.SetId(appName) | ||
configVarInfo, err := client.ConfigVarInfoForApp(context.TODO(), appName) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if configVarInfo == nil { | ||
return fmt.Errorf(`Error getting config vars for %s`, appName) | ||
} | ||
configMap := make(map[string]string) | ||
|
||
for configKey, configValue := range configVarInfo { | ||
configMap[configKey] = *configValue | ||
} | ||
|
||
d.Set("all_config_vars", configMap) | ||
|
||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
package heroku | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform/helper/acctest" | ||
"github.com/hashicorp/terraform/helper/resource" | ||
"github.com/hashicorp/terraform/terraform" | ||
heroku "github.com/heroku/heroku-go/v3" | ||
) | ||
|
||
func TestAccDatasourceHerokuAppConfigVars(t *testing.T) { | ||
appName := fmt.Sprintf("tftest-app-%s", acctest.RandString(10)) | ||
|
||
resource.Test(t, resource.TestCase{ | ||
PreCheck: func() { | ||
testAccPreCheck(t) | ||
}, | ||
Providers: testAccProviders, | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: testAccCheckHerokuAppConfigVar(appName), | ||
Check: resource.ComposeTestCheckFunc( | ||
resource.TestCheckResourceAttr( | ||
"heroku_app.app", "name", appName), | ||
testAccCheckHerokuConfigVarExists( | ||
"data.heroku_app_config_vars.app_config_vars", appName, "HOSTEDGRAPHITE_APIKEY"), | ||
testAccCheckHerokuConfigVarExists( | ||
"heroku_app.app2", appName, "HOSTEDGRAPHITE_APIKEY"), | ||
), | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func testAccCheckHerokuAppConfigVar(appName string) string { | ||
return fmt.Sprintf(` | ||
resource "heroku_app" "app" { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can turn on gofmt in your ide to fix the formatting indentation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. format passes fine for me:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, I think its the backtick making it look weird, but seems gofmt happy |
||
name = "%s" | ||
region = "us" | ||
stack = "heroku-16" | ||
} | ||
|
||
resource "heroku_addon" "hostedgraphite" { | ||
app = "${heroku_app.app.name}" | ||
plan = "hostedgraphite:free" | ||
} | ||
|
||
data "heroku_app_config_vars" "app_config_vars" { | ||
app = "${heroku_app.app.name}" | ||
depends = ["${heroku_addon.hostedgraphite.app}"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not too happy about this, there's a bug somewhere in the terraform provider that breaks a data source when used in conjunction with depends_on. Instead I'm sticking a dummy attribute to stick inferred dependencies on other terraform resources, so the data resource waits until the addon is added, before trying to slap configs onto downstream resources (ie: app2 doing a lookup of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess I'm looking for advice on whether we want to document this depends attribute to reflect this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. voila, here's the issue I hit: hashicorp/terraform#11806 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @talbright @bernerdschaefer @joestump for another quick reivew, thanks in advance! |
||
} | ||
|
||
resource "heroku_app" "app2" { | ||
name = "%s-2" | ||
region = "us" | ||
stack = "heroku-16" | ||
config_vars { | ||
HOSTEDGRAPHITE_APIKEY = "${data.heroku_app_config_vars.app_config_vars.all_config_vars["HOSTEDGRAPHITE_APIKEY"]}" | ||
} | ||
} | ||
`, appName, appName) | ||
} | ||
|
||
func testAccCheckHerokuConfigVarExists(resourceName string, appName string, configVar string) resource.TestCheckFunc { | ||
return func(s *terraform.State) error { | ||
rootModule := s.RootModule() | ||
resource, ok := rootModule.Resources[resourceName] | ||
if !ok { | ||
return fmt.Errorf("Not found: %s", resourceName) | ||
} | ||
if resource.Primary.ID == "" { | ||
return fmt.Errorf("No resource id found %s", resourceName) | ||
} | ||
client := testAccProvider.Meta().(*heroku.Service) | ||
configVarInfo, err := client.ConfigVarInfoForApp(context.TODO(), appName) | ||
if err != nil { | ||
return err | ||
} | ||
if configVarInfo == nil { | ||
return fmt.Errorf("No config vars found for %s", resourceName) | ||
} | ||
configValue := resource.Primary.Attributes["all_config_vars."+configVar] | ||
if configValue != *configVarInfo[configVar] { | ||
return fmt.Errorf("Mismatched config value set %s, actual: %s expected: %s", configVar, configValue, *configVarInfo[configVar]) | ||
} | ||
//happy path, expected config key found, and value matches | ||
return nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
--- | ||
layout: "heroku" | ||
page_title: "Heroku: heroku_app_config_vars" | ||
sidebar_current: "docs-heroku-datasource-heroku_app_config_vars-x" | ||
description: |- | ||
Get latest config vars of a given heroku_app resource. | ||
--- | ||
|
||
# Data Source: heroku_app_config_vars | ||
|
||
Use this data source to get latest config_vars from a heroku app. This allows pull off config_vars addons attach to an app, or pulling off latest current config_var values without relying on remote state of last refresh of `all_config_vars` attribute on a given `heroku_app` resource. | ||
|
||
## Example Usage | ||
|
||
```hcl | ||
# Look up a Heroku Private Space's peering info. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess you copied this example hcl from somewhere else :P |
||
data "heroku_app_config_vars" "foo_configs" { | ||
app = "${heroku_app.foo.name}" | ||
} | ||
|
||
# Initiate a VPC peering connection request. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
resource "heroku_app" "foo" { | ||
name = "foo" | ||
organization = "foobars" | ||
region = "virginia" | ||
} | ||
``` | ||
|
||
## Argument Reference | ||
|
||
The following arguments are supported: | ||
|
||
* `app` - The name of the heroku app to pull config_vars off of | ||
|
||
## Attributes Reference | ||
|
||
The following attributes are exported: | ||
|
||
* `all_config_vars` - Map of config keys to values pulled off `app` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would consider changing
depends
to maybewait_for_resources
orwait_for
.depends
seems a bit too close todepends_on
and could confuse users.Additionally I think if we were to move forward with this attribute, I suggest changing the
Type
toschema.TypeSet
so that the ordering doesn't matter.