Prevents importing of public code into server by tylersmalley · Pull Request #67149 · elastic/kibana

tylersmalley · 2020-05-21T00:08:42Z

We should not be allowing importing of public into server. Any shared code should reside in a common directory. After #66506, this will not even be possible as we will no longer be transpiling public code into commonjs.

Relates to #66506

We should not be allowing importing of public into server. Any shared code should reside in a common directory. After elastic#66506, this will not even be possible as we will no longer be transpiling public code into commonjs. Blocks elastic#66506 Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

elasticmachine · 2020-05-21T00:08:46Z

Pinging @elastic/apm-ui (Team:apm)

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley · 2020-05-21T05:18:10Z

.eslintrc.js

                errorMessage: `Plugins may only import from src/core/server and src/core/public.`,
              },
+              {
+                target: ['(src|x-pack)/plugins/*/server/**/*', '!x-pack/plugins/apm/**/*'],


APM is a less than straightforward update and I would rather not make assumptions for their desired organization. I will follow-up with an issue for the team to resolve.

I think what you are hitting is caused by the server side types consumed by the client.
Types are inferred from server-side routes and can therefore not be moved to common.

I expect we can resolve this when typescript is updated to 3.8 and we can use type only imports.

cc @dgieselaar

Introduced in elastic#66432 and I have a more thorough PR to prevent imports in server from public in elastic#67149 Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

elasticmachine · 2020-05-21T13:40:22Z

Pinging @elastic/kibana-operations (Team:Operations)

mistic

LGTM

mikecote

Alerting code LGTM

rylnd

SIEM and endpoint changes LGTM 👍

cjcenizal

@tylersmalley 👍 I'm onboard with the intention behind this change. However, one of ES UI's organizing principles is to organize code inside of __packages_do_not_import__ and then re-export from public, server, and common directories. Can we make a couple adjustments that will both follow this principle and fulfill the goal of preventing public code from being imported into server code?

Here's what I think will work:

Moving all the authorization code back into its original state inside of __packages_do_not_import__.
Changing the exports from public/authorization to be explicit. None of these modules are usable on the server, so common isn't the best place for them.

export {
  WithPrivileges,
  NotAuthorizedSection,
  AuthorizationProvider,
  AuthorizationContext,
  SectionError,
  Error,
  useAuthorizationContext,
} from '../../__packages_do_not_import__/authorization';

Changing the exports from common/index.ts to consist of the types only, since they're the only things that are usable on both client and server:

export {
  Privileges,
  MissingPrivileges,
} from '../../__packages_do_not_import__/authorization';

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

…port

cjcenizal

Code LGTM! Didn't test locally. Thanks for making the change I suggested!

nreese

maps changes LGTM
code review

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley · 2020-05-22T02:25:02Z

@elasticmachine merge upstream

tylersmalley · 2020-05-25T13:09:18Z

@elasticmachine merge upstream

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

…port

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

afharo

Usage collection changes (src/plugins/usage_collection/server/report/store_report.test.ts) LGTM!

tylersmalley · 2020-05-26T17:19:04Z

@elastic/kibana-app and @elastic/siem - can I get a codeowners review here?

rylnd · 2020-05-26T17:49:34Z

pinging @elastic/endpoint-app-team as they own the conflicting files.

lizozom

@elastic/kibana-app-arch LGTM

…port

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

kibanamachine · 2020-06-10T00:20:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: af145ca

History

💔 Build #52844 failed f172882
💚 Build #49628 succeeded 6675b80
💔 Build #49615 failed 11054f7
💔 Build #49574 failed b8ce198
💚 Build #49439 succeeded dbd0641

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

We should not be allowing importing of public into server. Any shared code should reside in a common directory. After elastic#66506, this will not even be possible as we will no longer be transpiling public code into commonjs. Blocks elastic#66506 Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

We should not be allowing importing of public into server. Any shared code should reside in a common directory. After #66506, this will not even be possible as we will no longer be transpiling public code into commonjs. Blocks #66506 Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

botelastic bot added the Team:APM - DEPRECATED Use Team:obs-ux-infra_services. label May 21, 2020

tylersmalley mentioned this pull request May 21, 2020

Move apm tutorial from apm plugin into apm_oss plugin #66432

Merged

7 tasks

mistic mentioned this pull request May 21, 2020

chore(NA): avoids imports of server code into public #67152

Merged

tylersmalley mentioned this pull request May 21, 2020

[build] Removes commonjs transforms #66506

Merged

Additional updates

9694362

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley commented May 21, 2020

View reviewed changes

tylersmalley added backport:skip This PR does not require backporting Team:Operations Kibana-Operations Team v7.8.0 v8.0.0 labels May 21, 2020

tylersmalley marked this pull request as ready for review May 21, 2020 13:40

tylersmalley requested a review from a team May 21, 2020 13:40

tylersmalley requested review from a team as code owners May 21, 2020 13:40

tylersmalley added release_note:skip Skip the PR/issue when compiling release notes and removed backport:skip This PR does not require backporting labels May 21, 2020

mistic approved these changes May 21, 2020

View reviewed changes

mikecote approved these changes May 21, 2020

View reviewed changes

rylnd approved these changes May 21, 2020

View reviewed changes

sorenlouv approved these changes May 21, 2020

View reviewed changes

cjcenizal suggested changes May 21, 2020

View reviewed changes

tylersmalley added 2 commits May 21, 2020 13:18

es_ui_shared feedback

b348579

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

es_ui_shared feedback

3826004

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Merge remote-tracking branch 'upstream/master' into prevent-public-im…

fd3f4b3

…port

tylersmalley requested a review from a team as a code owner May 21, 2020 20:41

tylersmalley mentioned this pull request May 21, 2020

[APM] Prevent imports of public in server code #67210

Closed

cjcenizal approved these changes May 21, 2020

View reviewed changes

nreese approved these changes May 21, 2020

View reviewed changes

tylersmalley added v7.9.0 and removed v7.8.0 labels May 21, 2020

Import license type from server

fafa902

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

mistic mentioned this pull request May 21, 2020

chore(NA): avoids imports of server or public code into common #67231

Merged

Merge branch 'master' into prevent-public-import

dbd0641

elasticmachine and others added 4 commits May 25, 2020 06:09

Merge branch 'master' into prevent-public-import

b8ce198

Resolves eslint error

11054f7

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Merge remote-tracking branch 'upstream/master' into prevent-public-im…

5b86b3e

…port

Prettier updates

6675b80

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley requested a review from a team as a code owner May 25, 2020 20:45

afharo approved these changes May 26, 2020

View reviewed changes

lizozom approved these changes May 31, 2020

View reviewed changes

tylersmalley added 2 commits June 9, 2020 13:06

Merge remote-tracking branch 'upstream/master' into prevent-public-im…

f172882

…port

Export Privileges/MissingPrivileges

af145ca

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley merged commit 8a5a7c3 into elastic:master Jun 10, 2020

tylersmalley mentioned this pull request Jun 10, 2020

[7.x] [eslint] Prevents importing of public code into server (#67149) #68782

Merged

Conversation

tylersmalley commented May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented May 21, 2020

Uh oh!

tylersmalley May 21, 2020

Choose a reason for hiding this comment

Uh oh!

sorenlouv May 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sorenlouv May 21, 2020

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented May 21, 2020

Uh oh!

mistic left a comment

Choose a reason for hiding this comment

Uh oh!

mikecote left a comment

Choose a reason for hiding this comment

Uh oh!

rylnd left a comment

Choose a reason for hiding this comment

Uh oh!

cjcenizal left a comment

Choose a reason for hiding this comment

Uh oh!

cjcenizal left a comment

Choose a reason for hiding this comment

Uh oh!

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented May 22, 2020

Uh oh!

tylersmalley commented May 25, 2020

Uh oh!

afharo left a comment

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented May 26, 2020

Uh oh!

rylnd commented May 26, 2020

Uh oh!

lizozom left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jun 10, 2020

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

tylersmalley commented May 21, 2020 •

edited

Loading

sorenlouv May 21, 2020 •

edited

Loading