diff --git a/src/commands/deploy/deploy.ts b/src/commands/deploy/deploy.ts index 2fbcb3211b0..29fb86f5a73 100644 --- a/src/commands/deploy/deploy.ts +++ b/src/commands/deploy/deploy.ts @@ -37,6 +37,7 @@ import { logJson, warn, type APIError, + NETLIFYDEVWARN, } from '../../utils/command-helpers.js' import { DEFAULT_CONCURRENT_HASH, DEFAULT_DEPLOY_TIMEOUT } from '../../utils/deploy/constants.js' import { type DeployEvent, deploySite } from '../../utils/deploy/deploy-site.js' @@ -438,7 +439,17 @@ const reportDeployError = ({ const deployProgressCb = function () { const spinnersByType: Record = {} + // Steps that produce concurrent stdout output (e.g., esbuild during bundling) + // should not use animated spinners to avoid mixing output (see #2391). + const noSpinnerTypes = new Set(['edge-functions-bundling']) return (event: DeployEvent) => { + if (noSpinnerTypes.has(event.type)) { + // For concurrent-output steps, log text status only (no spinner). + if (event.phase === 'stop') { + log(event.msg) + } + return + } switch (event.phase) { case 'start': { spinnersByType[event.type] = startSpinner({ @@ -769,10 +780,11 @@ const bundleEdgeFunctions = async (options: DeployOptionValues, command: BaseCom const argv = process.argv.slice(2) const statusCb = options.silent || argv.includes('--json') || argv.includes('--silent') ? () => {} : deployProgressCb() - + // During bundling, esbuild outputs to stdout concurrently. deployProgressCb + // skips the spinner for this step to avoid mixing output (see #2391). statusCb({ type: 'edge-functions-bundling', - msg: 'Bundling edge functions...\n', + msg: 'Bundling edge functions...', phase: 'start', }) @@ -944,6 +956,22 @@ const prepAndRunDeploy = async ({ const deployFolder = await getDeployFolder({ command, options, config, site, siteData }) const functionsFolder = getFunctionsFolder({ workingDir, options, config, site, siteData }) + // When deploying without running a build, warn if build plugins are configured + // because their config mutations are lost without a build run + // (see https://github.com/netlify/cli/issues/3792). + if (!options.build) { + type ConfigPlugin = { package?: unknown; origin?: string } + const plugins = + (config?.plugins as ConfigPlugin[] | undefined) ?? + (command.netlify.cachedConfig.config as { plugins?: ConfigPlugin[] } | undefined)?.plugins + const configuredPlugins = plugins?.filter((plugin) => plugin.origin !== 'default') ?? [] + if (configuredPlugins.length > 0) { + log( + `${NETLIFYDEVWARN} Site uses build plugins (${configuredPlugins.map((p) => p.package).join(', ')}) but no build is being run.\n` + + ` Config changes made by these plugins will not be applied. Use ${chalk.cyanBright('netlify deploy --build')} to build and deploy together.`, + ) + } + } const { configPath } = site // build flag wasn't used and edge functions directories exist diff --git a/src/commands/logs/sources/deploy.ts b/src/commands/logs/sources/deploy.ts index d58c548ddb0..044a23bc6a9 100644 --- a/src/commands/logs/sources/deploy.ts +++ b/src/commands/logs/sources/deploy.ts @@ -1,16 +1,16 @@ -import type { NetlifyAPI } from '@netlify/api' +import type { NetlifyAPI } from "@netlify/api"; -import { getWebSocket } from '../../../utils/websockets/index.js' -import { debugFetch } from '../log-api.js' -import type { LogEntry } from '../log-api.js' +import { getWebSocket } from "../../../utils/websockets/index.js"; +import { debugFetch } from "../log-api.js"; +import type { LogEntry } from "../log-api.js"; interface DeployLogLine { - ts: string - log?: string - message?: string - level?: string - section?: string - type?: string + ts: string; + log?: string; + message?: string; + level?: string; + section?: string; + type?: string; } export const fetchDeployHistoricalLogs = async ({ @@ -20,44 +20,44 @@ export const fetchDeployHistoricalLogs = async ({ from, to, }: { - apiBase: string - accessToken: string | null | undefined - deployId: string - from: number - to: number + apiBase: string; + accessToken: string | null | undefined; + deployId: string; + from: number; + to: number; }): Promise => { - const response = await debugFetch(`${apiBase}/api/v1/deploys/${encodeURIComponent(deployId)}/log`, { + const response = await debugFetch(`${apiBase}/deploys/${encodeURIComponent(deployId)}/log`, { headers: { - Authorization: `Bearer ${accessToken ?? ''}`, + Authorization: `Bearer ${accessToken ?? ""}`, }, - }) + }); if (!response.ok) { - throw new Error(`Failed to fetch deploy logs: ${response.status.toString()} ${response.statusText}`) + throw new Error(`Failed to fetch deploy logs: ${response.status.toString()} ${response.statusText}`); } - const logData = (await response.json()) as DeployLogLine[] + const logData = (await response.json()) as DeployLogLine[]; if (!Array.isArray(logData)) { - return [] + return []; } return logData .map((line): LogEntry | null => { - const ts = new Date(line.ts).getTime() + const ts = new Date(line.ts).getTime(); if (Number.isNaN(ts) || ts < from || ts > to) { - return null + return null; } return { - source: 'deploy', - name: 'deploy', + source: "deploy", + name: "deploy", ts, - level: line.level ?? 'INFO', - message: line.log ?? line.message ?? '', + level: line.level ?? "INFO", + message: line.log ?? line.message ?? "", section: line.section, - } + }; }) - .filter((entry): entry is LogEntry => entry !== null) -} + .filter((entry): entry is LogEntry => entry !== null); +}; export const streamDeploy = ( siteId: string, @@ -66,56 +66,56 @@ export const streamDeploy = ( onEntry: (entry: LogEntry) => void, onClose: () => void, ): (() => void) => { - const ws = getWebSocket('wss://socketeer.services.netlify.com/build/logs') + const ws = getWebSocket("wss://socketeer.services.netlify.com/build/logs"); - ws.on('open', () => { + ws.on("open", () => { ws.send( JSON.stringify({ deploy_id: deployId, site_id: siteId, access_token: accessToken, }), - ) - }) + ); + }); - ws.on('message', (data: string) => { + ws.on("message", (data: string) => { const logData = JSON.parse(data) as { - message: string - section?: string - type?: string - level?: string - ts?: string - } + message: string; + section?: string; + type?: string; + level?: string; + ts?: string; + }; onEntry({ - source: 'deploy', - name: 'deploy', + source: "deploy", + name: "deploy", ts: logData.ts ? new Date(logData.ts).getTime() : Date.now(), - level: logData.level ?? 'INFO', + level: logData.level ?? "INFO", message: logData.message, section: logData.section, - }) + }); - if (logData.type === 'report' && logData.section === 'building') { - ws.close() + if (logData.type === "report" && logData.section === "building") { + ws.close(); } - }) + }); - ws.on('close', () => { - onClose() - }) + ws.on("close", () => { + onClose(); + }); return () => { - ws.close() - } -} + ws.close(); + }; +}; export const findCurrentBuildingDeploy = async (client: NetlifyAPI, siteId: string): Promise => { - const deploys = (await client.listSiteDeploys({ siteId, state: 'building' })) as { id: string }[] - return deploys.length > 0 ? deploys[0].id : undefined -} + const deploys = (await client.listSiteDeploys({ siteId, state: "building" })) as { id: string }[]; + return deploys.length > 0 ? deploys[0].id : undefined; +}; export const findLatestReadyDeploy = async (client: NetlifyAPI, siteId: string): Promise => { - const deploys = (await client.listSiteDeploys({ siteId, state: 'ready', per_page: 1 })) as { id: string }[] - return deploys.length > 0 ? deploys[0].id : undefined -} + const deploys = (await client.listSiteDeploys({ siteId, state: "ready", per_page: 1 })) as { id: string }[]; + return deploys.length > 0 ? deploys[0].id : undefined; +}; diff --git a/src/commands/watch/watch.ts b/src/commands/watch/watch.ts index da36642468b..35b2f54e2a4 100644 --- a/src/commands/watch/watch.ts +++ b/src/commands/watch/watch.ts @@ -15,7 +15,7 @@ const BUILD_FINISH_INTERVAL = 1e3 // 20 minutes const BUILD_FINISH_TIMEOUT = 12e5 -const waitForBuildFinish = async function (api: NetlifyAPI, siteId: string, spinner: Spinner) { +const waitForBuildFinish = async function (api: NetlifyAPI, siteId: string, spinner: Spinner | undefined) { let firstPass = true const waitForBuildToFinish = async function () { @@ -27,7 +27,11 @@ const waitForBuildFinish = async function (api: NetlifyAPI, siteId: string, spin // @TODO implement build error messages into this if (!currentBuilds || currentBuilds.length === 0) { - stopSpinner({ spinner }) + if (spinner) { + stopSpinner({ spinner }) + } else { + log('Waiting for active project deploys to complete... done') + } return true } firstPass = false @@ -46,7 +50,7 @@ const waitForBuildFinish = async function (api: NetlifyAPI, siteId: string, spin return firstPass } -export const watch = async (_options: unknown, command: BaseCommand) => { +export const watch = async (options: { silent?: boolean; json?: boolean }, command: BaseCommand) => { await command.authenticate() const client = command.netlify.api let siteId = command.netlify.site.id @@ -80,7 +84,11 @@ export const watch = async (_options: unknown, command: BaseCommand) => { // "created_at": "2018-07-17T17:14:03.423Z" // } // - const spinner = startSpinner({ text: 'Waiting for active project deploys to complete' }) + // Allow suppressing the spinner via --silent or --json (see #5301) + const suppressSpinner = options?.silent || options?.json + const spinner = suppressSpinner + ? undefined + : startSpinner({ text: 'Waiting for active project deploys to complete' }) try { // Fetch all builds! // const builds = await client.listSiteBuilds({siteId}) diff --git a/src/utils/detect-server-settings.ts b/src/utils/detect-server-settings.ts index 0355c4a3097..1e711a75702 100644 --- a/src/utils/detect-server-settings.ts +++ b/src/utils/detect-server-settings.ts @@ -310,7 +310,7 @@ const detectServerSettings = async ( return { ...settings, port: acquiredPort, - jwtSecret: devConfig.jwtSecret || 'secret', + jwtSecret: devConfig.jwtSecret || process.env.NETLIFY_DEV_JWT_SECRET || 'secret', jwtRolePath: devConfig.jwtRolePath || 'app_metadata.authorization.roles', functions: functionsDir, functionsPort: await getPort({ port: devConfig.functionsPort || 0 }), diff --git a/src/utils/init/config-manual.ts b/src/utils/init/config-manual.ts index 40a026e9cee..eddaafc6367 100644 --- a/src/utils/init/config-manual.ts +++ b/src/utils/init/config-manual.ts @@ -35,7 +35,7 @@ const getRepoPath = async ({ repoData }: { repoData: RepoData }): Promise (SSH_URL_REGEXP.test(url) ? true : 'The URL provided does not use the SSH protocol'), }, ]) @@ -43,6 +43,41 @@ const getRepoPath = async ({ repoData }: { repoData: RepoData }): Promise { + if (SSH_URL_REGEXP.test(url)) { + return url + } + if (provider === 'github') { + return githubHttpsToSsh(url) + } + if (provider === 'gitlab') { + return gitlabHttpsToSsh(url) + } + return url +} + +const githubHttpsToSsh = (url: string): string => { + try { + const parsed = new URL(url) + return `git@${parsed.hostname}:${parsed.pathname.replace(/^\//, '').replace(/\.git$/, '')}.git` + } catch { + return url + } +} + +const gitlabHttpsToSsh = (url: string): string => { + try { + const parsed = new URL(url) + return `git@${parsed.hostname}:${parsed.pathname.replace(/^\//, '').replace(/\.git$/, '')}.git` + } catch { + return url + } +} + const addDeployHook = async (deployHook: string | undefined): Promise => { log('\nConfigure the following webhook for your repository:\n') // FIXME(serhalp): Handle nullish `deployHook` by throwing user-facing error or fixing upstream type. diff --git a/src/utils/redirects.ts b/src/utils/redirects.ts index ffa925e1a8d..46162570187 100644 --- a/src/utils/redirects.ts +++ b/src/utils/redirects.ts @@ -36,6 +36,11 @@ const getErrorMessage = function ({ message }) { // - `from` is called `origin` // - `query` is called `params` // - `conditions.role|country|language` are capitalized +// Leading and trailing whitespace in `from` and `to` is trimmed so that typos +// such as `to = " https://example.com"` do not silently break redirects +// (see https://github.com/netlify/cli/issues/4707). +const trimValue = (value: string | unknown): string | unknown => (typeof value === 'string' ? value.trim() : value) + const normalizeRedirect = function ({ // @ts-expect-error TS(7031) FIXME: Binding element 'country' implicitly has an 'any' ... Remove this comment to see the full error message conditions: { country, language, role, ...conditions }, @@ -45,11 +50,15 @@ const normalizeRedirect = function ({ query, // @ts-expect-error TS(7031) FIXME: Binding element 'signed' implicitly has an 'any' t... Remove this comment to see the full error message signed, + // @ts-expect-error TS(7031) FIXME: Binding element 'to' implicitly has an 'any type... + to, ...redirect }) { return { ...redirect, - origin: from, + origin: trimValue(from), + path: trimValue(from), + to: trimValue(to), params: query, conditions: { ...conditions, diff --git a/tests/unit/commands/logs/deploy-source.test.ts b/tests/unit/commands/logs/deploy-source.test.ts new file mode 100644 index 00000000000..540481ac16f --- /dev/null +++ b/tests/unit/commands/logs/deploy-source.test.ts @@ -0,0 +1,85 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; + +// Mock the debugFetch to capture the URL +const mockDebugFetch = vi.fn(); +vi.mock("../../../../src/commands/logs/log-api.js", () => ({ + debugFetch: (...args: unknown[]) => mockDebugFetch(...(args as [])) as unknown, +})); + +import { fetchDeployHistoricalLogs } from "../../../../src/commands/logs/sources/deploy.js"; + +describe("fetchDeployHistoricalLogs", () => { + beforeEach(() => { + mockDebugFetch.mockReset(); + mockDebugFetch.mockResolvedValue({ + ok: true, + json: () => Promise.resolve([]), + }); + }); + + test("constructs URL without duplicated /api/v1 prefix", async () => { + const apiBase = "https://api.netlify.com/api/v1"; + await fetchDeployHistoricalLogs({ + apiBase, + accessToken: "test-token", + deployId: "abc123", + from: 0, + to: Date.now(), + }); + + const calledUrl = mockDebugFetch.mock.calls[0]?.[0] as string; + // Should NOT have /api/v1/api/v1 + expect(calledUrl).not.toContain("/api/v1/api/v1"); + // Should have the correct URL format + expect(calledUrl).toBe("https://api.netlify.com/api/v1/deploys/abc123/log"); + }); + + test("works with apiBase that already ends with /api/v1", async () => { + const apiBase = "https://api.netlify.com/api/v1"; + await fetchDeployHistoricalLogs({ + apiBase, + accessToken: null, + deployId: "def456", + from: 0, + to: Date.now(), + }); + + const calledUrl = mockDebugFetch.mock.calls[0]?.[0] as string; + expect(calledUrl).toBe("https://api.netlify.com/api/v1/deploys/def456/log"); + }); + + test("returns empty array when response is not an array", async () => { + mockDebugFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve("not an array"), + }); + + const result = await fetchDeployHistoricalLogs({ + apiBase: "https://api.netlify.com/api/v1", + accessToken: null, + deployId: "xyz789", + from: 0, + to: Date.now(), + }); + + expect(result).toEqual([]); + }); + + test("throws on non-ok response", async () => { + mockDebugFetch.mockResolvedValueOnce({ + ok: false, + status: 404, + statusText: "Not Found", + }); + + await expect( + fetchDeployHistoricalLogs({ + apiBase: "https://api.netlify.com/api/v1", + accessToken: null, + deployId: "bad123", + from: 0, + to: Date.now(), + }), + ).rejects.toThrow("Failed to fetch deploy logs: 404 Not Found"); + }); +}); diff --git a/tests/unit/utils/redirects.test.ts b/tests/unit/utils/redirects.test.ts index 31492fd5da0..4484af07778 100644 --- a/tests/unit/utils/redirects.test.ts +++ b/tests/unit/utils/redirects.test.ts @@ -230,3 +230,29 @@ test('should parse redirect rules from _redirects file and netlify.toml', async expect(redirects).toEqual(expected) }) }) + +test('should trim leading and trailing whitespace from redirect `from` and `to`', async (t) => { + await withSiteBuilder(t, async (builder) => { + await builder + .withNetlifyToml({ + config: { + redirects: [ + { + from: ' /leading-space ', + status: 200, + to: ' https://www.netlify.com ', + }, + ], + }, + }) + .build() + + // @ts-expect-error TS(2345) FIXME: Argument of type '{ configPath: string; }' is not ... Remove this comment to see the full error message + const redirects = await parseRedirects({ configPath: `${builder.directory}/netlify.toml` }) + expect(redirects[0]).toMatchObject({ + origin: '/leading-space', + path: '/leading-space', + to: 'https://www.netlify.com', + }) + }) +}) diff --git a/tests/unit/utils/to-ssh-url.test.ts b/tests/unit/utils/to-ssh-url.test.ts new file mode 100644 index 00000000000..31cb93df80c --- /dev/null +++ b/tests/unit/utils/to-ssh-url.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from 'vitest' + +import { toSshUrl } from '../../../src/utils/init/config-manual.js' + +describe('toSshUrl', () => { + test('returns ssh url unchanged for github', () => { + const url = 'git@github.com:user/repo.git' + expect(toSshUrl(url, 'github')).toBe(url) + }) + + test('converts https github url to ssh format', () => { + const url = 'https://github.com/user/repo.git' + expect(toSshUrl(url, 'github')).toBe('git@github.com:user/repo.git') + }) + + test('converts https github url without .git extension', () => { + const url = 'https://github.com/user/repo' + expect(toSshUrl(url, 'github')).toBe('git@github.com:user/repo.git') + }) + + test('converts https gitlab url to ssh format', () => { + const url = 'https://gitlab.com/group/subgroup/repo.git' + expect(toSshUrl(url, 'gitlab')).toBe('git@gitlab.com:group/subgroup/repo.git') + }) + + test('returns https url unchanged for unknown provider', () => { + const url = 'https://bitbucket.org/user/repo.git' + expect(toSshUrl(url, 'bitbucket')).toBe(url) + }) + + test('returns https url unchanged for null provider', () => { + const url = 'https://example.com/user/repo.git' + expect(toSshUrl(url, null)).toBe(url) + }) + + test('returns invalid url unchanged', () => { + const url = 'not-a-valid-url' + expect(toSshUrl(url, 'github')).toBe(url) + }) + + test('handles ssh:// protocol', () => { + const url = 'ssh://git@github.com/user/repo.git' + expect(toSshUrl(url, 'github')).toBe(url) + }) +})