Skip to content

fix: layer publish support code is http url,model remove #144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rsonghuster merged 1 commit into from
Mar 9, 2026
Merged

fix: layer publish support code is http url,model remove #144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions __tests__/ut/commands/invoke_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ import logger from '../../../src/logger';
import { IInputs } from '../../../src/interface';
import fs from 'fs';

// Mock @serverless-devs/downloads module to prevent import errors
jest.mock('@serverless-devs/downloads', () => ({
__esModule: true,
default: jest.fn(),
}));

// Mock dependencies
jest.mock('../../../src/resources/fc', () => {
return {
Expand Down
6 changes: 6 additions & 0 deletions __tests__/ut/utils/utils_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ import log from '../../../src/logger';
import { execSync } from 'child_process';
log._set(console);

// Mock @serverless-devs/downloads module to prevent import errors
jest.mock('@serverless-devs/downloads', () => ({
__esModule: true,
default: jest.fn(),
}));

describe('isAuto', () => {
test('should return true if config is string "AUTO" or "auto"', () => {
expect(isAuto('AUTO')).toBe(true);
Expand Down
6 changes: 3 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"dependencies": {
"@alicloud/devs20230714": "^2.5.0",
"@alicloud/fc2": "^2.6.6",
"@alicloud/fc20230330": "4.6.7",
"@alicloud/fc20230330": "4.6.8",
"@alicloud/pop-core": "^1.8.0",
"@serverless-cd/srm-aliyun-oss": "^0.0.1-beta.8",
"@serverless-cd/srm-aliyun-pop-core": "^0.0.8-beta.1",
Expand All @@ -35,7 +35,7 @@
"@serverless-devs/load-component": "^0.0.9",
"@serverless-devs/utils": "^0.0.17",
"@serverless-devs/zip": "^0.0.3-beta.8",
"ajv": "^8.17.1",
"ajv": "^8.18.0",
"ali-oss": "6.18.1",
"aliyun-sdk": "^1.12.10",
"chalk": "^4.1.0",
Expand All @@ -60,7 +60,7 @@
"@serverless-devs/component-interface": "^0.0.6",
"@serverless-devs/logger": "^0.0.5",
"@types/jest": "^29.5.14",
"@types/lodash": "^4.17.23",
"@types/lodash": "^4.17.24",
"@types/node": "^20.12.11",
"@vercel/ncc": "^0.38.4",
"f2elint": "^2.2.1",
Expand Down
72 changes: 22 additions & 50 deletions src/subCommands/deploy/impl/function.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import _ from 'lodash';
import { diffConvertYaml } from '@serverless-devs/diff';
import inquirer from 'inquirer';
import fs from 'fs';
import os from 'os';
import assert from 'assert';
import path from 'path';
import { yellow } from 'chalk';
Expand All @@ -19,10 +18,15 @@ import FC, { GetApiType } from '../../../resources/fc';
import VPC_NAS from '../../../resources/vpc-nas';
import Base from './base';
import { ICredentials } from '@serverless-devs/component-interface';
import { calculateCRC64, getFileSize, parseAutoConfig, checkFcDir } from '../../../utils';
import {
calculateCRC64,
getFileSize,
parseAutoConfig,
checkFcDir,
_downloadFromUrl,
} from '../../../utils';
import OSS from '../../../resources/oss';
import { setNodeModulesBinPermissions } from '../../../resources/fc/impl/utils';
import downloads from '@serverless-devs/downloads';

type IType = 'code' | 'config' | boolean;
interface IOpts {
Expand Down Expand Up @@ -280,11 +284,11 @@ export default class Service extends Base {
}

let zipPath: string;
let downloadedTempDir = '';
let downloadedTempFile = '';
// 处理不同类型的 codeUri
if (codeUri.startsWith('http://') || codeUri.startsWith('https://')) {
zipPath = await this._downloadFromUrl(codeUri);
downloadedTempDir = path.dirname(zipPath);
zipPath = await _downloadFromUrl(codeUri);
downloadedTempFile = zipPath;
} else {
zipPath = path.isAbsolute(codeUri) ? codeUri : path.join(this.inputs.baseDir, codeUri);
}
Expand Down Expand Up @@ -321,6 +325,14 @@ export default class Service extends Base {
logger.debug(
yellow(`skip uploadCode because code is no changed, codeChecksum=${crc64Value}`),
);
if (downloadedTempFile) {
try {
logger.debug(`Removing temp download dir: ${downloadedTempFile}`);
fs.rmSync(downloadedTempFile, { recursive: true, force: true });
} catch (ex) {
logger.debug(`Unable to remove temp download dir: ${downloadedTempFile}`);
}
}
return false;
} else {
logger.debug(`\x1b[33mcodeChecksum from ${this.codeChecksum} to ${crc64Value}\x1b[0m`);
Expand All @@ -338,58 +350,18 @@ export default class Service extends Base {
}
}

if (downloadedTempDir) {
if (downloadedTempFile) {
try {
logger.debug(`Removing temp download dir: ${downloadedTempDir}`);
fs.rmSync(downloadedTempDir, { recursive: true, force: true });
logger.debug(`Removing temp download dir: ${downloadedTempFile}`);
fs.rmSync(downloadedTempFile, { recursive: true, force: true });
} catch (ex) {
logger.debug(`Unable to remove temp download dir: ${downloadedTempDir}`);
logger.debug(`Unable to remove temp download dir: ${downloadedTempFile}`);
}
}

return true;
}

/**
* 从URL下载文件到本地临时目录
*/
private async _downloadFromUrl(url: string): Promise<string> {
logger.info(`Downloading code from URL: ${url}`);

// 创建临时目录
const tempDir = path.join(os.tmpdir(), 'fc_code_download');
let downloadPath: string;

try {
// 从URL获取文件名
const urlPath = new URL(url).pathname;
const parsedPathName = path.parse(urlPath).name;
const filename = path.basename(urlPath) || `downloaded_code_${Date.now()}`;
downloadPath = path.join(tempDir, filename);

await downloads(url, {
dest: tempDir,
filename: parsedPathName,
extract: false,
});

logger.debug(`Downloaded file to: ${downloadPath}`);

// 返回下载文件路径，由主流程决定是否需要压缩
return downloadPath;
} catch (error) {
// 如果下载失败，清理临时目录
try {
fs.rmSync(tempDir, { recursive: true, force: true });
logger.debug(`Cleaned up temporary directory after error: ${tempDir}`);
} catch (cleanupError) {
logger.debug(`Failed to clean up temporary directory: ${cleanupError.message}`);
}

throw new Error(`Failed to download code from URL: ${error.message}`);
}
}

/**
* 生成 auto 资源，非 FC 资源，主要指 vpc、nas、log、role（oss mount 挂载点才有）
*/
Expand Down
32 changes: 31 additions & 1 deletion src/subCommands/layer/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
calculateCRC64,
getFileSize,
getUserAgent,
_downloadFromUrl,
} from '../../utils';
import chalk from 'chalk';

Expand All @@ -28,6 +29,7 @@ export default class Layer {
layerName: string,
compatibleRuntimeList: string[],
description: string,
downloadedTempFile = '',
): Promise<any> {
let zipPath = toZipDir;
let generateZipFilePath = '';
Expand All @@ -53,6 +55,14 @@ export default class Layer {
`Skip uploadCode because code is no changed, codeChecksum=${crc64Value}; Laster layerArn=${latestLayer.layerVersionArn}`,
),
);
if (downloadedTempFile) {
try {
logger.debug(`Removing temp download dir: ${downloadedTempFile}`);
fs.rmSync(downloadedTempFile, { recursive: true, force: true });
} catch (ex) {
logger.debug(`Unable to remove temp download dir: ${downloadedTempFile}`);
}
}
return latestLayer;
}
}
Expand All @@ -76,6 +86,16 @@ export default class Layer {
logger.debug(`Unable to remove zip file: ${zipPath}`);
}
}

if (downloadedTempFile) {
try {
logger.debug(`Removing temp download file: ${downloadedTempFile}`);
fs.rmSync(downloadedTempFile, { recursive: true, force: true });
} catch (ex) {
logger.debug(`Unable to remove temp download file: ${downloadedTempFile}`);
}
}

console.log(JSON.stringify(result));
return result;
}
Expand Down Expand Up @@ -220,7 +240,16 @@ export default class Layer {
);
}

const toZipDir: string = path.isAbsolute(codeUri) ? codeUri : path.join(this.baseDir, codeUri);
let toZipDir: string;
let downloadedTempFile = '';
// 处理不同类型的 codeUri
if (codeUri.startsWith('http://') || codeUri.startsWith('https://')) {
toZipDir = await _downloadFromUrl(codeUri);
downloadedTempFile = toZipDir;
} else {
toZipDir = path.isAbsolute(codeUri) ? codeUri : path.join(this.baseDir, codeUri);
}

const compatibleRuntimeList = compatibleRuntime.split(',');
return Layer.safe_publish_layer(
this.fcSdk,
Expand All @@ -229,6 +258,7 @@ export default class Layer {
layerName,
compatibleRuntimeList,
this.opts.description || '',
downloadedTempFile,
);
}

Expand Down
2 changes: 1 addition & 1 deletion src/subCommands/model/model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export class ModelService {
);
}

const nasPath = nasMountPoints[0]?.serverAddr?.split(':')[1];
const nasPath = nasMountPoints && nasMountPoints[0]?.serverAddr?.split(':')[1];

if (storage === 'nas' && nasMountPoints[0] && nasPath?.trim() === '/') {
throw new Error(
Expand Down
52 changes: 50 additions & 2 deletions src/utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import Table from 'tty-table';
import * as crc64 from 'crc64-ecma182.js';
import { promisify } from 'util';
import * as fs from 'fs';
import os from 'os';
import logger from '../logger';
import { execSync } from 'child_process';
import axios from 'axios';
import { FC_API_ERROR_CODE, isInvalidArgument } from '../resources/fc/error-code';
import path from 'path';
import downloads from '@serverless-devs/downloads';

export { default as verify } from './verify';
export { default as runCommand } from './run-command';
Expand Down Expand Up @@ -261,8 +264,8 @@ export function getUserAgent(userAgent: string, command: string) {
/**
* 验证并规范化路径
*/
export function checkFcDir(path: string, paramName = 'path'): string {
const normalizedPath = path.trim();
export function checkFcDir(inputPath: string, paramName = 'path'): string {
const normalizedPath = inputPath.trim();

if (!normalizedPath.startsWith('/')) {
throw new Error(`${paramName} does not start with '/'`);
Expand Down Expand Up @@ -308,3 +311,48 @@ export function checkFcDir(path: string, paramName = 'path'): string {

return normalizedPath;
}

/**
* 从URL下载文件到本地临时目录
*/
export async function _downloadFromUrl(url: string): Promise<string> {
// 创建临时目录
const tempDir = path.join(os.tmpdir(), 'fc_code_download');
let downloadPath: string;

try {
// 确保临时目录存在
if (!fs.existsSync(tempDir)) {
fs.mkdirSync(tempDir, { recursive: true });
}

// 从URL获取文件名
const urlPath = new URL(url).pathname;
const parsedPathName = path.parse(urlPath).name;
const filename = path.basename(urlPath) || `downloaded_code_${Date.now()}`;
downloadPath = path.join(tempDir, filename);

await downloads(url, {
dest: tempDir,
filename: parsedPathName,
extract: false,
});

logger.debug(`Downloaded file to: ${downloadPath}`);

// 返回下载文件路径，由主流程决定是否需要压缩
return downloadPath;
Comment on lines +329 to +344
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Mismatched filename causes returned path to reference a non-existent file.

The function computes two different filenames:

  • filename = path.basename(urlPath) → e.g., "code.zip"
  • parsedPathName = path.parse(urlPath).name → e.g., "code" (without extension)

The downloadPath is computed using filename, but downloads() saves the file using parsedPathName. This causes the returned path to point to a file that doesn't exist.

For example, with URL https://example.com/code.zip:

  • Returns: /tmp/fc_code_download/code.zip
  • Actual file: /tmp/fc_code_download/code
🐛 Proposed fix 
     // 从URL获取文件名
     const urlPath = new URL(url).pathname;
-    const parsedPathName = path.parse(urlPath).name;
     const filename = path.basename(urlPath) || `downloaded_code_${Date.now()}`;
     downloadPath = path.join(tempDir, filename);
 
     await downloads(url, {
       dest: tempDir,
-      filename: parsedPathName,
+      filename: filename,
       extract: false,
     });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 从URL获取文件名
const urlPath = new URL(url).pathname;
const parsedPathName = path.parse(urlPath).name;
const filename = path.basename(urlPath) || `downloaded_code_${Date.now()}`;
downloadPath = path.join(tempDir, filename);
await downloads(url, {
dest: tempDir,
filename: parsedPathName,
extract: false,
});
logger.debug(`Downloaded file to: ${downloadPath}`);
// 返回下载文件路径，由主流程决定是否需要压缩
return downloadPath;
// 从URL获取文件名
const urlPath = new URL(url).pathname;
const filename = path.basename(urlPath) || `downloaded_code_${Date.now()}`;
downloadPath = path.join(tempDir, filename);
await downloads(url, {
dest: tempDir,
filename: filename,
extract: false,
});
logger.debug(`Downloaded file to: ${downloadPath}`);
// 返回下载文件路径，由主流程决定是否需要压缩
return downloadPath;
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/utils/index.ts` around lines 329 - 344, The returned downloadPath points
to a different filename than the one passed to downloads(): you compute both
filename (path.basename(urlPath)) and parsedPathName (path.parse(urlPath).name)
but use filename for downloadPath and parsedPathName for downloads(); fix by
choosing a single canonical name and using it in both places — e.g., compute a
single variable (use either basename or parsedName+path.extname(urlPath)) and
replace both uses of filename and parsedPathName so downloadPath and the
downloads(...) filename option reference the exact same string (ensure tempDir
and URL parsing logic remain unchanged).

} catch (error) {
// 如果下载失败，清理临时目录
try {
if (fs.existsSync(tempDir)) {
fs.rmSync(tempDir, { recursive: true, force: true });
logger.debug(`Cleaned up temporary directory after error: ${tempDir}`);
}
} catch (cleanupError) {
logger.debug(`Failed to clean up temporary directory: ${cleanupError.message}`);
}

throw new Error(`Failed to download code from URL: ${error.message}`);
}
Comment on lines +345 to +357
Copy link

@coderabbitai coderabbitai bot Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Error cleanup removes entire temp directory, potentially affecting concurrent operations.

On error, the function removes the entire fc_code_download directory. Since this is a shared temp directory path (os.tmpdir()/fc_code_download), concurrent downloads from different processes or calls could have their files deleted unexpectedly.

Consider cleaning up only the specific downloaded file rather than the entire directory:

🛡️ Proposed fix 
   } catch (error) {
-    // 如果下载失败，清理临时目录
+    // 如果下载失败，清理下载的文件
     try {
-      if (fs.existsSync(tempDir)) {
-        fs.rmSync(tempDir, { recursive: true, force: true });
-        logger.debug(`Cleaned up temporary directory after error: ${tempDir}`);
+      if (downloadPath && fs.existsSync(downloadPath)) {
+        fs.rmSync(downloadPath, { force: true });
+        logger.debug(`Cleaned up downloaded file after error: ${downloadPath}`);
       }
     } catch (cleanupError) {
-      logger.debug(`Failed to clean up temporary directory: ${cleanupError.message}`);
+      logger.debug(`Failed to clean up downloaded file: ${cleanupError.message}`);
     }
 
     throw new Error(`Failed to download code from URL: ${error.message}`);
   }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/utils/index.ts` around lines 345 - 357, The cleanup currently removes the
entire shared tempDir (tempDir) which can disrupt concurrent operations;
instead, in the catch block only remove the specific downloaded file/temporary
artifact (e.g., the variable that holds the downloaded file path such as
tempFilePath or downloadedArchivePath) by checking fs.existsSync(tempFilePath)
and calling fs.unlinkSync(tempFilePath) (or async equivalent), leaving tempDir
intact; update the logger.debug messages to reference the specific file and
preserve the existing cleanupError logging behavior (logger.debug with
cleanupError.message) and then rethrow the original error as before.

}
Loading