provider/heroku: add acctest covering orgs; fixup issues

Switching up ResourceData interaction to not reach into the internal
dot-notation nesting.

Author: phinze

builtin/providers/heroku/resource_heroku_app.go

@@ -9,6 +9,9 @@ import (
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
+// herokuApplication is a value type used to hold the details of an
+// application. We use this for common storage of values needed for the
+// heroku.App and heroku.OrganizationApp types
 type herokuApplication struct {
 	Name             string
 	Region           string
@@ -134,10 +137,9 @@ func resourceHerokuApp() *schema.Resource {
 			},
 
 			"organization": &schema.Schema{
-				Description: "Name of Organization to create application in. Leave blank for personal apps.",
-				Type:        schema.TypeList,
-				Optional:    true,
-				ForceNew:    true,
+				Type:     schema.TypeList,
+				Optional: true,
+				ForceNew: true,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"name": &schema.Schema{
@@ -162,21 +164,16 @@ func resourceHerokuApp() *schema.Resource {
 }
 
 func isOrganizationApp(d *schema.ResourceData) bool {
-	_, ok := d.GetOk("organization.0.name")
-	return ok
+	v := d.Get("organization").([]interface{})
+	return len(v) > 0 && v[0] != nil
 }
 
 func switchHerokuAppCreate(d *schema.ResourceData, meta interface{}) error {
-	orgCount := d.Get("organization.#").(int)
-	if orgCount > 1 {
-		return fmt.Errorf("Error Creating Heroku App: Only 1 Heroku Organization is permitted")
-	}
-
 	if isOrganizationApp(d) {
 		return resourceHerokuOrgAppCreate(d, meta)
-	} else {
-		return resourceHerokuAppCreate(d, meta)
 	}
+
+	return resourceHerokuAppCreate(d, meta)
 }
 
 func resourceHerokuAppCreate(d *schema.ResourceData, meta interface{}) error {
@@ -225,19 +222,25 @@ func resourceHerokuOrgAppCreate(d *schema.ResourceData, meta interface{}) error
 	// Build up our creation options
 	opts := heroku.OrganizationAppCreateOpts{}
 
-	if v := d.Get("organization.0.name"); v != nil {
+	v := d.Get("organization").([]interface{})
+	if len(v) > 1 {
+		return fmt.Errorf("Error Creating Heroku App: Only 1 Heroku Organization is permitted")
+	}
+	orgDetails := v[0].(map[string]interface{})
+
+	if v := orgDetails["name"]; v != nil {
 		vs := v.(string)
 		log.Printf("[DEBUG] Organization name: %s", vs)
 		opts.Organization = &vs
 	}
 
-	if v := d.Get("organization.0.personal"); v != nil {
+	if v := orgDetails["personal"]; v != nil {
 		vs := v.(bool)
 		log.Printf("[DEBUG] Organization Personal: %t", vs)
 		opts.Personal = &vs
 	}
 
-	if v := d.Get("organization.0.locked"); v != nil {
+	if v := orgDetails["locked"]; v != nil {
 		vs := v.(bool)
 		log.Printf("[DEBUG] Organization locked: %t", vs)
 		opts.Locked = &vs
@@ -289,7 +292,8 @@ func resourceHerokuAppRead(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
-	_, organizationApp := d.GetOk("organization.0.name")
+	organizationApp := isOrganizationApp(d)
+
 	// Only set the config_vars that we have set in the configuration.
 	// The "all_config_vars" field has all of them.
 	app, err := resourceHerokuAppRetrieve(d.Id(), organizationApp, client)
@@ -315,10 +319,15 @@ func resourceHerokuAppRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("config_vars", configVarsValue)
 	d.Set("all_config_vars", app.Vars)
 	if organizationApp {
-		d.Set("organization.#", "1")
-		d.Set("organization.0.name", app.App.OrganizationName)
-		d.Set("organization.0.locked", app.App.Locked)
-		d.Set("organization.0.private", false)
+		orgDetails := map[string]interface{}{
+			"name":    app.App.OrganizationName,
+			"locked":  app.App.Locked,
+			"private": false,
+		}
+		err := d.Set("organization", []interface{}{orgDetails})
+		if err != nil {
+			return err
+		}
 	}
 
 	// We know that the hostname on heroku will be the name+herokuapp.com

builtin/providers/heroku/resource_heroku_app_test.go

@@ -2,6 +2,7 @@ package heroku
 
 import (
 	"fmt"
+	"os"
 	"testing"
 
 	"github.com/cyberdelia/heroku-go/v3"
@@ -102,6 +103,31 @@ func TestAccHerokuApp_NukeVars(t *testing.T) {
 	})
 }
 
+func TestAccHerokuApp_Organization(t *testing.T) {
+	var app heroku.OrganizationApp
+	org := os.Getenv("HEROKU_ORGANIZATION")
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+			if org == "" {
+				t.Skip("HEROKU_ORGANIZATION is not set; skipping test.")
+			}
+		},
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckHerokuAppDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: fmt.Sprintf(testAccCheckHerokuAppConfig_organization, org),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckHerokuAppExistsOrg("heroku_app.foobar", &app),
+					testAccCheckHerokuAppAttributesOrg(&app, org),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckHerokuAppDestroy(s *terraform.State) error {
 	client := testAccProvider.Meta().(*heroku.Service)
 
@@ -197,6 +223,39 @@ func testAccCheckHerokuAppAttributesNoVars(app *heroku.App) resource.TestCheckFu
 	}
 }
 
+func testAccCheckHerokuAppAttributesOrg(app *heroku.OrganizationApp, org string) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		client := testAccProvider.Meta().(*heroku.Service)
+
+		if app.Region.Name != "us" {
+			return fmt.Errorf("Bad region: %s", app.Region.Name)
+		}
+
+		if app.Stack.Name != "cedar-14" {
+			return fmt.Errorf("Bad stack: %s", app.Stack.Name)
+		}
+
+		if app.Name != "terraform-test-app" {
+			return fmt.Errorf("Bad name: %s", app.Name)
+		}
+
+		if app.Organization == nil || app.Organization.Name != org {
+			return fmt.Errorf("Bad org: %v", app.Organization)
+		}
+
+		vars, err := client.ConfigVarInfo(app.Name)
+		if err != nil {
+			return err
+		}
+
+		if vars["FOO"] != "bar" {
+			return fmt.Errorf("Bad config vars: %v", vars)
+		}
+
+		return nil
+	}
+}
+
 func testAccCheckHerokuAppExists(n string, app *heroku.App) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[n]
@@ -227,29 +286,73 @@ func testAccCheckHerokuAppExists(n string, app *heroku.App) resource.TestCheckFu
 	}
 }
 
+func testAccCheckHerokuAppExistsOrg(n string, app *heroku.OrganizationApp) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[n]
+
+		if !ok {
+			return fmt.Errorf("Not found: %s", n)
+		}
+
+		if rs.Primary.ID == "" {
+			return fmt.Errorf("No App Name is set")
+		}
+
+		client := testAccProvider.Meta().(*heroku.Service)
+
+		foundApp, err := client.OrganizationAppInfo(rs.Primary.ID)
+
+		if err != nil {
+			return err
+		}
+
+		if foundApp.Name != rs.Primary.ID {
+			return fmt.Errorf("App not found")
+		}
+
+		*app = *foundApp
+
+		return nil
+	}
+}
+
 const testAccCheckHerokuAppConfig_basic = `
 resource "heroku_app" "foobar" {
-	name = "terraform-test-app"
-	region = "us"
+  name   = "terraform-test-app"
+  region = "us"
 
-	config_vars {
-		FOO = "bar"
-	}
+  config_vars {
+    FOO = "bar"
+  }
 }`
 
 const testAccCheckHerokuAppConfig_updated = `
 resource "heroku_app" "foobar" {
-	name = "terraform-test-renamed"
-	region = "us"
+  name   = "terraform-test-renamed"
+  region = "us"
 
-	config_vars {
-		FOO = "bing"
-		BAZ = "bar"
-	}
+  config_vars {
+    FOO = "bing"
+    BAZ = "bar"
+  }
 }`
 
 const testAccCheckHerokuAppConfig_no_vars = `
 resource "heroku_app" "foobar" {
-	name = "terraform-test-app"
-	region = "us"
+  name   = "terraform-test-app"
+  region = "us"
+}`
+
+const testAccCheckHerokuAppConfig_organization = `
+resource "heroku_app" "foobar" {
+  name   = "terraform-test-app"
+  region = "us"
+
+  organization {
+    name = "%s"
+  }
+
+  config_vars {
+    FOO = "bar"
+  }
 }`