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] Make upload and registry package info consistent #126915

Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
cf96755
Update docker image + set up initial validation test
kpollich Mar 3, 2022
625f582
Get validation test passing
kpollich Mar 4, 2022
5ff6ee6
Merge branch 'main' into 126695-make-upload-and-registry-package-info…
kibanamachine Mar 4, 2022
c564592
Remove erroneous test load call
kpollich Mar 4, 2022
586c08c
Address PR review + improve comments + rename validation.ts -> parse.ts
kpollich Mar 7, 2022
6a9c1e9
Merge branch 'main' into 126695-make-upload-and-registry-package-info…
kibanamachine Mar 7, 2022
62a45cc
Replace packages in fleet_packages.json
kpollich Mar 7, 2022
cae823e
Add temp debug log to debug CI failures
kpollich Mar 7, 2022
37f60f3
Use a non-colliding package in bundled package tests
kpollich Mar 7, 2022
4212c27
(debug) Add logging output
kpollich Mar 7, 2022
0f779d9
(debug) More logging
kpollich Mar 7, 2022
000f3a1
(debug) Log bundled package dir in module
kpollich Mar 7, 2022
6fdfd1f
Use absolute path for bundled packages
kpollich Mar 8, 2022
8656915
Remove debug logs + use KIBANA_BUILD_LOCATION if it exists
kpollich Mar 8, 2022
29d903c
Add support for developer.bundledPackageLocation config value
kpollich Mar 8, 2022
a316ab1
(debug) Try some more logs
kpollich Mar 8, 2022
0ba1902
(debug) Try some more logs
kpollich Mar 9, 2022
14e7b87
Fix test hopefully 🤞
kpollich Mar 9, 2022
eadab35
Fix other failing tests
kpollich Mar 9, 2022
1da1eba
Merge branch 'main' into 126695-make-upload-and-registry-package-info…
kibanamachine Mar 9, 2022
a1254e7
Merge branch 'main' into 126695-make-upload-and-registry-package-info…
kibanamachine Mar 9, 2022
dadfe20
Move default for bundled package dir to schema definition
kpollich Mar 9, 2022
5d97961
Merge branch 'main' into 126695-make-upload-and-registry-package-info…
kibanamachine Mar 9, 2022
9122a63
Merge branch 'main' into 126695-make-upload-and-registry-package-info…
kibanamachine Mar 9, 2022
bd2e6c2
Fix schema default value for bundledPackageLocation
kpollich Mar 9, 2022
bfffa79
Merge branch '126695-make-upload-and-registry-package-info-consistent…
kpollich Mar 9, 2022
f0e9da1
Fix snapshot
kpollich Mar 9, 2022
29a30e4
Fix regression in bundled packages fetch
kpollich Mar 9, 2022
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
23 changes: 22 additions & 1 deletion fleet_packages.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,25 @@
in order to verify package integrity.
*/

[]
[
{
"name": "apm",
"version": "8.1.0"
},
{
"name": "elastic_agent",
"version": "1.3.0"
},
{
"name": "endpoint",
"version": "1.5.0"
},
{
"name": "fleet_server",
"version": "1.1.0"
},
{
"name": "synthetics",
"version": "0.9.2"
}
]
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/common/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export interface FleetConfigType {
developer?: {
disableRegistryVersionCheck?: boolean;
allowAgentUpgradeSourceUri?: boolean;
bundledPackageLocation?: string;
};
}

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ export const config: PluginConfigDescriptor = {
developer: schema.object({
disableRegistryVersionCheck: schema.boolean({ defaultValue: false }),
allowAgentUpgradeSourceUri: schema.boolean({ defaultValue: false }),
bundledPackageLocation: schema.maybe(schema.string()),
}),
}),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import JSON5 from 'json5';
import { REPO_ROOT } from '@kbn/utils';

import * as Registry from '../services/epm/registry';
import { parseAndVerifyArchiveEntries } from '../services/epm/archive';
import { generatePackageInfoFromArchiveBuffer } from '../services/epm/archive';

