pulumi / pulumi-aws-apigateway

Apache License 2.0
12 stars 5 forks source link

Binary media types set to `*/*` if you have more than one route assigned #118

Closed pierskarsenbarg closed 9 months ago

pierskarsenbarg commented 9 months ago

What happened?

If you set the binaryMediaTypes input to be an empty array (so nothing is set) and you have more than one route set, the binaryMediaTypes is set to [*/*]. If you then re-run the same update with no changes then it is corrected and the list of binary media types is empty.

If you set the binaryMediaTypes input to be an empty array (so nothing is set) and you have one route set, then no binary media types are set (this is what we expect).

Example

import * as aws from "@pulumi/aws";
import * as apigateway from "@pulumi/aws-apigateway";

const f = new aws.lambda.CallbackFunction("f", {
    callback: async (ev, ctx) => {
        console.log(JSON.stringify(ev));
        return {
            statusCode: 200,
            body: "goodbye",
        };
    },
});

const api = new apigateway.RestAPI("pk-aws-api", {
    binaryMediaTypes: [],
    routes: [
        {
            path: "/",
            method: "GET",
            eventHandler: f,
        },
        {
            path: "/",
            method: "POST",
            eventHandler: f,
        }
    ],
});

export const apiId = api.api.id;

Output of pulumi about

CLI          
Version      3.95.0
Go Version   go1.21.4
Go Compiler  gc

Plugins
NAME            VERSION
aws             6.13.1
aws             5.42.0
aws-apigateway  2.1.0
awsx            1.0.6
docker          3.6.1
nodejs          unknown

Host     
OS       darwin
Version  14.1.2
Arch     x86_64

This project is written in nodejs: executable='/Users/piers/.nvm/versions/node/v20.10.0/bin/node' version='v20.10.0'

Current Stack: pierskarsenbarg/api-gateway-binary/dev

TYPE                                               URN
pulumi:pulumi:Stack                                urn:pulumi:dev::api-gateway-binary::pulumi:pulumi:Stack::api-gateway-binary-dev
pulumi:providers:aws                               urn:pulumi:dev::api-gateway-binary::pulumi:providers:aws::default_5_42_0
aws:iam/role:Role                                  urn:pulumi:dev::api-gateway-binary::aws:iam/role:Role::f
aws:lambda/function:Function                       urn:pulumi:dev::api-gateway-binary::aws:lambda/function:Function::f
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-7cd09230
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-019020e7
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-e1a3786d
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-1b4caae3
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-6c156834
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-74d12784
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-a1de8170
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-b5aeb6b6
aws:iam/rolePolicyAttachment:RolePolicyAttachment  urn:pulumi:dev::api-gateway-binary::aws:iam/rolePolicyAttachment:RolePolicyAttachment::f-4aaabb8e
pulumi:providers:aws-apigateway                    urn:pulumi:dev::api-gateway-binary::pulumi:providers:aws-apigateway::default_2_1_0
pulumi:providers:pulumi                            urn:pulumi:dev::api-gateway-binary::pulumi:providers:pulumi::default
aws-apigateway:index:RestAPI                       urn:pulumi:dev::api-gateway-binary::aws-apigateway:index:RestAPI::pk-aws-api
pulumi:providers:aws                               urn:pulumi:dev::api-gateway-binary::pulumi:providers:aws::default_6_0_4
aws:apigateway/restApi:RestApi                     urn:pulumi:dev::api-gateway-binary::aws-apigateway:index:RestAPI$aws:apigateway/restApi:RestApi::pk-aws-api
aws:apigateway/deployment:Deployment               urn:pulumi:dev::api-gateway-binary::aws-apigateway:index:RestAPI$aws:apigateway/deployment:Deployment::pk-aws-api
aws:lambda/permission:Permission                   urn:pulumi:dev::api-gateway-binary::aws-apigateway:index:RestAPI$aws:lambda/permission:Permission::pk-aws-api-c171fd88
aws:lambda/permission:Permission                   urn:pulumi:dev::api-gateway-binary::aws-apigateway:index:RestAPI$aws:lambda/permission:Permission::pk-aws-api-fa520765
aws:apigateway/stage:Stage                         urn:pulumi:dev::api-gateway-binary::aws-apigateway:index:RestAPI$aws:apigateway/stage:Stage::pk-aws-api

Found no pending operations associated with dev

Backend        
Name           pulumi.com
URL            https://app.pulumi.com/pierskarsenbarg
User           pierskarsenbarg
Organizations  pierskarsenbarg, karsenbarg, team-ce, demo
Token type     personal

Dependencies:
NAME                    VERSION
@pulumi/aws-apigateway  2.1.0
@pulumi/aws             5.42.0
@pulumi/awsx            1.0.6
@pulumi/pulumi          3.93.0
@types/node             16.18.61

Pulumi locates its logs in /var/folders/69/3w1gr05s2pq36wn49bhyknym0000gn/T/ by default

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

pierskarsenbarg commented 9 months ago

It looks like this line is the cause: https://github.com/pulumi/pulumi-aws-apigateway/blob/1d4ef87b702d9b2e20d1ea25ae9adaf3a2b32226/provider/cmd/pulumi-resource-aws-apigateway/apigateway/api.ts#L820

If we were passing in an empty array or marking it as undefined this line in the generated swagger spec was conflicting and we're running into edge cases depending on whether you're updating the binaryMediaTypes input or the routes, which then re-generates the swagger body to include the x-amazon-apigateway-binary-media-types header.

If we completely remove the line linked above, it means that there's going to be no conflicts. If people want to use swagger to define their routes, then this wouldn't have been affected anyway. If users don't set the binaryMediaTypes then we fall back to */* anyway and if they do set it then we'll use that.

PR coming imminently.

raymzag commented 6 months ago

Hi, how can I set empty array binaryMediaTypes: [] when I am using yaml syntax?