From a1935a135d2acfdf445c09d993a237d63a4663fc Mon Sep 17 00:00:00 2001 From: Andrew Hodges Date: Thu, 14 Jan 2016 10:50:15 -0500 Subject: [PATCH] Handle external state changes for Packet resources gracefully. When a Packet provision exceeds our time limit, we move the device to an internal project for Packet staff to investigate. When this happens, the original user no longer has access to the device, and they get a 403. These changes make that and other external state changes more pleasant for users of Terraform. --- builtin/providers/packet/config.go | 2 +- builtin/providers/packet/errors.go | 43 +++++++ builtin/providers/packet/provider.go | 3 +- .../packet/resource_packet_device.go | 121 ++++++++---------- .../packet/resource_packet_project.go | 24 ++-- .../packet/resource_packet_project_test.go | 13 +- .../packet/resource_packet_ssh_key.go | 20 +-- .../packet/resource_packet_ssh_key_test.go | 13 +- 8 files changed, 119 insertions(+), 120 deletions(-) create mode 100644 builtin/providers/packet/errors.go diff --git a/builtin/providers/packet/config.go b/builtin/providers/packet/config.go index bce54bf48..92d0c22af 100644 --- a/builtin/providers/packet/config.go +++ b/builtin/providers/packet/config.go @@ -13,7 +13,7 @@ type Config struct { AuthToken string } -// Client() returns a new client for accessing packet. +// Client() returns a new client for accessing Packet's API. func (c *Config) Client() *packngo.Client { return packngo.NewClient(consumerToken, c.AuthToken, cleanhttp.DefaultClient()) } diff --git a/builtin/providers/packet/errors.go b/builtin/providers/packet/errors.go new file mode 100644 index 000000000..1c19dc4d9 --- /dev/null +++ b/builtin/providers/packet/errors.go @@ -0,0 +1,43 @@ +package packet + +import ( + "net/http" + "strings" + + "github.com/packethost/packngo" +) + +func friendlyError(err error) error { + if e, ok := err.(*packngo.ErrorResponse); ok { + return &ErrorResponse{ + StatusCode: e.Response.StatusCode, + Errors: Errors(e.Errors), + } + } + return err +} + +func isForbidden(err error) bool { + if r, ok := err.(*ErrorResponse); ok { + return r.StatusCode == http.StatusForbidden + } + return false +} + +func isNotFound(err error) bool { + if r, ok := err.(*ErrorResponse); ok { + return r.StatusCode == http.StatusNotFound + } + return false +} + +type Errors []string + +func (e Errors) Error() string { + return strings.Join(e, "; ") +} + +type ErrorResponse struct { + StatusCode int + Errors +} diff --git a/builtin/providers/packet/provider.go b/builtin/providers/packet/provider.go index c1efd6e83..82f7dbf77 100644 --- a/builtin/providers/packet/provider.go +++ b/builtin/providers/packet/provider.go @@ -5,7 +5,7 @@ import ( "github.com/hashicorp/terraform/terraform" ) -// Provider returns a schema.Provider for Packet. +// Provider returns a schema.Provider for managing Packet infrastructure. func Provider() terraform.ResourceProvider { return &schema.Provider{ Schema: map[string]*schema.Schema{ @@ -31,6 +31,5 @@ func providerConfigure(d *schema.ResourceData) (interface{}, error) { config := Config{ AuthToken: d.Get("auth_token").(string), } - return config.Client(), nil } diff --git a/builtin/providers/packet/resource_packet_device.go b/builtin/providers/packet/resource_packet_device.go index c7cd777a2..23775bde3 100644 --- a/builtin/providers/packet/resource_packet_device.go +++ b/builtin/providers/packet/resource_packet_device.go @@ -1,8 +1,8 @@ package packet import ( + "errors" "fmt" - "log" "time" "github.com/hashicorp/terraform/helper/resource" @@ -146,22 +146,23 @@ func resourcePacketDeviceCreate(d *schema.ResourceData, meta interface{}) error } } - log.Printf("[DEBUG] Device create configuration: %#v", createRequest) - newDevice, _, err := client.Devices.Create(createRequest) if err != nil { - return fmt.Errorf("Error creating device: %s", err) + return friendlyError(err) } - // Assign the device id d.SetId(newDevice.ID) - log.Printf("[INFO] Device ID: %s", d.Id()) - - _, err = WaitForDeviceAttribute(d, "active", []string{"queued", "provisioning"}, "state", meta) + // Wait for the device so we can get the networking attributes that show up after a while. + _, err = waitForDeviceAttribute(d, "active", []string{"queued", "provisioning"}, "state", meta) if err != nil { - return fmt.Errorf( - "Error waiting for device (%s) to become ready: %s", d.Id(), err) + if isForbidden(err) { + // If the device doesn't get to the active state, we can't recover it from here. + d.SetId("") + + return errors.New("provisioning time limit exceeded; the Packet team will investigate") + } + return err } return resourcePacketDeviceRead(d, meta) @@ -170,10 +171,17 @@ func resourcePacketDeviceCreate(d *schema.ResourceData, meta interface{}) error func resourcePacketDeviceRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*packngo.Client) - // Retrieve the device properties for updating the state device, _, err := client.Devices.Get(d.Id()) if err != nil { - return fmt.Errorf("Error retrieving device: %s", err) + err = friendlyError(err) + + // If the device somehow already destroyed, mark as succesfully gone. + if isNotFound(err) { + d.SetId("") + return nil + } + + return err } d.Set("name", device.Hostname) @@ -186,35 +194,36 @@ func resourcePacketDeviceRead(d *schema.ResourceData, meta interface{}) error { d.Set("created", device.Created) d.Set("updated", device.Updated) - tags := make([]string, 0) + tags := make([]string, 0, len(device.Tags)) for _, tag := range device.Tags { tags = append(tags, tag) } d.Set("tags", tags) - provisionerAddress := "" - - networks := make([]map[string]interface{}, 0, 1) + var ( + host string + networks = make([]map[string]interface{}, 0, 1) + ) for _, ip := range device.Network { - network := make(map[string]interface{}) - network["address"] = ip.Address - network["gateway"] = ip.Gateway - network["family"] = ip.Family - network["cidr"] = ip.Cidr - network["public"] = ip.Public + network := map[string]interface{}{ + "address": ip.Address, + "gateway": ip.Gateway, + "family": ip.Family, + "cidr": ip.Cidr, + "public": ip.Public, + } networks = append(networks, network) + if ip.Family == 4 && ip.Public == true { - provisionerAddress = ip.Address + host = ip.Address } } d.Set("network", networks) - log.Printf("[DEBUG] Provisioner Address set to %v", provisionerAddress) - - if provisionerAddress != "" { + if host != "" { d.SetConnInfo(map[string]string{ "type": "ssh", - "host": provisionerAddress, + "host": host, }) } @@ -224,19 +233,15 @@ func resourcePacketDeviceRead(d *schema.ResourceData, meta interface{}) error { func resourcePacketDeviceUpdate(d *schema.ResourceData, meta interface{}) error { client := meta.(*packngo.Client) - if d.HasChange("locked") && d.Get("locked").(bool) { - _, err := client.Devices.Lock(d.Id()) - - if err != nil { - return fmt.Errorf( - "Error locking device (%s): %s", d.Id(), err) + if d.HasChange("locked") { + var action func(string) (*packngo.Response, error) + if d.Get("locked").(bool) { + action = client.Devices.Lock + } else { + action = client.Devices.Unlock } - } else if d.HasChange("locked") { - _, err := client.Devices.Unlock(d.Id()) - - if err != nil { - return fmt.Errorf( - "Error unlocking device (%s): %s", d.Id(), err) + if _, err := action(d.Id()); err != nil { + return friendlyError(err) } } @@ -246,22 +251,14 @@ func resourcePacketDeviceUpdate(d *schema.ResourceData, meta interface{}) error func resourcePacketDeviceDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*packngo.Client) - log.Printf("[INFO] Deleting device: %s", d.Id()) if _, err := client.Devices.Delete(d.Id()); err != nil { - return fmt.Errorf("Error deleting device: %s", err) + return friendlyError(err) } return nil } -func WaitForDeviceAttribute( - d *schema.ResourceData, target string, pending []string, attribute string, meta interface{}) (interface{}, error) { - // Wait for the device so we can get the networking attributes - // that show up after a while - log.Printf( - "[INFO] Waiting for device (%s) to have %s of %s", - d.Id(), attribute, target) - +func waitForDeviceAttribute(d *schema.ResourceData, target string, pending []string, attribute string, meta interface{}) (interface{}, error) { stateConf := &resource.StateChangeConf{ Pending: pending, Target: target, @@ -270,27 +267,22 @@ func WaitForDeviceAttribute( Delay: 10 * time.Second, MinTimeout: 3 * time.Second, } - return stateConf.WaitForState() } -func newDeviceStateRefreshFunc( - d *schema.ResourceData, attribute string, meta interface{}) resource.StateRefreshFunc { +func newDeviceStateRefreshFunc(d *schema.ResourceData, attribute string, meta interface{}) resource.StateRefreshFunc { client := meta.(*packngo.Client) + return func() (interface{}, string, error) { - err := resourcePacketDeviceRead(d, meta) - if err != nil { + if err := resourcePacketDeviceRead(d, meta); err != nil { return nil, "", err } - // See if we can access our attribute if attr, ok := d.GetOk(attribute); ok { - // Retrieve the device properties device, _, err := client.Devices.Get(d.Id()) if err != nil { - return nil, "", fmt.Errorf("Error retrieving device: %s", err) + return nil, "", friendlyError(err) } - return &device, attr.(string), nil } @@ -298,19 +290,14 @@ func newDeviceStateRefreshFunc( } } -// Powers on the device and waits for it to be active +// powerOnAndWait Powers on the device and waits for it to be active. func powerOnAndWait(d *schema.ResourceData, meta interface{}) error { client := meta.(*packngo.Client) _, err := client.Devices.PowerOn(d.Id()) if err != nil { - return err + return friendlyError(err) } - // Wait for power on - _, err = WaitForDeviceAttribute(d, "active", []string{"off"}, "state", client) - if err != nil { - return err - } - - return nil + _, err = waitForDeviceAttribute(d, "active", []string{"off"}, "state", client) + return err } diff --git a/builtin/providers/packet/resource_packet_project.go b/builtin/providers/packet/resource_packet_project.go index e41ef1381..05c739b7a 100644 --- a/builtin/providers/packet/resource_packet_project.go +++ b/builtin/providers/packet/resource_packet_project.go @@ -1,10 +1,6 @@ package packet import ( - "fmt" - "log" - "strings" - "github.com/hashicorp/terraform/helper/schema" "github.com/packethost/packngo" ) @@ -53,14 +49,12 @@ func resourcePacketProjectCreate(d *schema.ResourceData, meta interface{}) error PaymentMethod: d.Get("payment_method").(string), } - log.Printf("[DEBUG] Project create configuration: %#v", createRequest) project, _, err := client.Projects.Create(createRequest) if err != nil { - return fmt.Errorf("Error creating Project: %s", err) + return friendlyError(err) } d.SetId(project.ID) - log.Printf("[INFO] Project created: %s", project.ID) return resourcePacketProjectRead(d, meta) } @@ -70,14 +64,16 @@ func resourcePacketProjectRead(d *schema.ResourceData, meta interface{}) error { key, _, err := client.Projects.Get(d.Id()) if err != nil { - // If the project somehow already destroyed, mark as - // succesfully gone - if strings.Contains(err.Error(), "404") { + err = friendlyError(err) + + // If the project somehow already destroyed, mark as succesfully gone. + if isNotFound(err) { d.SetId("") + return nil } - return fmt.Errorf("Error retrieving Project: %s", err) + return err } d.Set("id", key.ID) @@ -100,10 +96,9 @@ func resourcePacketProjectUpdate(d *schema.ResourceData, meta interface{}) error updateRequest.PaymentMethod = attr.(string) } - log.Printf("[DEBUG] Project update: %#v", d.Get("id")) _, _, err := client.Projects.Update(updateRequest) if err != nil { - return fmt.Errorf("Failed to update Project: %s", err) + return friendlyError(err) } return resourcePacketProjectRead(d, meta) @@ -112,10 +107,9 @@ func resourcePacketProjectUpdate(d *schema.ResourceData, meta interface{}) error func resourcePacketProjectDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*packngo.Client) - log.Printf("[INFO] Deleting Project: %s", d.Id()) _, err := client.Projects.Delete(d.Id()) if err != nil { - return fmt.Errorf("Error deleting SSH key: %s", err) + return friendlyError(err) } d.SetId("") diff --git a/builtin/providers/packet/resource_packet_project_test.go b/builtin/providers/packet/resource_packet_project_test.go index b0179cfbe..ff1b45f7c 100644 --- a/builtin/providers/packet/resource_packet_project_test.go +++ b/builtin/providers/packet/resource_packet_project_test.go @@ -37,11 +37,8 @@ func testAccCheckPacketProjectDestroy(s *terraform.State) error { if rs.Type != "packet_project" { continue } - - _, _, err := client.Projects.Get(rs.Primary.ID) - - if err == nil { - fmt.Errorf("Project cstill exists") + if _, _, err := client.Projects.Get(rs.Primary.ID); err == nil { + return fmt.Errorf("Project cstill exists") } } @@ -50,11 +47,9 @@ func testAccCheckPacketProjectDestroy(s *terraform.State) error { func testAccCheckPacketProjectAttributes(project *packngo.Project) resource.TestCheckFunc { return func(s *terraform.State) error { - if project.Name != "foobar" { return fmt.Errorf("Bad name: %s", project.Name) } - return nil } } @@ -62,11 +57,9 @@ func testAccCheckPacketProjectAttributes(project *packngo.Project) resource.Test func testAccCheckPacketProjectExists(n string, project *packngo.Project) 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 Record ID is set") } @@ -74,11 +67,9 @@ func testAccCheckPacketProjectExists(n string, project *packngo.Project) resourc client := testAccProvider.Meta().(*packngo.Client) foundProject, _, err := client.Projects.Get(rs.Primary.ID) - if err != nil { return err } - if foundProject.ID != rs.Primary.ID { return fmt.Errorf("Record not found: %v - %v", rs.Primary.ID, foundProject) } diff --git a/builtin/providers/packet/resource_packet_ssh_key.go b/builtin/providers/packet/resource_packet_ssh_key.go index 95e04bd8c..a70ed78a2 100644 --- a/builtin/providers/packet/resource_packet_ssh_key.go +++ b/builtin/providers/packet/resource_packet_ssh_key.go @@ -1,10 +1,6 @@ package packet import ( - "fmt" - "log" - "strings" - "github.com/hashicorp/terraform/helper/schema" "github.com/packethost/packngo" ) @@ -59,14 +55,12 @@ func resourcePacketSSHKeyCreate(d *schema.ResourceData, meta interface{}) error Key: d.Get("public_key").(string), } - log.Printf("[DEBUG] SSH Key create configuration: %#v", createRequest) key, _, err := client.SSHKeys.Create(createRequest) if err != nil { - return fmt.Errorf("Error creating SSH Key: %s", err) + return friendlyError(err) } d.SetId(key.ID) - log.Printf("[INFO] SSH Key: %s", key.ID) return resourcePacketSSHKeyRead(d, meta) } @@ -76,14 +70,16 @@ func resourcePacketSSHKeyRead(d *schema.ResourceData, meta interface{}) error { key, _, err := client.SSHKeys.Get(d.Id()) if err != nil { + err = friendlyError(err) + // If the key is somehow already destroyed, mark as // succesfully gone - if strings.Contains(err.Error(), "404") { + if isNotFound(err) { d.SetId("") return nil } - return fmt.Errorf("Error retrieving SSH key: %s", err) + return err } d.Set("id", key.ID) @@ -105,10 +101,9 @@ func resourcePacketSSHKeyUpdate(d *schema.ResourceData, meta interface{}) error Key: d.Get("public_key").(string), } - log.Printf("[DEBUG] SSH key update: %#v", d.Get("id")) _, _, err := client.SSHKeys.Update(updateRequest) if err != nil { - return fmt.Errorf("Failed to update SSH key: %s", err) + return friendlyError(err) } return resourcePacketSSHKeyRead(d, meta) @@ -117,10 +112,9 @@ func resourcePacketSSHKeyUpdate(d *schema.ResourceData, meta interface{}) error func resourcePacketSSHKeyDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*packngo.Client) - log.Printf("[INFO] Deleting SSH key: %s", d.Id()) _, err := client.SSHKeys.Delete(d.Id()) if err != nil { - return fmt.Errorf("Error deleting SSH key: %s", err) + return friendlyError(err) } d.SetId("") diff --git a/builtin/providers/packet/resource_packet_ssh_key_test.go b/builtin/providers/packet/resource_packet_ssh_key_test.go index 765086d4f..43cd4a54b 100644 --- a/builtin/providers/packet/resource_packet_ssh_key_test.go +++ b/builtin/providers/packet/resource_packet_ssh_key_test.go @@ -40,11 +40,8 @@ func testAccCheckPacketSSHKeyDestroy(s *terraform.State) error { if rs.Type != "packet_ssh_key" { continue } - - _, _, err := client.SSHKeys.Get(rs.Primary.ID) - - if err == nil { - fmt.Errorf("SSH key still exists") + if _, _, err := client.SSHKeys.Get(rs.Primary.ID); err == nil { + return fmt.Errorf("SSH key still exists") } } @@ -53,11 +50,9 @@ func testAccCheckPacketSSHKeyDestroy(s *terraform.State) error { func testAccCheckPacketSSHKeyAttributes(key *packngo.SSHKey) resource.TestCheckFunc { return func(s *terraform.State) error { - if key.Label != "foobar" { return fmt.Errorf("Bad name: %s", key.Label) } - return nil } } @@ -65,11 +60,9 @@ func testAccCheckPacketSSHKeyAttributes(key *packngo.SSHKey) resource.TestCheckF func testAccCheckPacketSSHKeyExists(n string, key *packngo.SSHKey) 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 Record ID is set") } @@ -77,11 +70,9 @@ func testAccCheckPacketSSHKeyExists(n string, key *packngo.SSHKey) resource.Test client := testAccProvider.Meta().(*packngo.Client) foundKey, _, err := client.SSHKeys.Get(rs.Primary.ID) - if err != nil { return err } - if foundKey.ID != rs.Primary.ID { return fmt.Errorf("SSh Key not found: %v - %v", rs.Primary.ID, foundKey) }