import { createAppContextStartContractMock } from '../mocks';
import { appContextService } from '../services';
Expand Down Expand Up @@ -64,7 +64,7 @@ describe('validate bundled packages', () => {
for (const packageObject of packageObjects) {
const { registryPackage, packageArchive } = packageObject;

const archivePackageInfo = await parseAndVerifyArchiveEntries(
const archivePackageInfo = await generatePackageInfoFromArchiveBuffer(
packageArchive.archiveBuffer,
'application/zip'
);
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/server/services/epm/archive/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { getBufferExtractor } from './extract';

export * from './cache';
export { getBufferExtractor, untarBuffer, unzipBuffer } from './extract';
export { parseAndVerifyArchiveBuffer as parseAndVerifyArchiveEntries } from './validation';
export { generatePackageInfoFromArchiveBuffer } from './parse';

export interface ArchiveEntry {
path: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,15 @@ const MANIFEST_NAME = 'manifest.yml';

const DEFAULT_RELEASE_VALUE = 'ga';

// Ingest pipelines are specified in a `data_stream/<name>/elasticsearch/ingest_pipeline/` directory where a `default`
// ingest pipeline should be specified by one of these filenames.
Comment on lines +38 to +39
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for these comments 💯

const DEFAULT_INGEST_PIPELINE_VALUE = 'default';
const DEFAULT_INGEST_PIPELINE_FILE_NAME_YML = 'default.yml';
const DEFAULT_INGEST_PIPELINE_FILE_NAME_JSON = 'default.json';

// Borrowed from https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/common/utils/expand_dotted.ts
// With some alterations around non-object values
// with some alterations around non-object values. The package registry service expands some dotted fields from manifest files,
// so we need to do the same here.
const expandDottedField = (dottedFieldName: string, val: unknown): object => {
const parts = dottedFieldName.split('.');

Expand Down Expand Up @@ -113,10 +116,19 @@ const registryPolicyTemplateProps = Object.values(RegistryPolicyTemplateKeys);
const registryStreamProps = Object.values(RegistryStreamKeys);
const registryDataStreamProps = Object.values(RegistryDataStreamKeys);

// TODO: everything below performs verification of manifest.yml files, and hence duplicates functionality already implemented in the
// package registry. At some point this should probably be replaced (or enhanced) with verification based on
// https://github.com/elastic/package-spec/
export async function parseAndVerifyArchiveBuffer(
/*
This function generates a package info object (see type `ArchivePackage`) by parsing and verifying the `manifest.yml` file as well
as the directory structure for the given package archive and other files adhering to the package spec: https://github.com/elastic/package-spec.

Currently, this process is duplicative of logic that's already implemented in the Package Registry codebase,
e.g. https://github.com/elastic/package-registry/blob/main/packages/package.go. Because of this duplication, it's likely for our parsing/verification
logic to fall out of sync with the registry codebase's implementation.

This should be addressed in https://github.com/elastic/kibana/issues/115032
where we'll no longer use the package registry endpoint as a source of truth for package info objects, and instead Fleet will _always_ generate
them in the manner implemented below.
*/
export async function generatePackageInfoFromArchiveBuffer(
archiveBuffer: Buffer,
contentType: string
): Promise<{ paths: string[]; packageInfo: ArchivePackage }> {
Expand Down Expand Up @@ -363,7 +375,6 @@ export function parseAndVerifyStreams(
streamObject.vars = vars;
}

// default template path name see https://github.com/elastic/package-registry/blob/master/util/dataset.go#L143
streams.push(
Object.entries(restOfProps).reduce((validatedStream, [key, value]) => {
if (registryStreamProps.includes(key as RegistryStreamKeys)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { appContextService } from '../../app_context';

import { getArchiveEntry, setArchiveEntry, setArchiveFilelist, setPackageInfo } from './index';
import type { ArchiveEntry } from './index';
import { parseAndVerifyPolicyTemplates, parseAndVerifyStreams } from './validation';
import { parseAndVerifyPolicyTemplates, parseAndVerifyStreams } from './parse';

const ONE_BYTE = 1024 * 1024;
// could be anything, picked this from https://github.com/elastic/elastic-agent-client/issues/17
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ import type { BundledPackage } from '../../../types';
import { appContextService } from '../../app_context';
import { splitPkgKey } from '../registry';

const BUNDLED_PACKAGE_DIRECTORY = path.join(__dirname, '../../../../target/bundled_packages');

export async function getBundledPackages(): Promise<BundledPackage[]> {
const config = appContextService.getConfig();

const BUNDLED_PACKAGE_DIRECTORY = config?.developer?.bundledPackageLocation
? config.developer.bundledPackageLocation
: path.join(__dirname, '../../../../target/bundled_packages');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be ideal if we put this default in the config schema in x-pack/plugins/fleet/server/index.ts:

bundledPackageLocation: schema.string({ defaultValue: '...' }),


try {
const dirContents = await fs.readdir(BUNDLED_PACKAGE_DIRECTORY);
const zipFiles = dirContents.filter((file) => file.endsWith('.zip'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jest.mock('../kibana/index_pattern/install', () => {
});
jest.mock('../archive', () => {
return {
parseAndVerifyArchiveEntries: jest.fn(() =>
generatePackageInfoFromArchiveBuffer: jest.fn(() =>
Promise.resolve({ packageInfo: { name: 'apache', version: '1.3.0' } })
),
unpackBufferToCache: jest.fn(),
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/packages/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ import type {
} from '../../../types';
import { appContextService } from '../../app_context';
import * as Registry from '../registry';
import { setPackageInfo, parseAndVerifyArchiveEntries, unpackBufferToCache } from '../archive';
import {
setPackageInfo,
generatePackageInfoFromArchiveBuffer,
unpackBufferToCache,
} from '../archive';
import { toAssetReference } from '../kibana/assets/install';
import type { ArchiveAsset } from '../kibana/assets/install';

Expand Down Expand Up @@ -391,7 +395,7 @@ async function installPackageByUpload({
let installType: InstallType = 'unknown';
const telemetryEvent: PackageUpdateEvent = getTelemetryEvent('', '');
try {
const { packageInfo } = await parseAndVerifyArchiveEntries(archiveBuffer, contentType);
const { packageInfo } = await generatePackageInfoFromArchiveBuffer(archiveBuffer, contentType);

const installedPkg = await getInstallationObject({
savedObjectsClient,
Expand Down
40 changes: 22 additions & 18 deletions x-pack/test/fleet_api_integration/apis/epm/install_bundled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import expect from '@kbn/expect';
import fs from 'fs/promises';
import path from 'path';

import { BUNDLED_PACKAGE_DIR } from '../../config';
import { FtrProviderContext } from '../../../api_integration/ftr_provider_context';
import { skipIfNoDockerRegistry } from '../../helpers';
import { setupFleetAndAgents } from '../agents/services';
Expand All @@ -22,30 +23,31 @@ export default function (providerContext: FtrProviderContext) {
path.dirname(__filename),
'../fixtures/bundled_packages'
);
const BUNDLED_PACKAGES_DIR = path.join(
path.dirname(__filename),
'../../../../plugins/fleet/target/bundled_packages'
);

const bundlePackage = async (name: string) => {
try {
await fs.access(BUNDLED_PACKAGES_DIR);
await fs.access(BUNDLED_PACKAGE_DIR);
} catch (error) {
await fs.mkdir(BUNDLED_PACKAGES_DIR);
await fs.mkdir(BUNDLED_PACKAGE_DIR);
}

await fs.copyFile(
path.join(BUNDLED_PACKAGE_FIXTURES_DIR, `${name}.zip`),
path.join(BUNDLED_PACKAGES_DIR, `${name}.zip`)
path.join(BUNDLED_PACKAGE_DIR, `${name}.zip`)
);
};

const removeBundledPackages = async () => {
try {
const files = await fs.readdir(BUNDLED_PACKAGES_DIR);
const files = await fs.readdir(BUNDLED_PACKAGE_DIR);

for (const file of files) {
await fs.unlink(path.join(BUNDLED_PACKAGES_DIR, file));
const isFixtureFile = !!(await fs.readFile(path.join(BUNDLED_PACKAGE_FIXTURES_DIR, file)));

// Only remove fixture files - leave normal bundled packages in place
if (isFixtureFile) {
await fs.unlink(path.join(BUNDLED_PACKAGE_DIR, file));
}
}
} catch (error) {
log.error('Error removing bundled packages');
Expand All @@ -63,10 +65,11 @@ export default function (providerContext: FtrProviderContext) {

describe('without registry', () => {
it('installs from bundled source via api', async () => {
await bundlePackage('elastic_agent-1.2.0');
// Need to bundle a package that doesn't conflict with those listed in `fleet_packages.json
await bundlePackage('nginx-1.2.1');

const response = await supertest
.post(`/api/fleet/epm/packages/elastic_agent/1.2.0`)
.post(`/api/fleet/epm/packages/nginx/1.2.1`)
.set('kbn-xsrf', 'xxxx')
.type('application/json')
.send({ force: true })
Expand All @@ -76,11 +79,11 @@ export default function (providerContext: FtrProviderContext) {
});

it('allows for upgrading from newer bundled source when outdated package was installed from bundled source', async () => {
await bundlePackage('elastic_agent-1.0.0');
await bundlePackage('elastic_agent-1.2.0');
await bundlePackage('nginx-1.1.0');
await bundlePackage('nginx-1.2.1');

const installResponse = await supertest
.post(`/api/fleet/epm/packages/elastic_agent/1.0.0`)
.post(`/api/fleet/epm/packages/nginx/1.1.0`)
.set('kbn-xsrf', 'xxxx')
.type('application/json')
.send({ force: true })
Expand All @@ -89,7 +92,7 @@ export default function (providerContext: FtrProviderContext) {
expect(installResponse.body._meta.install_source).to.be('bundled');

const updateResponse = await supertest
.post(`/api/fleet/epm/packages/elastic_agent/1.2.0`)
.post(`/api/fleet/epm/packages/nginx/1.2.1`)
.set('kbn-xsrf', 'xxxx')
.type('application/json')
.send({ force: true })
Expand All @@ -101,19 +104,20 @@ export default function (providerContext: FtrProviderContext) {

describe('with registry', () => {
it('allows for updating from registry when outdated package is installed from bundled source', async () => {
await bundlePackage('elastic_agent-1.2.0');
await bundlePackage('nginx-1.1.0');

const bundledInstallResponse = await supertest
.post(`/api/fleet/epm/packages/elastic_agent/1.2.0`)
.post(`/api/fleet/epm/packages/nginx/1.1.0`)
.set('kbn-xsrf', 'xxxx')
.type('application/json')
.send({ force: true })
.expect(200);

expect(bundledInstallResponse.body._meta.install_source).to.be('bundled');

// Update to one version prior to the bundled version of nginx
const registryUpdateResponse = await supertest
.post(`/api/fleet/epm/packages/elastic_agent/1.3.0`)
.post(`/api/fleet/epm/packages/elastic_agent/1.2.0`)
.set('kbn-xsrf', 'xxxx')
.type('application/json')
.send({ force: true })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default function (providerContext: FtrProviderContext) {
.type('application/gzip')
.send(buf)
.expect(200);
expect(res.body.items.length).to.be(27);
expect(res.body.items.length).to.be(29);
Copy link
Member Author

Choose a reason for hiding this comment

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

These responses have ingest pipelines, etc now, so they have additional elements.

});

it('should install a zip archive correctly and package info should return correctly after validation', async function () {
Expand All @@ -86,7 +86,7 @@ export default function (providerContext: FtrProviderContext) {
.type('application/zip')
.send(buf)
.expect(200);
expect(res.body.items.length).to.be(27);
expect(res.body.items.length).to.be(29);
});

it('should throw an error if the archive is zip but content type is gzip', async function () {
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
3 changes: 3 additions & 0 deletions x-pack/test/fleet_api_integration/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { defineDockerServersConfig } from '@kbn/test';
export const dockerImage =
'docker.elastic.co/package-registry/distribution@sha256:b3dfc6a11ff7dce82ba8689ea9eeb54e353c6b4bfd2d28127b20ef72fd8883e9';

export const BUNDLED_PACKAGE_DIR = '/tmp/fleet_bundled_packages';

export default async function ({ readConfigFile }: FtrConfigProviderContext) {
const xPackAPITestsConfig = await readConfigFile(require.resolve('../api_integration/config.ts'));

Expand Down Expand Up @@ -63,6 +65,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) {
`--xpack.fleet.packages.0.name=endpoint`,
`--xpack.fleet.packages.0.version=latest`,
...(registryPort ? [`--xpack.fleet.registryUrl=http://localhost:${registryPort}`] : []),
`--xpack.fleet.developer.bundledPackageLocation=${BUNDLED_PACKAGE_DIR}`,
],
},
};
Expand Down