fix(gitlabci): gracefully handle errors (#9163)

* fix(gitlabci): gracefully handle errors * fix: remove debugging * Update lib/manager/gitlabci/utils.ts Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com> Co-authored-by: HonkingGoose <34918129+HonkingGoose@users.noreply.github.com>
renovatebot · Mar 16, 2021 · 668c879 · 668c879
1 parent 295d5ab
commit 668c879
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 21 deletions.
diff --git a/lib/manager/gitlabci/__fixtures__/gitlab-ci.3.yaml b/lib/manager/gitlabci/__fixtures__/gitlab-ci.3.yaml
@@ -10,4 +10,11 @@ services:
 
 include:
   - local: 'lib/manager/gitlabci/__fixtures__/include.yml'
+  - local: 'lib/manager/gitlabci/__fixtures__/include.yml' # Loop detection
   - local: 'lib/manager/gitlabci/__fixtures__/include.1.yml'
+  - project: 'my-group/my-project'
+    ref: master
+    file: '/templates/.gitlab-ci-template.yml'
+
+script:
+  - !reference [.setup, script]
diff --git a/lib/manager/gitlabci/__fixtures__/gitlab-ci.4.yaml b/lib/manager/gitlabci/__fixtures__/gitlab-ci.4.yaml
@@ -0,0 +1,7 @@
+include:
+  - local: 'lib/manager/gitlabci/__fixtures__/include.yml'
+
+test:
+  script:
+    - !abc [.setup, script]
+    - echo running my own command
diff --git a/lib/manager/gitlabci/__fixtures__/include.1.yml b/lib/manager/gitlabci/__fixtures__/include.1.yml
@@ -1,3 +1,6 @@
+# not existing
+include: 'lib/manager/gitlabci/__fixtures__/include.2.yml'
+
 test:
   stage: test
   image: node:12

diff --git a/lib/manager/gitlabci/__fixtures__/include.yml b/lib/manager/gitlabci/__fixtures__/include.yml
@@ -1,5 +1,10 @@
+
+# Loop detection
+include: 'lib/manager/gitlabci/__fixtures__/include.yml'
+
 test:
   stage: test
   image: alpine:3.11
   script:
     - echo test
+    - !reference [.setup, script]
diff --git a/lib/manager/gitlabci/__snapshots__/extract.spec.ts.snap b/lib/manager/gitlabci/__snapshots__/extract.spec.ts.snap
@@ -148,30 +148,30 @@ Array [
     "deps": Array [
       Object {
         "autoReplaceStringTemplate": "{{depName}}{{#if newValue}}:{{newValue}}{{/if}}{{#if newDigest}}@{{newDigest}}{{/if}}",
+        "commitMessageTopic": "Node.js",
         "currentDigest": undefined,
-        "currentValue": "3.11",
+        "currentValue": "12",
         "datasource": "docker",
-        "depName": "alpine",
+        "depName": "node",
         "depType": "image",
-        "replaceString": "alpine:3.11",
+        "replaceString": "node:12",
       },
     ],
-    "packageFile": "lib/manager/gitlabci/__fixtures__/include.yml",
+    "packageFile": "lib/manager/gitlabci/__fixtures__/include.1.yml",
   },
   Object {
     "deps": Array [
       Object {
         "autoReplaceStringTemplate": "{{depName}}{{#if newValue}}:{{newValue}}{{/if}}{{#if newDigest}}@{{newDigest}}{{/if}}",
-        "commitMessageTopic": "Node.js",
         "currentDigest": undefined,
-        "currentValue": "12",
+        "currentValue": "3.11",
         "datasource": "docker",
-        "depName": "node",
+        "depName": "alpine",
         "depType": "image",
-        "replaceString": "node:12",
+        "replaceString": "alpine:3.11",
       },
     ],
-    "packageFile": "lib/manager/gitlabci/__fixtures__/include.1.yml",
+    "packageFile": "lib/manager/gitlabci/__fixtures__/include.yml",
   },
 ]
 `;
diff --git a/lib/manager/gitlabci/extract.spec.ts b/lib/manager/gitlabci/extract.spec.ts
@@ -1,4 +1,4 @@
-import { getName } from '../../../test/util';
+import { getName, logger } from '../../../test/util';
 import type { PackageDependency } from '../types';
 import { extractAllPackageFiles } from './extract';
 
@@ -11,6 +11,7 @@ describe(getName(__filename), () => {
         ])
       ).toBeNull();
     });
+
     it('extracts multiple included image lines', async () => {
       const res = await extractAllPackageFiles({}, [
         'lib/manager/gitlabci/__fixtures__/gitlab-ci.3.yaml',
@@ -26,6 +27,7 @@ describe(getName(__filename), () => {
       });
       expect(deps).toHaveLength(5);
     });
+
     it('extracts multiple image lines', async () => {
       const res = await extractAllPackageFiles({}, [
         'lib/manager/gitlabci/__fixtures__/gitlab-ci.yaml',
@@ -43,6 +45,7 @@ describe(getName(__filename), () => {
 
       expect(deps.some((dep) => dep.currentValue.includes("'"))).toBe(false);
     });
+
     it('extracts multiple image lines with comments', async () => {
       const res = await extractAllPackageFiles({}, [
         'lib/manager/gitlabci/__fixtures__/gitlab-ci.1.yaml',
@@ -58,5 +61,13 @@ describe(getName(__filename), () => {
       });
       expect(deps).toHaveLength(3);
     });
+
+    it('catches errors', async () => {
+      const res = await extractAllPackageFiles({}, [
+        'lib/manager/gitlabci/__fixtures__/gitlab-ci.4.yaml',
+      ]);
+      expect(res).toBeNull();
+      expect(logger.logger.warn).toHaveBeenCalled();
+    });
   });
 });
diff --git a/lib/manager/gitlabci/extract.ts b/lib/manager/gitlabci/extract.ts
@@ -4,6 +4,8 @@ import { logger } from '../../logger';
 import { readLocalFile } from '../../util/fs';
 import { getDep } from '../dockerfile/extract';
 import type { ExtractConfig, PackageDependency, PackageFile } from '../types';
+import type { GitlabPipeline } from './types';
+import { replaceReferenceTags } from './utils';
 
 function skipCommentLines(
   lines: string[],
@@ -84,28 +86,43 @@ export function extractPackageFile(content: string): PackageFile | null {
 }
 
 export async function extractAllPackageFiles(
-  config: ExtractConfig,
+  _config: ExtractConfig,
   packageFiles: string[]
 ): Promise<PackageFile[] | null> {
-  const filesToExamine = new Set<string>(packageFiles);
+  const filesToExamine = [...packageFiles];
+  const seen = new Set<string>(packageFiles);
   const results: PackageFile[] = [];
 
   // extract all includes from the files
-  while (filesToExamine.size > 0) {
-    const file = filesToExamine.values().next().value;
-    filesToExamine.delete(file);
+  while (filesToExamine.length > 0) {
+    const file = filesToExamine.pop();
 
     const content = await readLocalFile(file, 'utf8');
-    const doc = yaml.safeLoad(content, { json: true }) as any;
-    if (doc?.include && is.array(doc.include)) {
+    let doc: GitlabPipeline;
+    try {
+      doc = yaml.safeLoad(replaceReferenceTags(content), {
+        json: true,
+      }) as GitlabPipeline;
+    } catch (err) {
+      logger.warn({ err, file }, 'Error extracting GitLab CI dependencies');
+    }
+
+    if (is.array(doc?.include)) {
       for (const includeObj of doc.include) {
-        if (includeObj.local) {
-          const fileObj = (includeObj.local as string).replace(/^\//, '');
-          if (!filesToExamine.has(fileObj)) {
-            filesToExamine.add(fileObj);
+        if (is.string(includeObj.local)) {
+          const fileObj = includeObj.local.replace(/^\//, '');
+          if (!seen.has(fileObj)) {
+            seen.add(fileObj);
+            filesToExamine.push(fileObj);
           }
         }
       }
+    } else if (is.string(doc?.include)) {
+      const fileObj = doc.include.replace(/^\//, '');
+      if (!seen.has(fileObj)) {
+        seen.add(fileObj);
+        filesToExamine.push(fileObj);
+      }
     }
 
     const result = extractPackageFile(content);

diff --git a/lib/manager/gitlabci/types.ts b/lib/manager/gitlabci/types.ts
@@ -0,0 +1,7 @@
+export interface GitlabInclude {
+  local?: string;
+}
+
+export interface GitlabPipeline {
+  include?: GitlabInclude[] | string;
+}
diff --git a/lib/manager/gitlabci/utils.ts b/lib/manager/gitlabci/utils.ts
@@ -0,0 +1,12 @@
+const re = /!reference \[\.\w+?(?:, \w+?)\]/g;
+
+/**
+ * Replaces GitLab reference tags before parsing, because our yaml parser cannot process them anyway.
+ * @param content pipeline yaml
+ * @returns replaced pipeline content
+ * https://docs.gitlab.com/ee/ci/yaml/#reference-tags
+ */
+export function replaceReferenceTags(content: string): string {
+  const res = content.replace(re, '');
+  return res;
+}