Merge pull request #508 from snyk/fix/image-name-chopped-for-local-registries

Fix/image name chopped for local registries

Author: agatakrajewska

src/scanner/images/index.ts

@@ -41,18 +41,31 @@ export async function removePulledImages(images: IPullableImage[]): Promise<void
 }
 
 // Exported for testing
-export function removeTagFromImage(imageWithTag: string): string {
-  return imageWithTag.split('@')[0].split(':')[0];
+export function getImageParts(imageWithTag: string) : {imageName: string, imageTag: string} {
+  // we're matching pattern: <registry:port_number>(optional)/<image_name>(mandatory)@<tag_identifier>(optional):<image_tag>(optional)
+  const regex = /((?:.*(:\d{4})?\/)?(?:[a-z0-9]+))([@|:].+)?/ig;
+  const groups  = regex.exec(imageWithTag);
+  
+  if(!groups){
+    logger.error({image: imageWithTag}, 'Image with tag is malformed, cannot extract valid parts');
+    return {imageName: imageWithTag, imageTag: ''};
+  }
+
+  return {
+    imageName: groups[1],
+    imageTag: validateAndExtractImageTag(groups[3])
+  };
 }
 
-// Exported for testing
-export function getImageTag(imageWithTag: string): string {
-  const imageParts: string[] = imageWithTag.split(':');
-  if (imageParts.length === 2) { // image@sha256:hash or image:tag
-    return imageParts[1];
+function validateAndExtractImageTag(imageTagGroup: string | undefined): string {
+  if(imageTagGroup === undefined){
+    return '';
   }
 
-  return '';
+  const imageTagParts: string[]= imageTagGroup.split(':');
+
+  //valid formats: image@sha256:hash or image:tag
+  return imageTagParts.length === 2 ? imageTagParts[1] : '';
 }
 
 // Exported for testing
@@ -84,13 +97,15 @@ export async function scanImages(images: IPullableImage[]): Promise<IScanResult[
         throw Error('Unexpected empty result from docker-plugin');
       }
 
+      const imageParts: {imageName: string, imageTag: string} = getImageParts(imageName);
+
       result.imageMetadata = {
-        image: removeTagFromImage(imageName),
-        imageTag: getImageTag(imageName),
+        image: imageParts.imageName,
+        imageTag: imageParts.imageTag,
       };
 
       scannedImages.push({
-        image: removeTagFromImage(imageName),
+        image: imageParts.imageName,
         imageWithTag: imageName,
         pluginResult: result,
       });

test/integration/kubernetes.test.ts

@@ -258,13 +258,13 @@ tap.test('snyk-monitor pulls images from a local registry and sends data to kube
     t.pass('Not testing local container registry because we\'re not running in KinD');
     return;
   }
-  t.plan(4);
+  t.plan(3);
 
   const deploymentName = 'python-local';
   const namespace = 'services';
   const clusterName = 'Default cluster';
   const deploymentType = WorkloadKind.Deployment;
-  const imageName = 'kind-registry:5000/python:rc-buster';
+  const imageName = 'kind-registry:5000/python';
 
   await kubectl.waitForJob('push-to-local-registry', 'default');
 
@@ -288,13 +288,8 @@ tap.test('snyk-monitor pulls images from a local registry and sends data to kube
 
   t.ok('dependencyGraphResults' in depGraphResult,
   'expected dependencyGraphResults field to exist in /dependency-graphs response');
-
-  /* Because of a bug in removeTagFromImage() func in src/scanner/images/index.ts, 
-  which chops off everything after ':' from the image name, we store a wrong image name 
-  and the result does not exist in the object referred below */
-  t.same(depGraphResult.dependencyGraphResults[imageName], null,
-  'expected result for image kind-registry:5000/python:rc-buster does not exist');
-  t.ok('kind-registry' in depGraphResult.dependencyGraphResults, 'BUG: the full image name is not stored in kubernetes-upstream');
+  t.ok('imageMetadata' in JSON.parse(depGraphResult.dependencyGraphResults[imageName]),
+    'snyk-monitor sent expected data to upstream in the expected time frame');
 });
 
 tap.test('snyk-monitor sends deleted workload to kubernetes-upstream', async (t) => {

test/unit/scanner/images.test.ts

@@ -56,29 +56,39 @@ tap.test('constructStaticAnalysisOptions() tests', async (t) => {
   t.deepEqual(options, expectedResult, 'returned options match expectations');
 });
 
-tap.test('getImageTag() tests', async (t) => {
-  t.plan(4);
+tap.test('extracted image tag tests', async (t) => {
+  t.plan(6);
 
   const imageWithSha = 'nginx@sha256:1234567890abcdef';
-  const imageWithShaResult = scannerImages.getImageTag(imageWithSha);
-  t.same(imageWithShaResult, '1234567890abcdef', 'image sha is returned');
+  const imageWithShaResult = scannerImages.getImageParts(imageWithSha);
+  t.same(imageWithShaResult.imageTag, '1234567890abcdef', 'image sha is returned');
 
   const imageWithTag = 'nginx:latest';
-  const imageWithTagResult = scannerImages.getImageTag(imageWithTag);
-  t.same(imageWithTagResult, 'latest', 'image tag is returned');
+  const imageWithTagResult = scannerImages.getImageParts(imageWithTag);
+  t.same(imageWithTagResult.imageTag, 'latest', 'image tag is returned');
+
+  const imageWithFullRepository = 'kind-registry:5000/nginx:latest';
+  const imageWithFullRepositoryResult = scannerImages.getImageParts(imageWithFullRepository);
+  t.same(imageWithFullRepositoryResult.imageTag, 'latest', 'image tag is returned when full repo specified');
 
   const imageWithoutTag = 'nginx';
-  const imageWithoutTagResult = scannerImages.getImageTag(imageWithoutTag);
-  t.same(imageWithoutTagResult, '', 'empty tag returned when no tag is specified');
+  const imageWithoutTagResult = scannerImages.getImageParts(imageWithoutTag);
+  t.same(imageWithoutTagResult.imageTag, '', 'empty tag returned when no tag is specified');
 
   const imageWithManySeparators = 'nginx@abc:tag@bad:reallybad';
-  const imageWithManySeparatorsResult = scannerImages.getImageTag(imageWithManySeparators);
-  t.same(imageWithManySeparatorsResult, '', 'empty tag is returned on malformed image name and tag');
+  const imageWithManySeparatorsResult = scannerImages.getImageParts(imageWithManySeparators);
+  t.same(imageWithManySeparatorsResult.imageTag, '', 'empty tag is returned on malformed image name and tag');
+
+  const imageWithFullRepoAndManySeparators = 'kind-registry:5000/nginx@abc:tag@bad:reallybad';
+  const imageWithFullRepoAndManySeparatorsResult = scannerImages.getImageParts(imageWithFullRepoAndManySeparators);
+  t.same(imageWithFullRepoAndManySeparatorsResult.imageTag, '', 'empty tag is returned on malformed image name and tag with full repo');
 });
 
-tap.test('removeTagFromImage() tests', async (t) => {
-  t.plan(2);
+tap.test('extracted image name tests', async (t) => {
+  t.plan(4);
 
-  t.same(scannerImages.removeTagFromImage('nginx:latest'), 'nginx', 'removed image:tag');
-  t.same(scannerImages.removeTagFromImage('nginx:@sha256:1234567890abcdef'), 'nginx', 'removed image@sha:hex');
+  t.same(scannerImages.getImageParts('nginx:latest').imageName, 'nginx', 'removed image:tag');
+  t.same(scannerImages.getImageParts('nginx:@sha256:1234567890abcdef').imageName, 'nginx', 'removed malformed image:@sha:hex');
+  t.same(scannerImages.getImageParts('node@sha256:215a9fbef4df2c1ceb7c79481d3cfd94ad8f1f0105bade39f3be907bf386c5e1').imageName, 'node', 'removed image@sha:hex');
+  t.same(scannerImages.getImageParts('kind-registry:5000/python:rc-buster').imageName, 'kind-registry:5000/python', 'removed repository/image:tag');
 });