Merge pull request #1213 from snyk/fix/caching-2

ivanstanev · web-flow · commit 6b64746dc991 · 2022-11-30T16:21:52.000Z
Fix/caching 2
diff --git a/src/scanner/index.ts b/src/scanner/index.ts
@@ -134,8 +134,8 @@ export async function scanImagesAndSendResults(
 
   // All workloads are identical, pick the first one
   const workload = workloadMetadata[0];
-  const workloadState = await getWorkloadAlreadyScanned(workload);
-  const imageState = await getWorkloadImageAlreadyScanned(
+  const workloadState = getWorkloadAlreadyScanned(workload);
+  const imageState = getWorkloadImageAlreadyScanned(
     workload,
     workload.imageName,
     workload.imageId,
diff --git a/src/state.ts b/src/state.ts
@@ -2,6 +2,7 @@ import { KubernetesObject, V1Namespace } from '@kubernetes/client-node';
 import LruCache from 'lru-cache';
 
 import { config } from './common/config';
+import { logger } from './common/logger';
 import { extractNamespaceName } from './supervisor/watchers/internal-namespaces';
 
 const imagesLruCacheOptions: LruCache.Options<string, Set<string>> = {
@@ -42,43 +43,62 @@ function getWorkloadImageAlreadyScannedKey(
   return `${workload.uid}/${imageName}`;
 }
 
-export async function getWorkloadAlreadyScanned(
+export function getWorkloadAlreadyScanned(
   workload: WorkloadAlreadyScanned,
-): Promise<string | undefined> {
+): string | undefined {
   const key = workload.uid;
   return state.workloadsAlreadyScanned.get(key);
 }
 
-export async function setWorkloadAlreadyScanned(
+export function setWorkloadAlreadyScanned(
   workload: WorkloadAlreadyScanned,
   revision: string,
-): Promise<boolean> {
+): boolean {
   const key = workload.uid;
   return state.workloadsAlreadyScanned.set(key, revision);
 }
 
-export async function deleteWorkloadAlreadyScanned(
+export function deleteWorkloadAlreadyScanned(
   workload: WorkloadAlreadyScanned,
-): Promise<void> {
+): void {
   const key = workload.uid;
   state.workloadsAlreadyScanned.del(key);
 }
 
-export async function getWorkloadImageAlreadyScanned(
+export function getWorkloadImageAlreadyScanned(
   workload: WorkloadAlreadyScanned,
   imageName: string,
   imageId: string,
-): Promise<string | undefined> {
+): string | undefined {
+  const cachedImages = state.imagesAlreadyScanned.dump().map((entry) => {
+    const values = new Array<string>();
+    for (const v in entry.v.values()) {
+      values.push(v);
+    }
+    return `entry: ${entry.e}, key: ${entry.k}; values: ${values.join(',')}`;
+  });
+  logger.debug(
+    { 'kubernetes-monitor': { cachedImages } },
+    'images in the cache',
+  );
+
   const key = getWorkloadImageAlreadyScannedKey(workload, imageName);
   const hasImageId = state.imagesAlreadyScanned.get(key)?.has(imageId);
-  return hasImageId ? imageId : undefined;
+  const response = hasImageId ? imageId : undefined;
+  if (response !== undefined) {
+    logger.debug(
+      { 'kubernetes-monitor': { imageId } },
+      'image already exists in cache',
+    );
+  }
+  return response;
 }
 
-export async function setWorkloadImageAlreadyScanned(
+export function setWorkloadImageAlreadyScanned(
   workload: WorkloadAlreadyScanned,
   imageName: string,
   imageId: string,
-): Promise<boolean> {
+): boolean {
   const key = getWorkloadImageAlreadyScannedKey(workload, imageName);
   const images = state.imagesAlreadyScanned.get(key);
   if (images !== undefined) {
@@ -92,9 +112,9 @@ export async function setWorkloadImageAlreadyScanned(
   return true;
 }
 
-export async function deleteWorkloadImagesAlreadyScanned(
+export function deleteWorkloadImagesAlreadyScanned(
   workload: WorkloadImagesAlreadyScanned,
-): Promise<void> {
+): void {
   for (const imageId of workload.imageIds) {
     const key = getWorkloadImageAlreadyScannedKey(workload, imageId);
     state.imagesAlreadyScanned.del(key);
diff --git a/src/supervisor/watchers/handlers/argo-rollout.ts b/src/supervisor/watchers/handlers/argo-rollout.ts
@@ -116,16 +116,14 @@ export async function argoRolloutWatchHandler(
   const workloadAlreadyScanned =
     kubernetesObjectToWorkloadAlreadyScanned(rollout);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: rollout.spec.template.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: rollout.spec.template.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName = rollout.metadata.name || FALSY_WORKLOAD_NAME_MARKER;
diff --git a/src/supervisor/watchers/handlers/cron-job.ts b/src/supervisor/watchers/handlers/cron-job.ts
@@ -110,16 +110,14 @@ export async function cronJobWatchHandler(
   const workloadAlreadyScanned =
     kubernetesObjectToWorkloadAlreadyScanned(cronJob);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: cronJob.spec.jobTemplate.spec.template.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: cronJob.spec.jobTemplate.spec.template.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName = cronJob.metadata.name || FALSY_WORKLOAD_NAME_MARKER;
diff --git a/src/supervisor/watchers/handlers/daemon-set.ts b/src/supervisor/watchers/handlers/daemon-set.ts
@@ -64,16 +64,14 @@ export async function daemonSetWatchHandler(
   const workloadAlreadyScanned =
     kubernetesObjectToWorkloadAlreadyScanned(daemonSet);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: daemonSet.spec.template.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: daemonSet.spec.template.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName = daemonSet.metadata.name || FALSY_WORKLOAD_NAME_MARKER;
diff --git a/src/supervisor/watchers/handlers/deployment-config.ts b/src/supervisor/watchers/handlers/deployment-config.ts
@@ -116,16 +116,14 @@ export async function deploymentConfigWatchHandler(
   const workloadAlreadyScanned =
     kubernetesObjectToWorkloadAlreadyScanned(deploymentConfig);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: deploymentConfig.spec.template.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: deploymentConfig.spec.template.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName =
diff --git a/src/supervisor/watchers/handlers/deployment.ts b/src/supervisor/watchers/handlers/deployment.ts
@@ -64,16 +64,14 @@ export async function deploymentWatchHandler(
   const workloadAlreadyScanned =
     kubernetesObjectToWorkloadAlreadyScanned(deployment);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: deployment.spec.template.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: deployment.spec.template.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName = deployment.metadata.name || FALSY_WORKLOAD_NAME_MARKER;
diff --git a/src/supervisor/watchers/handlers/job.ts b/src/supervisor/watchers/handlers/job.ts
@@ -58,16 +58,14 @@ export async function jobWatchHandler(job: V1Job): Promise<void> {
 
   const workloadAlreadyScanned = kubernetesObjectToWorkloadAlreadyScanned(job);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: job.spec.template.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: job.spec.template.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName = job.metadata.name || FALSY_WORKLOAD_NAME_MARKER;
diff --git a/src/supervisor/watchers/handlers/pod.ts b/src/supervisor/watchers/handlers/pod.ts
@@ -30,7 +30,7 @@ export async function handleReadyPod(
 ): Promise<void> {
   const workloadToScan: IWorkload[] = [];
   for (const workload of workloadMetadata) {
-    const scanned = await getWorkloadImageAlreadyScanned(
+    const scanned = getWorkloadImageAlreadyScanned(
       workload,
       workload.imageName,
       workload.imageId,
@@ -49,7 +49,7 @@ export async function handleReadyPod(
       { workloadToScan, imageId: workload.imageId },
       'image has not been scanned',
     );
-    await setWorkloadImageAlreadyScanned(
+    setWorkloadImageAlreadyScanned(
       workload,
       workload.imageName,
       workload.imageId,
@@ -59,7 +59,7 @@ export async function handleReadyPod(
 
   const workload = workloadToScan[0];
   if (workloadToScan.length > 0) {
-    workloadsToScanQueue.push({
+    await workloadsToScanQueue.pushAsync({
       key: workload.uid,
       workloadMetadata: workloadToScan,
       enqueueTimestampMs: Date.now(),
@@ -68,29 +68,21 @@ export async function handleReadyPod(
 }
 
 export function isPodReady(pod: V1Pod): boolean {
-  const podStatus = pod.status !== undefined;
+  const isTerminating = pod.metadata?.deletionTimestamp !== undefined;
   const podStatusPhase = pod.status?.phase === PodPhase.Running;
-  const podContainerStatuses = pod.status?.containerStatuses !== undefined;
-  const containerReadyStatuses = pod.status?.containerStatuses?.some(
-    (container) =>
-      container.state !== undefined &&
-      (container.state.running !== undefined ||
-        container.state.waiting !== undefined),
-  ) as boolean;
+  const containerReadyStatuses = Boolean(
+    pod.status?.containerStatuses?.every(
+      (container) => container.state?.running !== undefined,
+    ),
+  );
 
   const logContext = {
-    podStatus,
+    isTerminating,
     podStatusPhase,
-    podContainerStatuses,
     containerReadyStatuses,
   };
   logger.debug(logContext, 'checking to see if pod is ready to process');
-  return (
-    podStatus &&
-    podStatusPhase &&
-    podContainerStatuses &&
-    containerReadyStatuses
-  );
+  return !isTerminating && podStatusPhase && containerReadyStatuses;
 }
 
 export async function paginatedNamespacedPodList(namespace: string): Promise<{
@@ -155,12 +147,12 @@ export async function podWatchHandler(pod: V1Pod): Promise<void> {
     const workloadRevision = workloadMember.revision
       ? workloadMember.revision.toString()
       : '';
-    const scannedRevision = await getWorkloadAlreadyScanned(workloadMember);
+    const scannedRevision = getWorkloadAlreadyScanned(workloadMember);
     const isRevisionDifferent =
       scannedRevision === undefined ||
       Number(workloadRevision) > Number(scannedRevision);
     if (isRevisionDifferent) {
-      await setWorkloadAlreadyScanned(workloadMember, workloadRevision);
+      setWorkloadAlreadyScanned(workloadMember, workloadRevision);
       await sendWorkloadMetadata(workloadMetadataPayload);
     }
 
@@ -177,16 +169,14 @@ export async function podDeletedHandler(pod: V1Pod): Promise<void> {
 
   const workloadAlreadyScanned = kubernetesObjectToWorkloadAlreadyScanned(pod);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: pod.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: pod.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName = pod.metadata.name || FALSY_WORKLOAD_NAME_MARKER;
diff --git a/src/supervisor/watchers/handlers/queue.ts b/src/supervisor/watchers/handlers/queue.ts
@@ -58,7 +58,7 @@ async function queueWorkerWorkloadScan(
       ...workloadMetadata[0],
       imageIds,
     };
-    await deleteWorkloadImagesAlreadyScanned(workload);
+    deleteWorkloadImagesAlreadyScanned(workload);
   }
 }
 
diff --git a/src/supervisor/watchers/handlers/replica-set.ts b/src/supervisor/watchers/handlers/replica-set.ts
@@ -65,16 +65,14 @@ export async function replicaSetWatchHandler(
   const workloadAlreadyScanned =
     kubernetesObjectToWorkloadAlreadyScanned(replicaSet);
   if (workloadAlreadyScanned !== undefined) {
-    await Promise.all([
-      deleteWorkloadAlreadyScanned(workloadAlreadyScanned),
-      deleteWorkloadImagesAlreadyScanned({
-        ...workloadAlreadyScanned,
-        imageIds: replicaSet.spec.template.spec.containers
-          .filter((container) => container.image !== undefined)
-          .map((container) => container.image!),
-      }),
-      deleteWorkloadFromScanQueue(workloadAlreadyScanned),
-    ]);
+    deleteWorkloadAlreadyScanned(workloadAlreadyScanned);
+    deleteWorkloadImagesAlreadyScanned({
+      ...workloadAlreadyScanned,
+      imageIds: replicaSet.spec.template.spec.containers
+        .filter((container) => container.image !== undefined)
+        .map((container) => container.image!),
+    });
+    deleteWorkloadFromScanQueue(workloadAlreadyScanned);
   }
 
   const workloadName = replicaSet.metadata.name || FALSY_WORKLOAD_NAME_MARKER;
diff --git a/src/supervisor/watchers/handlers/replication-controller.ts b/src/supervisor/watchers/handlers/replication-controller.ts
diff --git a/src/supervisor/watchers/handlers/stateful-set.ts b/src/supervisor/watchers/handlers/stateful-set.ts
diff --git a/src/supervisor/watchers/handlers/types.ts b/src/supervisor/watchers/handlers/types.ts
diff --git a/test/unit/scanner/scan-results-caching.spec.ts b/test/unit/scanner/scan-results-caching.spec.ts
diff --git a/test/unit/supervisor/pod-watch-handler-caches.spec.ts b/test/unit/supervisor/pod-watch-handler-caches.spec.ts

Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ async function queueWorkerWorkloadScan(`
`58`	`58`	`...workloadMetadata[0],`
`59`	`59`	`imageIds,`
`60`	`60`	`};`
`61`		`- await deleteWorkloadImagesAlreadyScanned(workload);`
	`61`	`+ deleteWorkloadImagesAlreadyScanned(workload);`
`62`	`62`	`}`
`63`	`63`	`}`
`64`	`64`