-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EDR Workflows] Improve agent downloader #196135
Changes from 4 commits
b02fad8
9dd496a
2b744f9
a118cec
1ccd0c8
60ff27e
7761c26
024fb38
3bc96a0
588e95e
a8d551d
955f62d
b67dc7d
2b80dc3
159c7ad
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,203 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
import { getAgentDownloadUrl, getAgentFileName } from '../common/fleet_services'; | ||
import { downloadAndStoreAgent } from '../common/agent_downloads_service'; | ||
import type { ToolingLog } from '@kbn/tooling-log'; | ||
import { agentDownloaderRunner } from './agent_downloader'; | ||
import type { RunContext } from '@kbn/dev-cli-runner'; | ||
|
||
jest.mock('../common/fleet_services'); | ||
jest.mock('../common/agent_downloads_service'); | ||
|
||
describe('agentDownloaderRunner', () => { | ||
let log: ToolingLog; | ||
|
||
beforeEach(() => { | ||
log = { | ||
info: jest.fn(), | ||
error: jest.fn(), | ||
} as unknown as ToolingLog; | ||
|
||
jest.clearAllMocks(); | ||
}); | ||
|
||
const version = '8.15.0'; | ||
let closestMatch = false; | ||
const url = 'http://example.com/agent.tar.gz'; | ||
const fileName = 'elastic-agent-8.15.0.tar.gz'; | ||
|
||
it('downloads and stores the specified version', async () => { | ||
(getAgentDownloadUrl as jest.Mock).mockResolvedValue({ url }); | ||
(getAgentFileName as jest.Mock).mockReturnValue('elastic-agent-8.15.0'); | ||
(downloadAndStoreAgent as jest.Mock).mockResolvedValue(undefined); | ||
|
||
await agentDownloaderRunner({ | ||
flags: { version, closestMatch }, | ||
log, | ||
} as unknown as RunContext); | ||
|
||
expect(getAgentDownloadUrl).toHaveBeenCalledWith(version, closestMatch, log); | ||
expect(getAgentFileName).toHaveBeenCalledWith(version); | ||
expect(downloadAndStoreAgent).toHaveBeenCalledWith(url, fileName); | ||
expect(log.info).toHaveBeenCalledWith('Successfully downloaded and stored version 8.15.0'); | ||
}); | ||
|
||
it('logs an error if the download fails', async () => { | ||
(getAgentDownloadUrl as jest.Mock).mockResolvedValue({ url }); | ||
(getAgentFileName as jest.Mock).mockReturnValue('elastic-agent-8.15.0'); | ||
(downloadAndStoreAgent as jest.Mock).mockRejectedValue(new Error('Download failed')); | ||
|
||
await agentDownloaderRunner({ | ||
flags: { version, closestMatch }, | ||
log, | ||
} as unknown as RunContext); | ||
|
||
expect(getAgentDownloadUrl).toHaveBeenCalledWith(version, closestMatch, log); | ||
expect(getAgentFileName).toHaveBeenCalledWith(version); | ||
expect(downloadAndStoreAgent).toHaveBeenCalledWith(url, fileName); | ||
expect(log.error).toHaveBeenCalledWith( | ||
'Failed to download or store version 8.15.0: Download failed' | ||
); | ||
}); | ||
|
||
it('downloads and stores the previous patch version if the specified version fails', async () => { | ||
const fallbackVersion = '8.15.0'; | ||
const fallbackFileName = 'elastic-agent-8.15.0.tar.gz'; | ||
|
||
(getAgentDownloadUrl as jest.Mock) | ||
.mockResolvedValueOnce({ url }) | ||
.mockResolvedValueOnce({ url }); | ||
(getAgentFileName as jest.Mock) | ||
.mockReturnValueOnce('elastic-agent-8.15.1') | ||
.mockReturnValueOnce('elastic-agent-8.15.0'); | ||
(downloadAndStoreAgent as jest.Mock) | ||
.mockRejectedValueOnce(new Error('Download failed')) | ||
.mockResolvedValueOnce(undefined); | ||
|
||
await agentDownloaderRunner({ | ||
flags: { version: '8.15.1', closestMatch }, | ||
log, | ||
} as unknown as RunContext); | ||
|
||
expect(getAgentDownloadUrl).toHaveBeenCalledWith('8.15.1', closestMatch, log); | ||
expect(getAgentDownloadUrl).toHaveBeenCalledWith(fallbackVersion, closestMatch, log); | ||
expect(getAgentFileName).toHaveBeenCalledWith('8.15.1'); | ||
expect(getAgentFileName).toHaveBeenCalledWith(fallbackVersion); | ||
expect(downloadAndStoreAgent).toHaveBeenCalledWith(url, 'elastic-agent-8.15.1.tar.gz'); | ||
expect(downloadAndStoreAgent).toHaveBeenCalledWith(url, fallbackFileName); | ||
expect(log.error).toHaveBeenCalledWith( | ||
'Failed to download or store version 8.15.1: Download failed' | ||
); | ||
expect(log.info).toHaveBeenCalledWith('Successfully downloaded and stored version 8.15.0'); | ||
}); | ||
|
||
it('logs an error if all downloads fail', async () => { | ||
(getAgentDownloadUrl as jest.Mock).mockResolvedValue({ url }); | ||
(getAgentFileName as jest.Mock) | ||
.mockReturnValueOnce('elastic-agent-8.15.1') | ||
.mockReturnValueOnce('elastic-agent-8.15.0'); | ||
(downloadAndStoreAgent as jest.Mock) | ||
.mockRejectedValueOnce(new Error('Download failed')) | ||
.mockRejectedValueOnce(new Error('Download failed')); | ||
|
||
await agentDownloaderRunner({ | ||
flags: { version: '8.15.1', closestMatch }, | ||
log, | ||
} as unknown as RunContext); | ||
|
||
expect(getAgentDownloadUrl).toHaveBeenCalledWith('8.15.1', closestMatch, log); | ||
expect(getAgentDownloadUrl).toHaveBeenCalledWith('8.15.0', closestMatch, log); | ||
expect(getAgentFileName).toHaveBeenCalledWith('8.15.1'); | ||
expect(getAgentFileName).toHaveBeenCalledWith('8.15.0'); | ||
expect(downloadAndStoreAgent).toHaveBeenCalledWith(url, 'elastic-agent-8.15.1.tar.gz'); | ||
expect(downloadAndStoreAgent).toHaveBeenCalledWith(url, 'elastic-agent-8.15.0.tar.gz'); | ||
expect(log.error).toHaveBeenCalledWith( | ||
'Failed to download or store version 8.15.1: Download failed' | ||
); | ||
expect(log.error).toHaveBeenCalledWith( | ||
'Failed to download or store version 8.15.0: Download failed' | ||
); | ||
}); | ||
|
||
it('does not attempt fallback when patch version is 0', async () => { | ||
(getAgentDownloadUrl as jest.Mock).mockResolvedValue({ url }); | ||
(getAgentFileName as jest.Mock).mockReturnValue('elastic-agent-8.15.0'); | ||
(downloadAndStoreAgent as jest.Mock).mockResolvedValue(undefined); | ||
|
||
await agentDownloaderRunner({ | ||
flags: { version: '8.15.0', closestMatch }, | ||
log, | ||
} as unknown as RunContext); | ||
|
||
expect(getAgentDownloadUrl).toHaveBeenCalledTimes(1); // Only one call for 8.15.0 | ||
expect(getAgentFileName).toHaveBeenCalledTimes(1); | ||
expect(downloadAndStoreAgent).toHaveBeenCalledWith(url, fileName); | ||
expect(log.info).toHaveBeenCalledWith('Successfully downloaded and stored version 8.15.0'); | ||
}); | ||
|
||
it('logs an error for an invalid version format', async () => { | ||
const invalidVersion = '7.x.x'; | ||
|
||
await expect( | ||
agentDownloaderRunner({ | ||
flags: { version: invalidVersion, closestMatch }, | ||
log, | ||
} as unknown as RunContext) | ||
).rejects.toThrow('Invalid version format'); | ||
}); | ||
|
||
it('passes the closestMatch flag correctly', async () => { | ||
closestMatch = true; | ||
|
||
(getAgentDownloadUrl as jest.Mock).mockResolvedValue({ url }); | ||
(getAgentFileName as jest.Mock).mockReturnValue('elastic-agent-8.15.0'); | ||
(downloadAndStoreAgent as jest.Mock).mockResolvedValue(undefined); | ||
|
||
await agentDownloaderRunner({ | ||
flags: { version, closestMatch }, | ||
log, | ||
} as unknown as RunContext); | ||
|
||
expect(getAgentDownloadUrl).toHaveBeenCalledWith(version, closestMatch, log); | ||
}); | ||
|
||
it('throws an error when version is not provided', async () => { | ||
await expect( | ||
agentDownloaderRunner({ | ||
flags: { closestMatch }, | ||
log, | ||
} as unknown as RunContext) | ||
).rejects.toThrow('version argument is required'); | ||
}); | ||
|
||
it('logs the correct messages when both version and fallback version are processed', async () => { | ||
const primaryVersion = '8.15.1'; | ||
|
||
(getAgentDownloadUrl as jest.Mock) | ||
.mockResolvedValueOnce({ url }) | ||
.mockResolvedValueOnce({ url }); | ||
|
||
(getAgentFileName as jest.Mock) | ||
.mockReturnValueOnce('elastic-agent-8.15.1') | ||
.mockReturnValueOnce('elastic-agent-8.15.0'); | ||
|
||
(downloadAndStoreAgent as jest.Mock) | ||
.mockRejectedValueOnce(new Error('Download failed')) // Fail on primary | ||
.mockResolvedValueOnce(undefined); // Success on fallback | ||
|
||
await agentDownloaderRunner({ | ||
flags: { version: primaryVersion, closestMatch }, | ||
log, | ||
} as unknown as RunContext); | ||
|
||
expect(log.error).toHaveBeenCalledWith( | ||
'Failed to download or store version 8.15.1: Download failed' | ||
); | ||
expect(log.info).toHaveBeenCalledWith('Successfully downloaded and stored version 8.15.0'); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,21 +11,63 @@ import type { ToolingLog } from '@kbn/tooling-log'; | |
import { getAgentDownloadUrl, getAgentFileName } from '../common/fleet_services'; | ||
import { downloadAndStoreAgent } from '../common/agent_downloads_service'; | ||
|
||
// Decrement the patch version by 1 | ||
const decrementPatchVersion = (version: string): string => { | ||
const [major, minor, patch] = version.split('.').map(Number); | ||
return `${major}.${minor}.${patch - 1}`; | ||
}; | ||
|
||
// Generate a list of versions to attempt downloading, including a fallback to the previous patch (GA) | ||
const getVersionsToDownload = (version: string): string[] => { | ||
const patch = parseInt(version.split('.')[2], 10); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good hint, I'll use semver here, and drop the pre-release tag |
||
|
||
// If patch version is 0, return only the current version. | ||
if (patch === 0) { | ||
return [version]; | ||
} | ||
|
||
return [version, decrementPatchVersion(version)]; | ||
}; | ||
// Download and store the Elastic Agent for the specified version(s) | ||
const downloadAndStoreElasticAgent = async ( | ||
version: string, | ||
closestMatch: boolean, | ||
log: ToolingLog | ||
) => { | ||
const downloadUrlResponse = await getAgentDownloadUrl(version, closestMatch, log); | ||
const fileNameNoExtension = getAgentFileName(version); | ||
const agentFile = `${fileNameNoExtension}.tar.gz`; | ||
await downloadAndStoreAgent(downloadUrlResponse.url, agentFile); | ||
): Promise<void> => { | ||
const versionsToDownload = getVersionsToDownload(version); | ||
|
||
for (const v of versionsToDownload) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Optional) Don't use single letter variable name. |
||
try { | ||
const { url } = await getAgentDownloadUrl(v, closestMatch, log); | ||
const fileName = `${getAgentFileName(v)}.tar.gz`; | ||
|
||
await downloadAndStoreAgent(url, fileName); | ||
log.info(`Successfully downloaded and stored version ${v}`); | ||
return; // Exit once successful | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you download just the first one? and then just exit? you are inside of a loop here, so I'm confused by this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great question! The reason we don’t download multiple versions is in most cases, only one version of the agent is needed to complete the task. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. So you are still only looking ot download 1 version, even though you get a list of versions for download. Maybe add this information to the above |
||
} catch (error) { | ||
log.error(`Failed to download or store version ${v}: ${error.message}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just want to ensure you mean to ONLY log an error here and then keep going in downloading other versions. is that right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's correct. If the first attempt fails, we move on to try downloading the fallback version. |
||
} | ||
} | ||
|
||
log.error(`Failed to download agent for any available version: ${versionsToDownload.join(', ')}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is misleading. you could reach have downloaded some successfully but this just reports that it all error'd out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I’m not sure about that. I believe the early return statement prevents us from reaching this point. |
||
}; | ||
|
||
const isValidVersion = (version: string): boolean => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please use |
||
return /^\d+\.\d+\.\d+$/.test(version); // Validate semantic version format | ||
}; | ||
|
||
export const agentDownloaderRunner: RunFn = async (cliContext) => { | ||
ok(cliContext.flags.version, 'version argument is required'); | ||
const { version } = cliContext.flags; | ||
|
||
ok(version, 'version argument is required'); | ||
|
||
// Validate version format | ||
if (!isValidVersion(version as string)) { | ||
throw new Error('Invalid version format'); | ||
} | ||
|
||
await downloadAndStoreElasticAgent( | ||
cliContext.flags.version as string, | ||
version as string, | ||
cliContext.flags.closestMatch as boolean, | ||
cliContext.log | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
// Adjust path if needed | ||
|
||
import { downloadAndStoreAgent, isAgentDownloadFromDiskAvailable } from './agent_downloads_service'; | ||
import fs from 'fs'; | ||
import nodeFetch from 'node-fetch'; | ||
import { finished } from 'stream/promises'; | ||
|
||
jest.mock('fs'); | ||
jest.mock('node-fetch'); | ||
jest.mock('stream/promises', () => ({ | ||
finished: jest.fn(), | ||
})); | ||
jest.mock('../../../common/endpoint/data_loaders/utils', () => ({ | ||
createToolingLogger: jest.fn(() => ({ | ||
debug: jest.fn(), | ||
info: jest.fn(), | ||
error: jest.fn(), | ||
})), | ||
})); | ||
|
||
describe('AgentDownloadStorage', () => { | ||
const url = 'http://example.com/agent.tar.gz'; | ||
const fileName = 'elastic-agent-7.10.0.tar.gz'; | ||
beforeEach(() => { | ||
jest.clearAllMocks(); // Ensure no previous test state affects the current one | ||
}); | ||
|
||
it('downloads and stores the agent if not cached', async () => { | ||
(fs.existsSync as unknown as jest.Mock).mockReturnValue(false); | ||
(fs.createWriteStream as unknown as jest.Mock).mockReturnValue({ | ||
on: jest.fn(), | ||
end: jest.fn(), | ||
}); | ||
(nodeFetch as unknown as jest.Mock).mockResolvedValue({ body: { pipe: jest.fn() } }); | ||
(finished as unknown as jest.Mock).mockResolvedValue(undefined); | ||
|
||
const result = await downloadAndStoreAgent(url, fileName); | ||
|
||
expect(result).toEqual({ | ||
url, | ||
filename: fileName, | ||
directory: expect.any(String), | ||
fullFilePath: expect.stringContaining(fileName), // Dynamically match the file path | ||
}); | ||
}); | ||
|
||
it('reuses cached agent if available', async () => { | ||
(fs.existsSync as unknown as jest.Mock).mockReturnValue(true); | ||
|
||
const result = await downloadAndStoreAgent(url, fileName); | ||
|
||
expect(result).toEqual({ | ||
url, | ||
filename: fileName, | ||
directory: expect.any(String), | ||
fullFilePath: expect.stringContaining(fileName), // Dynamically match the path | ||
}); | ||
}); | ||
|
||
it('checks if agent download is available from disk', () => { | ||
(fs.existsSync as unknown as jest.Mock).mockReturnValue(true); | ||
|
||
const result = isAgentDownloadFromDiskAvailable(fileName); | ||
|
||
expect(result).toEqual({ | ||
filename: fileName, | ||
directory: expect.any(String), | ||
fullFilePath: expect.stringContaining(fileName), // Dynamically match the path | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you see if the
semver
package can help here. The code here assumes thatversion
is a simple version of three separated values by.
which may not be true.Also - what happens if
patch
is0
? Example:8.15.0
,9.0.0
, etc...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give semver a try, great suggestion, thank you! 😊 As for the patch being 0, we only invoke decrementPatchVersion when the patch is greater than 0, so that shouldn't be an issue. 👍