fix: apply and validate DescribeRegions and DescribeAvailabilityZones filters and names

2026-10-15 09:01:31 +00:00 · 2026-09-29 15:41:09 +10:00
parent c85376c5a2
commit 957a5eac2a
2 changed files with 221 additions and 22 deletions
@@ -1,28 +1,73 @@
 package gateway_ec2_zone

 import (
+	"slices"
+	"strings"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
+	"github.com/mulgadc/spinifex/spinifex/filterutil"
 	handlers_ec2_vpc "github.com/mulgadc/spinifex/spinifex/handlers/ec2/vpc"
 )

+// The filter names AWS accepts on each call; any other, tag filters included,
+// is refused.
+var (
+	describeAvailabilityZonesValidFilters = map[string]bool{
+		"group-long-name":  true,
+		"group-name":       true,
+		"message":          true,
+		"opt-in-status":    true,
+		"parent-zone-id":   true,
+		"parent-zone-name": true,
+		"region-name":      true,
+		"state":            true,
+		"zone-id":          true,
+		"zone-name":        true,
+		"zone-type":        true,
+	}
+	describeRegionsValidFilters = map[string]bool{
+		"endpoint":      true,
+		"opt-in-status": true,
+		"region-name":   true,
+	}
+)
+
 func DescribeAvailabilityZones(input *ec2.DescribeAvailabilityZonesInput, region string, az string) (output *ec2.DescribeAvailabilityZonesOutput, err error) {
-	output = &ec2.DescribeAvailabilityZonesOutput{
-		AvailabilityZones: []*ec2.AvailabilityZone{
-			{
-				State:              aws.String("available"),
-				OptInStatus:        aws.String("opt-in-not-required"),
-				RegionName:         aws.String(region),
-				ZoneName:           aws.String(az),
-				ZoneId:             aws.String(handlers_ec2_vpc.SingleZoneID),
-				GroupName:          aws.String(region),
-				NetworkBorderGroup: aws.String(region),
-				ZoneType:           aws.String("availability-zone"),
-				Messages:           []*ec2.AvailabilityZoneMessage{},
-			},
-		},
+	filters, err := parseFilters(input.Filters, describeAvailabilityZonesValidFilters)
+	if err != nil {
+		return nil, err
 	}

+	zone := &ec2.AvailabilityZone{
+		State:              aws.String("available"),
+		OptInStatus:        aws.String("opt-in-not-required"),
+		RegionName:         aws.String(region),
+		ZoneName:           aws.String(az),
+		ZoneId:             aws.String(handlers_ec2_vpc.SingleZoneID),
+		GroupName:          aws.String(region),
+		NetworkBorderGroup: aws.String(region),
+		ZoneType:           aws.String("availability-zone"),
+		Messages:           []*ec2.AvailabilityZoneMessage{},
+	}
+
+	// AWS names the first unknown zone in the order the caller gave them.
+	for _, name := range aws.StringValueSlice(input.ZoneNames) {
+		if name != az {
+			return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "Invalid availability zone: [%s]", name)
+		}
+	}
+	for _, id := range aws.StringValueSlice(input.ZoneIds) {
+		if id != handlers_ec2_vpc.SingleZoneID {
+			return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "Invalid availability zone-id: [%s]", id)
+		}
+	}
+
+	output = &ec2.DescribeAvailabilityZonesOutput{AvailabilityZones: []*ec2.AvailabilityZone{}}
+	if matchesFilters(filters, zoneFilterValues(zone)) {
+		output.AvailabilityZones = append(output.AvailabilityZones, zone)
+	}
 	return output, nil
 }

