From 81779aa1d4f094a45ffd1d45986d4925970e8c08 Mon Sep 17 00:00:00 2001 From: Jakub Janczak Date: Sun, 14 Dec 2014 18:54:01 +0100 Subject: [PATCH 1/2] Fixing the situation when you've got an organization app, and want to have it in a private area instead --- .../providers/heroku/resource_heroku_app.go | 87 +++++++++++++++---- 1 file changed, 70 insertions(+), 17 deletions(-) diff --git a/builtin/providers/heroku/resource_heroku_app.go b/builtin/providers/heroku/resource_heroku_app.go index 4c2f3bf97ab0..a6672bd0dfb9 100644 --- a/builtin/providers/heroku/resource_heroku_app.go +++ b/builtin/providers/heroku/resource_heroku_app.go @@ -9,13 +9,24 @@ import ( "github.com/hashicorp/terraform/helper/schema" ) +type herokuApplication struct { + Name string + Region string + Stack string + GitURL string + WebURL string + OrganizationName string + Locked bool +} + // type application is used to store all the details of a heroku app type application struct { Id string // Id of the resource - App *heroku.App // The heroku application - Client *heroku.Service // Client to interact with the heroku API - Vars map[string]string // The vars on the application + App *herokuApplication // The heroku application + Client *heroku.Service // Client to interact with the heroku API + Vars map[string]string // The vars on the application + Organization bool // is the application organization app } // Updates the application to have the latest from remote @@ -23,9 +34,37 @@ func (a *application) Update() error { var errs []error var err error - a.App, err = a.Client.AppInfo(a.Id) - if err != nil { - errs = append(errs, err) + if !a.Organization { + app, err := a.Client.AppInfo(a.Id) + if err != nil { + errs = append(errs, err) + } else { + a.App = &herokuApplication{} + a.App.Name = app.Name + a.App.Region = app.Region.Name + a.App.Stack = app.Stack.Name + a.App.GitURL = app.GitURL + a.App.WebURL = app.WebURL + } + } else { + app, err := a.Client.OrganizationAppInfo(a.Id) + if err != nil { + errs = append(errs, err) + } else { + // No inheritance between OrganizationApp and App is killing it :/ + a.App = &herokuApplication{} + a.App.Name = app.Name + a.App.Region = app.Region.Name + a.App.Stack = app.Stack.Name + a.App.GitURL = app.GitURL + a.App.WebURL = app.WebURL + if app.Organization != nil { + a.App.OrganizationName = app.Organization.Name + } else { + log.Println("[DEBUG] Something is wrong - didn't get information about organization name, while the app is marked as being so") + } + a.App.Locked = app.Locked + } } a.Vars, err = retrieveConfigVars(a.Id, a.Client) @@ -122,13 +161,18 @@ func resourceHerokuApp() *schema.Resource { } } +func isOrganizationApp(d *schema.ResourceData) bool { + _, ok := d.GetOk("organization.0.name") + return ok +} + 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 _, ok := d.GetOk("organization.0.name"); ok { + if isOrganizationApp(d) { return resourceHerokuOrgAppCreate(d, meta) } else { return resourceHerokuAppCreate(d, meta) @@ -236,13 +280,7 @@ func resourceHerokuOrgAppCreate(d *schema.ResourceData, meta interface{}) error func resourceHerokuAppRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*heroku.Service) - app, err := resourceHerokuAppRetrieve(d.Id(), client) - if err != nil { - return err - } - // Only set the config_vars that we have set in the configuration. - // The "all_config_vars" field has all of them. configVars := make(map[string]string) care := make(map[string]struct{}) for _, v := range d.Get("config_vars").([]interface{}) { @@ -250,6 +288,15 @@ func resourceHerokuAppRead(d *schema.ResourceData, meta interface{}) error { care[k] = struct{}{} } } + + _, organizationApp := d.GetOk("organization.0.name") + // 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) + if err != nil { + return err + } + for k, v := range app.Vars { if _, ok := care[k]; ok { configVars[k] = v @@ -261,12 +308,18 @@ func resourceHerokuAppRead(d *schema.ResourceData, meta interface{}) error { } d.Set("name", app.App.Name) - d.Set("stack", app.App.Stack.Name) - d.Set("region", app.App.Region.Name) + d.Set("stack", app.App.Stack) + d.Set("region", app.App.Region) d.Set("git_url", app.App.GitURL) d.Set("web_url", app.App.WebURL) 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) + } // We know that the hostname on heroku will be the name+herokuapp.com // You need this to do things like create DNS CNAME records @@ -327,8 +380,8 @@ func resourceHerokuAppDelete(d *schema.ResourceData, meta interface{}) error { return nil } -func resourceHerokuAppRetrieve(id string, client *heroku.Service) (*application, error) { - app := application{Id: id, Client: client} +func resourceHerokuAppRetrieve(id string, organization bool, client *heroku.Service) (*application, error) { + app := application{Id: id, Client: client, Organization: organization} err := app.Update() From c52765417abeba770bf0ba9fd60370bf3443ae38 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Mon, 4 Jan 2016 12:46:52 -0600 Subject: [PATCH 2/2] provider/heroku: add acctest covering orgs; fixup issues Switching up ResourceData interaction to not reach into the internal dot-notation nesting. --- .../providers/heroku/resource_heroku_app.go | 51 ++++--- .../heroku/resource_heroku_app_test.go | 129 ++++++++++++++++-- 2 files changed, 146 insertions(+), 34 deletions(-) diff --git a/builtin/providers/heroku/resource_heroku_app.go b/builtin/providers/heroku/resource_heroku_app.go index a6672bd0dfb9..d8d5b316b3ce 100644 --- a/builtin/providers/heroku/resource_heroku_app.go +++ b/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 diff --git a/builtin/providers/heroku/resource_heroku_app_test.go b/builtin/providers/heroku/resource_heroku_app_test.go index 185d4b7d708e..17bb7afd1450 100644 --- a/builtin/providers/heroku/resource_heroku_app_test.go +++ b/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" + } }`