Merge pull request #1206 from snyk/fix/caching

Fix/caching

Author: ivanstanev

config.default.json

@@ -9,7 +9,7 @@
   },
   "WORKLOADS_SCANNED_CACHE": {
     "MAX_SIZE": 10000,
-    "MAX_AGE_MS": 60000
+    "MAX_AGE_MS": 86400000
   },
   "WORKERS_COUNT": 5,
   "REQUEST_QUEUE_LENGTH": 2,

src/scanner/index.ts

@@ -138,6 +138,7 @@ export async function scanImagesAndSendResults(
   const imageState = await getWorkloadImageAlreadyScanned(
     workload,
     workload.imageName,
+    workload.imageId,
   );
   if (workloadState === undefined && imageState === undefined) {
     logger.info(

src/state.ts

@@ -4,7 +4,7 @@ import LruCache from 'lru-cache';
 import { config } from './common/config';
 import { extractNamespaceName } from './supervisor/watchers/internal-namespaces';
 
-const imagesLruCacheOptions: LruCache.Options<string, string> = {
+const imagesLruCacheOptions: LruCache.Options<string, Set<string>> = {
   // limit cache size so we don't exceed memory limit
   max: config.IMAGES_SCANNED_CACHE.MAX_SIZE,
   // limit cache life so if our backend loses track of an image's data,
@@ -35,56 +35,61 @@ interface WorkloadImagesAlreadyScanned {
   imageIds: string[];
 }
 
-function getWorkloadAlreadyScannedKey(
-  workload: WorkloadAlreadyScanned,
-): string {
-  return `${workload.namespace}/${workload.type}/${workload.uid}`;
-}
-
 function getWorkloadImageAlreadyScannedKey(
   workload: WorkloadAlreadyScanned,
-  imageId: string,
+  imageName: string,
 ): string {
-  return `${workload.namespace}/${workload.type}/${workload.uid}/${imageId}`;
+  return `${workload.uid}/${imageName}`;
 }
 
 export async function getWorkloadAlreadyScanned(
   workload: WorkloadAlreadyScanned,
 ): Promise<string | undefined> {
-  const key = getWorkloadAlreadyScannedKey(workload);
+  const key = workload.uid;
   return state.workloadsAlreadyScanned.get(key);
 }
 
 export async function setWorkloadAlreadyScanned(
   workload: WorkloadAlreadyScanned,
-  value: string,
+  revision: string,
 ): Promise<boolean> {
-  const key = getWorkloadAlreadyScannedKey(workload);
-  return state.workloadsAlreadyScanned.set(key, value);
+  const key = workload.uid;
+  return state.workloadsAlreadyScanned.set(key, revision);
 }
 
 export async function deleteWorkloadAlreadyScanned(
   workload: WorkloadAlreadyScanned,
 ): Promise<void> {
-  const key = getWorkloadAlreadyScannedKey(workload);
+  const key = workload.uid;
   state.workloadsAlreadyScanned.del(key);
 }
 
 export async function getWorkloadImageAlreadyScanned(
   workload: WorkloadAlreadyScanned,
+  imageName: string,
   imageId: string,
 ): Promise<string | undefined> {
-  const key = getWorkloadImageAlreadyScannedKey(workload, imageId);
-  return state.imagesAlreadyScanned.get(key);
+  const key = getWorkloadImageAlreadyScannedKey(workload, imageName);
+  const hasImageId = state.imagesAlreadyScanned.get(key)?.has(imageId);
+  return hasImageId ? imageId : undefined;
 }
 
 export async function setWorkloadImageAlreadyScanned(
   workload: WorkloadAlreadyScanned,
+  imageName: string,
   imageId: string,
-  value: string,
 ): Promise<boolean> {
-  const key = getWorkloadImageAlreadyScannedKey(workload, imageId);
-  return state.imagesAlreadyScanned.set(key, value);
+  const key = getWorkloadImageAlreadyScannedKey(workload, imageName);
+  const images = state.imagesAlreadyScanned.get(key);
+  if (images !== undefined) {
+    images.add(imageId);
+  } else {
+    const set = new Set<string>();
+    set.add(imageId);
+    state.imagesAlreadyScanned.set(key, set);
+  }
+
+  return true;
 }
 
 export async function deleteWorkloadImagesAlreadyScanned(
@@ -126,7 +131,9 @@ export function deleteNamespace(namespace: V1Namespace): void {
 
 export const state = {
   shutdownInProgress: false,
-  imagesAlreadyScanned: new LruCache<string, string>(imagesLruCacheOptions),
+  imagesAlreadyScanned: new LruCache<string, Set<string>>(
+    imagesLruCacheOptions,
+  ),
   workloadsAlreadyScanned: new LruCache<string, string>(
     workloadsLruCacheOptions,
   ),

src/supervisor/watchers/handlers/informer-config.ts

@@ -1,4 +1,4 @@
-import { ADD, CHANGE, DELETE, UPDATE } from '@kubernetes/client-node';
+import { ADD, DELETE, UPDATE } from '@kubernetes/client-node';
 
 import { WorkloadKind } from '../../types';
 import * as pod from './pod';
@@ -37,7 +37,6 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint: '/api/v1/namespaces/{namespace}/pods',
     handlers: {
       [ADD]: pod.podWatchHandler,
-      [CHANGE]: async () => {},
       [DELETE]: pod.podDeletedHandler,
       [UPDATE]: pod.podWatchHandler,
     },
@@ -50,10 +49,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint:
       '/api/v1/watch/namespaces/{namespace}/replicationcontrollers',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: replicationController.replicationControllerWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () =>
       replicationController.paginatedClusterReplicationControllerList(),
@@ -66,10 +62,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     clusterEndpoint: '/apis/batch/v1/cronjobs',
     namespacedEndpoint: '/apis/batch/v1/watch/namespaces/{namespace}/cronjobs',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: cronJob.cronJobWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () => cronJob.paginatedClusterCronJobList(),
     namespacedListFactory: (namespace) => () =>
@@ -80,10 +73,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint:
       '/apis/batch/v1beta1/watch/namespaces/{namespace}/cronjobs',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: cronJob.cronJobWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () =>
       cronJob.paginatedClusterCronJobV1Beta1List(),
@@ -94,10 +84,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     clusterEndpoint: '/apis/batch/v1/jobs',
     namespacedEndpoint: '/apis/batch/v1/watch/namespaces/{namespace}/jobs',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: job.jobWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () => job.paginatedClusterJobList(),
     namespacedListFactory: (namespace) => () =>
@@ -107,10 +94,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     clusterEndpoint: '/apis/apps/v1/daemonsets',
     namespacedEndpoint: '/apis/apps/v1/watch/namespaces/{namespace}/daemonsets',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: daemonSet.daemonSetWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () => daemonSet.paginatedClusterDaemonSetList(),
     namespacedListFactory: (namespace) => () =>
@@ -121,10 +105,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint:
       '/apis/apps/v1/watch/namespaces/{namespace}/deployments',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: deployment.deploymentWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () => deployment.paginatedClusterDeploymentList(),
     namespacedListFactory: (namespace) => () =>
@@ -135,10 +116,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint:
       '/apis/apps/v1/watch/namespaces/{namespace}/replicasets',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: replicaSet.replicaSetWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () => replicaSet.paginatedClusterReplicaSetList(),
     namespacedListFactory: (namespace) => () =>
@@ -149,10 +127,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint:
       '/apis/apps/v1/watch/namespaces/{namespace}/statefulsets',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: statefulSet.statefulSetWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () =>
       statefulSet.paginatedClusterStatefulSetList(),
@@ -165,10 +140,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint:
       '/apis/apps.openshift.io/v1/watch/namespaces/{namespace}/deploymentconfigs',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: deploymentConfig.deploymentConfigWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () =>
       deploymentConfig.paginatedClusterDeploymentConfigList(),
@@ -180,10 +152,7 @@ export const workloadWatchMetadata: Readonly<IWorkloadWatchMetadata> = {
     namespacedEndpoint:
       '/apis/argoproj.io/v1alpha1/watch/namespaces/{namespace}/rollouts',
     handlers: {
-      [ADD]: async () => {},
-      [CHANGE]: async () => {},
       [DELETE]: rollout.argoRolloutWatchHandler,
-      [UPDATE]: async () => {},
     },
     clusterListFactory: () => () => rollout.paginatedClusterArgoRolloutList(),
     namespacedListFactory: (namespace) => () =>

src/supervisor/watchers/handlers/pod.ts

@@ -33,6 +33,7 @@ export async function handleReadyPod(
     const scanned = await getWorkloadImageAlreadyScanned(
       workload,
       workload.imageName,
+      workload.imageId,
     );
     // ImageID contains the resolved image digest.
     // ImageName may contain a tag. The image behind this tag can be mutated and can change over time.
@@ -154,9 +155,11 @@ export async function podWatchHandler(pod: V1Pod): Promise<void> {
     const workloadRevision = workloadMember.revision
       ? workloadMember.revision.toString()
       : '';
-    const scanned = await getWorkloadAlreadyScanned(workloadMember);
-    if (scanned !== workloadRevision) {
-      // either not exists or different
+    const scannedRevision = await getWorkloadAlreadyScanned(workloadMember);
+    const isRevisionDifferent =
+      scannedRevision === undefined ||
+      Number(workloadRevision) > Number(scannedRevision);
+    if (isRevisionDifferent) {
       await setWorkloadAlreadyScanned(workloadMember, workloadRevision);
       await sendWorkloadMetadata(workloadMetadataPayload);
     }

src/supervisor/watchers/handlers/types.ts

@@ -14,6 +14,11 @@ import { IncomingMessage } from 'http';
 export const FALSY_WORKLOAD_NAME_MARKER = 'falsy workload name';
 
 export type KubernetesInformerVerb = ADD | CHANGE | DELETE | UPDATE;
+
+type WorkloadHandlers = Partial<
+  Record<KubernetesInformerVerb, WorkloadHandlerFunc>
+>;
+
 type WorkloadHandlerFunc = (workload: any) => Promise<void>;
 
 type ListNamespacedWorkloadFunctionFactory = (
@@ -32,9 +37,7 @@ export interface IWorkloadWatchMetadata {
   [workloadKind: string]: {
     clusterEndpoint: string;
     namespacedEndpoint: string;
-    handlers: {
-      [kubernetesInformerVerb in KubernetesInformerVerb]: WorkloadHandlerFunc;
-    };
+    handlers: WorkloadHandlers;
     clusterListFactory: ListClusterWorkloadFunctionFactory;
     namespacedListFactory: ListNamespacedWorkloadFunctionFactory;
   };

test/unit/scanner/scan-results-caching.spec.ts

@@ -206,12 +206,8 @@ describe('scan results caching', () => {
       const sendScanResultsMock = jest.spyOn(transmitter, 'sendScanResults');
 
       // Act
-      await state.setWorkloadAlreadyScanned(workload, undefined as any);
-      await state.setWorkloadImageAlreadyScanned(
-        workload,
-        workload.imageName,
-        undefined as any,
-      );
+      state.state.workloadsAlreadyScanned.reset();
+      state.state.imagesAlreadyScanned.reset();
 
       const workloadName = 'mock';
       const pulledImages = [];