Merge pull request #784 from snyk/feat/stable-agent-id

ivanstanev · web-flow · commit 6c38f3e9ff9b · 2021-06-03T14:41:18.000+01:00
Feat/stable agent
diff --git a/snyk-monitor-deployment.yaml b/snyk-monitor-deployment.yaml
@@ -43,12 +43,18 @@ spec:
               secretKeyRef:
                 name: snyk-monitor
                 key: integrationId
-          - name: SNYK_NAMESPACE
+          - name: SNYK_WATCH_NAMESPACE
             valueFrom:
               configMapKeyRef:
                 name: snyk-monitor
                 key: namespace
                 optional: true
+          - name: SNYK_DEPLOYMENT_NAMESPACE
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.namespace
+          - name: SNYK_DEPLOYMENT_NAME
+            value: snyk-monitor
           - name: SNYK_INTEGRATION_API
             valueFrom:
               configMapKeyRef:
diff --git a/snyk-monitor/templates/deployment.yaml b/snyk-monitor/templates/deployment.yaml
@@ -70,8 +70,14 @@ spec:
               secretKeyRef:
                 name: {{ .Values.monitorSecrets }}
                 key: integrationId
-          - name: SNYK_NAMESPACE
+          - name: SNYK_WATCH_NAMESPACE
             value: {{ include "snyk-monitor.scope" . }}
+          - name: SNYK_DEPLOYMENT_NAMESPACE
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.namespace
+          - name: SNYK_DEPLOYMENT_NAME
+            value: {{ include "snyk-monitor.name" . }}
           - name: SNYK_INTEGRATION_API
             value: {{ .Values.integrationApi }}
           - name: SNYK_CLUSTER_NAME
diff --git a/src/common/config.ts b/src/common/config.ts
@@ -18,7 +18,9 @@ function loadExcludedNamespaces(): string[] | null {
   }
 }
 
+// NOTE: The agent identifier is replaced with a stable identifier once snyk-monitor starts up
 config.AGENT_ID = uuidv4();
+
 config.INTEGRATION_ID = config.INTEGRATION_ID.trim();
 config.CLUSTER_NAME = config.CLUSTER_NAME || 'Default cluster';
 config.IMAGE_STORAGE_ROOT = '/var/tmp';
diff --git a/src/index.ts b/src/index.ts
@@ -9,6 +9,7 @@ import { currentClusterName } from './supervisor/cluster';
 import { beginWatchingWorkloads } from './supervisor/watchers';
 import { loadAndSendWorkloadEventsPolicy } from './common/policy';
 import { sendClusterMetadata } from './transmitter';
