Merge pull request #511 from snyk/fix-docker-pull

fix docker pull issues
snyk · Jul 12, 2023 · 884d703 · 884d703
2 parents a642800 + 21d7666
commit 884d703
Show file tree

Hide file tree

Showing 11 changed files with 107 additions and 75 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -38,9 +38,13 @@ slack-success-notify: &slack-success-notify
 
 windows_big: &windows_big
   executor:
-    name: win/default
+    name: win/server-2022
     shell: bash.exe
     size: large
+    # we've pinned the version because without it, it uses "current" (at the time of writing, "2023.06.1"),
+    # which has a broken Docker installation. See https://discuss.circleci.com/t/build-failures-when-running-docker-on-junes-windows-executor/48605
+    # TODO: check if it works again with the next release and unpin the version.
+    version: "2023.05.1"
   parameters:
     node_version:
       type: string
@@ -143,7 +147,21 @@ jobs:
           command: echo "//registry.npmjs.org/:_authToken=${NPM_TOKEN}" >> .npmrc
       - run: npm ci
       - run: npm run test-jest
-  test_jest_windows:
+  test_jest_windows_with_docker:
+    <<: *windows_big
+    steps:
+      - checkout
+      - install_node_npm:
+          node_version: << parameters.node_version >>
+      - run:
+          name: Use snyk-main npmjs user
+          command: echo "//registry.npmjs.org/:_authToken=${NPM_TOKEN}" >> .npmrc
+      - run: npm ci
+      - run: docker version
+      - run:
+          command: npm run test-jest-windows
+          no_output_timeout: 20m
+  test_jest_windows_no_docker:
     <<: *windows_big
     steps:
       - checkout
@@ -153,6 +171,8 @@ jobs:
           name: Use snyk-main npmjs user
           command: echo "//registry.npmjs.org/:_authToken=${NPM_TOKEN}" >> .npmrc
       - run: npm ci
+        # make docker appear to be broken.
+      - run: "function docker() { return 1; }"
       - run:
           command: npm run test-jest-windows
           no_output_timeout: 20m
@@ -213,11 +233,10 @@ workflows:
           channel: lumos-alerts
           filters:
             branches:
-              ignore:
-                main
+              ignore: main
       - install:
           name: Install
-          context: 
+          context:
             - nodejs-install
             - team-lumos
       - lint:
@@ -259,8 +278,19 @@ workflows:
             - Build
           post-steps:
             - *slack-fail-notify
-      - test_jest_windows:
-          name: Test Jest Windows
+      - test_jest_windows_with_docker:
+          name: Test Jest Windows with Docker
+          context:
+            - nodejs-install
+            - team-lumos
+            - snyk-bot-slack
+          node_version: "12"
+          requires:
+            - Build
+          post-steps:
+            - *slack-fail-notify
+      - test_jest_windows_no_docker:
+          name: Test Jest Windows no Docker
           context:
             - nodejs-install
             - team-lumos
@@ -295,7 +325,8 @@ workflows:
             - Build
             - Test
             - Test Windows
-            - Test Jest Windows
+            - Test Jest Windows with Docker
+            - Test Jest Windows no Docker
           post-steps:
             - *slack-fail-notify
             - *slack-success-notify
@@ -307,14 +338,14 @@ workflows:
     jobs:
       - install:
           name: Install
-          context: 
+          context:
             - nodejs-install
             - team-lumos
           post-steps:
             - *slack-fail-notify
       - build_and_test_latest_go_binary:
           name: Build Go binary
-          context: 
+          context:
             - nodejs-install
             - team-lumos
           requires:

diff --git a/lib/analyzer/image-inspector.ts b/lib/analyzer/image-inspector.ts
@@ -34,7 +34,9 @@ function cleanupCallback(imageFolderPath: string, imageName: string) {
     }
     fs.rmdir(imageFolderPath, (err) => {
       if (err !== null) {
-        debug(`Can't remove folder ${imageFolderPath}, got error ${err}`);
+        debug(
+          `Can't remove folder ${imageFolderPath}, got error ${err.message}`,
+        );
       }
     });
   };
