From 08cb61f835b432c481ef58c55d2c1de5dc367c77 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Fri, 19 Jan 2018 06:59:35 +0100
Subject: [PATCH] feat: support multiple Docker FROM statements (#1409)

Adds support for multiple FROM statements within a Dockerfile. Thanks to @reicheltp for regex inspiration.

Closes #1011
---
 lib/manager/docker/detect.js                  |  5 +-
 lib/manager/docker/extract.js                 | 52 ++++++++++++-------
 test/manager/__snapshots__/index.spec.js.snap |  1 +
 .../docker/__snapshots__/extract.spec.js.snap | 49 +++++++++++++++++
 test/manager/docker/extract.spec.js           | 11 ++++
 test/manager/index.spec.js                    |  8 ++-
 6 files changed, 101 insertions(+), 25 deletions(-)

diff --git a/lib/manager/docker/detect.js b/lib/manager/docker/detect.js
index 4683b3359d..c2c0bb3ba8 100644
--- a/lib/manager/docker/detect.js
+++ b/lib/manager/docker/detect.js
@@ -10,10 +10,7 @@ async function detectPackageFiles(config, fileList) {
       if (file === 'Dockerfile' || file.endsWith('/Dockerfile')) {
         const content = await platform.getFile(file);
         if (content) {
-          const strippedComment = content.replace(/^((#.*?|\s*)\n)+/, '');
-          // This means we skip ones with ARG for now
-          const fromMatch = strippedComment.startsWith('FROM ');
-          if (fromMatch) {
+          if (content.match(/(^|\n)FROM .+\n/)) {
             packageFiles.push(file);
           }
         } else {
diff --git a/lib/manager/docker/extract.js b/lib/manager/docker/extract.js
index df402a78a2..15b4f611b0 100644
--- a/lib/manager/docker/extract.js
+++ b/lib/manager/docker/extract.js
@@ -2,27 +2,40 @@ module.exports = {
   extractDependencies,
 };
 
+function groupedRegex(string, pattern) {
+  const matches = string.match(new RegExp(pattern.source, pattern.flags));
+  if (!matches) {
+    return [];
+  }
+  return matches.map(
+    match => new RegExp(pattern.source, pattern.flags).exec(match)[1]
+  );
+}
+
 function extractDependencies(content) {
-  const fromMatch = content.match(/(\n|^)([Ff][Rr][Oo][Mm] .*)\n/);
-  if (!fromMatch) {
+  logger.debug('docker.extractDependencies()');
+  const deps = [];
+  const fromMatches = groupedRegex(content, /(?:^|\n)(FROM .+)\n/gi);
+  if (!fromMatches.length) {
     logger.warn({ content }, 'No FROM found');
     return [];
   }
-  const [, , fromLine] = fromMatch;
-  const [fromPrefix, currentFrom, ...fromRest] = fromLine.match(/\S+/g);
-  const fromSuffix = fromRest.join(' ');
-  let dockerRegistry;
-  const split = currentFrom.split('/');
-  if (split.length > 1 && split[0].includes('.')) {
-    [dockerRegistry] = split;
-    split.shift();
-  }
-  const currentDepTagDigest = split.join('/');
-  const [currentDepTag, currentDigest] = currentDepTagDigest.split('@');
-  const [depName, currentTag] = currentDepTag.split(':');
-  logger.info({ depName, currentTag, currentDigest }, 'Dockerfile FROM');
-  return [
-    {
+  logger.debug({ fromMatches }, 'Found matches');
+  fromMatches.forEach(fromLine => {
+    logger.debug({ fromLine }, 'fromLine');
+    const [fromPrefix, currentFrom, ...fromRest] = fromLine.match(/\S+/g);
+    const fromSuffix = fromRest.join(' ');
+    let dockerRegistry;
+    const split = currentFrom.split('/');
+    if (split.length > 1 && split[0].includes('.')) {
+      [dockerRegistry] = split;
+      split.shift();
+    }
+    const currentDepTagDigest = split.join('/');
+    const [currentDepTag, currentDigest] = currentDepTagDigest.split('@');
+    const [depName, currentTag] = currentDepTag.split(':');
+    logger.info({ depName, currentTag, currentDigest }, 'Dockerfile FROM');
+    deps.push({
       depType: 'Dockerfile',
       fromLine,
       fromPrefix,
@@ -34,6 +47,7 @@ function extractDependencies(content) {
       currentDigest,
       depName,
       currentTag,
-    },
-  ];
+    });
+  });
+  return deps;
 }
diff --git a/test/manager/__snapshots__/index.spec.js.snap b/test/manager/__snapshots__/index.spec.js.snap
index c3946b419a..cf561e12e6 100644
--- a/test/manager/__snapshots__/index.spec.js.snap
+++ b/test/manager/__snapshots__/index.spec.js.snap
@@ -16,6 +16,7 @@ Array [
 exports[`manager detectPackageFiles(config) finds Dockerfiles 1`] = `
 Array [
   "Dockerfile",
+  "other/Dockerfile",
 ]
 `;
 
diff --git a/test/manager/docker/__snapshots__/extract.spec.js.snap b/test/manager/docker/__snapshots__/extract.spec.js.snap
index 442dc8958e..0243b7a762 100644
--- a/test/manager/docker/__snapshots__/extract.spec.js.snap
+++ b/test/manager/docker/__snapshots__/extract.spec.js.snap
@@ -1,5 +1,36 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`lib/manager/docker/extract extractDependencies() extracts multiple FROM tags 1`] = `
+Array [
+  Object {
+    "currentDepTag": "node:6.12.3",
+    "currentDepTagDigest": "node:6.12.3",
+    "currentDigest": undefined,
+    "currentFrom": "node:6.12.3",
+    "currentTag": "6.12.3",
+    "depName": "node",
+    "depType": "Dockerfile",
+    "dockerRegistry": undefined,
+    "fromLine": "FROM node:6.12.3 as frontend",
+    "fromPrefix": "FROM",
+    "fromSuffix": "as frontend",
+  },
+  Object {
+    "currentDepTag": "python:3.6-slim",
+    "currentDepTagDigest": "python:3.6-slim",
+    "currentDigest": undefined,
+    "currentFrom": "python:3.6-slim",
+    "currentTag": "3.6-slim",
+    "depName": "python",
+    "depType": "Dockerfile",
+    "dockerRegistry": undefined,
+    "fromLine": "FROM python:3.6-slim",
+    "fromPrefix": "FROM",
+    "fromSuffix": "",
+  },
+]
+`;
+
 exports[`lib/manager/docker/extract extractDependencies() handles abnoral spacing 1`] = `
 Array [
   Object {
@@ -197,3 +228,21 @@ Array [
   },
 ]
 `;
+
+exports[`lib/manager/docker/extract extractDependencies() is case insensitive 1`] = `
+Array [
+  Object {
+    "currentDepTag": "node",
+    "currentDepTagDigest": "node",
+    "currentDigest": undefined,
+    "currentFrom": "node",
+    "currentTag": undefined,
+    "depName": "node",
+    "depType": "Dockerfile",
+    "dockerRegistry": undefined,
+    "fromLine": "From node",
+    "fromPrefix": "From",
+    "fromSuffix": "",
+  },
+]
+`;
diff --git a/test/manager/docker/extract.spec.js b/test/manager/docker/extract.spec.js
index a1fcc55cbf..a26fe78f16 100644
--- a/test/manager/docker/extract.spec.js
+++ b/test/manager/docker/extract.spec.js
@@ -10,6 +10,10 @@ describe('lib/manager/docker/extract', () => {
       const res = extractDependencies('FROM node\n', config);
       expect(res).toMatchSnapshot();
     });
+    it('is case insensitive', () => {
+      const res = extractDependencies('From node\n', config);
+      expect(res).toMatchSnapshot();
+    });
     it('handles tag', () => {
       const res = extractDependencies('FROM node:8.9.0-alpine\n', config);
       expect(res).toMatchSnapshot();
@@ -79,5 +83,12 @@ describe('lib/manager/docker/extract', () => {
       );
       expect(res).toMatchSnapshot();
     });
+    it('extracts multiple FROM tags', () => {
+      const res = extractDependencies(
+        'FROM node:6.12.3 as frontend\n\n# comment\nENV foo=bar\nFROM python:3.6-slim\n',
+        config
+      );
+      expect(res).toMatchSnapshot();
+    });
   });
 });
diff --git a/test/manager/index.spec.js b/test/manager/index.spec.js
index d33d3cbcf0..44e9a97a5c 100644
--- a/test/manager/index.spec.js
+++ b/test/manager/index.spec.js
@@ -53,16 +53,20 @@ describe('manager', () => {
       platform.getFileList.mockReturnValueOnce([
         'Dockerfile',
         'other/Dockerfile',
+        'another/Dockerfile',
       ]);
       platform.getFile.mockReturnValueOnce(
-        '### comment\n\n \nFROM something\nRUN something'
+        '### comment\n\n \nFROM something\nRUN something\nFROM something-else\nRUN bar'
       );
       platform.getFile.mockReturnValueOnce(
         'ARG foo\nFROM something\nRUN something'
       );
+      platform.getFile.mockReturnValueOnce(
+        'ARG foo\nno FROM at all\nRUN something'
+      );
       const res = await manager.detectPackageFiles(config);
       expect(res).toMatchSnapshot();
-      expect(res).toHaveLength(1);
+      expect(res).toHaveLength(2);
     });
     it('finds .travis.yml files', async () => {
       config.node.enabled = true;
-- 
GitLab