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

[Reporting] Fix check for security and added jest test #109429

Merged
merged 2 commits into from
Aug 20, 2021

Conversation

jloleysens
Copy link
Contributor

Summary

Per the issue reported here: #108774. This contribution fixes a check for whether security is available and adds a test to confirm the expected behaviour. This brings the check in line with other places in Kibana like:

isSecurityEnabled: () => security !== undefined && security.license.isEnabled(),

How to test

This is a little bit tricky to test, since the code for throwing the ES error is only on 7.x branches and relates to a very specific state of licensing (trial without security enabled).

On the reported issue (#108774), this can be tested by downloading the linked snapshots and editing the javascript directly (adding the security.license.isEnabled() to the if statement).

  1. Start Kibana & ES
  2. Ensure that trial license is active
  3. Navigate to the Reporting Management UI
  4. Check the Kibana terminal for Internal Server Error (should be clear with the fix applied)

Checklist

Delete any items that are not applicable to this PR.

@jloleysens jloleysens added (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Reporting Services v7.15.0 labels Aug 20, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

@jloleysens jloleysens enabled auto-merge (squash) August 20, 2021 12:02
Copy link
Contributor

@dokmic dokmic left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Co-authored-by: Michael Dokolin <dokmic@gmail.com>
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@jloleysens jloleysens merged commit 132f55d into elastic:master Aug 20, 2021
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 24, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 109429 or prevent reminders by adding the backport:skip label.

@jloleysens jloleysens added auto-backport Deprecated - use backport:version if exact versions are needed and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Aug 25, 2021
@kibanamachine
Copy link
Contributor

The following labels were identified as gaps in your version labels and will be added automatically:

  • v7.16.0

If any of these should not be on your pull request, please manually remove them.

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.15
7.x

The backport PRs will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Aug 25, 2021
Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>
kibanamachine added a commit that referenced this pull request Aug 25, 2021
Co-authored-by: Jean-Louis Leysens <jloleysens@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes v7.15.0 v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants