Skip to content
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

[Fleet] Rely on package content during installation #142353

Merged
merged 15 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {
InstallablePackage,
Installation,
RegistryPackage,
ArchivePackage,
BundledPackage,
} from '../../types';
import { checkSuperuser } from '../../routes/security';
Expand Down Expand Up @@ -49,7 +50,7 @@ export interface PackageClient {
getRegistryPackage(
packageName: string,
packageVersion: string
): Promise<{ packageInfo: RegistryPackage; paths: string[] }>;
): Promise<{ packageInfo: ArchivePackage; paths: string[] }>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems weird to me that getRegistryPackage return an ArchivePackage should this be a new method, or should rename it getPackage ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling it getPackage would be a good option. I've been in doubt about the naming because the whole flow is in Registry but it's not going to use the registry anymore.. I guess we'll have to refactor further

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the name to GetPackage, I will need additional approvals from other teams that also use that method.


reinstallEsAssets(
packageInfo: InstallablePackage,
Expand Down
10 changes: 2 additions & 8 deletions x-pack/plugins/fleet/server/services/epm/packages/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ export async function getPackageInfo({
pkgVersion: resolvedPkgVersion,
savedObjectsClient,
installedPkg: savedObject?.attributes,
getPkgInfoFromArchive: packageInfo?.type === 'input',
ignoreUnverified,
}));
}
Expand Down Expand Up @@ -235,13 +234,12 @@ interface PackageResponse {
}
type GetPackageResponse = PackageResponse | undefined;

