actions/publish-immutable-action
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactored path calculation

Browse Source
This commit is contained in:
Edwin Sirko
2024-01-26 16:00:36 -05:00
parent cb62dd8450
commit 7472b3f822
5 changed files with 194 additions and 123 deletions
Download Patch File Download Diff File Expand all files Collapse all files
__tests__/fs-helper.test.ts
+111 -52
View File
@@ -6,6 +6,117 @@ import { execSync } from 'child_process'
const fileContent = 'This is the content of the file'
describe('getConsolidatedDirectory', () => {
let sourceDir: string
beforeAll(() => {
sourceDir = `.` // fsHelper.createTempDir()
fs.mkdirSync(`${sourceDir}/folder1`)
fs.mkdirSync(`${sourceDir}/folder2`)
fs.mkdirSync(`${sourceDir}/folder2/folder3`)
fs.writeFileSync(`${sourceDir}/file0.txt`, fileContent)
fs.writeFileSync(`${sourceDir}/folder1/file1.txt`, fileContent)
fs.writeFileSync(`${sourceDir}/folder2/file2.txt`, fileContent)
fs.writeFileSync(`${sourceDir}/folder2/folder3/file3.txt`, fileContent)
})
beforeEach(() => {})
afterEach(() => {})
afterAll(() => {
fs.rmSync(`file0.txt`)
fs.rmSync(`folder1`, { recursive: true })
fs.rmSync(`folder2`, { recursive: true })
})
it('returns the directory itself if it is a single directory, and instructed to not clean it up', () => {
// TODO: In these tests, we're not really distinguishing between the `publish-action-package` directory and the consumer repo directory, i.e., they share the same space.
// In real life, when the consumer workflow runs, its own javascript is in ., but
// the publish-action-package's code is in ${{github.action_path}}.
// So.... I guess to emulate this, we should create a temp directory (representing the consumer repo)
// and cd there before the test starts?
const { consolidatedPath, needToCleanUpDir } =
fsHelper.getConsolidatedDirectory('.')
expect(needToCleanUpDir).toBe(false)
expect(consolidatedPath).toBe('.')
expect(fsHelper.readFileContents(`file0.txt`).toString()).toEqual(
fileContent
)
expect(fsHelper.readFileContents(`folder1/file1.txt`).toString()).toEqual(
fileContent
)
expect(fsHelper.readFileContents(`folder2/file2.txt`).toString()).toEqual(
fileContent
)
expect(
fsHelper.readFileContents(`folder2/folder3/file3.txt`).toString()
).toEqual(fileContent)
})
it('returns a new directory containing copies of the multiple paths if they are legally specified, and instruct to clean it up', () => {
const { consolidatedPath, needToCleanUpDir } =
fsHelper.getConsolidatedDirectory('file0.txt folder1')
expect(needToCleanUpDir).toBe(true)
expect(consolidatedPath).not.toBe('.')
expect(
fsHelper
.readFileContents(path.join(consolidatedPath, `file0.txt`))
.toString()
).toEqual(fileContent)
expect(
fsHelper
.readFileContents(path.join(consolidatedPath, `folder1/file1.txt`))
.toString()
).toEqual(fileContent)
expect(
fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`))
).toEqual(false)
expect(
fs.existsSync(path.join(consolidatedPath, `folder2/folder3/file3.txt`))
).toEqual(false)
})
it('what happens here?', () => {
const { consolidatedPath, needToCleanUpDir } =
fsHelper.getConsolidatedDirectory('folder1 folder2/folder3')
expect(needToCleanUpDir).toBe(true)
expect(consolidatedPath).not.toBe('.')
expect(fs.existsSync(path.join(consolidatedPath, `file0.txt`))).toEqual(
false
)
expect(
fsHelper
.readFileContents(path.join(consolidatedPath, `folder1/file1.txt`))
.toString()
).toEqual(fileContent)
expect(
fs.existsSync(path.join(consolidatedPath, `folder2/file2.txt`))
).toEqual(false)
expect(
fsHelper
.readFileContents(path.join(consolidatedPath, `folder3/file3.txt`))
.toString()
).toEqual(fileContent) // <--- TODO: This is what I'm unsure of
})
it('throws an error for illegal path spec - single', () => {
expect(() => {
fsHelper.getConsolidatedDirectory('folder4')
}).toThrow('filePath folder4 does not exist')
})
it('throws an error for illegal path spec - multiple', () => {
expect(() => {
fsHelper.getConsolidatedDirectory('folder1 folder4')
}).toThrow('filePath folder4 does not exist')
})
// TODO: consider doing the thing Michael suggested where we exclude directories starting with .
})
describe('createArchives', () => {
let tmpDir: string
let distDir: string
@@ -196,55 +307,3 @@ describe('removeDir', () => {
expect(fs.existsSync(dir)).toEqual(false)
})
})
describe('bundleFilesintoDirectory', () => {
let sourceDir: string
let targetDir: string
beforeEach(() => {
sourceDir = fsHelper.createTempDir()
targetDir = fsHelper.createTempDir()
})
afterEach(() => {
fs.rmSync(sourceDir, { recursive: true })
fs.rmSync(targetDir, { recursive: true })
})
it('bundles files and folders into a directory', () => {
// Create some test files and folders in the sourceDir
const file1 = `${sourceDir}/file1.txt`
const folder1 = `${sourceDir}/folder1`
const file2 = `${folder1}/file2.txt`
const folder2 = `${folder1}/folder2`
const file3 = `${folder2}/file3.txt`
fs.mkdirSync(folder1)
fs.mkdirSync(folder2)
fs.writeFileSync(file1, fileContent)
fs.writeFileSync(file2, fileContent)
fs.writeFileSync(file3, fileContent)
// Bundle the files and folders into the targetDir
fsHelper.bundleFilesintoDirectory([file1, folder1], targetDir)
// Check that the files and folders were copied
expect(fs.existsSync(file1)).toEqual(true)
expect(fsHelper.readFileContents(file1).toString()).toEqual(fileContent)
expect(fs.existsSync(`${targetDir}/folder1`)).toEqual(true)
expect(fs.existsSync(file2)).toEqual(true)
expect(fsHelper.readFileContents(file2).toString()).toEqual(fileContent)
expect(fs.existsSync(`${targetDir}/folder1/folder2`)).toEqual(true)
expect(fs.existsSync(file3)).toEqual(true)
expect(fsHelper.readFileContents(file3).toString()).toEqual(fileContent)
})
it('throws an error if a file or directory does not exist', () => {
expect(() => {
fsHelper.bundleFilesintoDirectory(['/does/not/exist'], targetDir)
}).toThrow('File /does/not/exist does not exist')
})
})
__tests__/main.test.ts
+10 -13
View File
@@ -20,10 +20,9 @@ let setOutputMock: jest.SpyInstance
// Mock the filesystem helper
let createTempDirMock: jest.SpyInstance
let isDirectoryMock: jest.SpyInstance
let createArchivesMock: jest.SpyInstance
let removeDirMock: jest.SpyInstance
let bundleFilesintoDirectoryMock: jest.SpyInstance
let getConsolidatedDirectoryMock: jest.SpyInstance
let isActionRepoMock: jest.SpyInstance
// Mock the GHCR Client
@@ -42,13 +41,12 @@ describe('action', () => {
createTempDirMock = jest
.spyOn(fsHelper, 'createTempDir')
.mockImplementation()
isDirectoryMock = jest.spyOn(fsHelper, 'isDirectory').mockImplementation()
createArchivesMock = jest
.spyOn(fsHelper, 'createArchives')
.mockImplementation()
removeDirMock = jest.spyOn(fsHelper, 'removeDir').mockImplementation()
bundleFilesintoDirectoryMock = jest
.spyOn(fsHelper, 'bundleFilesintoDirectory')
getConsolidatedDirectoryMock = jest
.spyOn(fsHelper, 'getConsolidatedDirectory')
.mockImplementation()
isActionRepoMock = jest.spyOn(fsHelper, 'isActionRepo').mockImplementation()
@@ -122,9 +120,7 @@ describe('action', () => {
return ''
})
isDirectoryMock.mockImplementation(() => true)
bundleFilesintoDirectoryMock.mockImplementation(() => {
getConsolidatedDirectoryMock.mockImplementation(() => {
throw new Error('Something went wrong')
})
@@ -154,7 +150,9 @@ describe('action', () => {
return ''
})
isDirectoryMock.mockImplementation(() => true)
getConsolidatedDirectoryMock.mockImplementation(() => {
return { consolidatedDirectory: '/tmp/test', needToCleanUpDir: false }
})
isActionRepoMock.mockImplementation(() => true)
createTempDirMock.mockImplementation(() => '/tmp/test')
@@ -167,7 +165,7 @@ describe('action', () => {
await main.run('directory')
// Check the results
expect(isDirectoryMock).toHaveBeenCalledWith('directory')
expect(getConsolidatedDirectoryMock).toHaveBeenCalledTimes(1)
expect(setFailedMock).toHaveBeenCalledWith('Something went wrong')
// Expect the files to be cleaned up
@@ -207,11 +205,10 @@ async function testHappyPath(version: string, path: string): Promise<void> {
return ''
})
isDirectoryMock.mockImplementation(() => true)
isActionRepoMock.mockImplementation(() => true)
bundleFilesintoDirectoryMock.mockImplementation(() => {
return '/tmp/test'
getConsolidatedDirectoryMock.mockImplementation(() => {
return { consolidatedDirectory: '/tmp/test', needToCleanUpDir: false } // TODO: I don't understand why I have to name the variables here but not in the implementation code
})
createTempDirMock.mockImplementation(() => '/tmp/test')
dist/index.js
Generated Vendored
+35 -29
View File
@@ -74693,7 +74693,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.bundleFilesintoDirectory = exports.readFileContents = exports.isActionRepo = exports.isDirectory = exports.createArchives = exports.removeDir = exports.createTempDir = void 0;
exports.readFileContents = exports.isActionRepo = exports.isDirectory = exports.createArchives = exports.getConsolidatedDirectory = exports.removeDir = exports.createTempDir = void 0;
const fs = __importStar(__nccwpck_require__(57147));
const fs_extra_1 = __importDefault(__nccwpck_require__(5630));
const path = __importStar(__nccwpck_require__(71017));
@@ -74716,6 +74716,24 @@ function removeDir(dir) {
}
}
exports.removeDir = removeDir;
// TODO: rename this function, it is not state-preserving, so it shouldn't just be called "get'"
function getConsolidatedDirectory(filePathSpec) {
const paths = filePathSpec.split(' '); // TODO: handle files with spaces
// TODO: do check on paths to make sure they're valid and not reaching outside the space
let consolidatedPath = '';
let needToCleanUpDir = false;
if (paths.length === 1 && isDirectory(paths[0])) {
// If the path is a single directory, we can skip the bundling step
consolidatedPath = paths[0];
}
else {
// Otherwise, we need to bundle the files & folders into a temporary directory
consolidatedPath = bundleFilesintoDirectory(paths);
needToCleanUpDir = true;
}
return { consolidatedPath, needToCleanUpDir };
}
exports.getConsolidatedDirectory = getConsolidatedDirectory;
// Creates both a tar.gz and zip archive of the given directory and returns the paths to both archives (stored in the provided target directory)
// as well as the size/sha256 hash of each file.
async function createArchives(distPath, archiveTargetPath = createTempDir()) {
@@ -74734,7 +74752,7 @@ async function createArchives(distPath, archiveTargetPath = createTempDir()) {
resolve(fileMetadata(zipPath));
});
archive.pipe(output);
archive.directory(distPath, false);
archive.directory(distPath, false); // TODO: make sure this doesn't include dirs that start with ., same with below
archive.finalize();
});
const createTarPromise = new Promise((resolve, reject) => {
@@ -74773,23 +74791,23 @@ function readFileContents(filePath) {
return fs.readFileSync(filePath);
}
exports.readFileContents = readFileContents;
function bundleFilesintoDirectory(files, targetDir = createTempDir()) {
for (const file of files) {
if (!fs.existsSync(file)) {
throw new Error(`File ${file} does not exist`);
function bundleFilesintoDirectory(filePaths) {
const targetDir = createTempDir();
for (const filePath of filePaths) {
if (!fs.existsSync(filePath)) {
throw new Error(`filePath ${filePath} does not exist`);
}
if (isDirectory(file)) {
const targetFolder = path.join(targetDir, path.basename(file));
fs_extra_1.default.copySync(file, targetFolder);
if (isDirectory(filePath)) {
const targetFolder = path.join(targetDir, path.basename(filePath)); // TODO: basename is probably not what we actually want here. Or is it? Maybe conflicts between dir1/dir2 and dir1/dir3/dir2 are just user error or ??
fs_extra_1.default.copySync(filePath, targetFolder); // TODO: ignore files preceded by .
}
else {
const targetFile = path.join(targetDir, path.basename(file));
fs.copyFileSync(file, targetFile);
const targetFile = path.join(targetDir, path.basename(filePath));
fs.copyFileSync(filePath, targetFile);
}
}
return targetDir;
}
exports.bundleFilesintoDirectory = bundleFilesintoDirectory;
// Converts a file path to a filemetadata object by querying the fs for relevant metadata.
async function fileMetadata(filePath) {
const stats = fs.statSync(filePath);
@@ -74987,7 +75005,6 @@ Object.defineProperty(exports, "__esModule", ({ value: true }));
const main_1 = __nccwpck_require__(70399);
const minimist_1 = __importDefault(__nccwpck_require__(35871));
const path = (0, minimist_1.default)(process.argv.slice(2)).path || '.';
console.log(path);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
(0, main_1.run)(path);
@@ -75061,29 +75078,18 @@ async function run(pathInput) {
return;
}
const token = process.env.TOKEN;
// Gather & validate user input
// Paths to be included in the OCI image
// const paths: string[] = core.getInput('path').split(' ')
const paths = pathInput.split(' ');
let path = '';
if (paths.length === 1 && fsHelper.isDirectory(paths[0])) {
// If the path is a single directory, we can skip the bundling step
path = paths[0];
const { consolidatedPath, needToCleanUpDir } = fsHelper.getConsolidatedDirectory(pathInput);
if (needToCleanUpDir) {
tmpDirs.push(consolidatedPath);
}
else {
// Otherwise, we need to bundle the files & folders into a temporary directory
const bundleDir = fsHelper.createTempDir();
tmpDirs.push(bundleDir);
path = fsHelper.bundleFilesintoDirectory(paths, bundleDir);
}
if (!fsHelper.isActionRepo(path)) {
if (!fsHelper.isActionRepo(consolidatedPath)) {
core.setFailed('action.y(a)ml not found. Action packages can be created only for action repositories.');
return;
}
// Create a temporary directory to store the archives
const archiveDir = fsHelper.createTempDir();
tmpDirs.push(archiveDir);
const archives = await fsHelper.createArchives(path, archiveDir);
const archives = await fsHelper.createArchives(consolidatedPath, archiveDir);
const manifest = ociContainer.createActionPackageManifest(archives.tarFile, archives.zipFile, repository, targetVersion.raw, new Date());
// Generate SHA-256 hash of the manifest
const manifestSHA = crypto_1.default.createHash('sha256');
src/fs-helper.ts
+32 -13
View File
@@ -29,6 +29,27 @@ export interface FileMetadata {
sha256: string
}
// TODO: rename this function, it is not state-preserving, so it shouldn't just be called "get'"
export function getConsolidatedDirectory(filePathSpec: string): {
consolidatedPath: string
needToCleanUpDir: boolean
} {
const paths: string[] = filePathSpec.split(' ') // TODO: handle files with spaces
// TODO: do check on paths to make sure they're valid and not reaching outside the space
let consolidatedPath = ''
let needToCleanUpDir = false
if (paths.length === 1 && isDirectory(paths[0])) {
// If the path is a single directory, we can skip the bundling step
consolidatedPath = paths[0]
} else {
// Otherwise, we need to bundle the files & folders into a temporary directory
consolidatedPath = bundleFilesintoDirectory(paths)
needToCleanUpDir = true
}
return { consolidatedPath, needToCleanUpDir }
}
// Creates both a tar.gz and zip archive of the given directory and returns the paths to both archives (stored in the provided target directory)
// as well as the size/sha256 hash of each file.
export async function createArchives(
@@ -55,7 +76,7 @@ export async function createArchives(
})
archive.pipe(output)
archive.directory(distPath, false)
archive.directory(distPath, false) // TODO: make sure this doesn't include dirs that start with ., same with below
archive.finalize()
})
@@ -102,21 +123,19 @@ export function readFileContents(filePath: string): Buffer {
return fs.readFileSync(filePath)
}
export function bundleFilesintoDirectory(
files: string[],
targetDir: string = createTempDir()
): string {
for (const file of files) {
if (!fs.existsSync(file)) {
throw new Error(`File ${file} does not exist`)
function bundleFilesintoDirectory(filePaths: string[]): string {
const targetDir: string = createTempDir()
for (const filePath of filePaths) {
if (!fs.existsSync(filePath)) {
throw new Error(`filePath ${filePath} does not exist`)
}
if (isDirectory(file)) {
const targetFolder = path.join(targetDir, path.basename(file))
fsExtra.copySync(file, targetFolder)
if (isDirectory(filePath)) {
const targetFolder = path.join(targetDir, path.basename(filePath)) // TODO: basename is probably not what we actually want here. Or is it? Maybe conflicts between dir1/dir2 and dir1/dir3/dir2 are just user error or ??
fsExtra.copySync(filePath, targetFolder) // TODO: ignore files preceded by .
} else {
const targetFile = path.join(targetDir, path.basename(file))
fs.copyFileSync(file, targetFile)
const targetFile = path.join(targetDir, path.basename(filePath))
fs.copyFileSync(filePath, targetFile)
}
}
src/main.ts
+6 -16
View File
@@ -41,23 +41,13 @@ export async function run(pathInput: string): Promise<void> {
const token: string = process.env.TOKEN!
// Gather & validate user input
// Paths to be included in the OCI image
// const paths: string[] = core.getInput('path').split(' ')
const paths: string[] = pathInput.split(' ')
let path = ''
if (paths.length === 1 && fsHelper.isDirectory(paths[0])) {
// If the path is a single directory, we can skip the bundling step
path = paths[0]
} else {
// Otherwise, we need to bundle the files & folders into a temporary directory
const bundleDir = fsHelper.createTempDir()
tmpDirs.push(bundleDir)
path = fsHelper.bundleFilesintoDirectory(paths, bundleDir)
const { consolidatedPath, needToCleanUpDir } =
fsHelper.getConsolidatedDirectory(pathInput)
if (needToCleanUpDir) {
tmpDirs.push(consolidatedPath)
}
if (!fsHelper.isActionRepo(path)) {
if (!fsHelper.isActionRepo(consolidatedPath)) {
core.setFailed(
'action.y(a)ml not found. Action packages can be created only for action repositories.'
)
@@ -68,7 +58,7 @@ export async function run(pathInput: string): Promise<void> {
const archiveDir = fsHelper.createTempDir()
tmpDirs.push(archiveDir)
const archives = await fsHelper.createArchives(path, archiveDir)
const archives = await fsHelper.createArchives(consolidatedPath, archiveDir)
const manifest = ociContainer.createActionPackageManifest(
archives.tarFile,