pulumi / pulumi-aws

An Amazon Web Services (AWS) Pulumi resource package, providing multi-language access to AWS
Apache License 2.0
462 stars 155 forks source link

AWS apigateway RestApiPolicy is deleted on change #4296

Open stooj opened 3 months ago

stooj commented 3 months ago

Describe what happened

When managing an aws:apigateway:RestApi with an OpenAPI spec and an aws.apigateway.RestApiPolicy, any change to the API spec will result in a recreation of the RestApi. This is expected (See warning in the docs).

However, when the RestApi is recreated, the RestApiPolicy is not attached to the new policy.

Pulumi gives no indication that this resource will be deleted, so there's no warning that you need to reattach the policy manually.

Sample program

Sample __main__.py

import getpass
import pulumi_aws as aws

username = getpass.getuser()

# Load the OpenAPI specification template from file
with open("api_spec.json", "r") as file:
    openapi_spec_template = file.read()

# Create the API Gateway
api_gw = aws.apigateway.RestApi(
    f"test_api_gw-{username}",
    name=f"TestJCa API {username}",
    description="TestJCa",
    endpoint_configuration={"types": "REGIONAL"},
    # put_rest_api_mode="merge",
    body=openapi_spec_template,
)

resource_policy_template = open(
    "TestJCa-API-resource-policy.json", "r", encoding="utf-8"
).read()

aws.apigateway.RestApiPolicy(
    f"test_api_gw_resource_policy-{username}",
    rest_api_id=api_gw.id,
    policy=resource_policy_template,
)

Sample api_spec.json

{
  "openapi": "3.0.1",
  "info": {
    "title": "Sample Test API",
    "description": "A sample API to demonstrate OpenAPI Specification for AWS API Gateway.",
    "version": "1.0.0"
  },
  "servers": [
    {
      "url": "https://api.example.com/v1",
      "description": "Production server"
    },
    {
      "url": "https://sandbox.api.example.com/v1",
      "description": "Sandbox server"
    }
  ],
  "paths": {
    "/pets": {
      "get": {
        "summary": "List all pets",
        "operationId": "listPets",
        "tags": ["pets"],
        "responses": {
          "200": {
            "description": "A list of pets",
            "content": {
              "application/json": {
                "schema": {
                  "type": "array",
                  "items": {
                    "$ref": "#/components/schemas/Pet"
                  }
                }
              }
            }
          },
          "500": {
            "description": "Server error"
          }
        }
      }
    },
    "/pets/{petId}": {
      "get": {
        "summary": "Get a pet by ID",
        "operationId": "getPetById",
        "tags": ["pets"],
        "parameters": [
          {
            "name": "petId",
            "in": "path",
            "required": true,
            "description": "The ID of the pet to retrieve",
            "schema": {
              "type": "integer",
              "format": "int64"
            }
          }
        ],
        "responses": {
          "200": {
            "description": "A pet object",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Pet"
                }
              }
            }
          },
          "404": {
            "description": "Pet not found"
          },
          "500": {
            "description": "Server error"
          }
        }
      },
      "put": {
        "summary": "Update a pet by ID",
        "operationId": "updatePetById",
        "tags": ["pets"],
        "parameters": [
          {
            "name": "petId",
            "in": "path",
            "required": true,
            "description": "The ID of the pet to update",
            "schema": {
              "type": "integer",
              "format": "int64"
            }
          }
        ],
        "requestBody": {
          "description": "Pet object that needs to be updated",
          "required": true,
          "content": {
            "application/json": {
              "schema": {
                "$ref": "#/components/schemas/Pet"
              }
            }
          }
        },
        "responses": {
          "200": {
            "description": "Pet updated successfully"
          },
          "404": {
            "description": "Pet not found"
          },
          "500": {
            "description": "Server error"
          }
        }
      },
      "delete": {
        "summary": "Delete a pet by ID",
        "operationId": "deletePetById",
        "tags": ["pets"],
        "parameters": [
          {
            "name": "petId",
            "in": "path",
            "required": true,
            "description": "The ID of the pet to delete",
            "schema": {
              "type": "integer",
              "format": "int64"
            }
          }
        ],
        "responses": {
          "204": {
            "description": "Pet deleted successfully"
          },
          "404": {
            "description": "Pet not found"
          },
          "500": {
            "description": "Server error"
          }
        }
      }
    }
  },
  "components": {
    "schemas": {
      "Pet": {
        "type": "object",
        "required": ["id", "name"],
        "properties": {
          "id": {
            "type": "integer",
            "format": "int64",
            "description": "The pet ID"
          },
          "name": {
            "type": "string",
            "description": "The pet's name"
          },
          "tag": {
            "type": "string",
            "description": "The pet's tag"
          }
        }
      }
    }
  }
}

