Skip to content

Commit 0296018

Browse files
author
Caleb Barnes
authored
fix: auto-install extension filtering logic and add comprehensive tests (#6434)
* fix: only auto-install extensions when required packages are present Previously, extensions were being auto-installed on all sites with the feature flag enabled, regardless of whether the required packages were present in package.json. This caused mass installations of extensions like Neon on accounts that didn't need them. Now properly filters extensions to only install when: 1. Extension is not already installed, AND 2. At least one required package exists in project dependencies Fixes the logic in handleAutoInstallExtensions to check package.json dependencies before attempting installation. * fix: use buildDir for package.json detection in auto-install-extensions Previously, auto-install extensions used `cwd` to look for package.json, but this isn't always the correct location. The config system calculates `buildDir` which represents the proper base directory where package.json should be found (either `build.base` or `repositoryRoot`). Changes: - Pass `buildDir` instead of `cwd` to handleAutoInstallExtensions - Update parameter and error logging to use buildDir - Gracefully fail if no package.json found in buildDir - Maintain existing package detection logic without new dependencies * fix: use buildDir for package.json detection and add comprehensive tests - Use buildDir instead of cwd when reading package.json in auto-install extensions - Add extensionApiBaseUrl parameter to fetchAutoInstallableExtensionsMeta for test mocking - Update main config to use testOpts.host for extension API calls in test environment - Add comprehensive test suite covering: - Feature flag disabled/enabled scenarios - Missing package.json graceful handling - Package detection logic (with/without required packages) - buildDir resolution with and without netlify.toml - Mock auto-installable extensions API for deterministic testing Fixes mass installation bug where extensions were installed on all sites regardless of whether required packages were present in dependencies. * test: add comprehensive HTTP mocking for extension installation endpoints - Add global fetch mock to intercept external extension installation requests - Mock installation endpoint responses without modifying implementation code - Track installation requests for comprehensive test assertions - Verify correct HTTP method, URL, headers, and request body - Add unique mock response identifiers for verification - Maintain clean separation between test and production code This approach properly mocks external HTTP requests using test-only code, ensuring no real network calls are made during testing while preserving the original implementation behavior. * refactor: use pure test-only HTTP mocking without implementation changes - Revert implementation changes to fetchAutoInstallableExtensionsMeta and main.ts - Extend global fetch mock to handle both installation and extension API endpoints - Remove server-side mocks in favor of pure fetch interception - Maintain clean separation between test and production code This approach provides complete HTTP mocking without polluting the implementation with test-specific logic, following proper testing best practices.
1 parent c185104 commit 0296018

File tree

9 files changed

+276
-15
lines changed

9 files changed

+276
-15
lines changed

packages/config/src/main.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ export const resolveConfig = async function (opts): Promise<Config> {
176176
siteId,
177177
accountId,
178178
token,
179-
cwd,
179+
buildDir,
180180
extensionApiBaseUrl,
181181
testOpts,
182182
offline,

packages/config/src/utils/extensions/auto-install-extensions.ts

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,21 @@ import { type ModeOption } from '../../types/options.js'
88
import { fetchAutoInstallableExtensionsMeta, installExtension } from './utils.js'
99

1010
function getPackageJSON(directory: string) {
11-
const require = createRequire(join(directory, 'package.json'))
12-
return require('./package.json')
11+
try {
12+
const require = createRequire(join(directory, 'package.json'))
13+
return require('./package.json')
14+
} catch {
15+
// Gracefully fail if no package.json found in buildDir
16+
return {}
17+
}
1318
}
1419

1520
interface AutoInstallOptions {
1621
featureFlags: any
1722
siteId: string
1823
accountId: string
1924
token: string
20-
cwd: string
25+
buildDir: string
2126
integrations: IntegrationResponse[]
2227
offline: boolean
2328
testOpts: any
@@ -30,7 +35,7 @@ export async function handleAutoInstallExtensions({
3035
siteId,
3136
accountId,
3237
token,
33-
cwd,
38+
buildDir,
3439
integrations,
3540
offline,
3641
testOpts = {},
@@ -44,7 +49,7 @@ export async function handleAutoInstallExtensions({
4449
console.error("Failed to auto install extension(s): Missing 'accountId'", {
4550
accountId,
4651
siteId,
47-
cwd,
52+
buildDir,
4853
offline,
4954
mode,
5055
})
@@ -54,7 +59,7 @@ export async function handleAutoInstallExtensions({
5459
console.error("Failed to auto install extension(s): Missing 'siteId'", {
5560
accountId,
5661
siteId,
57-
cwd,
62+
buildDir,
5863
offline,
5964
mode,
6065
})
@@ -64,17 +69,17 @@ export async function handleAutoInstallExtensions({
6469
console.error("Failed to auto install extension(s): Missing 'token'", {
6570
accountId,
6671
siteId,
67-
cwd,
72+
buildDir,
6873
offline,
6974
mode,
7075
})
7176
return integrations
7277
}
73-
if (!cwd) {
74-
console.error("Failed to auto install extension(s): Missing 'cwd'", {
78+
if (!buildDir) {
79+
console.error("Failed to auto install extension(s): Missing 'buildDir'", {
7580
accountId,
7681
siteId,
77-
cwd,
82+
buildDir,
7883
offline,
7984
mode,
8085
})
@@ -84,15 +89,15 @@ export async function handleAutoInstallExtensions({
8489
console.error("Failed to auto install extension(s): Running as 'offline'", {
8590
accountId,
8691
siteId,
87-
cwd,
92+
buildDir,
8893
offline,
8994
mode,
9095
})
9196
return integrations
9297
}
9398

9499
try {
95-
const packageJson = getPackageJSON(cwd)
100+
const packageJson = getPackageJSON(buildDir)
96101
if (
97102
!packageJson?.dependencies ||
98103
typeof packageJson?.dependencies !== 'object' ||
@@ -102,9 +107,19 @@ export async function handleAutoInstallExtensions({
102107
}
103108

104109
const autoInstallableExtensions = await fetchAutoInstallableExtensionsMeta()
105-
const extensionsToInstall = autoInstallableExtensions.filter((ext) => {
106-
return !integrations?.some((integration) => integration.slug === ext.slug)
110+
const enabledExtensionSlugs = new Set((integrations ?? []).map(({ slug }) => slug))
111+
const extensionsToInstallCandidates = autoInstallableExtensions.filter(
112+
({ slug }) => !enabledExtensionSlugs.has(slug),
113+
)
114+
const extensionsToInstall = extensionsToInstallCandidates.filter(({ packages }) => {
115+
for (const pkg of packages) {
116+
if (packageJson?.dependencies && Object.hasOwn(packageJson.dependencies, pkg)) {
117+
return true
118+
}
119+
}
120+
return false
107121
})
122+
108123
if (extensionsToInstall.length === 0) {
109124
return integrations
110125
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "test-project-no-netlify-toml-with-neon",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"@netlify/neon": "^1.0.0",
6+
"react": "^18.0.0"
7+
}
8+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[build]
2+
command = "echo 'no package.json here'"
3+
publish = "dist"
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[build]
2+
command = "npm run build"
3+
publish = "dist"
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "test-project-with-neon",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"@netlify/neon": "^0.1.0"
6+
}
7+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[build]
2+
command = "npm run build"
3+
publish = "dist"
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"name": "test-project-without-extension-packages",
3+
"version": "1.0.0",
4+
"dependencies": {
5+
"react": "^18.0.0",
6+
"lodash": "^4.17.21"
7+
}
8+
}
Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
import { Fixture } from '@netlify/testing'
2+
import test from 'ava'
3+
4+
// Mock fetch for external extension installation requests
5+
const originalFetch = globalThis.fetch
6+
const mockInstallationResponse = { success: true, mocked: true, testId: 'MOCK_RESPONSE_12345' }
7+
8+
// Track installation requests for testing
9+
let installationRequests = []
10+
11+
const mockFetch = async (url, options) => {
12+
// Convert URL object to string if needed
13+
const urlString = url.toString()
14+
15+
// If it's an installation request to an external extension URL
16+
if (urlString.includes('/.netlify/functions/handler/on-install')) {
17+
installationRequests.push({ url: urlString, options })
18+
return {
19+
ok: true,
20+
status: 200,
21+
json: async () => mockInstallationResponse,
22+
text: async () => JSON.stringify(mockInstallationResponse),
23+
}
24+
}
25+
26+
// If it's a request to the extension API for auto-installable extensions
27+
if (urlString.includes('api.netlifysdk.com/meta/auto-installable')) {
28+
return {
29+
ok: true,
30+
status: 200,
31+
json: async () => AUTO_INSTALLABLE_EXTENSIONS_RESPONSE.response,
32+
text: async () => JSON.stringify(AUTO_INSTALLABLE_EXTENSIONS_RESPONSE.response),
33+
}
34+
}
35+
36+
// For all other requests, use the original fetch
37+
return originalFetch(url, options)
38+
}
39+
40+
// Set up global fetch mock
41+
globalThis.fetch = mockFetch
42+
43+
// Reset installation requests before each test
44+
test.beforeEach(() => {
45+
installationRequests = []
46+
})
47+
48+
const SITE_INFO_DATA = {
49+
path: '/api/v1/sites/test',
50+
response: { id: 'test', name: 'test' },
51+
}
52+
53+
const TEAM_INSTALLATIONS_META_RESPONSE = {
54+
path: '/team/account1/integrations/installations/meta/test',
55+
response: [],
56+
}
57+
58+
const FETCH_INTEGRATIONS_EMPTY_RESPONSE = {
59+
path: '/integrations',
60+
response: [],
61+
}
62+
63+
// Mock response for auto-installable extensions API
64+
const AUTO_INSTALLABLE_EXTENSIONS_RESPONSE = {
65+
path: '/meta/auto-installable',
66+
response: [
67+
{
68+
slug: 'neon',
69+
hostSiteUrl: 'https://neon-extension.netlify.app', // Mocked by fetch mock
70+
packages: ['@netlify/neon'],
71+
},
72+
],
73+
}
74+
75+
test('Auto-install extensions: feature flag disabled returns integrations unchanged', async (t) => {
76+
const { output } = await new Fixture('./fixtures/with_neon_package')
77+
.withFlags({
78+
siteId: 'test',
79+
accountId: 'account1',
80+
token: 'test',
81+
mode: 'dev',
82+
featureFlags: {
83+
auto_install_required_extensions: false,
84+
},
85+
})
86+
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
87+
88+
const config = JSON.parse(output)
89+
90+
// Should not have attempted to install any extensions
91+
t.false(output.includes('Installing extension'))
92+
t.assert(config.integrations)
93+
t.is(config.integrations.length, 0)
94+
})
95+
96+
test('Auto-install extensions: gracefully handles missing package.json', async (t) => {
97+
const { output } = await new Fixture('./fixtures/no_package_json')
98+
.withFlags({
99+
siteId: 'test',
100+
accountId: 'account1',
101+
token: 'test',
102+
mode: 'dev',
103+
featureFlags: {
104+
auto_install_required_extensions: true,
105+
},
106+
})
107+
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
108+
109+
const config = JSON.parse(output)
110+
111+
// Should not have attempted to install any extensions
112+
t.false(output.includes('Installing extension'))
113+
t.assert(config.integrations)
114+
t.is(config.integrations.length, 0)
115+
})
116+
117+
test('Auto-install extensions: correctly reads package.json from buildDir', async (t) => {
118+
// This test verifies that the function correctly reads package.json from buildDir
119+
const { output } = await new Fixture('./fixtures/with_neon_package')
120+
.withFlags({
121+
siteId: 'test',
122+
accountId: 'account1',
123+
token: 'test',
124+
mode: 'dev',
125+
featureFlags: {
126+
auto_install_required_extensions: true,
127+
},
128+
})
129+
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
130+
131+
const config = JSON.parse(output)
132+
133+
// Should have found package.json in buildDir
134+
t.assert(config.integrations)
135+
t.assert(config.buildDir)
136+
t.true(config.buildDir.includes('with_neon_package'))
137+
138+
// Auto-installable extensions API call is mocked by global fetch mock
139+
// (not visible in requests array since it's intercepted before reaching test server)
140+
141+
// Should have attempted to install the extension (mocked)
142+
t.assert(installationRequests.length > 0, 'Should have attempted to install extension')
143+
t.assert(
144+
installationRequests[0].url.includes('/.netlify/functions/handler/on-install'),
145+
'Should have called installation endpoint',
146+
)
147+
t.assert(
148+
installationRequests[0].url.includes('neon-extension.netlify.app'),
149+
'Should have called correct external URL',
150+
)
151+
t.assert(installationRequests[0].options.method === 'POST', 'Should use POST method')
152+
t.assert(installationRequests[0].options.body.includes('account1'), 'Should include team ID in request body')
153+
})
154+
155+
test('Auto-install extensions: does not install when required packages are missing', async (t) => {
156+
// This test uses a fixture that has dependencies but not the extension packages
157+
const { output } = await new Fixture('./fixtures/without_packages')
158+
.withFlags({
159+
siteId: 'test',
160+
accountId: 'account1',
161+
token: 'test',
162+
mode: 'dev',
163+
featureFlags: {
164+
auto_install_required_extensions: true,
165+
},
166+
})
167+
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
168+
169+
const config = JSON.parse(output)
170+
171+
// Should not attempt to install extensions since required packages are missing
172+
t.false(output.includes('Installing extension'))
173+
t.assert(config.integrations)
174+
t.is(config.integrations.length, 0)
175+
176+
// Auto-installable extensions API call is mocked by global fetch mock
177+
// (not visible in requests array since it's intercepted before reaching test server)
178+
})
179+
180+
test('Auto-install extensions: correctly reads package.json when no netlify.toml exists', async (t) => {
181+
// This test verifies buildDir resolution works correctly when there's no netlify.toml
182+
// but package.json exists with extension packages
183+
const { output } = await new Fixture('./fixtures/no_netlify_toml_with_neon')
184+
.withFlags({
185+
siteId: 'test',
186+
accountId: 'account1',
187+
token: 'test',
188+
mode: 'dev',
189+
featureFlags: {
190+
auto_install_required_extensions: true,
191+
},
192+
})
193+
.runConfigServer([SITE_INFO_DATA, TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE])
194+
195+
const config = JSON.parse(output)
196+
197+
// Should have found package.json in buildDir even without netlify.toml
198+
t.assert(config.integrations)
199+
t.assert(config.buildDir)
200+
t.true(config.buildDir.includes('no_netlify_toml_with_neon'))
201+
202+
// buildDir should be the repository root since there's no build.base config
203+
t.true(config.buildDir.endsWith('no_netlify_toml_with_neon'))
204+
205+
// Auto-installable extensions API call is mocked by global fetch mock
206+
// (not visible in requests array since it's intercepted before reaching test server)
207+
208+
// Should have attempted to install the extension
209+
t.assert(installationRequests.length > 0, 'Should have attempted to install extension')
210+
t.assert(
211+
installationRequests[0].url.includes('/.netlify/functions/handler/on-install'),
212+
'Should have called installation endpoint',
213+
)
214+
})

0 commit comments

Comments
 (0)