Implement split fetch/update phases with separate containers and proxies

__tests__/container-service.test.ts

@@ -52,4 +52,94 @@ describe('ContainerService', () => {
       )
     })
   })
+
+  describe('updaterCommands', () => {
+    test('the fetch phase clones and copies the checkout to the handoff volume', () => {
+      expect(ContainerService.updaterCommands('fetch')).toEqual([
+        'mkdir -p /home/dependabot/dependabot-updater/output',
+        '$DEPENDABOT_HOME/dependabot-updater/bin/run fetch_files',
+        'cp -a /home/dependabot/dependabot-updater/repo/. /home/dependabot/dependabot-updater/repo-handoff/'
+      ])
+    })
+
+    test('the update phase only runs the file updater', () => {
+      expect(ContainerService.updaterCommands('update')).toEqual([
+        'mkdir -p /home/dependabot/dependabot-updater/output',
+        '$DEPENDABOT_HOME/dependabot-updater/bin/run update_files'
+      ])
+    })
+
+    test('the update phase honours the graph command', () => {
+      expect(ContainerService.updaterCommands('update', 'graph')).toEqual([
+        'mkdir -p /home/dependabot/dependabot-updater/output',
+        '$DEPENDABOT_HOME/dependabot-updater/bin/run update_graph'
+      ])
+    })
+
+    test('the all phase lets update_files fetch in-process', () => {
+      expect(ContainerService.updaterCommands('all')).toEqual([
+        'mkdir -p /home/dependabot/dependabot-updater/output',
+        '$DEPENDABOT_HOME/dependabot-updater/bin/run update_files'
+      ])
+    })
+  })
+
+  describe('the fetch phase', () => {
+    test('prepares the repository volume for the dependabot user', async () => {
+      const dependabotContainer: any = {
+        id: 'fetch-container',
+        start: jest.fn().mockResolvedValue(undefined),
+        inspect: jest.fn().mockResolvedValue({
+          Config: {Env: ['DEPENDABOT_JOB_ID=1']}
+        }),
+        remove: jest.fn().mockResolvedValue(undefined)
+      }
+      const execCommand = jest
+        .spyOn(ContainerService, 'execCommand')
+        .mockResolvedValue(undefined)
+
+      await ContainerService.runFileFetcher(dependabotContainer)
+
+      expect(execCommand.mock.calls).toEqual([
+        [dependabotContainer, ['/usr/sbin/update-ca-certificates'], 'root'],
+        [
+          dependabotContainer,
+          [
+            'chown',
+            'dependabot',
+            '/home/dependabot/dependabot-updater/repo',
+            '/home/dependabot/dependabot-updater/repo-handoff'
+          ],
+          'root'
+        ],
+        [
+          dependabotContainer,
+          [
+            '/bin/sh',
+            '-c',
+            'mkdir -p /home/dependabot/dependabot-updater/output'
+          ],
+          'dependabot'
+        ],
+        [
+          dependabotContainer,
+          [
+            '/bin/sh',
+            '-c',
+            '$DEPENDABOT_HOME/dependabot-updater/bin/run fetch_files'
+          ],
+          'dependabot'
+        ],
+        [
+          dependabotContainer,
+          [
+            '/bin/sh',
+            '-c',
+            'cp -a /home/dependabot/dependabot-updater/repo/. /home/dependabot/dependabot-updater/repo-handoff/'
+          ],
+          'dependabot'
+        ]
+      ])
+    })
+  })
 })

__tests__/proxy-integration.test.ts

@@ -82,6 +82,54 @@ integration('ProxyBuilder', () => {
     await proxy.shutdown()
   })
 
+  jest.setTimeout(20000)
+  it('namespaces the container and networks per phase', async () => {
+    const fetchProxy = await builder.run(
+      jobId,
+      jobToken,
+      dependabotApiUrl,
+      credentials,
+      'fetch'
+    )
+    await fetchProxy.container.start()
+
+    const updateProxy = await builder.run(
+      jobId,
+      jobToken,
+      dependabotApiUrl,
+      [],
+      'update'
+    )
+    await updateProxy.container.start()
+
+    const fetchInfo = await fetchProxy.container.inspect()
+    const updateInfo = await updateProxy.container.inspect()
+
+    expect(fetchInfo.Name).toBe('/dependabot-job-1-fetch-proxy')
+    expect(updateInfo.Name).toBe('/dependabot-job-1-update-proxy')
+
+    expect(fetchProxy.networkName).toBe(
+      'dependabot-job-1-fetch-internal-network'
+    )
+    expect(updateProxy.networkName).toBe(
+      'dependabot-job-1-update-internal-network'
+    )
+
+    // Each phase gets its own networks so that the two proxies are reachable
+    // only from their own phase's container.
+    expect(Object.keys(fetchInfo.NetworkSettings.Networks)).toEqual([
+      'dependabot-job-1-fetch-external-network',
+      'dependabot-job-1-fetch-internal-network'
+    ])
+    expect(Object.keys(updateInfo.NetworkSettings.Networks)).toEqual([
+      'dependabot-job-1-update-external-network',
+      'dependabot-job-1-update-internal-network'
+    ])
+
+    await updateProxy.shutdown()
+    await fetchProxy.shutdown()
+  })
+
   jest.setTimeout(20000)
   it('copies in a custom root CA if configured', async () => {
     // make a tmp dir at the repo root unless it already exists

__tests__/updater-builder.test.ts

@@ -110,4 +110,135 @@ describe('UpdaterBuilder', () => {
       })
     )
   })
+
+  it('sets UPDATER_ONE_CONTAINER when both phases share a container', async () => {
+    mockExtractUpdaterSha.mockReturnValue(null)
+
+    const updaterBuilder = new UpdaterBuilder(
+      mockDocker,
+      jobParams,
+      input,
+      mockProxy,
+      'test-image'
+    )
+
+    await updaterBuilder.run('test-container')
+
+    expect(mockCreateContainer).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Env: expect.arrayContaining(['UPDATER_ONE_CONTAINER=1'])
+      })
+    )
+  })
+
+  it.each(['fetch', 'update'] as const)(
+    'does not set UPDATER_ONE_CONTAINER for the %s phase',
+    async phase => {
+      mockExtractUpdaterSha.mockReturnValue(null)
+
+      const updaterBuilder = new UpdaterBuilder(
+        mockDocker,
+        jobParams,
+        input,
+        mockProxy,
+        'test-image',
+        phase
+      )
+
+      await updaterBuilder.run('test-container')
+
+      expect(mockCreateContainer).toHaveBeenCalledWith(
+        expect.objectContaining({
+          Env: expect.not.arrayContaining([
+            expect.stringMatching(/UPDATER_ONE_CONTAINER/)
+          ])
+        })
+      )
+    }
+  )
+
+  it('mounts clone and handoff volumes in the fetch phase', async () => {
+    mockExtractUpdaterSha.mockReturnValue(null)
+
+    const updaterBuilder = new UpdaterBuilder(
+      mockDocker,
+      jobParams,
+      input,
+      mockProxy,
+      'test-image',
+      'fetch',
+      'clone-volume',
+      'handoff-volume'
+    )
+
+    await updaterBuilder.run('test-container')
+
+    expect(mockCreateContainer).toHaveBeenCalledWith(
+      expect.objectContaining({
+        HostConfig: expect.objectContaining({
+          Mounts: [
+            {
+              Type: 'volume',
+              Source: 'clone-volume',
+              Target: '/home/dependabot/dependabot-updater/repo'
+            },
+            {
+              Type: 'volume',
+              Source: 'handoff-volume',
+              Target: '/home/dependabot/dependabot-updater/repo-handoff'
+            }
+          ]
+        })
+      })
+    )
+  })
+
+  it('requires a local checkout only in the update phase', async () => {
+    mockExtractUpdaterSha.mockReturnValue(null)
+
+    const updaterBuilder = new UpdaterBuilder(
+      mockDocker,
+      jobParams,
+      input,
+      mockProxy,
+      'test-image',
+      'update',
+      'repo-volume'
+    )
+
+    await updaterBuilder.run('test-container')
+
+    expect(mockCreateContainer).toHaveBeenCalledWith(
+      expect.objectContaining({
+        Env: expect.arrayContaining(['DEPENDABOT_LOCAL_CHECKOUT_ONLY=true'])
+      })
+    )
+  })
+
+  it.each(['all', 'fetch'] as const)(
+    'does not require a local checkout in the %s phase',
+    async phase => {
+      mockExtractUpdaterSha.mockReturnValue(null)
+
+      const updaterBuilder = new UpdaterBuilder(
+        mockDocker,
+        jobParams,
+        input,
+        mockProxy,
+        'test-image',
+        phase,
+        phase === 'fetch' ? 'repo-volume' : undefined
+      )
+
+      await updaterBuilder.run('test-container')
+
+      expect(mockCreateContainer).toHaveBeenCalledWith(
+        expect.objectContaining({
+          Env: expect.not.arrayContaining([
+            'DEPENDABOT_LOCAL_CHECKOUT_ONLY=true'
+          ])
+        })
+      )
+    }
+  )
 })

