Skip to content

[Monitoring] Fix the messaging around needing TLS enabled#72310

Merged
chrisronline merged 6 commits intoelastic:masterfrom
chrisronline:monitoring/fixed_copy
Jul 21, 2020
Merged

[Monitoring] Fix the messaging around needing TLS enabled#72310
chrisronline merged 6 commits intoelastic:masterfrom
chrisronline:monitoring/fixed_copy

Conversation

@chrisronline
Copy link
Copy Markdown
Contributor

@chrisronline chrisronline commented Jul 17, 2020 •
edited
Loading

Relates to #71846

Updates the copy per this comment

I changed it slightly and made it so we show the same error message in all error scenarios.

cc @lcawl @gchaps

@chrisronline chrisronline requested review from a team and igoristic July 17, 2020 17:03
@chrisronline chrisronline self-assigned this Jul 17, 2020
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 17, 2020

Pinging @elastic/stack-monitoring (Team:Monitoring)

@chrisronline chrisronline mentioned this pull request Jul 17, 2020
Merged
igoristic
igoristic reviewed Jul 20, 2020
View reviewed changes
x-pack/plugins/monitoring/public/alerts/lib/security_toasts.tsx Outdated
} else if (!hasPermanentEncryptionKey) {
showEncryptionError();
}
showTlsAndEncryptionError();
Copy link
Copy Markdown
Contributor

@igoristic igoristic Jul 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should still check for these cases:

Suggested change
showTlsAndEncryptionError();
if (!isSufficientlySecure || !hasPermanentEncryptionKey) {
showTlsAndEncryptionError();
}

igoristic
igoristic reviewed Jul 20, 2020
View reviewed changes
x-pack/plugins/monitoring/public/alerts/lib/security_toasts.tsx Outdated
{i18n.translate('xpack.monitoring.healthCheck.tlsAndEncryptionError', {
defaultMessage: `You must enable Transport Layer Security between Kibana and Elasticsearch
and configure an encryption key in your kibana.yml file to use the Alerting feature.`,
defaultMessage: `Stack monitoring alerts require Transport Layer Security between Kibana and Elasticsearch and an encryption key in your kibana.yml file.`,
Copy link
Copy Markdown
Contributor

@igoristic igoristic Jul 20, 2020

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: `Stack monitoring alerts require Transport Layer Security between Kibana and Elasticsearch and an encryption key in your kibana.yml file.`,
defaultMessage: `Stack monitoring alerts require Transport Layer Security between Kibana and Elasticsearch, and an encryption key in your kibana.yml file.`,

igoristic
igoristic approved these changes Jul 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

I think this looks good 👍

Added a couple of optional suggestions though

gchaps
gchaps approved these changes Jul 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

LGTM

@chrisronline chrisronline requested a review from igoristic July 20, 2020 17:49
@chrisronline
Copy link
Copy Markdown
Contributor Author

chrisronline commented Jul 20, 2020

@igoristic Made those changes. Do you mind double checking this?

igoristic
igoristic approved these changes Jul 20, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

Looking good now 👍

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jul 20, 2020

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
monitoring 1.1MB -3.0KB 1.1MB

History

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

@chrisronline chrisronline merged commit c63ab91 into elastic:master Jul 21, 2020
@chrisronline chrisronline deleted the monitoring/fixed_copy branch July 21, 2020 13:12
@chrisronline chrisronline mentioned this pull request Jul 21, 2020
Merged
chrisronline added a commit that referenced this pull request Jul 21, 2020
@chrisronline
[Monitoring] Fix the messaging around needing TLS enabled (#72310) (#…
d00c09e 
…72627)

* Fix the copy

* Fix type issues

* PR feedback

* Add missing code
@chrisronline
Copy link
Copy Markdown
Contributor Author

chrisronline commented Jul 21, 2020

Backport:

7.x: d00c09e

gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 21, 2020
@gmmorris
Merge branch 'master' into alerting/consumer-based-rbac
53aa8e9 
* master:
  [Security Solution][Timeline] Add Empty view to the Timelines page (elastic#72576)
  [Security Solution][Resolver] Show process detail panel when clicking a process node (elastic#72563)
  Move manifest packageConfig mocks into security_solution plugin (elastic#72527)
  [QA][Code Coverage] Fixup Team Assignment (elastic#72467)
  [docs] remove references to tile map visualization in supported aggregations (elastic#72493)
  [ci][apm-ui] fix argument name for disabling pr comments (elastic#72633)
  Only check that the event ids are the same in arrays (elastic#72624)
  Add doc titles to ES UI apps (elastic#71045)
  Add Upgrade Assistant API integration test to ensure the reindex operation saved object can handle immense error messages (elastic#72347)
  [APM] Disable flaky rum e2e’s (elastic#72614)
  Applying tiny fix from 72532 to main branch (elastic#72533)
  [APM] Update script with new roles/users (elastic#72599)
  [Security Solution] Add margin (elastic#72542)
  Migrated fixed_scroll karma tests to jest (elastic#72258)
  [ML] Handling data recognizer saved object errors (elastic#72447)
  [Monitoring] Fix the messaging around needing TLS enabled (elastic#72310)
  [Task Manager] Batches the update operations in Task Manager  (elastic#71470)
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 22, 2020
@gmmorris
Merge branch 'alerting/consumer-based-rbac' into alerting/management-…
fb1d793 
…feature-privileges

* alerting/consumer-based-rbac:
  [Security Solution][Timeline] Add Empty view to the Timelines page (elastic#72576)
  [Security Solution][Resolver] Show process detail panel when clicking a process node (elastic#72563)
  renamed variable to make it clear the SO client is unsecured
  Move manifest packageConfig mocks into security_solution plugin (elastic#72527)
  [QA][Code Coverage] Fixup Team Assignment (elastic#72467)
  [docs] remove references to tile map visualization in supported aggregations (elastic#72493)
  [ci][apm-ui] fix argument name for disabling pr comments (elastic#72633)
  Only check that the event ids are the same in arrays (elastic#72624)
  includes hidden params type in SO client
  Add doc titles to ES UI apps (elastic#71045)
  Add Upgrade Assistant API integration test to ensure the reindex operation saved object can handle immense error messages (elastic#72347)
  [APM] Disable flaky rum e2e’s (elastic#72614)
  Applying tiny fix from 72532 to main branch (elastic#72533)
  [APM] Update script with new roles/users (elastic#72599)
  [Security Solution] Add margin (elastic#72542)
  Migrated fixed_scroll karma tests to jest (elastic#72258)
  [ML] Handling data recognizer saved object errors (elastic#72447)
  [Monitoring] Fix the messaging around needing TLS enabled (elastic#72310)
  [Task Manager] Batches the update operations in Task Manager  (elastic#71470)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@igoristic igoristic igoristic approved these changes

@gchaps gchaps gchaps approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@chrisronline chrisronline

Labels

release_note:enhancement review Team:Monitoring Stack Monitoring team v7.10.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@chrisronline @elasticmachine @kibanamachine @igoristic @gchaps