Skip to content

Commit

Permalink
Merge branch 'master' into AndyHunt66-patch-1
Browse files Browse the repository at this point in the history
  • Loading branch information
kibanamachine authored Nov 30, 2020
2 parents ba318ee + cb55898 commit d336ec0
Show file tree
Hide file tree
Showing 110 changed files with 8,561 additions and 739 deletions.
76 changes: 74 additions & 2 deletions docs/developer/plugin/migrating-legacy-plugins-examples.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -902,8 +902,9 @@ The most significant changes on the Kibana side for the consumers are the follow
===== User client accessor
Internal /current user client accessors has been renamed and are now
properties instead of functions:
** `callAsInternalUser('ping')` -> `asInternalUser.ping()`
** `callAsCurrentUser('ping')` -> `asCurrentUser.ping()`

* `callAsInternalUser('ping')` -> `asInternalUser.ping()`
* `callAsCurrentUser('ping')` -> `asCurrentUser.ping()`
* the API now reflects the `Client`’s instead of leveraging the
string-based endpoint names the `LegacyAPICaller` was using.

Expand Down Expand Up @@ -1142,6 +1143,77 @@ router.get(
);
----

==== Accessing the client from a collector's `fetch` method

At the moment, the `fetch` method's context receives preconfigured
<<scoped-services, scoped clients>> for Elasticsearch and SavedObjects.
To help in the transition, both, the legacy (`callCluster`) and new clients are provided,
but we strongly discourage using the deprecated legacy ones for any new implementation.

[source,typescript]
----
usageCollection.makeUsageCollector<MyUsage>({
type: 'my-collector',
isReady: async () => true, // Logic to confirm the `fetch` method is ready to be called
schema: {...},
async fetch(context) {
const { callCluster, esClient, soClient } = context;
// Before:
const result = callCluster('search', options)
// After:
const { body: result } = esClient.search(options);
return result;
}
});
----

Regarding the `soClient`, it is encouraged to use it instead of the plugin's owned SavedObject's repository
as we used to do in the past.

Before:

[source,typescript]
----
function getUsageCollector(
usageCollection: UsageCollectionSetup,
getSavedObjectsRepository: () => ISavedObjectsRepository | undefined
) {
usageCollection.makeUsageCollector<MyUsage>({
type: 'my-collector',
isReady: () => typeof getSavedObjectsRepository() !== 'undefined',
schema: {...},
async fetch() {
const savedObjectsRepository = getSavedObjectsRepository();
const { attributes: result } = await savedObjectsRepository.get('my-so-type', 'my-so-id');
return result;
}
});
}
----

After:

[source,typescript]
----
function getUsageCollector(usageCollection: UsageCollectionSetup) {
usageCollection.makeUsageCollector<MyUsage>({
type: 'my-collector',
isReady: () => true,
schema: {...},
async fetch({ soClient }) {
const { attributes: result } = await soClient.get('my-so-type', 'my-so-id');
return result;
}
});
}
----

==== Creating a custom client

