Skip to content

fix: resolve false-positive configuration changes for Layers and Logg… #62

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
jfeddern wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: resolve false-positive configuration changes for Layers and Logg… #62

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
261 changes: 260 additions & 1 deletion __tests__/has_configuration_changed.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const core = require('@actions/core');
const { isEmptyValue, cleanNullKeys, hasConfigurationChanged } = require('../index');
const { isEmptyValue, cleanNullKeys, hasConfigurationChanged, normalizeConfigValue } = require('../index');

jest.mock('@actions/core');

Expand Down Expand Up @@ -445,4 +445,263 @@ describe('Has Configuration Changed Tests', () => {
falseValue: false
});
});

// Tests for normalizeConfigValue function
describe('normalizeConfigValue', () => {
test('should extract ARNs from Layer objects', () => {
const layerObjects = [
{
Arn: 'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
CodeSize: 12345,
SigningProfileVersionArn: 'arn:aws:signer:...',
SigningJobArn: 'arn:aws:signer:...'
},
{
Arn: 'arn:aws:lambda:us-east-1:123456789012:layer:layer2:2',
CodeSize: 67890
}
];

const result = normalizeConfigValue('Layers', layerObjects);
expect(result).toEqual([
'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
'arn:aws:lambda:us-east-1:123456789012:layer:layer2:2'
]);
});

test('should return Layer ARN strings unchanged', () => {
const layerStrings = [
'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
'arn:aws:lambda:us-east-1:123456789012:layer:layer2:2'
];

const result = normalizeConfigValue('Layers', layerStrings);
expect(result).toEqual(layerStrings);
});

test('should handle empty Layers array', () => {
const result = normalizeConfigValue('Layers', []);
expect(result).toEqual([]);
});

test('should remove LogGroup from LoggingConfig', () => {
const loggingConfig = {
LogFormat: 'JSON',
LogGroup: '/aws/lambda/my-function',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'INFO'
};

const result = normalizeConfigValue('LoggingConfig', loggingConfig);
expect(result).toEqual({
LogFormat: 'JSON',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'INFO'
});
expect(result.LogGroup).toBeUndefined();
});

test('should return LoggingConfig without LogGroup unchanged', () => {
const loggingConfig = {
LogFormat: 'JSON',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'INFO'
};

const result = normalizeConfigValue('LoggingConfig', loggingConfig);
expect(result).toEqual(loggingConfig);
});

test('should return non-special config values unchanged', () => {
expect(normalizeConfigValue('Runtime', 'nodejs20.x')).toBe('nodejs20.x');
expect(normalizeConfigValue('MemorySize', 256)).toBe(256);
expect(normalizeConfigValue('Timeout', 30)).toBe(30);

const envConfig = { Variables: { ENV: 'production' } };
expect(normalizeConfigValue('Environment', envConfig)).toEqual(envConfig);
});
});

// Integration tests for Layers comparison
describe('Layers comparison with normalization', () => {
test('should return false when Layers are identical (objects vs strings)', async () => {
const current = {
Runtime: 'nodejs18.x',
Layers: [
{
Arn: 'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
CodeSize: 12345,
SigningProfileVersionArn: 'arn:aws:signer:...',
SigningJobArn: 'arn:aws:signer:...'
}
]
};
const updated = {
Runtime: 'nodejs18.x',
Layers: ['arn:aws:lambda:us-east-1:123456789012:layer:layer1:1']
};

const result = await hasConfigurationChanged(current, updated);
expect(result).toBe(false);
expect(core.info).not.toHaveBeenCalledWith(expect.stringContaining('Configuration difference detected in Layers'));
});

test('should return true when Layers actually differ', async () => {
const current = {
Runtime: 'nodejs18.x',
Layers: [
{
Arn: 'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
CodeSize: 12345
}
]
};
const updated = {
Runtime: 'nodejs18.x',
Layers: [
'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
'arn:aws:lambda:us-east-1:123456789012:layer:layer2:1'
]
};

const result = await hasConfigurationChanged(current, updated);
expect(result).toBe(true);
expect(core.info).toHaveBeenCalledWith(expect.stringContaining('Configuration difference detected in Layers'));
});

test('should handle multiple identical Layers', async () => {
const current = {
Layers: [
{
Arn: 'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
CodeSize: 12345
},
{
Arn: 'arn:aws:lambda:us-east-1:123456789012:layer:layer2:2',
CodeSize: 67890
}
]
};
const updated = {
Layers: [
'arn:aws:lambda:us-east-1:123456789012:layer:layer1:1',
'arn:aws:lambda:us-east-1:123456789012:layer:layer2:2'
]
};

const result = await hasConfigurationChanged(current, updated);
expect(result).toBe(false);
});
});

