diff --git a/spinifex/gateway/ec2.go b/spinifex/gateway/ec2.go index 49b54b501..d57334a34 100644 --- a/spinifex/gateway/ec2.go +++ b/spinifex/gateway/ec2.go @@ -36,6 +36,7 @@ import ( gateway_ec2_zone "github.com/mulgadc/spinifex/spinifex/gateway/ec2/zone" handlers_quota "github.com/mulgadc/spinifex/spinifex/handlers/quota" "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/nats-io/nats.go" ) // EC2Handler processes parsed query args and returns XML response bytes. @@ -356,7 +357,9 @@ var ec2Actions = map[string]ec2Action{ return gateway_ec2_volume.DetachVolume(ctx, input, gw.NATSConn, accountID) }), "DescribeAccountAttributes": ec2Handler(func(ctx context.Context, input *ec2.DescribeAccountAttributesInput, gw *GatewayConfig, accountID string) (any, error) { - return gateway_ec2_account.DescribeAccountAttributes(input) + return gateway_ec2_account.DescribeAccountAttributes(input, func() (string, error) { + return defaultVPCID(ctx, gw.NATSConn, accountID) + }) }), "EnableEbsEncryptionByDefault": ec2Handler(func(ctx context.Context, input *ec2.EnableEbsEncryptionByDefaultInput, gw *GatewayConfig, accountID string) (any, error) { return gateway_ec2_account.EnableEbsEncryptionByDefault(ctx, input, gw.NATSConn, accountID) @@ -626,7 +629,26 @@ var ec2Actions = map[string]ec2Action{ }), } -// ec2LocalActions are actions that don't require NATS. +// defaultVPCID returns the ID of accountID's default VPC, or "" when it has +// none, as the account's own DescribeVpcs sees it. +func defaultVPCID(ctx context.Context, natsConn *nats.Conn, accountID string) (string, error) { + if natsConn == nil { + return "", errors.New(awserrors.ErrorServerInternal) + } + out, err := gateway_ec2_vpc.DescribeVpcs(ctx, &ec2.DescribeVpcsInput{ + Filters: []*ec2.Filter{{Name: aws.String("is-default"), Values: aws.StringSlice([]string{"true"})}}, + }, natsConn, accountID) + if err != nil { + return "", err + } + if len(out.Vpcs) == 0 { + return "", nil + } + return aws.StringValue(out.Vpcs[0].VpcId), nil +} + +// ec2LocalActions are actions that don't require NATS. DescribeAccountAttributes +// needs it only for default-vpc. var ec2LocalActions = map[string]bool{ "DescribeRegions": true, "DescribeAvailabilityZones": true, diff --git a/spinifex/gateway/ec2/account/DescribeAccountAttributes.go b/spinifex/gateway/ec2/account/DescribeAccountAttributes.go index 697d9ff08..0254411ad 100644 --- a/spinifex/gateway/ec2/account/DescribeAccountAttributes.go +++ b/spinifex/gateway/ec2/account/DescribeAccountAttributes.go @@ -7,8 +7,10 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" ) -// DescribeAccountAttributes returns static account attributes for the Spinifex platform. -func DescribeAccountAttributes(input *ec2.DescribeAccountAttributesInput) (*ec2.DescribeAccountAttributesOutput, error) { +// DescribeAccountAttributes returns the account's attributes. Every attribute +// but default-vpc is static; defaultVPCID looks that one up, is called only +// when it is asked for, and returns "" for an account with no default VPC. +func DescribeAccountAttributes(input *ec2.DescribeAccountAttributesInput, defaultVPCID func() (string, error)) (*ec2.DescribeAccountAttributesOutput, error) { requestedAttrs := make(map[string]bool) for _, name := range input.AttributeNames { if name != nil { @@ -30,10 +32,17 @@ func DescribeAccountAttributes(input *ec2.DescribeAccountAttributesInput) (*ec2. } if returnAll || requestedAttrs["default-vpc"] { + vpcID, err := defaultVPCID() + if err != nil { + return nil, err + } + if vpcID == "" { + vpcID = "none" + } accountAttributes = append(accountAttributes, &ec2.AccountAttribute{ AttributeName: aws.String("default-vpc"), AttributeValues: []*ec2.AccountAttributeValue{ - {AttributeValue: aws.String("none")}, + {AttributeValue: aws.String(vpcID)}, }, }) } diff --git a/spinifex/gateway/ec2/account/account_test.go b/spinifex/gateway/ec2/account/account_test.go index a390e9186..2d3e54110 100644 --- a/spinifex/gateway/ec2/account/account_test.go +++ b/spinifex/gateway/ec2/account/account_test.go @@ -2,6 +2,7 @@ package gateway_ec2_account import ( "context" + "errors" "testing" "github.com/aws/aws-sdk-go/aws" @@ -14,7 +15,7 @@ import ( func TestDescribeAccountAttributes_AllAttributes(t *testing.T) { input := &ec2.DescribeAccountAttributesInput{} - output, err := DescribeAccountAttributes(input) + output, err := DescribeAccountAttributes(input, noDefaultVPC) require.NoError(t, err) require.NotNil(t, output) @@ -38,7 +39,7 @@ func TestDescribeAccountAttributes_FilterSingle(t *testing.T) { AttributeNames: []*string{aws.String("max-instances")}, } - output, err := DescribeAccountAttributes(input) + output, err := DescribeAccountAttributes(input, noDefaultVPC) require.NoError(t, err) require.NotNil(t, output) @@ -55,7 +56,7 @@ func TestDescribeAccountAttributes_FilterMultiple(t *testing.T) { }, } - output, err := DescribeAccountAttributes(input) + output, err := DescribeAccountAttributes(input, noDefaultVPC) require.NoError(t, err) require.NotNil(t, output) @@ -74,7 +75,7 @@ func TestDescribeAccountAttributes_FilterNonExistent(t *testing.T) { AttributeNames: []*string{aws.String("nonexistent-attribute")}, } - output, err := DescribeAccountAttributes(input) + output, err := DescribeAccountAttributes(input, noDefaultVPC) require.NoError(t, err) require.NotNil(t, output) @@ -86,7 +87,7 @@ func TestDescribeAccountAttributes_EmptyAttributeNames(t *testing.T) { AttributeNames: []*string{}, } - output, err := DescribeAccountAttributes(input) + output, err := DescribeAccountAttributes(input, noDefaultVPC) require.NoError(t, err) require.NotNil(t, output) @@ -94,6 +95,37 @@ func TestDescribeAccountAttributes_EmptyAttributeNames(t *testing.T) { assert.Len(t, output.AccountAttributes, 6) } +func noDefaultVPC() (string, error) { return "", nil } + +func TestDescribeAccountAttributes_DefaultVPC(t *testing.T) { + named := &ec2.DescribeAccountAttributesInput{AttributeNames: []*string{aws.String("default-vpc")}} + + output, err := DescribeAccountAttributes(named, func() (string, error) { return "vpc-0abc", nil }) + require.NoError(t, err) + require.Len(t, output.AccountAttributes, 1) + assert.Equal(t, "vpc-0abc", aws.StringValue(output.AccountAttributes[0].AttributeValues[0].AttributeValue)) + + output, err = DescribeAccountAttributes(named, noDefaultVPC) + require.NoError(t, err) + assert.Equal(t, "none", aws.StringValue(output.AccountAttributes[0].AttributeValues[0].AttributeValue)) +} + +// A failed lookup fails the request rather than reporting "none" for an +// account that may well have a default VPC; other attributes never look. +func TestDescribeAccountAttributes_DefaultVPCLookupFailure(t *testing.T) { + failing := func() (string, error) { return "", errors.New(awserrors.ErrorServerInternal) } + + _, err := DescribeAccountAttributes(&ec2.DescribeAccountAttributesInput{}, failing) + require.Error(t, err) + assert.Equal(t, awserrors.ErrorServerInternal, err.Error()) + + output, err := DescribeAccountAttributes(&ec2.DescribeAccountAttributesInput{ + AttributeNames: []*string{aws.String("max-instances")}, + }, failing) + require.NoError(t, err) + assert.Len(t, output.AccountAttributes, 1) +} + func TestValidateEnableEbsEncryptionByDefaultInput(t *testing.T) { tests := []struct { name string diff --git a/spinifex/gateway/gateway_test.go b/spinifex/gateway/gateway_test.go index 351f77db6..ada093f5d 100644 --- a/spinifex/gateway/gateway_test.go +++ b/spinifex/gateway/gateway_test.go @@ -22,6 +22,7 @@ import ( handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" "github.com/mulgadc/spinifex/spinifex/testutil" "github.com/mulgadc/spinifex/spinifex/types" + "github.com/mulgadc/spinifex/spinifex/utils" "github.com/nats-io/nats.go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -1124,9 +1125,10 @@ func TestEC2Request_MissingAccountID(t *testing.T) { assert.Equal(t, awserrors.ErrorInternalError, err.Error()) } +// Static attributes are served without NATS; only default-vpc needs a lookup. func TestEC2Request_DescribeAccountAttributes(t *testing.T) { gw := &GatewayConfig{DisableLogging: true, NATSConn: nil, IAMService: allowAllIAMService()} - req := setupEC2Request("Action=DescribeAccountAttributes", "123456789012") + req := setupEC2Request("Action=DescribeAccountAttributes&AttributeName.1=max-instances", "123456789012") w := httptest.NewRecorder() err := gw.EC2_Request(w, req) @@ -1140,6 +1142,32 @@ func TestEC2Request_DescribeAccountAttributes(t *testing.T) { assert.Contains(t, string(body), "DescribeAccountAttributesResponse") } +// default-vpc is the caller's own default VPC, asked for with the is-default +// filter so a deleted default VPC reads as none. +func TestEC2Request_DescribeAccountAttributes_DefaultVPC(t *testing.T) { + nc := startTestNATS(t) + sub, err := nc.Subscribe("ec2.DescribeVpcs", func(msg *nats.Msg) { + var in awsec2.DescribeVpcsInput + if err := json.Unmarshal(msg.Data, &in); err != nil || len(in.Filters) != 1 || + aws.StringValue(in.Filters[0].Name) != "is-default" || msg.Header.Get(utils.AccountIDHeader) != "123456789012" { + _ = msg.Respond([]byte(`{}`)) + return + } + data, _ := json.Marshal(awsec2.DescribeVpcsOutput{Vpcs: []*awsec2.Vpc{{VpcId: aws.String("vpc-0default"), IsDefault: aws.Bool(true)}}}) + _ = msg.Respond(data) + }) + require.NoError(t, err) + t.Cleanup(func() { _ = sub.Unsubscribe() }) + + gw := &GatewayConfig{DisableLogging: true, NATSConn: nc, IAMService: allowAllIAMService()} + req := setupEC2Request("Action=DescribeAccountAttributes&AttributeName.1=default-vpc", "123456789012") + w := httptest.NewRecorder() + + require.NoError(t, gw.EC2_Request(w, req)) + body, _ := io.ReadAll(w.Result().Body) + assert.Contains(t, string(body), "vpc-0default") +} + func TestEC2Request_DescribeAvailabilityZones(t *testing.T) { gw := &GatewayConfig{ DisableLogging: true,