Skip to content

Commit

Permalink
Add asset.kind handling to assets API (#159065)
Browse files Browse the repository at this point in the history
## Summary

Implicit collection stores segmentation values on `asset.kind` instead
of on `asset.type`, like originally planned. This PR makes those changes
so that `asset.kind` is a valid filter. It leaves `asset.type` in place
for the moment.
  • Loading branch information
jasonrhodes authored Jun 6, 2023
1 parent dfdcd79 commit cc8e4d4
Show file tree
Hide file tree
Showing 12 changed files with 275 additions and 127 deletions.
15 changes: 13 additions & 2 deletions x-pack/plugins/asset_manager/common/types_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,17 @@ export const assetTypeRT = rt.union([

export type AssetType = rt.TypeOf<typeof assetTypeRT>;

export type AssetKind = 'cluster' | 'host' | 'pod' | 'container' | 'service';
export const assetKindRT = rt.union([
rt.literal('cluster'),
rt.literal('host'),
rt.literal('pod'),
rt.literal('container'),
rt.literal('service'),
rt.literal('alert'),
]);

export type AssetKind = rt.TypeOf<typeof assetKindRT>;

export type AssetStatus =
| 'CREATING'
| 'ACTIVE'
Expand Down Expand Up @@ -124,10 +134,11 @@ export interface K8sCluster extends WithTimestamp {

export interface AssetFilters {
type?: AssetType | AssetType[];
kind?: AssetKind;
kind?: AssetKind | AssetKind[];
ean?: string | string[];
id?: string;
typeLike?: string;
kindLike?: string;
eanLike?: string;
collectionVersion?: number | 'latest' | 'all';
from?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,21 @@
*/

jest.mock('./get_assets', () => ({ getAssets: jest.fn() }));
jest.mock('./get_related_assets', () => ({ getRelatedAssets: jest.fn() }));
jest.mock('./get_indirectly_related_assets', () => ({ getIndirectlyRelatedAssets: jest.fn() }));

import { elasticsearchClientMock } from '@kbn/core-elasticsearch-client-server-mocks';
import { v4 as uuid } from 'uuid';
import { AssetWithoutTimestamp } from '../../common/types_api';
import { getAssets } from './get_assets'; // Mocked
import { getRelatedAssets } from './get_related_assets'; // Mocked
import { getIndirectlyRelatedAssets } from './get_indirectly_related_assets'; // Mocked
import { getAllRelatedAssets } from './get_all_related_assets';

const esClientMock = elasticsearchClientMock.createScopedClusterClient().asCurrentUser;

describe('getAllRelatedAssets', () => {
beforeEach(() => {
(getAssets as jest.Mock).mockReset();
(getRelatedAssets as jest.Mock).mockReset();
(getIndirectlyRelatedAssets as jest.Mock).mockReset();
});

it('throws if it cannot find the primary asset', async () => {
Expand All @@ -35,7 +35,9 @@ describe('getAllRelatedAssets', () => {
(getAssets as jest.Mock).mockResolvedValueOnce([]);
// Ensure maxDistance is respected
(getAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getRelatedAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getIndirectlyRelatedAssets as jest.Mock).mockRejectedValueOnce(
new Error('Should respect maxDistance')
);

await expect(
getAllRelatedAssets(esClientMock, {
Expand All @@ -61,10 +63,12 @@ describe('getAllRelatedAssets', () => {

(getAssets as jest.Mock).mockResolvedValueOnce([primaryAssetWithoutParents]);
// Distance 1
(getRelatedAssets as jest.Mock).mockResolvedValueOnce([]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValueOnce([]);
// Ensure maxDistance is respected
(getAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getRelatedAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getIndirectlyRelatedAssets as jest.Mock).mockRejectedValueOnce(
new Error('Should respect maxDistance')
);

await expect(
getAllRelatedAssets(esClientMock, {
Expand All @@ -82,7 +86,7 @@ describe('getAllRelatedAssets', () => {

it('returns the primary and a directly referenced parent', async () => {
const parentAsset: AssetWithoutTimestamp = {
'asset.ean': 'primary-ean',
'asset.ean': 'parent-ean',
'asset.type': 'k8s.pod',
'asset.kind': 'pod',
'asset.id': uuid(),
Expand All @@ -100,10 +104,12 @@ describe('getAllRelatedAssets', () => {
(getAssets as jest.Mock).mockResolvedValueOnce([primaryAssetWithDirectParent]);
// Distance 1
(getAssets as jest.Mock).mockResolvedValueOnce([parentAsset]);
(getRelatedAssets as jest.Mock).mockResolvedValueOnce([]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValueOnce([]);
// Ensure maxDistance is respected
(getAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getRelatedAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getIndirectlyRelatedAssets as jest.Mock).mockRejectedValueOnce(
new Error('Should respect maxDistance')
);

await expect(
getAllRelatedAssets(esClientMock, {
Expand Down Expand Up @@ -145,10 +151,12 @@ describe('getAllRelatedAssets', () => {
(getAssets as jest.Mock).mockResolvedValueOnce([primaryAssetWithIndirectParent]);
// Distance 1
(getAssets as jest.Mock).mockResolvedValueOnce([]);
(getRelatedAssets as jest.Mock).mockResolvedValueOnce([parentAsset]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValueOnce([parentAsset]);
// Ensure maxDistance is respected
(getAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getRelatedAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getIndirectlyRelatedAssets as jest.Mock).mockRejectedValueOnce(
new Error('Should respect maxDistance')
);

await expect(
getAllRelatedAssets(esClientMock, {
Expand Down Expand Up @@ -198,10 +206,12 @@ describe('getAllRelatedAssets', () => {
(getAssets as jest.Mock).mockResolvedValueOnce([primaryAsset]);
// Distance 1
(getAssets as jest.Mock).mockResolvedValueOnce([directlyReferencedParent]);
(getRelatedAssets as jest.Mock).mockResolvedValueOnce([indirectlyReferencedParent]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValueOnce([indirectlyReferencedParent]);
// Ensure maxDistance is respected
(getAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getRelatedAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getIndirectlyRelatedAssets as jest.Mock).mockRejectedValueOnce(
new Error('Should respect maxDistance')
);

await expect(
getAllRelatedAssets(esClientMock, {
Expand Down Expand Up @@ -249,10 +259,12 @@ describe('getAllRelatedAssets', () => {
// Distance 1
(getAssets as jest.Mock).mockResolvedValueOnce([parentAsset]);
// Code should filter out any directly referenced parent from the indirectly referenced parents query
(getRelatedAssets as jest.Mock).mockResolvedValueOnce([]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValueOnce([]);
// Ensure maxDistance is respected
(getAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getRelatedAssets as jest.Mock).mockRejectedValueOnce(new Error('Should respect maxDistance'));
(getIndirectlyRelatedAssets as jest.Mock).mockRejectedValueOnce(
new Error('Should respect maxDistance')
);

await expect(
getAllRelatedAssets(esClientMock, {
Expand Down Expand Up @@ -330,7 +342,7 @@ describe('getAllRelatedAssets', () => {
};

// Only using directly referenced parents
(getRelatedAssets as jest.Mock).mockResolvedValue([]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValue([]);

// Primary
(getAssets as jest.Mock).mockResolvedValueOnce([primaryAsset]);
Expand Down Expand Up @@ -415,7 +427,7 @@ describe('getAllRelatedAssets', () => {
};

// Only using directly referenced parents
(getRelatedAssets as jest.Mock).mockResolvedValue([]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValue([]);

// Primary
(getAssets as jest.Mock).mockResolvedValueOnce([primaryAsset]);
Expand Down Expand Up @@ -507,7 +519,7 @@ describe('getAllRelatedAssets', () => {
};

// Only using directly referenced parents
(getRelatedAssets as jest.Mock).mockResolvedValue([]);
(getIndirectlyRelatedAssets as jest.Mock).mockResolvedValue([]);

// Primary
(getAssets as jest.Mock).mockResolvedValueOnce([primaryAsset]);
Expand Down
44 changes: 27 additions & 17 deletions x-pack/plugins/asset_manager/server/lib/get_all_related_assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@

import { ElasticsearchClient } from '@kbn/core/server';
import { flatten, without } from 'lodash';
import { Asset, AssetType, Relation, RelationField } from '../../common/types_api';
import { debug } from '../../common/debug_log';
import { Asset, AssetType, AssetKind, Relation, RelationField } from '../../common/types_api';
import { getAssets } from './get_assets';
import { getRelatedAssets } from './get_related_assets';
import { getIndirectlyRelatedAssets } from './get_indirectly_related_assets';
import { AssetNotFoundError } from './errors';
import { toArray } from './utils';

Expand All @@ -19,6 +20,7 @@ interface GetAllRelatedAssetsOptions {
to?: string;
relation: Relation;
type?: AssetType[];
kind?: AssetKind[];
maxDistance: number;
size: number;
}
Expand All @@ -28,7 +30,7 @@ export async function getAllRelatedAssets(
options: GetAllRelatedAssetsOptions
) {
// How to put size into this?
const { ean, from, to, relation, maxDistance, type = [] } = options;
const { ean, from, to, relation, maxDistance, kind = [] } = options;

const primary = await findPrimary(esClient, { ean, from, to });

Expand All @@ -42,10 +44,10 @@ export async function getAllRelatedAssets(
to,
visitedEans: [primary['asset.ean'], ...relatedAssets.map((asset) => asset['asset.ean'])],
};
// if we enforce the type filter before the last query we'll miss nodes with
// possible edges to the requested types
if (currentDistance === maxDistance && type.length) {
queryOptions.type = type;
// if we enforce the kind filter before the last query we'll miss nodes with
// possible edges to the requested kind values
if (currentDistance === maxDistance && kind.length) {
queryOptions.kind = kind;
}

const results = flatten(
Expand All @@ -66,8 +68,8 @@ export async function getAllRelatedAssets(

return {
primary,
[relation]: type.length
? relatedAssets.filter((asset) => asset['asset.type'] && type.includes(asset['asset.type']))
[relation]: kind.length
? relatedAssets.filter((asset) => asset['asset.kind'] && kind.includes(asset['asset.kind']))
: relatedAssets,
};
}
Expand Down Expand Up @@ -95,36 +97,44 @@ async function findPrimary(

type FindRelatedAssetsOptions = Pick<
GetAllRelatedAssetsOptions,
'relation' | 'type' | 'from' | 'to'
'relation' | 'kind' | 'from' | 'to'
> & { visitedEans: string[] };

async function findRelatedAssets(
esClient: ElasticsearchClient,
primary: Asset,
{ relation, from, to, type, visitedEans }: FindRelatedAssetsOptions
{ relation, from, to, kind, visitedEans }: FindRelatedAssetsOptions
): Promise<Asset[]> {
const relationField = relationToDirectField(relation);
const directlyRelatedEans = toArray(primary[relationField]);
const directlyRelatedEans = toArray<string>(primary[relationField]);

debug('Directly Related EAN values found on primary asset', directlyRelatedEans);

let directlyRelatedAssets: Asset[] = [];
if (directlyRelatedEans.length) {
// get the directly related assets we haven't visited already

// get the directly related assets we haven't visited already
const remainingEansToFind = without(directlyRelatedEans, ...visitedEans);
if (remainingEansToFind.length > 0) {
directlyRelatedAssets = await getAssets({
esClient,
filters: { ean: without(directlyRelatedEans, ...visitedEans), from, to, type },
filters: { ean: remainingEansToFind, from, to, kind },
});
}

const indirectlyRelatedAssets = await getRelatedAssets({
debug('Directly related assets found:', JSON.stringify(directlyRelatedAssets));

const indirectlyRelatedAssets = await getIndirectlyRelatedAssets({
esClient,
ean: primary['asset.ean'],
excludeEans: visitedEans.concat(directlyRelatedEans),
relation,
from,
to,
type,
kind,
});

debug('Indirectly related assets found:', JSON.stringify(indirectlyRelatedAssets));

return [...directlyRelatedAssets, ...indirectlyRelatedAssets];
}

Expand Down
22 changes: 16 additions & 6 deletions x-pack/plugins/asset_manager/server/lib/get_assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { debug } from '../../common/debug_log';
import { Asset, AssetFilters } from '../../common/types_api';
import { ASSETS_INDEX_PREFIX } from '../constants';
import { ElasticsearchAccessorOptions } from '../types';
import { isStringOrNonEmptyArray } from './utils';

interface GetAssetsOptions extends ElasticsearchAccessorOptions {
size?: number;
Expand All @@ -23,6 +24,7 @@ export async function getAssets({
filters = {},
}: GetAssetsOptions): Promise<Asset[]> {
// Maybe it makes the most sense to validate the filters here?
debug('Get Assets Filters:', JSON.stringify(filters));

const { from = 'now-24h', to = 'now' } = filters;
const must: QueryDslQueryContainer[] = [];
Expand All @@ -36,23 +38,23 @@ export async function getAssets({
});
}

if (filters.type?.length) {
if (isStringOrNonEmptyArray(filters.type)) {
must.push({
terms: {
['asset.type']: Array.isArray(filters.type) ? filters.type : [filters.type],
},
});
}

if (filters.kind) {
if (isStringOrNonEmptyArray(filters.kind)) {
must.push({
term: {
['asset.kind']: filters.kind,
terms: {
['asset.kind']: Array.isArray(filters.kind) ? filters.kind : [filters.kind],
},
});
}

if (filters.ean) {
if (isStringOrNonEmptyArray(filters.ean)) {
must.push({
terms: {
['asset.ean']: Array.isArray(filters.ean) ? filters.ean : [filters.ean],
Expand All @@ -76,6 +78,14 @@ export async function getAssets({
});
}

if (filters.kindLike) {
must.push({
wildcard: {
['asset.kind']: filters.kindLike,
},
});
}

if (filters.eanLike) {
must.push({
wildcard: {
Expand Down Expand Up @@ -113,7 +123,7 @@ export async function getAssets({
},
};

debug('Performing Asset Query', '\n\n', JSON.stringify(dsl, null, 2));
debug('Performing Get Assets Query', '\n\n', JSON.stringify(dsl, null, 2));

const response = await esClient.search<Asset>(dsl);
return response.hits.hits.map((hit) => hit._source).filter((asset): asset is Asset => !!asset);
Expand Down
Loading

0 comments on commit cc8e4d4

Please sign in to comment.