Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task
georeeve opened this issue Oct 16, 2024 · 3 comments · May be fixed by #31844
Open
1 task
Labels
@aws-cdk/aws-amplify Related to AWS Amplify bug This issue is a bug. effort/small Small work item – less than a day of effort p3

Comments

@georeeve
Copy link

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 georeeve added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 16, 2024
@github-actions github-actions bot added the @aws-cdk/aws-amplify Related to AWS Amplify label Oct 16, 2024
@georeeve
Copy link
Author

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

@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 and removed needs-triage This issue or PR still needs to be triaged. labels Oct 16, 2024
@khushail khushail self-assigned this Oct 16, 2024
@khushail
Copy link
Contributor

khushail commented Oct 21, 2024

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
Copy link
Contributor

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

@khushail khushail added p3 effort/small Small work item – less than a day of effort and removed p2 investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Oct 21, 2024
@khushail khushail removed their assignment Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-amplify Related to AWS Amplify bug This issue is a bug. effort/small Small work item – less than a day of effort p3
Projects
None yet
2 participants