Note that the `plugins` option is no longer available on the new
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ kibana_vars=(
xpack.code.security.gitProtocolWhitelist
xpack.encryptedSavedObjects.encryptionKey
xpack.encryptedSavedObjects.keyRotation.decryptionOnlyKeys
xpack.fleet.agents.elasticsearch.host
xpack.fleet.agents.kibana.host
xpack.fleet.agents.tlsCheckDisabled
xpack.graph.enabled
xpack.graph.canEditDrillDownUrls
xpack.graph.savePolicy
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/discover/public/application/angular/discover.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
if (!_.isEqual(newStatePartial, oldStatePartial)) {
$scope.$evalAsync(async () => {
if (oldStatePartial.index !== newStatePartial.index) {
//in case of index switch the route has currently to be reloaded, legacy
//in case of index pattern switch the route has currently to be reloaded, legacy
$route.reload();
return;
}

Expand Down Expand Up @@ -289,8 +290,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
$scope.state.sort,
config.get(MODIFY_COLUMNS_ON_SWITCH)
);
await replaceUrlAppState(nextAppState);
$route.reload();
await setAppState(nextAppState);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const applicationUsageSchema = {
logs: commonSchema,
metrics: commonSchema,
infra: commonSchema, // It's a forward app so we'll likely never report it
ingestManager: commonSchema,
fleet: commonSchema,
lens: commonSchema,
maps: commonSchema,
ml: commonSchema,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('telemetry_application_usage', () => {

const logger = loggingSystemMock.createLogger();

let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function getCoreUsageCollector(
usageCollection: UsageCollectionSetup,
getCoreUsageDataService: () => CoreUsageDataStart
) {
return usageCollection.makeUsageCollector<CoreUsageData, { core: CoreUsageData }>({
return usageCollection.makeUsageCollector<CoreUsageData>({
type: 'core',
isReady: () => typeof getCoreUsageDataService() !== 'undefined',
schema: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { CoreUsageData } from 'src/core/server/';
const logger = loggingSystemMock.createLogger();

describe('telemetry_core', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { registerKibanaUsageCollector } from './';
const logger = loggingSystemMock.createLogger();

describe('telemetry_kibana', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down Expand Up @@ -66,23 +66,4 @@ describe('telemetry_kibana', () => {
timelion_sheet: { total: 0 },
});
});

test('formatForBulkUpload', async () => {
const resultFromFetch = {
index: '.kibana-tests',
dashboard: { total: 0 },
visualization: { total: 0 },
search: { total: 0 },
index_pattern: { total: 0 },
graph_workspace: { total: 0 },
timelion_sheet: { total: 0 },
};

expect(collector.formatForBulkUpload!(resultFromFetch)).toStrictEqual({
type: 'kibana_stats',
payload: {
usage: resultFromFetch,
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { Observable } from 'rxjs';
import { take } from 'rxjs/operators';
import { SharedGlobalConfig } from 'kibana/server';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { KIBANA_STATS_TYPE } from '../../../common/constants';
import { getSavedObjectsCounts, KibanaSavedObjectCounts } from './get_saved_object_counts';

interface KibanaUsage extends KibanaSavedObjectCounts {
Expand All @@ -32,7 +31,7 @@ export function getKibanaUsageCollector(
usageCollection: UsageCollectionSetup,
legacyConfig$: Observable<SharedGlobalConfig>
) {
return usageCollection.makeUsageCollector<KibanaUsage, { usage: KibanaUsage }>({
return usageCollection.makeUsageCollector<KibanaUsage>({
type: 'kibana',
isReady: () => true,
schema: {
Expand All @@ -53,20 +52,6 @@ export function getKibanaUsageCollector(
...(await getSavedObjectsCounts(callCluster, index)),
};
},

/*
* Format the response data into a model for internal upload
* 1. Make this data part of the "kibana_stats" type
* 2. Organize the payload in the usage namespace of the data payload (usage.index, etc)
*/
formatForBulkUpload: (result) => {
return {
type: KIBANA_STATS_TYPE,
payload: {
usage: result,
},
};
},
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { registerManagementUsageCollector } from './';
const logger = loggingSystemMock.createLogger();

describe('telemetry_application_usage_collector', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { loggingSystemMock } from '../../../../../core/server/mocks';
const logger = loggingSystemMock.createLogger();

describe('telemetry_ops_stats', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeStatsCollector.mockImplementation((config) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { registerUiMetricUsageCollector } from './';
const logger = loggingSystemMock.createLogger();

describe('telemetry_ui_metric', () => {
let collector: Collector<unknown, unknown>;
let collector: Collector<unknown>;

const usageCollectionMock = createUsageCollectionSetupMock();
usageCollectionMock.makeUsageCollector.mockImplementation((config) => {
Expand Down
45 changes: 24 additions & 21 deletions src/plugins/share/server/routes/lib/short_url_assert_valid.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,36 +19,39 @@

import { shortUrlAssertValid } from './short_url_assert_valid';

const PROTOCOL_ERROR = /^Short url targets cannot have a protocol/;
const HOSTNAME_ERROR = /^Short url targets cannot have a hostname/;
const PATH_ERROR = /^Short url target path must be in the format/;

describe('shortUrlAssertValid()', () => {
const invalid = [
['protocol', 'http://localhost:5601/app/kibana'],
['protocol', 'https://localhost:5601/app/kibana'],
['protocol', 'mailto:[email protected]'],
['protocol', 'javascript:alert("hi")'], // eslint-disable-line no-script-url
['hostname', 'localhost/app/kibana'],
['hostname and port', 'local.host:5601/app/kibana'],
['hostname and auth', 'user:[email protected]/app/kibana'],
['path traversal', '/app/../../not-kibana'],
['deep path', '/app/kibana/foo'],
['deep path', '/app/kibana/foo/bar'],
['base path', '/base/app/kibana'],
['protocol', 'http://localhost:5601/app/kibana', PROTOCOL_ERROR],
['protocol', 'https://localhost:5601/app/kibana', PROTOCOL_ERROR],
['protocol', 'mailto:[email protected]', PROTOCOL_ERROR],
['protocol', 'javascript:alert("hi")', PROTOCOL_ERROR], // eslint-disable-line no-script-url
['hostname', 'localhost/app/kibana', PATH_ERROR], // according to spec, this is not a valid URL -- you cannot specify a hostname without a protocol
['hostname and port', 'local.host:5601/app/kibana', PROTOCOL_ERROR], // parser detects 'local.host' as the protocol
['hostname and auth', 'user:[email protected]/app/kibana', PROTOCOL_ERROR], // parser detects 'user' as the protocol
['path traversal', '/app/../../not-kibana', PATH_ERROR], // fails because there are >2 path parts
['path traversal', '/../not-kibana', PATH_ERROR], // fails because first path part is not 'app'
['deep path', '/app/kibana/foo', PATH_ERROR], // fails because there are >2 path parts
['deeper path', '/app/kibana/foo/bar', PATH_ERROR], // fails because there are >2 path parts
['base path', '/base/app/kibana', PATH_ERROR], // fails because there are >2 path parts
['path with an extra leading slash', '//foo/app/kibana', HOSTNAME_ERROR], // parser detects 'foo' as the hostname
['path with an extra leading slash', '///app/kibana', HOSTNAME_ERROR], // parser detects '' as the hostname
['path without app', '/foo/kibana', PATH_ERROR], // fails because first path part is not 'app'
['path without appId', '/app/', PATH_ERROR], // fails because there is only one path part (leading and trailing slashes are trimmed)
];

invalid.forEach(([desc, url]) => {
it(`fails when url has ${desc}`, () => {
try {
shortUrlAssertValid(url);
throw new Error(`expected assertion to throw`);
} catch (err) {
if (!err || !err.isBoom) {
throw err;
}
}
invalid.forEach(([desc, url, error]) => {
it(`fails when url has ${desc as string}`, () => {
expect(() => shortUrlAssertValid(url as string)).toThrowError(error);
});
});

const valid = [
'/app/kibana',
'/app/kibana/', // leading and trailing slashes are trimmed
'/app/monitoring#angular/route',
'/app/text#document-id',
'/app/some?with=query',
Expand Down
12 changes: 8 additions & 4 deletions src/plugins/share/server/routes/lib/short_url_assert_valid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,22 @@ import { trim } from 'lodash';
import Boom from '@hapi/boom';

export function shortUrlAssertValid(url: string) {
const { protocol, hostname, pathname } = parse(url);
const { protocol, hostname, pathname } = parse(
url,
false /* parseQueryString */,
true /* slashesDenoteHost */
);

if (protocol) {
if (protocol !== null) {
throw Boom.notAcceptable(`Short url targets cannot have a protocol, found "${protocol}"`);
}

if (hostname) {
if (hostname !== null) {
throw Boom.notAcceptable(`Short url targets cannot have a hostname, found "${hostname}"`);
}

const pathnameParts = trim(pathname === null ? undefined : pathname, '/').split('/');
if (pathnameParts.length !== 2) {
if (pathnameParts.length !== 2 || pathnameParts[0] !== 'app' || !pathnameParts[1]) {
throw Boom.notAcceptable(
`Short url target path must be in the format "/app/{{appId}}", found "${pathname}"`
);
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/telemetry/schema/oss_plugins.json
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@
}
}
},
"ingestManager": {
"fleet": {
"properties": {
"clicks_total": {
"type": "long"
Expand Down
Loading

0 comments on commit d336ec0

Please sign in to comment.