[7.x] Improve upgrade assistance for the legacy audit logger #114995
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
legrego
added
backport:skip
legrego
added
the
Team:Security
|
Pinging @elastic/kibana-security (Team:Security) |
jportner
reviewed
Oct 14, 2021
There was a problem hiding this comment.
Good call differentiating this for Cloud and non-Cloud users!
You didn't change this section of the code so I can't comment on it directly, but what do you think about changing the non-Cloud manual steps to something like this?
The new ECS-compliant audit logger will be automatically enabled after upgrading to 8.0. You can optionally enable the new audit logger before upgrading by configuring an appender with "xpack.security.audit.appender". Learn more
Edit: looks like message can only be a string, bummer. I wonder if it would render to a clickable link if you just included the URL? probably not...
I have some other comments below!
Comment on lines
+37
to
+39
| // Gross, but the cloud plugin depends on the security plugin already, | ||
| // so we can't add a dependency in the other direction to check this in a more conventional manner. | ||
| const isCloudInstance = typeof settings?.xpack?.cloud?.id === 'string'; |
|
|
||
| const message = i18n.translate('xpack.security.deprecations.auditLoggerMessage', { | ||
| defaultMessage: | ||
| 'The legacy audit logger is deprecated in favor of the new ECS-compliant audit logger.', |
There was a problem hiding this comment.
I'm thinking we should change this a bit:
Suggested change
| 'The legacy audit logger is deprecated in favor of the new ECS-compliant audit logger.', | |
| 'Use the new ECS-compliant audit logger. The legacy audit logger will be removed in 8.0.', |
but I'm interested to see what Gail and Deb think.
| 'The legacy audit logger is deprecated in favor of the new ECS-compliant audit logger.', | ||
| }); | ||
|
|
||
| const documentationUrl = `https://www.elastic.co/guide/en/kibana/${branch}/security-settings-kb.html#audit-logging-settings`; |
There was a problem hiding this comment.
The current link directs the user to the "Audit logging settings" page. But you have to scroll down quite a bit before you find where it mentions the legacy audit logger is deprecated (and even then it is not very noticeable).
I'm a bit on the fence about this, but it might be better to direct user to our "Audit logs" page.
That clearly describes the two different types of loggers and it might be a bit more appropriate for users to learn more about the deprecation.
Suggested change
| const documentationUrl = `https://www.elastic.co/guide/en/kibana/${branch}/security-settings-kb.html#audit-logging-settings`; | |
| const documentationUrl = `https://www.elastic.co/guide/en/kibana/${branch}/xpack-security-audit-logging.html`; |
There was a problem hiding this comment.
I like this suggestion! I'll hold off on making changes until the docs team has a chance to review
|
@gchaps and I chatted and we thought this text would be good: (ON-PREM / NON-CLOUD) The legacy audit logger is deprecated Use the new ECS-compliant audit logger. The legacy audit logger will be removed in 8.0. Fix manually
(CLOUD) The legacy audit logger is deprecated Use the new ECS-compliant audit logger. The legacy audit logger will be removed in 8.0. Fix manually
(Both are identical except Step 1) |
|
Thanks for the suggestions @jportner & @gchaps. Text was updated in I also downgraded the level of the on-prem notice from |
|
@jportner and I chatted this morning and we need to remove the line "The legacy audit logger will be removed in 8.0." to meet the deprecation guidelines. So the messages should be: (ON-PREM / NON-CLOUD) The legacy audit logger is deprecated Use the new ECS-compliant audit logger. Fix manually
(CLOUD) The legacy audit logger is deprecated Use the new ECS-compliant audit logger. Fix manually
|
|
@elasticmachine merge upstream |
jportner
approved these changes
Oct 18, 2021
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / X-Pack Detection Engine API Integration Tests.x-pack/test/detection_engine_api_integration/security_and_spaces/tests/exception_operators_data_types/text·ts.detection engine api security and spaces enabled Detection exceptions data types and operators Rule exception operators for data type text "is not" operator should filter all words using a common piece of textStandard OutStack TraceMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Improves the assistance for migrating away from the legacy audit logger on Cloud installations. The existing assistance was aimed at on-prem installations, and would have been both incorrect and confusing on Cloud.
ExistingRevised on-prem assistanceNew Cloud assistance