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

feat: Origin of agent webpack chunks now changeable #659

Merged
merged 27 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f099ea8
Allow agent chunk domain to be configured NR-149705
cwli24 Aug 23, 2023
7d6c282
Add e2e test for assets proxy
cwli24 Aug 24, 2023
a7cf585
Make public-path in npm a dummy
cwli24 Aug 24, 2023
1150208
Improve asset url validation
cwli24 Aug 24, 2023
6b4324a
PR comments
cwli24 Aug 24, 2023
cfe0b75
Merge commit 'd233e1428e1a7f3fe8b5ff1803bc1a306be01246' into proxy-as…
cwli24 Aug 24, 2023
052229a
Fix linting
cwli24 Aug 24, 2023
20daa9f
Update src/common/config/state/init.js
cwli24 Aug 24, 2023
887e56b
Merge commit '6cb823842fab406a33b9698edee1932c29204df3' into proxy-as…
cwli24 Aug 30, 2023
f790ff6
Include path in asset url
cwli24 Aug 30, 2023
5e2fb08
Allow only https unless retro ssl is set false in validation
cwli24 Aug 30, 2023
2116c25
Fix e2e test
cwli24 Aug 31, 2023
9fa7616
Fix wdio test
cwli24 Sep 1, 2023
ca6e005
Revert prod arg to wdio
cwli24 Sep 1, 2023
41bbb15
Merge commit '85336a43595bbf3d2793aafe665a47650a20ed21' into proxy-as…
cwli24 Sep 1, 2023
6e28f92
Put beacon under same url validation
cwli24 Sep 1, 2023
8e4b232
Fix behavior in IE & add beacon proxy tests
cwli24 Sep 1, 2023
c0e182a
Add sm on proxy & test
cwli24 Sep 1, 2023
028c8c6
Workaround local testing for ie11
cwli24 Sep 1, 2023
d781a96
Remove validation & rework proxy
cwli24 Sep 6, 2023
021b3fa
Merge commit '71d47d6e3cac3b0a2f1bbc23447f7eb272679d3f' into proxy-sm
cwli24 Sep 6, 2023
2537f97
Update the proxy e2e tests
cwli24 Sep 6, 2023
5c51476
pr comments
cwli24 Sep 11, 2023
87ab6f3
Merge commit 'a34edb982d9f6879b2a066cfaadea7dd568ad932' into proxy-as…
cwli24 Sep 11, 2023
afa8bc3
Merge commit '2004f5fc7fbec53db5dd2bcbf1180cdb0aba5a9e' into proxy-as…
cwli24 Sep 11, 2023
029cdfd
Get micro agent to work bc of ee issue
cwli24 Sep 12, 2023
6da45ab
tweak
cwli24 Sep 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ module.exports = function (api, ...args) {
[
'./tools/babel/plugins/transform-import',
{
'(constants/)env$': '$1env.npm'
'(constants/)env$': '$1env.npm',
'(configure/)public-path$': '$1public-path.npm'
}
]
]
Expand All @@ -112,7 +113,8 @@ module.exports = function (api, ...args) {
[
'./tools/babel/plugins/transform-import',
{
'(constants/)env$': '$1env.npm'
'(constants/)env$': '$1env.npm',
'(configure/)public-path$': '$1public-path.npm'
}
]
]
Expand Down
1 change: 0 additions & 1 deletion src/common/config/state/info.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const model = {
beacon: nrDefaults.beacon,
errorBeacon: nrDefaults.errorBeacon,
// others must be populated by user
assetsPath: undefined,
licenseKey: undefined,
applicationID: undefined,
sa: undefined,
Expand Down
1 change: 1 addition & 0 deletions src/common/config/state/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const model = () => {
}
return {
allow_bfcache: true, // *cli - temporary feature flag for BFCache work
assetsPath: undefined,
cwli24 marked this conversation as resolved.
Show resolved Hide resolved
privacy: { cookies_enabled: true }, // *cli - per discussion, default should be true
ajax: { deny_list: undefined, block_internal: true, enabled: true, harvestTimeSeconds: 10 },
distributed_tracing: {
Expand Down
29 changes: 22 additions & 7 deletions src/common/url/check-url.js
cwli24 marked this conversation as resolved.
Show resolved Hide resolved
cwli24 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,12 +1,27 @@
export function isValidAssetUrl (string) {
/* Note: This API is not available in IE11 and throws in Safari v14- when base is undefined.
Hence, this will always return false for those environments.
/**
* Checks input string to see if it's acceptable as agent's public path for loading source code.
* @param {string} string - supposed url string
* @returns Absolute url if 'string' is accepted. Empty string if it's denied.
*/
export function validateAssetUrl (string) {
if (!string || typeof string !== 'string') return ''

if (string.indexOf(':/') !== -1) {
if (!string.startsWith('http')) return '' // non http schemes, such as ftp, are not allowed to be an asset source
} else { // there's no scheme on this supposed URL, so we'll assume https -- the api will require a scheme.
if (string.startsWith('/')) return '' // no relative paths...
string = 'https://' + string
}
/* Note: URL API is not available in IE11 and throws in Safari v14- when base is undefined.
Hence, this will always return empty string for those environments.
*/
let properUrl
try {
let url = new URL(string)
properUrl = new URL(string)
properUrl.protocol = 'https:' // enforce HTTPS over HTTP
} catch (_) {
return false
return ''
}
if (!string.endsWith('/')) return false // webpack concats base verbatim so this is a stringent req
return true
// We want a url with scheme, domain, and port minus username|password => origin; webpack concats this verbatim, so an ending slash is required
return properUrl.origin + '/'
cwli24 marked this conversation as resolved.
Show resolved Hide resolved
}
25 changes: 12 additions & 13 deletions src/common/url/check-url.test.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import { isValidAssetUrl } from './check-url'
import { validateAssetUrl } from './check-url'

test('isValidAssetUrl works correctly', () => {
expect(isValidAssetUrl(undefined)).toEqual(false)
expect(isValidAssetUrl(123)).toEqual(false)
expect(isValidAssetUrl('')).toEqual(false)
expect(isValidAssetUrl('blah')).toEqual(false)
expect(isValidAssetUrl('/someRelativePath')).toEqual(false)
expect(isValidAssetUrl('www.missingscheme.com')).toEqual(false)
expect(isValidAssetUrl('http://noendingslash.net')).toEqual(false)
test('validateAssetUrl works correctly', () => {
expect(validateAssetUrl(undefined)).toEqual('')
expect(validateAssetUrl(123)).toEqual('')
expect(validateAssetUrl('')).toEqual('')
expect(validateAssetUrl('/someRelativePath')).toEqual('')
expect(validateAssetUrl('data://anotherscheme.com/')).toEqual('')

expect(isValidAssetUrl('http://absolutepath.net/')).toEqual(true)
expect(isValidAssetUrl('https://www.absolutepath.org/')).toEqual(true)
expect(isValidAssetUrl('data://anotherscheme.com/')).toEqual(true)
expect(isValidAssetUrl('https://js-agent.nr.com/somepath/')).toEqual(true)
expect(validateAssetUrl('www.missingscheme.com/')).toEqual('https://www.missingscheme.com/')
expect(validateAssetUrl('http:/enforcescheme.net/')).toEqual('https://enforcescheme.net/')
expect(validateAssetUrl('enforceendslash.net:1234')).toEqual('https://enforceendslash.net:1234/')
expect(validateAssetUrl('https://www.absolutepath.org/somepath')).toEqual('https://www.absolutepath.org/')
expect(validateAssetUrl('js-agent.nr.com:9876/foo/bar?query=search')).toEqual('https://js-agent.nr.com:9876/')
})
11 changes: 6 additions & 5 deletions src/loaders/configure/configure.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { addToNREUM, gosCDN, gosNREUMInitializedAgents } from '../../common/wind
import { getConfiguration, setConfiguration, setInfo, setLoaderConfig, setRuntime } from '../../common/config/config'
import { activatedFeatures } from '../../common/util/feature-flags'
import { isWorkerScope } from '../../common/constants/runtime'
import { isValidAssetUrl } from '../../common/url/check-url'
import { validateAssetUrl } from '../../common/url/check-url'
import { redefinePublicPath } from './public-path'
import { warn } from '../../common/util/console'

Expand All @@ -18,6 +18,11 @@ export function configure (agentIdentifier, opts = {}, loaderType, forceDrain) {
loader_config = nr.loader_config
}

if (init.assetsPath) {
const retVal = validateAssetUrl(init.assetsPath)
if (retVal !== '') redefinePublicPath(retVal)
else warn('New public path must be a valid URL. Chunk origin remains unchanged.')
}
setConfiguration(agentIdentifier, init || {})
// eslint-disable-next-line camelcase
setLoaderConfig(agentIdentifier, loader_config || {})
Expand All @@ -26,10 +31,6 @@ export function configure (agentIdentifier, opts = {}, loaderType, forceDrain) {
if (isWorkerScope) { // add a default attr to all worker payloads
info.jsAttributes.isWorker = true
}
if (info.assetsPath) {
if (isValidAssetUrl(info.assetsPath)) redefinePublicPath(info.assetsPath)
else warn('New assets path must be a valid URL. Chunk origin remains unchanged.')
}
setInfo(agentIdentifier, info)

const updatedInit = getConfiguration(agentIdentifier)
Expand Down
4 changes: 2 additions & 2 deletions src/loaders/configure/public-path.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Set the default CDN or remote for fetching the assets; NPM shouldn't change this var.
if (typeof WEBP_CONST_PUBLIC_PATH !== 'undefined') {
__webpack_public_path__ = WEBP_CONST_PUBLIC_PATH
__webpack_public_path__ = WEBP_CONST_PUBLIC_PATH // eslint-disable-line
}

export const redefinePublicPath = (url) => {
// There's no URL validation here, so caller should check arg if need be.
__webpack_public_path__ = url
__webpack_public_path__ = url // eslint-disable-line
}
4 changes: 4 additions & 0 deletions src/loaders/configure/public-path.npm.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

export const redefinePublicPath = () => {
// We don't support setting public path in webpack via NPM build.
}
2 changes: 1 addition & 1 deletion tests/specs/proxy.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('Using proxy servers -', () => {
/** This is where we expect the agent to fetch its chunk. '/build/' is necessary within the URL because the way our asset server
* handles each test requests -- see /testing-server/plugins/test-handle/index.js. */
const assetServerChangedUrl = `http://${host}:${port}/build/fakepath/`
let url = await browser.testHandle.assetURL('instrumented.html', { config: { assetsPath: assetServerChangedUrl } })
let url = await browser.testHandle.assetURL('instrumented.html', { init: { assetsPath: assetServerChangedUrl } })

// Expecting a GET to http://bam-test-1.nr-local.net:<asset port>/build/fakepath/nr-spa.min.js for chunk and response of 404 (no cross-origin)
let expectRequest = browser.testHandle.expect('assetServer', {
Expand Down
6 changes: 0 additions & 6 deletions tools/webpack/env.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,6 @@ export default async (env) => {
env.PUBLIC_PATH = PUBLIC_PATH
env.VERSION = VERSION

// -- These are likely not being used(?)
// process.env.PATH_VERSION = PATH_VERSION
// process.env.SUBVERSION = SUBVERSION
// process.env.PUBLIC_PATH = PUBLIC_PATH
// process.env.VERSION = VERSION

// These values are used in babel to replace ENV variables in src/common/constants/env.cdn.js
process.env.BUILD_VERSION = VERSION
process.env.BUILD_ENV = SUBVERSION
Expand Down