Skip to content

Commit 3963f7f

Browse files
egyptianbmanegyptianbman
authored
fix: allow empty values in proxyConfiguration.properties (#168)
* fix: allow empty values in proxyConfiguration.properties Fixes: #163 App Mesh requires the properties array contain a name and a value for each key. The `cleanNullKeys` removes the values that are empty strings. This commit loops through the properties array if the proxyConfiguration.type is APPMESH and restores any deleted values. * Better variable name * Changes requested in code review
1 parent 879db76 commit 3963f7f

File tree

4 files changed

+175
-7
lines changed

4 files changed

+175
-7
lines changed

‎dist/index.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,20 @@ function removeIgnoredAttributes(taskDef) {
294294
return taskDef;
295295
}
296296

297+
function maintainAppMeshConfiguration(taskDef) {
298+
if ('proxyConfiguration' in taskDef && taskDef.proxyConfiguration.type == 'APPMESH' && taskDef.proxyConfiguration.properties.length > 0) {
299+
taskDef.proxyConfiguration.properties.forEach((property, index, arr) => {
300+
if (!('value' in property)) {
301+
arr[index].value = '';
302+
}
303+
if (!('name' in property)) {
304+
arr[index].name = '';
305+
}
306+
});
307+
}
308+
return taskDef;
309+
}
310+
297311
// Deploy to a service that uses the 'CODE_DEPLOY' deployment controller
298312
async function createCodeDeployDeployment(codedeploy, clusterName, service, taskDefArn, waitForService, waitForMinutes) {
299313
core.debug('Updating AppSpec file with new task definition ARN');
@@ -393,14 +407,14 @@ async function run() {
393407

394408
const forceNewDeployInput = core.getInput('force-new-deployment', { required: false }) || 'false';
395409
const forceNewDeployment = forceNewDeployInput.toLowerCase() === 'true';
396-
410+
397411
// Register the task definition
398412
core.debug('Registering the task definition');
399413
const taskDefPath = path.isAbsolute(taskDefinitionFile) ?
400414
taskDefinitionFile :
401415
path.join(process.env.GITHUB_WORKSPACE, taskDefinitionFile);
402416
const fileContents = fs.readFileSync(taskDefPath, 'utf8');
403-
const taskDefContents = removeIgnoredAttributes(cleanNullKeys(yaml.parse(fileContents)));
417+
const taskDefContents = maintainAppMeshConfiguration(removeIgnoredAttributes(cleanNullKeys(yaml.parse(fileContents))));
404418
let registerResponse;
405419
try {
406420
registerResponse = await ecs.registerTaskDefinition(taskDefContents).promise();

‎index.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,20 @@ function removeIgnoredAttributes(taskDef) {
129129
return taskDef;
130130
}
131131

132+
function maintainAppMeshConfiguration(taskDef) {
133+
if ('proxyConfiguration' in taskDef && taskDef.proxyConfiguration.type == 'APPMESH' && taskDef.proxyConfiguration.properties.length > 0) {
134+
taskDef.proxyConfiguration.properties.forEach((property, index, arr) => {
135+
if (!('value' in property)) {
136+
arr[index].value = '';
137+
}
138+
if (!('name' in property)) {
139+
arr[index].name = '';
140+
}
141+
});
142+
}
143+
return taskDef;
144+
}
145+
132146
// Deploy to a service that uses the 'CODE_DEPLOY' deployment controller
133147
async function createCodeDeployDeployment(codedeploy, clusterName, service, taskDefArn, waitForService, waitForMinutes) {
134148
core.debug('Updating AppSpec file with new task definition ARN');
@@ -228,14 +242,14 @@ async function run() {
228242

229243
const forceNewDeployInput = core.getInput('force-new-deployment', { required: false }) || 'false';
230244
const forceNewDeployment = forceNewDeployInput.toLowerCase() === 'true';
231-
245+
232246
// Register the task definition
233247
core.debug('Registering the task definition');
234248
const taskDefPath = path.isAbsolute(taskDefinitionFile) ?
235249
taskDefinitionFile :
236250
path.join(process.env.GITHUB_WORKSPACE, taskDefinitionFile);
237251
const fileContents = fs.readFileSync(taskDefPath, 'utf8');
238-
const taskDefContents = removeIgnoredAttributes(cleanNullKeys(yaml.parse(fileContents)));
252+
const taskDefContents = maintainAppMeshConfiguration(removeIgnoredAttributes(cleanNullKeys(yaml.parse(fileContents))));
239253
let registerResponse;
240254
try {
241255
registerResponse = await ecs.registerTaskDefinition(taskDefContents).promise();

‎index.test.js‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,140 @@ describe('Deploy to ECS', () => {
252252
});
253253
});
254254

255+
test('maintains empty keys in proxyConfiguration.properties for APPMESH', async () => {
256+
fs.readFileSync.mockImplementation((pathInput, encoding) => {
257+
if (encoding != 'utf8') {
258+
throw new Error(`Wrong encoding ${encoding}`);
259+
}
260+
261+
return `
262+
{
263+
"memory": "",
264+
"containerDefinitions": [ {
265+
"name": "sample-container",
266+
"logConfiguration": {},
267+
"repositoryCredentials": { "credentialsParameter": "" },
268+
"command": [
269+
""
270+
],
271+
"environment": [
272+
{
273+
"name": "hello",
274+
"value": "world"
275+
},
276+
{
277+
"name": "",
278+
"value": ""
279+
}
280+
],
281+
"secretOptions": [ {
282+
"name": "",
283+
"valueFrom": ""
284+
} ],
285+
"cpu": 0,
286+
"essential": false
287+
} ],
288+
"requiresCompatibilities": [ "EC2" ],
289+
"registeredAt": 1611690781,
290+
"family": "task-def-family",
291+
"proxyConfiguration": {
292+
"type": "APPMESH",
293+
"containerName": "envoy",
294+
"properties": [
295+
{
296+
"name": "ProxyIngressPort",
297+
"value": "15000"
298+
},
299+
{
300+
"name": "AppPorts",
301+
"value": "1234"
302+
},
303+
{
304+
"name": "EgressIgnoredIPs",
305+
"value": "169.254.170.2,169.254.169.254"
306+
},
307+
{
308+
"name": "IgnoredGID",
309+
"value": ""
310+
},
311+
{
312+
"name": "EgressIgnoredPorts",
313+
"value": ""
314+
},
315+
{
316+
"name": "IgnoredUID",
317+
"value": "1337"
318+
},
319+
{
320+
"name": "ProxyEgressPort",
321+
"value": "15001"
322+
},
323+
{
324+
"value": "some-value"
325+
}
326+
]
327+
}
328+
}
329+
`;
330+
});
331+
332+
await run();
333+
expect(core.setFailed).toHaveBeenCalledTimes(0);
334+
expect(mockEcsRegisterTaskDef).toHaveBeenNthCalledWith(1, {
335+
family: 'task-def-family',
336+
containerDefinitions: [
337+
{
338+
name: 'sample-container',
339+
cpu: 0,
340+
essential: false,
341+
environment: [{
342+
name: 'hello',
343+
value: 'world'
344+
}]
345+
}
346+
],
347+
requiresCompatibilities: [ 'EC2' ],
348+
proxyConfiguration: {
349+
type: "APPMESH",
350+
containerName: "envoy",
351+
properties: [
352+
{
353+
name: "ProxyIngressPort",
354+
value: "15000"
355+
},
356+
{
357+
name: "AppPorts",
358+
value: "1234"
359+
},
360+
{
361+
name: "EgressIgnoredIPs",
362+
value: "169.254.170.2,169.254.169.254"
363+
},
364+
{
365+
name: "IgnoredGID",
366+
value: ""
367+
},
368+
{
369+
name: "EgressIgnoredPorts",
370+
value: ""
371+
},
372+
{
373+
name: "IgnoredUID",
374+
value: "1337"
375+
},
376+
{
377+
name: "ProxyEgressPort",
378+
value: "15001"
379+
},
380+
{
381+
name: "",
382+
value: "some-value"
383+
}
384+
]
385+
}
386+
});
387+
});
388+
255389
test('cleans invalid keys out of the task definition contents', async () => {
256390
fs.readFileSync.mockImplementation((pathInput, encoding) => {
257391
if (encoding != 'utf8') {

‎package-lock.json‎

Lines changed: 9 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)