-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Import for compute_address now requires region/name. Changed the ID format. #378
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
Changes from 6 commits
2ce3872
98c1197
cc9e336
5e70850
900c90a
98614ec
122ee50
d56c327
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 |
---|---|---|
|
@@ -6,6 +6,13 @@ import ( | |
|
||
"github.com/hashicorp/terraform/helper/schema" | ||
"google.golang.org/api/compute/v1" | ||
"regexp" | ||
"strings" | ||
) | ||
|
||
var ( | ||
computeAddressIdTemplate = "projects/%s/regions/%s/addresses/%s" | ||
computeAddressLinkRegex = regexp.MustCompile("projects/(.*)/regions/(.*)/addresses/(.*)$") | ||
) | ||
|
||
func resourceComputeAddress() *schema.Resource { | ||
|
@@ -14,8 +21,12 @@ func resourceComputeAddress() *schema.Resource { | |
Read: resourceComputeAddressRead, | ||
Delete: resourceComputeAddressDelete, | ||
Importer: &schema.ResourceImporter{ | ||
State: schema.ImportStatePassthrough, | ||
State: resourceComputeAddressImportState, | ||
}, | ||
|
||
SchemaVersion: 1, | ||
MigrateState: resourceComputeAddressMigrateState, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"name": &schema.Schema{ | ||
Type: schema.TypeString, | ||
|
@@ -37,6 +48,7 @@ func resourceComputeAddress() *schema.Resource { | |
"region": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
ForceNew: true, | ||
}, | ||
|
||
|
@@ -70,7 +82,11 @@ func resourceComputeAddressCreate(d *schema.ResourceData, meta interface{}) erro | |
} | ||
|
||
// It probably maybe worked, so store the ID now | ||
d.SetId(addr.Name) | ||
d.SetId(computeAddressId{ | ||
Project: project, | ||
Region: region, | ||
Name: addr.Name, | ||
}.canonicalId()) | ||
|
||
err = computeOperationWait(config, op, project, "Creating Address") | ||
if err != nil { | ||
|
@@ -83,55 +99,107 @@ func resourceComputeAddressCreate(d *schema.ResourceData, meta interface{}) erro | |
func resourceComputeAddressRead(d *schema.ResourceData, meta interface{}) error { | ||
config := meta.(*Config) | ||
|
||
region, err := getRegion(d, config) | ||
addressId, err := parseComputeAddressId(d.Id(), config) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
project, err := getProject(d, config) | ||
if err != nil { | ||
return err | ||
} | ||
log.Println("[rosbo] resourceComputeAddressRead", addressId) | ||
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. Oops this guy snuck in 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. Oupps. Removed. |
||
|
||
addr, err := config.clientCompute.Addresses.Get( | ||
project, region, d.Id()).Do() | ||
addressId.Project, addressId.Region, addressId.Name).Do() | ||
if err != nil { | ||
return handleNotFoundError(err, d, fmt.Sprintf("Address %q", d.Get("name").(string))) | ||
} | ||
|
||
d.Set("address", addr.Address) | ||
d.Set("self_link", addr.SelfLink) | ||
d.Set("name", addr.Name) | ||
d.Set("region", addr.Region) | ||
|
||
return nil | ||
} | ||
|
||
func resourceComputeAddressDelete(d *schema.ResourceData, meta interface{}) error { | ||
config := meta.(*Config) | ||
|
||
region, err := getRegion(d, config) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
project, err := getProject(d, config) | ||
addressId, err := parseComputeAddressId(d.Id(), config) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Delete the address | ||
log.Printf("[DEBUG] address delete request") | ||
op, err := config.clientCompute.Addresses.Delete( | ||
project, region, d.Id()).Do() | ||
addressId.Project, addressId.Region, addressId.Name).Do() | ||
if err != nil { | ||
return fmt.Errorf("Error deleting address: %s", err) | ||
} | ||
|
||
err = computeOperationWait(config, op, project, "Deleting Address") | ||
err = computeOperationWait(config, op, addressId.Project, "Deleting Address") | ||
if err != nil { | ||
return err | ||
} | ||
|
||
d.SetId("") | ||
return nil | ||
} | ||
|
||
func resourceComputeAddressImportState(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { | ||
config := meta.(*Config) | ||
|
||
addressId, err := parseComputeAddressId(d.Id(), config) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
d.SetId(addressId.canonicalId()) | ||
|
||
return []*schema.ResourceData{d}, nil | ||
} | ||
|
||
type computeAddressId struct { | ||
Project string | ||
Region string | ||
Name string | ||
} | ||
|
||
func (s computeAddressId) canonicalId() string { | ||
return fmt.Sprintf(computeAddressIdTemplate, s.Project, s.Region, s.Name) | ||
} | ||
|
||
func parseComputeAddressId(id string, config *Config) (*computeAddressId, error) { | ||
var parts []string | ||
if computeAddressLinkRegex.MatchString(id) { | ||
parts = computeAddressLinkRegex.FindStringSubmatch(id) | ||
|
||
return &computeAddressId{ | ||
Project: parts[1], | ||
Region: parts[2], | ||
Name: parts[3], | ||
}, nil | ||
} else { | ||
parts = strings.Split(id, "/") | ||
} | ||
|
||
if len(parts) == 3 { | ||
return &computeAddressId{ | ||
Project: parts[0], | ||
Region: parts[1], | ||
Name: parts[2], | ||
}, nil | ||
} else if len(parts) == 2 { | ||
// Project is optional. | ||
if config.Project == "" { | ||
return nil, fmt.Errorf("Invalid compute address id. The default project for the provider must be set.") | ||
} | ||
|
||
return &computeAddressId{ | ||
Project: config.Project, | ||
Region: parts[0], | ||
Name: parts[1], | ||
}, nil | ||
} | ||
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. Missing one more case (only specifying 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. It was on purpose but now that I think about it, I see no good reason for not supporting this case. Support added. |
||
|
||
return nil, fmt.Errorf("Invalid compute address id. Expecting resource link or {project}/{region}/{name} format.") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
package google | ||
|
||
import ( | ||
"fmt" | ||
"github.com/hashicorp/terraform/terraform" | ||
"log" | ||
) | ||
|
||
func resourceComputeAddressMigrateState(v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { | ||
if is.Empty() { | ||
log.Println("[DEBUG] Empty InstanceState; nothing to migrate.") | ||
return is, nil | ||
} | ||
|
||
switch v { | ||
case 0: | ||
log.Println("[INFO] Found Container Node Pool State v0; migrating to v1") | ||
return migrateComputeAddressV0toV1(is, meta) | ||
default: | ||
return is, fmt.Errorf("Unexpected schema version: %d", v) | ||
} | ||
} | ||
|
||
func migrateComputeAddressV0toV1(is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { | ||
log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) | ||
log.Printf("[DEBUG] ID before migration: %s", is.ID) | ||
|
||
config := meta.(*Config) | ||
|
||
project, err := getProjectFromInstanceState(is, config) | ||
if err != nil { | ||
return is, err | ||
} | ||
|
||
region, err := getRegionFromInstanceState(is, config) | ||
if err != nil { | ||
return is, err | ||
} | ||
|
||
is.ID = computeAddressId{ | ||
Project: project, | ||
Region: region, | ||
Name: is.Attributes["name"], | ||
}.canonicalId() | ||
|
||
log.Printf("[DEBUG] ID after migration: %s", is.ID) | ||
return is, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package google | ||
|
||
import ( | ||
"github.com/hashicorp/terraform/terraform" | ||
"testing" | ||
) | ||
|
||
func TestComputeAddressMigrateState(t *testing.T) { | ||
cases := map[string]struct { | ||
StateVersion int | ||
Attributes map[string]string | ||
ExpectedId string | ||
Meta interface{} | ||
}{ | ||
"update id from name to region/name": { | ||
StateVersion: 0, | ||
Attributes: map[string]string{ | ||
"name": "address-1", | ||
}, | ||
ExpectedId: "projects/gcp-project/regions/us-central1/addresses/address-1", | ||
Meta: &Config{Region: "us-central1", Project: "gcp-project"}, | ||
}, | ||
} | ||
|
||
for tn, tc := range cases { | ||
is := &terraform.InstanceState{ | ||
ID: tc.Attributes["name"], | ||
Attributes: tc.Attributes, | ||
} | ||
|
||
is, err := resourceComputeAddressMigrateState(tc.StateVersion, is, tc.Meta) | ||
|
||
if err != nil { | ||
t.Fatalf("bad: %s, err: %#v", tn, err) | ||
} | ||
|
||
if is.ID != tc.ExpectedId { | ||
t.Fatalf("Id should be set to `%s` but is `%s`", tc.ExpectedId, is.ID) | ||
} | ||
} | ||
} | ||
|
||
func TestComputeAddressMigrateState_empty(t *testing.T) { | ||
var is *terraform.InstanceState | ||
var meta *Config | ||
|
||
// should handle nil | ||
is, err := resourceComputeAddressMigrateState(0, is, meta) | ||
|
||
if err != nil { | ||
t.Fatalf("err: %#v", err) | ||
} | ||
|
||
if is != nil { | ||
t.Fatalf("expected nil instancestate, got: %#v", is) | ||
} | ||
|
||
// should handle non-nil but empty | ||
is = &terraform.InstanceState{} | ||
is, err = resourceComputeAddressMigrateState(0, is, meta) | ||
|
||
if err != nil { | ||
t.Fatalf("err: %#v", err) | ||
} | ||
} |
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.
Minor quibble: using .+ is preferred to .* as it excludes a few invalid compute regexes that match (that probably won't ever occur but oh well)
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.
Totally agree, fixed.