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

[Osquery] 7.14 bug squash #105387

Merged
merged 15 commits into from
Jul 20, 2021
Merged
Show file tree
Hide file tree
Changes from 6 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 @@ -53,6 +53,18 @@ const ActionResultsSummaryComponent: React.FC<ActionResultsSummaryProps> = ({
sortField: '@timestamp',
isLive,
});
if (expired) {
// @ts-expect-error update types
edges.forEach((edge) => {
if (!edge.fields.completed_at) {
edge.fields['error.keyword'] = edge.fields.error = [
i18n.translate('xpack.osquery.liveQueryActionResults.table.expiredErrorText', {
defaultMessage: 'The action request timed out.',
}),
];
}
});
}

const { data: logsResults } = useAllResults({
actionId,
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugins/osquery/public/agents/use_all_agents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ export const useAllAgents = (
const { isLoading: agentsLoading, data: agentData } = useQuery<GetAgentsResponse>(
['agents', osqueryPolicies, searchValue, perPage],
() => {
const policyFragment = osqueryPolicies.map((p) => `policy_id:${p}`).join(' or ');
let kuery = `last_checkin_status: online and (${policyFragment})`;
let kuery = `${osqueryPolicies.map((p) => `policy_id:${p}`).join(' or ')}`;

if (searchValue) {
kuery += ` and (local_metadata.host.hostname:*${searchValue}* or local_metadata.elastic.agent.id:*${searchValue}*)`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import { isArray } from 'lodash';
import uuid from 'uuid';
import { produce } from 'immer';

import { useMemo } from 'react';
import { useForm } from '../../shared_imports';
import { formSchema } from '../../scheduled_query_groups/queries/schema';
import { createFormSchema } from '../../scheduled_query_groups/queries/schema';
import { ScheduledQueryGroupFormData } from '../../scheduled_query_groups/queries/use_scheduled_query_group_query_form';
import { useSavedQueries } from '../use_saved_queries';

const SAVED_QUERY_FORM_ID = 'savedQueryForm';

Expand All @@ -20,8 +22,22 @@ interface UseSavedQueryFormProps {
handleSubmit: (payload: unknown) => Promise<void>;
}

export const useSavedQueryForm = ({ defaultValue, handleSubmit }: UseSavedQueryFormProps) =>
useForm({
export const useSavedQueryForm = ({ defaultValue, handleSubmit }: UseSavedQueryFormProps) => {
const { data } = useSavedQueries({});
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move it to the backend?

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 like the idea of having it fail validation eagerly, but i also like the idea of having redundant protections. i'll add a check in the backend for both this and the scheduled query endpoint

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 added additional logic to the saved query endpoints, but skipped on the scheduled query endpoints since it seems like they are commented out currently. it seems like all the saved object write logic is done on the frontend anyway, from what i can tell; the saved query ui elements don't actually call into the backend, unless i'm missing something.

Copy link
Contributor

Choose a reason for hiding this comment

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

you're right 🤦
maybe let's move the validation fix to 7.15 and in this PR only merge telemetry, expired message, and agents count?
as we're going to introduce new logic for packs in 7.15 maybe we will be able to solve the validation differently

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'll rip out the backend logic, but i think it's worthwhile to keep the frontend validation in place to avoid the duplicate id problem. we can always rip it out when we implement something more general purpose.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it makes a lot of sense to me to put defensive logic around the points where the data is created/modified, so that sounds great. i'll add that.

const ids: string[] = useMemo<string[]>(
() => data?.savedObjects.map((obj) => obj.attributes.id) ?? [],
[data]
);
const idSet = useMemo<Set<string>>(() => {
const res = new Set<string>(ids);
// @ts-expect-error update types
if (defaultValue && defaultValue.id) res.delete(defaultValue.id);
return res;
}, [ids, defaultValue]);
const formSchema = useMemo<ReturnType<typeof createFormSchema>>(() => createFormSchema(idSet), [
idSet,
]);
return useForm({
id: SAVED_QUERY_FORM_ID + uuid.v4(),
schema: formSchema,
onSubmit: handleSubmit,
Expand Down Expand Up @@ -62,3 +78,4 @@ export const useSavedQueryForm = ({ defaultValue, handleSubmit }: UseSavedQueryF
};
},
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,19 @@ import { FormattedMessage } from '@kbn/i18n/react';

import { FIELD_TYPES } from '../../shared_imports';

import { idFieldValidations, intervalFieldValidation, queryFieldValidation } from './validations';
import {
createIdFieldValidations,
intervalFieldValidation,
queryFieldValidation,
} from './validations';

export const formSchema = {
export const createFormSchema = (ids: Set<string>) => ({
id: {
type: FIELD_TYPES.TEXT,
label: i18n.translate('xpack.osquery.scheduledQueryGroup.queryFlyoutForm.idFieldLabel', {
defaultMessage: 'ID',
}),
validations: idFieldValidations.map((validator) => ({ validator })),
validations: createIdFieldValidations(ids).map((validator) => ({ validator })),
},
description: {
type: FIELD_TYPES.TEXT,
Expand Down Expand Up @@ -69,4 +73,4 @@ export const formSchema = {
) as unknown) as string,
validations: [],
},
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import { isArray } from 'lodash';
import uuid from 'uuid';
import { produce } from 'immer';

import { useMemo } from 'react';
import { FormConfig, useForm } from '../../shared_imports';
import { OsqueryManagerPackagePolicyConfigRecord } from '../../../common/types';
import { formSchema } from './schema';
import { useScheduledQueryGroups } from '../use_scheduled_query_groups';
import { createFormSchema } from './schema';

const FORM_ID = 'editQueryFlyoutForm';

Expand All @@ -34,8 +36,28 @@ export interface ScheduledQueryGroupFormData {
export const useScheduledQueryGroupQueryForm = ({
defaultValue,
handleSubmit,
}: UseScheduledQueryGroupQueryFormProps) =>
useForm<OsqueryManagerPackagePolicyConfigRecord, ScheduledQueryGroupFormData>({
}: UseScheduledQueryGroupQueryFormProps) => {
const { data } = useScheduledQueryGroups();
const ids = useMemo<string[]>(
() =>
data?.items
.map((it) =>
it.inputs.map((input) => input.streams.map((stream) => stream.compiled_stream.id)).flat()
lykkin marked this conversation as resolved.
Show resolved Hide resolved
)
.flat() ?? [],
[data]
);
const idSet = useMemo<Set<string>>(() => {
const res = new Set<string>(ids);
if (defaultValue && defaultValue.id.value) {
res.delete(defaultValue.id.value);
}
return res;
}, [ids, defaultValue]);
const formSchema = useMemo<ReturnType<typeof createFormSchema>>(() => createFormSchema(idSet), [
idSet,
]);
return useForm<OsqueryManagerPackagePolicyConfigRecord, ScheduledQueryGroupFormData>({
id: FORM_ID + uuid.v4(),
onSubmit: handleSubmit,
options: {
Expand Down Expand Up @@ -75,3 +97,4 @@ export const useScheduledQueryGroupQueryForm = ({
},
schema: formSchema,
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,28 @@ const idSchemaValidation: ValidationFunc<any, string, string> = ({ value }) => {
}
};

export const idFieldValidations = [
const createUniqueIdValidation = (ids: Set<string>) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const uniqueIdCheck: ValidationFunc<any, string, string> = ({ value }) => {
if (ids.has(value)) {
return {
message: i18n.translate('xpack.osquery.scheduledQueryGroup.queryFlyoutForm.uniqueIdError', {
defaultMessage: 'ID must be unique',
}),
};
}
};
return uniqueIdCheck;
};

export const createIdFieldValidations = (ids: Set<string>) => [
fieldValidators.emptyField(
i18n.translate('xpack.osquery.scheduledQueryGroup.queryFlyoutForm.emptyIdError', {
defaultMessage: 'ID is required',
})
),
idSchemaValidation,
createUniqueIdValidation(ids),
];

export const intervalFieldValidation: ValidationFunc<
Expand Down
23 changes: 7 additions & 16 deletions x-pack/plugins/osquery/server/routes/usage/recorder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { usageMetricSavedObjectType } from '../../../common/types';

import {
CounterValue,
createMetricObjects,
getOrCreateMetricObject,
getRouteMetric,
incrementCount,
RouteString,
Expand Down Expand Up @@ -45,31 +45,22 @@ describe('Usage metric recorder', () => {
get.mockClear();
create.mockClear();
});
it('should seed route metrics objects', async () => {
it('should create metrics that do not exist', async () => {
get.mockRejectedValueOnce('stub value');
create.mockReturnValueOnce('stub value');
const result = await createMetricObjects(savedObjectsClient);
const result = await getOrCreateMetricObject(savedObjectsClient, 'live_query');
checkGetCalls(get.mock.calls);
checkCreateCalls(create.mock.calls);
expect(result).toBe(true);
expect(result).toBe('stub value');
});

it('should handle previously seeded objects properly', async () => {
it('should handle previously created objects properly', async () => {
get.mockReturnValueOnce('stub value');
create.mockRejectedValueOnce('stub value');
const result = await createMetricObjects(savedObjectsClient);
const result = await getOrCreateMetricObject(savedObjectsClient, 'live_query');
checkGetCalls(get.mock.calls);
checkCreateCalls(create.mock.calls, []);
expect(result).toBe(true);
});

it('should report failure to create the metrics object', async () => {
get.mockRejectedValueOnce('stub value');
create.mockRejectedValueOnce('stub value');
const result = await createMetricObjects(savedObjectsClient);
checkGetCalls(get.mock.calls);
checkCreateCalls(create.mock.calls);
expect(result).toBe(false);
expect(result).toBe('stub value');
});
});

Expand Down
40 changes: 19 additions & 21 deletions x-pack/plugins/osquery/server/routes/usage/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,28 @@ export type RouteString = 'live_query';

export const routeStrings: RouteString[] = ['live_query'];

export async function createMetricObjects(soClient: SavedObjectsClientContract) {
const res = await Promise.allSettled(
routeStrings.map(async (route) => {
try {
await soClient.get(usageMetricSavedObjectType, route);
} catch (e) {
await soClient.create(
usageMetricSavedObjectType,
{
errors: 0,
count: 0,
},
{
id: route,
}
);
export async function getOrCreateMetricObject<T>(
soClient: SavedObjectsClientContract,
route: string
) {
try {
return await soClient.get<T>(usageMetricSavedObjectType, route);
} catch (e) {
return await soClient.create(
usageMetricSavedObjectType,
{
errors: 0,
count: 0,
},
{
id: route,
}
})
);
return !res.some((e) => e.status === 'rejected');
);
}
}

export async function getCount(soClient: SavedObjectsClientContract, route: RouteString) {
return await soClient.get<LiveQuerySessionUsage>(usageMetricSavedObjectType, route);
return await getOrCreateMetricObject<LiveQuerySessionUsage>(soClient, route);
}

export interface CounterValue {
Expand All @@ -55,7 +53,7 @@ export async function incrementCount(
key: keyof CounterValue = 'count',
increment = 1
) {
const metric = await soClient.get<CounterValue>(usageMetricSavedObjectType, route);
const metric = await getOrCreateMetricObject<CounterValue>(soClient, route);
metric.attributes[key] += increment;
await soClient.update(usageMetricSavedObjectType, route, metric.attributes);
}
Expand Down
6 changes: 1 addition & 5 deletions x-pack/plugins/osquery/server/usage/collector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import { CoreSetup, SavedObjectsClient } from '../../../../../src/core/server';
import { CollectorFetchContext } from '../../../../../src/plugins/usage_collection/server';
import { createMetricObjects } from '../routes/usage';
import { getBeatUsage, getLiveQueryUsage, getPolicyLevelUsage } from './fetchers';
import { CollectorDependencies, usageSchema, UsageData } from './types';

Expand All @@ -25,10 +24,7 @@ export const registerCollector: RegisterCollector = ({ core, osqueryContext, usa
const collector = usageCollection.makeUsageCollector<UsageData>({
type: 'osquery',
schema: usageSchema,
isReady: async () => {
const savedObjectsClient = new SavedObjectsClient(await getInternalSavedObjectsClient(core));
return await createMetricObjects(savedObjectsClient);
},
isReady: () => true,
fetch: async ({ esClient }: CollectorFetchContext): Promise<UsageData> => {
const savedObjectsClient = new SavedObjectsClient(await getInternalSavedObjectsClient(core));
return {
Expand Down
8 changes: 7 additions & 1 deletion x-pack/plugins/osquery/server/usage/fetchers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ export async function getPolicyLevelUsage(
const agentResponse = await esClient.search({
body: {
size: 0,
query: {
match: {
active: true,
},
},
aggs: {
policied: {
filter: {
Expand Down Expand Up @@ -87,7 +92,8 @@ export function getScheduledQueryUsage(packagePolicies: ListResult<PackagePolicy
return packagePolicies.items.reduce(
(acc, item) => {
++acc.queryGroups.total;
if (item.inputs.length === 0) {
const policyAgents = item.inputs.reduce((sum, input) => sum + input.streams.length, 0);
if (policyAgents === 0) {
++acc.queryGroups.empty;
}
return acc;
Expand Down