Skip to content

chore: add skew-version support in Cloudflare open-next worker #7980

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dario-piotrowicz
Copy link
Member

Description

This PR adds support for skew protection in the Cloudflare open-next deployment (which has been recently added to the open-next adapter (in 1.5.0))

For more details on the feature see: https://opennext.js.org/cloudflare/howtos/skew

Validation

I've applied the changes and deployed the site: https://nodejs-org.testing.devprod.cloudflare.dev/en
And everything seems to be working as expected 🙂

I'll double check with @vicb if he has some suggestion to best test that the skew protection is actually in place and working as expected 🙂

Related Issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run pnpm format to ensure the code follows the style guide.
  • I have run pnpm test to check if all tests are passing.
  • I have run pnpm build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

Copy link

vercel bot commented Jul 14, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview Jul 15, 2025 10:07pm

Copy link

codecov bot commented Jul 14, 2025

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 73.09%. Comparing base (b035290) to head (9256c7a).
Report is 4 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
packages/rehype-shiki/src/index.mjs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7980      +/-   ##
==========================================
- Coverage   73.10%   73.09%   -0.02%     
==========================================
  Files          95       95              
  Lines        8355     8354       -1     
  Branches      219      219              
==========================================
- Hits         6108     6106       -2     
- Misses       2246     2247       +1     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dario-piotrowicz
Copy link
Member Author

https://github.com/nodejs/nodejs.org/actions/runs/16280236085/job/45968187377?pr=7980#step:11:74 👀

again! is this something that is happening quite often? 😓

@avivkeller
Copy link
Member

https://github.com/nodejs/nodejs.org/actions/runs/16280236085/job/45968187377?pr=7980#step:11:74 👀

again! is this something that is happening quite often? 😓

Yea, it happens pretty often now that I think about it.

https://github.com/nodejs/nodejs.org/actions/workflows/playwright-cloudflare-open-next.yml?query=is%3Afailure

@dario-piotrowicz
Copy link
Member Author

https://github.com/nodejs/nodejs.org/actions/runs/16280236085/job/45968187377?pr=7980#step:11:74 👀
again! is this something that is happening quite often? 😓

Yea, it happens pretty often now that I think about it.

https://github.com/nodejs/nodejs.org/actions/workflows/playwright-cloudflare-open-next.yml?query=is%3Afailure

oof... and do you know if it is just happening on the open-next build?

@avivkeller
Copy link
Member

oof... and do you know if it is just happening on the open-next build?

yes, it's not duplicated on the regular Playwright run. We've had no1 failures on that.

Footnotes

  1. Excluding changes that you would expect to break Playwright.

@dario-piotrowicz
Copy link
Member Author

oof... and do you know if it is just happening on the open-next build?

yes, it's not duplicated on the regular Playwright run. We've had no1 failures on that.

Footnotes

  1. Excluding changes that you would expect to break Playwright.

ok, so sorry for the inconvenience 🙇 I'll look into addressing the issue then 🙇

@avivkeller
Copy link
Member

No problem! It's not a required check so it's not breaking anything

@dario-piotrowicz
Copy link
Member Author

No problem! It's not a required check so it's not breaking anything

good! but it must be pretty annoying 😅😓

@dario-piotrowicz dario-piotrowicz force-pushed the dario/open-next-skew-protection branch from 3f20cb6 to 9256c7a Compare July 15, 2025 22:05
@dario-piotrowicz dario-piotrowicz marked this pull request as ready for review July 15, 2025 22:11
@dario-piotrowicz dario-piotrowicz requested review from a team as code owners July 15, 2025 22:11
Copy link
Member

@AugustinMauroy AugustinMauroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT !

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Buit can we document this variable for the time being somewhere? (the OPEN_NEXT_CLOUDFLARE)

@ovflowd
Copy link
Member

ovflowd commented Jul 16, 2025

LGMT !

Every time you write LGMT I feel like I want to eat Subway's BLT for some reason

@AugustinMauroy

This comment was marked as off-topic.

@avivkeller
Copy link
Member

LGMT !

Every time you write LGMT I feel like I want to eat Subway's BLT for some reason

Looks good, me too!

@ovflowd

This comment was marked as off-topic.

@AugustinMauroy

This comment was marked as off-topic.

@avivkeller avivkeller added the github_actions:pull-request Trigger Pull Request Checks label Jul 16, 2025
@avivkeller avivkeller enabled auto-merge July 16, 2025 22:59
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Jul 16, 2025
@dario-piotrowicz
Copy link
Member Author

Sorry @avivkeller let's wait for tomorrow to merge this if that's ok 🙏

It should be all good but before merging I wanted to double check that the deployed version does implement version skew protection correctly, I'll do that at some point tomorrow 🙇

@dario-piotrowicz
Copy link
Member Author

Sorry @avivkeller let's wait for tomorrow to merge this if that's ok 🙏

It should be all good but before merging I wanted to double check that the deployed version does implement version skew protection correctly, I'll do that at some point tomorrow 🙇

Sorry, I'm having some trouble validating that the skew protection works correctly (since it seems like it (intentionally) does not work on .workers.dev subdomains and I don't currently have a custom domain at hand), I'll let y'all know when I manage to validate this change 🙂

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.

8 participants