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

fix : support overrideAction for WAF #582

Merged
merged 1 commit into from
Feb 11, 2024

Conversation

yasuto-nishii
Copy link
Contributor

@yasuto-nishii yasuto-nishii commented Mar 20, 2023

Fix for #578.

When v1 to v2 migration, overrideAction in WAF code was missed to migrate.

override action is typically None: {}, or Count : {}
https://docs.aws.amazon.com/waf/latest/APIReference/API_OverrideAction.html

I referred around this code in v1 branch.
If overrideAction is set, use overrideAction. Otherwise, Action is used same as before.
https://github.com/sid88in/serverless-appsync-plugin/blob/v1.14.0/src/index.js#L1612

tested with AWS

  • cloud formation was generated same as v1
  • sls deploy suceeded
      "Name": "AWSManagedRulesCommonRuleSet",
      "Priority": 20,
      "Statement": {
        "ManagedRuleGroupStatement": {
          "VendorName": "AWS",
          "Name": "AWSManagedRulesCommonRuleSet"
        }
      },
      "VisibilityConfig": {
        "CloudWatchMetricsEnabled": true,
        "MetricName": "AWSManagedRulesCommonRuleSet",
        "SampledRequestsEnabled": true
      },
      "OverrideAction": {
        "None": {}
      }
    },

@yasuto-nishii yasuto-nishii force-pushed the fix/waf-overrideaction branch 2 times, most recently from ab82bc9 to d3742d2 Compare March 20, 2023 05:22
@yasuto-nishii yasuto-nishii reopened this Mar 20, 2023
@yasuto-nishii yasuto-nishii marked this pull request as draft March 20, 2023 05:23
@yasuto-nishii yasuto-nishii marked this pull request as ready for review March 22, 2023 10:44
@yasuto-nishii
Copy link
Contributor Author

yasuto-nishii commented Mar 22, 2023

@bboure
Hi, to address WAF CFn issue #578, I created PR based on v1 implementation.
https://github.com/sid88in/serverless-appsync-plugin/blob/v1.14.0/src/index.js#L1612

I had tested on my actual AWS env by installing my branch, also I verified jest result is expected one.
Hope this code helps to fix the bug.

@bboure
Copy link
Collaborator

bboure commented Nov 10, 2023

Sorry for taking so long to review this, and thanks for this!

I haven't used WAF for a long time, but the changes and explanations look good to me.

You will need to rebase/resolve conflicts though.

@yasuto-nishii yasuto-nishii reopened this Nov 13, 2023
@yasuto-nishii
Copy link
Contributor Author

yasuto-nishii commented Nov 13, 2023

@bboure
Thanks for reply. I had rebased and resolved the conflict.

@bboure
Copy link
Collaborator

bboure commented Feb 11, 2024

Thanks. ! @yasuto-nishii

@bboure bboure merged commit e99c51a into sid88in:master Feb 11, 2024
@ShuLaPy
Copy link

ShuLaPy commented May 30, 2024

Guys this commit is failing the build with the error Property 'overrideAction' does not exist on type 'WafRuleCustom'.. @yasuto-nishii you forgot to add type for rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants