Skip to content

Commit

Permalink
apply comments
Browse files Browse the repository at this point in the history
  • Loading branch information
WafaaNasr committed Feb 2, 2023
1 parent 680530f commit 31bcd73
Show file tree
Hide file tree
Showing 11 changed files with 131 additions and 38 deletions.
27 changes: 16 additions & 11 deletions x-pack/plugins/alerting/server/rules_client/lib/validate_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export async function validateActions(
data: Pick<RawRule, 'notifyWhen' | 'throttle' | 'schedule'> & {
actions: NormalizedAlertAction[];
},
skipMissingSecretsValidation?: boolean
allowMissingConnectorSecrets?: boolean
): Promise<void> {
const { actions, notifyWhen, throttle } = data;
const hasRuleLevelNotifyWhen = typeof notifyWhen !== 'undefined';
Expand All @@ -31,16 +31,21 @@ export async function validateActions(

const errors = [];

if (!skipMissingSecretsValidation) {
// check for actions using connectors with missing secrets
const actionsClient = await context.getActionsClient();
const actionIds = [...new Set(actions.map((action) => action.id))];
const actionResults = (await actionsClient.getBulk(actionIds)) || [];
const actionsUsingConnectorsWithMissingSecrets = actionResults.filter(
(result) => result.isMissingSecrets
);

if (actionsUsingConnectorsWithMissingSecrets.length) {
// check for actions using connectors with missing secrets
const actionsClient = await context.getActionsClient();
const actionIds = [...new Set(actions.map((action) => action.id))];
const actionResults = (await actionsClient.getBulk(actionIds)) || [];
const actionsUsingConnectorsWithMissingSecrets = actionResults.filter(
(result) => result.isMissingSecrets
);
if (actionsUsingConnectorsWithMissingSecrets.length) {
if (allowMissingConnectorSecrets) {
context.logger.error(
`Invalid connectors with "allowMissingConnectorSecrets": ${actionsUsingConnectorsWithMissingSecrets
.map((connector) => connector.name)
.join(', ')}`
);
} else {
errors.push(
i18n.translate('xpack.alerting.rulesClient.validateActions.misconfiguredConnector', {
defaultMessage: 'Invalid connectors: {groups}',
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/alerting/server/rules_client/methods/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ export interface CreateOptions<Params extends RuleTypeParams> {
| 'nextRun'
> & { actions: NormalizedAlertAction[] };
options?: SavedObjectOptions;
skipMissingSecretsValidation?: boolean;
allowMissingConnectorSecrets?: boolean;
}

export async function create<Params extends RuleTypeParams = never>(
context: RulesClientContext,
{ data, options, skipMissingSecretsValidation }: CreateOptions<Params>
{ data, options, allowMissingConnectorSecrets }: CreateOptions<Params>
): Promise<SanitizedRule<Params>> {
const id = options?.id || SavedObjectsUtils.generateId();

Expand Down Expand Up @@ -105,9 +105,9 @@ export async function create<Params extends RuleTypeParams = never>(
}
}

await validateActions(context, ruleType, data, skipMissingSecretsValidation);
await validateActions(context, ruleType, data, allowMissingConnectorSecrets);
await withSpan({ name: 'validateActions', type: 'rules' }, () =>
validateActions(context, ruleType, data, skipMissingSecretsValidation)
validateActions(context, ruleType, data, allowMissingConnectorSecrets)
);

// Throw error if schedule interval is less than the minimum and we are enforcing it
Expand Down
14 changes: 7 additions & 7 deletions x-pack/plugins/alerting/server/rules_client/methods/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,23 @@ export interface UpdateOptions<Params extends RuleTypeParams> {
throttle?: string | null;
notifyWhen?: RuleNotifyWhenType | null;
};
skipMissingSecretsValidation?: boolean;
allowMissingConnectorSecrets?: boolean;
}

export async function update<Params extends RuleTypeParams = never>(
context: RulesClientContext,
{ id, data, skipMissingSecretsValidation }: UpdateOptions<Params>
{ id, data, allowMissingConnectorSecrets }: UpdateOptions<Params>
): Promise<PartialRule<Params>> {
return await retryIfConflicts(
context.logger,
`rulesClient.update('${id}')`,
async () => await updateWithOCC<Params>(context, { id, data, skipMissingSecretsValidation })
async () => await updateWithOCC<Params>(context, { id, data, allowMissingConnectorSecrets })
);
}

async function updateWithOCC<Params extends RuleTypeParams>(
context: RulesClientContext,
{ id, data, skipMissingSecretsValidation }: UpdateOptions<Params>
{ id, data, allowMissingConnectorSecrets }: UpdateOptions<Params>
): Promise<PartialRule<Params>> {
let alertSavedObject: SavedObject<RawRule>;

Expand Down Expand Up @@ -102,7 +102,7 @@ async function updateWithOCC<Params extends RuleTypeParams>(

const updateResult = await updateAlert<Params>(
context,
{ id, data, skipMissingSecretsValidation },
{ id, data, allowMissingConnectorSecrets },
alertSavedObject
);

Expand Down Expand Up @@ -143,7 +143,7 @@ async function updateWithOCC<Params extends RuleTypeParams>(

async function updateAlert<Params extends RuleTypeParams>(
context: RulesClientContext,
{ id, data, skipMissingSecretsValidation }: UpdateOptions<Params>,
{ id, data, allowMissingConnectorSecrets }: UpdateOptions<Params>,
{ attributes, version }: SavedObject<RawRule>
): Promise<PartialRule<Params>> {
const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId);
Expand All @@ -161,7 +161,7 @@ async function updateAlert<Params extends RuleTypeParams>(

// Validate
const validatedAlertTypeParams = validateRuleTypeParams(data.params, ruleType.validate?.params);
await validateActions(context, ruleType, data, skipMissingSecretsValidation);
await validateActions(context, ruleType, data, allowMissingConnectorSecrets);

// Throw error if schedule interval is less than the minimum and we are enforcing it
const intervalInMs = parseDuration(data.schedule.interval);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3047,7 +3047,7 @@ describe('create()', () => {
expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
expect(taskManager.schedule).not.toHaveBeenCalled();
});
test('should create a rule even if action is missing secret when skipMissingSecretsValidation is true', async () => {
test('should create a rule even if action is missing secret when allowMissingConnectorSecrets is true', async () => {
const data = getMockData({
actions: [
{
Expand Down Expand Up @@ -3135,7 +3135,7 @@ describe('create()', () => {
},
references: [],
});
const result = await rulesClient.create({ data, skipMissingSecretsValidation: true });
const result = await rulesClient.create({ data, allowMissingConnectorSecrets: true });
expect(result).toMatchInlineSnapshot(`
Object {
"actions": Array [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2022,7 +2022,7 @@ describe('update()', () => {
expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
expect(taskManager.schedule).not.toHaveBeenCalled();
});
test('should update a rule even if action is missing secret when skipMissingSecretsValidation is true', async () => {
test('should update a rule even if action is missing secret when allowMissingConnectorSecrets is true', async () => {
// Reset from default behaviour
actionsClient.getBulk.mockReset();
actionsClient.getBulk.mockResolvedValue([
Expand Down Expand Up @@ -2098,7 +2098,7 @@ describe('update()', () => {
},
],
},
skipMissingSecretsValidation: true,
allowMissingConnectorSecrets: true,
});

expect(unsecuredSavedObjectsClient.create).toHaveBeenNthCalledWith(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export const importRulesRoute = (
exceptionsClient,
spaceId: ctx.securitySolution.getSpaceId(),
existingLists: foundReferencedExceptionLists,
skipMissingSecretsValidation: !!actionConnectors.length,
allowMissingConnectorSecrets: !!actionConnectors.length,
});

const errorsResp = importRuleResponse.filter((resp) => isBulkError(resp)) as BulkError[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export interface CreateRulesOptions<T extends RuleCreateProps = RuleCreateProps>
id?: string;
immutable?: boolean;
defaultEnabled?: boolean;
skipMissingSecretsValidation?: boolean;
allowMissingConnectorSecrets?: boolean;
}

export const createRules = async ({
Expand All @@ -28,15 +28,15 @@ export const createRules = async ({
id,
immutable = false,
defaultEnabled = true,
skipMissingSecretsValidation,
allowMissingConnectorSecrets,
}: CreateRulesOptions): Promise<SanitizedRule<RuleParams>> => {
const internalRule = convertCreateAPIToInternalSchema(params, immutable, defaultEnabled);
const rule = await rulesClient.create<RuleParams>({
options: {
id,
},
data: internalRule,
skipMissingSecretsValidation,
allowMissingConnectorSecrets,
});

// Mute the rule if it is first created with the explicit no actions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ export interface PatchRulesOptions {
rulesClient: RulesClient;
nextParams: PatchRuleRequestBody;
existingRule: RuleAlertType | null | undefined;
skipMissingSecretsValidation?: boolean;
allowMissingConnectorSecrets?: boolean;
}

export const patchRules = async ({
rulesClient,
existingRule,
nextParams,
skipMissingSecretsValidation,
allowMissingConnectorSecrets,
}: PatchRulesOptions): Promise<PartialRule<RuleParams> | null> => {
if (existingRule == null) {
return null;
Expand All @@ -34,7 +34,7 @@ export const patchRules = async ({
const update = await rulesClient.update({
id: existingRule.id,
data: patchedRule,
skipMissingSecretsValidation,
allowMissingConnectorSecrets,
});

if (nextParams.throttle !== undefined) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ describe('checkRuleExceptionReferences', () => {
jest.clearAllMocks();
});
it('should show an error message when the user has a Read Actions permission and stops the importing ', async () => {
// jest.resetAllMocks();
const newCore = coreMock.createRequestHandlerContext();
const error = {
output: { payload: { message: 'Unable to bulk_create action' }, statusCode: 403 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const importRules = async ({
exceptionsClient,
spaceId,
existingLists,
skipMissingSecretsValidation,
allowMissingConnectorSecrets,
}: {
ruleChunks: PromiseFromStreams[][];
rulesResponseAcc: ImportRuleResponse[];
Expand All @@ -72,7 +72,7 @@ export const importRules = async ({
exceptionsClient: ExceptionListClient | undefined;
spaceId: string;
existingLists: Record<string, ExceptionListSchema>;
skipMissingSecretsValidation?: boolean;
allowMissingConnectorSecrets?: boolean;
}) => {
let importRuleResponse: ImportRuleResponse[] = [...rulesResponseAcc];

Expand Down Expand Up @@ -121,7 +121,7 @@ export const importRules = async ({
...parsedRule,
exceptions_list: [...exceptions],
},
skipMissingSecretsValidation,
allowMissingConnectorSecrets,
});
resolve({
rule_id: parsedRule.rule_id,
Expand All @@ -140,7 +140,7 @@ export const importRules = async ({
...parsedRule,
exceptions_list: [...exceptions],
},
skipMissingSecretsValidation,
allowMissingConnectorSecrets,
});
resolve({
rule_id: parsedRule.rule_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,95 @@ export default ({ getService }: FtrProviderContext): void => {
missing_action_connections: [],
});
});
it('should export rules with actions connectors', async () => {
// create new actions
const webHookAction = await createWebHookConnector();

const defaultRuleAction = {
id: webHookAction.id,
action_type_id: '.webhook',
group: 'default',
params: {
body: '{"test":"a default action"}',
},
};

const ruleId = 'rule-1';
await createRule(supertest, log, {
...getSimpleRule(ruleId),
actions: [defaultRuleAction],
});
const exportedConnectors = {
attributes: {
actionTypeId: '.webhook',
config: {
hasAuth: true,
method: 'post',
url: 'http://localhost',
},
isMissingSecrets: true,
name: 'Some connector',
secrets: {},
},
coreMigrationVersion: '8.7.0',
id: webHookAction.id,
migrationVersion: {
action: '8.3.0',
},
references: [],
type: 'action',
};

const { body } = await postBulkAction()
.send({ query: '', action: BulkActionType.export })
.expect(200)
.expect('Content-Type', 'application/ndjson')
.expect('Content-Disposition', 'attachment; filename="rules_export.ndjson"')
.parse(binaryToString);

const [ruleJson, connectorsJson, exportDetailsJson] = body.toString().split(/\n/);

const rule = removeServerGeneratedProperties(JSON.parse(ruleJson));
expect(rule).to.eql({
...getSimpleRuleOutput(),
throttle: 'rule',
actions: [
{
action_type_id: '.webhook',
group: 'default',
id: webHookAction.id,
params: {
body: '{"test":"a default action"}',
},
},
],
});
const { attributes, id, type } = JSON.parse(connectorsJson);
expect(attributes.actionTypeId).to.eql(exportedConnectors.attributes.actionTypeId);
expect(id).to.eql(exportedConnectors.id);
expect(type).to.eql(exportedConnectors.type);
expect(attributes.name).to.eql(exportedConnectors.attributes.name);
expect(attributes.secrets).to.eql(exportedConnectors.attributes.secrets);
expect(attributes.isMissingSecrets).to.eql(exportedConnectors.attributes.isMissingSecrets);
const exportDetails = JSON.parse(exportDetailsJson);
expect(exportDetails).to.eql({
exported_exception_list_count: 0,
exported_exception_list_item_count: 0,
exported_count: 2,
exported_rules_count: 1,
missing_exception_list_item_count: 0,
missing_exception_list_items: [],
missing_exception_lists: [],
missing_exception_lists_count: 0,
missing_rules: [],
missing_rules_count: 0,
excluded_action_connection_count: 0,
excluded_action_connections: [],
exported_action_connector_count: 1,
missing_action_connection_count: 0,
missing_action_connections: [],
});
});

it('should delete rules', async () => {
const ruleId = 'ruleId';
Expand Down

0 comments on commit 31bcd73

Please sign in to comment.