@@ -30,15 +75,74 @@ func DescribeAvailabilityZones(input *ec2.DescribeAvailabilityZonesInput, region
 // caller resolves endpoint from the gateway's advertised host so a remote
 // workload is never pointed at its own loopback.
 func DescribeRegions(input *ec2.DescribeRegionsInput, region string, endpoint string) (output *ec2.DescribeRegionsOutput, err error) {
-	output = &ec2.DescribeRegionsOutput{
-		Regions: []*ec2.Region{
-			{
-				Endpoint:    aws.String(endpoint),
-				RegionName:  aws.String(region),
-				OptInStatus: aws.String("opt-in-not-required"),
-			},
-		},
+	filters, err := parseFilters(input.Filters, describeRegionsValidFilters)
+	if err != nil {
+		return nil, err
 	}

+	// AWS names the alphabetically first unknown Region, not the first given.
+	var unknown []string
+	for _, name := range aws.StringValueSlice(input.RegionNames) {
+		if name != region {
+			unknown = append(unknown, name)
+		}
+	}
+	if len(unknown) > 0 {
+		return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "Invalid region: [%s]", slices.Min(unknown))
+	}
+
+	r := &ec2.Region{
+		Endpoint:    aws.String(endpoint),
+		RegionName:  aws.String(region),
+		OptInStatus: aws.String("opt-in-not-required"),
+	}
+	output = &ec2.DescribeRegionsOutput{Regions: []*ec2.Region{}}
+	if matchesFilters(filters, map[string][]string{
+		"endpoint":      {endpoint},
+		"opt-in-status": {aws.StringValue(r.OptInStatus)},
+		"region-name":   {region},
+	}) {
+		output.Regions = append(output.Regions, r)
+	}
 	return output, nil
 }
+
+// parseFilters is filterutil.ParseFilters without its blanket acceptance of
+// tag filters, which AWS refuses on both calls.
+func parseFilters(filters []*ec2.Filter, valid map[string]bool) (map[string][]string, error) {
+	for _, f := range filters {
+		if name := aws.StringValue(f.Name); strings.HasPrefix(name, "tag:") {
+			return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "The filter '%s' is invalid", name)
+		}
+	}
+	return filterutil.ParseFilters(filters, valid)
+}
+
+// zoneFilterValues maps each filter name to the zone's values for it. A name
+// with no values, such as parent-zone-id on a zone with no parent, matches nothing.
+func zoneFilterValues(zone *ec2.AvailabilityZone) map[string][]string {
+	values := map[string][]string{
+		"group-name":    {aws.StringValue(zone.GroupName)},
+		"opt-in-status": {aws.StringValue(zone.OptInStatus)},
+		"region-name":   {aws.StringValue(zone.RegionName)},
+		"state":         {aws.StringValue(zone.State)},
+		"zone-id":       {aws.StringValue(zone.ZoneId)},
+		"zone-name":     {aws.StringValue(zone.ZoneName)},
+		"zone-type":     {aws.StringValue(zone.ZoneType)},
+	}
+	for _, m := range zone.Messages {
+		values["message"] = append(values["message"], aws.StringValue(m.Message))
+	}
+	return values
+}
+
+// matchesFilters reports whether every filter matches one of the resource's
+// values for that name.
+func matchesFilters(filters map[string][]string, values map[string][]string) bool {
+	for name, patterns := range filters {
+		if !slices.ContainsFunc(values[name], func(v string) bool { return filterutil.MatchesAny(patterns, v) }) {
+			return false
+		}
+	}
+	return true
+}
@@ -3,7 +3,9 @@ package gateway_ec2_zone
 import (
 	"testing"

+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/mulgadc/spinifex/spinifex/awserrors"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -85,3 +87,96 @@ func TestDescribeRegions(t *testing.T) {
 		})
 	}
 }