// Integration tests for LoggingConfig comparison
describe('LoggingConfig comparison with normalization', () => {
test('should return false when LoggingConfig is identical except for LogGroup', async () => {
const current = {
Runtime: 'nodejs18.x',
LoggingConfig: {
LogFormat: 'JSON',
LogGroup: '/aws/lambda/my-function',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'INFO'
}
};
const updated = {
Runtime: 'nodejs18.x',
LoggingConfig: {
LogFormat: 'JSON',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'INFO'
}
};

const result = await hasConfigurationChanged(current, updated);
expect(result).toBe(false);
expect(core.info).not.toHaveBeenCalledWith(expect.stringContaining('Configuration difference detected in LoggingConfig'));
});

test('should return true when LoggingConfig actually differs', async () => {
const current = {
Runtime: 'nodejs18.x',
LoggingConfig: {
LogFormat: 'JSON',
LogGroup: '/aws/lambda/my-function',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'INFO'
}
};
const updated = {
Runtime: 'nodejs18.x',
LoggingConfig: {
LogFormat: 'Text',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'INFO'
}
};

const result = await hasConfigurationChanged(current, updated);
expect(result).toBe(true);
expect(core.info).toHaveBeenCalledWith(expect.stringContaining('Configuration difference detected in LoggingConfig'));
});

test('should handle LoggingConfig with only LogFormat', async () => {
const current = {
LoggingConfig: {
LogFormat: 'JSON',
LogGroup: '/aws/lambda/my-function'
}
};
const updated = {
LoggingConfig: {
LogFormat: 'JSON'
}
};

const result = await hasConfigurationChanged(current, updated);
expect(result).toBe(false);
});
});

// Combined test for real-world scenario
describe('Real-world scenario: Issue #50', () => {
test('should not report false positives for Layers and LoggingConfig', async () => {
// This simulates the exact scenario from GitHub issue #50
const currentFromAWS = {
Runtime: 'nodejs20.x',
MemorySize: 256,
Timeout: 30,
Layers: [
{
Arn: 'arn:aws:lambda:us-east-1:123456789012:layer:my-layer:1',
CodeSize: 12345,
SigningProfileVersionArn: 'arn:aws:signer:us-east-1:123456789012:/signing-profiles/...',
SigningJobArn: 'arn:aws:signer:us-east-1:123456789012:...'
}
],
LoggingConfig: {
LogFormat: 'JSON',
LogGroup: '/aws/lambda/my-function',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'WARN'
}
};

const userProvidedConfig = {
Runtime: 'nodejs20.x',
MemorySize: 256,
Timeout: 30,
Layers: ['arn:aws:lambda:us-east-1:123456789012:layer:my-layer:1'],
LoggingConfig: {
LogFormat: 'JSON',
ApplicationLogLevel: 'INFO',
SystemLogLevel: 'WARN'
}
};

const result = await hasConfigurationChanged(currentFromAWS, userProvidedConfig);
expect(result).toBe(false);
expect(core.info).not.toHaveBeenCalledWith(expect.stringContaining('Configuration difference detected in Layers'));
expect(core.info).not.toHaveBeenCalledWith(expect.stringContaining('Configuration difference detected in LoggingConfig'));
});
});
});
36 changes: 32 additions & 4 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,13 +682,17 @@ async function hasConfigurationChanged(currentConfig, updatedConfig) {
continue;
}

if (typeof value === 'object' && value !== null) {
if (!deepEqual(currentConfig[key] || {}, value)) {
// Normalize the current config value for comparison
const normalizedCurrentValue = normalizeConfigValue(key, currentConfig[key]);
const normalizedUpdatedValue = normalizeConfigValue(key, value);

if (typeof normalizedUpdatedValue === 'object' && normalizedUpdatedValue !== null) {
if (!deepEqual(normalizedCurrentValue || {}, normalizedUpdatedValue)) {
core.info(`Configuration difference detected in ${key}`);
hasChanged = true;
}
} else if (currentConfig[key] !== value) {
core.info(`Configuration difference detected in ${key}: ${currentConfig[key]} -> ${value}`);
} else if (normalizedCurrentValue !== normalizedUpdatedValue) {
core.info(`Configuration difference detected in ${key}: ${normalizedCurrentValue} -> ${normalizedUpdatedValue}`);
hasChanged = true;
}
}
Expand All @@ -697,6 +701,29 @@ async function hasConfigurationChanged(currentConfig, updatedConfig) {
return hasChanged;
}

function normalizeConfigValue(key, value) {
// Normalize Layers: AWS returns array of Layer objects, but UpdateFunctionConfiguration accepts array of ARN strings
if (key === 'Layers' && Array.isArray(value) && value.length > 0) {
// If the array contains objects (from GetFunctionConfiguration), extract just the ARNs
if (typeof value[0] === 'object' && value[0] !== null && 'Arn' in value[0]) {
return value.map(layer => layer.Arn);
}
// Otherwise it's already an array of strings (from user input)
return value;
}

// Normalize LoggingConfig: Remove read-only fields that AWS adds
if (key === 'LoggingConfig' && typeof value === 'object' && value !== null) {
const normalized = { ...value };
// LogGroup is a read-only field that AWS populates automatically
// Users cannot set it via UpdateFunctionConfiguration, so exclude it from comparison
delete normalized.LogGroup;
return normalized;
}

return value;
}

function isEmptyValue(value) {
if (value === null || value === undefined || value === '') {
return true;
Expand Down Expand Up @@ -1115,6 +1142,7 @@ module.exports = {
packageCodeArtifacts,
checkFunctionExists,
hasConfigurationChanged,
normalizeConfigValue,
waitForFunctionUpdated,
waitForFunctionActive,
isEmptyValue,
Expand Down