Skip to content

Commit

Permalink
fix: cleanup blobs before build (#5413)
Browse files Browse the repository at this point in the history
  • Loading branch information
kitop authored Nov 28, 2023
1 parent ca3d3a6 commit 54e62eb
Show file tree
Hide file tree
Showing 24 changed files with 128 additions and 55 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ packages/*/lib/
**/fixtures/**/edge-functions-dist
**/fixtures/**/edge-functions-import-map.json
**/**/secrets_scanning/fixtures/**/dist
!**/blobs_upload/fixtures/**/dist/.netlify
!**/fixtures/**/node_modules
!**/fixtures/**/plugins_cache*/.netlify
!**/fixtures/**/pin_*/.netlify
Expand Down
4 changes: 2 additions & 2 deletions packages/build/src/plugins_core/blobs_upload/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import pMap from 'p-map'
import semver from 'semver'

import { log, logError } from '../../log/logger.js'
import { getBlobsDir } from '../../utils/blobs.js'
import { anyBlobsToUpload, getBlobsDir } from '../../utils/blobs.js'

import { getKeysToUpload, getFileWithMetadata, anyBlobsToUpload } from './utils.js'
import { getKeysToUpload, getFileWithMetadata } from './utils.js'

const coreStep = async function ({
debug,
Expand Down
8 changes: 0 additions & 8 deletions packages/build/src/plugins_core/blobs_upload/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,9 @@ import path from 'node:path'

import { fdir } from 'fdir'

import { getBlobsDir } from '../../utils/blobs.js'

const METADATA_PREFIX = '$'
const METADATA_SUFFIX = '.json'

export const anyBlobsToUpload = async function ({ buildDir, publishDir }) {
const blobsDir = getBlobsDir({ buildDir, publishDir })
const { files } = await new fdir().onlyCounts().crawl(blobsDir).withPromise()
return files > 0
}

/** Given output directory, find all file paths to upload excluding metadata files */
export async function getKeysToUpload(blobsDir: string): Promise<string[]> {
const files = await new fdir()
Expand Down
13 changes: 0 additions & 13 deletions packages/build/src/plugins_core/build_command.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { rm } from 'node:fs/promises'
import { platform } from 'process'

import { execa } from 'execa'
Expand All @@ -7,16 +6,6 @@ import { addErrorInfo } from '../error/info.js'
import { getBuildCommandDescription } from '../log/description.js'
import { logBuildCommandStart } from '../log/messages/steps.js'
import { getBuildCommandStdio, handleBuildCommandOutput } from '../log/stream.js'
import { getBlobsDir } from '../utils/blobs.js'

async function cleanupBlobsDir(buildDir, publishDir) {
const blobsDir = getBlobsDir({ buildDir, publishDir })
try {
await rm(blobsDir, { recursive: true, force: true })
} catch {
// Ignore errors if it fails, we can continue anyway.
}
}

// Fire `build.command`
const coreStep = async function ({
Expand All @@ -25,12 +14,10 @@ const coreStep = async function ({
nodePath,
childEnv,
logs,
constants: { PUBLISH_DIR },
netlifyConfig: {
build: { command: buildCommand, commandOrigin: buildCommandOrigin },
},
}) {
await cleanupBlobsDir(buildDir, PUBLISH_DIR)
logBuildCommandStart(logs, buildCommand)

const stdio = getBuildCommandStdio(logs)
Expand Down
27 changes: 27 additions & 0 deletions packages/build/src/plugins_core/pre_cleanup/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { rm } from 'node:fs/promises'

import { anyBlobsToUpload, getBlobsDir } from '../../utils/blobs.js'

const coreStep = async function ({ buildDir, constants: { PUBLISH_DIR } }) {
const blobsDir = getBlobsDir({ buildDir, publishDir: PUBLISH_DIR })
try {
await rm(blobsDir, { recursive: true, force: true })
} catch {
// Ignore errors if it fails, we can continue anyway.
}

return {}
}

const blobsPresent = async function ({ buildDir, constants: { PUBLISH_DIR } }): Promise<boolean> {
return await anyBlobsToUpload({ buildDir, publishDir: PUBLISH_DIR })
}

export const preCleanup = {
event: 'onPreBuild',
coreStep,
coreStepId: 'pre_cleanup',
coreStepName: 'Pre cleanup',
coreStepDescription: () => 'Cleaning up leftover files from previous builds',
condition: blobsPresent,
}
2 changes: 2 additions & 0 deletions packages/build/src/steps/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { buildCommandCore } from '../plugins_core/build_command.js'
import { deploySite } from '../plugins_core/deploy/index.js'
import { bundleEdgeFunctions } from '../plugins_core/edge_functions/index.js'
import { bundleFunctions } from '../plugins_core/functions/index.js'
import { preCleanup } from '../plugins_core/pre_cleanup/index.js'
import { saveArtifacts } from '../plugins_core/save_artifacts/index.js'
import { scanForSecrets } from '../plugins_core/secrets_scanning/index.js'

Expand Down Expand Up @@ -68,6 +69,7 @@ const getEventSteps = function (eventHandlers) {

const addCoreSteps = function (steps) {
return [
preCleanup,
buildCommandCore,
...steps,
bundleFunctions,
Expand Down
8 changes: 8 additions & 0 deletions packages/build/src/utils/blobs.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import path from 'node:path'

import { fdir } from 'fdir'

const BLOBS_PATH = '.netlify/blobs/deploy'

export const getBlobsDir = function ({ buildDir, publishDir }) {
return path.resolve(buildDir, publishDir, BLOBS_PATH)
}

export const anyBlobsToUpload = async function ({ buildDir, publishDir }) {
const blobsDir = getBlobsDir({ buildDir, publishDir })
const { files } = await new fdir().onlyCounts().crawl(blobsDir).withPromise()
return files > 0
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { mkdir, writeFile } from 'node:fs/promises'

await mkdir('dist/.netlify/blobs/deploy/nested', { recursive: true })

await Promise.all([
writeFile('dist/.netlify/blobs/deploy/something.txt', 'some value'),
writeFile('dist/.netlify/blobs/deploy/with-metadata.txt', 'another value'),
writeFile('dist/.netlify/blobs/deploy/$with-metadata.txt.json', JSON.stringify({ "meta": "data", "number": 1234 })),
writeFile('dist/.netlify/blobs/deploy/nested/file.txt', 'file value'),
writeFile('dist/.netlify/blobs/deploy/nested/$file.txt.json', JSON.stringify({ "some": "metadata" })),
])

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[build]
command = "node build.mjs"
base = "/"
publish = "/dist"
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { mkdir, writeFile } from 'node:fs/promises'

await mkdir('dist/.netlify/blobs/deploy', { recursive: true })

await Promise.all([
writeFile('dist/.netlify/blobs/deploy/with-metadata.txt', 'another value'),
writeFile('dist/.netlify/blobs/deploy/$with-metadata.txt.json', 'this is not json'),
])

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[build]
command = "node build.mjs"
base = "/"
publish = "/dist"
22 changes: 18 additions & 4 deletions packages/build/tests/blobs_upload/tests.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { access } from 'node:fs/promises'
import { version as nodeVersion } from 'node:process'
import { join } from 'path'

import { BlobsServer, getDeployStore } from '@netlify/blobs'
import { Fixture } from '@netlify/testing'
Expand Down Expand Up @@ -53,23 +55,31 @@ test("blobs upload, don't run when deploy id is provided and no files in directo
})

test("blobs upload, don't run when there are files but deploy id is not provided", async (t) => {
const fixture = await new Fixture('./fixtures/src_with_blobs').withCopyRoot({ git: false })

const {
success,
logs: { stdout },
} = await new Fixture('./fixtures/src_with_blobs').withFlags({ token: TOKEN, offline: true }).runBuildProgrammatic()
} = await fixture.withFlags({ token: TOKEN, offline: true, cwd: fixture.repositoryRoot }).runBuildProgrammatic()

t.true(success)

const blobsDir = join(fixture.repositoryRoot, 'dist', '.netlify', 'blobs', 'deploy')
await t.notThrowsAsync(access(blobsDir))

t.is(t.context.blobRequestCount.set, 0)

t.false(stdout.join('\n').includes('Uploading blobs to deploy store'))
})

test.serial('blobs upload, uploads files to deploy store', async (t) => {
const fixture = await new Fixture('./fixtures/src_with_blobs').withCopyRoot({ git: false })

const {
success,
logs: { stdout },
} = await new Fixture('./fixtures/src_with_blobs')
.withFlags({ deployId: 'abc123', siteId: 'test', token: TOKEN, offline: true })
} = await fixture
.withFlags({ deployId: 'abc123', siteId: 'test', token: TOKEN, offline: true, cwd: fixture.repositoryRoot })
.runBuildProgrammatic()

t.true(success)
Expand Down Expand Up @@ -99,10 +109,14 @@ test.serial('blobs upload, uploads files to deploy store', async (t) => {
})

test('blobs upload, cancels deploy if blob metadata is malformed', async (t) => {
const { success, severityCode } = await new Fixture('./fixtures/src_with_malformed_blobs_metadata')
const fixture = await new Fixture('./fixtures/src_with_malformed_blobs_metadata').withCopyRoot({ git: false })
const { success, severityCode } = await fixture
.withFlags({ deployId: 'abc123', siteId: 'test', token: TOKEN, offline: true, debug: false })
.runBuildProgrammatic()

const blobsDir = join(fixture.repositoryRoot, 'dist', '.netlify', 'blobs', 'deploy')
await t.notThrowsAsync(access(blobsDir))

t.is(t.context.blobRequestCount.set, 0)

t.false(success)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[build]
base = "/"
publish = "/"
47 changes: 47 additions & 0 deletions packages/build/tests/pre_cleanup/tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { access } from 'node:fs/promises'
import { join } from 'path'

import { Fixture } from '@netlify/testing'
import test from 'ava'

test('Build removes blobs directory before starting', async (t) => {
const fixture = await new Fixture('./fixtures/with_preexisting_blobs').withCopyRoot({ git: false })

const blobsDir = join(fixture.repositoryRoot, 'out', '.netlify', 'blobs', 'deploy')

await t.notThrowsAsync(access(blobsDir))

const {
success,
logs: { stdout },
} = await fixture
.withFlags({
cwd: fixture.repositoryRoot,
})
.runBuildProgrammatic()

t.true(success)
t.true(stdout.join('\n').includes('Cleaning up leftover files from previous builds'))

await t.throwsAsync(access(blobsDir))
})

test('Build does not log if there is nothing to cleanup', async (t) => {
const fixture = await new Fixture('./fixtures/src_empty').withCopyRoot({ git: false })

const blobsDir = join(fixture.repositoryRoot, 'out', '.netlify', 'blobs', 'deploy')

await t.throwsAsync(access(blobsDir))

const {
success,
logs: { stdout },
} = await fixture
.withFlags({
cwd: fixture.repositoryRoot,
})
.runBuildProgrammatic()

t.true(success)
t.false(stdout.join('\n').includes('Cleaning up leftover files from previous builds'))
})
20 changes: 0 additions & 20 deletions packages/build/tests/steps/tests.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import fsPromises from 'node:fs/promises'
import path from 'path'
import { platform } from 'process'

import { Fixture, normalizeOutput } from '@netlify/testing'
Expand Down Expand Up @@ -43,21 +41,3 @@ test('Invalid package.json does not make build fail', async (t) => {
const output = await new Fixture('./fixtures/invalid_package_json').runWithBuild()
t.snapshot(normalizeOutput(output))
})

test('Build removes blobs directory before starting if there is a build command', async (t) => {
const fixture = await new Fixture('./fixtures/with_preexisting_blobs').withCopyRoot({ git: false })

const blobsDir = path.join(fixture.repositoryRoot, 'out', '.netlify', 'blobs', 'deploy')

await t.notThrowsAsync(fsPromises.access(blobsDir))

const { success } = await fixture
.withFlags({
cwd: fixture.repositoryRoot,
})
.runBuildProgrammatic()

t.true(success)

await t.throwsAsync(fsPromises.access(blobsDir))
})

0 comments on commit 54e62eb

Please sign in to comment.