@@ -59,55 +61,49 @@ async function pullWithDockerBinary(
     await docker.save(targetImage, saveLocation);
     return (pullAndSaveSuccessful = true);
   } catch (err) {
-    debug(`couldn't pull ${targetImage} using docker binary: ${err}`);
+    debug(`couldn't pull ${targetImage} using docker binary: ${err.message}`);
 
-    if (
-      err.stderr &&
-      err.stderr.includes("unknown operating system or architecture")
-    ) {
-      throw new Error("Unknown operating system or architecture");
-    }
+    handleDockerPullError(err.stderr, platform);
 
-    if (
-      err.stderr &&
-      err.stderr.includes("operating system is not supported")
-    ) {
-      throw new Error(`Operating system is not supported`);
-    }
+    return pullAndSaveSuccessful;
+  }
+}
 
-    const unknownManifestConditions = [
-      "no matching manifest for",
-      "manifest unknown",
-    ];
-    if (
-      err.stderr &&
-      unknownManifestConditions.some((value) => err.stderr.includes(value))
-    ) {
-      if (platform) {
-        throw new Error(`The image does not exist for ${platform}`);
-      }
-      throw new Error(`The image does not exist for the current platform`);
-    }
+function handleDockerPullError(err: string, platform?: string) {
+  if (err && err.includes("unknown operating system or architecture")) {
+    throw new Error("Unknown operating system or architecture");
+  }
 
-    if (err.stderr && err.stderr.includes("invalid reference format")) {
-      throw new Error(`invalid image format`);
-    }
+  if (err.includes("operating system is not supported")) {
+    throw new Error(`Operating system is not supported`);
+  }
 
-    if (err.stderr.includes("unknown flag: --platform")) {
-      throw new Error(
-        '"--platform" is only supported on a Docker daemon with version later than 17.09',
-      );
+  const unknownManifestConditions = [
+    "no matching manifest for",
+    "manifest unknown",
+  ];
+  if (unknownManifestConditions.some((value) => err.includes(value))) {
+    if (platform) {
+      throw new Error(`The image does not exist for ${platform}`);
     }
+    throw new Error(`The image does not exist for the current platform`);
+  }
 
-    if (
-      err.stderr &&
-      err.stderr ===
-        '"--platform" is only supported on a Docker daemon with experimental features enabled'
-    ) {
-      throw new Error(err.stderr);
-    }
+  if (err.includes("invalid reference format")) {
+    throw new Error(`invalid image format`);
+  }
 
-    return pullAndSaveSuccessful;
+  if (err.includes("unknown flag: --platform")) {
+    throw new Error(
+      '"--platform" is only supported on a Docker daemon with version later than 17.09',
+    );
+  }
+
+  if (
+    err ===
+    '"--platform" is only supported on a Docker daemon with experimental features enabled'
+  ) {
+    throw new Error(err);
   }
 }
 
@@ -122,14 +118,19 @@ async function pullFromContainerRegistry(
   debug(
     `Attempting to pull: registry: ${hostname}, image: ${imageName}, tag: ${tag}`,
   );
-  return await docker.pull(
-    hostname,
-    imageName,
-    tag,
-    imageSavePath,
-    username,
-    password,
-  );
+  try {
+    return await docker.pull(
+      hostname,
+      imageName,
+      tag,
+      imageSavePath,
+      username,
+      password,
+    );
+  } catch (err) {
+    handleDockerPullError(err.message);
+    throw err;
+  }
 }
 
 async function pullImage(
@@ -315,7 +316,7 @@ function isLocalImageSameArchitecture(
     // Note: this is using the same flag/input pattern as the new Docker buildx: eg. linux/arm64/v8
     platformArchitecture = platformOption.split("/")[1];
   } catch (error) {
-    debug(`Error parsing platform flag: '${error}'`);
+    debug(`Error parsing platform flag: '${error.message}'`);
     return false;
   }
 

diff --git a/lib/analyzer/os-release/static.ts b/lib/analyzer/os-release/static.ts
@@ -59,7 +59,7 @@ export async function detect(
     try {
       osRelease = await handler(osReleaseFile);
     } catch (err) {
-      debug(`Malformed OS release file: ${err}`);
+      debug(`Malformed OS release file: ${err.message}`);
     }
     if (osRelease) {
       break;

diff --git a/lib/analyzer/static-analyzer.ts b/lib/analyzer/static-analyzer.ts
@@ -159,7 +159,7 @@ export async function analyze(
       dockerfileAnalysis,
     );
   } catch (err) {
-    debug(`Could not detect OS release: ${err}`);
+    debug(`Could not detect OS release: ${err.message}`);
     throw new Error("Failed to detect OS release");
   }
 
@@ -180,7 +180,7 @@ export async function analyze(
       aptDistrolessAnalyze(targetImage, distrolessAptFiles),
     ]);
   } catch (err) {
-    debug(`Could not detect installed OS packages: ${err}`);
+    debug(`Could not detect installed OS packages: ${err.message}`);
     throw new Error("Failed to detect installed OS packages");
   }
 

diff --git a/lib/extractor/docker-archive/layer.ts b/lib/extractor/docker-archive/layer.ts
@@ -43,7 +43,7 @@ export async function extractArchive(
               extractActions,
             );
           } catch (error) {
-            debug(`Error extracting layer content from: '${error}'`);
+            debug(`Error extracting layer content from: '${error.message}'`);
             reject(new Error("Error reading tar archive"));
           }
         } else if (isManifestFile(normalizedName)) {
@@ -67,7 +67,7 @@ export async function extractArchive(
         );
       } catch (error) {
         debug(
-          `Error getting layers and manifest content from docker archive: ${error}`,
+          `Error getting layers and manifest content from docker archive: ${error.message}`,
         );
         reject(new InvalidArchiveError("Invalid Docker archive"));
       }