__tests__/updater-integration.test.ts

@@ -45,9 +45,6 @@ integration('Updater', () => {
   beforeAll(async () => {
     await ImageService.pull(updaterImageName('npm_and_yarn'))
     await ImageService.pull(PROXY_IMAGE_NAME)
-
-    const testRetry = true
-    server = await runFakeDependabotApi(FAKE_SERVER_PORT, testRetry)
   })
 
   afterEach(async () => {
@@ -57,6 +54,9 @@ integration('Updater', () => {
 
   jest.setTimeout(120000)
   it('should run the updater, retry on apiClient failure, and create a pull request', async () => {
+    const testRetry = true
+    server = await runFakeDependabotApi(FAKE_SERVER_PORT, testRetry)
+
     const details = await apiClient.getJobDetails()
     const credentials = await apiClient.getCredentials()
 
@@ -79,4 +79,50 @@ integration('Updater', () => {
       'Bump fetch-factory from 0.0.1 to 0.2.1'
     )
   })
+
+  jest.setTimeout(120000)
+  const splitUpdaterImage = process.env.DEPENDABOT_SPLIT_TEST_UPDATER_IMAGE
+  const splitIntegration = splitUpdaterImage ? it : it.skip
+
+  // Use a locally patched image until the pinned updater image contains the
+  // Core fetch_files entrypoint and local-checkout-only behavior.
+  splitIntegration(
+    'should create the same pull request when the phases are split',
+    async () => {
+      if (!splitUpdaterImage) {
+        throw new Error('DEPENDABOT_SPLIT_TEST_UPDATER_IMAGE is required')
+      }
+
+      // Each test gets its own server, as afterEach tears the previous one down.
+      server = await runFakeDependabotApi(FAKE_SERVER_PORT)
+
+      const details = await apiClient.getJobDetails()
+      const credentials = await apiClient.getCredentials()
+
+      const updater = new Updater(
+        splitUpdaterImage,
+        PROXY_IMAGE_NAME,
+        apiClient,
+        {
+          ...details,
+          experiments: {
+            ...details.experiments,
+            isolate_fetch_update: true
+          }
+        },
+        credentials
+      )
+
+      await updater.runUpdater()
+
+      const res = await client.getJson<any>(
+        `${dependabotApiUrl}/pull_requests/1`
+      )
+
+      expect(res.statusCode).toEqual(200)
+      expect(res.result['pr-title']).toEqual(
+        'Bump fetch-factory from 0.0.1 to 0.2.1'
+      )
+    }
+  )
 })