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

[index patterns] Add pattern validation method to index patterns fetcher #90170

Merged
merged 8 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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 @@ -22,4 +22,5 @@ export declare class IndexPatternsFetcher
| --- | --- | --- |
| [getFieldsForTimePattern(options)](./kibana-plugin-plugins-data-server.indexpatternsfetcher.getfieldsfortimepattern.md) | | Get a list of field objects for a time pattern |
| [getFieldsForWildcard(options)](./kibana-plugin-plugins-data-server.indexpatternsfetcher.getfieldsforwildcard.md) | | Get a list of field objects for an index pattern that may contain wildcards |
| [validatePatternListActive(patternList)](./kibana-plugin-plugins-data-server.indexpatternsfetcher.validatepatternlistactive.md) | | Returns an index pattern list of only those index pattern strings in the given list that return indices |

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-data-server](./kibana-plugin-plugins-data-server.md) &gt; [IndexPatternsFetcher](./kibana-plugin-plugins-data-server.indexpatternsfetcher.md) &gt; [validatePatternListActive](./kibana-plugin-plugins-data-server.indexpatternsfetcher.validatepatternlistactive.md)

## IndexPatternsFetcher.validatePatternListActive() method

Returns an index pattern list of only those index pattern strings in the given list that return indices

<b>Signature:</b>

