aws-cdk icon indicating copy to clipboard operation
aws-cdk copied to clipboard

(amplify-alpha): Removing `customResponseHeaders` property does not remove the headers from the app

Open georeeve opened this issue 1 year ago • 1 comments

Describe the bug

Creating an app with the customResponseHeaders property, and then removing the property does not remove the custom headers from the app.

Regression Issue

  • [ ] Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

Removing the customResponseHeaders property from the App construct should delete it from the Amplify app in the dashboard.

Current Behavior

The custom headers stay in their previous configuration.

Reproduction Steps

Create an example Amplify app with the customResponseHeaders property:

new amplify.App(this, 'App', {
  customResponseHeaders: [
    {
      pattern: '**',
      headers: {
        Test: 'test',
      },
    },
  ],
});

Then remove the customResponseHeaders property, observe that the headers are still in the Amplify dashboard.

Possible Solution

Omitting CustomHeaders in CloudFormation does not remove the headers from the app, instead we could explicitly set it to an empty string if the customResponseHeaders property is not provided, similar to how we currently handle the basicAuth property.

Additional Information/Context

No response

CDK CLI Version

2.162.1 (build 10aa526)

Framework Version

2.162.1-alpha.0

Node.js Version

v22.9.0

OS

macOS 14.7 (23H124)

Language

TypeScript

Language Version

TypeScript 5.6.3

Other information

No response

georeeve avatar Oct 16 '24 15:10 georeeve

I'm happy to submit a PR with the proposed solution, if that's what we want to do.

georeeve avatar Oct 16 '24 15:10 georeeve

Hi @georeeve , thanks for reaching out.

I tried to repro the scenario by following the shared instructions and code. The app successfully deployed with custom headers initially but when the customResourceHeaders were removed, although I see the cdk diff result - Screenshot 2024-10-21 at 2 56 23 PM

the deployed template also has the change -


"amplifyapp996AC4E0": {
      "Type": "AWS::Amplify::App",
      "Properties": {
        "BasicAuthConfig": {
          "EnableBasicAuth": false
        },
        "EnableBranchAutoDeletion": true,
        "IAMServiceRole": {
          "Fn::GetAtt": [
            "amplifyappRole19C50DA6",
            "Arn"
          ]
        },
        "Name": "amplifyapp",
        "Platform": "WEB"
      },
      "Metadata": {
        "aws:cdk:path": "AmplifyalphaStack/amplifyapp/Resource"
      }
    },

but the change is not propagated to Amplify app -

Screenshot 2024-10-21 at 3 01 37 PM

Checking the clooudformation docs, this field does not have any defaults -

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-amplify-app.html#cfn-amplify-app-customheaders

As per the diff result, During deployment, cloudformation should update the App's customResponseheader and delete it. So this definitely seems to be a bug.

However passing an empty array did the trick for me -


    const amplifyapp = new amplify.App(this, 'amplifyapp', {
      customResponseHeaders: [],
      autoBranchDeletion: true, // Automatically disconnect a branch when you delete a branch from your repository
    }); 

Result of cdk diff -

Screenshot 2024-10-21 at 3 27 31 PM

Amplify app updated with empty customResponseHeader -

Screenshot 2024-10-21 at 3 28 47 PM

Please feel free to submit a PR. Team would be happy to review it.

khushail avatar Oct 21 '24 22:10 khushail

I would be marking this issue as P3 as workaround exists , and contributions are welcome in this regard.

khushail avatar Oct 21 '24 22:10 khushail

Hi @georeeve,

Thank you for reporting this issue. This appears to be a bug in either CloudFormation or Amplify's handling of the customResponseHeaders property removal. While the CloudFormation template correctly shows the removal of the headers, the changes are not being propagated to the Amplify app itself.

I've created an internal ticket to track this issue with our service teams. We'll follow up on this thread if there are any updates or when we have a resolution.

Abogical avatar Aug 07 '25 14:08 Abogical

Hi @Abogical, thanks, if this is an issue with CFN or within Amplify, should we be fixing it within CDK or should we close the PR for this (#31844)? I think the best case would be if it could be fixed within either CFN or Amplify, so we don't have to do any kind of special handling for it within CDK?

georeeve avatar Aug 08 '25 10:08 georeeve

Hi @georeeve ! You're correct that it should ideally be fixed by CFN or Amplify. However, I'm unsure when exactly this issue will be resolved on their end. In the meantime however, your PR is simple and good enough workaround. I see no harm in merging it for the time being.

Abogical avatar Aug 08 '25 10:08 Abogical

Sure, I'll solve the conflicts and update the integ tests. Can you also unlock the comments on that PR? It was locked after it was closed as stale, but wasn't unlocked when the PR was reopened (see #34890).

georeeve avatar Aug 08 '25 10:08 georeeve

@georeeve Sure! I've unlocked it now.

Abogical avatar Aug 08 '25 10:08 Abogical