To reproduce:

  1. pulumi up
  2. Check the policy is attached:

    
    # 1. Find out the policy id
    # TODO: Change the name to whatever is appropriate for your env
    aws apigateway get-rest-apis --region=eu-central-1 | \
    jq '.items[] | select( .name == "<REPLACE_WITH_RESTAPI_NAME") | .id'
    
    # 2. Check the policy using the above id:
    
    aws apigateway --region=eu-central-1 get-rest-api --rest-api-id <REPLACE_WITH_POLICY_ID> | \
    jq '.policy'
  3. Make any change to the api spec
  4. Run pulumi up, RestAPI is recreated as expected
  5. Run the get-rest-api command again, and null will be returned. The policy is gone

Log output

No response

Affected Resource(s)

Output of pulumi about

pulumi about                                                                    ─╯
CLI
Version      3.126.0
Go Version   go1.22.5
Go Compiler  gc

Plugins
KIND      NAME    VERSION
resource  aws     6.44.0
language  python  unknown

Host
OS       nixos
Version  24.05 (Uakari)
Arch     x86_64

This project is written in python: executable='/home/stooj/.cache/pypoetry/virtualenvs/non-package-mode-0fnHLno2-py3.11/bin/python' version='3.11.9'

Current Stack: stooj/5495/dev

TYPE                                        URN
pulumi:pulumi:Stack                         urn:pulumi:dev::5495::pulumi:pulumi:Stack::5495-dev
pulumi:providers:aws                        urn:pulumi:dev::5495::pulumi:providers:aws::default_6_44_0
aws:apigateway/restApi:RestApi              urn:pulumi:dev::5495::aws:apigateway/restApi:RestApi::test_api_gw-stooj
aws:apigateway/restApiPolicy:RestApiPolicy  urn:pulumi:dev::5495::aws:apigateway/restApiPolicy:RestApiPolicy::test_api_gw_resource_policy-stooj

Found no pending operations associated with dev

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

Dependencies:
NAME                    VERSION
aiodns                  3.2.0
aiohttp                 3.9.5
asn1crypto              1.5.1
async-timeout           4.0.3
awscli                  2.15.43
azure-cli               2.60.0
azure-loganalytics      0.1.1
azure-mgmt-common       0.20.0
azure-mgmt-consumption  10.0.0
azure-mgmt-relay        1.1.0
azure-storage-blob      12.19.1
bcdoc                   0.16.0
black                   24.4.0
botocore                1.34.87
Brotli                  1.1.0
brotlicffi              1.1.0.0
curio                   1.6
Jinja2                  3.1.4
pbr                     6.0.0
pip                     24.0
poetry                  1.8.3
pulumi_aws              6.44.0
pyasn1                  0.6.0
python-socks            2.4.4
redis                   5.0.3
ruamel.base             1.0.0
setuptools              69.5.1.post0
trio                    0.25.0

Additional context

The largest concern here is that pulumi is removing resources without warning the user beforehand. I suspect this is a unique result of this bug though, so fixing the bug will mean users can rely on the preview being accurate.

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).

stooj commented 3 months ago

@t0yv0 tagging as requested. Thanks

t0yv0 commented 3 months ago

