From d964627c45b82269d933bedc168a2789cc27feb9 Mon Sep 17 00:00:00 2001 From: David Ji Date: Thu, 4 Mar 2021 14:23:20 +0900 Subject: [PATCH 1/3] Implement mechanism to allow users to not set heroku_app.all_config_vars in state --- docs/index.md | 9 +++++- heroku/config.go | 50 +++++++++++++++++++++--------- heroku/provider.go | 15 +++++++++ heroku/resource_heroku_app.go | 12 +++++-- heroku/resource_heroku_app_test.go | 33 ++++++++++++++++++++ 5 files changed, 100 insertions(+), 19 deletions(-) diff --git a/docs/index.md b/docs/index.md index aa3a9196..a646154d 100644 --- a/docs/index.md +++ b/docs/index.md @@ -114,9 +114,16 @@ The following arguments are supported: provided, it will be sourced from the `HEROKU_HEADERS` environment variable (if set). +* `customizations` - (Optional) Various attributes altering the behavior of certain resources. + Only a single `customizations` block may be specified, and it supports the following arguments: + + * `set_app_all_config_vars_in_state` - (Optional) Controls whether the `heroku_app.all_config_vars` attribute + is set in the state file. Set to `false` if you'd like not to have non-managed config vars set in state. + Defaults to `true`. + * `delays` - (Optional) Delays help mitigate issues that can arise due to Heroku's eventually consistent data model. Only a single `delays` block may be - specified and it supports the following arguments: + specified, and it supports the following arguments: * `post_app_create_delay` - (Optional) The number of seconds to wait after an app is created. Default is to wait 5 seconds. diff --git a/heroku/config.go b/heroku/config.go index 23c4da5f..ecceffa7 100644 --- a/heroku/config.go +++ b/heroku/config.go @@ -23,22 +23,28 @@ const ( DefaultPostDomainCreateDelay = int64(5) // Default custom timeouts - DefaultAddonCreateTimeout = int64(20) + DefaultAddonCreateTimeout = int64(20) + DefaultSetAppAllConfigVarsInState = true ) type Config struct { - Api *heroku.Service - APIKey string - DebugHTTP bool - Email string - Headers http.Header + Api *heroku.Service + APIKey string + DebugHTTP bool + Email string + Headers http.Header + URL string + + // Delays PostAppCreateDelay int64 PostDomainCreateDelay int64 PostSpaceCreateDelay int64 - URL string - // Custom Timeouts + // Timeouts AddonCreateTimeout int64 + + // Customization + SetAppAllConfigVarsInState bool } func (c Config) String() string { @@ -48,11 +54,12 @@ func (c Config) String() string { func NewConfig() *Config { config := &Config{ - Headers: make(http.Header), - PostAppCreateDelay: DefaultPostAppCreateDelay, - PostDomainCreateDelay: DefaultPostDomainCreateDelay, - PostSpaceCreateDelay: DefaultPostSpaceCreateDelay, - AddonCreateTimeout: DefaultAddonCreateTimeout, + Headers: make(http.Header), + PostAppCreateDelay: DefaultPostAppCreateDelay, + PostDomainCreateDelay: DefaultPostDomainCreateDelay, + PostSpaceCreateDelay: DefaultPostSpaceCreateDelay, + AddonCreateTimeout: DefaultAddonCreateTimeout, + SetAppAllConfigVarsInState: DefaultSetAppAllConfigVarsInState, } if logging.IsDebugOrHigher() { config.DebugHTTP = true @@ -103,10 +110,23 @@ func (c *Config) applySchema(d *schema.ResourceData) (err error) { c.URL = url.(string) } + if v, ok := d.GetOk("customizations"); ok { + vL := v.([]interface{}) + if len(vL) > 1 { + return fmt.Errorf("Provider configuration error: only one customizations block is permitted") + } + for _, v := range vL { + delaysConfig := v.(map[string]interface{}) + if v, ok := delaysConfig["set_app_all_config_vars_in_state"].(bool); ok { + c.SetAppAllConfigVarsInState = v + } + } + } + if v, ok := d.GetOk("delays"); ok { vL := v.([]interface{}) if len(vL) > 1 { - return fmt.Errorf("Provider configuration error: only 1 delays config is permitted") + return fmt.Errorf("Provider configuration error: only one delays block is permitted") } for _, v := range vL { delaysConfig := v.(map[string]interface{}) @@ -125,7 +145,7 @@ func (c *Config) applySchema(d *schema.ResourceData) (err error) { if v, ok := d.GetOk("timeouts"); ok { vL := v.([]interface{}) if len(vL) > 1 { - return fmt.Errorf("provider configuration error: only 1 timeouts config is permitted") + return fmt.Errorf("provider configuration error: only one timeouts block is permitted") } for _, v := range vL { diff --git a/heroku/provider.go b/heroku/provider.go index 096314b5..89e6dcf0 100644 --- a/heroku/provider.go +++ b/heroku/provider.go @@ -36,6 +36,21 @@ func Provider() *schema.Provider { DefaultFunc: schema.EnvDefaultFunc("HEROKU_API_URL", heroku.DefaultURL), }, + "customizations": { + Type: schema.TypeList, + MaxItems: 1, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "set_app_all_config_vars_in_state": { + Type: schema.TypeBool, + Optional: true, + Default: DefaultSetAppAllConfigVarsInState, + }, + }, + }, + }, + "delays": { Type: schema.TypeList, MaxItems: 1, diff --git a/heroku/resource_heroku_app.go b/heroku/resource_heroku_app.go index 398d98ef..15436764 100644 --- a/heroku/resource_heroku_app.go +++ b/heroku/resource_heroku_app.go @@ -358,7 +358,8 @@ func setAppDetails(d *schema.ResourceData, app *application) (err error) { } func resourceHerokuAppRead(d *schema.ResourceData, meta interface{}) error { - client := meta.(*Config).Api + config := meta.(*Config) + client := config.Api care := make(map[string]struct{}) configVars := make(map[string]string) @@ -407,8 +408,13 @@ func resourceHerokuAppRead(d *schema.ResourceData, meta interface{}) error { log.Printf("[WARN] Error setting sensitive config vars: %s", err) } - if err := d.Set("all_config_vars", app.Vars); err != nil { - log.Printf("[WARN] Error setting all_config_vars: %s", err) + // Set `all_config_vars` to empty map initially. Only set this attribute + // if set_app_all_config_vars_in_state is `true`. + d.Set("all_config_vars", map[string]string{}) + if config.SetAppAllConfigVarsInState { + if err := d.Set("all_config_vars", app.Vars); err != nil { + log.Printf("[WARN] Error setting all_config_vars: %s", err) + } } buildpacksErr := d.Set("buildpacks", app.Buildpacks) diff --git a/heroku/resource_heroku_app_test.go b/heroku/resource_heroku_app_test.go index 8f7ef853..8e4d8035 100644 --- a/heroku/resource_heroku_app_test.go +++ b/heroku/resource_heroku_app_test.go @@ -35,6 +35,39 @@ func TestAccHerokuApp_Basic(t *testing.T) { "heroku_app.foobar", "config_vars.FOO", "bar"), resource.TestCheckResourceAttr( "heroku_app.foobar", "internal_routing", "false"), + resource.TestCheckResourceAttr( + "heroku_app.foobar", "all_config_vars.%", "1"), + ), + }, + }, + }) +} + +func TestAccHerokuApp_DontSetAllConfigVars(t *testing.T) { + var app heroku.App + appName := fmt.Sprintf("tftest-%s", acctest.RandString(10)) + appStack := "heroku-20" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckHerokuAppDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCheckHerokuAppConfig_basic(appName, appStack), + Check: resource.ComposeTestCheckFunc( + testAccCheckHerokuAppExists("heroku_app.foobar", &app), + testAccCheckHerokuAppAttributes(&app, appName, "heroku-20"), + resource.TestCheckResourceAttr( + "heroku_app.foobar", "name", appName), + resource.TestCheckResourceAttrSet( + "heroku_app.foobar", "uuid"), + resource.TestCheckResourceAttr( + "heroku_app.foobar", "config_vars.FOO", "bar"), + resource.TestCheckResourceAttr( + "heroku_app.foobar", "internal_routing", "false"), + resource.TestCheckResourceAttr( + "heroku_app.foobar", "all_config_vars.%", "0"), ), }, }, From 71ae9505f48c4739d0f716e4e0c272ab6ff72bc1 Mon Sep 17 00:00:00 2001 From: David Ji Date: Tue, 9 Mar 2021 11:04:04 +0900 Subject: [PATCH 2/3] improve doc wording for set_app_all_config_vars_in_state --- docs/index.md | 6 ++++-- docs/resources/app.md | 9 +++++---- heroku/config.go | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/docs/index.md b/docs/index.md index a646154d..55f0785a 100644 --- a/docs/index.md +++ b/docs/index.md @@ -118,8 +118,10 @@ The following arguments are supported: Only a single `customizations` block may be specified, and it supports the following arguments: * `set_app_all_config_vars_in_state` - (Optional) Controls whether the `heroku_app.all_config_vars` attribute - is set in the state file. Set to `false` if you'd like not to have non-managed config vars set in state. - Defaults to `true`. + is set in the state file. The aforementioned attribute stores a snapshot of all config vars in Terraform state, + even if they are not defined in Terraform. This means sensitive Heroku add-on config vars, + such as Postgres' `DATABASE_URL`, are always accessible in the state. + Set to `false` to only track managed config vars in the state. Defaults to `true`. * `delays` - (Optional) Delays help mitigate issues that can arise due to Heroku's eventually consistent data model. Only a single `delays` block may be diff --git a/docs/resources/app.md b/docs/resources/app.md index ebafb54a..0b7941b9 100644 --- a/docs/resources/app.md +++ b/docs/resources/app.md @@ -103,10 +103,11 @@ The following attributes are exported: at by default. * `heroku_hostname` - A hostname for the Heroku application, suitable for pointing DNS records. -* `all_config_vars` - A map of all of the configuration variables that - exist for the app, containing both those set by Terraform and those - set externally. (These are treated as "sensitive" so that - their values are redacted in console output.) +* `all_config_vars` - A map of all configuration variables that + exist for the app, containing both those set by Terraform and those + set externally. (These are treated as "sensitive" so that + their values are redacted in console output.) This attribute is not set in state if the `provider` + attribute `set_app_all_config_vars_in_state` is `false`. * `uuid` - The unique UUID of the Heroku app. **NOTE:** Use this for `null_resource` triggers. ## Import diff --git a/heroku/config.go b/heroku/config.go index ecceffa7..83108b40 100644 --- a/heroku/config.go +++ b/heroku/config.go @@ -116,8 +116,8 @@ func (c *Config) applySchema(d *schema.ResourceData) (err error) { return fmt.Errorf("Provider configuration error: only one customizations block is permitted") } for _, v := range vL { - delaysConfig := v.(map[string]interface{}) - if v, ok := delaysConfig["set_app_all_config_vars_in_state"].(bool); ok { + customizations := v.(map[string]interface{}) + if v, ok := customizations["set_app_all_config_vars_in_state"].(bool); ok { c.SetAppAllConfigVarsInState = v } } From da49baaecb481f78f891cb05d4e1691bb980ba8f Mon Sep 17 00:00:00 2001 From: David Ji Date: Tue, 9 Mar 2021 14:21:21 +0900 Subject: [PATCH 3/3] add test: TestAccHerokuApp_DontSetAllConfigVars --- heroku/provider_test.go | 30 ++++++++++++++++++++++++++++++ heroku/resource_heroku_app_test.go | 29 ++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/heroku/provider_test.go b/heroku/provider_test.go index f9373972..3c9d3aa4 100644 --- a/heroku/provider_test.go +++ b/heroku/provider_test.go @@ -13,15 +13,45 @@ import ( helper "github.com/heroku/terraform-provider-heroku/v4/helper/test" ) +const ( + ProviderNameHeroku = "heroku" +) + +var providers []*schema.Provider +var testAccProviderFactories map[string]func() (*schema.Provider, error) var testAccProviders map[string]*schema.Provider var testAccProvider *schema.Provider var testAccConfig *helper.TestConfig +func testAccProviderFactoriesInternal(providers []*schema.Provider) map[string]func() (*schema.Provider, error) { + return testAccProviderFactoriesInit(providers, []string{ProviderNameHeroku}) +} + +// testAccProviderFactoriesInit creates ProviderFactories for the provider under testing. +func testAccProviderFactoriesInit(providers []*schema.Provider, providerNames []string) map[string]func() (*schema.Provider, error) { + var factories = make(map[string]func() (*schema.Provider, error), len(providerNames)) + + for _, name := range providerNames { + p := Provider() + + factories[name] = func() (*schema.Provider, error) { + return p, nil + } + + if providers != nil { + providers = append(providers, p) + } + } + + return factories +} + func init() { testAccProvider = Provider() testAccProviders = map[string]*schema.Provider{ "heroku": testAccProvider, } + testAccProviderFactories = testAccProviderFactoriesInit(providers, []string{ProviderNameHeroku}) testAccConfig = helper.NewTestConfig() } diff --git a/heroku/resource_heroku_app_test.go b/heroku/resource_heroku_app_test.go index 8e4d8035..a0bd0965 100644 --- a/heroku/resource_heroku_app_test.go +++ b/heroku/resource_heroku_app_test.go @@ -44,20 +44,16 @@ func TestAccHerokuApp_Basic(t *testing.T) { } func TestAccHerokuApp_DontSetAllConfigVars(t *testing.T) { - var app heroku.App appName := fmt.Sprintf("tftest-%s", acctest.RandString(10)) appStack := "heroku-20" resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - CheckDestroy: testAccCheckHerokuAppDestroy, + PreCheck: func() { testAccPreCheck(t) }, + ProviderFactories: testAccProviderFactories, Steps: []resource.TestStep{ { - Config: testAccCheckHerokuAppConfig_basic(appName, appStack), + Config: testAccCheckHerokuAppConfig_DontSetConfigVars(appName, appStack), Check: resource.ComposeTestCheckFunc( - testAccCheckHerokuAppExists("heroku_app.foobar", &app), - testAccCheckHerokuAppAttributes(&app, appName, "heroku-20"), resource.TestCheckResourceAttr( "heroku_app.foobar", "name", appName), resource.TestCheckResourceAttrSet( @@ -985,3 +981,22 @@ resource "heroku_app" "foobar" { } }`, appName, org, locked) } + +func testAccCheckHerokuAppConfig_DontSetConfigVars(appName, appStack string) string { + return fmt.Sprintf(` +provider "heroku" { + customizations { + set_app_all_config_vars_in_state = false + } +} + +resource "heroku_app" "foobar" { + name = "%s" + stack = "%s" + region = "us" + + config_vars = { + FOO = "bar" + } +}`, appName, appStack) +}