-
Notifications
You must be signed in to change notification settings - Fork 14
fix: generate default allowedHosts with available variables instead of skipping all due to a single missing one #441
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
Merged
Merged
Changes from all commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
63c4b60
update deps, apply prettier, change to ESM, update tests
hrishikesh-k d723b43
update ci tests to use Node 22
hrishikesh-k a41c8f4
coderabbit 1
hrishikesh-k 5228f75
remove exclude
hrishikesh-k a772d28
update lockfiles
hrishikesh-k aa3369d
lint fixes
hrishikesh-k a87042c
fix prettier config
hrishikesh-k f17a9b0
fix npm ci
hrishikesh-k 2d4a587
coderabbit 2
hrishikesh-k 0fed716
downgrade and pin angular
hrishikesh-k a4c1f7e
update engine version range to match Angular
hrishikesh-k 49f5442
fix version and readme
hrishikesh-k 8ee6796
update readme to suggest latest CLI
hrishikesh-k 1967b06
add allowedHosts and trustedProxyHeaders
hrishikesh-k 43517b0
Merge branch 'main' into hk/angular-22
hrishikesh-k e47da59
regenrate lockfiles
hrishikesh-k 65c394d
fix type for getContext
hrishikesh-k 766344a
update readme for deploy instructions
hrishikesh-k ed297f1
prevent missing env vars from failing build
hrishikesh-k 306b642
fix for loop bug
hrishikesh-k a3cb3b7
update swapped file templates
hrishikesh-k fc84504
Merge branch 'main' into hk/angular-22
hrishikesh-k 9877951
fix context potentially being undefined
hrishikesh-k 74d27cc
consolidate app-engine exports in a single file
hrishikesh-k dc29e1a
conditionally update Angular config
hrishikesh-k 846d7fc
revert pathname to request
hrishikesh-k 35aa5cb
fix typo
hrishikesh-k 065cff2
Merge branch 'main' into hk/angular-22
hrishikesh-k 4a44694
fix merge conflicts
hrishikesh-k ca53395
Merge branch 'main' into hk/angular-22
hrishikesh-k 285d979
match version of angular/ssr instead of core
hrishikesh-k 5cafda5
Update README.md
hrishikesh-k d8af697
seperate version detection for angular/core and ssr
hrishikesh-k 08952e1
update tests
hrishikesh-k 095815b
update failBuild message and remove dead code
hrishikesh-k 6058b19
Merge branch 'main' into hk/angular-22
hrishikesh-k 3d96443
update readme
hrishikesh-k 36a92f3
allow specifying own hosts
hrishikesh-k e66d9ea
Merge branch 'main' into hk/angular-22
hrishikesh-k 3e6508d
don't inject defaults when allowedHosts includes `*`
hrishikesh-k 27aa870
allow trusting all proxy headers
hrishikesh-k edc7fce
fix set logic
hrishikesh-k 3d15f4c
fix type definition
hrishikesh-k 0d7a751
prettify
hrishikesh-k 13bb97a
remove unnecessary config options
hrishikesh-k File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,54 +1,142 @@ | ||
| import { env } from 'node:process' | ||
|
|
||
| /** | ||
| * Generate `allowedHosts` config for `AngularAppEngine` from `@angular/ssr` | ||
| * @returns {string[]} | ||
| */ | ||
| export function getAllowedHosts() { | ||
| const allowedHosts = [] | ||
| const defaultAllowedHosts = [] | ||
|
|
||
| let deployId | ||
| let deployPrimeUrlHostname | ||
| let siteId | ||
| let siteName | ||
|
|
||
| const environmentVariables = ['DEPLOY_ID', 'DEPLOY_PRIME_URL', 'DEPLOY_URL', 'SITE_ID', 'SITE_NAME', 'URL'] | ||
|
|
||
| for (const environmentVariable of environmentVariables) { | ||
| if (!env[environmentVariable] || env[environmentVariable] === 'undefined') { | ||
| console.warn( | ||
| `Missing Netlify-specific environment variable ${environmentVariable}. \`allowedHosts\` config might be incomplete.`, | ||
| ) | ||
| return allowedHosts | ||
| switch (environmentVariable) { | ||
| case 'DEPLOY_ID': | ||
| // not setting this directly above so that validation | ||
| // and warnings remain centralized in the helper | ||
| deployId = getEnvironmentVariable(environmentVariable) | ||
| break | ||
| case 'DEPLOY_PRIME_URL': | ||
| // DEPLOY_PRIME_URL: | ||
| // <branch-name>--<subdomain>.netlify.app or | ||
| // <dp-#>--<subdomain>.netlify.app (supports ADS) | ||
| deployPrimeUrlHostname = getHostnameFromEnvironmentVariable(environmentVariable) | ||
| if (deployPrimeUrlHostname) { | ||
| defaultAllowedHosts.push(deployPrimeUrlHostname) | ||
| } | ||
| break | ||
| case 'DEPLOY_URL': | ||
| case 'URL': { | ||
| // DEPLOY_URL: <deploy-id>--<subdomain>.netlify.app | ||
| // URL: <subdomain>.netlify.app OR <custom-domain> | ||
| // www handling is not required as Netlify auto-redirects | ||
| const hostname = getHostnameFromEnvironmentVariable(environmentVariable) | ||
| if (hostname) { | ||
| defaultAllowedHosts.push(hostname) | ||
| } | ||
| break | ||
| } | ||
| case 'SITE_ID': | ||
| // SITE_ID: <site_id> | ||
| // this makes it <site_id>.netlify.app | ||
| // separate handling because we need it later | ||
| siteId = getEnvironmentVariable(environmentVariable) | ||
| if (siteId) { | ||
| defaultAllowedHosts.push(`${siteId}.netlify.app`) | ||
| } | ||
| break | ||
| case 'SITE_NAME': | ||
| // SITE_NAME: <subdomain> | ||
| // this makes it <subdomain>.netlify.app | ||
| // this may duplicate URL for sites without a custom domain | ||
| // but duplicates are removed later and this is still useful | ||
| // if a custom domain is removed after deployment | ||
| siteName = getEnvironmentVariable(environmentVariable) | ||
| if (siteName) { | ||
| defaultAllowedHosts.push(`${siteName}.netlify.app`) | ||
| } | ||
| break | ||
| default: | ||
| break | ||
| } | ||
| } | ||
|
|
||
| const deployPrimeUrlHostname = new URL(env.DEPLOY_PRIME_URL).hostname | ||
|
|
||
| // <subdomain>.netlify.app OR <custom-domain> | ||
| // www handling is not required as Netlify will auto-redirect | ||
| allowedHosts.push(new URL(env.URL).hostname) | ||
| // <deploy-id>--<subdomain>.netlify.app | ||
| allowedHosts.push(new URL(env.DEPLOY_URL).hostname) | ||
| // <branch-name>--<subdomain>.netlify.app or <dp-#>--<subdomain>.netlify.app (supports ADS) | ||
| allowedHosts.push(deployPrimeUrlHostname) | ||
| // <subdomain>.netlify.app | ||
| // this will be duplicated for sites without custom domain | ||
| // but it's important to have in case a site's custom domain is removed after a deploy | ||
| allowedHosts.push(`${env.SITE_NAME}.netlify.app`) | ||
| // <site-id>.netlify.app | ||
| allowedHosts.push(`${env.SITE_ID}.netlify.app`) | ||
| // <deploy-id>--<site-id>.netlify.app | ||
| allowedHosts.push(`${env.DEPLOY_ID}--${env.SITE_ID}.netlify.app`) | ||
|
|
||
| // we need to extract the branch name or the deploy preview number | ||
| // so we can add the subdomain as well as site-id specific URLs | ||
| // this would be required for sites using ADS so that | ||
| // we can add netlify.app URLs as well | ||
| if (deployPrimeUrlHostname.includes('--')) { | ||
| // extract the branch name or deploy preview number | ||
| // so we can generate ADS-compatible URLs | ||
| if (deployPrimeUrlHostname?.includes('--') && siteId && siteName) { | ||
| const [branchNameOrDpNumber] = deployPrimeUrlHostname.split('--') | ||
| allowedHosts.push(`${branchNameOrDpNumber}--${env.SITE_NAME}.netlify.app`) | ||
| allowedHosts.push(`${branchNameOrDpNumber}--${env.SITE_ID}.netlify.app`) | ||
| defaultAllowedHosts.push(`${branchNameOrDpNumber}--${siteName}.netlify.app`) | ||
| defaultAllowedHosts.push(`${branchNameOrDpNumber}--${siteId}.netlify.app`) | ||
| } | ||
|
|
||
| if (deployId && siteId) { | ||
| // <deploy-id>--<site-id>.netlify.app | ||
| defaultAllowedHosts.push(`${deployId}--${siteId}.netlify.app`) | ||
| } | ||
|
|
||
| return allowedHosts | ||
| return [...new Set(defaultAllowedHosts)] | ||
| } | ||
|
|
||
| /** | ||
| * Return Netlify-specific context | ||
| * @returns {import('@netlify/edge-functions').Context | undefined} | ||
| */ | ||
| export function getContext() { | ||
| // eslint-disable-next-line no-undef | ||
| return typeof Netlify !== 'undefined' ? Netlify?.context : undefined | ||
| } | ||
|
|
||
| /** | ||
| * Return the value of an environment variable | ||
| * @param {string} environmentVariable | ||
| * | ||
| * @returns {string | undefined} | ||
| */ | ||
| function getEnvironmentVariable(environmentVariable) { | ||
| const value = env[environmentVariable] | ||
|
|
||
| // we inject the literal string "undefined" for variables that don't have a value | ||
| if (value == null || value === '' || value === 'undefined') { | ||
| console.warn( | ||
| `Missing Netlify-specific environment variable ${environmentVariable}. ` + | ||
| '`allowedHosts` config might be incomplete.', | ||
| ) | ||
|
|
||
| return | ||
| } | ||
|
|
||
| return value | ||
| } | ||
|
|
||
| /** | ||
| * Return hostname from the value of an environment variable | ||
| * @param {string} environmentVariable | ||
| * | ||
| * @returns {string | undefined} | ||
| */ | ||
| function getHostnameFromEnvironmentVariable(environmentVariable) { | ||
| const value = getEnvironmentVariable(environmentVariable) | ||
|
|
||
| if (value == null) { | ||
| return | ||
| } | ||
|
|
||
| try { | ||
| return new URL(value).hostname | ||
| } catch { | ||
| console.warn(`Netlify-specific environment variable ${environmentVariable} does not contain a valid URL`) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Generate `trustProxyHeaders` config for `AngularAppEngine` from `@angular/ssr` | ||
| * @returns {string[]} | ||
| */ | ||
| export function getTrustProxyHeaders() { | ||
| return ['x-forwarded-for'] | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.