```typescript
validatePatternListActive(patternList: string[]): Promise<string[]>;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| patternList | <code>string[]</code> | |

<b>Returns:</b>

`Promise<string[]>`

Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { IndexPatternsFetcher } from '.';
import { ElasticsearchClient } from 'kibana/server';

describe('Index Pattern Fetcher - server', () => {
let indexPatterns: IndexPatternsFetcher;
let esClient: ElasticsearchClient;
const emptyResponse = {
body: {
indices: [],
},
};
const response = {
body: {
indices: ['hello', 'world'],
},
};
const patternList = ['a', 'b', 'c'];
beforeEach(() => {
esClient = ({
transport: {
request: jest.fn().mockReturnValueOnce(emptyResponse).mockReturnValue(response),
},
} as unknown) as ElasticsearchClient;
indexPatterns = new IndexPatternsFetcher(esClient);
});

it('Removes pattern without matching indices', async () => {
const result = await indexPatterns.validatePatternListActive(patternList);
expect(result).toEqual(['b', 'c']);
});

it('Returns all patterns when all match indices', async () => {
esClient = ({
transport: {
request: jest.fn().mockReturnValue(response),
},
} as unknown) as ElasticsearchClient;
indexPatterns = new IndexPatternsFetcher(esClient);
const result = await indexPatterns.validatePatternListActive(patternList);
expect(result).toEqual(patternList);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,24 @@ export class IndexPatternsFetcher {
rollupIndex?: string;
}): Promise<FieldDescriptor[]> {
const { pattern, metaFields, fieldCapsOptions, type, rollupIndex } = options;
const patternList = Array.isArray(pattern) ? pattern : pattern.split(',');
let patternListActive: string[] = patternList;
// if only one pattern, don't bother with validation. We let getFieldCapabilities fail if the single pattern is bad regardless
if (patternList.length > 1) {
patternListActive = await this.validatePatternListActive(patternList);
}
const fieldCapsResponse = await getFieldCapabilities(
this.elasticsearchClient,
pattern,
// if none of the patterns are active, pass the original list to get an error
patternListActive.length > 0 ? patternListActive : patternList,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if none of the patterns are active, pass the original list to get an error:
Screen Shot 2021-02-04 at 7 32 20 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to add this as a comment to the code. It definitely makes sense but its not obvious why this is being done.

metaFields,
{
allow_no_indices: fieldCapsOptions
? fieldCapsOptions.allow_no_indices
: this.allowNoIndices,
}
);

if (type === 'rollup' && rollupIndex) {
const rollupFields: FieldDescriptor[] = [];
const rollupIndexCapabilities = getCapabilitiesForRollupIndices(
Expand Down Expand Up @@ -118,4 +126,25 @@ export class IndexPatternsFetcher {
}
return await getFieldCapabilities(this.elasticsearchClient, indices, metaFields);
}

/**
* Returns an index pattern list of only those index pattern strings in the given list that return indices
*
* @param patternList string[]
* @return {Promise<string[]>}
*/
async validatePatternListActive(patternList: string[]) {
const result = await Promise.all(
patternList.map((pattern) =>
this.elasticsearchClient.count({
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 from the _resolve/indexapi to the count api as read only users got 403'd on the resolve. count works well: https://www.elastic.co/guide/en/elasticsearch/reference/current/search-count.html

Copy link
Contributor

Choose a reason for hiding this comment

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

Its annoying that the resolve endpoint works this way but this is a good choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes it is annoying. i would not have caught it if it weren't for that ML test!

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 also tried ElasticsearchClient.indices.exists and it appears read-only users get 403'd on those indices methods as well. I'm not sure if this has been taken into account for testing anywhere that uses the getFieldsForTimePattern which eventually calls ElasticsearchClient.indices.getAlias: https://github.com/elastic/kibana/blob/master/src/plugins/data/server/index_patterns/fetcher/lib/es_api.ts#L42

Copy link
Contributor

Choose a reason for hiding this comment

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

That code needs to be removed, time pattern index patterns were dropped in 7.x.

index: pattern,
})
)
);
return result.reduce(
(acc: string[], { body: { count } }, patternListIndex) =>
count > 0 ? [...acc, patternList[patternListIndex]] : acc,
[]
);
}
}
1 change: 1 addition & 0 deletions src/plugins/data/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,7 @@ export class IndexPatternsFetcher {
type?: string;
rollupIndex?: string;
}): Promise<FieldDescriptor[]>;
validatePatternListActive(patternList: string[]): Promise<string[]>;
}

// Warning: (ae-forgotten-export) The symbol "IndexPatternsServiceStart" needs to be exported by the entry point index.d.ts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,55 @@ export default function ({ getService }) {
expect(resp.body.fields).to.eql(sortBy(resp.body.fields, 'name'));
};

const testFields = [
{
type: 'boolean',
esTypes: ['boolean'],
searchable: true,
aggregatable: true,
name: 'bar',
readFromDocValues: true,
},
{
type: 'string',
esTypes: ['text'],
searchable: true,
aggregatable: false,
name: 'baz',
readFromDocValues: false,
},
{
type: 'string',
esTypes: ['keyword'],
searchable: true,
aggregatable: true,
name: 'baz.keyword',
readFromDocValues: true,
subType: { multi: { parent: 'baz' } },
},
{
type: 'number',
esTypes: ['long'],
searchable: true,
aggregatable: true,
name: 'foo',
readFromDocValues: true,
},
{
aggregatable: true,
esTypes: ['keyword'],
name: 'nestedField.child',
readFromDocValues: true,
searchable: true,
subType: {
nested: {
path: 'nestedField',
},
},
type: 'string',
},
];

describe('fields_for_wildcard_route response', () => {
before(() => esArchiver.load('index_patterns/basic_index'));
after(() => esArchiver.unload('index_patterns/basic_index'));
Expand All @@ -26,54 +75,7 @@ export default function ({ getService }) {
.get('/api/index_patterns/_fields_for_wildcard')
.query({ pattern: 'basic_index' })
.expect(200, {
fields: [
{
type: 'boolean',
esTypes: ['boolean'],
searchable: true,
aggregatable: true,
name: 'bar',
readFromDocValues: true,
},
{
type: 'string',
esTypes: ['text'],
searchable: true,
aggregatable: false,
name: 'baz',
readFromDocValues: false,
},
{
type: 'string',
esTypes: ['keyword'],
searchable: true,
aggregatable: true,
name: 'baz.keyword',
readFromDocValues: true,
subType: { multi: { parent: 'baz' } },
},
{
type: 'number',
esTypes: ['long'],
searchable: true,
aggregatable: true,
name: 'foo',
readFromDocValues: true,
},
{
aggregatable: true,
esTypes: ['keyword'],
name: 'nestedField.child',
readFromDocValues: true,
searchable: true,
subType: {
nested: {
path: 'nestedField',
},
},
type: 'string',
},
],
fields: testFields,
})
.then(ensureFieldsAreSorted);
});
Expand Down Expand Up @@ -162,11 +164,19 @@ export default function ({ getService }) {
.then(ensureFieldsAreSorted);
});

it('returns 404 when the pattern does not exist', async () => {
it('returns fields when one pattern exists and the other does not', async () => {
await supertest
.get('/api/index_patterns/_fields_for_wildcard')
.query({ pattern: 'bad_index,basic_index' })
.expect(200, {
fields: testFields,
});
});
it('returns 404 when no patterns exist', async () => {
await supertest
.get('/api/index_patterns/_fields_for_wildcard')
.query({
pattern: '[non-existing-pattern]its-invalid-*',
pattern: 'bad_index',
})
.expect(404);
});
Expand Down