+
+func filter(name string, values ...string) []*ec2.Filter {
+	return []*ec2.Filter{{Name: aws.String(name), Values: aws.StringSlice(values)}}
+}
+
+func requireInvalidParameterValue(t *testing.T, err error, msg string) {
+	t.Helper()
+	code, got, ok := awserrors.ResolveErrorDetail(err)
+	require.True(t, ok)
+	assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
+	assert.Equal(t, msg, got)
+}
+
+// Names are checked against the configured catalogue, so a custom Region name
+// is as valid as an AWS one. The messages are AWS's own.
+func TestDescribeAvailabilityZones_Selection(t *testing.T) {
+	const region, az = "mulga-west", "mulga-west-a"
+
+	for _, tc := range []struct {
+		name  string
+		input *ec2.DescribeAvailabilityZonesInput
+		want  int
+	}{
+		{"zone name", &ec2.DescribeAvailabilityZonesInput{ZoneNames: aws.StringSlice([]string{az})}, 1},
+		{"zone id", &ec2.DescribeAvailabilityZonesInput{ZoneIds: aws.StringSlice([]string{"spinifexz1"})}, 1},
+		{"zone-name filter", &ec2.DescribeAvailabilityZonesInput{Filters: filter("zone-name", az)}, 1},
+		{"wildcard filter", &ec2.DescribeAvailabilityZonesInput{Filters: filter("region-name", "mulga-*")}, 1},
+		{"state filter", &ec2.DescribeAvailabilityZonesInput{Filters: filter("state", "available")}, 1},
+		{"filter matching nothing", &ec2.DescribeAvailabilityZonesInput{Filters: filter("zone-name", "other")}, 0},
+		{"filter on an absent field", &ec2.DescribeAvailabilityZonesInput{Filters: filter("parent-zone-id", "*")}, 0},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			out, err := DescribeAvailabilityZones(tc.input, region, az)
+			require.NoError(t, err)
+			assert.Len(t, out.AvailabilityZones, tc.want)
+		})
+	}
+
+	for _, tc := range []struct {
+		name  string
+		input *ec2.DescribeAvailabilityZonesInput
+		msg   string
+	}{
+		{"unknown filter", &ec2.DescribeAvailabilityZonesInput{Filters: filter("bogus-filter", "x")}, "The filter 'bogus-filter' is invalid"},
+		{"tag filter", &ec2.DescribeAvailabilityZonesInput{Filters: filter("tag:Name", "x")}, "The filter 'tag:Name' is invalid"},
+		{"network-border-group filter", &ec2.DescribeAvailabilityZonesInput{Filters: filter("network-border-group", "x")}, "The filter 'network-border-group' is invalid"},
+		{"first unknown name in request order", &ec2.DescribeAvailabilityZonesInput{ZoneNames: aws.StringSlice([]string{az, "zz-nowhere-9z", "yy-nowhere-8y"})}, "Invalid availability zone: [zz-nowhere-9z]"},
+		{"unknown zone id", &ec2.DescribeAvailabilityZonesInput{ZoneIds: aws.StringSlice([]string{"spinifexz1", "zz-az9"})}, "Invalid availability zone-id: [zz-az9]"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := DescribeAvailabilityZones(tc.input, region, az)
+			requireInvalidParameterValue(t, err, tc.msg)
+		})
+	}
+}
+
+func TestDescribeRegions_Selection(t *testing.T) {
+	const region, endpoint = "mulga-west", "https://10.0.0.1:9999"
+
+	for _, tc := range []struct {
+		name  string
+		input *ec2.DescribeRegionsInput
+		want  int
+	}{
+		{"region name", &ec2.DescribeRegionsInput{RegionNames: aws.StringSlice([]string{region})}, 1},
+		{"region-name filter", &ec2.DescribeRegionsInput{Filters: filter("region-name", region)}, 1},
+		{"endpoint filter", &ec2.DescribeRegionsInput{Filters: filter("endpoint", "*10.0.0.1*")}, 1},
+		{"filter matching nothing", &ec2.DescribeRegionsInput{Filters: filter("region-name", "zz-nowhere-9")}, 0},
+		{"names and a disagreeing filter", &ec2.DescribeRegionsInput{RegionNames: aws.StringSlice([]string{region}), Filters: filter("region-name", "us-east-1")}, 0},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			out, err := DescribeRegions(tc.input, region, endpoint)
+			require.NoError(t, err)
+			assert.Len(t, out.Regions, tc.want)
+		})
+	}
+
+	for _, tc := range []struct {
+		name  string
+		input *ec2.DescribeRegionsInput
+		msg   string
+	}{
+		{"unknown filter", &ec2.DescribeRegionsInput{Filters: filter("bogus-filter", "x")}, "The filter 'bogus-filter' is invalid"},
+		{"tag filter", &ec2.DescribeRegionsInput{Filters: filter("tag:Name", "x")}, "The filter 'tag:Name' is invalid"},
+		{"an AWS region this cluster is not", &ec2.DescribeRegionsInput{RegionNames: aws.StringSlice([]string{"us-east-1"})}, "Invalid region: [us-east-1]"},
+		{"alphabetically first unknown name", &ec2.DescribeRegionsInput{RegionNames: aws.StringSlice([]string{region, "zz-nowhere-9", "yy-nowhere-8"})}, "Invalid region: [yy-nowhere-8]"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			_, err := DescribeRegions(tc.input, region, endpoint)
+			requireInvalidParameterValue(t, err, tc.msg)
+		})
+	}
+}