+import { setSnykMonitorAgentId } from './supervisor/agent';
 
 process.on('uncaughtException', (err) => {
   if (state.shutdownInProgress) {
@@ -63,6 +64,7 @@ cleanUpTempStorage();
 
 // Allow running in an async context
 setImmediate(async function setUpAndMonitor(): Promise<void> {
+  await setSnykMonitorAgentId();
   await sendClusterMetadata();
   await loadAndSendWorkloadEventsPolicy();
   await monitor();
diff --git a/src/supervisor/agent.ts b/src/supervisor/agent.ts
@@ -0,0 +1,34 @@
+import { config } from '../common/config';
+import { logger } from '../common/logger';
+import { k8sApi } from './cluster';
+import { retryKubernetesApiRequest } from './kuberenetes-api-wrappers';
+
+export async function setSnykMonitorAgentId(): Promise<void> {
+  const name = config.DEPLOYMENT_NAME;
+  const namespace = config.DEPLOYMENT_NAMESPACE;
+
+  const agentId = await getSnykMonitorDeploymentUid(name, namespace);
+  if (agentId === undefined) {
+    return;
+  }
+
+  config.AGENT_ID = agentId;
+}
+
+async function getSnykMonitorDeploymentUid(
+  name: string,
+  namespace: string,
+): Promise<string | undefined> {
+  try {
+    const attemptedApiCall = await retryKubernetesApiRequest(() =>
+      k8sApi.appsClient.readNamespacedDeployment(name, namespace),
+    );
+    return attemptedApiCall.body.metadata?.uid;
+  } catch (error) {
+    logger.error(
+      { error, namespace, name },
+      'could not read the snyk-monitor deployment unique id',
+    );
+    return undefined;
+  }
+}
diff --git a/src/supervisor/watchers/index.ts b/src/supervisor/watchers/index.ts
@@ -116,12 +116,12 @@ async function setupWatchesForCluster(): Promise<void> {
 }
 
 export async function beginWatchingWorkloads(): Promise<void> {
-  if (config.NAMESPACE) {
+  if (config.WATCH_NAMESPACE) {
     logger.info(
-      { namespace: config.NAMESPACE },
+      { namespace: config.WATCH_NAMESPACE },
       'kubernetes-monitor restricted to specific namespace',
     );
-    await setupWatchesForNamespace(config.NAMESPACE);
+    await setupWatchesForNamespace(config.WATCH_NAMESPACE);
     return;
   }
 
diff --git a/src/transmitter/payload.ts b/src/transmitter/payload.ts
@@ -9,7 +9,6 @@ import {
   IWorkloadMetadataPayload,
   IWorkloadMetadata,
   IWorkloadLocator,
-  IKubernetesMonitorMetadata,
   ScanResultsPayload,
   IDependencyGraphPayload,
   IWorkloadEventsPolicyPayload,
@@ -38,17 +37,10 @@ export function constructDepGraph(
       name,
     };
 
-    const monitorMetadata: IKubernetesMonitorMetadata = {
-      agentId: config.AGENT_ID,
-      namespace: config.NAMESPACE,
-      version: config.MONITOR_VERSION,
-    };
-
     return {
       imageLocator,
       agentId: config.AGENT_ID,
       dependencyGraph: JSON.stringify(scannedImage.pluginResult),
-      metadata: monitorMetadata,
     };
   });
 
@@ -78,17 +70,10 @@ export function constructScanResults(
       name,
     };
 
-    const monitorMetadata: IKubernetesMonitorMetadata = {
-      agentId: config.AGENT_ID,
-      namespace: config.NAMESPACE,
-      version: config.MONITOR_VERSION,
-    };
-
     return {
       imageLocator,
       agentId: config.AGENT_ID,
       scanResults: scannedImage.scanResults,
-      metadata: monitorMetadata,
     };
   });
 }
diff --git a/src/transmitter/types.ts b/src/transmitter/types.ts
@@ -31,25 +31,16 @@ export interface IImageLocator extends IWorkloadLocator {
   imageWithDigest?: string;
 }
 
-export interface IKubernetesMonitorMetadata {
-  agentId: string;
-  version: string;
-  namespace?: string;
-}
-
 export interface IDependencyGraphPayload {
   imageLocator: IImageLocator;
   agentId: string;
   dependencyGraph?: string;
-  metadata: IKubernetesMonitorMetadata;
 }
 
 export interface ScanResultsPayload {
   imageLocator: IImageLocator;
   agentId: string;
   scanResults: ScanResult[];
-  /** @deprecated TODO: This should be sent in a separate API. */
-  metadata: IKubernetesMonitorMetadata;
 }
 
 export interface IWorkloadMetadataPayload {
diff --git a/test/fixtures/pod-spec.json b/test/fixtures/pod-spec.json
@@ -12,7 +12,7 @@
           }
         },
         {
-          "name": "SNYK_NAMESPACE"
+          "name": "SNYK_WATCH_NAMESPACE"
         },
         {
           "name": "SNYK_INTEGRATION_API"
diff --git a/test/setup/deployers/yaml.ts b/test/setup/deployers/yaml.ts
@@ -34,14 +34,18 @@ function createTestYamlDeployment(
   );
   const deployment = parse(originalDeploymentYaml);
 
-  deployment.spec.template.spec.containers[0].image = imageNameAndTag;
-  deployment.spec.template.spec.containers[0].imagePullPolicy = imagePullPolicy;
+  const container = deployment.spec.template.spec.containers.find(
+    (container) => container.name === 'snyk-monitor',
+  );
+  container.image = imageNameAndTag;
+  container.imagePullPolicy = imagePullPolicy;
 
   // Inject the baseUrl of kubernetes-upstream that snyk-monitor container use to send metadata
-  deployment.spec.template.spec.containers[0].env[2] = {
-    name: 'SNYK_INTEGRATION_API',
-    value: 'https://kubernetes-upstream.dev.snyk.io',
-  };
+  const envVar = container.env.find(
+    (env) => env.name === 'SNYK_INTEGRATION_API',
+  );
+  envVar.value = 'https://kubernetes-upstream.dev.snyk.io';
+  delete envVar.valueFrom;
 
   writeFileSync(newYamlPath, stringify(deployment));
   console.log('Created YAML snyk-monitor deployment');
diff --git a/test/system/kind.spec.ts b/test/system/kind.spec.ts
@@ -3,6 +3,7 @@ import * as nock from 'nock';
 import { copyFile, readFile, mkdir, exists } from 'fs';
 import { promisify } from 'util';
 import { resolve as resolvePath } from 'path';
+import { v4 as uuid } from 'uuid';
 
 import * as kubectl from '../helpers/kubectl';
 import * as kind from '../setup/platforms/kind';
@@ -25,6 +26,7 @@ const existsAsync = promisify(exists);
  *   Error: Client network socket disconnected before secure TLS connection was established
  */
 import { state as kubernetesMonitorState } from '../../src/state';
+import * as kubernetesApiWrappers from '../../src/supervisor/kuberenetes-api-wrappers';
 
 async function tearDown() {
   console.log('Begin removing the snyk-monitor...');
@@ -38,6 +40,8 @@ async function tearDown() {
 
 beforeAll(tearDown);
 afterAll(async () => {
+  jest.restoreAllMocks();
+
   kubernetesMonitorState.shutdownInProgress = true;
   await tearDown();
   // TODO cleanup the images we saved to /var/tmp?
@@ -48,6 +52,17 @@ test('Kubernetes-Monitor with KinD', async (jestDoneCallback) => {
     .spyOn(fsExtra, 'emptyDirSync')
     .mockReturnValue({});
 
+  const agentId = uuid();
+  const retryKubernetesApiRequestMock = jest
+    .spyOn(kubernetesApiWrappers, 'retryKubernetesApiRequest')
+    .mockResolvedValueOnce({
+      body: {
+        metadata: {
+          uid: agentId,
+        },
+      },
+    });
+
   try {
     await exec('which skopeo');
     console.log('Skopeo already installed :tada:');
@@ -102,7 +117,7 @@ test('Kubernetes-Monitor with KinD', async (jestDoneCallback) => {
           expect(
             requestBody,
           ).toEqual<transmitterTypes.IWorkloadEventsPolicyPayload>({
-            agentId: expect.any(String),
+            agentId,
             cluster: expect.any(String),
             userLocator: expect.any(String),
             policy: regoPolicyContents,
@@ -123,7 +138,7 @@ test('Kubernetes-Monitor with KinD', async (jestDoneCallback) => {
           expect(requestBody).toEqual<
             Partial<transmitterTypes.IClusterMetadataPayload>
           >({
-            agentId: expect.any(String),
+            agentId,
             cluster: expect.any(String),
             userLocator: expect.any(String),
             // also should have version here but due to test limitation it is undefined
@@ -199,7 +214,7 @@ test('Kubernetes-Monitor with KinD', async (jestDoneCallback) => {
                 ]),
               }),
             }),
-            agentId: expect.any(String),
+            agentId,
           });
         } catch (error) {
           jestDoneCallback(error);
@@ -230,8 +245,7 @@ test('Kubernetes-Monitor with KinD', async (jestDoneCallback) => {
     .reply(500, (uri, requestBody: transmitterTypes.ScanResultsPayload) => {
       try {
         expect(requestBody).toEqual<transmitterTypes.ScanResultsPayload>({
-          metadata: expect.any(Object),
-          agentId: expect.any(String),
+          agentId,
           imageLocator: expect.objectContaining({
             imageId: expect.any(String),
           }),
@@ -277,7 +291,7 @@ test('Kubernetes-Monitor with KinD', async (jestDoneCallback) => {
         try {
           expect(requestBody).toEqual<transmitterTypes.IDependencyGraphPayload>(
             {
-              agentId: expect.any(String),
+              agentId,
               dependencyGraph: expect.stringContaining('docker-image|java'),
               imageLocator: {
                 userLocator: expect.any(String),
@@ -288,11 +302,10 @@ test('Kubernetes-Monitor with KinD', async (jestDoneCallback) => {
                 type: expect.any(String),
                 imageWithDigest: expect.any(String),
               },
-              metadata: expect.objectContaining({
-                agentId: expect.any(String),
-              }),
             },
           );
+
+          expect(retryKubernetesApiRequestMock).toHaveBeenCalled();
           jestDoneCallback();
         } catch (error) {
           jestDoneCallback(error);
diff --git a/test/unit/transmitter-payload.spec.ts b/test/unit/transmitter-payload.spec.ts
@@ -101,10 +101,10 @@ describe('transmitter payload tests', () => {
     // These values are populated at runtime (injected by the deployment) so we have to mock them
     // to make sure the function uses them to construct the payload (otherwise they are undefined).
     const backups = {
-      namespace: config.NAMESPACE,
+      namespace: config.WATCH_NAMESPACE,
       version: config.MONITOR_VERSION,
     };
-    config.NAMESPACE = 'b7';
+    config.WATCH_NAMESPACE = 'b7';
     config.MONITOR_VERSION = '1.2.3';
 
     const payloads = payload.constructScanResults(
@@ -125,13 +125,8 @@ describe('transmitter payload tests', () => {
         type: 'type',
       }),
     );
-    expect(firstPayload.metadata).toEqual({
-      agentId: config.AGENT_ID,
-      namespace: 'b7',
-      version: '1.2.3',
-    });
 
-    config.NAMESPACE = backups.namespace;
+    config.WATCH_NAMESPACE = backups.namespace;
     config.MONITOR_VERSION = backups.version;
   });
 

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,9 @@ function loadExcludedNamespaces(): string[] \| null {`
`18`	`18`	`}`
`19`	`19`	`}`
`20`	`20`
	`21`	`+// NOTE: The agent identifier is replaced with a stable identifier once snyk-monitor starts up`
`21`	`22`	`config.AGENT_ID = uuidv4();`
	`23`	`+`
`22`	`24`	`config.INTEGRATION_ID = config.INTEGRATION_ID.trim();`
`23`	`25`	`config.CLUSTER_NAME = config.CLUSTER_NAME \|\| 'Default cluster';`
`24`	`26`	`config.IMAGE_STORAGE_ROOT = '/var/tmp';`
Original file line number	Diff line number	Diff line change
`@@ -116,12 +116,12 @@ async function setupWatchesForCluster(): Promise<void> {`
`116`	`116`	`}`
`117`	`117`
`118`	`118`	`export async function beginWatchingWorkloads(): Promise<void> {`
`119`		`- if (config.NAMESPACE) {`
	`119`	`+ if (config.WATCH_NAMESPACE) {`
`120`	`120`	`logger.info(`
`121`		`- { namespace: config.NAMESPACE },`
	`121`	`+ { namespace: config.WATCH_NAMESPACE },`
`122`	`122`	`'kubernetes-monitor restricted to specific namespace',`
`123`	`123`	`);`
`124`		`- await setupWatchesForNamespace(config.NAMESPACE);`
	`124`	`+ await setupWatchesForNamespace(config.WATCH_NAMESPACE);`
`125`	`125`	`return;`
`126`	`126`	`}`
`127`	`127`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@`
`12`	`12`	`}`
`13`	`13`	`},`
`14`	`14`	`{`
`15`		`- "name": "SNYK_NAMESPACE"`
	`15`	`+ "name": "SNYK_WATCH_NAMESPACE"`
`16`	`16`	`},`
`17`	`17`	`{`
`18`	`18`	`"name": "SNYK_INTEGRATION_API"`