From d5962374c2d706f315ebf59957ef1676fe013ea8 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Fri, 17 May 2024 09:51:22 +0100 Subject: [PATCH 01/15] chore: add flag to split logic - in progress --- packages/config/src/api/site_info.ts | 61 +++++++++++++++++++--------- 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index 93d08df1ee..4b765d66a6 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -31,36 +31,48 @@ export const getSiteInfo = async function ({ siteId, mode, siteFeatureFlagPrefix, + featureFlags, context, offline = false, testOpts = {}, }: GetSiteInfoOpts) { const { env: testEnv = false } = testOpts - if (api === undefined || mode === 'buildbot' || testEnv) { - const siteInfo = siteId === undefined ? {} : { id: siteId } + const useV1Endpoint = !featureFlags?.integration_installations_meta - const integrations = mode === 'buildbot' && !offline ? await getIntegrations({ siteId, testOpts, offline }) : [] + if (useV1Endpoint) { + // Question: When would we ever hit this condition? I.e. what user flow would lead to this? + // Answer: Ask Karin, she may know 🤷 + // + // If we can't make a call to getSite then we can't get integrations as we need the accountId for the site + if (api === undefined || mode === 'buildbot' || testEnv) { + const siteInfo = siteId === undefined ? {} : { id: siteId } - return { siteInfo, accounts: [], addons: [], integrations } - } + const integrations = mode === 'buildbot' && !offline ? await getIntegrations({ siteId, testOpts, offline }) : [] - const promises = [ - getSite(api, siteId, siteFeatureFlagPrefix), - getAccounts(api), - getAddons(api, siteId), - getIntegrations({ siteId, testOpts, offline }), - ] + return { siteInfo, accounts: [], addons: [], integrations } + } - const [siteInfo, accounts, addons, integrations] = await Promise.all(promises) + const siteInfo = await getSite(api, siteId, siteFeatureFlagPrefix) - if (siteInfo.use_envelope) { - const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context }) + const promises = [ + getAccounts(api), + getAddons(api, siteId), + getIntegrations({ siteId, testOpts, offline, accountId: siteInfo.account_id, featureFlags }), + ] - siteInfo.build_settings.env = envelope - } + const [accounts, addons, integrations] = await Promise.all(promises) + + if (siteInfo.use_envelope) { + const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context }) + + siteInfo.build_settings.env = envelope + } - return { siteInfo, accounts, addons, integrations } + return { siteInfo, accounts, addons, integrations } + } else { + // add old code back here + } } const getSite = async function (api: NetlifyAPI, siteId: string, siteFeatureFlagPrefix: string | null = null) { @@ -100,14 +112,18 @@ const getAddons = async function (api: NetlifyAPI, siteId: string) { type GetIntegrationsOpts = { siteId?: string + accountId?: string testOpts: TestOptions offline: boolean + featureFlags?: Record } const getIntegrations = async function ({ siteId, + accountId, testOpts, offline, + featureFlags, }: GetIntegrationsOpts): Promise { if (!siteId || offline) { return [] @@ -117,13 +133,20 @@ const getIntegrations = async function ({ const baseUrl = new URL(host ? `http://${host}` : `https://api.netlifysdk.com`) + const useV1Endpoint = !featureFlags?.integration_installations_meta + + const url = useV1Endpoint + ? `${baseUrl}site/${siteId}/integrations/safe` + : `${baseUrl}team/${accountId}/integrations/installations/meta` + try { - const response = await fetch(`${baseUrl}site/${siteId}/integrations/safe`) + const response = await fetch(url) const integrations = await response.json() return Array.isArray(integrations) ? integrations : [] } catch (error) { - // for now, we'll just ignore errors, as this is early days + // Integrations should not block the build if they fail to load + // TODO: We should consider blocking the build as integrations are a critical part of the build process return [] } } From 12cdb9d4d9bb3b8c8f71bb282142586e6cffbdca Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Fri, 17 May 2024 12:10:17 +0100 Subject: [PATCH 02/15] chore: link --- package-lock.json | 2 +- packages/build/package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index e40f345996..a65b31e577 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26032,7 +26032,7 @@ "@bugsnag/js": "^7.0.0", "@netlify/blobs": "^7.3.0", "@netlify/cache-utils": "^5.1.5", - "@netlify/config": "^20.12.5", + "@netlify/config": "file:../config", "@netlify/edge-bundler": "12.0.1", "@netlify/framework-info": "^9.8.12", "@netlify/functions-utils": "^5.2.56", diff --git a/packages/build/package.json b/packages/build/package.json index c983744ab9..e12c25845c 100644 --- a/packages/build/package.json +++ b/packages/build/package.json @@ -70,7 +70,7 @@ "@bugsnag/js": "^7.0.0", "@netlify/blobs": "^7.3.0", "@netlify/cache-utils": "^5.1.5", - "@netlify/config": "^20.12.5", + "@netlify/config": "file:../config", "@netlify/edge-bundler": "12.0.1", "@netlify/framework-info": "^9.8.12", "@netlify/functions-utils": "^5.2.56", From 8d0ec6742f217493489e0dadf9a62342f227061b Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 11:14:22 +0100 Subject: [PATCH 03/15] chore: in progress with ff split --- packages/config/src/api/site_info.ts | 41 ++++++++++++++++++++-------- packages/config/src/bin/flags.ts | 7 +++++ packages/config/src/bin/main.js | 1 + packages/config/src/main.ts | 1 - 4 files changed, 38 insertions(+), 12 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index 4b765d66a6..ba28d8f35a 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -14,7 +14,7 @@ type GetSiteInfoOpts = { offline?: boolean api?: NetlifyAPI context?: string - featureFlags?: Record + //featureFlags?: Record testOpts?: TestOptions } /** @@ -31,28 +31,49 @@ export const getSiteInfo = async function ({ siteId, mode, siteFeatureFlagPrefix, - featureFlags, context, offline = false, testOpts = {}, }: GetSiteInfoOpts) { const { env: testEnv = false } = testOpts - const useV1Endpoint = !featureFlags?.integration_installations_meta + if (api === undefined || testEnv) { + const siteInfo = siteId === undefined ? {} : { id: siteId } + + return { siteInfo, accounts: [], addons: [], integrations: [] } + } + + const siteInfo = await getSite(api, siteId, siteFeatureFlagPrefix) + const featureFlags = siteInfo.feature_flags + + const useV1Endpoint = !featureFlags?.cli_integration_installations_meta + console.log(useV1Endpoint, featureFlags) if (useV1Endpoint) { - // Question: When would we ever hit this condition? I.e. what user flow would lead to this? - // Answer: Ask Karin, she may know 🤷 - // - // If we can't make a call to getSite then we can't get integrations as we need the accountId for the site - if (api === undefined || mode === 'buildbot' || testEnv) { + if (mode === 'buildbot') { const siteInfo = siteId === undefined ? {} : { id: siteId } - const integrations = mode === 'buildbot' && !offline ? await getIntegrations({ siteId, testOpts, offline }) : [] + const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) return { siteInfo, accounts: [], addons: [], integrations } } + const promises = [ + getAccounts(api), + getAddons(api, siteId), + getIntegrations({ siteId, testOpts, offline, featureFlags }), + ] + + const [accounts, addons, integrations] = await Promise.all(promises) + + if (siteInfo.use_envelope) { + const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context }) + + siteInfo.build_settings.env = envelope + } + + return { siteInfo, accounts, addons, integrations } + } else { const siteInfo = await getSite(api, siteId, siteFeatureFlagPrefix) const promises = [ @@ -70,8 +91,6 @@ export const getSiteInfo = async function ({ } return { siteInfo, accounts, addons, integrations } - } else { - // add old code back here } } diff --git a/packages/config/src/bin/flags.ts b/packages/config/src/bin/flags.ts index 005ec4b6fe..ef24ba318b 100644 --- a/packages/config/src/bin/flags.ts +++ b/packages/config/src/bin/flags.ts @@ -15,6 +15,8 @@ export const parseFlags = function () { const featureFlags = normalizeCliFeatureFlags(cliFeatureFlags) const flagsA = { ...flags, featureFlags } const flagsB = includeKeys(flagsA, isUserFlag) + // log every single flag declaration here + return flagsB } @@ -170,6 +172,11 @@ Default: false`, describe: 'Buffer output instead of streaming it', hidden: true, }, + lewistest: { + boolean: true, + describe: 'Enable Lewis test mode', + hidden: true, + }, } const USAGE = `netlify-config [OPTIONS...] diff --git a/packages/config/src/bin/main.js b/packages/config/src/bin/main.js index 6915cb8295..034b44ee22 100755 --- a/packages/config/src/bin/main.js +++ b/packages/config/src/bin/main.js @@ -15,6 +15,7 @@ import { parseFlags } from './flags.js' // CLI entry point const runCli = async function () { try { + console.log('cli') const { stable, output = DEFAULT_OUTPUT, ...flags } = parseFlags() const result = await resolveConfig(flags) await handleCliSuccess(result, stable, output) diff --git a/packages/config/src/main.ts b/packages/config/src/main.ts index 1971acb574..8440061e85 100644 --- a/packages/config/src/main.ts +++ b/packages/config/src/main.ts @@ -73,7 +73,6 @@ export const resolveConfig = async function (opts) { mode, offline, siteFeatureFlagPrefix, - featureFlags, testOpts, }) From ebeebddf032ae69986e4b4607174335ea34a2901 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 11:28:06 +0100 Subject: [PATCH 04/15] chore: correct flag --- packages/config/src/api/site_info.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index ba28d8f35a..f7e3ab257f 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -152,7 +152,7 @@ const getIntegrations = async function ({ const baseUrl = new URL(host ? `http://${host}` : `https://api.netlifysdk.com`) - const useV1Endpoint = !featureFlags?.integration_installations_meta + const useV1Endpoint = !featureFlags?.cli_integration_installations_meta const url = useV1Endpoint ? `${baseUrl}site/${siteId}/integrations/safe` From f271f78df193c1b1481d0d794cbc5ba8e73a6f06 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 12:55:03 +0100 Subject: [PATCH 05/15] chore: debug tests --- packages/config/src/api/site_info.ts | 21 +++++++++++---- packages/config/tests/api/tests.js | 39 ++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index f7e3ab257f..ce01c13280 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -37,6 +37,7 @@ export const getSiteInfo = async function ({ }: GetSiteInfoOpts) { const { env: testEnv = false } = testOpts + console.log('siteInfo', testEnv, api) if (api === undefined || testEnv) { const siteInfo = siteId === undefined ? {} : { id: siteId } @@ -44,6 +45,7 @@ export const getSiteInfo = async function ({ } const siteInfo = await getSite(api, siteId, siteFeatureFlagPrefix) + console.log(siteInfo, 'siteInfo') const featureFlags = siteInfo.feature_flags const useV1Endpoint = !featureFlags?.cli_integration_installations_meta @@ -51,11 +53,12 @@ export const getSiteInfo = async function ({ if (useV1Endpoint) { if (mode === 'buildbot') { - const siteInfo = siteId === undefined ? {} : { id: siteId } - - const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) - - return { siteInfo, accounts: [], addons: [], integrations } + throw new Error('ava sucks') + // const siteInfo = siteId === undefined ? {} : { id: siteId } + // + // const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) + // + // return { siteInfo, accounts: [], addons: [], integrations } } const promises = [ @@ -104,6 +107,8 @@ const getSite = async function (api: NetlifyAPI, siteId: string, siteFeatureFlag return { ...site, id: siteId } } catch (error) { throwUserError(`Failed retrieving site data for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) + // Added to satisfy TypeScript only an object can be returned + return { id: siteId } } } @@ -113,6 +118,8 @@ const getAccounts = async function (api: NetlifyAPI) { return Array.isArray(accounts) ? accounts : [] } catch (error) { throwUserError(`Failed retrieving user account: ${error.message}. ${ERROR_CALL_TO_ACTION}`) + // Added to satisfy TypeScript only an array can be returned + return [] } } @@ -126,6 +133,8 @@ const getAddons = async function (api: NetlifyAPI, siteId: string) { return Array.isArray(addons) ? addons : [] } catch (error) { throwUserError(`Failed retrieving addons for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) + // Added to satisfy TypeScript only an array can be returned + return [] } } @@ -161,6 +170,8 @@ const getIntegrations = async function ({ try { const response = await fetch(url) + console.log(response.status, response.statusText, url) + const integrations = await response.json() return Array.isArray(integrations) ? integrations : [] } catch (error) { diff --git a/packages/config/tests/api/tests.js b/packages/config/tests/api/tests.js index 61f9a2a113..e67f76932d 100644 --- a/packages/config/tests/api/tests.js +++ b/packages/config/tests/api/tests.js @@ -26,6 +26,17 @@ const SITE_INTEGRATIONS_RESPONSE = { ], } +const TEAM_INSTALLATIONS_META_RESPONSE = { + path: '/team/team1/integrations/installations/meta', + response: [ + { + slug: 'test', + version: 'so-cool', + has_build: true, + }, + ], +} + const SITE_INTEGRATIONS_EMPTY_RESPONSE = { path: '/site/test/integrations/safe', response: [], @@ -307,7 +318,8 @@ test('In integration dev mode, integration specified in config is returned and b t.assert(config.integrations[0].version === undefined) }) -test('Integrations are returned if feature flag is true, mode buildbot', async (t) => { +// old tests +test.skip('Integrations are returned if feature flag is true, mode buildbot', async (t) => { const { output } = await new Fixture('./fixtures/base') .withFlags({ siteId: 'test', @@ -324,7 +336,7 @@ test('Integrations are returned if feature flag is true, mode buildbot', async ( t.assert(config.integrations[0].has_build === true) }) -test('Integrations are not returned if offline', async (t) => { +test.skip('Integrations are not returned if offline', async (t) => { const { output } = await new Fixture('./fixtures/base') .withFlags({ offline: true, @@ -339,6 +351,29 @@ test('Integrations are not returned if offline', async (t) => { t.assert(config.integrations.length === 0) }) +// new tests +test.only('Integrations are returned if flag is true for site and mode is buildbot', async (t) => { + console.log('logs work') + const { output } = await new Fixture('./fixtures/base') + .withFlags({ + siteId: 'test', + mode: 'buildbot', + }) + .runConfigServer([TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE]) + + console.log(output) + + const config = JSON.parse(output) + + t.assert(config.integrations) + t.assert(config.integrations.length === 1) + t.assert(config.integrations[0].slug === 'test') + t.assert(config.integrations[0].version === 'so-cool') + t.assert(config.integrations[0].has_build === true) +}) +test('Integrations are returned if flag is true for site and mode is dev', () => {}) +test('2Integrations are not returned if offline', () => {}) + test('baseRelDir is true if build.base is overridden', async (t) => { const fixturesDir = normalize(`${fileURLToPath(test.meta.file)}/../fixtures`) From a39c4591d3f39067565ee4ecdb911772a44fbddd Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 12:55:11 +0100 Subject: [PATCH 06/15] chore: debug tests --- packages/testing/src/fixture.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/testing/src/fixture.ts b/packages/testing/src/fixture.ts index e498ff3620..7f236cc258 100644 --- a/packages/testing/src/fixture.ts +++ b/packages/testing/src/fixture.ts @@ -270,6 +270,7 @@ export class Fixture { /** Run the @netlify/config wrapped with a server and and the provided handler */ runConfigServer(handler: ServerHandler): Promise<{ output: string; requests: Request[] }> { + console.log('test') return this.runServer(this.runWithConfig, handler) } From 876602b7864a4e72e888e2fdf8d7d4595dd835a8 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 14:12:20 +0100 Subject: [PATCH 07/15] chore: test coverage added --- packages/config/src/api/site_info.ts | 18 ++---- packages/config/tests/api/tests.js | 97 ++++++++++++++++++++++++---- 2 files changed, 89 insertions(+), 26 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index ce01c13280..b37b29fdba 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -37,28 +37,24 @@ export const getSiteInfo = async function ({ }: GetSiteInfoOpts) { const { env: testEnv = false } = testOpts - console.log('siteInfo', testEnv, api) - if (api === undefined || testEnv) { + if (api === undefined || testEnv || offline) { const siteInfo = siteId === undefined ? {} : { id: siteId } return { siteInfo, accounts: [], addons: [], integrations: [] } } const siteInfo = await getSite(api, siteId, siteFeatureFlagPrefix) - console.log(siteInfo, 'siteInfo') const featureFlags = siteInfo.feature_flags const useV1Endpoint = !featureFlags?.cli_integration_installations_meta - console.log(useV1Endpoint, featureFlags) if (useV1Endpoint) { if (mode === 'buildbot') { - throw new Error('ava sucks') - // const siteInfo = siteId === undefined ? {} : { id: siteId } - // - // const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) - // - // return { siteInfo, accounts: [], addons: [], integrations } + const siteInfo = siteId === undefined ? {} : { id: siteId } + + const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) + + return { siteInfo, accounts: [], addons: [], integrations } } const promises = [ @@ -77,8 +73,6 @@ export const getSiteInfo = async function ({ return { siteInfo, accounts, addons, integrations } } else { - const siteInfo = await getSite(api, siteId, siteFeatureFlagPrefix) - const promises = [ getAccounts(api), getAddons(api, siteId), diff --git a/packages/config/tests/api/tests.js b/packages/config/tests/api/tests.js index e67f76932d..67464ab446 100644 --- a/packages/config/tests/api/tests.js +++ b/packages/config/tests/api/tests.js @@ -27,7 +27,7 @@ const SITE_INTEGRATIONS_RESPONSE = { } const TEAM_INSTALLATIONS_META_RESPONSE = { - path: '/team/team1/integrations/installations/meta', + path: '/team/account1/integrations/installations/meta', response: [ { slug: 'test', @@ -42,6 +42,21 @@ const SITE_INTEGRATIONS_EMPTY_RESPONSE = { response: [], } +const siteInfoWithFeatureFlag = (flag) => { + return { + path: SITE_INFO_PATH, + response: { + ssl_url: 'test', + name: 'test-name', + build_settings: { repo_url: 'test' }, + account_id: 'account1', + feature_flags: { + [flag]: true, + }, + }, + } +} + const SITE_INFO_BUILD_SETTINGS = { path: SITE_INFO_PATH, response: { @@ -318,8 +333,22 @@ test('In integration dev mode, integration specified in config is returned and b t.assert(config.integrations[0].version === undefined) }) -// old tests -test.skip('Integrations are returned if feature flag is true, mode buildbot', async (t) => { +test('Integrations are not returned if offline', async (t) => { + const { output } = await new Fixture('./fixtures/base') + .withFlags({ + offline: true, + siteId: 'test', + mode: 'buildbot', + }) + .runConfigServer([SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE]) + + const config = JSON.parse(output) + + t.assert(config.integrations) + t.assert(config.integrations.length === 0) +}) + +test('Integrations are not returned if no api', async (t) => { const { output } = await new Fixture('./fixtures/base') .withFlags({ siteId: 'test', @@ -329,6 +358,21 @@ test.skip('Integrations are returned if feature flag is true, mode buildbot', as const config = JSON.parse(output) + t.assert(config.integrations) + t.assert(config.integrations.length === 0) +}) + +test('Integrations are returned if feature flag is false and mode is buildbot', async (t) => { + const { output } = await new Fixture('./fixtures/base') + .withFlags({ + siteId: 'test', + mode: 'buildbot', + token: 'test', + }) + .runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE]) + + const config = JSON.parse(output) + t.assert(config.integrations) t.assert(config.integrations.length === 1) t.assert(config.integrations[0].slug === 'test') @@ -336,32 +380,59 @@ test.skip('Integrations are returned if feature flag is true, mode buildbot', as t.assert(config.integrations[0].has_build === true) }) -test.skip('Integrations are not returned if offline', async (t) => { +test('Integrations are returned if feature flag is false and mode is dev', async (t) => { const { output } = await new Fixture('./fixtures/base') .withFlags({ - offline: true, siteId: 'test', - mode: 'buildbot', + mode: 'dev', + token: 'test', }) - .runConfigServer([SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE]) + .runConfigServer([SITE_INFO_DATA, SITE_INTEGRATIONS_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE]) const config = JSON.parse(output) t.assert(config.integrations) - t.assert(config.integrations.length === 0) + t.assert(config.integrations.length === 1) + t.assert(config.integrations[0].slug === 'test') + t.assert(config.integrations[0].version === 'so-cool') + t.assert(config.integrations[0].has_build === true) }) // new tests -test.only('Integrations are returned if flag is true for site and mode is buildbot', async (t) => { - console.log('logs work') +test('Integrations are returned if flag is true for site and mode is buildbot', async (t) => { const { output } = await new Fixture('./fixtures/base') .withFlags({ siteId: 'test', mode: 'buildbot', + token: 'test', }) - .runConfigServer([TEAM_INSTALLATIONS_META_RESPONSE, FETCH_INTEGRATIONS_EMPTY_RESPONSE]) + .runConfigServer([ + siteInfoWithFeatureFlag('cli_integration_installations_meta'), + TEAM_INSTALLATIONS_META_RESPONSE, + FETCH_INTEGRATIONS_EMPTY_RESPONSE, + ]) - console.log(output) + const config = JSON.parse(output) + + t.assert(config.integrations) + t.assert(config.integrations.length === 1) + t.assert(config.integrations[0].slug === 'test') + t.assert(config.integrations[0].version === 'so-cool') + t.assert(config.integrations[0].has_build === true) +}) + +test('Integrations are returned if flag is true for site and mode is dev', async (t) => { + const { output } = await new Fixture('./fixtures/base') + .withFlags({ + siteId: 'test', + mode: 'dev', + token: 'test', + }) + .runConfigServer([ + siteInfoWithFeatureFlag('cli_integration_installations_meta'), + TEAM_INSTALLATIONS_META_RESPONSE, + FETCH_INTEGRATIONS_EMPTY_RESPONSE, + ]) const config = JSON.parse(output) @@ -371,8 +442,6 @@ test.only('Integrations are returned if flag is true for site and mode is buildb t.assert(config.integrations[0].version === 'so-cool') t.assert(config.integrations[0].has_build === true) }) -test('Integrations are returned if flag is true for site and mode is dev', () => {}) -test('2Integrations are not returned if offline', () => {}) test('baseRelDir is true if build.base is overridden', async (t) => { const fixturesDir = normalize(`${fileURLToPath(test.meta.file)}/../fixtures`) From 9e5dec2b10475775dd19cd7999036adc1e582ef0 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 14:13:25 +0100 Subject: [PATCH 08/15] chore: remove logs --- packages/config/src/api/site_info.ts | 2 -- packages/testing/src/fixture.ts | 1 - 2 files changed, 3 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index b37b29fdba..114fe661ab 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -164,8 +164,6 @@ const getIntegrations = async function ({ try { const response = await fetch(url) - console.log(response.status, response.statusText, url) - const integrations = await response.json() return Array.isArray(integrations) ? integrations : [] } catch (error) { diff --git a/packages/testing/src/fixture.ts b/packages/testing/src/fixture.ts index 7f236cc258..e498ff3620 100644 --- a/packages/testing/src/fixture.ts +++ b/packages/testing/src/fixture.ts @@ -270,7 +270,6 @@ export class Fixture { /** Run the @netlify/config wrapped with a server and and the provided handler */ runConfigServer(handler: ServerHandler): Promise<{ output: string; requests: Request[] }> { - console.log('test') return this.runServer(this.runWithConfig, handler) } From 16e8eae2c20910a624fe9098ba2a8dfac91b35cc Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 14:22:51 +0100 Subject: [PATCH 09/15] chore: change local dep --- package-lock.json | 2 +- packages/build/package.json | 2 +- packages/config/src/bin/flags.ts | 7 ------- packages/config/src/bin/main.js | 1 - 4 files changed, 2 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index a65b31e577..e40f345996 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26032,7 +26032,7 @@ "@bugsnag/js": "^7.0.0", "@netlify/blobs": "^7.3.0", "@netlify/cache-utils": "^5.1.5", - "@netlify/config": "file:../config", + "@netlify/config": "^20.12.5", "@netlify/edge-bundler": "12.0.1", "@netlify/framework-info": "^9.8.12", "@netlify/functions-utils": "^5.2.56", diff --git a/packages/build/package.json b/packages/build/package.json index e12c25845c..c983744ab9 100644 --- a/packages/build/package.json +++ b/packages/build/package.json @@ -70,7 +70,7 @@ "@bugsnag/js": "^7.0.0", "@netlify/blobs": "^7.3.0", "@netlify/cache-utils": "^5.1.5", - "@netlify/config": "file:../config", + "@netlify/config": "^20.12.5", "@netlify/edge-bundler": "12.0.1", "@netlify/framework-info": "^9.8.12", "@netlify/functions-utils": "^5.2.56", diff --git a/packages/config/src/bin/flags.ts b/packages/config/src/bin/flags.ts index ef24ba318b..005ec4b6fe 100644 --- a/packages/config/src/bin/flags.ts +++ b/packages/config/src/bin/flags.ts @@ -15,8 +15,6 @@ export const parseFlags = function () { const featureFlags = normalizeCliFeatureFlags(cliFeatureFlags) const flagsA = { ...flags, featureFlags } const flagsB = includeKeys(flagsA, isUserFlag) - // log every single flag declaration here - return flagsB } @@ -172,11 +170,6 @@ Default: false`, describe: 'Buffer output instead of streaming it', hidden: true, }, - lewistest: { - boolean: true, - describe: 'Enable Lewis test mode', - hidden: true, - }, } const USAGE = `netlify-config [OPTIONS...] diff --git a/packages/config/src/bin/main.js b/packages/config/src/bin/main.js index 034b44ee22..6915cb8295 100755 --- a/packages/config/src/bin/main.js +++ b/packages/config/src/bin/main.js @@ -15,7 +15,6 @@ import { parseFlags } from './flags.js' // CLI entry point const runCli = async function () { try { - console.log('cli') const { stable, output = DEFAULT_OUTPUT, ...flags } = parseFlags() const result = await resolveConfig(flags) await handleCliSuccess(result, stable, output) From 18cc0815111b3df7a8057f4875b52380a8502f95 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 14:27:21 +0100 Subject: [PATCH 10/15] chore: remove flag comment --- packages/config/src/api/site_info.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index 114fe661ab..c8b22e065a 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -14,7 +14,6 @@ type GetSiteInfoOpts = { offline?: boolean api?: NetlifyAPI context?: string - //featureFlags?: Record testOpts?: TestOptions } /** From 2db6580040245a12bfd907eabea3db0703cace4b Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Tue, 21 May 2024 15:49:02 +0100 Subject: [PATCH 11/15] chore: user v2 as flag var name --- packages/config/src/api/site_info.ts | 32 ++++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index c8b22e065a..5fc0127cc2 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -45,21 +45,13 @@ export const getSiteInfo = async function ({ const siteInfo = await getSite(api, siteId, siteFeatureFlagPrefix) const featureFlags = siteInfo.feature_flags - const useV1Endpoint = !featureFlags?.cli_integration_installations_meta - - if (useV1Endpoint) { - if (mode === 'buildbot') { - const siteInfo = siteId === undefined ? {} : { id: siteId } - - const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) - - return { siteInfo, accounts: [], addons: [], integrations } - } + const useV2Endpoint = featureFlags?.cli_integration_installations_meta + if (useV2Endpoint) { const promises = [ getAccounts(api), getAddons(api, siteId), - getIntegrations({ siteId, testOpts, offline, featureFlags }), + getIntegrations({ siteId, testOpts, offline, accountId: siteInfo.account_id, featureFlags }), ] const [accounts, addons, integrations] = await Promise.all(promises) @@ -72,10 +64,18 @@ export const getSiteInfo = async function ({ return { siteInfo, accounts, addons, integrations } } else { + if (mode === 'buildbot') { + const siteInfo = siteId === undefined ? {} : { id: siteId } + + const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) + + return { siteInfo, accounts: [], addons: [], integrations } + } + const promises = [ getAccounts(api), getAddons(api, siteId), - getIntegrations({ siteId, testOpts, offline, accountId: siteInfo.account_id, featureFlags }), + getIntegrations({ siteId, testOpts, offline, featureFlags }), ] const [accounts, addons, integrations] = await Promise.all(promises) @@ -154,11 +154,11 @@ const getIntegrations = async function ({ const baseUrl = new URL(host ? `http://${host}` : `https://api.netlifysdk.com`) - const useV1Endpoint = !featureFlags?.cli_integration_installations_meta + const useV2Endpoint = featureFlags?.cli_integration_installations_meta - const url = useV1Endpoint - ? `${baseUrl}site/${siteId}/integrations/safe` - : `${baseUrl}team/${accountId}/integrations/installations/meta` + const url = useV2Endpoint + ? `${baseUrl}team/${accountId}/integrations/installations/meta` + : `${baseUrl}site/${siteId}/integrations/safe` try { const response = await fetch(url) From f510e1995c4b0b5062c526cfffe9a4a88dc5d217 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Fri, 24 May 2024 11:18:00 +0100 Subject: [PATCH 12/15] chore: add link to linear issue --- packages/config/src/api/site_info.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index 5fc0127cc2..e49483d591 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -168,6 +168,7 @@ const getIntegrations = async function ({ } catch (error) { // Integrations should not block the build if they fail to load // TODO: We should consider blocking the build as integrations are a critical part of the build process + // https://linear.app/netlify/issue/CT-1214/implement-strategy-in-builds-to-deal-with-integrations-that-we-fail-to return [] } } From 7bf6d8f045a0875256410d8158d3753abf4303fe Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Fri, 24 May 2024 11:30:29 +0100 Subject: [PATCH 13/15] chore: make error func return never --- packages/config/src/api/site_info.ts | 6 ------ packages/config/src/error.ts | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index e49483d591..2bcb04c0b6 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -100,8 +100,6 @@ const getSite = async function (api: NetlifyAPI, siteId: string, siteFeatureFlag return { ...site, id: siteId } } catch (error) { throwUserError(`Failed retrieving site data for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) - // Added to satisfy TypeScript only an object can be returned - return { id: siteId } } } @@ -111,8 +109,6 @@ const getAccounts = async function (api: NetlifyAPI) { return Array.isArray(accounts) ? accounts : [] } catch (error) { throwUserError(`Failed retrieving user account: ${error.message}. ${ERROR_CALL_TO_ACTION}`) - // Added to satisfy TypeScript only an array can be returned - return [] } } @@ -126,8 +122,6 @@ const getAddons = async function (api: NetlifyAPI, siteId: string) { return Array.isArray(addons) ? addons : [] } catch (error) { throwUserError(`Failed retrieving addons for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) - // Added to satisfy TypeScript only an array can be returned - return [] } } diff --git a/packages/config/src/error.ts b/packages/config/src/error.ts index 343dc84a95..75bdf4fa1c 100644 --- a/packages/config/src/error.ts +++ b/packages/config/src/error.ts @@ -1,6 +1,6 @@ // We distinguish between errors thrown intentionally and uncaught exceptions // (such as bugs) with a `customErrorInfo.type` property. -export const throwUserError = function (messageOrError: string | Error, error?: Error) { +export const throwUserError = function (messageOrError: string | Error, error?: Error): never { const errorA = getError(messageOrError, error) errorA[CUSTOM_ERROR_KEY] = { type: USER_ERROR_TYPE } throw errorA From d56db4ae2ceccf4d4455300b4b7cb63caf6460e4 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Fri, 24 May 2024 11:33:49 +0100 Subject: [PATCH 14/15] chore: remove else --- packages/config/src/api/site_info.ts | 35 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index 2bcb04c0b6..f11df6c220 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -63,31 +63,30 @@ export const getSiteInfo = async function ({ } return { siteInfo, accounts, addons, integrations } - } else { - if (mode === 'buildbot') { - const siteInfo = siteId === undefined ? {} : { id: siteId } - - const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) + } + if (mode === 'buildbot') { + const siteInfo = siteId === undefined ? {} : { id: siteId } - return { siteInfo, accounts: [], addons: [], integrations } - } + const integrations = await getIntegrations({ siteId, testOpts, offline, featureFlags }) - const promises = [ - getAccounts(api), - getAddons(api, siteId), - getIntegrations({ siteId, testOpts, offline, featureFlags }), - ] + return { siteInfo, accounts: [], addons: [], integrations } + } - const [accounts, addons, integrations] = await Promise.all(promises) + const promises = [ + getAccounts(api), + getAddons(api, siteId), + getIntegrations({ siteId, testOpts, offline, featureFlags }), + ] - if (siteInfo.use_envelope) { - const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context }) + const [accounts, addons, integrations] = await Promise.all(promises) - siteInfo.build_settings.env = envelope - } + if (siteInfo.use_envelope) { + const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context }) - return { siteInfo, accounts, addons, integrations } + siteInfo.build_settings.env = envelope } + + return { siteInfo, accounts, addons, integrations } } const getSite = async function (api: NetlifyAPI, siteId: string, siteFeatureFlagPrefix: string | null = null) { From 3f8f4e169f23467dd0840772d36d7996e6aa6b86 Mon Sep 17 00:00:00 2001 From: Lewis Thorley Date: Fri, 24 May 2024 11:50:47 +0100 Subject: [PATCH 15/15] chore: fix typing by return error func that returns never --- packages/config/src/api/site_info.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/config/src/api/site_info.ts b/packages/config/src/api/site_info.ts index f11df6c220..d5c67371b1 100644 --- a/packages/config/src/api/site_info.ts +++ b/packages/config/src/api/site_info.ts @@ -54,7 +54,7 @@ export const getSiteInfo = async function ({ getIntegrations({ siteId, testOpts, offline, accountId: siteInfo.account_id, featureFlags }), ] - const [accounts, addons, integrations] = await Promise.all(promises) + const [accounts, addons, integrations] = await Promise.all(promises) if (siteInfo.use_envelope) { const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context }) @@ -98,7 +98,7 @@ const getSite = async function (api: NetlifyAPI, siteId: string, siteFeatureFlag const site = await (api as any).getSite({ siteId, feature_flags: siteFeatureFlagPrefix }) return { ...site, id: siteId } } catch (error) { - throwUserError(`Failed retrieving site data for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) + return throwUserError(`Failed retrieving site data for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) } } @@ -107,7 +107,7 @@ const getAccounts = async function (api: NetlifyAPI) { const accounts = await (api as any).listAccountsForUser() return Array.isArray(accounts) ? accounts : [] } catch (error) { - throwUserError(`Failed retrieving user account: ${error.message}. ${ERROR_CALL_TO_ACTION}`) + return throwUserError(`Failed retrieving user account: ${error.message}. ${ERROR_CALL_TO_ACTION}`) } } @@ -120,7 +120,7 @@ const getAddons = async function (api: NetlifyAPI, siteId: string) { const addons = await (api as any).listServiceInstancesForSite({ siteId }) return Array.isArray(addons) ? addons : [] } catch (error) { - throwUserError(`Failed retrieving addons for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) + return throwUserError(`Failed retrieving addons for site ${siteId}: ${error.message}. ${ERROR_CALL_TO_ACTION}`) } }