From d4df46a252266f3acb368a55cd0f34090bd5e211 Mon Sep 17 00:00:00 2001 From: Remy Suen Date: Thu, 20 Jul 2017 06:33:47 +0900 Subject: [PATCH] Fix #89 Warn if --retries of HEALTHCHECK is not positive The retries flag of a HEALTHCHECK specifies the number of times that the HEALTHCHECK CMD should be run before the container is considered to be `unhealthy`. As such, the retries flag should be a positive integer as negative values or "zero retries" don't make any sense. A diagnostic will now be created to handle this error in Dockerfiles. Signed-off-by: Remy Suen --- CHANGELOG.md | 1 + src/dockerValidator.ts | 26 ++++++++++++++++++++++---- test/dockerValidator.test.ts | 21 +++++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 139309f..ea619ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ All notable changes to this project will be documented in this file. - HEALTHCHECK - warn if arguments follow a HEALTHCHECK NONE ([#84](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/84)) - warn if the retries flag doesn't specify a number ([#85](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/85)) + - warn if the retries flag is not a positive intger ([#89](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/89)) - created a CHANGELOG.md file to document the project's changes ([#77](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/77)) ### Fixed diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts index b850cb2..cec7bc3 100644 --- a/src/dockerValidator.ts +++ b/src/dockerValidator.ts @@ -21,6 +21,7 @@ export enum ValidationCode { ARGUMENT_REQUIRES_TWO, ARGUMENT_REQUIRES_ONE_OR_THREE, ARGUMENT_UNNECESSARY, + FLAG_AT_LEAST_ONE, NO_SOURCE_IMAGE, INVALID_ESCAPE_DIRECTIVE, INVALID_AS, @@ -276,11 +277,16 @@ export class Validator { if (diagnostic === null) { if (value.indexOf("--retries=") === 0) { value = value.substring(10); + let integer = parseInt(value); // test for NaN or numbers with decimals - if (isNaN(parseInt(value)) || value.indexOf('.') !== -1) { + if (isNaN(integer) || value.indexOf('.') !== -1) { let start = Position.create(range.start.line, range.start.character + 10); let end = Position.create(range.end.line, range.end.character); problems.push(Validator.createInvalidSyntax(start, end, value)); + } else if (integer < 1) { + let start = Position.create(range.start.line, range.start.character + 10); + let end = Position.create(range.end.line, range.end.character); + problems.push(Validator.createFlagAtLeastOne(start, end, "--retries", integer.toString())); } } } else { @@ -365,7 +371,8 @@ export class Validator { "invalidSyntax": "parsing \"${0}\": invalid syntax", "syntaxMissingEquals": "Syntax error - can't find = in \"${0}\". Must be of the form: name=value", - + + "flagAtLeastOne": "${0} must be at least 1 (not ${1})", "flagUnknown": "Unknown flag: ${0}", "instructionExtraArgument": "Instruction has an extra argument", @@ -380,8 +387,11 @@ export class Validator { "deprecatedMaintainer": "MAINTAINER has been deprecated", }; - private static formatMessage(text: string, variable: string): string { - return text.replace("${0}", variable); + private static formatMessage(text: string, ...variables: string[]): string { + for (let i = 0; i < variables.length; i++) { + text = text.replace("${" + i + "}", variables[i]); + } + return text; } public static getDiagnosticMessage_DirectiveCasing() { @@ -396,6 +406,10 @@ export class Validator { return Validator.dockerProblems["noSourceImage"]; } + public static getDiagnosticMessage_FlagAtLeastOne(flagName: string, flagValue: string) { + return Validator.formatMessage(Validator.dockerProblems["flagAtLeastOne"], flagName, flagValue); + } + public static getDiagnosticMessage_FlagUnknown(flag: string) { return Validator.formatMessage(Validator.dockerProblems["flagUnknown"], flag); } @@ -464,6 +478,10 @@ export class Validator { return Validator.createError(start, end, Validator.getDiagnosticMessage_DirectiveEscapeInvalid(value), ValidationCode.INVALID_ESCAPE_DIRECTIVE); } + static createFlagAtLeastOne(start: Position, end: Position, flagName: string, flagValue: string): Diagnostic { + return Validator.createError(start, end, Validator.getDiagnosticMessage_FlagAtLeastOne(flagName, flagValue), ValidationCode.FLAG_AT_LEAST_ONE); + } + static createFlagUnknown(start: Position, end: Position, flag: string): Diagnostic { return Validator.createError(start, end, Validator.getDiagnosticMessage_FlagUnknown(flag), ValidationCode.UNKNOWN_FLAG); } diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts index d15f2d1..02834cf 100644 --- a/test/dockerValidator.test.ts +++ b/test/dockerValidator.test.ts @@ -54,6 +54,17 @@ function assertNoSourceImage(diagnostic: Diagnostic, startLine: number, startCha assert.equal(diagnostic.range.end.character, endCharacter); } +function assertFlagAtLeastOne(diagnostic: Diagnostic, flagName: string, flagValue: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) { + assert.equal(diagnostic.code, ValidationCode.FLAG_AT_LEAST_ONE); + assert.equal(diagnostic.severity, DiagnosticSeverity.Error); + assert.equal(diagnostic.source, source); + assert.equal(diagnostic.message, Validator.getDiagnosticMessage_FlagAtLeastOne(flagName, flagValue)); + assert.equal(diagnostic.range.start.line, startLine); + assert.equal(diagnostic.range.start.character, startCharacter); + assert.equal(diagnostic.range.end.line, endLine); + assert.equal(diagnostic.range.end.character, endCharacter); +} + function assertFlagUnknown(diagnostic: Diagnostic, flag: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) { assert.equal(diagnostic.code, ValidationCode.UNKNOWN_FLAG); assert.equal(diagnostic.severity, DiagnosticSeverity.Error); @@ -1466,6 +1477,16 @@ describe("Docker Validator Tests", function() { assert.equal(diagnostics.length, 1); assertInvalidSyntax(diagnostics[0], "1.0", 1, 22, 1, 25); }); + + it("--retries at least one", function() { + let diagnostics = validate("FROM alpine\nHEALTHCHECK --retries=0 CMD ls"); + assert.equal(diagnostics.length, 1); + assertFlagAtLeastOne(diagnostics[0], "--retries", "0", 1, 22, 1, 23); + + diagnostics = validate("FROM alpine\nHEALTHCHECK --retries=-1 CMD ls"); + assert.equal(diagnostics.length, 1); + assertFlagAtLeastOne(diagnostics[0], "--retries", "-1", 1, 22, 1, 24); + }); }); });