It appears that the upstream provider is designed to treat changes to body as in-place updates (it is not marked ForceNew). In-place updates do not cascade in Pulumi to dependent resources and there is no way I have found to do this to force the re-creation of aws.apigateway.RestApiPolicy whenever there is an update to the body of aws.apigateway.RestApi. One crude workaround is to apply a replace_on_changes=["body"] option to aws.apigateway.RestApi to treat this property as if it was marked with ForceNew. If this is done, any changes to body will trigger a replace plan that will replace both the RestApi and RestApiPolicy and pass the above check.

I ended up using this variation for testing (with api_spec.json as above), and check.sh:

#!/usr/bin/env bash

set -euo pipefail

aid=$(pulumi stack output apiid)

aws apigateway --region=us-east-1 get-rest-api --rest-api-id "$aid" |
    jq '.policy'
import pulumi as pulumi
import pulumi_aws as aws

username = 't0yv0'

# Load the OpenAPI specification template from file
with open("api_spec.json", "r") as file:
    openapi_spec_template = file.read()

# Create the API Gateway
api_gw = aws.apigateway.RestApi(
    f"test_api_gw-{username}",
    name=f"TestJCa API {username}",
    description="TestJCa",
    endpoint_configuration={"types": "REGIONAL"},
    # put_rest_api_mode="merge",
    body=openapi_spec_template,
    opts=pulumi.ResourceOptions(replace_on_changes=["body"]),
)

test = aws.iam.get_policy_document_output(statements=[{
    "effect": "Allow",
    "principals": [{
        "type": "AWS",
        "identifiers": ["*"],
    }],
    "actions": ["execute-api:Invoke"],
    "resources": [api_gw.execution_arn],
    "conditions": [{
        "test": "IpAddress",
        "variable": "aws:SourceIp",
        "values": ["123.123.123.123/32"],
    }],
}])

aws.apigateway.RestApiPolicy(
    f"test_api_gw_resource_policy-{username}",
    rest_api_id=api_gw.id,
    policy=test.json,
)

pulumi.export("apiid", api_gw.id)

Could you please let us know if this workaround gets the customer unblocked?

t0yv0 commented 3 months ago

Digging a bit further into this surprising behavior. So when the body of aws.apigateway.RestApi changes, without any resource options, Pulumi executes an Update plan just on the RestApi resource. It hits this code path:

https://github.com/hashicorp/terraform-provider-aws/blob/master/internal/service/apigateway/rest_api.go#L505

  // Terraform implementation uses the `overwrite` mode by default.
  // Overwrite mode will delete existing literal properties if they are not explicitly set in the OpenAPI definition.
  // The VPC endpoints deletion and immediate recreation can cause a race condition.
  //      Impacted properties: ApiKeySourceType, BinaryMediaTypes, Description, EndpointConfiguration, MinimumCompressionSize, Name, Policy
  // The `merge` mode will not delete literal properties of a RestApi if they’re not explicitly set in the OAS definition.

The overwrite update mode is the one that I believe removes policy. If I add this option:

    put_rest_api_mode="merge",

Then I'm no longer observing the policy from disappearing. Perhaps the repro I'm trying is missing some important aspect? It sounds like from the description that the customer has already tried put_rest_api_mode.

stooj commented 3 months ago

put_rest_api_mode="merge" works around the disappearing policy because it won't recreate the RestApi, but it breaks updates to the api spec (which I'd argue is usually more important).

The merge behaviour ignores any changes to the api spec that would normally remove an endpoint.

For example, if put_rest_api_mode is set to merge, then you delete the /pets path from the API spec, this path removal will not be applied to the api on any future pulumi up. (/pets is still an endpoint on the api)

So the merge workaround introduces a worse behaviour :(

t0yv0 commented 3 months ago

Thanks for clarifying that, it wasn't obvious! How about the replace_on_changes workaround, that is not acceptable either?

t0yv0 commented 1 month ago

@stooj is there anything we need to follow up on here or can we close the issue? Appreciated!