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 content loading #235

Merged
merged 4 commits into from
Apr 10, 2025
Merged

Fix content loading #235

merged 4 commits into from
Apr 10, 2025

Conversation

rmainwork
Copy link
Contributor

@rmainwork rmainwork commented Mar 27, 2025

Add Vercel deployment protection bypass header to both the CI(broken link checker) and the two fetch() requests used to fetch internal docs content. The latter was causing HTTP requests to fail before due to the Vercel authentication page preventing access to the Vercel "CDN"(public folder) used to host and share content.

Iterates on #230

Copy link

github-actions bot commented Mar 27, 2025

Vercel Previews Deployed

Name Status Preview Updated (UTC)
Dev Portal ✅ Ready (Inspect) Visit Preview Thu Apr 10 18:41:38 UTC 2025
Unified Docs API ✅ Ready (Inspect) Visit Preview Thu Apr 10 18:37:35 UTC 2025

# paths:
# Hello Security 👋, we are checking to make sure forked repo PR changed paths are only in content/** inside the job security-check.
# We are doing this so we can also reuse this workflow for internal PRs, as pull_request_target also triggers on internal PRs. (As does pull_request)
push:
Copy link
Contributor

@RubenSandwich RubenSandwich Mar 31, 2025

Choose a reason for hiding this comment

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

@rmainwork because this is file is a pull_request_target workflow file, your changes in this PR will not go into effect in this PR. (This because security is bigger concern with pull_request_target actions and will only update for changes on main.) Please duplicate this workflow file into a new push/pull_request workflow file so that you can see your test runs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I copied the workflow into another file, pushed it and (with lots of rebasing afterward) tested it to make sure it works - it seems like it does.

@rmainwork rmainwork force-pushed the rm/fix-content-loading branch 2 times, most recently from df8e931 to a998ab1 Compare April 7, 2025 19:57
Builds were previously breaking when UDR attempted to make HTTP requests to the
CDN (vercel public folder) to retrieve content.

Adding the `x-vercel-protection-bypass` header should fix that
@rmainwork rmainwork force-pushed the rm/fix-content-loading branch from a998ab1 to 0867b58 Compare April 7, 2025 20:08
@rmainwork rmainwork marked this pull request as ready for review April 7, 2025 20:09
@rmainwork rmainwork requested a review from a team as a code owner April 7, 2025 20:09
Comment on lines +15 to +19
const headers = process.env.VERCEL_URL
? new Headers({
'x-vercel-protection-bypass': process.env.VERCEL_AUTOMATION_BYPASS_SECRET,
})
: new Headers()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add 'use server' to the top of the file so this is only run server-side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added that in 4f2ecae

@heatlikeheatwave
Copy link
Contributor

Just left a suggestion, but otherwise lgtm

@heatlikeheatwave
Copy link
Contributor

Sorry about closing this! It was a mistake made while refactoring the the repo-sync workflow.

@rmainwork rmainwork requested review from a team as code owners April 10, 2025 17:30
@rmainwork rmainwork force-pushed the rm/fix-content-loading branch from 5db12c6 to 4f2ecae Compare April 10, 2025 17:45
Adding a 'use server' directive to this file causes 404s on the content
API.

This reverts commit 4f2ecae.
@rmainwork
Copy link
Contributor Author

I'm going to merge this PR. There are issues with failing checks (or there were) that are related to the incorrect header formatting (key value instead of key=value) in the GH action. That was fixed here but doesn't take effect until the PR is merged into main, therefore if the link checker runs on this PR, that will continue to fail until the updated action is merged into main.

@rmainwork rmainwork merged commit 22a0b56 into main Apr 10, 2025
10 checks passed
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.

4 participants