Skip to content

Commit

Permalink
[eslint] no_restricted_paths config cleanup (elastic#63741)
Browse files Browse the repository at this point in the history
Major cleanup of the no_restricted_paths rule for imports of core.

For relative imports, we use eslint-module-utils/resolve which resolves
to the full filesystem path. So, to support relative and absolute
imports from the src alias we need to define both the directory and the
index including file extension.

This rule was handling both core imports, as well as imports from other
plugins. Imports from other plugins are being used much more liberally
allowed through the exceptions in tests. I choose to break these up,
removing this exception for tests for core imports.

Fixes:
Absolute imports of src/core/server/mocks were not allowed in src. This
was not an issue in x-pack due to the target excluding
!x-pack/**/*.test.* and !x-pack/test/**/*.

Non-top-level public and server imports were allowed from X-Pack tests
to the previously mentioned exclusion.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>
  • Loading branch information
Tyler Smalley committed Apr 23, 2020
1 parent 9e11bcb commit feed406
Show file tree
Hide file tree
Showing 48 changed files with 171 additions and 137 deletions.
39 changes: 24 additions & 15 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,31 +185,40 @@ module.exports = {
zones: [
{
target: [
'src/legacy/**/*',
'x-pack/**/*',
'!x-pack/**/*.test.*',
'!x-pack/test/**/*',
'(src|x-pack)/legacy/**/*',
'(src|x-pack)/plugins/**/(public|server)/**/*',
'src/core/(public|server)/**/*',
'examples/**/*',
],
from: [
'src/core/public/**/*',
'!src/core/public/index.ts',
'!src/core/public/mocks.ts',
'!src/core/public/*.test.mocks.ts',
'!src/core/public/index.ts', // relative import
'!src/core/public/mocks{,.ts}',
'!src/core/server/types{,.ts}',
'!src/core/public/utils/**/*',
'!src/core/public/*.test.mocks{,.ts}',

'src/core/server/**/*',
'!src/core/server/index.ts',
'!src/core/server/mocks.ts',
'!src/core/server/types.ts',
'!src/core/server/test_utils.ts',
'!src/core/server/index.ts', // relative import
'!src/core/server/mocks{,.ts}',
'!src/core/server/types{,.ts}',
'!src/core/server/test_utils',
// for absolute imports until fixed in
// https://github.com/elastic/kibana/issues/36096
'!src/core/server/types',
'!src/core/server/*.test.mocks.ts',

'!src/core/server/*.test.mocks{,.ts}',
],
allowSameFolder: true,
errorMessage:
'Plugins may only import from top-level public and server modules in core.',
},
{
target: [
'(src|x-pack)/legacy/**/*',
'(src|x-pack)/plugins/**/(public|server)/**/*',
'examples/**/*',
'!(src|x-pack)/**/*.test.*',
'!(x-pack/)?test/**/*',
],
from: [
'(src|x-pack)/plugins/**/(public|server)/**/*',
'!(src|x-pack)/plugins/**/(public|server)/(index|mocks).{js,ts,tsx}',
],
Expand Down
1 change: 1 addition & 0 deletions src/core/public/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@
*/

export { HttpService } from './http_service';
export { HttpFetchError } from './http_fetch_error';
export * from './types';
1 change: 1 addition & 0 deletions src/core/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export {
export {
HttpHeadersInit,
HttpRequestInit,
HttpFetchError,
HttpFetchOptions,
HttpFetchOptionsWithPath,
HttpFetchQuery,
Expand Down
17 changes: 17 additions & 0 deletions src/core/public/public.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,23 @@ export type HandlerFunction<T extends object> = (context: T, ...args: any[]) =>
// @public
export type HandlerParameters<T extends HandlerFunction<any>> = T extends (context: any, ...args: infer U) => any ? U : never;

// @internal (undocumented)
export class HttpFetchError extends Error implements IHttpFetchError {
constructor(message: string, name: string, request: Request, response?: Response | undefined, body?: any);
// (undocumented)
readonly body?: any;
// (undocumented)
readonly name: string;
// (undocumented)
readonly req: Request;
// (undocumented)
readonly request: Request;
// (undocumented)
readonly res?: Response;
// (undocumented)
readonly response?: Response | undefined;
}

// @public
export interface HttpFetchOptions extends HttpRequestInit {
asResponse?: boolean;
Expand Down
1 change: 1 addition & 0 deletions src/core/server/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export { elasticsearchServiceMock } from './elasticsearch/elasticsearch_service.
export { httpServiceMock } from './http/http_service.mock';
export { loggingServiceMock } from './logging/logging_service.mock';
export { savedObjectsRepositoryMock } from './saved_objects/service/lib/repository.mock';
export { savedObjectsServiceMock } from './saved_objects/saved_objects_service.mock';
export { typeRegistryMock as savedObjectsTypeRegistryMock } from './saved_objects/saved_objects_type_registry.mock';
export { uiSettingsServiceMock } from './ui_settings/ui_settings_service.mock';
export { metricsServiceMock } from './metrics/metrics_service.mock';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
import React from 'react';
import { EmptyStateComponent } from '../empty_state';
import { StatesIndexStatus } from '../../../../../common/runtime_types';
import { IHttpFetchError } from '../../../../../../../../../target/types/core/public/http';
import { HttpFetchError } from '../../../../../../../../../src/core/public/http/http_fetch_error';
import { HttpFetchError, IHttpFetchError } from 'src/core/public';
import { mountWithRouter, shallowWithRouter } from '../../../../lib';

describe('EmptyState component', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { fetchSnapshotCount } from '../snapshot';
import { apiService } from '../utils';
import { HttpFetchError } from '../../../../../../../../src/core/public/http/http_fetch_error';
import { HttpFetchError } from 'src/core/public';

describe('snapshot API', () => {
let fetchMock: jest.SpyInstance<Partial<unknown>>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { call, put } from 'redux-saga/effects';
import { fetchEffectFactory } from '../fetch_effect';
import { indexStatusAction } from '../../actions';
import { HttpFetchError } from '../../../../../../../../src/core/public/http/http_fetch_error';
import { HttpFetchError } from 'src/core/public';
import { StatesIndexStatus } from '../../../../common/runtime_types';
import { fetchIndexStatus } from '../../api';

Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugins/actions/server/routes/create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { createActionRoute } from './create';
import { mockRouter, RouterMock } from '../../../../../src/core/server/http/router/router.mock';
import { httpServiceMock } from 'src/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess, ActionTypeDisabledError } from '../lib';
import { mockHandlerArguments } from './_mock_handler_arguments';
Expand All @@ -20,7 +20,7 @@ beforeEach(() => {
describe('createActionRoute', () => {
it('creates an action with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

createActionRoute(router, licenseState);

Expand Down Expand Up @@ -83,7 +83,7 @@ describe('createActionRoute', () => {

it('ensures the license allows creating actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

createActionRoute(router, licenseState);

Expand All @@ -107,7 +107,7 @@ describe('createActionRoute', () => {

it('ensures the license check prevents creating actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('OMG');
Expand Down Expand Up @@ -135,7 +135,7 @@ describe('createActionRoute', () => {

it('ensures the action type gets validated for the license', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

createActionRoute(router, licenseState);

Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/actions/server/routes/delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { deleteActionRoute } from './delete';
import { mockRouter, RouterMock } from '../../../../../src/core/server/http/router/router.mock';
import { httpServiceMock } from 'src/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib';
import { mockHandlerArguments } from './_mock_handler_arguments';
Expand All @@ -20,7 +20,7 @@ beforeEach(() => {
describe('deleteActionRoute', () => {
it('deletes an action with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

deleteActionRoute(router, licenseState);

Expand Down Expand Up @@ -65,7 +65,7 @@ describe('deleteActionRoute', () => {

it('ensures the license allows deleting actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

deleteActionRoute(router, licenseState);

Expand All @@ -86,7 +86,7 @@ describe('deleteActionRoute', () => {

it('ensures the license check prevents deleting actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('OMG');
Expand Down
12 changes: 6 additions & 6 deletions x-pack/plugins/actions/server/routes/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { executeActionRoute } from './execute';
import { mockRouter, RouterMock } from '../../../../../src/core/server/http/router/router.mock';
import { httpServiceMock } from 'src/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { ActionExecutorContract, verifyApiAccess, ActionTypeDisabledError } from '../lib';
Expand All @@ -21,7 +21,7 @@ beforeEach(() => {
describe('executeActionRoute', () => {
it('executes an action with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

const [context, req, res] = mockHandlerArguments(
{},
Expand Down Expand Up @@ -77,7 +77,7 @@ describe('executeActionRoute', () => {

it('returns a "204 NO CONTENT" when the executor returns a nullish value', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

const [context, req, res] = mockHandlerArguments(
{},
Expand Down Expand Up @@ -115,7 +115,7 @@ describe('executeActionRoute', () => {

it('ensures the license allows action execution', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

const [context, req, res] = mockHandlerArguments(
{},
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('executeActionRoute', () => {

it('ensures the license check prevents action execution', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('OMG');
Expand Down Expand Up @@ -183,7 +183,7 @@ describe('executeActionRoute', () => {

it('ensures the action type gets validated for the license', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

const [context, req, res] = mockHandlerArguments(
{},
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/actions/server/routes/get.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { getActionRoute } from './get';
import { mockRouter, RouterMock } from '../../../../../src/core/server/http/router/router.mock';
import { httpServiceMock } from 'src/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib';
import { mockHandlerArguments } from './_mock_handler_arguments';
Expand All @@ -21,7 +21,7 @@ beforeEach(() => {
describe('getActionRoute', () => {
it('gets an action with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

getActionRoute(router, licenseState);

Expand Down Expand Up @@ -75,7 +75,7 @@ describe('getActionRoute', () => {

it('ensures the license allows getting actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

getActionRoute(router, licenseState);

Expand Down Expand Up @@ -105,7 +105,7 @@ describe('getActionRoute', () => {

it('ensures the license check prevents getting actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('OMG');
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/actions/server/routes/get_all.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { getAllActionRoute } from './get_all';
import { mockRouter, RouterMock } from '../../../../../src/core/server/http/router/router.mock';
import { httpServiceMock } from 'src/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib';
import { mockHandlerArguments } from './_mock_handler_arguments';
Expand All @@ -21,7 +21,7 @@ beforeEach(() => {
describe('getAllActionRoute', () => {
it('get all actions with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

getAllActionRoute(router, licenseState);

Expand Down Expand Up @@ -57,7 +57,7 @@ describe('getAllActionRoute', () => {

it('ensures the license allows getting all actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

getAllActionRoute(router, licenseState);

Expand Down Expand Up @@ -85,7 +85,7 @@ describe('getAllActionRoute', () => {

it('ensures the license check prevents getting all actions', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('OMG');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { listActionTypesRoute } from './list_action_types';
import { mockRouter, RouterMock } from '../../../../../src/core/server/http/router/router.mock';
import { httpServiceMock } from 'src/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib';
import { mockHandlerArguments } from './_mock_handler_arguments';
Expand All @@ -21,7 +21,7 @@ beforeEach(() => {
describe('listActionTypesRoute', () => {
it('lists action types with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

listActionTypesRoute(router, licenseState);

Expand Down Expand Up @@ -67,7 +67,7 @@ describe('listActionTypesRoute', () => {

it('ensures the license allows listing action types', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

listActionTypesRoute(router, licenseState);

Expand Down Expand Up @@ -105,7 +105,7 @@ describe('listActionTypesRoute', () => {

it('ensures the license check prevents listing action types', async () => {
const licenseState = licenseStateMock.create();
const router: RouterMock = mockRouter.create();
const router = httpServiceMock.createRouter();

(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('OMG');
Expand Down
Loading

0 comments on commit feed406

Please sign in to comment.