Skip to content

fix: restore package-type Image branching (regression in v1.1.1) #71

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

Open
JamBalaya56562 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: restore package-type Image branching (regression in v1.1.1) #71

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
113 changes: 109 additions & 4 deletions __tests__/container_image_support.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,38 @@ const core = require('@actions/core');
const originalValidations = require('../validations');

jest.mock('@actions/core');
jest.mock('@aws-sdk/client-lambda', () => {
const original = jest.requireActual('@aws-sdk/client-lambda');
return {
...original,
CreateFunctionCommand: jest.fn().mockImplementation((params) => ({
...params,
type: 'CreateFunctionCommand'
})),
UpdateFunctionCodeCommand: jest.fn().mockImplementation((params) => ({
...params,
type: 'UpdateFunctionCodeCommand'
})),
GetFunctionConfigurationCommand: jest.fn().mockImplementation((params) => ({
...params,
type: 'GetFunctionConfigurationCommand'
})),
LambdaClient: jest.fn().mockImplementation(() => ({
send: jest.fn()
})),
waitUntilFunctionUpdated: jest.fn().mockResolvedValue({})
};
});

describe('Container Image Support Tests', () => {
let originalEnv;

beforeEach(() => {
jest.clearAllMocks();
originalEnv = process.env;
process.env = { ...originalEnv };
process.env.GITHUB_SHA = 'abc123';

// Default mock implementations
core.getInput.mockImplementation((name) => {
const inputs = {
Expand All @@ -22,7 +44,7 @@ describe('Container Image Support Tests', () => {
};
return inputs[name] || '';
});

core.getBooleanInput.mockReturnValue(false);
});

Expand Down Expand Up @@ -94,12 +116,95 @@ describe('Container Image Support Tests', () => {
};
return inputs[name] || '';
});

const result = originalValidations.validateAllInputs();
expect(result.valid).toBe(true);
expect(result.packageType).toBe('Zip');
});
});

// Regression tests for issue #70: v1.1.1 broke Image package type by
// unconditionally calling packageCodeArtifacts and stripping packageType
// from createFunction / updateFunctionCode.
describe('Image package type wiring (regression for #70)', () => {
const { LambdaClient, CreateFunctionCommand, UpdateFunctionCodeCommand } = require('@aws-sdk/client-lambda');
const index = require('../index');

test('createFunction sends ImageUri and PackageType=Image, omits Runtime/Handler/Layers/ZipFile', async () => {
// Respond to both CreateFunctionCommand (the assertion target) and the
// GetFunctionConfigurationCommand polled by waitForFunctionActive.
const mockSend = jest.fn().mockImplementation((cmd) => {
if (cmd && cmd.type === 'GetFunctionConfigurationCommand') {
return Promise.resolve({ State: 'Active' });
}
return Promise.resolve({
FunctionArn: 'arn:aws:lambda:us-east-1:123456789012:function:test-function',
Version: '1'
});
});
LambdaClient.mockImplementation(() => ({ send: mockSend }));

const client = new LambdaClient();
const inputs = {
functionName: 'test-function',
packageType: 'Image',
imageUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/my-repo:latest',
region: 'us-east-1',
role: 'arn:aws:iam::123456789012:role/test-role',
runtime: 'nodejs20.x', // must be ignored for Image
handler: 'index.handler', // must be ignored for Image
layers: ['arn:aws:lambda:us-east-1:123:layer:l1:1'], // must be ignored for Image
parsedLayers: ['arn:aws:lambda:us-east-1:123:layer:l1:1'],
parsedEnvironment: {}
};

await index.createFunction(client, inputs, false);

const createCall = mockSend.mock.calls.find(
([c]) => c && c.type === 'CreateFunctionCommand'
);
expect(createCall).toBeDefined();
const sentInput = createCall[0];
expect(sentInput.PackageType).toBe('Image');
expect(sentInput.Code).toEqual({
ImageUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/my-repo:latest'
});
expect(sentInput.Code.ZipFile).toBeUndefined();
expect(sentInput.Runtime).toBeUndefined();
expect(sentInput.Handler).toBeUndefined();
expect(sentInput.Layers).toBeUndefined();
});

test('updateFunctionCode sends ImageUri instead of ZipFile when packageType=Image', async () => {
const mockSend = jest.fn().mockResolvedValue({
FunctionArn: 'arn:aws:lambda:us-east-1:123456789012:function:test-function',
Version: '2'
});
LambdaClient.mockImplementation(() => ({ send: mockSend }));

const client = new LambdaClient();
const params = {
functionName: 'test-function',
packageType: 'Image',
imageUri: '123456789012.dkr.ecr.us-east-1.amazonaws.com/my-repo:latest',
finalZipPath: null,
useS3Method: false,
architectures: 'x86_64',
publish: false,
dryRun: false,
region: 'us-east-1'
};

await index.updateFunctionCode(client, params);

expect(mockSend).toHaveBeenCalledTimes(1);
const sentInput = mockSend.mock.calls[0][0];
expect(sentInput.type).toBe('UpdateFunctionCodeCommand');
expect(sentInput.ImageUri).toBe('123456789012.dkr.ecr.us-east-1.amazonaws.com/my-repo:latest');
expect(sentInput.ZipFile).toBeUndefined();
expect(sentInput.S3Bucket).toBeUndefined();
expect(sentInput.S3Key).toBeUndefined();
});
});

});
91 changes: 62 additions & 29 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ async function run() {
}

