Skip to content
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

r/aws_ec2_availability_zone_group: Prevent crash on unknown group name #24422

Merged
merged 7 commits into from
Apr 26, 2022
3 changes: 3 additions & 0 deletions .changelog/24422.txt
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
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
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
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
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