Don't register management section if capabilities are not granted by legrego · Pull Request #90782 · elastic/kibana

legrego · 2021-02-09T14:29:55Z

Summary

Only registers the Beats CM and Logstash management sections if the user is authorized for them. Addresses a race condition where the section would be registered after the Management app had gone through and disabled sections based on capabilities.

Ideally Beats CM wouldn't register itself in an async manner (i.e., potentially during the start phase), but priorities aren't aligned with rewriting the way this application is structured.

Flaky test verification:

x-pack CI Group 2: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1279/
x-pack CI Group 7: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1276/
x-pack CI Group 8: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1277/
x-pack CI Group 9: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1278/

Resolves #90849
Resolves #90770

legrego · 2021-02-09T17:37:13Z

@elasticmachine merge upstream

azasypkin · 2021-02-10T09:20:32Z

ACK: reviewing...

azasypkin

LGTM, thanks for the quick fix!

Would you mind running a flaky test runner on the backport PR as well (if possible)? I'm a bit worried that in 7.11 I've seen both Logstash pipelines and Beats Management sections rendered when there should have been none of these sections (see gif in #90770).

One thing I noticed is that this fix made another minor problem more apparent, but since tests don't complain I'm fine with it. I believe Beats team needs to eventually implement a proper fix and register sections in setup though:

Logout
Try to go directly to https://localhost:5601/app/management
Log in as a super user and observe that Beats Management section is missing sometimes
Browser refresh fixes this problem

…ts-flakyness

… into chore/fix-beats-flakyness

legrego · 2021-02-10T11:27:22Z

One thing I noticed is that this fix made another minor problem more apparent, but since tests don't complain I'm fine with it. I believe Beats team needs to eventually implement a proper fix and register sections in setup though:

Logout

Try to go directly to https://localhost:5601/app/management

Log in as a super user and observe that Beats Management section is missing sometimes

Browser refresh fixes this problem

Great catch, thanks @azasypkin! I think I identified this, and fixed in 845b2c8 (#90782) 🤞

legrego · 2021-02-10T11:41:28Z

I commented the wrong part above. I fixed the logstash issue, not the beats issue you described

legrego · 2021-02-10T13:09:21Z

I'm re-running the flaky test runner for CI groups 7,8,9, and additionally for CI group 2, which includes the Logstash functional tests for feature controls.

legrego · 2021-02-10T14:12:42Z

@azasypkin Would you mind reviewing the latest logstash changes?
The flaky test runners completed for all test suites linked above, with no flakyness

azasypkin · 2021-02-10T15:31:04Z

x-pack/plugins/logstash/public/plugin.ts


 export class LogstashPlugin implements Plugin<void, void, SetupDeps> {
  private licenseSubscription?: Subscription;
+  private capabilities$ = new BehaviorSubject<null | Capabilities>(null);


nit: it feels like you can just use new Subject<Capabilities>() and remove null check in the subscription?

Good call, will do!

azasypkin

Still LGTM with optional nit, thanks for handling Logstash Pipelines as well!

kibanamachine · 2021-02-10T19:08:52Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 367d03e

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`beatsManagement`	538.9KB	538.7KB	-198.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`beatsManagement`	126.4KB	128.4KB	+1.9KB
`logstash`	22.2KB	22.6KB	+418.0B
total			+2.4KB

History

💛 Build #105534 was flaky d0bab51
💚 Build #105276 succeeded 4a1fc8b
💔 Build #105169 failed 74aef23

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

…astic#90782) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine · 2021-02-10T19:13:24Z

Backport result

{"level":"info","message":"POST https://api.github.com/graphql (status: 200)"}
{"level":"info","message":"POST https://api.github.com/graphql (status: 200)"}
{"meta":{"labels":["auto-backport","release_note:skip","v7.11.1","v7.12.0","v8.0.0"],"branchLabelMapping":{"^v8.0.0$":"master","^v7.12.0$":"7.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"},"existingTargetPullRequests":[]},"level":"info","message":"Inputs when calculating target branches:"}
{"meta":["7.11","7.x"],"level":"info","message":"Target branches inferred from labels:"}
{"meta":{"killed":false,"code":2,"signal":null,"cmd":"git remote rm kibanamachine","stdout":"","stderr":"error: No such remote: 'kibanamachine'\n"},"level":"info","message":"exec error 'git remote rm kibanamachine':"}
{"meta":{"killed":false,"code":2,"signal":null,"cmd":"git remote rm elastic","stdout":"","stderr":"error: No such remote: 'elastic'\n"},"level":"info","message":"exec error 'git remote rm elastic':"}
{"level":"info","message":"Backporting [{\"sourceBranch\":\"master\",\"targetBranchesFromLabels\":[\"7.11\",\"7.x\"],\"sha\":\"9778060ae81d29644b543b27bac5e3e6ce4ae82f\",\"formattedMessage\":\"Don't register management section if capabilities are not granted (#90782)\",\"originalMessage\":\"Don't register management section if capabilities are not granted (#90782)\\n\\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>\",\"pullNumber\":90782,\"existingTargetPullRequests\":[]}] to 7.11"}

Backporting to 7.11:
{"level":"info","message":"Backporting via filesystem"}
{"level":"info","message":"Creating PR with title: \"[7.11] Don't register management section if capabilities are not granted (#90782)\". kibanamachine:backport/7.11/pr-90782 -> 7.11"}
{"level":"info","message":"POST /repos/elastic/kibana/pulls - 201 in 1230ms"}
{"level":"info","message":"Adding assignees to #91006: legrego"}
{"level":"info","message":"POST /repos/elastic/kibana/issues/91006/assignees - 201 in 763ms"}
{"level":"info","message":"Adding labels: backport"}
{"level":"info","message":"POST /repos/elastic/kibana/issues/91006/labels - 200 in 581ms"}
View pull request: https://github.com/elastic/kibana/pull/91006
{"level":"info","message":"Backporting [{\"sourceBranch\":\"master\",\"targetBranchesFromLabels\":[\"7.11\",\"7.x\"],\"sha\":\"9778060ae81d29644b543b27bac5e3e6ce4ae82f\",\"formattedMessage\":\"Don't register management section if capabilities are not granted (#90782)\",\"originalMessage\":\"Don't register management section if capabilities are not granted (#90782)\\n\\nCo-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>\",\"pullNumber\":90782,\"existingTargetPullRequests\":[]}] to 7.x"}

Backporting to 7.x:
{"level":"info","message":"Backporting via filesystem"}
{"level":"info","message":"Creating PR with title: \"[7.x] Don't register management section if capabilities are not granted (#90782)\". kibanamachine:backport/7.x/pr-90782 -> 7.x"}
{"level":"info","message":"POST /repos/elastic/kibana/pulls - 201 in 1128ms"}
{"level":"info","message":"Adding assignees to #91007: legrego"}
{"level":"info","message":"POST /repos/elastic/kibana/issues/91007/assignees - 201 in 695ms"}
{"level":"info","message":"Adding labels: backport"}
{"level":"info","message":"POST /repos/elastic/kibana/issues/91007/labels - 200 in 362ms"}
View pull request: https://github.com/elastic/kibana/pull/91007

…0782) (#91006) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co>

…ed (#90782) (#91007) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Larry Gregory <larry.gregory@elastic.co>

Don't register management section if capabilities are not granted

74aef23

Merge branch 'master' into chore/fix-beats-flakyness

4a1fc8b

legrego added release_note:skip Skip the PR/issue when compiling release notes v7.11.1 v7.12.0 v8.0.0 labels Feb 10, 2021

legrego marked this pull request as ready for review February 10, 2021 02:34

legrego requested a review from azasypkin February 10, 2021 02:34

azasypkin approved these changes Feb 10, 2021

View reviewed changes

legrego added 3 commits February 10, 2021 06:25

Fix Logstash Central Pipelines flakyness

845b2c8

Merge branch 'master' of github.com:elastic/kibana into chore/fix-bea…

be815b0

…ts-flakyness

Merge branch 'chore/fix-beats-flakyness' of github.com:legrego/kibana…

d0bab51

… into chore/fix-beats-flakyness

azasypkin self-requested a review February 10, 2021 15:09

azasypkin reviewed Feb 10, 2021

View reviewed changes

azasypkin approved these changes Feb 10, 2021

View reviewed changes

switch to Subject instead of BehaviorSubject

367d03e

legrego added the auto-backport Deprecated - use backport:version if exact versions are needed label Feb 10, 2021

legrego merged commit 9778060 into elastic:master Feb 10, 2021

legrego deleted the chore/fix-beats-flakyness branch February 10, 2021 19:10

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Feb 10, 2021

Don't register management section if capabilities are not granted (el…

d444ed3

…astic#90782) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine mentioned this pull request Feb 10, 2021

Don't register management section if capabilities are not granted (#90782) #91006

Merged

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Feb 10, 2021

Don't register management section if capabilities are not granted (el…

3bc5140

…astic#90782) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

kibanamachine mentioned this pull request Feb 10, 2021

[7.x] Don't register management section if capabilities are not granted (#90782) #91007

Merged

azasypkin mentioned this pull request Feb 11, 2021

[ML] Transform functional tests - stabilize feature controls test #90772

Closed

pheyos mentioned this pull request Feb 11, 2021

[ML] Transform functional tests - re-enable feature controls suite #91095

Merged

Dosant mentioned this pull request Feb 15, 2021

Management app shows the sections that are not allowed by the current user capabilities #90770

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't register management section if capabilities are not granted#90782

Don't register management section if capabilities are not granted#90782
legrego merged 6 commits intoelastic:masterfrom
legrego:chore/fix-beats-flakyness

legrego commented Feb 9, 2021 •

edited

Loading

Uh oh!

legrego commented Feb 9, 2021

Uh oh!

azasypkin commented Feb 10, 2021

Uh oh!

azasypkin left a comment

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

azasypkin Feb 10, 2021

Uh oh!

legrego Feb 10, 2021

Uh oh!

azasypkin left a comment

Uh oh!

kibanamachine commented Feb 10, 2021

Uh oh!

kibanamachine commented Feb 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

legrego commented Feb 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

legrego commented Feb 9, 2021

Uh oh!

azasypkin commented Feb 10, 2021

Uh oh!

azasypkin left a comment

Choose a reason for hiding this comment

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

legrego commented Feb 10, 2021

Uh oh!

azasypkin Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

legrego Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

azasypkin left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Feb 10, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

kibanamachine commented Feb 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

legrego commented Feb 9, 2021 •

edited

Loading