From 957a5eac2a93cc2d978e3d9c0ae9fcd03df291cd Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Tue, 29 Sep 2026 15:41:09 +1000 Subject: [PATCH] fix: apply and validate DescribeRegions and DescribeAvailabilityZones filters and names --- spinifex/gateway/ec2/zone/AvailabilityZone.go | 148 +++++++++++++++--- spinifex/gateway/ec2/zone/zone_test.go | 95 +++++++++++ 2 files changed, 221 insertions(+), 22 deletions(-) diff --git a/spinifex/gateway/ec2/zone/AvailabilityZone.go b/spinifex/gateway/ec2/zone/AvailabilityZone.go index 37a41e10f..2c4494cb4 100644 --- a/spinifex/gateway/ec2/zone/AvailabilityZone.go +++ b/spinifex/gateway/ec2/zone/AvailabilityZone.go @@ -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 +} diff --git a/spinifex/gateway/ec2/zone/zone_test.go b/spinifex/gateway/ec2/zone/zone_test.go index 458457bd6..cd3637ea0 100644 --- a/spinifex/gateway/ec2/zone/zone_test.go +++ b/spinifex/gateway/ec2/zone/zone_test.go @@ -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) + }) + } +}