feat: Experimentally support storage in PVC

[RUN-1250] Update helm templates to allow for `pvc` values, which when enabled replaces emptyDir with a PVC using a PV which needs to have been provisioned separately. At startup, ensure the temp dir is empty, to avoid inheriting a volume with existing data.

Co-authored-by @ivanstanev

Author: michael-c-snyk

README.md

@@ -148,6 +148,15 @@ To lower `snyk-monitor`'s logging verbosity `log_level` value could be set to on
 
 By default, `log_level` is `'INFO'`.
 
+## Using a PVC ##
+
+By default, `snyk-monitor` uses an emptyDir for temporary storage. If you prefer to have a PVC that uses a statically or
+ dynamically provisioned PV that you have created, then set the following value
+* `pvc.enabled` `true`
+
+The PVC's name defaults to `snyk-monitor-pvc`. If you prefer to override this, then use the following value:
+* `pvc.name`
+
 ## Terms and conditions ##
 
 *The Snyk Container Kubernetes integration uses Red Hat UBI (Universal Base Image).*

package-lock.json

@@ -39,6 +39,7 @@
     "aws-sdk": "^2.633.0",
     "bunyan": "^1.8.13",
     "child-process-promise": "^2.2.1",
+    "fs-extra": "^9.0.1",
     "lru-cache": "^5.1.1",
     "needle": "^2.5.0",
     "sleep-promise": "^8.0.1",

package.json

@@ -142,6 +142,15 @@ helm upgrade --install snyk-monitor snyk-charts/snyk-monitor \
   --set log_level="WARN"
 ```
 
+## Using a PVC ##
+
+By default, `snyk-monitor` uses an emptyDir for temporary storage. If you prefer to have a PVC that uses a statically or
+ dynamically provisioned PV that you have created, then set the following value
+* `pvc.enabled` `true`
+
+The PVC's name defaults to `snyk-monitor-pvc`. If you prefer to override this, then use the following value:
+* `pvc.name`
+
 ## PodSecurityPolicies
 **This should not be used when installing on OpenShift.**
 

snyk-monitor/README.md

@@ -20,6 +20,13 @@ spec:
     spec:
       serviceAccountName: {{ include "snyk-monitor.name" . }}
       restartPolicy: Always
+      initContainers:
+        - name: volume-permissions
+          image: "{{ .Values.initContainerImage.repository }}:{{ .Values.initContainerImage.tag }}"
+          command : ['sh', '-c', 'chmod -R 777 /var/tmp']
+          volumeMounts:
+            - name: temporary-storage
+              mountPath: "/var/tmp"
       containers:
         - name: {{ include "snyk-monitor.name" . }}
           image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}"
@@ -83,8 +90,13 @@ spec:
               - key: dockercfg.json
                 path: config.json
         - name: temporary-storage
+          {{- if .Values.pvc.enabled }}
+          persistentVolumeClaim:
+            claimName: {{ .Values.pvc.name }}
+          {{- else }}
           emptyDir:
             sizeLimit: {{ .Values.temporaryStorageSize }}
+          {{- end }}
         - name: ssl-certs
           configMap:
             name: {{ .Values.certsConfigMap }}

snyk-monitor/templates/deployment.yaml

@@ -0,0 +1,13 @@
+{{ if .Values.pvc.enabled }}
+kind: PersistentVolumeClaim
+apiVersion: v1
+metadata:
+  name: {{ .Values.pvc.name }}
+spec:
+  accessModes:
+    - ReadWriteOnce
+  volumeMode: Filesystem
+  resources:
+    requests:
+      storage: {{ .Values.temporaryStorageSize }}
+{{ end }}

snyk-monitor/templates/pvc.yaml

@@ -22,14 +22,24 @@ image:
   tag: IMAGE_TAG_OVERRIDE_WHEN_PUBLISHING
   pullPolicy: Always
 
+# If deploying in an air-gapped environment that can't pull from DockerHub, override the initContainer's image here for one that is accessible to your environment.
+initContainerImage:
+  repository: busybox
+  tag: latest
+
 # The snyk-monitor requires knowing the cluster name so that it can organise
 # scanned workloads. The Kubernetes API does not provide an API to query this.
 # Set the name of the cluster, otherwise the snyk-monitor will set this to a default value.
 clusterName: ""
 
 # The snyk-monitor requires disk storage to temporarily pull container images and to scan them for vulnerabilities.
-# This value controls how much disk storage _at most_ may be allocated for the snyk-monitor. The snyk-monitor mounts an emptyDir for storage.
-temporaryStorageSize: 50Gi
+# This value controls how much disk storage _at most_ may be allocated for the snyk-monitor. Unless overridden by the `pvc` value, the snyk-monitor mounts an emptyDir for storage.
+temporaryStorageSize: 50Gi #  Applies to PVC too
+
+# Change to true to use a PVC instead of emptyDir for local storage
+pvc:
+  enabled: false
+  name: 'snyk-monitor-pvc'
 
 # CPU/Mem requests and limits for snyk-monitor
 requests:

snyk-monitor/values.yaml

@@ -1,6 +1,9 @@
+import { emptyDirSync } from 'fs-extra';
+
 import * as SourceMapSupport from 'source-map-support';
 
 import * as state from './state';
+import * as config from './common/config';
 import logger = require('./common/logger');
 import { currentClusterName } from './supervisor/cluster';
 import { beginWatchingWorkloads } from './supervisor/watchers';
@@ -33,6 +36,16 @@ process.on('unhandledRejection', (reason) => {
   }
 });
 
+function cleanUpTempStorage() {
+  const { IMAGE_STORAGE_ROOT } = config;
+  try {
+    emptyDirSync(IMAGE_STORAGE_ROOT);
+    logger.info({}, 'Cleaned temp storage');
+  } catch (err) {
+    logger.error({ err }, 'Error deleting files');
+  }
+};
+
 function monitor(): void {
   try {
     logger.info({cluster: currentClusterName}, 'starting to monitor');
@@ -44,4 +57,5 @@ function monitor(): void {
 }
 
 SourceMapSupport.install();
+cleanUpTempStorage();
 monitor();

src/index.ts

@@ -1,3 +1,5 @@
+import * as sinon from 'sinon';
+import * as fsExtra from 'fs-extra';
 import * as tap from 'tap';
 import * as nock from 'nock';
 import * as sleep from 'sleep-promise';
@@ -19,6 +21,8 @@ tap.tearDown(tearDown);
 
 tap.test('Kubernetes-Monitor with KinD', async (t) => {
 
+  const emptyDirSyncStub = sinon.stub(fsExtra, 'emptyDirSync').returns({});
+
   // Start fresh
   try {
     await tearDown();
@@ -162,6 +166,8 @@ tap.test('Kubernetes-Monitor with KinD', async (t) => {
   // Start the monitor
   require('../../src');
 
+  sinon.assert.called(emptyDirSyncStub);
+
   // TODO: replace with being event driven?
   // will still need SOME timeout 
   while (true) {