Fix #75 Validate HEALTHCHECK flags

Implemented validation for HEALTHCHECK's flags. However, note that when a HEALTHCHECK is a NONE, its flags will not get validated as the Docker builder itself seems to ignore them. Signed-off-by: Remy Suen <[email protected]>
rcjsuen · Jul 18, 2017 · 8affead · 8affead
1 parent efbc652
commit 8affead
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,10 @@ All notable changes to this project will be documented in this file.
     - HEALTHCHECK CMD flags ([#69](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/69))
   - textDocument/hover
     - HEALTHCHECK CMD flags ([#82](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/82))
+  - textDocument/publishDiagnostics
+    - check the spelling of instruction flags
+      - COPY's from
+      - HEALTHCHECK's interval, retries, start-period, timeout
   - 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
@@ -211,6 +211,38 @@ export class Validator {
 							return null;
 						}, Validator.createRequiresOneOrThreeArguments);
 						break;
+					case "HEALTHCHECK":
+						let args = instruction.getArguments();
+						if (args.length === 0) {
+							// all instructions are expected to have at least one argument
+							let range = instruction.getInstructionRange();
+							problems.push(Validator.createMissingArgument(range.start, range.end));
+						} else {
+							// check all the args
+							let flagsStart = args.length;
+							for (let i = 0; i < args.length; i++) {
+								let value = args[i].getValue();
+								let uppercase = value.toUpperCase();
+								if (uppercase === "NONE") {
+									// ignore all flags
+									flagsStart = 0;
+									break;
+								} else if (uppercase === "CMD") {
+									// check for flags stopping at i
+									flagsStart = i;
+									break;
+								}
+							}
+
+							let flags = [ "interval", "retries", "start-period", "timeout" ];
+							for (let i = 0; i < flagsStart; i++) {
+								let diagnostic = this.checkFlagName(args[i].getValue(), args[i].getRange(), flags);
+								if (diagnostic !== null) {
+									problems.push(diagnostic);
+								}
+							}
+						}
+						break;
 					case "STOPSIGNAL":
 						this.checkArguments(instruction, problems, [ 1 ], function(index: number, argument: string) {
 							if (argument.indexOf("SIG") === 0 || argument.indexOf('$') != -1) {
@@ -260,12 +292,12 @@ export class Validator {
 		return problems;
 	}
 
-	private checkFlagName(argument: string, range: Range, expectedFlagName: string): Diagnostic {
+	private checkFlagName(argument: string, range: Range, expectedFlagNames: string[]): Diagnostic {
 		if (argument.indexOf("--") === 0) {
 			let index = argument.indexOf('=');
 			index = index === -1 ? argument.length : index;
 			let actualFlagName = argument.substring(2, index);
-			if (actualFlagName !== expectedFlagName) {
+			if (expectedFlagNames.indexOf(actualFlagName) === -1) {
 				let offset = this.document.offsetAt(range.start);
 				return Validator.createFlagUnknown(this.document.positionAt(offset + 2), this.document.positionAt(offset + 2 + actualFlagName.length), actualFlagName);
 			}

diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts
@@ -1310,6 +1310,91 @@ describe("Docker Validator Tests", function() {
 		});
 	});
 
+	describe("HEALTHCHECK", function() {
+		describe("flags", function() {
+			it("ok", function() {
+				let diagnostics = validate("FROM alpine\nHEALTHCHECK --interval=30s --retries=3 --start-period=0s --timeout=30s CMD ls");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --interval=30s CMD ls");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --retries=3 CMD ls");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --start-period=0s CMD ls");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --timeout=30s CMD ls");
+				assert.equal(diagnostics.length, 0);
+
+				// flags don't make sense for a NONE but the builder ignores so we should too
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --interval=30s --retries=3 --start-period=0s --timeout=30s NONE");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --interval=30s NONE");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --retries=3 NONE");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --start-period=0s NONE");
+				assert.equal(diagnostics.length, 0);
+
+				diagnostics = validate("FROM alpine\nHEALTHCHECK --timeout=30s NONE");
+				assert.equal(diagnostics.length, 0);
+			});
+
+			describe("unknown flag", function() {
+				it("wrong name", function() {
+					let diagnostics = validate("FROM alpine\nHEALTHCHECK --interva=30s CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "interva", 1, 14, 1, 21);
+
+					// empty value
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --interva= CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "interva", 1, 14, 1, 21);
+
+					// no equals sign
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --interva CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "interva", 1, 14, 1, 21);
+
+					// no CMD
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --interva");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "interva", 1, 14, 1, 21);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --interva=");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "interva", 1, 14, 1, 21);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --interva=30s");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "interva", 1, 14, 1, 21);
+
+					// flags are case-sensitive
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --INTERVAL=30s CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "INTERVAL", 1, 14, 1, 22);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --RETRIES=3 CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "RETRIES", 1, 14, 1, 21);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --START-PERIOD=0s CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "START-PERIOD", 1, 14, 1, 26);
+
+					diagnostics = validate("FROM alpine\nHEALTHCHECK --TIMEOUT=30s CMD ls");
+					assert.equal(diagnostics.length, 1);
+					assertFlagUnknown(diagnostics[0], "TIMEOUT", 1, 14, 1, 21);
+				});
+			})
+		});
+	});
+
 	describe("MAINTAINER", function() {
 		it("default", function() {
 			let validator = new Validator();