// gets package from install_source if it exists otherwise gets from registry
// gets package from install_source
export async function getPackageFromSource(options: {
pkgName: string;
pkgVersion: string;
installedPkg?: Installation;
savedObjectsClient: SavedObjectsClientContract;
getPkgInfoFromArchive?: boolean;
ignoreUnverified?: boolean;
}): Promise<PackageResponse> {
const logger = appContextService.getLogger();
Expand All @@ -250,7 +248,6 @@ export async function getPackageFromSource(options: {
pkgVersion,
installedPkg,
savedObjectsClient,
getPkgInfoFromArchive = true,
ignoreUnverified = false,
} = options;
let res: GetPackageResponse;
Expand Down Expand Up @@ -295,10 +292,7 @@ export async function getPackageFromSource(options: {
}
} else {
// else package is not installed or installed and missing from cache and storage and installed from registry
res = await Registry.getRegistryPackage(pkgName, pkgVersion, {
getPkgInfoFromArchive,
ignoreUnverified,
});
res = await Registry.getRegistryPackage(pkgName, pkgVersion, { ignoreUnverified });
logger.debug(`retrieved uninstalled package ${pkgName}-${pkgVersion} from registry`);
}
if (!res) {
Expand Down
75 changes: 48 additions & 27 deletions x-pack/plugins/fleet/server/services/epm/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,35 +250,59 @@ export async function getInfo(name: string, version: string) {
});
}

// Check that the packageInfo exists in cache
// If not, retrieve it from the archive
async function getPackageInfoFromArchiveOrCache(
name: string,
version: string,
archiveBuffer: Buffer,
archivePath: string
): Promise<ArchivePackage> {
const cachedInfo = getPackageInfo({ name, version });

if (!cachedInfo) {
const { packageInfo } = await generatePackageInfoFromArchiveBuffer(
archiveBuffer,
ensureContentType(archivePath)
);
setPackageInfo({ packageInfo, name, version });
return packageInfo;
} else {
return cachedInfo;
}
}

export async function getRegistryPackage(
name: string,
version: string,
options?: { ignoreUnverified?: boolean; getPkgInfoFromArchive?: boolean }
options?: { ignoreUnverified?: boolean }
): Promise<{
paths: string[];
packageInfo: RegistryPackage;
packageInfo: ArchivePackage;
verificationResult?: PackageVerificationResult;
}> {
const verifyPackage = appContextService.getExperimentalFeatures().packageVerification;
let paths = getArchiveFilelist({ name, version });
let verificationResult = verifyPackage ? getVerificationResult({ name, version }) : undefined;

const {
archiveBuffer,
archivePath,
verificationResult: latestVerificationResult,
} = await withPackageSpan('Fetch package archive from archive buffer', () =>
fetchArchiveBuffer({
pkgName: name,
pkgVersion: version,
shouldVerify: verifyPackage,
ignoreUnverified: options?.ignoreUnverified,
})
);

if (latestVerificationResult) {
verificationResult = latestVerificationResult;
setVerificationResult({ name, version }, latestVerificationResult);
}
if (!paths || paths.length === 0) {
const {
archiveBuffer,
archivePath,
verificationResult: latestVerificationResult,
} = await withPackageSpan('Fetch package archive from registry', () =>
fetchArchiveBuffer({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function internally calls getInfo to retrieve the ArchiveBuffer:

export async function fetchArchiveBuffer({
pkgName,
pkgVersion,
shouldVerify,
ignoreUnverified = false,
}: {
pkgName: string;
pkgVersion: string;
shouldVerify: boolean;
ignoreUnverified?: boolean;
}): Promise<{
archiveBuffer: Buffer;
archivePath: string;
verificationResult?: PackageVerificationResult;
}> {
const logger = appContextService.getLogger();
const { download: archivePath } = await getInfo(pkgName, pkgVersion);
const archiveUrl = `${getRegistryUrl()}${archivePath}`;
const archiveBuffer = await getResponseStream(archiveUrl).then(streamToBuffer);

I wonder if I should get the ArchiveBuffer from the cache instead and remove the calls to getInfo altogether.

pkgName: name,
pkgVersion: version,
shouldVerify: verifyPackage,
ignoreUnverified: options?.ignoreUnverified,
})
);
if (latestVerificationResult) {
verificationResult = latestVerificationResult;
setVerificationResult({ name, version }, latestVerificationResult);
}
paths = await withPackageSpan('Unpack archive', () =>
unpackBufferToCache({
name,
Expand All @@ -287,17 +311,14 @@ export async function getRegistryPackage(
contentType: ensureContentType(archivePath),
})
);
const cachedInfo = getPackageInfo({ name, version });
if (options?.getPkgInfoFromArchive && !cachedInfo) {
const { packageInfo } = await generatePackageInfoFromArchiveBuffer(
archiveBuffer,
ensureContentType(archivePath)
);
setPackageInfo({ packageInfo, name, version });
}
}

const packageInfo = await getInfo(name, version);
const packageInfo = await getPackageInfoFromArchiveOrCache(
name,
version,
archiveBuffer,
archivePath
);
return { paths, packageInfo, verificationResult };
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { capitalize, flatten } from 'lodash';
import type { PackagePolicy, RegistryPackage } from '@kbn/fleet-plugin/common';
import type { PackagePolicy, ArchivePackage } from '@kbn/fleet-plugin/common';
import type {
InstalledIntegration,
InstalledIntegrationArray,
Expand All @@ -18,7 +18,7 @@ import type {

export interface IInstalledIntegrationSet {
addPackagePolicy(policy: PackagePolicy): void;
addRegistryPackage(registryPackage: RegistryPackage): void;
addRegistryPackage(registryPackage: ArchivePackage): void;

getPackages(): InstalledPackageArray;
getIntegrations(): InstalledIntegrationArray;
Expand Down Expand Up @@ -56,7 +56,7 @@ export const createInstalledIntegrationSet = (): IInstalledIntegrationSet => {
}
};

const addRegistryPackage = (registryPackage: RegistryPackage): void => {
const addRegistryPackage = (registryPackage: ArchivePackage): void => {
const policyTemplates = registryPackage.policy_templates ?? [];
const packageKey = `${registryPackage.name}:${registryPackage.version}`;
const existingPackageInfo = packageMap.get(packageKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default function (providerContext: FtrProviderContext) {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');
const kibanaServer = getService('kibanaServer');

describe('fleet_agent_policies', () => {
skipIfNoDockerRegistry(providerContext);
describe('POST /api/fleet/agent_policies', () => {
Expand Down Expand Up @@ -287,8 +288,20 @@ export default function (providerContext: FtrProviderContext) {
setupFleetAndAgents(providerContext);
after(async () => {
await esArchiver.unload('x-pack/test/functional/es_archives/fleet/agents');
if (systemPkgVersion) {
await supertest.delete(`/api/fleet/epm/packages/system-${systemPkgVersion}`);
}
if (packagePoliciesToDeleteIds.length > 0) {
await kibanaServer.savedObjects.bulkDelete({
objects: packagePoliciesToDeleteIds.map((id) => ({
id,
type: PACKAGE_POLICY_SAVED_OBJECT_TYPE,
})),
});
}
});

let systemPkgVersion: string;
const packagePoliciesToDeleteIds: string[] = [];
const TEST_POLICY_ID = 'policy1';

it('should work with valid values', async () => {
Expand Down Expand Up @@ -327,13 +340,43 @@ export default function (providerContext: FtrProviderContext) {
},
} = await supertest.get(`/api/fleet/agent_policies/${policyId}`).expect(200);

const matches = packagePolicies[0].name.match(/^(.*)\s\(copy\s?([0-9]*)\)$/);
const matches = packagePolicies[0]?.name.match(/^(.*)\s\(copy\s?([0-9]*)\)$/);

if (matches) {
return parseInt(matches[2], 10) || 1;
}

return 0;
}

const policyId = 'package-policy-test-1';
packagePoliciesToDeleteIds.push(policyId);
const getPkRes = await supertest
.get(`/api/fleet/epm/packages/system`)
.set('kbn-xsrf', 'xxxx')
.expect(200);
systemPkgVersion = getPkRes.body.item.version;
// we must first force install the system package to override package verification error on policy create
const installPromise = supertest
.post(`/api/fleet/epm/packages/system-${systemPkgVersion}`)
.set('kbn-xsrf', 'xxxx')
.send({ force: true })
.expect(200);

await Promise.all([
installPromise,
kibanaServer.savedObjects.create({
id: policyId,
type: PACKAGE_POLICY_SAVED_OBJECT_TYPE,
overwrite: true,
attributes: {
name: `system-1`,
package: {
name: 'system',
},
},
}),
]);

const {
body: {
item: { id: originalPolicyId },
Expand All @@ -349,7 +392,6 @@ export default function (providerContext: FtrProviderContext) {
namespace: 'default',
})
.expect(200);

expect(await getSystemPackagePolicyCopyVersion(originalPolicyId)).to.be(0);

const {
Expand Down