diff --git a/lib/extractor/layer.ts b/lib/extractor/layer.ts
@@ -46,7 +46,7 @@ export async function extractImageLayer(
           } catch (error) {
             // An ExtractAction has thrown an uncaught exception, likely a bug in the code!
             debug(
-              `Exception thrown while applying callbacks during image layer extraction: ${error}`,
+              `Exception thrown while applying callbacks during image layer extraction: ${error.message}`,
             );
             reject(error);
           }

diff --git a/lib/extractor/oci-archive/layer.ts b/lib/extractor/oci-archive/layer.ts
@@ -100,7 +100,7 @@ export async function extractArchive(
         );
       } catch (error) {
         debug(
-          `Error getting layers and manifest content from oci archive: '${error}'`,
+          `Error getting layers and manifest content from oci archive: '${error.message}'`,
         );
         reject(new InvalidArchiveError("Invalid OCI archive"));
       }

diff --git a/lib/go-parser/index.ts b/lib/go-parser/index.ts
@@ -181,7 +181,7 @@ export async function goModulesToScannedProjects(
         },
       });
     } catch (err) {
-      debug(`Go binary scan for file ${filePath} failed: ${err}`);
+      debug(`Go binary scan for file ${filePath} failed: ${err.message}`);
     }
   }
 

diff --git a/lib/inputs/rpm/static.ts b/lib/inputs/rpm/static.ts
@@ -32,7 +32,7 @@ export async function getRpmDbFileContent(
     }
     return parserResponse.response;
   } catch (error) {
-    debug(`An error occurred while analysing RPM packages: ${error}`);
+    debug(`An error occurred while analysing RPM packages: ${error.message}`);
     return [];
   }
 }
@@ -56,7 +56,7 @@ export async function getRpmSqliteDbFileContent(
     }
     return results.response;
   } catch (error) {
-    debug(`An error occurred while analysing RPM packages: ${error}`);
+    debug(`An error occurred while analysing RPM packages: ${error.message}`);
     return [];
   }
 }

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -46,7 +46,7 @@
     "mkdirp": "^1.0.4",
     "packageurl-js": "^1.0.2",
     "semver": "^7.5.1",
-    "shescape": "1.7.1",
+    "shescape": "^1.7.2",
     "snyk-nodejs-lockfile-parser": "^1.50.0",
     "snyk-poetry-lockfile-parser": "^1.1.7",
     "tar-stream": "^2.1.0",