Skip to content

Commit

Permalink
Merge pull request #24422 from hashicorp/b-aws_ec2_availability_zone_…
Browse files Browse the repository at this point in the history 
…group-crash

r/aws_ec2_availability_zone_group: Prevent crash on unknown group name
  • Loading branch information
@ewbankkit
ewbankkit authored Apr 26, 2022
2 parents f7040e8 + 9ec09ec commit 9dadb50
Show file tree
Hide file tree
Showing 10 changed files with 254 additions and 177 deletions.
3 changes: 3 additions & 0 deletions .changelog/24422.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ec2_availability_zone_group: Don't crash if `group_name` is not found
```
45 changes: 18 additions & 27 deletions internal/service/ec2/ec2_availability_zone_data_source.go
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package ec2

import (
"fmt"
"log"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
)

func DataSourceAvailabilityZone() *schema.Resource {
Expand Down Expand Up @@ -75,48 +74,40 @@ func DataSourceAvailabilityZone() *schema.Resource {
func dataSourceAvailabilityZoneRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*conns.AWSClient).EC2Conn

req := &ec2.DescribeAvailabilityZonesInput{}
input := &ec2.DescribeAvailabilityZonesInput{}

if v, ok := d.GetOk("all_availability_zones"); ok {
req.AllAvailabilityZones = aws.Bool(v.(bool))
input.AllAvailabilityZones = aws.Bool(v.(bool))
}

if v := d.Get("name").(string); v != "" {
req.ZoneNames = []*string{aws.String(v)}
if v, ok := d.GetOk("zone_id"); ok {
input.ZoneIds = aws.StringSlice([]string{v.(string)})
}
if v := d.Get("zone_id").(string); v != "" {
req.ZoneIds = []*string{aws.String(v)}

if v, ok := d.GetOk("name"); ok {
input.ZoneNames = aws.StringSlice([]string{v.(string)})
}
req.Filters = BuildAttributeFilterList(

input.Filters = BuildAttributeFilterList(
map[string]string{
"state": d.Get("state").(string),
},
)

if filters, filtersOk := d.GetOk("filter"); filtersOk {
req.Filters = append(req.Filters, BuildCustomFilterList(
filters.(*schema.Set),
)...)
}
input.Filters = append(input.Filters, BuildCustomFilterList(
d.Get("filter").(*schema.Set),
)...)

if len(req.Filters) == 0 {
if len(input.Filters) == 0 {
// Don't send an empty filters list; the EC2 API won't accept it.
req.Filters = nil
input.Filters = nil
}

log.Printf("[DEBUG] Reading Availability Zone: %s", req)
resp, err := conn.DescribeAvailabilityZones(req)
az, err := FindAvailabilityZone(conn, input)

if err != nil {
return err
return tfresource.SingularDataSourceFindError("EC2 Availability Zone", err)
}
if resp == nil || len(resp.AvailabilityZones) == 0 {
return fmt.Errorf("no matching AZ found")
}
if len(resp.AvailabilityZones) > 1 {
return fmt.Errorf("multiple AZs matched; use additional constraints to reduce matches to a single AZ")
}

az := resp.AvailabilityZones[0]

// As a convenience when working with AZs generically, we expose
// the AZ suffix alone, without the region name.
Expand Down
4 changes: 2 additions & 2 deletions internal/service/ec2/ec2_availability_zone_data_source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func testAccPreCheckLocalZoneAvailable(t *testing.T) {
}),
}

output, err := conn.DescribeAvailabilityZones(input)
output, err := tfec2.FindAvailabilityZones(conn, input)

if acctest.PreCheckSkipError(err) {
t.Skipf("skipping acceptance testing: %s", err)
Expand All @@ -200,7 +200,7 @@ func testAccPreCheckLocalZoneAvailable(t *testing.T) {
t.Fatalf("unexpected PreCheck error: %s", err)
}

if output == nil || len(output.AvailabilityZones) == 0 {
if len(output) == 0 {
t.Skip("skipping since no Local Zones are available")
}
}
Expand Down
129 changes: 26 additions & 103 deletions internal/service/ec2/ec2_availability_zone_group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,9 @@ package ec2

import (
"fmt"
"log"
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
Expand All @@ -19,12 +16,9 @@ func ResourceAvailabilityZoneGroup() *schema.Resource {
Read: resourceAvailabilityZoneGroupRead,
Update: resourceAvailabilityZoneGroupUpdate,
Delete: schema.Noop,
Importer: &schema.ResourceImporter{
State: func(d *schema.ResourceData, m interface{}) ([]*schema.ResourceData, error) {
d.Set("group_name", d.Id())

return []*schema.ResourceData{d}, nil
},
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
Expand All @@ -48,41 +42,31 @@ func ResourceAvailabilityZoneGroup() *schema.Resource {
func resourceAvailabilityZoneGroupCreate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*conns.AWSClient).EC2Conn

configurationOptInStatus := d.Get("opt_in_status").(string)
groupName := d.Get("group_name").(string)
availabilityZone, err := FindAvailabilityZoneGroupByName(conn, groupName)

d.SetId(d.Get("group_name").(string))

if err := resourceAvailabilityZoneGroupRead(d, meta); err != nil {
return err
if err != nil {
return fmt.Errorf("reading EC2 Availability Zone Group (%s): %w", groupName, err)
}

apiOptInStatus := d.Get("opt_in_status").(string)

if apiOptInStatus != configurationOptInStatus {
input := &ec2.ModifyAvailabilityZoneGroupInput{
GroupName: aws.String(d.Id()),
OptInStatus: aws.String(configurationOptInStatus),
}

if _, err := conn.ModifyAvailabilityZoneGroup(input); err != nil {
return fmt.Errorf("error modifying EC2 Availability Zone Group (%s): %w", d.Id(), err)
}

if err := waitForEc2AvailabilityZoneGroupOptInStatus(conn, d.Id(), configurationOptInStatus); err != nil {
return fmt.Errorf("error waiting for EC2 Availability Zone Group (%s) opt-in status update: %w", d.Id(), err)
if v := d.Get("opt_in_status").(string); v != aws.StringValue(availabilityZone.OptInStatus) {
if err := modifyAvailabilityZoneOptInStatus(conn, groupName, v); err != nil {
return err
}
}

d.SetId(groupName)

return resourceAvailabilityZoneGroupRead(d, meta)
}

func resourceAvailabilityZoneGroupRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*conns.AWSClient).EC2Conn

availabilityZone, err := ec2DescribeAvailabilityZoneGroup(conn, d.Id())
availabilityZone, err := FindAvailabilityZoneGroupByName(conn, d.Id())

if err != nil {
return fmt.Errorf("error describing EC2 Availability Zone Group (%s): %w", d.Id(), err)
return fmt.Errorf("reading EC2 Availability Zone Group (%s): %w", d.Id(), err)
}

if aws.StringValue(availabilityZone.OptInStatus) == ec2.AvailabilityZoneOptInStatusOptInNotRequired {
Expand All @@ -97,93 +81,32 @@ func resourceAvailabilityZoneGroupRead(d *schema.ResourceData, meta interface{})

func resourceAvailabilityZoneGroupUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*conns.AWSClient).EC2Conn
optInStatus := d.Get("opt_in_status").(string)

input := &ec2.ModifyAvailabilityZoneGroupInput{
GroupName: aws.String(d.Id()),
OptInStatus: aws.String(optInStatus),
}

if _, err := conn.ModifyAvailabilityZoneGroup(input); err != nil {
return fmt.Errorf("error modifying EC2 Availability Zone Group (%s): %w", d.Id(), err)
}

if err := waitForEc2AvailabilityZoneGroupOptInStatus(conn, d.Id(), optInStatus); err != nil {
return fmt.Errorf("error waiting for EC2 Availability Zone Group (%s) opt-in status update: %w", d.Id(), err)
if err := modifyAvailabilityZoneOptInStatus(conn, d.Id(), d.Get("opt_in_status").(string)); err != nil {
return err
}

return resourceAvailabilityZoneGroupRead(d, meta)
}

func ec2DescribeAvailabilityZoneGroup(conn *ec2.EC2, groupName string) (*ec2.AvailabilityZone, error) {
input := &ec2.DescribeAvailabilityZonesInput{
AllAvailabilityZones: aws.Bool(true),
Filters: []*ec2.Filter{
{
Name: aws.String("group-name"),
Values: aws.StringSlice([]string{groupName}),
},
},
}

output, err := conn.DescribeAvailabilityZones(input)

if err != nil {
return nil, err
}

if output == nil || len(output.AvailabilityZones) == 0 {
return nil, nil
}

for _, availabilityZone := range output.AvailabilityZones {
if availabilityZone == nil {
continue
}

if aws.StringValue(availabilityZone.GroupName) == groupName {
return availabilityZone, nil
}
func modifyAvailabilityZoneOptInStatus(conn *ec2.EC2, groupName, optInStatus string) error {
input := &ec2.ModifyAvailabilityZoneGroupInput{
GroupName: aws.String(groupName),
OptInStatus: aws.String(optInStatus),
}

return nil, nil
}

func ec2AvailabilityZoneGroupOptInStatusRefreshFunc(conn *ec2.EC2, groupName string) resource.StateRefreshFunc {
return func() (interface{}, string, error) {
availabilityZone, err := ec2DescribeAvailabilityZoneGroup(conn, groupName)

if err != nil {
return nil, "", fmt.Errorf("error describing EC2 Availability Zone Group (%s): %w", groupName, err)
}

if availabilityZone == nil {
return nil, "", fmt.Errorf("error describing EC2 Availability Zone Group (%s): not found", groupName)
}

return availabilityZone, aws.StringValue(availabilityZone.OptInStatus), nil
if _, err := conn.ModifyAvailabilityZoneGroup(input); err != nil {
return fmt.Errorf("modifying EC2 Availability Zone Group (%s): %w", groupName, err)
}
}

func waitForEc2AvailabilityZoneGroupOptInStatus(conn *ec2.EC2, groupName string, optInStatus string) error {
pending := ec2.AvailabilityZoneOptInStatusNotOptedIn

waiter := WaitAvailabilityZoneGroupOptedIn
if optInStatus == ec2.AvailabilityZoneOptInStatusNotOptedIn {
pending = ec2.AvailabilityZoneOptInStatusOptedIn
waiter = WaitAvailabilityZoneGroupNotOptedIn
}

stateConf := &resource.StateChangeConf{
Pending: []string{pending},
Target: []string{optInStatus},
Refresh: ec2AvailabilityZoneGroupOptInStatusRefreshFunc(conn, groupName),
Timeout: 10 * time.Minute,
Delay: 10 * time.Second,
MinTimeout: 2 * time.Second,
ContinuousTargetOccurence: 3,
if _, err := waiter(conn, groupName); err != nil {
return fmt.Errorf("waiting for EC2 Availability Zone Group (%s) opt-in status update: %w", groupName, err)
}

log.Printf("[DEBUG] Waiting for EC2 Availability Zone Group (%s) opt-in status update", groupName)
_, err := stateConf.WaitForState()

return err
return nil
}
44 changes: 6 additions & 38 deletions internal/service/ec2/ec2_availability_zone_group_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/endpoints"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-provider-aws/internal/acctest"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
)

func TestAccEC2AvailabilityZoneGroup_optInStatus(t *testing.T) {
Expand All @@ -17,16 +16,16 @@ func TestAccEC2AvailabilityZoneGroup_optInStatus(t *testing.T) {
// Filter to one Availability Zone Group per Region as Local Zones become available
// e.g. ensure there are not two us-west-2-XXX when adding to this list
// (Not including in config to avoid lintignoring entire config.)
localZone := "us-west-2-lax-1" // lintignore:AWSAT003 // currently the only generally available local zone
localZone := "us-west-2-lax-1" // lintignore:AWSAT003

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t); testAccPreCheckAvailabilityZoneGroup(t) },
PreCheck: func() { acctest.PreCheck(t); acctest.PreCheckRegion(t, endpoints.UsWest2RegionID) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: nil,
Steps: []resource.TestStep{
{
Config: testAccEc2AvailabilityZoneGroupConfigOptInStatus(localZone, ec2.AvailabilityZoneOptInStatusOptedIn),
Config: testAccEc2AvailabilityZoneGroupOptInStatusConfig(localZone, ec2.AvailabilityZoneOptInStatusOptedIn),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "opt_in_status", ec2.AvailabilityZoneOptInStatusOptedIn),
),
Expand Down Expand Up @@ -55,38 +54,7 @@ func TestAccEC2AvailabilityZoneGroup_optInStatus(t *testing.T) {
})
}

func testAccPreCheckAvailabilityZoneGroup(t *testing.T) {
conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn

input := &ec2.DescribeAvailabilityZonesInput{
AllAvailabilityZones: aws.Bool(true),
Filters: []*ec2.Filter{
{
Name: aws.String("opt-in-status"),
Values: aws.StringSlice([]string{
ec2.AvailabilityZoneOptInStatusNotOptedIn,
ec2.AvailabilityZoneOptInStatusOptedIn,
}),
},
},
}

output, err := conn.DescribeAvailabilityZones(input)

if acctest.PreCheckSkipError(err) {
t.Skipf("skipping acceptance testing: %s", err)
}

if err != nil {
t.Fatalf("unexpected PreCheck error: %s", err)
}

if output == nil || len(output.AvailabilityZones) == 0 || output.AvailabilityZones[0] == nil {
t.Skipf("skipping acceptance testing: no opt-in EC2 Availability Zone Groups found")
}
}

func testAccEc2AvailabilityZoneGroupConfigOptInStatus(localZone, optInStatus string) string {
func testAccEc2AvailabilityZoneGroupOptInStatusConfig(name, optInStatus string) string {
return fmt.Sprintf(`
data "aws_availability_zones" "test" {
all_availability_zones = true
Expand All @@ -104,5 +72,5 @@ resource "aws_ec2_availability_zone_group" "test" {
group_name = tolist(data.aws_availability_zones.test.group_names)[0]
opt_in_status = %[2]q
}
`, localZone, optInStatus)
`, name, optInStatus)
}
Loading

0 comments on commit 9dadb50

Please sign in to comment.