const {
functionName, codeArtifactsDir,
functionName, packageType, codeArtifactsDir, imageUri,
ephemeralStorage, parsedMemorySize, timeout,
role, codeSigningConfigArn, kmsKeyArn, sourceKmsKeyArn,
vpcConfig, deadLetterConfig, tracingConfig,
Expand Down Expand Up @@ -69,13 +69,18 @@ async function run() {
}
}

// Creating zip file
core.info(`Packaging code artifacts from ${codeArtifactsDir}`);
let finalZipPath = await packageCodeArtifacts(codeArtifactsDir);
// Creating zip file (only for Zip package type)
let finalZipPath = null;
if (packageType === 'Zip') {
core.info(`Packaging code artifacts from ${codeArtifactsDir}`);
finalZipPath = await packageCodeArtifacts(codeArtifactsDir);
} else if (packageType === 'Image') {
core.info(`Using container image: ${imageUri}`);
}

// Create function
await createFunction(client, {
functionName, region, finalZipPath, dryRun, role,
functionName, packageType, region, finalZipPath, imageUri, dryRun, role,
s3Bucket, s3Key, sourceKmsKeyArn, runtime, handler,
functionDescription, parsedMemorySize, timeout,
publish, architectures, ephemeralStorage,
Expand All @@ -93,22 +98,30 @@ async function run() {
const configCommand = new GetFunctionConfigurationCommand({FunctionName: functionName});
let currentConfig = await client.send(configCommand);

// Check if package type is being changed (not supported by AWS)
if (currentConfig.PackageType && currentConfig.PackageType !== packageType) {
core.setFailed(`Cannot change package type of existing Lambda function from ${currentConfig.PackageType} to ${packageType}`);
return;
}

const configChanged = hasConfigurationChanged(currentConfig, {
...(role && { Role: role }),
...(handler && { Handler: handler }),
// Only include handler, runtime, and layers for Zip package type
...(packageType === 'Zip' && handler && { Handler: handler }),
...(functionDescription && { Description: functionDescription }),
...(parsedMemorySize && { MemorySize: parsedMemorySize }),
...(timeout && { Timeout: timeout }),
...(runtime && { Runtime: runtime }),
...(packageType === 'Zip' && runtime && { Runtime: runtime }),
...(kmsKeyArn && { KMSKeyArn: kmsKeyArn }),
...(ephemeralStorage && { EphemeralStorage: { Size: ephemeralStorage } }),
...(vpcConfig && { VpcConfig: parsedVpcConfig }),
Environment: { Variables: parsedEnvironment },
...(deadLetterConfig && { DeadLetterConfig: parsedDeadLetterConfig }),
...(tracingConfig && { TracingConfig: parsedTracingConfig }),
...(layers && { Layers: parsedLayers }),
...(packageType === 'Zip' && layers && { Layers: parsedLayers }),
...(fileSystemConfigs && { FileSystemConfigs: parsedFileSystemConfigs }),
...(imageConfig && { ImageConfig: parsedImageConfig }),
// Only include ImageConfig for Image package type
...(packageType === 'Image' && imageConfig && { ImageConfig: parsedImageConfig }),
...(snapStart && { SnapStart: parsedSnapStart }),
...(loggingConfig && { LoggingConfig: parsedLoggingConfig }),
...(durableConfig && { DurableConfig: parsedDurableConfig })
Expand All @@ -123,29 +136,30 @@ async function run() {
await updateFunctionConfiguration(client, {
functionName,
role,
handler,
// Only include handler, runtime, and layers for Zip package type
...(packageType === 'Zip' && { handler }),
functionDescription,
parsedMemorySize,
timeout,
runtime,
...(packageType === 'Zip' && { runtime }),
kmsKeyArn,
ephemeralStorage,
vpcConfig,
parsedEnvironment,
deadLetterConfig,
tracingConfig,
layers,
...(packageType === 'Zip' && { layers }),
fileSystemConfigs,
imageConfig,
...(packageType === 'Image' && { imageConfig }),
snapStart,
loggingConfig,
durableConfig,
parsedVpcConfig,
parsedDeadLetterConfig,
parsedTracingConfig,
parsedLayers,
...(packageType === 'Zip' && { parsedLayers }),
parsedFileSystemConfigs,
parsedImageConfig,
...(packageType === 'Image' && { parsedImageConfig }),
parsedSnapStart,
parsedLoggingConfig,
parsedDurableConfig
Expand All @@ -157,6 +171,8 @@ async function run() {
// Update Function Code
await updateFunctionCode(client, {
functionName,
packageType,
imageUri,
finalZipPath,
useS3Method,
s3Bucket,
Expand Down Expand Up @@ -302,7 +318,7 @@ async function checkFunctionExists(client, functionName) {
// Helper functions for creating Lambda function
async function createFunction(client, inputs, functionExists) {
const {
functionName, region, finalZipPath, dryRun, role, s3Bucket, s3Key,
functionName, packageType, region, finalZipPath, imageUri, dryRun, role, s3Bucket, s3Key,
sourceKmsKeyArn, runtime, handler, functionDescription, parsedMemorySize,
timeout, publish, architectures, ephemeralStorage, revisionId,
vpcConfig, parsedEnvironment, deadLetterConfig, tracingConfig,
Expand All @@ -327,11 +343,17 @@ async function createFunction(client, inputs, functionExists) {
}

try {
core.info('Creating Lambda function with deployment package');
core.info(`Creating Lambda function with ${packageType} package type`);

let codeParameter;

if (s3Bucket) {
if (packageType === 'Image') {
// For container images, use ImageUri
core.info(`Using container image: ${imageUri}`);
codeParameter = {
ImageUri: imageUri
};
} else if (s3Bucket) {
try {
await uploadToS3(finalZipPath, s3Bucket, s3Key, region);
core.info(`Successfully uploaded package to S3: s3://${s3Bucket}/${s3Key}`);
Expand Down Expand Up @@ -374,9 +396,11 @@ async function createFunction(client, inputs, functionExists) {
const input = {
FunctionName: functionName,
Code: codeParameter,
...(runtime && { Runtime: runtime }),
PackageType: packageType,
...(role && { Role: role }),
...(handler && { Handler: handler }),
// Only include Runtime, Handler, and Layers for Zip package type
...(packageType === 'Zip' && runtime && { Runtime: runtime }),
...(packageType === 'Zip' && handler && { Handler: handler }),
...(functionDescription && { Description: functionDescription }),
...(parsedMemorySize && { MemorySize: parsedMemorySize }),
...(timeout && { Timeout: timeout }),
Expand All @@ -388,9 +412,10 @@ async function createFunction(client, inputs, functionExists) {
Environment: { Variables: parsedEnvironment },
...(deadLetterConfig && { DeadLetterConfig: parsedDeadLetterConfig }),
...(tracingConfig && { TracingConfig: parsedTracingConfig }),
...(layers && { Layers: parsedLayers }),
...(packageType === 'Zip' && layers && { Layers: parsedLayers }),
...(fileSystemConfigs && { FileSystemConfigs: parsedFileSystemConfigs }),
...(imageConfig && { ImageConfig: parsedImageConfig }),
// Only include ImageConfig for Image package type
...(packageType === 'Image' && imageConfig && { ImageConfig: parsedImageConfig }),
...(snapStart && { SnapStart: parsedSnapStart }),
...(loggingConfig && { LoggingConfig: parsedLoggingConfig }),
...(tags && { Tags: parsedTags }),
Expand Down Expand Up @@ -579,25 +604,31 @@ async function waitForFunctionUpdated(client, functionName, waitForMinutes = 5)
// Helper function for updating Lambda function code
async function updateFunctionCode(client, params) {
const {
functionName, finalZipPath, useS3Method, s3Bucket, s3Key,
functionName, packageType, imageUri, finalZipPath, useS3Method, s3Bucket, s3Key,
codeArtifactsDir, architectures, publish, revisionId,
sourceKmsKeyArn, dryRun, region
} = params;

core.info(`Updating function code for ${functionName} with ${finalZipPath}`);
core.info(`Updating function code for ${functionName}`);

try {
const commonCodeParams = {
FunctionName: functionName,
...(architectures && { Architectures: Array.isArray(architectures) ? architectures : [architectures] }),
...(publish !== undefined && { Publish: publish }),
...(revisionId && { RevisionId: revisionId }),
...(sourceKmsKeyArn && { SourceKmsKeyArn: sourceKmsKeyArn })
...(revisionId && { RevisionId: revisionId })
};

let codeInput;

if (useS3Method) {
if (packageType === 'Image') {
// For container images, use ImageUri
core.info(`Using container image: ${imageUri}`);
codeInput = {
...commonCodeParams,
ImageUri: imageUri
};
} else if (useS3Method) {
core.info(`Using S3 deployment method with bucket: ${s3Bucket}, key: ${s3Key}`);

await uploadToS3(finalZipPath, s3Bucket, s3Key, region);
Expand All @@ -606,7 +637,8 @@ async function updateFunctionCode(client, params) {
codeInput = {
...commonCodeParams,
S3Bucket: s3Bucket,
S3Key: s3Key
S3Key: s3Key,
...(sourceKmsKeyArn && { SourceKmsKeyArn: sourceKmsKeyArn })
};
} else {
let zipFileContent;
Expand All @@ -631,7 +663,8 @@ async function updateFunctionCode(client, params) {

codeInput = {
...commonCodeParams,
ZipFile: zipFileContent
ZipFile: zipFileContent,
...(sourceKmsKeyArn && { SourceKmsKeyArn: sourceKmsKeyArn })
};

core.info(`Original buffer length: ${zipFileContent.length} bytes`);
Expand Down
Loading