Bad request when describe key pairs with empty filters slice

stgleb commented 2 months ago

Acknowledgements

[X] I have searched (https://github.com/aws/aws-sdk/issues?q=is%3Aissue) for past instances of this issue
[X] I have verified all of my SDK modules are up-to-date (you can perform a bulk update with go get -u github.com/aws/aws-sdk-go-v2/...)

Describe the bug

After upgrading github.com/aws/aws-sdk-go-v2 to the latest version start getting 400 Bad Request on describe key pairs.

Expected Behavior

Describe keys pairs gives successfull response.

Current Behavior


2024-04-29T17:02:14.493067396Z stdout F time="2024-04-29T17:02:14Z" level=debug msg="request failed with unretryable error https response error StatusCode: 400, RequestID: f20c5dc2-c06a-44a3-921a-fb8b2cc127ba, api error InvalidRequest: The request received was invalid." cluster_id=502e63b7-e0f9-4c6f-9c53-b41bd56d9613 level_int=5 node_id=c4b77fae-125b-4db4-a975-7f2bcbf8f2b6 operation=add_node operation_id=26dd3d10-c86a-4c2c-a0ab-33892c4de8bf provider_type=eks trace_id=68dd68a6878a079a |  
-- | --
  |   | 2024-04-29 19:02:14.702 | 2024-04-29T17:02:14.492658324Z stdout F time="2024-04-29T17:02:14Z" level=debug msg="Response\nHTTP/1.1 400 Bad Request\r\nConnection: close\r\nTransfer-Encoding: chunked\r\nCache-Control: no-cache, no-store\r\nContent-Type: text/xml;charset=UTF-8\r\nDate: Mon, 29 Apr 2024 17:02:14 GMT\r\nServer: AmazonEC2\r\nStrict-Transport-Security: max-age=31536000; includeSubDomains\r\nVary: accept-encoding\r\nX-Amzn-Requestid: f20c5dc2-c06a-44a3-921a-fb8b2cc127ba\r\n\r\ne6\r\n<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Response><Errors><Error><Code>InvalidRequest</Code><Message>The request received was invalid.</Message></Error></Errors><RequestID>f20c5dc2-c06a-44a3-921a-fb8b2cc127ba</RequestID></Response>\r\n0\r\n\r\n" cluster_id=502e63b7-e0f9-4c6f-9c53-b41bd56d9613 level_int=5 node_id=c4b77fae-125b-4db4-a975-7f2bcbf8f2b6 operation=add_node operation_id=26dd3d10-c86a-4c2c-a0ab-33892c4de8bf provider_type=eks trace_id=68dd68a6878a079a |  
  |   | 2024-04-29 19:02:14.452 | 2024-04-29T17:02:14.409708305Z stdout F time="2024-04-29T17:02:14Z" level=debug msg="Request\nPOST / HTTP/1.1\r\nHost: ec2.eu-central-1.amazonaws.com\r\nUser-Agent: aws-sdk-go-v2/1.26.1 os/linux lang/go#1.22.2 md/GOOS#linux md/GOARCH#arm64 api/ec2#1.158.0\r\nContent-Length: 84\r\nAmz-Sdk-Invocation-Id: ea0bf281-01dd-49d0-9bf5-c784cc0bbf4e\r\nAmz-Sdk-Request: attempt=1; max=5\r\nAuthorization: AWS4-HMAC-SHA256 Credential=ASIAQNCLJGYSHGPHWJE3/20240429/eu-central-1/ec2/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-date;x-amz-security-token, Signature=c22e63c93c30784919837849adf5b56e7956533333b39e66269ee9a3cc2db790\r\nContent-Type: application/x-www-form-urlencoded\r\nX-Amz-Date: 20240429T170214Z\r\nX-Amz-Security-Token: IQoJb3JpZ2luX2VjEMH//////////wEaDGV1LWNlbnRyYWwtMSJHMEUCIHFUUZZyWu1KI4DkZ7hQy4ikdWSxsm3K5zO+pgKndlT8AiEA4k86VHwKhvtzyJKqQOsPunRhiTGCXW/TnCsdgmvuNdUqqwIIGhADGgwwMjgwNzUxNzc1MDgiDBTDZHlMSZ16PReoFiqIAqKwjI9J4ZorqtvILXk1AvqZB2ULgMi/QjmvTS6fcKv9c+iFHKu/2kIEYTo6jC5FMws2ea+QnP5chhOJpt+g0mwLw8dsVKI0GNShjNDXo1jObyOSWukYfSHO9z981fFq5DKiPHwmGAk+hmlAS+gIbNsTRfM3IZcgZH9bB3rzoO45OF/Y10IjRQHTeO4DWII6PquT64d3wrJJ530c8rX2cWP3Wj00oNN2bKZ0XcfqvjLlPYfzxmclYUBizMc7CZrqo/SuEZmZqWTY4ukEnIkG7m+Zdhj2esQATRNW+rSYeJGTbMKP6i0mrya5YUWhpKalfjtuM6BHGMBhI/Kd3YAVb1zGgkuefOj1cDCVpb+xBjqdATQZsUimC3z6lfQrGVR1Fl3Lkt2ThZNbw9ywuuyxTyxO7tHPvxv35bjlfkq02TP3XKv8uz7cnlNgnRXWcoHrnazSBP7kEs9YlQb0yTy2ERbDUT+A7LbQJZX6IIfXkb6Pl+wjGaJ4MqYF/6shinXEsNRGjGcMYTjg6KMUvUJEs//e5Rtf5Ghr3xcStnuOlyHPDmC9LP0gyWQ9fxWllck=\r\nAccept-Encoding: gzip\r\n\r\nAction=DescribeKeyPairs&Filter=&KeyPairId.1=key-0d42034f8dbc0254b&Version=2016-11-15" cluster_id=502e63b7-e0f9-4c6f-9c53-b41bd56d9613 level_int=5 node_id=c4b77fae-125b-4db4-a975-7f2bcbf8f2b6 operation=add_node operation_id=26dd3d10-c86a-4c2c-a0ab-33892c4de8bf provider_type=eks trace_id=68dd68a6878a079a

Reproduction Steps

Upgrade aws-sdk-go-v2 packages
Describe key pairs using sts
get 400 bad request instead of 200 ok.

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2 v1.26.1
github.com/aws/aws-sdk-go-v2/config v1.27.11
github.com/aws/aws-sdk-go-v2/credentials v1.17.11
github.com/aws/aws-sdk-go-v2/service/autoscaling v1.40.5
github.com/aws/aws-sdk-go-v2/service/ec2 v1.159.0
github.com/aws/aws-sdk-go-v2/service/ecr v1.27.4
github.com/aws/aws-sdk-go-v2/service/eks v1.42.1
github.com/aws/aws-sdk-go-v2/service/iam v1.32.0
github.com/aws/aws-sdk-go-v2/service/sts v1.28.6
github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.30.5
github.com/aws/aws-sdk-go-v2/service/kms v1.31.0
github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.16.15 // indirect
github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.28.6 // indirect
github.com/aws/aws-sdk-go-v2/service/ssm v1.50.0
github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.2 // indirect
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.1 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.5 // indirect
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.5 // indirect
github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.5 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.2 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.3.7 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.7 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.17.5 // indirect
github.com/aws/aws-sdk-go-v2/service/s3 v1.53.1 // indirect
github.com/aws/aws-sdk-go-v2/service/sns v1.29.4 // indirect
github.com/aws/aws-sdk-go-v2/service/sqs v1.31.4 // indirect
github.com/aws/aws-sdk-go-v2/service/sso v1.20.5 // indirect
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.23.4 // indirect

Compiler and Version used

1.22

Operating System and version

23.1.0 Darwin Kernel Version 23.1.0

stgleb commented 2 months ago

This is normal request response:


2024-04-30T09:55:17.364741772Z stdout F time="2024-04-30T09:55:17Z" level=debug msg="Response\nHTTP/1.1 200 OK\r\nContent-Length: 953\r\nCache-Control: no-cache, no-store\r\nContent-Type: text/xml;charset=UTF-8\r\nDate: Tue, 30 Apr 2024 09:55:16 GMT\r\nServer: AmazonEC2\r\nStrict-Transport-Security: max-age=31536000; includeSubDomains\r\nX-Amzn-Requestid: 231d5aab-64e9-4b05-85c7-d62cdc75e495\r\n\r\n<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<DescribeKeyPairsResponse xmlns=\"http://ec2.amazonaws.com/doc/2016-11-15/\">\n    <requestId>231d5aab-64e9-4b05-85c7-d62cdc75e495</requestId>\n    <keySet>\n        <item>\n            <keyName>cast-node-config-test-configuration-4c5def66</keyName>\n            <keyPairId>key-0d42034f8dbc0254b</keyPairId>\n            <keyFingerprint>0f:df:f8:e0:68:05:f3:a2:6f:ce:c9:4d:b7:bb:48:e2</keyFingerprint>\n            <keyType>rsa</keyType>\n            <createTime>2024-04-22T16:27:32.336Z</createTime>\n            <tagSet>\n                <item>\n                    <key>cast:cluster-id</key>\n                    <value>4c5def66-332a-4ef2-a611-93ad9d30caec</value>\n                </item>\n                <item>\n                    <key>cast:node-configuration</key>\n                    <value>test-configuration</value>\n                </item>\n            </tagSet>\n        </item>\n    </keySet>\n</DescribeKeyPairsResponse>\n" cluster_id=502e63b7-e0f9-4c6f-9c53-b41bd56d9613 level_int=5 node_id=5c9e0c07-4bb9-40ce-a6f4-3f525eda4af2 operation=add_node operation_id=0c4b4d77-9ec8-42b0-9af9-3d0943c4977e organization_id=d4bdbc93-c0f0-4d11-b0f5-03ab3b0a02b1 provider_type=eks trace_id=2523b738bb9faa04 |  
-- | --
  |   | 2024-04-30 11:55:17.537 | 2024-04-30T09:55:17.300503581Z stdout F time="2024-04-30T09:55:17Z" level=debug msg="Request\nPOST / HTTP/1.1\r\nHost: ec2.eu-central-1.amazonaws.com\r\nUser-Agent: aws-sdk-go-v2/1.22.2 os/linux lang/go#1.22.2 md/GOOS#linux md/GOARCH#arm64 api/ec2#1.72.1\r\nContent-Length: 76\r\nAmz-Sdk-Invocation-Id: c3fb593a-88d9-447a-a9f5-a91c48418db1\r\nAmz-Sdk-Request: attempt=1; max=5\r\nAuthorization: AWS4-HMAC-SHA256 Credential=ASIAQNCLJGYSMXV6SVBL/20240430/eu-central-1/ec2/aws4_request, SignedHeaders=amz-sdk-invocation-id;amz-sdk-request;content-length;content-type;host;x-amz-date;x-amz-security-token, Signature=e9dbb37bbf55f9fe9e958132a37b6b4f72d91258c45a1e7e738be9703509a78c\r\nContent-Type: application/x-www-form-urlencoded\r\nX-Amz-Date: 20240430T095517Z\r\nX-Amz-Security-Token: IQoJb3JpZ2luX2VjENL//////////wEaDGV1LWNlbnRyYWwtMSJHMEUCIFKhL/QkVkjv7nfKog6Dfu2HAd6SGebqcdTitLBnQsPLAiEAgtYoIuvWaNHJPZerEz1cjDSMrO6AuTifVJWlIpZqE3IqqwIIKxADGgwwMjgwNzUxNzc1MDgiDKZeFrAzqT4Or5KBcyqIAgzPUgn1WJehzCQQXaVVC+nxyo8ZR2FAm+Vn4DiA5lp7Z6saxo170RqHtY3Qcher6S+JHkWsaFYRaT1C5ToXSmYWDeia12rLWsCKc4fXj9Qq6nY/ar+SsTK5C/UGpw82pEqVoiGa210lq+tYLHezCldK/s6SXmNC1pPpSUDzyNiyLHLHnq2ChTAr1Tj9XoORpTPIA/uE7HgEi2WttTma3Ofdc5v0NUBNAA/TzPY7bpE08AI2MNfW9QPEtsdsFtikVNcW/ymVX7uYdEUGMFUwGYmCejnJRm0DPxIk1s+3fv2i7uieU7U+acb/VZzKxk++uK8SFVVUM1r6T91RBgdYDhNmzNqumUvnyTCEgMOxBjqdAeIzjFXu6i4jtGa7JJfoiP+oL5qHDikV9UUiZVuW1nyfmQqOE6G4U5TTQ4QoZE+xX8DSfbLnKgHCxooDlKiUuf57GGJpy7M5Tdput5M4Unr0JD+b1pxIFHTfJvUYh39wTorNb2DA+fKmDHUT0AAgNn53FSdVW5LvVn2/lINHLK25/H1iefQz8Oxjsyxr7FrRvjYwCLxUkXPOqNTGk9g=\r\nAccept-Encoding: gzip\r\n\r\nAction=DescribeKeyPairs&KeyPairId.1=key-0d42034f8dbc0254b&Version=2016-11-15" cluster_id=502e63b7-e0f9-4c6f-9c53-b41bd56d9613 level_int=5 node_id=5c9e0c07-4bb9-40ce-a6f4-3f525eda4af2 operation=add_node operation_id=0c4b4d77-9ec8-42b0-9af9-3d0943c4977e organization_id=d4bdbc93-c0f0-4d11-b0f5-03ab3b0a02b1 provider_type=eks trace_id=2523b738bb9faa04

for go.mod:

    github.com/aws/aws-sdk-go v1.47.9
    github.com/aws/aws-sdk-go-v2 v1.22.2
    github.com/aws/aws-sdk-go-v2/config v1.23.0
    github.com/aws/aws-sdk-go-v2/credentials v1.15.2
    github.com/aws/aws-sdk-go-v2/service/autoscaling v1.26.0
    github.com/aws/aws-sdk-go-v2/service/ec2 v1.72.1
    github.com/aws/aws-sdk-go-v2/service/ecr v1.17.12
    github.com/aws/aws-sdk-go-v2/service/eks v1.2.2
    github.com/aws/aws-sdk-go-v2/service/iam v1.3.1
    github.com/aws/aws-sdk-go-v2/service/sts v1.25.1
    github.com/aws/smithy-go v1.16.0
    github.com/aws/aws-sdk-go-v2/service/elasticloadbalancingv2 v1.23.0
    github.com/aws/aws-sdk-go-v2/service/kms v1.24.5
    github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.21.3 // indirect
    github.com/aws/aws-sdk-go-v2/service/ssm v1.37.5
    github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.5.0 // indirect
    github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.14.3 // indirect
    github.com/aws/aws-sdk-go-v2/internal/configsources v1.2.2 // indirect
    github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.5.2 // indirect
    github.com/aws/aws-sdk-go-v2/internal/ini v1.6.0 // indirect
    github.com/aws/aws-sdk-go-v2/internal/v4a v1.2.2 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.10.0 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.2.2 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.10.2 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.16.2 // indirect
    github.com/aws/aws-sdk-go-v2/service/s3 v1.42.1 // indirect
    github.com/aws/aws-sdk-go-v2/service/sns v1.20.6 // indirect
    github.com/aws/aws-sdk-go-v2/service/sqs v1.28.0 // indirect
    github.com/aws/aws-sdk-go-v2/service/sso v1.17.1 // indirect
    github.com/aws/aws-sdk-go-v2/service/ssooidc v1.19.1 // indirect

stgleb commented 2 months ago

I can see that newest version added param filter, which is empty:

Action=DescribeKeyPairs&Filter=&KeyPairId.1=key-0d42034f8dbc0254b&Version=2016-11-15

while for old version request looks like:

Action=DescribeKeyPairs&KeyPairId.1=key-0d42034f8dbc0254b&Version=2016-11-15

stgleb commented 2 months ago

Okay, I've found a root cause and potential fix for this:

old code:

    if filter.KeyPairID != "" {
        input.Filters = []ec2types.Filter{}
        input.KeyPairIds = []string{filter.KeyPairID}
    }

new code:

    if filter.KeyPairID != "" {
        input.Filters = nil
        input.KeyPairIds = []string{filter.KeyPairID}
    }

stgleb commented 2 months ago

Example programm to test it

package main

import (
    "context"
    "fmt"
    "log"
    "os"

    "github.com/aws/aws-sdk-go-v2/aws"
    "github.com/aws/aws-sdk-go-v2/config"
    "github.com/aws/aws-sdk-go-v2/service/ec2"
    "github.com/aws/aws-sdk-go-v2/service/ec2/types"
    "github.com/aws/smithy-go/logging"
)

func main() {
    // Create a logger
    logger := logging.NewStandardLogger(os.Stdout)

    // Load the Shared AWS Configuration (~/.aws/config)
    cfg, err := config.LoadDefaultConfig(context.TODO(),
        config.WithRegion("eu-central-1"),
        config.WithLogger(logger),
        config.WithClientLogMode(aws.LogRetries|aws.LogRequest|aws.LogResponse),
    )

    if err != nil {
        log.Fatalf("Unable to load SDK config, %v", err)
    }

    // Create an Amazon EC2 service client
    svc := ec2.NewFromConfig(cfg)
    input := &ec2.DescribeKeyPairsInput{
        KeyPairIds: []string{"key-0d42034f8dbc0254b"},
        Filters:    []types.Filter{},
    }
    // Call to get detailed information on key pairs
    result, err := svc.DescribeKeyPairs(context.TODO(), input)
    if err != nil {
        log.Fatalf("Error: %v", err)
    }
    // Print the IDs and names of the key pairs
    fmt.Println("Key Pairs:")
    for _, keyPair := range result.KeyPairs {
        fmt.Printf("ID: %s, Name: %s\n", aws.ToString(keyPair.KeyPairId), aws.ToString(keyPair.KeyName))
    }
}

stgleb commented 2 months ago

So, this is how this function looked like in 1.22.2

func awsEc2query_serializeDocumentFilterList(v []types.Filter, value query.Value) error {
    if len(v) == 0 {
        return nil
    }
    array := value.Array("Filter")

    for i := range v {
        av := array.Value()
        if err := awsEc2query_serializeDocumentFilter(&v[i], av); err != nil {
            return err
        }
    }
    return nil
}

and this is how it looks like in 1.26.1

func awsEc2query_serializeDocumentFilterList(v []types.Filter, value query.Value) error {
    array := value.Array("Filter")

    for i := range v {
        av := array.Value()
        if err := awsEc2query_serializeDocumentFilter(&v[i], av); err != nil {
            return err
        }
    }
    return nil
}

stgleb commented 2 months ago

now I get it, this error starts from ec2 version: v1.84.1 where change log states:

* **Dependency Update**: Upgrade smithy to 1.27.2 and correct empty query list serialization.

where this correct handling of nil slice was introduced.

stgleb commented 2 months ago

Maybe it makes sense to serialize empy, but not nil slices to empty array like Filter=[] rather then to empty string?

RanVaknin commented 2 months ago

Hi @stgleb ,

Thanks for the details. I'm looking into this with priority.

Ran

RanVaknin commented 2 months ago

Hi @stgleb ,

Thanks for your patience. After inspecting this issue, we have determined that is likely related to this issue.

The SDK for Go recently updated its serialization strategy to more closely follow the EC2 wire protocol specifications. This update changed how empty lists are serialized within API requests. Previously, empty lists were not serialized at all, effectively omitting them from requests. The new updated causes the SDK to now explicitly serializes empty lists, ensuring that they are included in the request body even when they contain no elements.

Issue with `DescribeKeyPairs` and Empty Filters:

Before the changes to the serializer in the AWS SDK for Go v2, if the Filters parameter was specified as an empty list in the DescribeKeyPairs operation, it was not serialized in the request body at all, leading to requests that effectively ignored the empty Filters field:

Behavior Before Change:

POST / HTTP/1.1
Host: ec2.amazonaws.com
...
Action=DescribeKeyPairs&Version=2016-11-15

After the changes, following the EC2 protocol more closely, the SDK began explicitly serializing empty lists. Thus, an empty Filters array resulted in including it in the serialized request, albeit with no values:

Behavior After Change:

POST / HTTP/1.1
Host: ec2.amazonaws.com
...
Action=DescribeKeyPairs&Filter=&Version=2016-11-15

We are not sure why EC2 is rejecting these requests with the InvalidRequest exception as the SDK now follows the protocol that EC2 itself has defined.

I'll upstream this to the EC2 service team and investigate further.

Thanks, Ran~

lucix-aws commented 2 months ago

The change we previously made to serialize empty lists should have only applied to awsQuery services, not ec2Query. Will be resolved by addressing #2627.

github-actions[bot] commented 2 months ago

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.

lucix-aws commented 2 months ago

We should keep this open for visibility/searchability until the upstream issue is resolved.

github-actions[bot] commented 1 month ago

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.

aws / aws-sdk-go-v2