Skip to content

Remove index audit output type#37707

Merged
albertzaharovits merged 14 commits intoelastic:masterfrom
albertzaharovits:remove_audit_index
Jan 24, 2019
Merged

Remove index audit output type#37707
albertzaharovits merged 14 commits intoelastic:masterfrom
albertzaharovits:remove_audit_index

Conversation

@albertzaharovits
Copy link
Copy Markdown
Contributor

@albertzaharovits albertzaharovits commented Jan 22, 2019

Supersedes #37301

This PR removes the Index Audit Output type, following its deprecation in 6.7 by #37671 . It also adds the migration notice (settings notice).

Closes #29881

@albertzaharovits albertzaharovits self-assigned this Jan 22, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Jan 22, 2019

Pinging @elastic/es-security

jaymode
jaymode requested changes Jan 22, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

nice stats. I left some comments

docs/reference/migration/migrate_7_0/settings.asciidoc Outdated
These settings enabled and configured the audit index output type. This output
type has been removed because it was unreliable in certain scenarios and this
could have lead to dropping audit events while the operations on the system
were allowed to continue as usual. This is a terrible failure state for an
Copy link
Copy Markdown
Member

@jaymode jaymode Jan 22, 2019

Choose a reason for hiding this comment

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

Can we leave out This is a terrible failure state for an audit system which brought about its demise. ?

docs/reference/settings/audit-settings.asciidoc Outdated
information, but it uses the older (pre-6.5.0) formatting style.
If the backwards compatible format is not required, it should be disabled.
To do that, change its logger level to `off` in the `log4j2.properties` file.
For backwards compatibility reasons, a `<clustername>_access.log` file is also
Copy link
Copy Markdown
Member

@jaymode jaymode Jan 22, 2019

Choose a reason for hiding this comment

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

These are master docs, so I think this doesn't apply anymore?

Copy link
Copy Markdown
Contributor Author

@albertzaharovits albertzaharovits Jan 22, 2019

Choose a reason for hiding this comment

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

You're right 🙂 At first, this made me believe I still had a PR to do to remove the bwc audit format, that's how much I trust our docs!

jaymode and others added 4 commits January 22, 2019 17:44
@jaymode @albertzaharovits
Apply suggestions from code Jay's review
edbf2c7 
Co-Authored-By: albertzaharovits <albert.zaharovits@gmail.com>
@jaymode @albertzaharovits
Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…
c9b6d69 
…ecurity/Security.java

Co-Authored-By: albertzaharovits <albert.zaharovits@gmail.com>
@albertzaharovits
Address review
87b7d88
@albertzaharovits
Feedback
8219937
@albertzaharovits
Copy link
Copy Markdown
Contributor Author

albertzaharovits commented Jan 22, 2019 •
edited
Loading

nice stats. I left some comments

++ 😀

Thanks @jaymode ! I've addressed your comments.

jaymode
jaymode approved these changes Jan 22, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

I left two minor comments. Otherwise LGTM

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

albertzaharovits commented Jan 23, 2019

@elasticmachine run elasticsearch-ci/1

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

albertzaharovits commented Jan 23, 2019

@elasticmachine run elasticsearch-ci/2

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

albertzaharovits commented Jan 23, 2019

18:58:42 FAILURE: Build failed with an exception.
18:58:42
18:58:42 * Where:
18:58:42 Build file '/tmp/junit14158968466601980319/build.gradle' line: 48
18:58:42
18:58:42 * What went wrong:
18:58:42 A problem occurred evaluating root project 'junit14158968466601980319'.
18:58:42 > Could not get unknown property 'unitTest' for root project 'junit14158968466601980319' of type org.gradle.api.Project.

@elasticmachine run elasticsearch-ci/1

@albertzaharovits
Copy link
Copy Markdown
Contributor Author

albertzaharovits commented Jan 23, 2019

@elasticmachine run elasticsearch-ci/packaging-sample

tvernum
tvernum approved these changes Jan 23, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM.
We could probably do a followup to simplify/remove AuditTrail etc, but that's not needed here.

@albertzaharovits albertzaharovits merged commit b6936e3 into elastic:master Jan 24, 2019
@albertzaharovits albertzaharovits mentioned this pull request Jan 24, 2019
Closed
@albertzaharovits albertzaharovits deleted the remove_audit_index branch January 24, 2019 10:44
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 24, 2019
@jasontedor
Merge remote-tracking branch 'elastic/master' into sync-retention-leases
1cb71ec 
* elastic/master:
  Optimize warning header de-duplication (elastic#37725)
  Bubble exceptions up in ClusterApplierService (elastic#37729)
  SQL: Improve handling of invalid args for PERCENTILE/PERCENTILE_RANK (elastic#37803)
  Remove unused ThreadBarrier class (elastic#37666)
  Add built-in user and role for code plugin (elastic#37030)
  Consolidate testclusters tests into a single project (elastic#37362)
  Fix docs for MappingUpdatedAction
  SQL: Introduce SQL DATE data type (elastic#37693)
  disabling bwc test while backporting elastic#37639
  Mute ClusterDisruptionIT testAckedIndexing
  Set acking timeout to 0 on dynamic mapping update (elastic#31140)
  Remove index audit output type (elastic#37707)
  Mute FollowerFailOverIT testReadRequestsReturnsLatestMappingVersion
  [ML] Increase close job timeout and lower the max number (elastic#37770)
  Remove Custom Listeners from SnapshotsService (elastic#37629)
  Use m_m_nodes from Zen1 master for Zen2 bootstrap (elastic#37701)
  Fix index filtering in follow info api. (elastic#37752)
  Use project dependency instead of substitutions for distributions (elastic#37730)
  Update authenticate to allow unknown fields (elastic#37713)
  Deprecate HLRC EmptyResponse used by security (elastic#37540)
@albertzaharovits albertzaharovits mentioned this pull request Feb 27, 2023
Merged
albertzaharovits added a commit that referenced this pull request Feb 27, 2023
@albertzaharovits
Remove dead code related to auditing (#94114)
7294508 
This PR removes:
 * the CompositeAuditTrail which is designed to fan-out
auditing events to multiple logger implementation types.
This is not needed because since v7.0 there's only one
audit logger implementation, the logfile.
 * any traces of the index-based logger implementation,
namely the permission of the internal _xpack user to
read the audit log index.

Related: #37707
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tvernum tvernum tvernum approved these changes

@jaymode jaymode jaymode approved these changes

@lcawl lcawl Awaiting requested review from lcawl

Assignees

@albertzaharovits albertzaharovits

Labels

>breaking :Security/Audit X-Pack Audit logging v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@albertzaharovits @elasticmachine @tvernum @jaymode @colings86